Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copying and pasting wavetable oscillators sometimes does not work properly #1060

Closed
VincyZed opened this issue Aug 20, 2019 · 13 comments · Fixed by #1061 or #1068
Closed

Copying and pasting wavetable oscillators sometimes does not work properly #1060

VincyZed opened this issue Aug 20, 2019 · 13 comments · Fixed by #1061 or #1068
Milestone

Comments

@VincyZed
Copy link
Collaborator

VincyZed commented Aug 20, 2019

Bug Description
When pasting certain wavetables in an oscillator after copying them from another oscillator, the result is not the same as the first one.

  • Surge Version: Latest Nightly
  • Plugin type: VST3
  • 64 bits

To Reproduce
Steps to reproduce the behavior:

  1. Open an instance of Surge
  2. Set the type of the first OSC to Wavetable
  3. For example, select the wavetable "bells" under the "rhythmic" category
  4. Morph through the wavetable to see expected result
  5. Copy the first oscillator (right-click on the "1" button at the top of the display)
  6. Paste it on OSC 2 (right-click on the "2" button at the top of the display)
  7. Go on the second OSC and morph through the wavetable. It is not the same content as the first OSC.

Expected behavior
It should behave like the first OSC we copied the data from.

Screenshots

OSC 1 (the one we copied from)

image

OSC 2 (the one we pasted data from 1)

image

Desktop config:

  • OS: Windows 10 x64
  • Host FL Studio 20.5.1
@baconpaul baconpaul added this to the 1.6.2 milestone Aug 20, 2019
@baconpaul
Copy link
Collaborator

I've confirmed this is a bug in 1.6.1.1 as well; which is good. It means my .wav file changes didn't introduce it. But also is bad. It means I have more code to look at to figure it out :)

Thanks for the accurate bug report.

@baconpaul
Copy link
Collaborator

baconpaul commented Aug 20, 2019

Oh geez - it's super clear in the code

SurgeStorage::clipboard_paste in the cp_osc block just ignores the wavetable on the clipboard. Should be an easy fix.

Well that was wrong! Will keep notes to self in a more private spot until this is fixed ;)

@baconpaul
Copy link
Collaborator

OK this bug was introduced between 1.6.0 beta-9 and 1.6.1.1

Looking at the "copy" code I think it is that the "copy" code doesn't deal with dynamic wavetables properly.

baconpaul added a commit to baconpaul/surge that referenced this issue Aug 20, 2019
With the introduction of Dynamic WTs, Wavetable Copy was broken
meaning Oscillator Copy and Paste on wavetables was broken.
This fix makes copy follow the dynamic code path correctly.

Closes surge-synthesizer#1060
baconpaul added a commit that referenced this issue Aug 20, 2019
With the introduction of Dynamic WTs, Wavetable Copy was broken
meaning Oscillator Copy and Paste on wavetables was broken.
This fix makes copy follow the dynamic code path correctly.

Closes #1060
@VincyZed
Copy link
Collaborator Author

VincyZed commented Aug 20, 2019

Confirmed, it works now!

The only thing is that the name of the selected wavetable from the copied OSC does not appear after pasting, instead it says the generic "(Patch Wavetable)" name. I don't know if this is related to this issue or even fixable, since I just realized that Cloning an instance of Surge in FL Studio with a wavetable OSC loaded gives the same result; the new instance of surge does not have the name of the wavetable either.

@baconpaul
Copy link
Collaborator

Yeah that’s right; cloning makes an in memory copy and loses affiliation with the on-disk image. Just like if you save your state in the DAW and then re-open it.

I’ve occasionally thought about fixing this (like store an ‘original name’) but that’s a bit deceptive. It is a wierd behavior though I agree.

@baconpaul
Copy link
Collaborator

(I think the fundamental reason for this - that surge stores the contents of the wavetable in the patch rather than the reference - is absolutely correct behavior. It’s just the display name issue)

@VincyZed
Copy link
Collaborator Author

OK, sounds good, not really important after all!

baconpaul added a commit to baconpaul/surge that referenced this issue Aug 20, 2019
When fixing the unison for tuning, I divided where I should have
multiplied. This restores the untuned unison to the 1.6.1.1 behavior
and still works coherently with tuning.

All parth of surge-synthesizer#1060
baconpaul added a commit that referenced this issue Aug 20, 2019
When fixing the unison for tuning, I divided where I should have
multiplied. This restores the untuned unison to the 1.6.1.1 behavior
and still works coherently with tuning.

All parth of #1060
@VincyZed
Copy link
Collaborator Author

Copying and pasting oscillators now always pastes OSC 1 to whatever destination you paste to. Probably a side-effect to the fix above.

@VincyZed
Copy link
Collaborator Author

VincyZed commented Aug 20, 2019

Copying (with modulation) does seem to work fine though.

@baconpaul baconpaul reopened this Aug 20, 2019
@baconpaul
Copy link
Collaborator

Huh the fix above wouldn’t do that but could easily be another bug! I’ll look

@baconpaul
Copy link
Collaborator

can reproduce though. Will look!

@baconpaul
Copy link
Collaborator

        char txt[256];
         sprintf(txt, "Osc %i", a + 1);
         contextMenu->addEntry(txt, eid++);
         contextMenu->addEntry("-", eid++);
         addCallbackMenu(contextMenu, "Copy",
                         [this]() { synth->storage.clipboard_copy(cp_osc, current_scene, 0); });
         eid++;

         addCallbackMenu(contextMenu, "Copy (with modulation)", [this, a]() {
            synth->storage.clipboard_copy(cp_oscmod, current_scene, a);
         });
         eid++;

Can you spot the bug? (I can!)

baconpaul added a commit to baconpaul/surge that referenced this issue Aug 20, 2019
Way back in 1.6.0 beta 5 we introduced this bug that always copied
oscillator 0.

Closes surge-synthesizer#1060
@baconpaul
Copy link
Collaborator

Anyway, pull request in

baconpaul added a commit that referenced this issue Aug 20, 2019
Way back in 1.6.0 beta 5 we introduced this bug that always copied
oscillator 0.

Closes #1060
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants