Move the keyswitch logic in the synth rather than the regions

src/sfizz/Region.cpp

@@ -1588,20 +1588,11 @@ bool sfz::Region::registerNoteOn(int noteNumber, float velocity, float randValue
 {
     ASSERT(velocity >= 0.0f && velocity <= 1.0f);
 
-    if (keyswitchDown && *keyswitchDown == noteNumber)
-        keySwitched = true;
-
-    if (keyswitchUp && *keyswitchUp == noteNumber)
-        keySwitched = false;
-
     const bool keyOk = keyRange.containsWithEnd(noteNumber);
     if (keyOk) {
         // Sequence activation
         sequenceSwitched =
             ((sequenceCounter++ % sequenceLength) == sequencePosition - 1);
-
-        if (previousNote)
-            previousKeySwitched = (*previousNote == noteNumber);
     }
 
     if (!isSwitchedOn())
@@ -1610,9 +1601,6 @@ bool sfz::Region::registerNoteOn(int noteNumber, float velocity, float randValue
     if (!triggerOnNote)
         return false;
 
-    if (previousNote && !(previousKeySwitched && noteNumber != *previousNote))
-        return false;
-
     const bool velOk = velocityRange.containsWithEnd(velocity);
     const bool randOk = randRange.contains(randValue) || (randValue == 1.0f && randRange.getEnd() == 1.0f);
     const bool firstLegatoNote = (trigger == SfzTrigger::first && midiState.getActiveNotes() == 1);
@@ -1626,12 +1614,6 @@ bool sfz::Region::registerNoteOff(int noteNumber, float velocity, float randValu
 {
     ASSERT(velocity >= 0.0f && velocity <= 1.0f);
 
-    if (keyswitchDown && *keyswitchDown == noteNumber)
-        keySwitched = false;
-
-    if (keyswitchUp && *keyswitchUp == noteNumber)
-        keySwitched = true;
-
     if (!isSwitchedOn())
         return false;
 

src/sfizz/Resources.h

@@ -33,8 +33,6 @@ struct Resources
     absl::optional<StretchTuning> stretch;
     ModMatrix modMatrix;
 
-    std::vector<uint8_t> keyswitches;
-
     void setSampleRate(float samplerate)
     {
         midiState.setSampleRate(samplerate);
@@ -56,7 +54,6 @@ struct Resources
         logger.clear();
         midiState.reset();
         modMatrix.clear();
-        keyswitches.clear();
     }
 };
 }

src/sfizz/Synth.cpp

@@ -175,11 +175,17 @@ void sfz::Synth::buildRegion(const std::vector<Opcode>& regionOpcodes)
     if (octaveOffset != 0 || noteOffset != 0)
         lastRegion->offsetAllKeys(octaveOffset * 12 + noteOffset);
 
-    if (lastRegion->keyswitch) {
+    if (lastRegion->keyswitch)
-        auto it = absl::c_find(resources.keyswitches, *lastRegion->keyswitch);
+        lastKeyswitchLists[*lastRegion->keyswitch].push_back(lastRegion.get());
-        if (it == resources.keyswitches.end())
+
-            resources.keyswitches.push_back(*lastRegion->keyswitch);
+    if (lastRegion->keyswitchUp)
-    }
+        upKeyswitchLists[*lastRegion->keyswitchUp].push_back(lastRegion.get());
+
+    if (lastRegion->keyswitchDown)
+        downKeyswitchLists[*lastRegion->keyswitchDown].push_back(lastRegion.get());
+
+    if (lastRegion->previousNote)
+        previousKeyswitchLists.push_back(lastRegion.get());
 
     // There was a combination of group= and polyphony= on a region, so set the group polyphony
     if (lastRegion->group != Default::group && lastRegion->polyphony != config::maxVoices)
@@ -203,10 +209,17 @@ void sfz::Synth::clear()
 
     for (auto& voice : voices)
         voice->reset();
+    for (auto& list : lastKeyswitchLists)
+        list.clear();
+    for (auto& list : downKeyswitchLists)
+        list.clear();
+    for (auto& list : upKeyswitchLists)
+        list.clear();
     for (auto& list : noteActivationLists)
         list.clear();
     for (auto& list : ccActivationLists)
         list.clear();
+    previousKeyswitchLists.clear();
 
     currentSet = nullptr;
     sets.clear();
@@ -220,7 +233,7 @@ void sfz::Synth::clear()
     resources.clear();
     numGroups = 0;
     numMasters = 0;
-    defaultSwitch = absl::nullopt;
+    currentSwitch = absl::nullopt;
     defaultPath = "";
     resources.midiState.reset();
     resources.filePool.clear();
@@ -262,7 +275,7 @@ void sfz::Synth::handleMasterOpcodes(const std::vector<Opcode>& members)
                 currentSet->setPolyphonyLimit(*value);
             break;
         case hash("sw_default"):
-            setValueFromOpcode(member, defaultSwitch, Default::keyRange);
+            setValueFromOpcode(member, currentSwitch, Default::keyRange);
             break;
         }
     }
@@ -280,7 +293,7 @@ void sfz::Synth::handleGlobalOpcodes(const std::vector<Opcode>& members)
                 currentSet->setPolyphonyLimit(*value);
             break;
         case hash("sw_default"):
-            setValueFromOpcode(member, defaultSwitch, Default::keyRange);
+            setValueFromOpcode(member, currentSwitch, Default::keyRange);
             break;
         case hash("volume"):
             // FIXME : Probably best not to mess with this and let the host control the volume
@@ -306,7 +319,7 @@ void sfz::Synth::handleGroupOpcodes(const std::vector<Opcode>& members, const st
             setValueFromOpcode(member, maxPolyphony, Default::polyphonyRange);
             break;
         case hash("sw_default"):
-            setValueFromOpcode(member, defaultSwitch, Default::keyRange);
+            setValueFromOpcode(member, currentSwitch, Default::keyRange);
             break;
         }
     };
