From 863a08c421d563b11ae425a3ece59f89b8dc1c30 Mon Sep 17 00:00:00 2001
From: Paul Ferrand <paul@ferrand.cc>
Date: Mon, 7 Sep 2020 23:18:02 +0200
Subject: [PATCH] Use Cakewalk/RGC behavior for self-choking notes Basically if
 group=off_by, a note will not choke other voices with the same note number

---
 src/sfizz/Synth.cpp | 18 +++++++-----------
 src/sfizz/Synth.h   |  8 --------
 src/sfizz/Voice.cpp |  8 +++++---
 src/sfizz/Voice.h   |  3 ++-
 tests/SynthT.cpp    | 23 +++++++++++++++++++----
 5 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/src/sfizz/Synth.cpp b/src/sfizz/Synth.cpp
index 64505a62..b5f7298d 100644
--- a/src/sfizz/Synth.cpp
+++ b/src/sfizz/Synth.cpp
@@ -1040,16 +1040,6 @@ void sfz::Synth::checkSetPolyphony(const Region* region, int delay) noexcept
     }
 }
 
-void sfz::Synth::checkOffGroups(Region* region, int delay) noexcept
-{
-    for (auto& voice : voices) {
-        if (voice->checkOffGroup(delay, region->group)) {
-            const TriggerEvent& event = voice->getTriggerEvent();
-            noteOffDispatch(delay, event.number, event.value);
-        }
-    }
-}
-
 void sfz::Synth::noteOnDispatch(int delay, int noteNumber, float velocity) noexcept
 {
     const auto randValue = randNoteDistribution(Random::randomGenerator);
@@ -1058,7 +1048,13 @@ void sfz::Synth::noteOnDispatch(int delay, int noteNumber, float velocity) noexc
 
     for (auto& region : noteActivationLists[noteNumber]) {
         if (region->registerNoteOn(noteNumber, velocity, randValue)) {
-            checkOffGroups(region, delay);
+            for (auto& voice : voices) {
+                if (voice->checkOffGroup(region, delay, noteNumber)) {
+                    const TriggerEvent& event = voice->getTriggerEvent();
+                    noteOffDispatch(delay, event.number, event.value);
+                }
+            }
+
             startVoice(region, delay, triggerEvent, ring);
         }
     }
diff --git a/src/sfizz/Synth.h b/src/sfizz/Synth.h
index 6c22fdeb..995bb5e6 100644
--- a/src/sfizz/Synth.h
+++ b/src/sfizz/Synth.h
@@ -848,14 +848,6 @@ private:
      */
     void startVoice(Region* region, int delay, const TriggerEvent& triggerEvent, SisterVoiceRingBuilder& ring) noexcept;
 
-    /**
-     * @brief Check the off groups of all playing voices, releasing if necessary
-     *
-     * @param region
-     * @param delay
-     */
-    void checkOffGroups(Region* region, int delay) noexcept;
-
     /**
      * @brief Start all delayed release voices of the region if necessary
      *
diff --git a/src/sfizz/Voice.cpp b/src/sfizz/Voice.cpp
index 117af317..4f120348 100644
--- a/src/sfizz/Voice.cpp
+++ b/src/sfizz/Voice.cpp
@@ -708,12 +708,14 @@ void sfz::Voice::fillWithGenerator(AudioSpan<float> buffer) noexcept
 #endif
 }
 
-bool sfz::Voice::checkOffGroup(int delay, uint32_t group) noexcept
+bool sfz::Voice::checkOffGroup(const Region* other, int delay, int noteNumber) noexcept
 {
-    if (region == nullptr)
+    if (region == nullptr || other == nullptr)
         return false;
 
-    if (triggerEvent.type == TriggerEventType::NoteOn && region->offBy == group) {
+    if (triggerEvent.type == TriggerEventType::NoteOn 
+        && region->offBy == other->group
+        && noteNumber != triggerEvent.number) {
         off(delay);
         return true;
     }
diff --git a/src/sfizz/Voice.h b/src/sfizz/Voice.h
index 682132ae..a010aaa8 100644
--- a/src/sfizz/Voice.h
+++ b/src/sfizz/Voice.h
@@ -163,11 +163,12 @@ public:
      * This will trigger the release if true.
      *
      * @param delay
+     * @param noteNumber
      * @param group
      * @return true
      * @return false
      */
-    bool checkOffGroup(int delay, uint32_t group) noexcept;
+    bool checkOffGroup(const Region* other, int delay, int noteNumber) noexcept;
 
     /**
      * @brief Render a block of data for this voice into the span
diff --git a/tests/SynthT.cpp b/tests/SynthT.cpp
index 3cbc68fe..69f722ae 100644
--- a/tests/SynthT.cpp
+++ b/tests/SynthT.cpp
@@ -1256,8 +1256,24 @@ TEST_CASE("[Synth] Off by same group")
     REQUIRE( playingVoices.front()->getRegion()->keyRange.containsWithEnd(60) );
 }
 
+TEST_CASE("[Synth] Off by alone and repeated")
+{
+    sfz::Synth synth;
+    sfz::AudioBuffer<float> buffer { 2, 256 };
 
-TEST_CASE("[Synth] Off by same note")
+    synth.loadSfzString(fs::current_path(), R"(
+        <region> group=1 off_by=1 sample=*sine key=60
+    )");
+    synth.noteOn(0, 60, 85);
+    REQUIRE( numPlayingVoices(synth) == 1 );
+    synth.noteOn(0, 60, 85);
+    REQUIRE( numPlayingVoices(synth) == 2 );
+    synth.noteOn(0, 60, 85);
+    REQUIRE( numPlayingVoices(synth) == 3 );
+}
+
+
+TEST_CASE("[Synth] Off by same note and group")
 {
     sfz::Synth synth;
     sfz::AudioBuffer<float> buffer { 2, 256 };
@@ -1267,7 +1283,6 @@ TEST_CASE("[Synth] Off by same note")
         <region> group=1 off_by=1 sample=*triangle key=60
     )");
     synth.noteOn(0, 60, 85);
-    REQUIRE( numPlayingVoices(synth) == 1 );
-    auto playingVoices = getPlayingVoices(synth);
-    REQUIRE( playingVoices.front()->getRegion()->sampleId.filename() == "*triangle" );
+    REQUIRE( numPlayingVoices(synth) == 2 );
+    synth.noteOn(0, 60, 85);
 }