feat: add SubtractiveInstrumentEngine for pianoroll synth tracks by ChuxiJ · Pull Request #1037 · ace-step/ACE-Step-DAW

ChuxiJ · 2026-03-27T12:26:10Z

Summary

Creates SubtractiveEngine class that maps SubtractiveInstrumentSettings to Tone.js PolySynth with filter (Tone.Filter), LFO, and unison support
Integrates with useTransport.ts to route pianoroll tracks with instrument.kind === 'subtractive' through the new engine
Supports noteOn/noteOff, triggerAttackRelease, playSlideNote, setParameter for real-time control
30 unit tests covering parameter mapping, note triggering, octave offset, filter/LFO creation, and lifecycle

Test plan

npx tsc --noEmit — 0 type errors
npm test — 3002 tests pass (26 new)
npm run build — succeeds
Manual: create pianoroll track with subtractive instrument, verify MIDI notes produce sound
Manual: adjust filter cutoff/resonance, verify real-time parameter changes

🤖 Generated with Claude Code

Creates SubtractiveEngine that maps SubtractiveInstrumentSettings to Tone.js PolySynth with filter, LFO, and unison support. Integrates with useTransport to route pianoroll tracks with instrument.kind === 'subtractive' through the new engine instead of the legacy preset-based SynthEngine. Closes #1020 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Adds a new runtime engine to make pianoroll tracks with instrument.kind === 'subtractive' actually produce sound by mapping SubtractiveInstrumentSettings onto a Tone.js synth graph, and wires it into transport playback scheduling.

Changes:

Introduces SubtractiveEngine to create/manage per-track Tone.js PolySynth chains (filter/LFO/output gain) plus note triggering helpers.
Integrates subtractive instruments into useTransport track setup and MIDI scheduling (including slide handling).
Adds unit tests for instance lifecycle, parameter mapping, octave offsets, and note triggering.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File	Description
`src/hooks/useTransport.ts`	Starts the subtractive engine, routes subtractive pianoroll tracks to it, schedules MIDI/slide events, and releases notes on stop/pause.
`src/engine/SubtractiveEngine.ts`	Implements the subtractive synth engine (instance management, graph creation, note APIs, parameter application).
`src/engine/__tests__/SubtractiveEngine.test.ts`	Adds vitest coverage for core behavior and parameter mapping expectations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-27T12:31:18Z

src/engine/SubtractiveEngine.ts

+      this._disposeInstance(this.previewInstance);
+    }
+    this.previewInstance = this._createInstance(settings);
+    this.previewInstance.output.toDestination();


previewNote() creates the preview instance via _createInstance(settings) which already routes output to destination when connectTo is undefined, then calls this.previewInstance.output.toDestination() again. This can create duplicate connections (and increase volume) each preview; remove the extra toDestination() call or change _createInstance to not auto-connect for preview instances.

Suggested change

this.previewInstance.output.toDestination();

Copilot · 2026-03-27T12:31:18Z

src/engine/SubtractiveEngine.ts