@@ -535,7 +548,6 @@ void sfz::Synth::finalizeSfzLoad()
     bool haveFilterEG { false };
 
     FlexEGs::clearUnusedCurves();
-    absl::c_sort(resources.keyswitches);
 
     while (currentRegionIndex < currentRegionCount) {
         auto region = regions[currentRegionIndex].get();
@@ -603,8 +615,8 @@ void sfz::Synth::finalizeSfzLoad()
         }
 
         if (region->keyswitch) {
-            if (defaultSwitch)
+            if (currentSwitch)
-                region->keySwitched = (*defaultSwitch == *region->keyswitch);
+                region->keySwitched = (*currentSwitch == *region->keyswitch);
 
             if (region->keyswitchLabel)
                 insertPairUniquely(keyswitchLabels, *region->keyswitch, *region->keyswitchLabel);
@@ -617,15 +629,7 @@ void sfz::Synth::finalizeSfzLoad()
         }
 
         for (auto note = 0; note < 128; note++) {
-            bool noteIsKeyswitch = (absl::c_binary_search(resources.keyswitches, note));
+            if (region->keyRange.containsWithEnd(note))
-
-            if (
-                region->keyRange.containsWithEnd(note)
-                || (region->keyswitch && noteIsKeyswitch)
-                || (region->keyswitchDown && *region->keyswitchDown == note)
-                || (region->keyswitchUp && *region->keyswitchUp == note)
-                || (region->previousNote && *region->previousNote == note)
-            )
                 noteActivationLists[note].push_back(region);
         }
 
@@ -1032,6 +1036,12 @@ void sfz::Synth::noteOffDispatch(int delay, int noteNumber, float velocity) noex
     SisterVoiceRingBuilder ring;
     const TriggerEvent triggerEvent { TriggerEventType::NoteOff, noteNumber, velocity };
 
