fix(vm)!: first modulate delay time, then notetracking

BREAKING CHANGE: the order of these operations was inconsistent
across the different VMs. Go VM was the only one to first modulate
and then apply note tracking multiplication. But that made most
sense. So now all different VM versions work in this same way.

Author: vsariola

CHANGELOG.md

@@ -45,6 +45,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   be computed every draw. ([#176][i176])
 
 ### Fixed
+- BREAKING CHANGE: always first modulate delay time, then apply notetracking. In
+  a delay unit, modulation adds to the delay time, while note tracking
+  multiplies it with a multiplier dependent on the note. The order of these
+  operations was different in the Go VM vs. x86 VM & WebAssembly VM. In the Go
+  VM, it first modulated, and then applied the note tracking multiplication. In
+  the two assembly VMs, it first applied the note tracking and then modulated.
+  Of these two behaviours, the Go VM behaviour made more sense: if you make a
+  vibrato of +-50 cents for C4, you probably want a vibrato of +-50 cents for C6
+  also. Thus, first modulating and then applying the note tracking
+  multiplication is now the behaviour accross all VMs.
 - Loading instrument forgot to close the file (in model.ReadInstrument)
 - We try to honor the MIDI event time stamps, so that the timing between MIDI
   events (as reported to us by RTMIDI) will be correct.

tests/CMakeLists.txt

@@ -158,6 +158,7 @@ regression_test(test_filter_resmod "VCO_SINE;ENVELOPE;FOP_MULP;SEND")
 regression_test(test_delay "ENVELOPE;FOP_MULP;PANNING;VCO_SINE")
 regression_test(test_delay_stereo "ENVELOPE;FOP_MULP;PANNING;VCO_SINE")
 regression_test(test_delay_notetracking "ENVELOPE;FOP_MULP;PANNING;NOISE")
+regression_test(test_delay_notetracking_modulation "ENVELOPE;FOP_MULP;PANNING;NOISE")
 regression_test(test_delay_reverb "ENVELOPE;FOP_MULP;PANNING;VCO_SINE")
 regression_test(test_delay_feedbackmod "ENVELOPE;FOP_MULP;PANNING;VCO_SINE;SEND")
 regression_test(test_delay_pregainmod "ENVELOPE;FOP_MULP;PANNING;VCO_SINE;SEND")

tests/expected_output/test_delay_notetracking_modulation.raw

@@ -0,0 +1,43 @@
+bpm: 100
+rowsperbeat: 4
+score:
+    tracks:
+        - numvoices: 1
+          order: [0]
+          patterns: [[73, 1, 1, 1, 0, 1, 1, 1, 77, 1, 1, 1, 0]]
+    rowsperpattern: 16
+    length: 1
+patch:
+    - name: Instr
+      numvoices: 1
+      units:
+        - type: envelope
+          id: 1
+          parameters: {attack: 64, decay: 64, gain: 64, release: 64, stereo: 0, sustain: 64}
+        - type: noise
+          id: 10
+          parameters: {gain: 64, shape: 64, stereo: 0}
+        - type: filter
+          id: 12
+          parameters: {bandpass: 0, frequency: 39, highpass: 0, lowpass: 1, negbandpass: 0, neghighpass: 0, resonance: 128, stereo: 0}
+        - type: delay
+          id: 11
+          parameters: {damp: 0, dry: 71, feedback: 114, notetracking: 1, pregain: 128, stereo: 0}
+          varargs: [21574]
+        - type: mulp
+          id: 3
+          parameters: {stereo: 0}
+        - type: pan
+          id: 5
+          parameters: {panning: 64, stereo: 0}
+        - type: out
+          id: 16
+          parameters: {gain: 128, stereo: 1}
+        - id: 13
+          parameters: {}
+        - type: oscillator
+          id: 14
+          parameters: {color: 128, detune: 64, gain: 5, lfo: 1, phase: 0, shape: 64, stereo: 0, transpose: 76, type: 0}
+        - type: send
+          id: 15
+          parameters: {amount: 96, port: 4, sendpop: 1, stereo: 0, target: 11, voice: 0}

tests/test_delay_notetracking_modulation.yml

@@ -309,6 +309,12 @@ su_op_delay_mono:               ; flow into mono delay
 su_op_delay_loop:
         {{- if or (.SupportsModulation "delay" "delaytime") (.SupportsParamValue "delay" "notetracking" 1)}} ; delaytime modulation or note syncing require computing the delay time in floats
         fild    word [{{.BX}}]         ; k dr*y p*p*x, where k = delay time
+        {{- if .SupportsModulation "delay" "delaytime"}}
+        fld     dword [{{.Modulation "delay" "delaytime"}}]
+        {{- .Float 32767.0 | .Prepare | indent 8}}
+        fmul    dword [{{.Float 32767.0 | .Use}}] ; scale it up, as the modulations would be too small otherwise
+        faddp   st1, st0
+        {{- end}}
         {{- if .SupportsParamValue "delay" "notetracking" 1}}
         test    ah, 1 ; note syncing is the least significant bit of ah, 0 = ON, 1 = OFF
         jne     su_op_delay_skipnotesync
@@ -319,12 +325,6 @@ su_op_delay_loop:
         fdivp   st1, st0                 ; use 10787 for delaytime to have neutral transpose
         su_op_delay_skipnotesync:
         {{- end}}
-        {{- if .SupportsModulation "delay" "delaytime"}}
-        fld     dword [{{.Modulation "delay" "delaytime"}}]
-        {{- .Float 32767.0 | .Prepare | indent 8}}
-        fmul    dword [{{.Float 32767.0 | .Use}}] ; scale it up, as the modulations would be too small otherwise
-        faddp   st1, st0
-        {{- end}}
         fistp   dword [{{.SP}}-4]                       ; dr*y p*p*x, dword [{{.SP}}-4] = integer amount of delay (samples)
         mov     edi, esi                            ; edi = esi = current time
         sub     di, word [{{.SP}}-4]                    ; we perform the math in 16-bit to wrap around

vm/compiler/templates/amd64-386/effects.asm

@@ -295,10 +295,23 @@
                         (i32.sub ;; globalTick-delaytimes[delayIndex]
                             (global.get $globaltick)
 {{- if or (.SupportsModulation "delay" "delaytime") (.SupportsParamValue "delay" "notetracking" 1)}} ;; delaytime modulation or note syncing require computing the delay time in floats
+{{- if .SupportsModulation "delay" "delaytime"}}
+                            (local.set $delayTime (f32.add
+                                (f32.convert_i32_u (i32.load16_u
+                                    offset={{index .Labels "su_delay_times"}}
+                                    (local.get $delayIndex)
+                                ))
+                                (f32.mul
+                                    (f32.load offset={{.InputNumber "delay" "delaytime" | mul 4 | add 32}} (global.get $WRK))
+                                    (f32.const 32767)
+                                )
+                            ))
+{{- else}}
                             (local.set $delayTime (f32.convert_i32_u (i32.load16_u
                                     offset={{index .Labels "su_delay_times"}}
                                     (local.get $delayIndex)
                             )))
+{{- end}}
 {{- if .SupportsParamValue "delay" "notetracking" 1}}
                             (if (i32.eqz (i32.and (local.get $delayCount) (i32.const 1)))(then
                                 (local.set $delayTime (f32.div
@@ -312,20 +325,7 @@
                                 ))
                             ))
 {{- end}}
-{{- if .SupportsModulation "delay" "delaytime"}}
-                            (i32.trunc_f32_s (f32.add
-                                (f32.add
-                                    (local.get $delayTime)
-                                    (f32.mul
-                                        (f32.load offset={{.InputNumber "delay" "delaytime" | mul 4 | add 32}} (global.get $WRK))
-                                        (f32.const 32767)
-                                    )
-                                )
-                                (f32.const 0.5)
-                            ))
-{{- else}}
                             (i32.trunc_f32_s (f32.add (local.get $delayTime) (f32.const 0.5)))
-{{- end}}
 {{- else}}
                             (i32.load16_u
                                 offset={{index .Labels "su_delay_times"}}