+    if (instance.filter && settings.filter.enabled) {
+      instance.filter.frequency.value = settings.filter.cutoffHz;
+      instance.filter.Q.value = settings.filter.resonance * 30;
+      instance.filter.type = settings.filter.type;
+    }
+
+    if (instance.lfo && settings.lfo.enabled) {
+      instance.lfo.frequency.value = settings.lfo.rateHz;


_updateSettings() only updates the filter/LFO if they already exist; if a track instance was created with filter.enabled === false (or LFO disabled) and settings later enable them, the nodes are never created (and disabling never tears them down). Consider recreating the instance (or rebuilding the signal chain) when filter.enabled / lfo config changes so ensureTrackSynth() actually reflects the new settings.

Suggested change

if (instance.filter && settings.filter.enabled) {

instance.filter.frequency.value = settings.filter.cutoffHz;

instance.filter.Q.value = settings.filter.resonance * 30;

instance.filter.type = settings.filter.type;

}

if (instance.lfo && settings.lfo.enabled) {

instance.lfo.frequency.value = settings.lfo.rateHz;

// Ensure the filter node and routing reflect the current settings.

if (settings.filter.enabled) {

if (!instance.filter) {

// Create and insert a new filter between the synth and the output.

const filter = new Tone.Filter({

frequency: settings.filter.cutoffHz,

type: settings.filter.type,

Q: settings.filter.resonance * 30,

});

// Rewire: synth -> filter -> output.

instance.synth.disconnect();

instance.synth.connect(filter);

filter.connect(instance.output);

instance.filter = filter;

// If an LFO already exists, retarget it to the new filter frequency.

if (instance.lfo) {

instance.lfo.disconnect();

instance.lfo.connect(filter.frequency);

}

} else {

instance.filter.frequency.value = settings.filter.cutoffHz;

instance.filter.Q.value = settings.filter.resonance * 30;

instance.filter.type = settings.filter.type;

}

} else if (instance.filter) {

// Disable and remove the filter from the signal chain.

instance.synth.disconnect();

instance.filter.disconnect();

instance.filter.dispose();

instance.filter = null;

// Reconnect synth directly to output.

instance.synth.connect(instance.output);

}

// Ensure the LFO node and modulation reflect the current settings.

if (settings.lfo.enabled) {

const depth = settings.lfo.depth;

if (!instance.lfo) {

const lfo = new Tone.LFO(settings.lfo.rateHz, -depth, depth);

if (instance.filter) {

lfo.connect(instance.filter.frequency);

}

lfo.start();

instance.lfo = lfo;

} else {

instance.lfo.frequency.value = settings.lfo.rateHz;

instance.lfo.min = -depth;

instance.lfo.max = depth;

}

} else if (instance.lfo) {

instance.lfo.stop();

instance.lfo.disconnect();

instance.lfo.dispose();

instance.lfo = null;

Copilot · 2026-03-27T12:31:19Z

src/engine/SubtractiveEngine.ts

+    const { oscillator, ampEnvelope, filter, lfo, unison } = settings;
+
+    const voiceCount = Math.max(1, Math.min(8, unison.voices));
+    const detuneValue = oscillator.detuneCents + (voiceCount > 1 ? unison.detuneCents : 0);
+    const synth = new Tone.PolySynth(Tone.Synth);


Several fields in SubtractiveInstrumentSettings are currently ignored (e.g. filterEnvelope, filter.drive, filter.keyTracking, lfo.retrigger, LFO targets pitch/pan, and most unison fields like voices/stereoSpread/blend). This conflicts with the PR description / Issue #1020 acceptance criteria that runtime support exists for all defined parameters; either implement the missing mappings or update the scope/docs/types to match what is actually supported.

Copilot · 2026-03-27T12:31:19Z

src/engine/SubtractiveEngine.ts

+    if (!instance) return;
+    this._applyParameter(instance, name, value);
+  }
+


setParameter() applies changes to Tone nodes but does not update instance.settings. Methods like _pitchToFreq() (octave), playSlideNote() (glideTime), and any future logic reading instance.settings will keep using stale values after real-time parameter updates. Update instance.settings in sync with setParameter() (or avoid relying on instance.settings after creation).

Suggested change

if (!instance) return;

this._applyParameter(instance, name, value);

}

if (!instance) return;

// Keep instance.settings in sync with real-time parameter changes

this._updateInstanceSettingsForParameter(instance, name, value);

this._applyParameter(instance, name, value);

}

/**

* Update the in-memory settings object to reflect a parameter change.

* Supports both top-level keys (e.g. "glideTime") and dotted paths

* (e.g. "oscillator.octave").

*/

private _updateInstanceSettingsForParameter(

instance: SubtractiveInstance,

name: string,

value: number | string | boolean,

): void {

const settings: any = instance.settings;

if (!settings) return;

// Direct top-level property

if (Object.prototype.hasOwnProperty.call(settings, name)) {

(settings as any)[name] = value as any;

return;

}

// Nested property via dotted path (e.g. "oscillator.octave")

if (name.indexOf('.') === -1) {

return;

}

const parts = name.split('.');

let current: any = settings;

for (let i = 0; i < parts.length - 1; i++) {

const key = parts[i];

if (

current == null ||

typeof current !== 'object' ||

!Object.prototype.hasOwnProperty.call(current, key)

) {

// Path does not exist in settings; do not create new structure.

return;

}

current = current[key];

}

const lastKey = parts[parts.length - 1];

if (

current != null &&

typeof current === 'object' &&

Object.prototype.hasOwnProperty.call(current, lastKey)

) {

current[lastKey] = value as any;

}

}

Copilot · 2026-03-27T12:31:19Z

src/engine/SubtractiveEngine.ts

+    const octave = instance.settings.oscillator.octave;
+    const glideTime = instance.settings.glideTime > 0
+      ? instance.settings.glideTime
+      : Math.max(0.03, Math.min(0.12, duration * 0.35));
+    const fromFreq = this._pitchToFreq(fromPitch, octave);
+    const toFreq = this._pitchToFreq(toPitch, octave);
+
+    const synth = instance.synth as unknown as {
+      set: (options: Record<string, unknown>) => void;
+      triggerAttack: (note: number, time?: string | number, velocity?: number) => void;
+      triggerRelease: (note: number, time?: string | number) => void;
+      triggerAttackRelease: (note: number, duration: number, time?: string | number, velocity?: number) => void;
+    };
+    synth.set({ portamento: glideTime });
+    synth.triggerAttack(fromFreq, undefined, velocity / 127);
+    synth.triggerRelease(fromFreq, `+${glideTime}`);
+    synth.triggerAttackRelease(toFreq, Math.max(0.04, duration), `+${glideTime}`, velocity / 127);
+  }