+    for (auto& region : upKeyswitchLists[noteNumber])
+        region->keySwitched = true;
+
+    for (auto& region : downKeyswitchLists[noteNumber])
+        region->keySwitched = false;
+
     for (auto& region : noteActivationLists[noteNumber]) {
         if (region->registerNoteOff(noteNumber, velocity, randValue)) {
             if (region->trigger == SfzTrigger::release && !region->rtDead && !playingAttackVoice(region))
@@ -1142,11 +1152,24 @@ void sfz::Synth::noteOnDispatch(int delay, int noteNumber, float velocity) noexc
     SisterVoiceRingBuilder ring;
     const TriggerEvent triggerEvent { TriggerEventType::NoteOn, noteNumber, velocity };
 
-    bool noteIsKeyswitch = absl::c_binary_search(resources.keyswitches, noteNumber);
+    if (!lastKeyswitchLists[noteNumber].empty()) {
+        if (currentSwitch && *currentSwitch != noteNumber) {
+            for (auto& region : lastKeyswitchLists[*currentSwitch])
+                region->keySwitched = false;
+        }
+        currentSwitch = noteNumber;
+    }
+
+    for (auto& region : lastKeyswitchLists[noteNumber])
+        region->keySwitched = true;
+
+    for (auto& region : upKeyswitchLists[noteNumber])
+        region->keySwitched = false;
+
+    for (auto& region : downKeyswitchLists[noteNumber])
+        region->keySwitched = true;
 
     for (auto& region : noteActivationLists[noteNumber]) {
-        if (noteIsKeyswitch && region->keyswitch)
-            region->keySwitched = (*region->keyswitch == noteNumber);
 
         if (region->registerNoteOn(noteNumber, velocity, randValue)) {
             for (auto& voice : voices) {
@@ -1159,6 +1182,9 @@ void sfz::Synth::noteOnDispatch(int delay, int noteNumber, float velocity) noexc
             startVoice(region, delay, triggerEvent, ring);
         }
     }
+
+    for (auto& region : previousKeyswitchLists)
+        region->previousKeySwitched = (*region->previousNote == noteNumber);
 }
 
 void sfz::Synth::startDelayedReleaseVoices(Region* region, int delay, SisterVoiceRingBuilder& ring) noexcept

src/sfizz/Synth.h

@@ -803,8 +803,8 @@ private:
     std::vector<NoteNamePair> keyLabels;
     std::vector<NoteNamePair> keyswitchLabels;
 
-    // Default active switch if multiple keyswitchable regions are present
+    // Set as sw_default if present in the file
-    absl::optional<uint8_t> defaultSwitch;
+    absl::optional<uint8_t> currentSwitch;
     std::vector<std::string> unknownOpcodes;
     using RegionViewVector = std::vector<Region*>;
     using VoiceViewVector = std::vector<Voice*>;
@@ -899,6 +899,10 @@ private:
      */
     bool playingAttackVoice(const Region* releaseRegion) noexcept;
 
+    std::array<RegionViewVector, 128> lastKeyswitchLists;
+    std::array<RegionViewVector, 128> downKeyswitchLists;
+    std::array<RegionViewVector, 128> upKeyswitchLists;
+    RegionViewVector previousKeyswitchLists;
     std::array<RegionViewVector, 128> noteActivationLists;
     std::array<RegionViewVector, config::numCCs> ccActivationLists;
 

tests/RegionActivationT.cpp

@@ -114,27 +114,6 @@ TEST_CASE("Region activation", "Region tests")
         REQUIRE(!region.isSwitchedOn());
     }
 
-    SECTION("Keyswitches: sw_previous")
-    {
-        region.parseOpcode({ "sw_previous", "40" });
-        REQUIRE(!region.isSwitchedOn());
-        region.registerNoteOn(40, 64_norm, 0.5f);
-        REQUIRE(region.isSwitchedOn());
-        region.registerNoteOff(40, 0_norm, 0.5f);
-        REQUIRE(region.isSwitchedOn());
-        region.registerNoteOn(41, 64_norm, 0.5f);
-        REQUIRE(!region.isSwitchedOn());
-        region.registerNoteOn(40, 64_norm, 0.5f);
-        REQUIRE(region.isSwitchedOn());
-        region.registerNoteOff(40, 0_norm, 0.5f);
-        region.registerNoteOff(41, 0_norm, 0.5f);
-        REQUIRE(region.isSwitchedOn());
-        region.registerNoteOn(41, 64_norm, 0.5f);
-        REQUIRE(!region.isSwitchedOn());
-        region.registerNoteOff(41, 0_norm, 0.5f);
-        REQUIRE(!region.isSwitchedOn());
-    }
-
     SECTION("Sequences: length 2, default position")
     {
         region.parseOpcode({ "seq_length", "2" });
@@ -396,3 +375,53 @@ TEST_CASE("[Keyswitches] sw_default and playing with switches")
     REQUIRE( !synth.getRegionView(2)->isSwitchedOn() );
     REQUIRE( synth.getRegionView(3)->isSwitchedOn() );
 }
+
+TEST_CASE("[Keyswitches] sw_previous in range")
+{
+    sfz::Synth synth;
+    synth.loadSfzString(fs::current_path() / "tests/TestFiles/sw_previous.sfz", R"(
+        <region> sample=*saw sw_previous=60 lokey=50 hikey=70
+    )");
+    // Note: sforzando seems to activate by default if sw_previous is indeed 60,
+    // but not any other value. As it does not seem really useful at this point
+    // the test assumes that sw_previous regions are disabled by default
+    REQUIRE(!synth.getRegionView(0)->isSwitchedOn());
+    synth.noteOn(0, 51, 64);
+    REQUIRE(synth.getNumActiveVoices(true) == 0);
+    synth.noteOn(0, 60, 64);
+    REQUIRE(synth.getNumActiveVoices(true) == 0);
+    REQUIRE(synth.getRegionView(0)->isSwitchedOn());
+    synth.noteOn(0, 51, 64);
+    REQUIRE(synth.getNumActiveVoices(true) == 1);
+    REQUIRE(!synth.getRegionView(0)->isSwitchedOn());
+    synth.noteOn(0, 60, 64);
+    REQUIRE(synth.getNumActiveVoices(true) == 1);
+    REQUIRE(synth.getRegionView(0)->isSwitchedOn());
+    synth.noteOn(0, 60, 64);
+    REQUIRE(synth.getNumActiveVoices(true) == 2);
+    REQUIRE(synth.getRegionView(0)->isSwitchedOn());
+}
+
+
+TEST_CASE("[Keyswitches] sw_previous out of range")
+{
+    // The behavior is the same in this case, regardless of the keyrange
+    sfz::Synth synth;
+    synth.loadSfzString(fs::current_path() / "tests/TestFiles/sw_previous.sfz", R"(
+        <region> sample=*saw sw_previous=60 lokey=50 hikey=55
+    )");
+    REQUIRE(!synth.getRegionView(0)->isSwitchedOn());
+    synth.noteOn(0, 51, 64);
+    REQUIRE(synth.getNumActiveVoices(true) == 0);
+    synth.noteOn(0, 60, 64);
+    REQUIRE(synth.getNumActiveVoices(true) == 0);
+    REQUIRE(synth.getRegionView(0)->isSwitchedOn());
+    synth.noteOn(0, 51, 64);
+    REQUIRE(synth.getNumActiveVoices(true) == 1);
+    REQUIRE(!synth.getRegionView(0)->isSwitchedOn());
+    synth.noteOn(0, 60, 64);
+    REQUIRE(synth.getNumActiveVoices(true) == 1);
+    REQUIRE(synth.getRegionView(0)->isSwitchedOn());
+    synth.noteOn(0, 61, 64);
+    REQUIRE(!synth.getRegionView(0)->isSwitchedOn());
+}