From 88dea8e989bb83d6e474429770ee0b08570b9f6d Mon Sep 17 00:00:00 2001
From: Takeshi Yokemura <yokemura@ymck.net>
Date: Mon, 16 Aug 2021 12:43:58 +0900
Subject: [PATCH] Keep arpeggiated also in the release phase

---
 Source/TonalVoice.cpp | 80 ++++++++++++++++++++++++++++++++++---------
 Source/TonalVoice.h   |  3 ++
 2 files changed, 66 insertions(+), 17 deletions(-)

diff --git a/Source/TonalVoice.cpp b/Source/TonalVoice.cpp
index 9c49fbb..9b5574f 100644
--- a/Source/TonalVoice.cpp
+++ b/Source/TonalVoice.cpp
@@ -37,6 +37,8 @@ void TonalVoice::startNote (int midiNoteNumber, float velocity, SynthesiserSound
     arpeggioFrameLength = 0;
     currentNumNoteBuffer = 0;
     for (int i=0; i<10; i++) { noteBuffer[i] = 0; }
+    currentNumRetireBuffer = 0;
+    for (int i=0; i<10; i++) { retireBuffer[i] = 0; }
 }
 
 void TonalVoice::advanceControlFrame()
@@ -176,17 +178,12 @@ int TonalVoice::removeArpeggioNote(int midiNoteNumber)
         return 0;
     }
     
-    int i;
-    for (i=0; i<currentNumNoteBuffer; i++) {
-        if (noteBuffer[i] == midiNoteNumber) break;
-    }
-    if (i == currentNumNoteBuffer) { // not found
-        return currentNumNoteBuffer;
-    }
-    shiftNoteBuffer(i);
-    currentNumNoteBuffer--;
-    
-    return currentNumNoteBuffer;
+    // Just push the note number to Retire Buffer
+    retireBuffer[currentNumRetireBuffer] = midiNoteNumber;
+    currentNumRetireBuffer++;
+   
+    // Observed like current number of notes already decreased
+    return currentNumNoteBuffer - currentNumRetireBuffer;
 }
 
 void TonalVoice::pushNoteBuffer(int index, int value) {
@@ -246,16 +243,55 @@ void TonalVoice::onFrameAdvanced()
         }
     }
     
-    if (arpeggioFrameLength > 0) {
+    if (arpeggioFrameLength > 0) { // Arpeggio mode is on
         arpeggioFrameTimer += 1.0 / getSampleRate();
         
         if (arpeggioFrameTimer >= arpeggioFrameLength)
-        {
-            currentArpeggioFrame++;
+        { // Arpeggio phase advances
+            if (!isInReleasePhase()) {
+                // Process the retirements first
+                for(int j = 0; j<currentNumRetireBuffer; j++) {
+                    int target = retireBuffer[j];
+                    int i = 0;
+                    
+                    // Find first match
+                    for (i=0; i<currentNumNoteBuffer; i++) {
+                        if (noteBuffer[i] == target) break;
+                    }
+                    if (i == currentNumNoteBuffer) { // not found
+                        continue;
+                    }
+                    shiftNoteBuffer(i);
+                    currentNumNoteBuffer--;
+                }
+                
+                // Clear Retire Buffer
+                currentNumRetireBuffer = 0;
+                for (int i=0; i<10; i++) { retireBuffer[i] = 0; }
+            } /* else {
+               Retirements should not happen in Release Phase
+               to keep the arpeggio playing during the release.
+            } */
+            
+            if (noteBuffer[currentArpeggioFrame] == noteNumber || noteBuffer[currentArpeggioFrame] == 0) {
+                // Normally 'currentArpaggioFrame' will be updated every frame with
+                // 'noteBuffer[currentArpeggioFrame] == noteNumber' condition,
+                // but when the retirement happens currentArpeggioFrame may point at the slot already gone.
+                // to handle this situation the condition 'noteBuffer[currentArpeggioFrame] == 0' is added.
+                
+                currentArpeggioFrame++;
 
-            if (currentArpeggioFrame >= currentNumNoteBuffer) {
-                currentArpeggioFrame = 0;
-            }
+                if (currentArpeggioFrame >= currentNumNoteBuffer) {
+                    currentArpeggioFrame = 0;
+                }
+            } /* else {
+               In cases like retirement happens, or new arpeggio note is added
+               the note at currentArpeggioFrame is no longer the same,
+               so currentArpeggioFrame doesn't have to be updated.
+               Moreover, updating currentArpeggioFrame in this case
+               often results in sounding the same note over two frames.
+            } */
+            
             noteNumber = noteBuffer[currentArpeggioFrame];
 
             while (arpeggioFrameTimer >= arpeggioFrameLength)
@@ -264,4 +300,14 @@ void TonalVoice::onFrameAdvanced()
             }
         }
     }
+    
+    
 };
+
+bool TonalVoice::isInReleasePhase() {
+    if (settingRefs->isVolumeSequenceEnabled()) {
+        return settingRefs->volumeSequence.isInRelease(currentVolumeSequenceFrame);
+    } else {
+        return envelopePhase == kEnvelopePhaseR;
+    }
+}
diff --git a/Source/TonalVoice.h b/Source/TonalVoice.h
index d273f81..c1f9649 100644
--- a/Source/TonalVoice.h
+++ b/Source/TonalVoice.h
@@ -47,6 +47,8 @@ struct TonalVoice : public BaseVoice   // The base for Pulse and Triangle
     int currentArpeggioFrame = 0;
     double arpeggioFrameTimer = 0;
     double arpeggioFrameLength = 0; // Unit: seconds. Set non-zero value to enable arpeggio
+    int retireBuffer[10];
+    int currentNumRetireBuffer = 0;
 
     void startNote (int midiNoteNumber, float velocity,
                     SynthesiserSound*, int currentPitchWheelPosition) override;
@@ -74,4 +76,5 @@ struct TonalVoice : public BaseVoice   // The base for Pulse and Triangle
     bool isArpeggioEnabled() {
         return arpeggioFrameLength > 0;
     }
+    bool isInReleasePhase();
 };