playSlideNote() sets portamento to an auto-computed glide time when settings.glideTime is 0, but never restores it afterward. That means subsequent non-slide notes will unexpectedly glide. Consider restoring portamento to settings.glideTime after scheduling the slide (or set it per-event without mutating the shared synth state).

Copilot · 2026-03-27T12:31:19Z

src/engine/SubtractiveEngine.ts

+          instance.lfo.min = -(value as number);
+          instance.lfo.max = value as number;


setParameter('lfo.depth', ...) sets lfo.min/max to ±depth, but the LFO is connected either to output.gain or filter.frequency, where the expected min/max ranges are not centered on 0. This can drive gain negative or push filter cutoff to invalid ranges. Update the depth handling to recompute min/max based on the current base value and target (similar to _createInstance()’s min/max calculations).

Suggested change

instance.lfo.min = -(value as number);

instance.lfo.max = value as number;

const depth = value as number;

const settingsAny = instance.settings as any;

const lfoTarget = settingsAny?.lfoTarget;

if (lfoTarget === 'amp') {

// LFO targets output gain: modulate around current gain, clamp to >= 0

const baseGain = instance.output.gain.value;

const minGain = Math.max(0, baseGain - depth);

const maxGain = baseGain + depth;

instance.lfo.min = minGain;

instance.lfo.max = maxGain;

} else if (lfoTarget === 'filter' && instance.filter) {

// LFO targets filter cutoff: modulate around current frequency within a safe audio range

const baseFreq = instance.filter.frequency.value;

const context = Tone.getContext();

const nyquist = context.sampleRate / 2;

const minFreq = Math.max(20, baseFreq - depth);

const maxFreq = Math.min(nyquist, baseFreq + depth);

instance.lfo.min = minFreq;

instance.lfo.max = maxFreq;

} else {

// Fallback: keep previous behavior but avoid negative values

instance.lfo.min = 0;

instance.lfo.max = Math.max(0, depth);

}

Copilot · 2026-03-27T12:31:19Z

src/engine/SubtractiveEngine.ts

+    if (detuneValue !== 0) {
+      instance.synth.set({ detune: detuneValue });
+    }
+


_updateSettings() only applies detune when the new detuneValue !== 0, but never resets detune back to 0 when settings change to remove detune (same issue for oscillator.level/volume, which is set in _createInstance() but not updated here). This can leave stale synth state after settings updates; consider always setting these fields (or explicitly resetting them when the new value is neutral).

Suggested change

if (detuneValue !== 0) {

instance.synth.set({ detune: detuneValue });

}

instance.synth.set({ detune: detuneValue });

ChuxiJ · 2026-03-27T19:48:25Z

Triage: Merge candidate after rebase — Fixes #1020 (SubtractiveTrackInstrument has type defs but no runtime). 30 unit tests, CI was green. Needs rebase due to projectStore changes. This closes a real bug.

Generated by Claude Code

Copilot AI review requested due to automatic review settings March 27, 2026 12:26

Copilot started reviewing on behalf of ChuxiJ March 27, 2026 12:26 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

ChuxiJ mentioned this pull request Mar 27, 2026

chore: Triage and resolve 27 stalled open PRs #1107

Open

31 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add SubtractiveInstrumentEngine for pianoroll synth tracks#1037

feat: add SubtractiveInstrumentEngine for pianoroll synth tracks#1037
ChuxiJ wants to merge 1 commit intomainfrom
fix/issue-1020-subtractive-engine

ChuxiJ commented Mar 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

