Enabling MPE mode while a pitch bend is active causes the pitch bend to stick forever

[selenologist]

Bug Description:
Enabling MPE mode while a pitch bend is active causes Surge to remain detuned, because in MPE mode pitch bend is handled differently. Surge will remain detuned.

Surge XT Version
This information is found on the About screen, which you get to from the bottom right menu.

Version: Surge XT 1.1.HEAD.beab6b17 (filter lib wasn't clean - rest of build is)
Build: 2022-07-26 @ 15:56:33 on 'rainbow/local' with 'Clang-14.0.6' using JUCE 6.1.6
System: Linux 64-bit VST3 on AMD Ryzen 7 2700X Eight-Core Processor
Host: Ardour @ 48.0 kHz

Reproduction Steps:
Steps to reproduce the behavior:

1. While MPE mode is disabled, hold a note and apply pitch bend.
2. While the note is held and pitch bend is applied, enable MPE mode.
3. You can now stop doing pitch bend/
4. Surge is still detuned?

Expected Behavior:
Maybe when we toggle MPE mode we should reset pitch bend.

[baconpaul]

I think this also is partly dependent on which channel your bend is on when you toggle MPE mode. Surge will respond to global channel pitch bend distinctly from MPE bend but some info on midi state which did this would be helpful

[baconpaul]

Heya @selenologist

Well this doesn't happen on macos

Can you give me some more details of the channels and midi messages? Or capture a midi file where it happens if you activate MPE half way through?

[selenologist]

Aha, I see now - I forgot to mention the pitch bend has to happen on a channel other than channel 1, because we handle channel 1 pitch bend the same in MPE mode so the reset to zero will still work in MPE mode.

Here while MPE mode was disabled I dragged from a C to D-ish on my linnstrument which was using channel 2, then turned on MPE mode and released the note. Then I played another note on channel 3 that shouldn't have had any bend applied to it.

[baconpaul]

Right I see why this is happening.

Basically: SurgeSynthesizer::pitchBend does this

if (mpe && channel != 0)
   set voice bend 
else
   set global bend

which means if you are not in MPE mode and send in a bend on channel 3, then turn on MPE, the global bend will still be set and releasing the channel 3 bend won't change it. Essentially "all bends act like global bends" switches from true to false

The problem is we don't know where the pre-toggle bend came from. That is, if you sent a bend on channel 0 and then turned it on, you want that bend to stick.

So fixing this properly is a wee bit hairy.

Fixing it less properly (namely, when you toggle MPE your global pitch bend and all channel state pitch bend goes to zero) is easier but will lead to other play problems. Also we don't have a single code point to toggle MPE - we update the atomic directly in a few spots. So even going that route requires a bit of a refactor.

If you do get stuck in this way, send a pitch bend on channel 0 of 0 will fix your system.

So with all of that explained, I'm going to bump this out of 1.1.1; it's an edge case and its tricky to fix and the fix interacts with quite a bit of the rest of the code. Not a 'first point' thing. but now i see why it happens.

Thanks!

[mkruselj]

Maybe we could just all-sound-off when toggling MPE on, IF we have any voices active? Not terrific, but...

[baconpaul]

So doing that is the similar work as fixing the bend, because i have to find all the places that toggle and make a little API for it. But yeah that actually makes sense to me. The voices mean something totally different and their state will be all wrong anyway.

[mkruselj]

@baconpaul I have fixed this, somehow. This seems to work just fine!

void SurgeGUIEditor::toggleMPE()
{
    this->synth->mpeEnabled = !this->synth->mpeEnabled;

    if (statusMPE)
    {
        synth->storage.pitch_bend = 0.f;
        synth->pitchbendMIDIVal = 0;

        for (int i = 0; i < 16; i++)
        {
            synth->channelState[i].pitchBend = 0;
        }

        for (int sc = 0; sc < n_scenes; sc++)
        {
            ((ControllerModulationSource *)synth->storage.getPatch()
                 .scene[sc]
                 .modsources[ms_pitchbend])
                ->set_target(synth->storage.pitch_bend);
        }

        statusMPE->setValue(this->synth->mpeEnabled ? 1 : 0);
        statusMPE->asJuceComponent()->repaint();
    }

    synth->refresh_editor = true;
}

[baconpaul]

oh clever
lets add a method called "resetPitchBend" which does that and then call it there for clarity
Also that set_target on the controller may want to be instant so call init() rather than set_target() but i could make the case for either.
would happily merge such a change.