diff --git a/docs/COMPLETED.md b/docs/COMPLETED.md index c34c465..fba8466 100644 --- a/docs/COMPLETED.md +++ b/docs/COMPLETED.md @@ -1248,3 +1248,75 @@ unlimited — and Daniel has ruled that a future track will change the bake to p limiter. Until that lands this is a recorded, known limitation, not an oversight. **Neither track has been verified in a running DAW; both are asserted in CTest only.** + +### Γ-W4-T1 — vst3-parameter-set + +The instrument now reports its automatable parameters to the host: 44 of 44 issue, under a +FOREVER-FROZEN `ParamID` table — blocks of 100 per deck group in signal-flow order, steps +of 10 within a block, a curve dial at its outer knob's id + 1 — each with a real plain +range, `units` string and display precision at the host boundary, not a raw normalized +float. The exposed set is DERIVED from `deckParamCommit` / `liveCommitFor`, never +hand-maintained: a control qualifies iff its class is `Live` or `NoteOnLatched`. Everything +else — play mode, the pitch engine, filter enable, the three Staged↔Spline toggles, voice +count, Poly|Mono, Retrigger|Legato, and the limiter enable — is OMITTED from the list +entirely rather than exposed read-only, a named limitation rather than a silent one. A new +pure module, `core/instrument/param` (`param_id`, `param_units`, `param_format`, +`param_live`, `param_merge`), holds the id table, the plain-value layer, the one formatter +per unit category (eight of them), and the audio thread's block-boundary merge decision; +`shell/instrument/instrument_params` adapts it onto `Steinberg::Vst::Parameter` and decides +nothing itself. + +**Both VST3 delivery channels are serviced.** An earlier pass routed host automation +through `IEditController::setParamNormalized` alone — the SDK documents that as the +GUI-update channel only ("should update the according GUI element(s) only") — while +`ProcessData::inputParameterChanges` is the audio-side one; the SDK's own +`SingleComponentEffect` sample (`public.sdk/samples/vst/again/source/againsimple.cpp`) +drains the queue in `process()` *and* implements `setParamNormalized`. Both are now +serviced. + +**A host automation point's authority is bounded, not permanent.** It outranks the model +only between the point landing and the UI thread folding it into the model and +republishing — at most one UI tick — never a later restore, bake reset, or knob move. An +earlier pass made the hold permanent, which silently defeated `setState`, preset load, +undo, and the bake's reset for any parameter that had ever carried an automation point. +The model is now written down in full — `shell/instrument/CLAUDE.md`'s "THE AUTHORITY +MODEL" section — and enforced by the pure `param_merge`; `test_param_merge` asserts both +halves: that a held point outranks the model until the model catches up, and that a writer +after the release reaches the audio again. + +**Two rulings, both Daniel, 2026-08-02.** (1) Pitch key-track and Trigger length promote +from `Reload` to `NoteOnLatched` — the promotion that takes the count to 44 of 44 and +issues ids 1000 and 1450. It was **not** the predicate-only change the plan anticipated: +key-track lives on `InstrumentParams`, not `PlaySeconds`, so the host's write path could +not reach it without `LiveValues` and `foldLive`'s input widening and `Voice::start` +taking the two latched values as arguments beside rate; the new `param::valueHomeFor` +guard closes the class of bug this exposed (a promoted control with no home would have +no-oped silently in both directions) by asserting every exposed control has a home and +branching the shell's own read/write paths on it. (2) The curve-shape dials' ±0.01 +snap-to-centre band now applies on the mouse-drag path only, never on a host-facing map — +*"our continuous ranges should be continuous."* + +**Two adjacent SDK surfaces were assessed and left unimplemented, with dispositions +recorded rather than re-surveyed later.** `IMidiMapping` — no CC vocabulary fits what's +exposed, and REAPER's own per-parameter MIDI learn is expected to cover the case. +`IParameterFunctionName` and `IAutomationState` are also not implemented; the latter +reports the host's automation mode for the whole plug-in, not per parameter, so it cannot +answer the bake's "is this parameter automated" question. + +**The bake's reset now notifies the host, and its one remaining gap is named rather than +hidden.** Every internal writer of an exposed parameter's value goes through the one +`beginEdit`/`performEdit`/`endEdit` path, the bake's reset included. What it cannot do: +clear a host automation lane. If a reset-class parameter carries one, the lane replays its +curve onto audio the bake already baked that processing into — double processing — and +`IAutomationState`'s whole-plugin (not per-parameter) granularity means there is no way to +detect or refuse it. Documented as a boundary of the bake's fidelity claim, not discovered +later as a bug against Phase Ξ. + +**The per-sample voice path is byte-identical across the whole track.** + +**Not verified in a running DAW — CTest-asserted only.** `docs/TODO.md` carries the +residual DAW-verification items: whether REAPER renders `ParameterInfo::units` beside the +formatted string, whether REAPER's MIDI learn actually covers the un-shipped `IMidiMapping` +case, the three migration round trips (a pre-parameter project, a save/reopen in an older +binary, automation drawn and replayed), whether an offline render replays automation, and +whether REAPER restores instance state through `setState` rather than `setComponentState`. diff --git a/docs/PLAN.md b/docs/PLAN.md index af9997a..fc5ce24 100644 --- a/docs/PLAN.md +++ b/docs/PLAN.md @@ -1121,243 +1121,50 @@ not own the processor's live block, the limiter DSP itself, the parameter surfac ### Γ-W4 — VST3 parameters -**Depends on every earlier wave, and each dependency is a hard prerequisite rather than a -courtesy:** +**Depended on every earlier wave in this phase** — W1-T1's taper module (the host-facing +normalization itself), W1-T2 and W2-T1's control inventory and three-state commit +predicate (the exposed set is exactly `Live ∪ NoteOnLatched`), W3-T1's final MASTER +inventory, and W3-T2's completed bake reset list — each a hard prerequisite rather than a +courtesy. See `docs/COMPLETED.md` for the full narrative of each. -1. **← W1-T1.** The taper and the 10 s ceiling **are** the host-facing normalization, and - W1-T1 is also what extracts them into the one pure module the host reads through. Declaring - parameters against a taper that is still moving is the one-way door this whole phase is - ordered around. -2. **← W1-T2 and W2-T1.** Every control that could be a parameter must exist before the list - is declared. The list is derived from the control inventory; an inventory still growing - produces a list that has to be re-frozen, and it cannot be. -3. **← W2-T1 specifically.** `deckParamCommit` becoming three-valued is the *prerequisite* of - the classification, not an incidental: the exposed set is exactly `Live ∪ NoteOnLatched`. -4. **← W3-T1.** MASTER's inventory (limiter toggle, GR bubble, reserved cell) is the last - change to what controls exist at all. -5. **← W3-T2.** The bake's reset list must already be complete, so this track adds the - host-notification obligation once rather than amending an amendment. - -**One track.** The storage decision governs every part of the work — the projection rule, the -migration path, what `getParamNormalized` returns, and what the bake's reset must do — exactly -as Ξ-W2-T1's crossing decision governs its chain. Every candidate split (a pure -model/classification half and a host-wiring half) is **serial**, so it buys no concurrency and -puts the decision on one side of a boundary and its consequences on the other. +**The track has landed** — Γ-W4-T1 (`vst3-parameter-set`) — see `docs/COMPLETED.md` for +the full narrative. One track: the storage decision governed every part of the work, so a +pure/host-wiring split would have been serial and bought no concurrency. #### Γ-W4-T1 — `vst3-parameter-set` -**Goal.** The instrument reports its automatable parameters to the host, under a frozen id -contract and a logical order — which also hands it REAPER's whole per-parameter modulation -block (LFO, envelope follower, MIDI link, parameter linking) for free. +**Landed** — see `docs/COMPLETED.md` for the full narrative. The instrument now reports +its automatable parameters to the host under a FOREVER-FROZEN `ParamID` table (blocks of +100 per deck group in signal-flow order, steps of 10 within a block, a curve dial at its +outer knob's id + 1) — 44 of 44 issue, derived from `deckParamCommit` / `liveCommitFor` +rather than hand-maintained, each with real units, a plain range and display precision at +the host boundary. A new pure module, `core/instrument/param`, holds the id table, the +plain-value layer, the one formatter per unit category, and the audio thread's +block-boundary merge decision; `shell/instrument/instrument_params` adapts it onto +`Steinberg::Vst::Parameter` and decides nothing. -**Consolidates:** nothing from the seventeen. **Ruling 1** (Daniel, 2026-08-01): *"correct the -phase gamma plan to account for complying with the VST3 standard for parameter reporting… by -the end of gamma we have the automatable params reported. Make the parameter order logical."* +**Both VST3 delivery channels are serviced** — the controller's `setParamNormalized` and +the audio thread's `IParameterChanges` drain, mirroring the SDK's own +`SingleComponentEffect` sample rather than the controller-only delivery an earlier pass +shipped. **A host automation point's authority is bounded**, not permanent: it outranks +the model only until the UI thread folds it in and republishes — at most one UI tick — +never a later restore, bake reset or knob move. `shell/instrument/CLAUDE.md`'s "THE +AUTHORITY MODEL" section states it, `param_merge` enforces it. -**Spec:** `docs/product/parameter-automation.md` — **§§6–10 are the specification; §§1–5 are -the analysis behind it.** Read §6.1 (storage), §6.3 (the freeze), §7 (the classification) and -§8 (the one-way-door sweep) before scoping. **Today the plugin has zero parameters:** -`ReaSamplerProcessor::initialize` (`reasampler_processor.cpp:56-73`) never populates -`SingleComponentEffect::parameters`, so `getParameterCount()` returns the SDK default 0. This -track introduces the whole surface. +**Two rulings, both Daniel, 2026-08-02.** Pitch key-track and Trigger length promote from +`Reload` to `NoteOnLatched` — the promotion that takes the count to 44 of 44 and issues +ids 1000 and 1450, and which needed `LiveValues`/`foldLive` widened and `Voice::start` +handed the two latched values, not just a predicate flip; the new `param::valueHomeFor` +guard closes the class of bug the promotion exposed. And the curve-shape dials' ±0.01 +snap-to-centre band now applies on the mouse-drag path only, never on a host-facing map. -**Surface boundary — owns:** a **new pure parameter-identity module** (the frozen id table, -the `DeckParam` ↔ `ParamID` mapping, the derived exposed set, the unit assignment — with its -own `_tests` target), `shell/instrument/reasampler_processor` + `processor_state` (the -`IEditController` parameter surface and the `IParameterChanges` read), -`shell/instrument/editor_controls.cpp` and the editor's drag-commit sites (the -`beginEdit`/`performEdit`/`endEdit` bracketing), and the bake's reset step **for the -notification path only**. **Does not own** the taper (W1-T1's module, consumed), any control's -value semantics, any deck geometry, or the bake's reset *membership* (W3-T2's). +**The bake's reset now notifies the host; the double-processing limitation (a host +automation lane outranking the reset) is a named, documented boundary, not discovered +later.** `IMidiMapping`, `IParameterFunctionName` and `IAutomationState` are all assessed +and NOT implemented. -**Behavior.** -- **The blob stays authoritative; a parameter is a THIRD SURFACE onto the one model** — - a peer of the deck knob and the overlay node, not a second copy of the value. - `docs/product/parameter-automation.md` §6.1 states the load, host→plugin, plugin→host and - save rules, and the two verified findings that closed the fork: this plugin is a - `SingleComponentEffect`, where the SDK itself collapses `IComponent::setState` and - `IEditController::setState` (`vstsinglecomponenteffect.h:41-47`), so there is one state and - §3.3's drift hazard describes a split-component design we do not use; and the blob is a - **cross-artifact contract** the extension's `instrument_drop` writes, which - parameters-as-truth would silently make partial. -- **`ParamID` is an independent, hand-assigned, FOREVER-FROZEN table** — blocks of 100 per - deck group **in signal-flow order** (Γ-F7), steps of 10 within a block, a curve dial at its - outer knob's id + 1, blocks starting at 1000. **The full 44-id assignment is stated at - §6.2** and is to be transcribed, not re-derived. §6.2 also for why hand-assignment beats - derivation and why the within-block order is seeded ONCE rather than tracked against - `cellIds`; **§6.3 for the freeze invariant, which is to be stated in the table's header with - the same force as the command-id strings, the class UIDs and the payload field order.** -- **The exposed set is DERIVED from `deckParamCommit` / `liveCommitFor`, never - hand-maintained** — a control is a parameter iff its class is `Live` or `NoteOnLatched`. - **44 parameters** at the end of Γ-W3, enumerated by group in §7.1. -- **Everything else is OMITTED from the list entirely**, not exposed-and-flagged: the reload - and rebuild tiers, all structural state, and the limiter enable (§3.8, settled). §7.2 - states why omission beats `kIsReadOnly`, and names the limitation plainly — the user cannot - automate filter on/off, play mode, the pitch engine, Staged↔Spline or polyphony, and the - unlock is to give the control a live path first. -- **Every exposed parameter reports REAL UNITS to the host** (Ruling 3). Each declares a - **plain range**, a **`units` string**, and a **display precision**; the complete - eight-category table covering all 44 — ranges, units, precision, and which taper each - category carries — is `docs/product/parameter-automation.md` §6.7.1, and it is a - specification, not a suggestion. VST3's wire format stays normalized (it cannot be - otherwise); the requirement is met through the **plain-value layer** the SDK provides, whose - direct precedent in the vendored tree is `public.sdk/samples/vst/common/logscale.h:221-229` - overriding `toPlain`/`toNormalized` for a log law — the exact shape our log ms and log2 - semitone knobs need. -- **`normalizedParamToPlain` / `plainParamToNormalized` / `getParamStringByValue` / - `getParamValueByString` route through W1-T1's taper module and the ONE formatter per unit - category.** Three functions that agree today is a defect; the host's normalization, the - needle angle and the overlay node must be the same function. **`toNormalized` IS the - taper** — §6.7.3 — which is what makes the phase's one-way-door ordering structurally true - rather than a warning someone has to remember. -- **`stepCount = 0` on all 44, and the editor's shift-snap is never exposed as `stepCount`.** - Snapped drag and parameter continuity are independent axes; `stepCount` quantizes the - parameter permanently, including for the host's automation, and freezes into the forever - contract. The sweep is done and clean (§6.7.6) — every discrete control is reload or rebuild - tier and therefore already omitted, so continuity is structural rather than lucky. -- **`IParameterChanges` is observed at BLOCK boundaries, stated in the header** — the last - point in a block wins. Sample-accurate application would put a per-sample "did anything - change" question on the per-voice-per-sample path, which the phase-wide guardrail forbids. -- **A host parameter change takes the control's existing commit tier and no other.** Nothing - on the automation path may reach `reloadInstrument` or `rebuildVoiceEngine` — which §7.2's - omissions guarantee structurally rather than by care. -- **`IUnitInfo`: one unit per deck group**, mirroring the group inventory rather than the - editor's rows. **Order is signal flow — Γ-F7, RULED** (§6.4). Presentation index order is - ascending id, so identity and presentation agree by construction. -- **The filter's four report plain units WITHOUT being re-tapered.** `toPlain` is read-side - only; reporting Hz / Q / drive depth means calling `filterCutoffHzFromNorm`, - `filterQFromNorm` and `filterDriveDepthFromNorm` — the filter module's own frozen laws, - which `deckValueLabel` already calls today — not restating them. **The one additive piece: - `filterNormFromDriveDepth` does not exist and must be added in `filter_params`**, beside the - two inverses that do; the analytic inverse of a frozen law is not a change to it. §6.7.5. -- **No `kIsBypass` on anything.** The plugin is an instrument and exposes no bypass - parameter; the limiter is a safety device, not a bypass, and binding it there would hand - the host a control that restarts the component. -- **The bake's reset gains a notification obligation** (§9): every internal writer of a value - that is an exposed parameter must go through the one `beginEdit`/`performEdit`/`endEdit` - path, and the bake's reset is the codebase's first non-gesture writer. **Enumerating those - sites is part of this track**, not a follow-up. -- **Two adjacent SDK surfaces are assessed, with dispositions, so they are not re-surveyed:** - `IMidiMapping` is **in scope and nearly free** (a CC → `ParamID` map, one function); - `IParameterFunctionName` is **not implemented** (its vocabulary is compressor/panner - semantics that name nothing here); `IAutomationState` is **not implemented** (it reports the - host's automation mode for the whole plug-in, not per parameter, so it cannot answer the one - question §9 would have wanted it for). - -**Acceptance criteria.** -- **The host lists exactly the derived set, in the ruled order, with no parameter the - predicate does not classify `Live` or `NoteOnLatched`** — asserted against the predicate, - not against a literal count. -- **Every id in the table is asserted unique, in its group's block, and on its step** — and a - test fails if any id changes value, which is what makes the freeze mechanical rather than - cultural. **The asserted values are §6.2's table verbatim**, including the signal-flow block - sequence; a test that recomputes the ids from `cellIds` would defeat the freeze it exists - to hold. -- **`toPlain(info.defaultNormalizedValue)` compares EXACTLY equal to the default**, per - parameter, against a default-constructed `PlaySeconds` (and against `master_gain`'s unity), - so a host's reset-to-default and the editor's double-click land on the same value. - **`defaultNormalizedValue` is computed as `toNormalized(default)`, not written as a - literal** — a grep finds no normalized default constant. If the exactness fails, it is a - W1-T1 defect surfacing here, not a defect of this track. **Round-trip exactness at - arbitrary values is NOT asserted** — it is not required (§6.7.7) and asserting it would - over-constrain the taper. -- **`getParamStringByValue` prints what the editor's knob label prints**, digit for digit, at - the same stored value, across every unit category — asserted by calling **the same - formatter** from both sides in one test, not by comparing two independently produced - strings. **A grep finds exactly one formatter per unit category** and no `snprintf` of a - parameter value outside it. -- **Every exposed parameter's `units` and plain range match §6.7.1**, asserted per parameter; - `stepCount` is asserted **zero on all 44**. -- **A host automation lane moving a Live parameter moves a sounding note; a lane moving a - NoteOnLatched parameter takes effect on the next note and does NOT trigger a reload or an - engine rebuild** — assert the tier, not just the sound. All three NoteOnLatched controls now - have a delivery test in `tests/test_live_delivery.cpp` (rate, key-track, Trigger length), each - asserting BOTH halves: the sounding note byte-identical, the next note taking the value. -- **A host automation point's AUTHORITY IS BOUNDED**, and its release is where both review passes - went wrong: it outranks the model only until the UI thread has folded it in and republished. - `shell/instrument/CLAUDE.md`'s Authority section states the model, `param_merge` enforces it, - `test_param_merge` fails if a hold is never released or is released too early. -- **No automation path reaches `reloadInstrument` or `rebuildVoiceEngine`.** -- **A project saved before this change opens with every parameter reading the blob's value - and sounds identical**; a project saved by this build opens in an older binary with its - sound intact; and a project with automation drawn, saved and reopened, replays against the - same plain values. -- **`process()` takes no new indirection and no new per-sample work** — the parameter read is - a block-boundary act. **Correction to the original wording ("on the existing live-publish - path"):** it cannot be, and the SDK is what decides that. `IParameterChanges` is delivered ON - the audio thread, and the model's publish path allocates (`resolvePlay` copies velocity curves - and spline contours), so the drain lands in `process()` and patches the live block in place. - The block-boundary rule is unchanged and the per-sample path is untouched; what moved is which - thread performs the fold. -- **The bake's reset notifies the host**, verified by the host's displayed value following it - rather than snapping back on next touch. -- **The double-processing limitation is documented, not discovered** — a bake whose - reset-class parameter carries a host lane is a named boundary of the bake's fidelity claim - (§9), stated in the product doc and in this track's review. -- **The lane-linearity consequence is stated in the header, not left to be found** — under a - tapered parameter a straight line drawn in a host automation lane is **not** linear in the - plain unit (exponential in ms, linear in octaves on cutoff, linear in dB on master gain). - This is standard and desirable, it follows directly from Ruling 3 plus the taper, and §6.7.4 - gives it per category. Writing it down is the acceptance criterion; changing the taper to - avoid it is not an option. -- **The filter's four are proven untouched**: a regression baseline shows their audio - unchanged, and their persisted `*Norm` values are byte-identical across a save/reload that - passes through the parameter surface. Reporting Hz/Q/depth changed display only. - -**Open questions.** -- **No [Daniel] questions. Γ-F7 is RULED — signal flow** (2026-08-01, *"signal flow order."*), - and Ruling 3 (real units) arrived specified rather than forked. **There is no unanswered - [Daniel]-class question in this track or anywhere in this plan.** -- **CLOSED from the SDK, not the DAW.** Two things were bundled here and they separate. - (a) The DELIVERY CHANNEL: `ivsteditcontroller.h` documents `setParamNormalized` as the - GUI-update channel ("should update the according GUI element(s) only"), and the SDK's own - `SingleComponentEffect` sample (`public.sdk/samples/vst/again/source/againsimple.cpp`) drains - `ProcessData::inputParameterChanges` in `process()` while also implementing - `setParamNormalized`. **Both are serviced.** This was never a DAW question — the headers - answer it, and building on the paragraph alone is exactly what the first pass did. - (b) The ORDERING of `setState` against the first parameter block: no longer a question, but not - for the reason the second pass gave. A point held by the audio thread is re-applied over every - merge only until the UI thread has folded it into the model — the hold is bounded, and a lane - that is genuinely DRIVING re-sends and so keeps outranking the restore, while a lane that sent - one point and had it folded does not. That is the correct reading of the rule "a lane in - read/write mode outranks a plug-in-side set" (stated in `docs/product/parameter-automation.md` - §6.1 as reasoning from the host's replay behaviour, not from a header — the SDK does not spell - it out). The second pass's unbounded latch made the claim true by making every later writer - permanently deaf; `shell/instrument/CLAUDE.md`'s Authority section is the model now. - (c) **STILL OPEN, and it is the half the bundled `[verify, FIRST]` originally asked**: that - REAPER calls `setState` (not `setComponentState`) on a single-component plug-in — a state - ENTRY-POINT question, not a delivery-channel one. Recorded in `docs/TODO.md` rather than closed: - `vstsinglecomponenteffect.h:41-47` does collapse the names as §6.1 claims, our overrides land on - the `IComponent` pair with `setEditorState`/`getEditorState` left at the base's `kNotImplemented`, - and the blob has shipped through payload v1…v16, so the behaviour is very likely fine — but that - is inference, not observation. The rest of the DAW work is in `docs/TODO.md` too, and none of it - can change the frozen contract. -- **[verify]** whether REAPER renders `ParameterInfo::units` beside the string - `getParamStringByValue` returns, or shows the string alone. **We ship the SDK's own - convention** — digits in the string, unit carried separately, which is what - `RangeParameter::toString` and the `Parameter` constructor's signature both express. If - REAPER shows no unit at all, the fallback is to append the unit **inside the one formatter**: - a one-line change in one place, touching neither the frozen id table nor the editor, because - display strings are explicitly not frozen (§6.7.1). Do not discover this after shipping. -- **RULED (Daniel, 2026-08-02): promote both.** **Key-track** and **Trigger length** move from - `Reload` to `NoteOnLatched` (§7.4) and are exposed; ids **1000** and **1450** issue and the - count is **44 of 44**. **The promotion is NOT the predicate-only change this bullet originally - advertised** — the predicate flip is the smallest part of it. Key-track lives on - `InstrumentParams`, not `PlaySeconds`, so the host's write path (`setDeckParam`/`deckParamNorm`) - structurally could not see it and id 1000 would have no-oped in both directions with no - compile-time guard; both controls also had to reach the engine, which meant widening - `LiveValues` and `foldLive`'s input and handing `Voice::start` the two latched values as - arguments beside the rate. The guard that closes the class is `param::valueHomeFor` — asserted - over the exposed set (every control has a home, the instance-scalar set has exactly two - members) AND branched on by the shell's own read and write paths, so the three cannot drift. -- **[propose at review]** whether to ship a default `IMidiMapping` CC table here or leave MIDI - control to REAPER's host-side learn. Either is defensible; **skipping it silently is not.** -- **[propose at review]** whether this track spends the reserved payload rung. §6.1 says - nothing new is persisted and therefore it should not; if the `setState` verification says - otherwise, it takes the reserved rung and says so. -- **Closed, do not reopen:** Rate lifted from latched to live (§3.5 records the cost); the - limiter enable made automatable (§3.8 — its one reopening condition was the reload/activation - decoupling, which landed in Γ-W3, so the condition is discharged rather than pending). +**Not verified in a running DAW — CTest-asserted only.** `docs/TODO.md` carries the +residual DAW-verification items. ---