ChuxiJ commented Mar 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-    if (instance.filter && settings.filter.enabled) {
-      instance.filter.frequency.value = settings.filter.cutoffHz;
-      instance.filter.Q.value = settings.filter.resonance * 30;
-      instance.filter.type = settings.filter.type;
-    }
-    if (instance.lfo && settings.lfo.enabled) {
-      instance.lfo.frequency.value = settings.lfo.rateHz;
+    // Ensure the filter node and routing reflect the current settings.
+    if (settings.filter.enabled) {
+      if (!instance.filter) {
+        // Create and insert a new filter between the synth and the output.
+        const filter = new Tone.Filter({
+          frequency: settings.filter.cutoffHz,
+          type: settings.filter.type,
+          Q: settings.filter.resonance * 30,
+        });
+        // Rewire: synth -> filter -> output.
+        instance.synth.disconnect();
+        instance.synth.connect(filter);
+        filter.connect(instance.output);
+        instance.filter = filter;
+        // If an LFO already exists, retarget it to the new filter frequency.
+        if (instance.lfo) {
+          instance.lfo.disconnect();
+          instance.lfo.connect(filter.frequency);
+        }
+      } else {
+        instance.filter.frequency.value = settings.filter.cutoffHz;
+        instance.filter.Q.value = settings.filter.resonance * 30;
+        instance.filter.type = settings.filter.type;
+      }
+    } else if (instance.filter) {
+      // Disable and remove the filter from the signal chain.
+      instance.synth.disconnect();
+      instance.filter.disconnect();
+      instance.filter.dispose();
+      instance.filter = null;
+      // Reconnect synth directly to output.
+      instance.synth.connect(instance.output);
+    }
+    // Ensure the LFO node and modulation reflect the current settings.
+    if (settings.lfo.enabled) {
+      const depth = settings.lfo.depth;
+      if (!instance.lfo) {
+        const lfo = new Tone.LFO(settings.lfo.rateHz, -depth, depth);
+        if (instance.filter) {
+          lfo.connect(instance.filter.frequency);
+        }
+        lfo.start();
+        instance.lfo = lfo;
+      } else {
+        instance.lfo.frequency.value = settings.lfo.rateHz;
+        instance.lfo.min = -depth;
+        instance.lfo.max = depth;
+      }
+    } else if (instance.lfo) {
+      instance.lfo.stop();
+      instance.lfo.disconnect();
+      instance.lfo.dispose();
+      instance.lfo = null;

-    if (!instance) return;
-    this._applyParameter(instance, name, value);
-  }
+    if (!instance) return;
+    // Keep instance.settings in sync with real-time parameter changes
+    this._updateInstanceSettingsForParameter(instance, name, value);
+    this._applyParameter(instance, name, value);
+  }
+  /**
+   * Update the in-memory settings object to reflect a parameter change.
+   * Supports both top-level keys (e.g. "glideTime") and dotted paths
+   * (e.g. "oscillator.octave").
+   */
+  private _updateInstanceSettingsForParameter(
+    instance: SubtractiveInstance,
+    name: string,
+    value: number | string | boolean,
+  ): void {
+    const settings: any = instance.settings;
+    if (!settings) return;
+    // Direct top-level property
+    if (Object.prototype.hasOwnProperty.call(settings, name)) {
+      (settings as any)[name] = value as any;
+      return;
+    }
+    // Nested property via dotted path (e.g. "oscillator.octave")
+    if (name.indexOf('.') === -1) {
+      return;
+    }
+    const parts = name.split('.');
+    let current: any = settings;
+    for (let i = 0; i < parts.length - 1; i++) {
+      const key = parts[i];
+      if (
+        current == null ||
+        typeof current !== 'object' ||
+        !Object.prototype.hasOwnProperty.call(current, key)
+      ) {
+        // Path does not exist in settings; do not create new structure.
+        return;
+      }
+      current = current[key];
+    }
+    const lastKey = parts[parts.length - 1];
+    if (
+      current != null &&
+      typeof current === 'object' &&
+      Object.prototype.hasOwnProperty.call(current, lastKey)
+    ) {
+      current[lastKey] = value as any;
+    }
+  }

		instance.lfo.min = -(value as number);
		instance.lfo.max = value as number;

-          instance.lfo.min = -(value as number);
-          instance.lfo.max = value as number;
+          const depth = value as number;
+          const settingsAny = instance.settings as any;
+          const lfoTarget = settingsAny?.lfoTarget;
+          if (lfoTarget === 'amp') {
+            // LFO targets output gain: modulate around current gain, clamp to >= 0
+            const baseGain = instance.output.gain.value;
+            const minGain = Math.max(0, baseGain - depth);
+            const maxGain = baseGain + depth;
+            instance.lfo.min = minGain;
+            instance.lfo.max = maxGain;
+          } else if (lfoTarget === 'filter' && instance.filter) {
+            // LFO targets filter cutoff: modulate around current frequency within a safe audio range
+            const baseFreq = instance.filter.frequency.value;
+            const context = Tone.getContext();
+            const nyquist = context.sampleRate / 2;
+            const minFreq = Math.max(20, baseFreq - depth);
+            const maxFreq = Math.min(nyquist, baseFreq + depth);
+            instance.lfo.min = minFreq;
+            instance.lfo.max = maxFreq;
+          } else {
+            // Fallback: keep previous behavior but avoid negative values
+            instance.lfo.min = 0;
+            instance.lfo.max = Math.max(0, depth);
+          }

Conversation

ChuxiJ commented Mar 27, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

ChuxiJ commented Mar 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants