diff --git a/docs/PLAN.md b/docs/PLAN.md index 72dcffe..6d8a291 100644 --- a/docs/PLAN.md +++ b/docs/PLAN.md @@ -1277,7 +1277,12 @@ value semantics, any deck geometry, or the bake's reset *membership* (W3-T2's). 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, on the existing live-publish path. + 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 @@ -1297,11 +1302,17 @@ value semantics, any deck geometry, or the bake's reset *membership* (W3-T2's). - **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.** -- **[verify, FIRST]** that REAPER calls `setState` (not `setComponentState`) on a - single-component plug-in, and the ordering of `setState` against the first - `IParameterChanges` block after a project load. §6.1 is built on the SDK's own - name-collapse; **verify it in the DAW before wiring, and do not build on the paragraph - alone.** +- **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. An + automation point held by the audio thread is re-applied over every merge, so a written lane + outranks the restore whichever way round the two arrive — VST3's own rule. What survives as + DAW work is recorded in `docs/TODO.md`, 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 @@ -1309,16 +1320,16 @@ value semantics, any deck geometry, or the bake's reset *membership* (W3-T2's). 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. -- **[propose at review]** promoting **key-track** and **Trigger length** from `Reload` to - `NoteOnLatched` (§7.4). Both are excluded from the live set *for the note-on-latch reason* - in the predicate's own words, so the promotion aligns routing with documented semantics — - and it is what makes them automatable at all. **If either promotion is refused, that - control simply drops out of the parameter list.** The list follows the predicate; the - predicate is never bent to fill the list. **Consequence for the frozen table:** a refusal - drops ids 1000 and 1260 (key-track) or 1450 (Trigger length) and the count falls below 44. - Those slots are then simply **never issued** — not retired, since nothing shipped under - them — and remain available to the same control if it is promoted later. No other id moves; - that is what the block-and-step scheme buys. +- **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. - **[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 diff --git a/docs/TODO.md b/docs/TODO.md index bd0f01f..6deb626 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -819,3 +819,20 @@ doc-keeper edit. **Done looks like.** The enumeration distinguishes "in the project's state" from "on disk in the `.rpp`", and does not gain a second home for the distinction. + +## The VST3 parameter surface's DAW-verifiable claims + +**Context (what shipped — Γ-W4-T1).** The instrument reports 44 automatable parameters under the frozen id table, services both delivery channels (the controller's `setParamNormalized` and the audio thread's `IParameterChanges` drain), and folds automated values back into the blob on the UI thread. + +**What is settled without a DAW.** The channel question itself is answered by the vendored SDK, not by observation: `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`. Servicing both is what the SDK's own precedent does; it needs no verification, only exercise. + +**What genuinely needs a running REAPER, and why none of it can change the design.** Each item below is a host BEHAVIOUR, not a contract — the plug-in is correct under either answer, so discovering the answer costs a display fix at worst: + +1. **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). If REAPER shows no unit at all, the fallback is to append the unit inside the one formatter — one line in one place, touching neither the frozen id table nor the editor, because display strings are explicitly not frozen. +2. **Whether REAPER's own per-parameter MIDI learn covers what a shipped `IMidiMapping` CC table would have.** The decision to ship no default map rests on it; if learn does not reach these parameters, a CC table is additive and frozen by nothing. +3. **That the three migration round trips hold**: a pre-parameter project opens with every parameter reading the blob's value and sounds identical; a project saved by this build restores fully in an older binary; a project with automation drawn, saved and reopened, replays against the same plain values. +4. **That an offline render replays automation** — the sharpest case for the audio-side drain, because the host drives `process()` and may never touch the controller. + +**Priority / risk.** Low. Nothing here is load-bearing on the frozen contract: the id table, the plain ranges and the norm↔plain laws are all decided and tested without a host. + +**Done looks like.** Each of the four exercised once in REAPER, with the unit-rendering answer recorded and, if it went the other way, the one-line formatter change made. diff --git a/docs/product/parameter-automation.md b/docs/product/parameter-automation.md index 0fed5e7..2b46bb9 100644 --- a/docs/product/parameter-automation.md +++ b/docs/product/parameter-automation.md @@ -336,10 +336,18 @@ invariant Θ-W1-T1 was run to establish. > automation and is not a defect to design away — but it has one sharp consequence for the > resample bake, and that is §9. -**[verify] at the track, before wiring:** that REAPER calls `setState` (not -`setComponentState`) on a single-component plug-in, and the ordering of `setState` against -the first `IParameterChanges` block after a project load. Verify against the vendored SDK -and in the DAW — do not build on the paragraph above without it. +**SETTLED at the track, from the vendored SDK.** The delivery question the `[verify]` here +bundled is answered by the headers rather than by the DAW: `setParamNormalized` is documented as +the GUI-update channel (*"should update the according GUI element(s) only"*, +`ivsteditcontroller.h`), and `ProcessData::inputParameterChanges` is the audio-side one — the +SDK's own `SingleComponentEffect` sample services BOTH +(`public.sdk/samples/vst/again/source/againsimple.cpp`), and so do we. The `setState` ordering +half dissolves with it: an automation point held by the audio thread is re-applied over every +merge, so a written lane outranks the restore whichever way round the two arrive, which is +VST3's own authority rule rather than a race. **The audio thread cannot run the model path** +(`resolvePlay` copies velocity curves and spline contours, so it allocates), so the drain patches +the live block in place through one pure RT-safe function whose routing is pinned by an +exhaustive equivalence test against the model path. ### 6.2 The ID space: hand-assigned constants in one frozen table @@ -897,11 +905,16 @@ state fits them exactly: - **Trigger length** — *"resolves `playEnd_`, a fact about the note, not a setting of it."* Same shape. -**[propose at review, Γ-W4-T1]** promote both. The promotion aligns the routing with the -predicate's own stated semantics — and it is what makes them automatable, since today they -would re-decode a WAV per automation point. **If either promotion is refused, that control -simply drops out of the parameter list.** The list follows the predicate; the predicate is -never bent to fill the list. +**RULED (Daniel, 2026-08-02): promote both.** Ids 1000 and 1450 issue; the count is 44 of 44. +The promotion aligns the routing with the predicate's own stated semantics — and it is what +makes them automatable, since otherwise they would re-decode a WAV per automation point. + +**It was not the predicate-only change this section implied.** Key-track lives on +`InstrumentParams`, not `PlaySeconds`, so the host's write path could not reach it and id 1000 +would have no-oped in both directions with nothing failing to compile; both controls also had to +reach the engine, which widened `LiveValues` and `foldLive`'s input and gave `Voice::start` the +two latched values as arguments beside the rate. `param::valueHomeFor`, asserted over the +exposed set, is what makes the next promotion of this shape a test failure instead of a silence. **Not promoted, and not proposed for promotion: Rate to Live.** §3.5 records the cost; that paragraph is the first thing to read if it is ever proposed. diff --git a/src/core/instrument/CLAUDE.md b/src/core/instrument/CLAUDE.md index 64f35ef..5563d48 100644 --- a/src/core/instrument/CLAUDE.md +++ b/src/core/instrument/CLAUDE.md @@ -1,8 +1,8 @@ -# src/core/instrument — pure VST3-instrument core (bake / engine / map / note / ui) +# src/core/instrument — pure VST3-instrument core (bake / engine / map / note / param / ui) ## Scope -The ReaSampler 9000 instrument's pure, REAPER-free, VST3-free, unit-tested core, in five +The ReaSampler 9000 instrument's pure, REAPER-free, VST3-free, unit-tested core, in six subdirectories: - **`engine/`** — the polyphonic voice engine, the one set of play params, pitch shifting, diff --git a/src/core/instrument/engine/filter/CMakeLists.txt b/src/core/instrument/engine/filter/CMakeLists.txt index 6765ad0..934fe66 100644 --- a/src/core/instrument/engine/filter/CMakeLists.txt +++ b/src/core/instrument/engine/filter/CMakeLists.txt @@ -1,10 +1,16 @@ # Control mapping, SVF coefficients, morph weights, and the filter type each get their own # TU; VoiceFilter::process stays header-inline so the kernel still inlines at the call site. +# The frozen control-position laws are their own target: they are the filter's PARAMETER +# surface, and the VST3 parameter layer reports Hz/Q/drive through them. Kept separable so +# that consumer does not take a link edge onto the per-voice kernel — the extension's link +# graph must never be able to reach the voice DSP (root CLAUDE.md, the bake invariant). +reasampler_pure_library(filter_params SOURCES filter_params.cpp) + reasampler_pure_library(filter SOURCES - filter_params.cpp filter_coeffs.cpp filter_morph.cpp - voice_filter.cpp) + voice_filter.cpp + LINK PUBLIC filter_params) # Four test targets along the module's own seams so each asserts one domain. filter_tests # alone owns the analytic reference and the steady-state gain measurement — a forked copy of diff --git a/src/core/instrument/engine/live_params.cpp b/src/core/instrument/engine/live_params.cpp index 2153782..6dbb219 100644 --- a/src/core/instrument/engine/live_params.cpp +++ b/src/core/instrument/engine/live_params.cpp @@ -5,8 +5,13 @@ namespace reasampler::instrument::engine { -LiveValues foldLive(const PlayParams& params) { +LiveValues foldLive(const PlayParams& params, double keyTrack) { LiveValues v; + v.keyTrack = keyTrack; + // Folded here, not at the voice: Voice::start reads the block's value directly, so the + // spline rule has to be applied on the way in or the two would answer differently. + v.splineActive = splineActive(params); + v.lengthFraction = effectiveLengthFraction(params); v.filterSettings = params.filter.settings; v.filterModAmount = params.filter.modAmount; v.filterVelAmount = params.filter.velAmount; diff --git a/src/core/instrument/engine/live_params.h b/src/core/instrument/engine/live_params.h index 7874011..23fbab3 100644 --- a/src/core/instrument/engine/live_params.h +++ b/src/core/instrument/engine/live_params.h @@ -52,6 +52,19 @@ struct LiveValues { // ordinarily live. double playRate = 1.0; double pitchOffsetSemitones = 0.0; + // Two more members of playRate's note-on-latched class, here for the same reason it is: + // both resolve a fact the voice fixes at note-on (the pitch ratio, and playEnd_), so live + // delivery would retune or re-span a note already struck. Voice::start receives them as + // arguments; applyLive never touches either. + double keyTrack = kKeyTrackDefault; + // ALREADY spline-folded (effectiveLengthFraction) — a drawn contour is a pure time function + // over the whole sample, so the stored knob is inert while one is active and the block must + // carry what the voice will actually play, not the stored value. + double lengthFraction = 1.0; + // The drawn-EG state the fold above reads. A mode flip travels by reload like the contours + // themselves, so this is not a control; it rides here only so a block-boundary write of + // Trigger length (a host automation point) can apply the SAME fold rather than un-doing it. + bool splineActive = false; }; // The seqlock copies the block as raw bytes, which is only defensible for a plain value type. @@ -59,8 +72,10 @@ static_assert(std::is_trivially_copyable_v, "the live block is copied under a seqlock — it must stay a plain value"); // The ONE derivation of the live block from the parameter set. Every publisher goes through -// here so there is a single site to keep in step with PlayParams. -LiveValues foldLive(const PlayParams& params); +// here so there is a single site to keep in step with PlayParams. `keyTrack` is passed in +// because it belongs to the capture/instrument scalar beside the play bundle, not to +// PlayParams — SampleData::keyTrack at the reload, InstrumentParams::keyTrack at a live commit. +LiveValues foldLive(const PlayParams& params, double keyTrack); // Single-writer / single-reader seqlock. The writer publishes a whole block between an odd // and an even generation; the reader copies the block and re-checks the generation, retrying @@ -96,6 +111,10 @@ public: seq_.store(next, std::memory_order_release); // even: complete and coherent } + // The last generation published, without copying the block — one relaxed load, so a reader + // that only needs "has anything moved" pays nothing for asking on a block where nothing has. + std::uint32_t generation() const { return seq_.load(std::memory_order_relaxed); } + // Copies the block into `out` and returns the generation actually observed, or 0 when // nothing has been published yet or the retry budget ran out (in which case `out` may hold // a torn copy and MUST be discarded — compare the return against 0 before using it). diff --git a/src/core/instrument/engine/master_gain.cpp b/src/core/instrument/engine/master_gain.cpp index b48ba58..806a917 100644 --- a/src/core/instrument/engine/master_gain.cpp +++ b/src/core/instrument/engine/master_gain.cpp @@ -17,6 +17,13 @@ double masterGainMaxLinear() { return std::pow(10.0, kMasterGainMaxDb / 20.0); } double masterGainDbFromNorm(double norm) { norm = clamp01(norm); if (norm <= 0.0) return -std::numeric_limits::infinity(); + // UNITY IS EXACT, and the argument is arithmetic rather than structural — a host's + // reset-to-default arrives here as toPlain(defaultNormalized) and must land on 0.0 dB, not a + // hair off it. fl(60/84) differs from 60/84 by δ ≈ 1.6e-17; 84·δ ≈ 1.33e-15 sits under the + // half-ulp of 60 (3.55e-15), so -60 + fl(60/84)·84 rounds to exactly 60 and the sum to 0. + // PRECONDITION: no FP contraction. Fused into a single FMA the residue survives as 1.33e-15. + // Safe on the shipped MSVC/x64 default (no FMA without /arch:AVX2); a build that enables + // contraction here breaks the exactness test in test_param_units, which is where it surfaces. return kMasterGainMinDb + norm * (kMasterGainMaxDb - kMasterGainMinDb); } diff --git a/src/core/instrument/engine/play_params.h b/src/core/instrument/engine/play_params.h index 8851210..c4229e9 100644 --- a/src/core/instrument/engine/play_params.h +++ b/src/core/instrument/engine/play_params.h @@ -148,6 +148,11 @@ struct FilterParams { // with the pitch envelope's own depth throw so the two pitch modulators speak one range. inline constexpr double kVelocityPitchRangeSemitones = 24.0; +// Standard 12-tone-ET tracking, and the ONE home for that number: the capture's own scalar, the +// instrument's stored scalar and the live block all default from here, so a blob predating the +// field and a block published before the first note can never disagree about it. +inline constexpr double kKeyTrackDefault = 1.0; + // Bundle a voice reads at start(). Defaults reproduce the bare engine (Gate, hold-0 AHDSR, // Varispeed, pitch envelope off, filter off, no velocity->pitch) — core regression tests rely // on this; the Preserve product default is layered on at (de)serialization, see @@ -267,7 +272,7 @@ struct SampleData { // How far keyboard pitch tracks the root: 1.0 = standard 12-tone-ET (default); 0.0 = no // tracking (every key plays root pitch); 2.0 = double-rate. Scales the (note-root) semitone // offset in keyTrackedRatio; rides both repitch engines via the voice's baseRatio_. - double keyTrack = 1.0; + double keyTrack = kKeyTrackDefault; // Maps note-on velocity (0..127) to the voice's amp gain, eval'd once in Voice::start // (never per frame). Default flat y=1 — every velocity plays at unity. diff --git a/src/core/instrument/engine/voice.cpp b/src/core/instrument/engine/voice.cpp index 73d12f8..a8b4e11 100644 --- a/src/core/instrument/engine/voice.cpp +++ b/src/core/instrument/engine/voice.cpp @@ -19,7 +19,7 @@ void Voice::presizePreserveShifters(std::int64_t windowFrames) { } void Voice::start(int note, int velocity, const SampleData& sample, bool declickTakeover, - double stretchRate) { + double stretchRate, double keyTrack, double lengthFraction) { // Before any state reset, record the pre-cut reference (last rendered output) and mark // the compensation pending iff this start is a takeover/steal of a sounding voice and the // caller opted in. The ramp is seeded on the first frame rendered after the restart, from @@ -72,6 +72,7 @@ void Voice::start(int note, int velocity, const SampleData& sample, bool declick // the control outright — the predicate is spelled the same way advanceFrame spells it. preserveRead_ = (pitchEngine_ == PitchEngine::Preserve) && shiftL_.configured(); rateRatio_ = preserveRead_ ? 1.0 : stretchRate_; + keyTrack_ = (keyTrack < 0.0) ? sample.keyTrack : keyTrack; recomputeBaseRatio(); // pitchOffsetRatio_ is a power of 2 and never zero, so this inverse is well-defined — and at // Pitch 0 it is a division by exactly 1.0. @@ -120,10 +121,9 @@ void Voice::start(int note, int velocity, const SampleData& sample, bool declick } else { // Trigger: play [start, playEnd) where playEnd = start + round(frac*(frames-start)) — // map/trigger_seam.h's formula, evaluated inline because the engine does not depend on - // map/. The spline fold is effectiveLengthFraction (play_params.h); a second copy of it - // here is what let a stored-but-inert %-knob shorten the bake while the voice played - // the whole take. - double frac = effectiveLengthFraction(p); + // map/. The caller's value is ALREADY spline-folded (foldLive does it); the snapshot + // fallback folds here, because a stored-but-inert %-knob must not shorten the span. + double frac = (lengthFraction < 0.0) ? effectiveLengthFraction(p) : lengthFraction; if (!(frac > 0.0)) frac = 0.0; // %=0 (or a corrupt NaN) -> finishes immediately if (frac > 1.0) frac = 1.0; std::int64_t playLen = static_cast( diff --git a/src/core/instrument/engine/voice.h b/src/core/instrument/engine/voice.h index e104eb8..7a135e6 100644 --- a/src/core/instrument/engine/voice.h +++ b/src/core/instrument/engine/voice.h @@ -45,6 +45,9 @@ inline double pitchRatio(int note, int rootNote) { // ((note-root)*1.0 is exact in IEEE-754 for an integer-valued double, feeding the same // std::pow call); 0.0 means every key plays the root pitch; 2.0 doubles the tracking rate. // At the root note the offset is 0 regardless of keyTrack. +// "Not supplied" for Voice::start's two snapshot-defaulted note-on latches; see start(). +inline constexpr double kLatchFromSnapshot = -1.0; + inline double keyTrackedRatio(int note, int rootNote, double keyTrack) { const double semis = static_cast(note - rootNote) * keyTrack; return std::pow(2.0, semis / 12.0); @@ -124,8 +127,16 @@ public: // to consult gets exactly that; VoiceEngine::startVoice is what resolves the real value — // sample.play.playRate is NOT read here, because the published block outranks the snapshot's // possibly-stale copy of it. + // + // `keyTrack` and `lengthFraction` are the other two members of stretchRate's note-on-latched + // class and arrive the same way, for the same structural reason. Negative = not supplied, + // which reads the snapshot's own value (sample.keyTrack, effectiveLengthFraction(play)) — + // both are non-negative by domain, so the sentinel can never collide with a real one. + // VoiceEngine::startVoice always supplies them, resolved from the published block when there + // is one; the sentinel is for a caller that has no block to consult. void start(int note, int velocity, const SampleData& sample, bool declickTakeover = false, - double stretchRate = 1.0); + double stretchRate = 1.0, double keyTrack = kLatchFromSnapshot, + double lengthFraction = kLatchFromSnapshot); // Mono legato takeover: re-pitch this active voice to `note` without touching the // amplitude envelope, read position, or shifter state — pitch moves, no re-attack. Both @@ -201,7 +212,7 @@ private: // shifter's transpose. Cold: note-on, legato retune, and a live block, never per frame. void recomputeBaseRatio() { if (sample_ == nullptr) return; - baseRatio_ = keyTrackedRatio(note_, sample_->rootNote, sample_->keyTrack) * + baseRatio_ = keyTrackedRatio(note_, sample_->rootNote, keyTrack_) * velPitchRatio_ * pitchOffsetRatio_ * rateRatio_; } @@ -710,6 +721,10 @@ private: double velPitchRatio_ = 1.0; // the velocity->pitch factor alone; retune re-applies it double pitchOffsetRatio_ = 1.0; // the Pitch knob's factor — LIVE, re-applied by applyLive double rateRatio_ = 1.0; // Rate's factor of the read increment; start() owns when it is 1 + // Key-track, LATCHED at note-on beside the rate. Held here rather than re-read off the + // snapshot so a legato retune and a live block re-apply the note's own value; a published + // move reaches the next note only. + double keyTrack_ = kKeyTrackDefault; // Whether this note is ACTUALLY taking the Preserve read — a Preserve voice whose shifters // were never sized falls back to the varispeed one, and the two domains differ. Latched at // note-on beside rateRatio_, which start() resolves from the same predicate. diff --git a/src/core/instrument/engine/voice_engine.cpp b/src/core/instrument/engine/voice_engine.cpp index 2417d57..f7ab3cf 100644 --- a/src/core/instrument/engine/voice_engine.cpp +++ b/src/core/instrument/engine/voice_engine.cpp @@ -56,9 +56,14 @@ void VoiceEngine::startVoice(Voice& voice, int note, int velocity) { refreshLive(); // THE read of the note-on-latched commit class, and the only one: a published block outranks // the snapshot's own copy (a live edit deliberately leaves that stale), and applyLive below - // never touches the rate — so a Rate move reaches the next note and no sounding one. + // touches none of these three — so a move reaches the next note and no sounding one. const double rate = haveLive_ ? live_.playRate : sample_.play.playRate; - voice.start(note, velocity, sample_, /*declickTakeover=*/takeoverDeclick_, rate); + const double keyTrack = haveLive_ ? live_.keyTrack : sample_.keyTrack; + // Already spline-folded in the block; the snapshot branch folds here so the two agree. + const double lengthFraction = + haveLive_ ? live_.lengthFraction : effectiveLengthFraction(sample_.play); + voice.start(note, velocity, sample_, /*declickTakeover=*/takeoverDeclick_, rate, keyTrack, + lengthFraction); if (haveLive_) voice.applyLive(live_, /*snap=*/true); voice.setStartOrder(nextStartOrder_++); } diff --git a/src/core/instrument/map/play_seconds.h b/src/core/instrument/map/play_seconds.h index 61e9dcb..e570b07 100644 --- a/src/core/instrument/map/play_seconds.h +++ b/src/core/instrument/map/play_seconds.h @@ -5,12 +5,25 @@ // not link the bank model and the WAV codec to reach one value struct. `resolvePlay`, which // turns them into the engine's frame domain, stays in sample_map with the rest of the mapping. +#include + #include "core/instrument/engine/play_params.h" // PlayMode / TriggerParams / SplineEnv / … namespace reasampler::instrument::map { using instrument::engine::VelocityCurve; +// THE seconds -> frames fold, and the one home for its rounding: resolvePlay resolves the whole +// bundle through it, and the audio thread's live patch (param/param_live) resolves one stage +// time through it, so a stage time can never land on a different frame depending on the writer. +// A non-positive rate yields 0 rather than inventing one; a negative time floors at 0. +inline std::int64_t secondsToFrames(double seconds, double sampleRate) { + if (!(sampleRate > 0.0)) return 0; + double f = seconds * sampleRate; + if (!(f > 0.0)) return 0; // also catches NaN + return static_cast(f + 0.5); +} + // Daniel's standing ruling: no hardcoded sample rate anywhere in the program. The // instrument stores/edits wall-clock performance times (AHDSR A/H/D/R, pitch-env A/D) as // SECONDS, rate-free; the engine receives FRAMES resolved from the LIVE sample rate at diff --git a/src/core/instrument/map/sample_map.cpp b/src/core/instrument/map/sample_map.cpp index 25fe0c5..6e912e8 100644 --- a/src/core/instrument/map/sample_map.cpp +++ b/src/core/instrument/map/sample_map.cpp @@ -215,11 +215,7 @@ PlayParams resolvePlay(const PlaySeconds& stored, int sampleRate) { // carries through untouched, already a fraction. assert(sampleRate > 0 && "resolvePlay: sampleRate must be > 0 (programming error)"); const double sr = sampleRate > 0 ? static_cast(sampleRate) : 1.0; // 1.0 avoids div-by-zero; assert fires first - const auto secToFrames = [sr](double sec) { - double f = sec * sr; - if (f < 0.0) f = 0.0; - return static_cast(f + 0.5); - }; + const auto secToFrames = [sr](double sec) { return secondsToFrames(sec, sr); }; // The one seconds->frames fold for a stored AHD; the fraction and the curves are rate-free. const auto resolveAhd = [&secToFrames](const AhdSeconds& s) { AhdParams a; diff --git a/src/core/instrument/map/sample_map.h b/src/core/instrument/map/sample_map.h index 0dd6172..ae1be19 100644 --- a/src/core/instrument/map/sample_map.h +++ b/src/core/instrument/map/sample_map.h @@ -180,7 +180,7 @@ struct InstrumentParams { // exactly 1.0, so already-saved instances are bit-identical. 0.0 = no tracking (every // key plays root pitch); 2.0 = double. Applied in keyTrackedRatio inside both repitch // engines. - double keyTrack = 1.0; + double keyTrack = kKeyTrackDefault; // Velocity->amp transfer curve: maps note-on MIDI velocity (0..127) to voice amp gain, // replacing the old fixed linear velocity/127. Default = flat y=1 (Daniel-approved): @@ -215,7 +215,7 @@ struct InstrumentParams { struct ResolvedCapture { std::string relativePath; // project-relative; the shell resolves + decodes it int rootNote = 60; // effective: override, else bank intrinsic, else 60 - double keyTrack = 1.0; + double keyTrack = kKeyTrackDefault; VelocityCurve velocityCurve = VelocityCurve::flat(); SampleLoop loop; // effective: loopOverride, else bank intrinsic std::int64_t loopCrossfadeFrames = 0; // instrument-owned; no bank intrinsic to beat diff --git a/src/core/instrument/param/CLAUDE.md b/src/core/instrument/param/CLAUDE.md index c65fe4b..f6635d9 100644 --- a/src/core/instrument/param/CLAUDE.md +++ b/src/core/instrument/param/CLAUDE.md @@ -11,6 +11,12 @@ these onto `Steinberg::Vst::Parameter`; it decides nothing. A sixth peer of `engine/` / `map/` / `note/` / `bake/` / `ui/`, and it sits ABOVE `ui/`: the parameter list is a function of `deckParamCommit` and the value binding, never the reverse. +**Where an exposed control's value lives is `valueHomeFor`'s answer, and the exposed set is +asserted against it.** Two controls sit beside the parameter set rather than in it — master gain +(the processor's atomic) and pitch key-track (`InstrumentParams::keyTrack`) — and a promotion +whose control has no home would no-op silently in both directions on the host path with nothing +to catch it at compile time. That is exactly what happened to id 1000 before the guard existed. + ## Invariants ### The id table is FOREVER-FROZEN @@ -57,6 +63,13 @@ no longer exist. - `param_units` — `UnitKind`, `unitStringFor`, `plainRangeFor`, the `toPlain` / `toNormalized` pair, and the defaults read off a default-constructed `PlaySeconds`. - `param_format` — the eight formatters and the digits parser behind `getParamValueByString`. +- `param_live` — the AUDIO-THREAD half: one exposed control patched into the live block in + place, allocation-free and lock-free, for the host's `IParameterChanges` queue. It exists + because the model layer cannot run there (`PlaySeconds` carries velocity curves and spline + contours, so `resolvePlay` allocates) while the queue is delivered there. Every law is called — + `ui::storedFromNorm` and `map::secondsToFrames` are the same two the model path uses; what is + new is the ROUTING, and that is pinned by an exhaustive equivalence test against the model path + over every exposed control rather than by two tables that happen to agree. ## Gotchas @@ -68,18 +81,33 @@ no longer exist. - **Round-trip exactness at arbitrary values is NOT a property here and must not be asserted.** No log map satisfies `toNormalized(toPlain(n)) == n` in double, and demanding it would rule out the taper the range needs. Exactness is required at the defaults; monotonicity everywhere. -- **A curve exponent inside the knob detent but not exactly neutral reads `1.00` to the host.** - The detent lives in `curve_law`'s norm↔exponent map and the host's only handle is the norm, so - the host cannot see an off-detent near-neutral exponent — reachable only by an overlay knot - drag, which writes through `curveFromLevelAt` rather than the knob law. The editor's own label - deliberately reads the stored field directly and still shows the true value; that divergence is - structural to VST3, not a formatter defect. +- **A curve exponent inside the knob detent but not exactly neutral is NEUTRALIZED by any host + touch — a VALUE consequence, not a display one.** The detent lives in `curve_law`'s + norm↔exponent map and the host's only handle is the norm, so the host reads such an exponent + back as `1.00` (the editor's own label reads the stored field and still shows the true value). + The sharp half is the WRITE: a host write of that norm reaches `ui::setDeckParam` → + `storedFromNorm` → `util::curveFromKnobNorm`, whose ±0.01 detent rewrites the stored exponent + to exactly `1.0`. So an off-detent near-neutral exponent set by an overlay knot drag is + silently flattened by any host touch or lane pass over that parameter. + **Assessed and ACCEPTED, not merely documented:** the alternative is to widen the exposed + parameter's law so the detent band is addressable, and §6.3 freezes that law on the first + shipped build — a permanent change to twelve parameters' normalization, to preserve a + difference the user cannot see on the knob (the detent exists precisely because a drag cannot + land on the identity reliably) and cannot hear (the band is ±0.047 of the exponent). Removing + the detent from the WRITE path alone would leave the knob unable to reach the identity, which + is the defect it was added for. The residual is confined to knot-drawn near-neutral curves. - **Master gain's plain value at norm 0 is `-inf`**, which is outside the declared −60…+24 range on purpose — norm 0 is true silence, not the floor. The formatter prints `-inf` there. -- **The filter's four store their position as a `float`, so a not-yet-stored norm can display - one digit differently.** A host previewing a value it has sent but that has not round-tripped - through the stored float differs from the editor by up to a float ulp; at a value landing - exactly on a display rounding boundary that is worth one integer percent on morph. Both - surfaces read the MODEL in every settled state, so this is a transient of the write itself, - not a standing divergence — `test_param_format` holds those four to the plain value rather - than to the string for exactly this reason. +- **MORPH ALONE can display one digit differently from a not-yet-stored norm.** The filter's four + store their position as a `float`, but cutoff, Q and drive cast the incoming norm to `float` + *inside* `toPlain`, so `toPlain(n)` and `toPlain(double(float(n)))` are bit-identical and those + three are held to digit-for-digit string equality like everything else. Morph's path is + full-double (`clamp01(n) * 100`), so the float the model stores and the double the host holds + are genuinely different inputs — worth one integer percent at a value landing on a display + rounding boundary. Both surfaces read the MODEL in every settled state, so it is a transient of + the write itself, not a standing divergence; `test_param_format` holds morph alone to the plain + value rather than to the string. +- **A host write the MODEL clamps is not a settled state either.** Trigger length's stored + domain is `(0,1]`, so a host norm of 0 comes back as 0.01. `setParamNormalized` caches what the + model took, so the host never holds the rejected value — the sweep skips the clamped steps for + that reason rather than loosening its comparison. diff --git a/src/core/instrument/param/CMakeLists.txt b/src/core/instrument/param/CMakeLists.txt index 97c95eb..ada0656 100644 --- a/src/core/instrument/param/CMakeLists.txt +++ b/src/core/instrument/param/CMakeLists.txt @@ -5,13 +5,28 @@ reasampler_test(param_id LINK param_id) # The norm <-> plain layer. deck_values carries the tapers' full scales and the two field # resolvers the defaults are read through; filter_params and master_gain are the frozen laws the -# filter's four and the gain report through, CALLED rather than restated. +# filter's four and the gain report through, CALLED rather than restated. filter_params rather +# than the whole `filter` target: this is the parameter surface, and a link edge from it onto the +# per-voice filter KERNEL would put the voice DSP in reach of any future extension-side consumer +# of param_format — which root CLAUDE.md's bake invariant forbids. reasampler_pure_library(param_units SOURCES param_units.cpp - LINK PUBLIC deck_values param_taper curve_law master_gain filter) -reasampler_test(param_units LINK param_units param_id) + LINK PUBLIC deck_values param_taper curve_law master_gain filter_params) +# sample_map for the test alone: the host-vs-editor default agreement reads the two instance +# scalars where they LIVE, and one of them is a field of InstrumentParams. +reasampler_test(param_units LINK param_units param_id sample_map) reasampler_pure_library(param_format SOURCES param_format.cpp LINK PUBLIC param_units) -# param_id is linked for the test only: the one-formatter-two-consumers assertion sweeps the -# exposed set, which is identity's answer rather than this module's. -reasampler_test(param_format LINK param_format param_id) +# param_id and sample_map are linked for the test only: the one-formatter-two-consumers assertion +# sweeps the exposed set (identity's answer, not this module's) and reads pitch key-track where it +# lives, on InstrumentParams. +reasampler_test(param_format LINK param_format param_id sample_map) + +# The audio thread's half: the live block plus the two laws it patches through. No engine — +# the block is a value, not a thing the voice owns. +reasampler_pure_library(param_live + SOURCES param_live.cpp + LINK PUBLIC deck_values live_params) +# sample_map for the test alone: the equivalence assertion drives the MODEL path +# (setDeckParam -> resolvePlay -> foldLive) as its reference. +reasampler_test(param_live LINK param_live param_id param_units sample_map) diff --git a/src/core/instrument/param/param_live.cpp b/src/core/instrument/param/param_live.cpp new file mode 100644 index 0000000..37c044c --- /dev/null +++ b/src/core/instrument/param/param_live.cpp @@ -0,0 +1,105 @@ +// param_live.cpp — see param_live.h. Three field resolvers plus one dispatch; every law is +// called, none is restated. + +#include "core/instrument/param/param_live.h" + +#include "core/instrument/map/play_seconds.h" // secondsToFrames (resolvePlay's own fold) +#include "core/instrument/ui/deck_values.h" // storedFromNorm (setDeckParam's own map) + +namespace reasampler::instrument::param { + +namespace { + +using engine::LiveValues; + +// The block member a control names, in the same shape deck_values' two field resolvers take: +// LOCATION only, no law. Null for a control the block does not carry. +std::int64_t* frameField(LiveValues& v, DeckParam deck) { + switch (deck) { + case DeckParam::kAttack: return &v.adsr.attackFrames; + case DeckParam::kHold: return &v.adsr.holdFrames; + case DeckParam::kDecay: return &v.adsr.decayFrames; + case DeckParam::kRelease: return &v.adsr.releaseFrames; + case DeckParam::kTrigAttack: return &v.ampAhd.attackFrames; + case DeckParam::kTrigDecay: return &v.ampAhd.decayFrames; + case DeckParam::kPitchEnvAttack: return &v.pitchEnv.shape.attackFrames; + case DeckParam::kPitchEnvDecay: return &v.pitchEnv.shape.decayFrames; + case DeckParam::kFilterEnvAttack: return &v.filterEnv.attackFrames; + case DeckParam::kFilterEnvHold: return &v.filterEnv.holdFrames; + case DeckParam::kFilterEnvDecay: return &v.filterEnv.decayFrames; + case DeckParam::kFilterEnvRelease: return &v.filterEnv.releaseFrames; + case DeckParam::kFilterTrigAttack: return &v.filterAhd.attackFrames; + case DeckParam::kFilterTrigDecay: return &v.filterAhd.decayFrames; + default: return nullptr; + } +} + +// The filter's four, which store their normalized position as float in the block exactly as the +// parameter set stores it. +float* normField(LiveValues& v, DeckParam deck) { + switch (deck) { + case DeckParam::kFilterMorph: return &v.filterSettings.morphNorm; + case DeckParam::kFilterCutoff: return &v.filterSettings.cutoffNorm; + case DeckParam::kFilterQ: return &v.filterSettings.resonanceNorm; + case DeckParam::kFilterDrive: return &v.filterSettings.driveNorm; + default: return nullptr; + } +} + +double* doubleField(LiveValues& v, DeckParam deck) { + switch (deck) { + case DeckParam::kSustain: return &v.adsr.sustainLevel; + case DeckParam::kAttackCurve: return &v.adsr.attackCurve; + case DeckParam::kDecayCurve: return &v.adsr.decayCurve; + case DeckParam::kReleaseCurve: return &v.adsr.releaseCurve; + case DeckParam::kTrigHold: return &v.ampAhd.holdFraction; + case DeckParam::kTrigAttackCurve: return &v.ampAhd.attackCurve; + case DeckParam::kTrigDecayCurve: return &v.ampAhd.decayCurve; + case DeckParam::kPitchEnvHold: return &v.pitchEnv.shape.holdFraction; + case DeckParam::kPitchEnvAttackCurve: return &v.pitchEnv.shape.attackCurve; + case DeckParam::kPitchEnvDecayCurve: return &v.pitchEnv.shape.decayCurve; + case DeckParam::kPitchEnvDepth: return &v.pitchEnv.peakSemitones; + case DeckParam::kFilterEnvSustain: return &v.filterEnv.sustainLevel; + case DeckParam::kFilterEnvAttackCurve: return &v.filterEnv.attackCurve; + case DeckParam::kFilterEnvDecayCurve: return &v.filterEnv.decayCurve; + case DeckParam::kFilterEnvReleaseCurve: return &v.filterEnv.releaseCurve; + case DeckParam::kFilterTrigHold: return &v.filterAhd.holdFraction; + case DeckParam::kFilterTrigAttackCurve: return &v.filterAhd.attackCurve; + case DeckParam::kFilterTrigDecayCurve: return &v.filterAhd.decayCurve; + case DeckParam::kFilterModAmt: return &v.filterModAmount; + case DeckParam::kFilterVel: return &v.filterVelAmount; + case DeckParam::kFilterKeyTrack: return &v.filterKeyTrack; + case DeckParam::kRate: return &v.playRate; + case DeckParam::kPitch: return &v.pitchOffsetSemitones; + case DeckParam::kKeyTrack: return &v.keyTrack; + default: return nullptr; + } +} + +} // namespace + +bool applyLiveParam(LiveValues& block, DeckParam deck, double normalized, int sampleRate) { + // Trigger length is the one control the block does not carry verbatim: what it publishes is + // the SPLINE-FOLDED fraction, so a write while a contour is active must be inert here for + // the same reason the knob is inert in the editor. + if (deck == DeckParam::kTrigLength) { + if (!block.splineActive) block.lengthFraction = ui::storedFromNorm(deck, normalized); + return true; + } + if (std::int64_t* f = frameField(block, deck)) { + *f = map::secondsToFrames(ui::storedFromNorm(deck, normalized), + static_cast(sampleRate)); + return true; + } + if (float* f = normField(block, deck)) { + *f = static_cast(ui::storedFromNorm(deck, normalized)); + return true; + } + if (double* f = doubleField(block, deck)) { + *f = ui::storedFromNorm(deck, normalized); + return true; + } + return false; +} + +} // namespace reasampler::instrument::param diff --git a/src/core/instrument/param/param_live.h b/src/core/instrument/param/param_live.h new file mode 100644 index 0000000..1c20bf7 --- /dev/null +++ b/src/core/instrument/param/param_live.h @@ -0,0 +1,29 @@ +// param_live.h — the AUDIO-THREAD half of a host parameter write: one exposed control patched +// into the live block, in place, with no allocation and no lock. It exists because the model +// layer cannot run on the audio thread (PlaySeconds carries velocity curves and spline contours, +// so resolvePlay allocates), while `IParameterChanges` is delivered there. + +#pragma once + +#include "core/instrument/engine/live_params.h" +#include "core/instrument/ui/deck_groups.h" // DeckParam + +namespace reasampler::instrument::param { + +using ui::DeckParam; + +// Writes `normalized` for `deck` into `block`. RT-SAFE: no allocation, no lock, no transcendental +// beyond the taper's own. Returns false for a control this block does not carry — master gain, +// which reaches the audio as the processor's own atomic, and anything unexposed. +// +// The value laws are NOT restated here: `ui::storedFromNorm` is the same norm -> stored map +// `setDeckParam` writes with, and `map::secondsToFrames` the same fold `resolvePlay` uses. What +// IS new is the routing — which member of the block a control names — and that is pinned by an +// exhaustive equivalence test against the model path over every exposed control, rather than by +// two tables that happen to agree. +// +// `sampleRate` is the rate the loaded capture was BUILT at (the processor's builtSampleRate_), +// so a patched stage time lands on exactly the frames the build would have resolved. +bool applyLiveParam(engine::LiveValues& block, DeckParam deck, double normalized, int sampleRate); + +} // namespace reasampler::instrument::param diff --git a/src/core/instrument/param/param_units.cpp b/src/core/instrument/param/param_units.cpp index ffd659b..bcd20bc 100644 --- a/src/core/instrument/param/param_units.cpp +++ b/src/core/instrument/param/param_units.cpp @@ -66,10 +66,49 @@ UnitKind unitKindFor(DeckParam deck) { return UnitKind::Decibels; case DeckParam::kFilterCutoff: return UnitKind::Hertz; - default: - // The twelve curve exponents, Q and drive. Everything else has no row at all. + // The twelve curve exponents and the filter's two dimensionless tone controls. Listed + // rather than defaulted, and everything with no parameter row at all is listed with + // them: a `default:` here would let a control promoted later inherit Dimensionless + // silently, and §6.3 freezes an exposed parameter's normalization on the first shipped + // build — so the wrong answer would be permanent rather than correctable. + case DeckParam::kFilterQ: + case DeckParam::kFilterDrive: + case DeckParam::kAttackCurve: + case DeckParam::kDecayCurve: + case DeckParam::kReleaseCurve: + case DeckParam::kTrigAttackCurve: + case DeckParam::kTrigDecayCurve: + case DeckParam::kPitchEnvAttackCurve: + case DeckParam::kPitchEnvDecayCurve: + case DeckParam::kFilterEnvAttackCurve: + case DeckParam::kFilterEnvDecayCurve: + case DeckParam::kFilterEnvReleaseCurve: + case DeckParam::kFilterTrigAttackCurve: + case DeckParam::kFilterTrigDecayCurve: + case DeckParam::kPlayMode: + case DeckParam::kPitchEngine: + case DeckParam::kPitchEnvEnable: + case DeckParam::kFilterEnable: + case DeckParam::kFilterLaw: + case DeckParam::kAmpVelCurve: + case DeckParam::kPitchVelCurve: + case DeckParam::kFilterVelCurve: + case DeckParam::kAmpEnvSelect: + case DeckParam::kPitchEnvSelect: + case DeckParam::kFilterEnvSelect: + case DeckParam::kAmpEnvMode: + case DeckParam::kPitchEnvMode: + case DeckParam::kFilterEnvMode: + case DeckParam::kVoiceCount: + case DeckParam::kVoiceMode: + case DeckParam::kMonoTrigger: + case DeckParam::kLimiterEnable: + case DeckParam::kMasterMeter: + case DeckParam::kMasterGr: + case DeckParam::kCount: return UnitKind::Dimensionless; } + return UnitKind::Dimensionless; // unreachable for a valid enumerator; silences a warning. } const char* unitStringFor(DeckParam deck) { @@ -192,9 +231,18 @@ double toNormalized(DeckParam deck, double plain) { return plain; } -bool storesNormalized(DeckParam deck) { +ValueHome valueHomeFor(DeckParam deck) { PlaySeconds defaults; - return ui::deckFloatField(deck, defaults) != nullptr; + if (ui::deckFloatField(deck, defaults)) return ValueHome::ParamSetNorm; + if (ui::deckDoubleField(deck, defaults)) return ValueHome::ParamSet; + if (deck == DeckParam::kMasterGain || deck == DeckParam::kKeyTrack) { + return ValueHome::InstanceScalar; + } + return ValueHome::None; +} + +bool storesNormalized(DeckParam deck) { + return valueHomeFor(deck) == ValueHome::ParamSetNorm; } double defaultPlain(DeckParam deck) { @@ -204,10 +252,17 @@ double defaultPlain(DeckParam deck) { if (const float* stored = ui::deckFloatField(deck, defaults)) { return toPlain(deck, static_cast(*stored)); } + // The two instance scalars, whose default is not a field of PlaySeconds. if (deck == DeckParam::kMasterGain) return 0.0; // unity, and the sharpest exactness case + if (deck == DeckParam::kKeyTrack) { + return kKeyTrackDefault * kPercentFullScale; + } const double* field = ui::deckDoubleField(deck, defaults); if (!field) return 0.0; switch (unitKindFor(deck)) { + // Time converts seconds -> ms here and ms -> seconds in toNormalized, so its exactness + // additionally rests on x*1000/1000 == x — param_taper guarantees its quantum in SECONDS, + // not in ms. It holds for today's three Time defaults; a new one is a case to re-check. case UnitKind::Time: return *field * 1000.0; // stored seconds case UnitKind::PercentUnipolar: return *field * kPercentFullScale; case UnitKind::PercentKeyTrack: return *field * kPercentFullScale; // stored 0..2 diff --git a/src/core/instrument/param/param_units.h b/src/core/instrument/param/param_units.h index 58b3c88..bbe2ce5 100644 --- a/src/core/instrument/param/param_units.h +++ b/src/core/instrument/param/param_units.h @@ -48,6 +48,17 @@ PlainRange plainRangeFor(DeckParam deck); double toPlain(DeckParam deck, double normalized); double toNormalized(DeckParam deck, double plain); +// WHERE a control's value actually lives. The host's read and write paths branch on this, and +// the exposed set is asserted against it: a control promoted into the list with no home would +// otherwise no-op silently in BOTH directions, with nothing to catch it at compile time. +enum class ValueHome { + None, // not a scalar control at all — a toggle, a radio, a curve-popup cell + ParamSetNorm, // the filter's four: the stored double IS the normalized position + ParamSet, // every other knob the parameter set carries + InstanceScalar, // beside the parameter set: master gain, and the pitch key-track scalar +}; +ValueHome valueHomeFor(DeckParam deck); + // The filter's four tone controls STORE their normalized position (payload v9), so their default // normalized value is that stored double verbatim and no taper participates in a host's // reset-to-default. Reporting Hz / Q / drive depth for them means CALLING their frozen laws, not diff --git a/src/core/instrument/ui/deck_groups.cpp b/src/core/instrument/ui/deck_groups.cpp index 8988a5a..4f51259 100644 --- a/src/core/instrument/ui/deck_groups.cpp +++ b/src/core/instrument/ui/deck_groups.cpp @@ -201,8 +201,10 @@ DeckParam curveParamFor(DeckParam knob) { LiveCommit deckParamCommit(DeckParam id) { switch (id) { - // The one note-on-latched control; the header owns why. + // The note-on-latched controls; the header owns why each one latches. case DeckParam::kRate: + case DeckParam::kKeyTrack: + case DeckParam::kTrigLength: return LiveCommit::NoteOnLatched; // Live by the tier's own definition — one atomic store the audio thread picks up at the // next block, no bridge read and no re-decode. It reaches the audio beside the live @@ -256,9 +258,7 @@ LiveCommit deckParamCommit(DeckParam id) { // to non-live. Reasons live in the header. case DeckParam::kPlayMode: case DeckParam::kPitchEngine: - case DeckParam::kTrigLength: case DeckParam::kPitchEnvEnable: - case DeckParam::kKeyTrack: case DeckParam::kFilterEnable: case DeckParam::kAmpVelCurve: case DeckParam::kPitchVelCurve: diff --git a/src/core/instrument/ui/deck_values.cpp b/src/core/instrument/ui/deck_values.cpp index f2d61d6..66e4b49 100644 --- a/src/core/instrument/ui/deck_values.cpp +++ b/src/core/instrument/ui/deck_values.cpp @@ -89,6 +89,49 @@ double deckParamNorm(DeckParam id, const PlaySeconds& play) { } } +double storedFromNorm(DeckParam id, double norm) { + switch (id) { + // The filter's four STORE their normalized position (payload v9), so the identity IS + // their law — deckFloatField's four, and the reason it is a separate resolver. + case DeckParam::kFilterMorph: + case DeckParam::kFilterCutoff: + case DeckParam::kFilterQ: + case DeckParam::kFilterDrive: + return clamp01(norm); + case DeckParam::kRate: + return rateRatioFromNorm(norm, kRateMinRatio, kRateMaxRatio); + case DeckParam::kPitch: + case DeckParam::kPitchEnvDepth: + return depthSemitonesFromNorm(norm, kPitchDepthMaxSemis); + case DeckParam::kFilterModAmt: + case DeckParam::kFilterVel: + return deckBipolarFromNorm(norm); + case DeckParam::kKeyTrack: + case DeckParam::kFilterKeyTrack: + return keyTrackFromNorm(norm); + case DeckParam::kTrigLength: + // lengthFraction is (0,1]; a small floor so a zero-length trigger never plays + // nothing. + return (std::max)(0.01, clamp01(norm)); + default: + break; + } + // The rest are decided by the display unit alone, which is what makes the fourteen stage + // times and the twelve curve dials one line each rather than twenty-six. + switch (deckParamUnit(id)) { + case UnitCategory::Milliseconds: return timeSecondsFromNorm(norm); + case UnitCategory::Exponent: return util::curveFromKnobNorm(norm); + case UnitCategory::Percent: return clamp01(norm); // hold fractions, sustain levels + // Decibels is master gain, whose stored value is a LINEAR gain the processor owns + // rather than a field of the parameter set; the two enums above are handled by id. + case UnitCategory::Semitones: + case UnitCategory::Decibels: + case UnitCategory::None: + break; + } + return norm; +} + void setDeckParam(DeckParam id, PlaySeconds& play, double value, int segment) { switch (id) { case DeckParam::kPlayMode: @@ -112,88 +155,24 @@ void setDeckParam(DeckParam id, PlaySeconds& play, double value, int segment) { case DeckParam::kPitchEngine: play.pitchEngine = (segment == 1) ? PitchEngine::Preserve : PitchEngine::Varispeed; break; - case DeckParam::kRate: - play.playRate = rateRatioFromNorm(value, kRateMinRatio, kRateMaxRatio); break; - case DeckParam::kPitch: - play.pitchOffsetSemitones = depthSemitonesFromNorm(value, kPitchDepthMaxSemis); break; - case DeckParam::kAttack: play.adsr.attackSeconds = timeSecondsFromNorm(value); break; - case DeckParam::kHold: play.adsr.holdSeconds = timeSecondsFromNorm(value); break; - case DeckParam::kDecay: play.adsr.decaySeconds = timeSecondsFromNorm(value); break; - case DeckParam::kSustain: play.adsr.sustainLevel = clamp01(value); break; - case DeckParam::kRelease: play.adsr.releaseSeconds = timeSecondsFromNorm(value); break; - case DeckParam::kAttackCurve: play.adsr.attackCurve = util::curveFromKnobNorm(value); break; - case DeckParam::kDecayCurve: play.adsr.decayCurve = util::curveFromKnobNorm(value); break; - case DeckParam::kReleaseCurve: play.adsr.releaseCurve = util::curveFromKnobNorm(value); break; - case DeckParam::kTrigLength: - // lengthFraction is (0,1]; keep a small floor so a zero-length trigger never plays - // nothing. - play.trigger.lengthFraction = (std::max)(0.01, clamp01(value)); - break; - case DeckParam::kTrigAttack: play.trigAhd.attackSeconds = timeSecondsFromNorm(value); break; - case DeckParam::kTrigHold: play.trigAhd.holdFraction = clamp01(value); break; - case DeckParam::kTrigDecay: play.trigAhd.decaySeconds = timeSecondsFromNorm(value); break; - case DeckParam::kTrigAttackCurve: - play.trigAhd.attackCurve = util::curveFromKnobNorm(value); break; - case DeckParam::kTrigDecayCurve: - play.trigAhd.decayCurve = util::curveFromKnobNorm(value); break; case DeckParam::kPitchEnvEnable: play.pitchEnv.enabled = (segment == 1); break; - case DeckParam::kPitchEnvAttack: - play.pitchEnv.shape.attackSeconds = timeSecondsFromNorm(value); break; - case DeckParam::kPitchEnvHold: - play.pitchEnv.shape.holdFraction = clamp01(value); break; - case DeckParam::kPitchEnvDecay: - play.pitchEnv.shape.decaySeconds = timeSecondsFromNorm(value); break; - case DeckParam::kPitchEnvAttackCurve: - play.pitchEnv.shape.attackCurve = util::curveFromKnobNorm(value); break; - case DeckParam::kPitchEnvDecayCurve: - play.pitchEnv.shape.decayCurve = util::curveFromKnobNorm(value); break; - case DeckParam::kPitchEnvDepth: - play.pitchEnv.peakSemitones = depthSemitonesFromNorm(value, kPitchDepthMaxSemis); - break; case DeckParam::kFilterEnable: play.filter.enabled = (segment == 1); break; case DeckParam::kFilterLaw: play.filter.settings.morphLaw = (segment == 1) ? MorphLaw::HighNotchLow : MorphLaw::HighBandLow; break; - case DeckParam::kFilterMorph: - play.filter.settings.morphNorm = static_cast(clamp01(value)); break; - case DeckParam::kFilterCutoff: - play.filter.settings.cutoffNorm = static_cast(clamp01(value)); break; - case DeckParam::kFilterQ: - play.filter.settings.resonanceNorm = static_cast(clamp01(value)); break; - case DeckParam::kFilterDrive: - play.filter.settings.driveNorm = static_cast(clamp01(value)); break; - case DeckParam::kFilterModAmt: play.filter.modAmount = deckBipolarFromNorm(value); break; - case DeckParam::kFilterVel: play.filter.velAmount = deckBipolarFromNorm(value); break; - case DeckParam::kFilterKeyTrack: - play.filter.keyTrack = clamp01(value) * kKeyTrackMax; break; - case DeckParam::kFilterEnvAttack: - play.filter.env.attackSeconds = timeSecondsFromNorm(value); break; - case DeckParam::kFilterEnvHold: - play.filter.env.holdSeconds = timeSecondsFromNorm(value); break; - case DeckParam::kFilterEnvDecay: - play.filter.env.decaySeconds = timeSecondsFromNorm(value); break; - case DeckParam::kFilterEnvSustain: - play.filter.env.sustainLevel = clamp01(value); break; - case DeckParam::kFilterEnvRelease: - play.filter.env.releaseSeconds = timeSecondsFromNorm(value); break; - case DeckParam::kFilterEnvAttackCurve: - play.filter.env.attackCurve = util::curveFromKnobNorm(value); break; - case DeckParam::kFilterEnvDecayCurve: - play.filter.env.decayCurve = util::curveFromKnobNorm(value); break; - case DeckParam::kFilterEnvReleaseCurve: - play.filter.env.releaseCurve = util::curveFromKnobNorm(value); break; - case DeckParam::kFilterTrigAttack: - play.filter.trigEnv.attackSeconds = timeSecondsFromNorm(value); break; - case DeckParam::kFilterTrigHold: - play.filter.trigEnv.holdFraction = clamp01(value); break; - case DeckParam::kFilterTrigDecay: - play.filter.trigEnv.decaySeconds = timeSecondsFromNorm(value); break; - case DeckParam::kFilterTrigAttackCurve: - play.filter.trigEnv.attackCurve = util::curveFromKnobNorm(value); break; - case DeckParam::kFilterTrigDecayCurve: - play.filter.trigEnv.decayCurve = util::curveFromKnobNorm(value); break; - default: break; + default: + // Every knob: the one norm -> stored law, into the one field the control names. + // Both halves are shared with the audio thread's live patch (param/param_live), so + // a control cannot take a different taper or land in a different field depending on + // which surface wrote it. A toggle or a value living outside PlaySeconds resolves to + // neither field and falls through untouched. + if (float* f = deckFloatField(id, play)) { + *f = static_cast(storedFromNorm(id, value)); + } else if (double* d = deckDoubleField(id, play)) { + *d = storedFromNorm(id, value); + } + break; } // ONE normalization point for every control that can flip splineActive — a mode toggle // (above) or an enable toggle (kPitchEnvEnable/kFilterEnable), whose enabling can make an diff --git a/src/core/instrument/ui/deck_values.h b/src/core/instrument/ui/deck_values.h index 1ddd276..4303ac9 100644 --- a/src/core/instrument/ui/deck_values.h +++ b/src/core/instrument/ui/deck_values.h @@ -11,6 +11,7 @@ #include "core/instrument/ui/deck_groups.h" // DeckParam #include "core/instrument/ui/envelope_overlay.h" // kGateStageMaxSeconds #include "core/instrument/ui/param_taper.h" // UnitCategory + the shared tapers +#include "core/util/clamp01.h" namespace reasampler::instrument::ui { @@ -28,6 +29,13 @@ inline constexpr double kPitchDepthMaxSemis = kVelocityPitchRangeSemitones; // Key-track knob ceiling (0..200%), shared by the pitch and filter key-track controls. inline constexpr double kKeyTrackMax = 2.0; +// The pitch key-track scalar lives beside the play bundle (on InstrumentParams / SampleData), +// so its two conversions cannot ride the PlaySeconds binding below. One home for them anyway: +// the editor knob, the host's write path and the live fold would otherwise each spell the +// division out. +inline double keyTrackFromNorm(double norm) { return util::clamp01(norm) * kKeyTrackMax; } +inline double keyTrackNormFrom(double keyTrack) { return util::clamp01(keyTrack / kKeyTrackMax); } + // Rate's range: ALIASES of the stretcher's own measured ratio bounds, so the knob's ends are the // engine's clamp rather than a second opinion of it. The taper takes them as arguments for the // same reason the depth taper takes its throw — engine/time_stretch.h owns the numbers. @@ -41,6 +49,12 @@ inline constexpr double kRateMaxRatio = engine::kStretchRateMax; // shell reads those from the processor. double deckParamNorm(DeckParam id, const PlaySeconds& play); +// The STORED value a knob's normalized position maps to — the norm -> value half of the binding +// on its own, because the audio thread needs it without a PlaySeconds to write into +// (`param/param_live`). setDeckParam IS this composed with the field lookup below, so the two +// cannot carry different tapers. Answers `norm` unchanged for a control with no stored scalar. +double storedFromNorm(DeckParam id, double norm); + // Applies a committed interaction: a knob's normalized `value`, or a toggle's `segment` (0/1). // Mutates `play` in place, touching exactly the one field the control names. void setDeckParam(DeckParam id, PlaySeconds& play, double value, int segment); diff --git a/src/shell/instrument/CLAUDE.md b/src/shell/instrument/CLAUDE.md index cc8a260..698829c 100644 --- a/src/shell/instrument/CLAUDE.md +++ b/src/shell/instrument/CLAUDE.md @@ -98,22 +98,34 @@ pure half — the frozen id table, the exposed set, the plain-value layer, the f next touch re-imposing, a superseded value. Master gain has its own funnel (`setMasterGainLinear`) because it is the one exposed control that does not ride the parameter set. -- **`setState` ordering against the host's first parameter block is irrelevant by - construction.** There is one model and one funnel per control, so whichever writes last wins - and the host's display follows the model either way — the ordering is not assumed, it is - removed as a question. -- **`process()` reads no parameter queue and is unchanged by the parameter surface.** A host - write arrives on the UI/main thread and reaches the audio thread through the SAME live block - the editor's knobs publish into, observed once per `render()` — block boundaries, last write - wins. `[verify — DAW]` that REAPER delivers automation to a single-component plug-in through - `IEditController::setParamNormalized` and not through `ProcessData::inputParameterChanges` - alone; if it is the latter only, an RT-safe drain is required and `process()` is where it - would have to land. +- **BOTH delivery channels are serviced, and the audio-side one is the normative one.** + `IEditController::setParamNormalized` is the CONTROLLER channel — the SDK says a controller + "should update the according GUI element(s) only" there, so nothing about the audio may depend + on a host calling it. `ProcessData::inputParameterChanges` is the AUDIO channel, and the SDK's + own single-component sample (`public.sdk/samples/vst/again/source/againsimple.cpp`) drains it + in `process()` while also implementing `setParamNormalized`. We do both, for the same reason. +- **The audio thread is the sole writer of the block the ENGINE reads.** Two `LiveParams` + blocks: the model's publishers (editor commits, reload, `setState`) write `liveParams_` off + the audio thread and may allocate on the way; `process()` merges that block with the host's + automation points into `automationLive_`, which is what `SampleData::live` points at. Two + blocks rather than one because the seqlock's single-writer contract is load-bearing and the two + writers genuinely differ in thread. The merge republishes ONLY when either side moved, so a + block carrying neither costs one relaxed load and the engine's read shape is unchanged. +- **The automation values fold back into the model on the UI thread** (`drainAutomationToModel`, + called from `getState`, the editor's sync tick, and the bake's reload tail). The blob is + authoritative, so a value that never came back would be lost on save. The fold is suppressed + from notifying the host — the values came FROM it, and echoing them would let a lane in write + mode re-record its own playback. +- **`setState` does not need an ordering guarantee against the host's first parameter block.** + An automation point held by the audio thread is re-applied over every merge, so a written lane + outranks the restore whichever way round the two arrive — which is VST3's own rule, not a race + we lost. - **`IMidiMapping` is deliberately NOT implemented** — no conventional CC names most of what is exposed, an invented map would hijack CCs the user's controller already sends, and - REAPER's own per-parameter MIDI learn covers the case without freezing anything. - `IParameterFunctionName` and `IAutomationState` are assessed and not implemented; the reasons - are in the product spec and are not re-surveyed here. + `[verify — DAW]` REAPER's own per-parameter MIDI learn is expected to cover the case without + freezing anything. `IParameterFunctionName` and `IAutomationState` are assessed and not + implemented — `bake/CLAUDE.md` owns the `IAutomationState` reasoning, at its one consequence + site. **Non-goals / guardrails.** - The instrument never captures and never inserts into the arrange. Playback is a @@ -151,6 +163,13 @@ pure half — the frozen id table, the exposed set, the plain-value layer, the f ## Gotchas +- **`reasampler_processor.h` is a documented ~600-line-ceiling exception** (root `CLAUDE.md`, + structural heuristic 1), on the same footing as `voice.h`'s: it is ONE class declaration, so + the seam the heuristic asks for does not exist — a split would be an arbitrary bisection, and + the implementation is already split across three TUs on its real seams. Its bulk is the + drain-slot proof, the RT-discipline constraints and the two-block automation contract, all of + which the comment conventions name as keep-worthy. Not silent overshoot. + - **The bake click only ARMS; the editor's sync tick runs it.** Calling `Main_OnCommandEx` inline from `WM_LBUTTONDOWN` would run the extension's whole landing nested inside a mouse handler with `SetCapture` held, while the invoked action re-points diff --git a/src/shell/instrument/CMakeLists.txt b/src/shell/instrument/CMakeLists.txt index 13994fd..0b43c81 100644 --- a/src/shell/instrument/CMakeLists.txt +++ b/src/shell/instrument/CMakeLists.txt @@ -90,7 +90,7 @@ if(WIN32 AND EXISTS "${VST3_SDK}/public.sdk/source/main/pluginfactory.cpp") waveform_view loop_marks bank_sync browser_scroll param_slider tooltip theme component_geometry bank_grid trigger_seam envelope_overlay envelope_edit knob_deck deck_groups deck_values curve_popup spline_edit master_gain sample_usage - param_id param_units param_format + param_id param_units param_format param_live limiter meter_accumulate meter_ballistics master_meter bake_hold file_bytes curve_law stroke_aa curve_tessellate diff --git a/src/shell/instrument/editor_controls.cpp b/src/shell/instrument/editor_controls.cpp index 93b057b..43fb78f 100644 --- a/src/shell/instrument/editor_controls.cpp +++ b/src/shell/instrument/editor_controls.cpp @@ -1,13 +1,14 @@ // editor_controls.cpp — the ReaSamplerEditor's parameter plumbing: the band-stack layout // resolve every paint/hit-test path shares, the shell's half of the control-value binding (the // per-instance controls the parameter set does not carry — key-track, voice count, master gain, -// preview velocity — plus the value labels), and the node-drag clamp bounds. The parameter-set -// half is the pure `deck_values` module. The orthogonal half — which stored struct each editor -// selection names — is editor_models. Value logic only: no painting, no window plumbing. +// preview velocity — plus each knob's plain value and its label), and the node-drag clamp +// bounds. The parameter-set half is the pure `deck_values` module. The orthogonal half — which +// stored struct each editor selection names — is editor_models. Value logic only. #include "shell/instrument/reasampler_editor.h" #include +#include // isfinite (the gain's -inf label) #include #include // snprintf (deck value labels) #include @@ -241,11 +242,20 @@ std::string ReaSamplerEditor::deckValueLabel(int id) const { // The digits come from the ONE formatter; everything the editor adds around them is static // chrome — a constant prefix or suffix cannot diverge from what the host shows. + const double plain = deckPlainValue(id); char digits[24]; - instrument::param::formatPlainFor(deck, deckPlainValue(id), digits, sizeof(digits)); + instrument::param::formatPlainFor(deck, plain, digits, sizeof(digits)); + const auto kind = instrument::param::unitKindFor(deck); const char* caret = instrument::ui::deckParamUnit(deck) == instrument::ui::UnitCategory::Exponent ? "^" : ""; - return caret + std::string(digits) + instrument::param::unitStringFor(deck); + // The gain at true silence reads "-inf", not "-infdB": there is no decibel value there. + if (kind == instrument::param::UnitKind::Decibels && !std::isfinite(plain)) { + return std::string(digits); + } + // The stage times are the one category that carries a space before its unit, and always did — + // this surface's own typography, not the host's (ParameterInfo::units is the bare string). + const char* gap = kind == instrument::param::UnitKind::Time ? " " : ""; + return caret + std::string(digits) + gap + instrument::param::unitStringFor(deck); } EnvClampBounds ReaSamplerEditor::envClampBounds() const { diff --git a/src/shell/instrument/editor_input_waveform.cpp b/src/shell/instrument/editor_input_waveform.cpp index 64cfcee..7b2860b 100644 --- a/src/shell/instrument/editor_input_waveform.cpp +++ b/src/shell/instrument/editor_input_waveform.cpp @@ -119,6 +119,10 @@ bool ReaSamplerEditor::mouseDownWaveform(const FaceLayout& fl, int x, int y) { dragStartEnv_ = env; dragSampleFrames_ = frames; dragStartParams_ = params_; + // Peer of mouseDownDeck's bracket. LATCHING with no id named, because a node or + // knot drag can move more than one exposed parameter and the grab cannot know + // which; the release and capture-lost paths close it generically. + if (processor_) processor_->beginParamGestureLatch(); return true; // node moves once the cursor drags } return splineOverlayClick(overlay, x, y, gesture, /*addOnEmptySpace=*/false); diff --git a/src/shell/instrument/editor_platform.cpp b/src/shell/instrument/editor_platform.cpp index 020721f..0500fcf 100644 --- a/src/shell/instrument/editor_platform.cpp +++ b/src/shell/instrument/editor_platform.cpp @@ -120,6 +120,11 @@ void ReaSamplerEditor::attachedToParent() { } void ReaSamplerEditor::removedFromParent() { + // The processor outlives this view, so a bracket left open here would leave the host holding + // an edit forever and every later internal write to that parameter would emit a bare + // performEdit. The capture-lost path normally closes it; this does not rely on Windows + // delivering WM_CAPTURECHANGED before the window goes away. + if (processor_) processor_->endParamGesture(); if (childHwnd_) { KillTimer(childHwnd_, kSyncTimerId); // stop the poll before the window goes away DestroyWindow(childHwnd_); diff --git a/src/shell/instrument/editor_session.cpp b/src/shell/instrument/editor_session.cpp index 699058e..e7f7335 100644 --- a/src/shell/instrument/editor_session.cpp +++ b/src/shell/instrument/editor_session.cpp @@ -141,6 +141,10 @@ void ReaSamplerEditor::onSyncTimer() { // edit surface exactly as a reload would. Unconditional: it self-cancels when nothing is // armed, so no commit site has to remember to ask for it. processor_->flushLatencyRestart(); + // Peers of it: both deliver work the originating thread could not do where it stood — a host + // callback from inside reloadMutex_, and a model write from the audio thread. + processor_->flushGainNotify(); + processor_->drainAutomationToModel(); // Resolve the bake affordance's availability on the SAME tick that paints it, so it // can never be enabled on one tick and refuse on the next. diff --git a/src/shell/instrument/instrument_bake.cpp b/src/shell/instrument/instrument_bake.cpp index 3c2e206..1a0c967 100644 --- a/src/shell/instrument/instrument_bake.cpp +++ b/src/shell/instrument/instrument_bake.cpp @@ -120,6 +120,9 @@ BakeChainResult runBake(ReaSamplerProcessor& processor) { const std::optional tempo = Tempo::fromBpm(bridge.projectTempoBpm()); if (!tempo) return fail("the project tempo could not be read"); + // Fold anything the host's automation wrote into the model FIRST: the bake renders the sound + // the user approved, and an automated value the model has not picked up yet is part of it. + processor.drainAutomationToModel(); const InstrumentParams dialed = processor.instrumentParams(); const int rootNote = dialed.rootOverride ? *dialed.rootOverride : source->rootNote; diff --git a/src/shell/instrument/instrument_params.cpp b/src/shell/instrument/instrument_params.cpp index 446a18e..882ba19 100644 --- a/src/shell/instrument/instrument_params.cpp +++ b/src/shell/instrument/instrument_params.cpp @@ -1,17 +1,14 @@ // instrument_params.cpp — the VST3 adapter over core/instrument/param: the Parameter subclass -// whose toPlain/toNormalized ARE the taper, the one construction of the unit and parameter -// lists, and the model projection both directions. It DECIDES nothing — the pure module owns -// the frozen table, the laws and the formatter. -// -// The blob stays authoritative. A parameter is a THIRD SURFACE onto InstrumentParams/PlaySeconds -// — a peer of the deck knob and the overlay node, never a second copy of the value. getState -// serializes the model; the controller's own value list is a cache written FROM the model and -// never read as truth. +// whose toPlain/toNormalized ARE the taper, the construction of the unit and parameter lists, +// the model projection both directions, and both delivery channels (the controller's write and +// the audio thread's queue drain). It DECIDES nothing — the pure module owns the frozen table, +// the laws and the formatter. #include "shell/instrument/reasampler_processor.h" #include "base/source/fstring.h" #include "pluginterfaces/base/ustring.h" +#include "pluginterfaces/vst/ivstparameterchanges.h" // IParameterChanges / IParamValueQueue #include "core/instrument/engine/master_gain.h" #include "core/instrument/param/param_format.h" @@ -123,7 +120,7 @@ tresult PLUGIN_API ReaSamplerProcessor::setParamNormalized(ParamID tag, ParamVal setMasterGainLinear(instrument::engine::masterGainLinearFromNorm(value)); } else { InstrumentParams params = instrumentParams(); - instrument::ui::setDeckParam(row->deck, params.play, value, /*segment=*/0); + writeDeckParamToModel(params, row->deck, value); setInstrumentParams(params); publishLiveParams(); } @@ -134,11 +131,25 @@ tresult PLUGIN_API ReaSamplerProcessor::setParamNormalized(ParamID tag, ParamVal tag, modelParamNormalized(instrumentParams(), row->deck)); } +void ReaSamplerProcessor::writeDeckParamToModel(InstrumentParams& params, DeckParam deck, + double normalized) { + // The two homes a control's value can have, and the ONE place the host path knows the + // difference — the editor draws the same split at applyParamControl. param::valueHomeFor is + // the predicate; a promotion whose control has neither home fails param_units' own test + // rather than no-oping silently here. + if (deck == DeckParam::kKeyTrack) { + params.keyTrack = instrument::ui::keyTrackFromNorm(normalized); + return; + } + instrument::ui::setDeckParam(deck, params.play, normalized, /*segment=*/0); +} + double ReaSamplerProcessor::modelParamNormalized(const InstrumentParams& params, DeckParam deck) const { if (deck == DeckParam::kMasterGain) { return instrument::engine::masterGainNormFromLinear(masterGainLinear()); } + if (deck == DeckParam::kKeyTrack) return instrument::ui::keyTrackNormFrom(params.keyTrack); return instrument::ui::deckParamNorm(deck, params.play); } @@ -152,43 +163,145 @@ void ReaSamplerProcessor::syncParamsFromModel() { } } -void ReaSamplerProcessor::notifyParamsFromModel(const InstrumentParams& before, +void ReaSamplerProcessor::notifyParamsFromModel(const double* beforeNorms, const InstrumentParams& after) { if (paramNotifySuppressed_) return; + // The bake's reset moves ~40 values at once. Grouping them tells the host they are ONE act, + // which is what an undo stack and an automation lane both want; the SDK provides exactly + // this for exactly this case (ivsteditcontroller.h, IComponentHandler2). + const bool group = componentHandler2 && !gestureLatching_; + if (group) componentHandler2->startGroupEdit(); for (const param::ParamRow& row : param::exposedParams()) { if (row.deck == DeckParam::kMasterGain) continue; // its own funnel notifies it const double now = modelParamNormalized(after, row.deck); - if (now == modelParamNormalized(before, row.deck)) continue; + if (now == beforeNorms[static_cast(row.deck)]) continue; notifyParamChanged(row.id, now); } + if (group) componentHandler2->finishGroupEdit(); +} + +bool ReaSamplerProcessor::gestureIsOpen(param::ParamId id) const { + for (std::size_t i = 0; i < openGestureCount_; ++i) { + if (openGestureIds_[i] == id) return true; + } + return false; } void ReaSamplerProcessor::notifyParamChanged(param::ParamId id, double normalized) { EditControllerEx1::setParamNormalized(id, normalized); if (!componentHandler) return; - // A drag holds its own begin/end across the whole gesture so a host in touch or latch mode - // sees one continuous edit; every other writer — a reset, an envelope-node drag, the bake's - // reset — emits a degenerate one-point gesture, which is what makes the host DISPLAY follow - // it instead of re-imposing the pre-write value on the next touch. - const bool inGesture = openGestureId_ == id; - if (!inGesture) beginEdit(id); - performEdit(id, normalized); - if (!inGesture) endEdit(id); -} - -void ReaSamplerProcessor::beginParamGesture(DeckParam deck) { - const param::ParamId id = param::paramIdFor(deck); - if (id == 0 || !param::isExposed(deck)) return; - endParamGesture(); // a grab while one is open cannot leave the previous unclosed - openGestureId_ = id; + if (gestureIsOpen(id)) { + performEdit(id, normalized); + return; + } + if (gestureLatching_ && openGestureCount_ < kMaxOpenGestures) { + // First move this drag has made on this parameter: open its bracket and hold it, so the + // whole drag is one edit rather than a run of one-point ones. + openGestureIds_[openGestureCount_++] = id; + beginEdit(id); + performEdit(id, normalized); + return; + } + // Every non-drag writer — a reset, the bake's reset — emits a degenerate one-point gesture, + // which is what makes the host DISPLAY follow it instead of re-imposing the pre-write value + // on the next touch. beginEdit(id); -} - -void ReaSamplerProcessor::endParamGesture() { - if (openGestureId_ == 0) return; - const param::ParamId id = openGestureId_; - openGestureId_ = 0; // cleared FIRST: endEdit can re-enter through a host's own callback + performEdit(id, normalized); endEdit(id); } +void ReaSamplerProcessor::beginParamGestureLatch() { + endParamGesture(); // a grab while one is open cannot leave the previous unclosed + gestureLatching_ = true; +} + +void ReaSamplerProcessor::beginParamGesture(DeckParam deck) { + beginParamGestureLatch(); + const param::ParamId id = param::paramIdFor(deck); + if (id == 0 || !param::isExposed(deck)) return; + openGestureIds_[openGestureCount_++] = id; + beginEdit(id); +} + +bool ReaSamplerProcessor::drainInputParameterChanges(IParameterChanges* changes) { + if (!changes) return false; + // RT-SAFE, and the one non-obvious part of that: exposedRowFor walks exposedParams(), whose + // backing vector is a function-local static built on FIRST CALL. buildParameterList() calls + // it from initialize(), which the SDK guarantees precedes any process() — so the allocation + // has already happened by the time the audio thread gets here. + bool landed = false; + const int32 queues = changes->getParameterCount(); + for (int32 q = 0; q < queues; ++q) { + IParamValueQueue* queue = changes->getParameterData(q); + if (!queue) continue; + const int32 points = queue->getPointCount(); + if (points <= 0) continue; + // The LAST point of the queue wins for the block. Applying every point at its sample + // offset would put a "did anything change" question on the per-voice-per-sample path, + // which the phase-wide guardrail forbids. + int32 offset = 0; + ParamValue value = 0.0; + if (queue->getPoint(points - 1, offset, value) != kResultTrue) continue; + const param::ParamRow* row = param::exposedRowFor(queue->getParameterId()); + if (!row) continue; + landed = true; + const auto slot = static_cast(row->deck); + automationNorm_[slot] = value; + automationHeld_[slot] = true; + // Master gain reaches the audio beside the block rather than through it, so its + // automation write is the same one relaxed store the knob makes. + if (row->deck == DeckParam::kMasterGain) { + masterGain_.store( + static_cast(instrument::engine::masterGainLinearFromNorm(value)), + std::memory_order_relaxed); + } + // Publish to the UI thread, which folds it back into the model — the blob stays + // authoritative, so a value that never came back would be lost on save. + automationPublished_[slot].store(value, std::memory_order_relaxed); + automationPending_[slot].store(true, std::memory_order_release); + } + if (landed) automationAny_.store(true, std::memory_order_release); + return landed; +} + +void ReaSamplerProcessor::drainAutomationToModel() { + if (!automationAny_.exchange(false, std::memory_order_acquire)) return; + InstrumentParams params = instrumentParams(); + bool moved = false; + for (const param::ParamRow& row : param::exposedParams()) { + const auto slot = static_cast(row.deck); + if (!automationPending_[slot].exchange(false, std::memory_order_acquire)) continue; + const double value = automationPublished_[slot].load(std::memory_order_relaxed); + // Master gain's model IS the atomic the audio thread already wrote; there is nothing to + // fold, only the controller cache to refresh below. + if (row.deck != DeckParam::kMasterGain) { + writeDeckParamToModel(params, row.deck, value); + moved = true; + } + } + // Suppressed for the whole fold: these values CAME from the host, and echoing them back + // through performEdit would let a lane in write mode re-record its own playback. The + // controller cache is still refreshed, so the host's display and the editor follow. + const bool wasSuppressed = paramNotifySuppressed_; + paramNotifySuppressed_ = true; + if (moved) { + setInstrumentParams(params); + publishLiveParams(); + } + syncParamsFromModel(); + paramNotifySuppressed_ = wasSuppressed; +} + +void ReaSamplerProcessor::endParamGesture() { + gestureLatching_ = false; + if (openGestureCount_ == 0) return; + // Latched into a local and the state cleared FIRST: endEdit can re-enter through a host's + // own callback, and must not find a bracket this call is in the middle of closing. + param::ParamId closing[kMaxOpenGestures]; + const std::size_t count = openGestureCount_; + for (std::size_t i = 0; i < count; ++i) closing[i] = openGestureIds_[i]; + openGestureCount_ = 0; + for (std::size_t i = 0; i < count; ++i) endEdit(closing[i]); +} + } // namespace reasampler::vst diff --git a/src/shell/instrument/processor_reload.cpp b/src/shell/instrument/processor_reload.cpp index 865db9a..3e5b05a 100644 --- a/src/shell/instrument/processor_reload.cpp +++ b/src/shell/instrument/processor_reload.cpp @@ -146,16 +146,18 @@ std::string ReaSamplerProcessor::reloadInstrument() { buildFromRef(*sel, params, projectDir, mode)) { sample = std::move(*decoded); havePlayable = true; - // Point the built snapshot at the instance's ONE live block and seed it from - // the very PlayParams the voices latch, so an untouched knob folds to the same - // frames the build resolved and a note-on with a live block sounds identical - // to one without. - sample.live = &liveParams_; + // Point the built snapshot at the instance's ONE engine-facing block and seed the + // MODEL block from the very PlayParams the voices latch, so an untouched knob folds + // to the same frames the build resolved and a note-on with a live block sounds + // identical to one without. process() merges the seed into automationLive_ at the + // top of the first block after this, which is where the swap below becomes visible + // too — so the new snapshot's first note reads it. + sample.live = &automationLive_; { // reloadMutex_ (held for this whole function) nests livePublishMutex_ here; // publishLiveParams never holds reloadMutex_, so this is the only nesting. std::lock_guard lp(livePublishMutex_); - liveParams_.publish(instrument::engine::foldLive(sample.play)); + liveParams_.publish(instrument::engine::foldLive(sample.play, sample.keyTrack)); } builtSampleRate_.store(sample.sampleRate, std::memory_order_relaxed); resolvedId = selId; // the concrete pick that resolved @@ -226,6 +228,8 @@ void ReaSamplerProcessor::adoptBakedCapture(const SampleRefEntry& entry, // bake chain only ever runs from that tick, so the arm would be drained on the next one // anyway. At the tail for the same reason setState's is (see there). flushLatencyRestart(); + // The reset's gain notification, armed under reloadMutex_ inside that reload. + flushGainNotify(); } void ReaSamplerProcessor::publishUsage(const SampleRefs& refs, @@ -289,8 +293,16 @@ void ReaSamplerProcessor::publishBuiltLocked(std::unique_ptr b // gain sitting above every snapshot, the same shape as ONE BLOCK, ONE RATE (see // builtSampleRate_). if (gainAtNextPublish_) { - setMasterGainLinear(*gainAtNextPublish_); + // The MIRROR is inline (that is the sound this publish belongs to); the host + // notification is ARMED and delivered after reloadMutex_ is released. performEdit + // reaches the host handler, a host may re-enter this object synchronously from it, and + // setActive(false) takes this same non-recursive mutex — the identical hazard the + // latency restart is deferred for. + const bool moved = publishMasterGainLinear(*gainAtNextPublish_); gainAtNextPublish_.reset(); + // Armed only on a real change, so a reset that lands on the gain already set writes + // nothing into a host's automation lane — the same compare setMasterGainLinear makes. + if (moved) gainNotifyPending_.store(true, std::memory_order_release); } LoadedInstrument* evicted = draining_.exchange(prev); if (evicted) graveyard_.push_back(std::unique_ptr(evicted)); diff --git a/src/shell/instrument/processor_state.cpp b/src/shell/instrument/processor_state.cpp index 335e6e1..30d63ba 100644 --- a/src/shell/instrument/processor_state.cpp +++ b/src/shell/instrument/processor_state.cpp @@ -91,20 +91,26 @@ tresult PLUGIN_API ReaSamplerProcessor::setState(IBStream* state) { legacyLiftConcluded_.store(false, std::memory_order_relaxed); reloadInstrument(); paramNotifySuppressed_ = false; - // Every exposed parameter now reads the blob's value. Ordering against the host's first - // parameter block is irrelevant BY CONSTRUCTION rather than by assumption: there is one - // model and one funnel per control, so whichever of the two writes last simply wins, and - // the host's display follows the model either way. + // Every exposed parameter now reads the blob's value. Its ordering against the host's first + // parameter block does not need to be known: an automation point held by the audio thread is + // re-applied over every merge, so a lane outranks this restore whichever way round the two + // arrive. That is VST3's own rule — a written lane outranks anything the plug-in sets — not + // a race we lost. syncParamsFromModel(); // This caller has no editor to flush for it. At the TAIL on purpose: a host that services the // restart synchronously deactivates/reactivates, and our setActive(true) resumes or reloads // against the refs above, which are only fully restored once this function has run to here. flushLatencyRestart(); + flushGainNotify(); return kResultOk; } tresult PLUGIN_API ReaSamplerProcessor::getState(IBStream* state) { if (!state) return kResultFalse; + // Before the snapshot, not after: the blob is authoritative, so anything the host's + // automation wrote must be in the model by the time it is serialised. This is the one drain + // site that is not an optimisation — a save with no editor open still has to see it. + drainAutomationToModel(); // Persists the full instance state — never written to the "reasampler" bank ext-state. // No pick serializes to {"", default params}, restoring as silence (never auto-playing // sample #1). @@ -163,10 +169,15 @@ InstrumentParams ReaSamplerProcessor::instrumentParams() { } void ReaSamplerProcessor::setInstrumentParams(const InstrumentParams& params) { - InstrumentParams before; + // The exposed values alone, not the whole set: this funnel fires per mouse move on every live + // knob and node drag, and InstrumentParams owns seven vectors — copying all of them to diff + // 44 doubles is the cost, and the diff is what the notification actually needs. + double before[kDeckParamSlots]; { std::lock_guard lock(paramsMutex_); - before = params_; + for (const instrument::param::ParamRow& row : instrument::param::exposedParams()) { + before[static_cast(row.deck)] = modelParamNormalized(params_, row.deck); + } params_ = params; } // Every writer of the parameter set — setState, the editor's commits, the bake's adopt — @@ -250,8 +261,9 @@ void ReaSamplerProcessor::clearMasterBusClip() { void ReaSamplerProcessor::publishLiveParams() { const int rate = builtSampleRate_.load(std::memory_order_relaxed); if (rate <= 0) return; + const InstrumentParams params = instrumentParams(); const instrument::engine::LiveValues block = - instrument::engine::foldLive(resolvePlay(instrumentParams().play, rate)); + instrument::engine::foldLive(resolvePlay(params.play, rate), params.keyTrack); // livePublishMutex_ enforces the seqlock's single-writer contract (live_params.h) against // reloadInstrument's publish — held for the publish call only, not the fold above. std::lock_guard lock(livePublishMutex_); @@ -329,20 +341,32 @@ void ReaSamplerProcessor::setMonoTrigger(MonoTrigger trigger) { rebuildVoiceEngine(); } -void ReaSamplerProcessor::setMasterGainLinear(double linear) { +bool ReaSamplerProcessor::publishMasterGainLinear(double linear) { // Clamp to the master_gain taper (0 = silence, cap = +24 dB). One relaxed atomic // store — no rebuild, no lock (a post-sum trim is not a keymap fact). if (!(linear >= 0.0)) linear = 0.0; // also catches NaN const double maxLin = masterGainMaxLinear(); if (linear > maxLin) linear = maxLin; const float value = static_cast(linear); - const float previous = masterGain_.exchange(value, std::memory_order_relaxed); + return masterGain_.exchange(value, std::memory_order_relaxed) != value; +} + +void ReaSamplerProcessor::setMasterGainLinear(double linear) { + const bool moved = publishMasterGainLinear(linear); // Gain's own notification funnel — it is the one exposed control that does not ride the // parameter set, so setInstrumentParams' diff cannot see it. Compared for a real change so a // reload's republish of an unmoved gain writes nothing into a host's automation lane. - if (paramNotifySuppressed_ || previous == value) return; + if (paramNotifySuppressed_ || !moved) return; notifyParamChanged(instrument::param::kParamMasterGain, - instrument::engine::masterGainNormFromLinear(linear)); + instrument::engine::masterGainNormFromLinear(masterGainLinear())); +} + +void ReaSamplerProcessor::flushGainNotify() { + if (!componentHandler) return; // an arm raised before the handler connected waits + if (!gainNotifyPending_.exchange(false, std::memory_order_acquire)) return; + if (paramNotifySuppressed_) return; + notifyParamChanged(instrument::param::kParamMasterGain, + instrument::engine::masterGainNormFromLinear(masterGainLinear())); } void ReaSamplerProcessor::previewNoteOn(int note) { diff --git a/src/shell/instrument/reasampler_processor.cpp b/src/shell/instrument/reasampler_processor.cpp index d4d7259..1934dd5 100644 --- a/src/shell/instrument/reasampler_processor.cpp +++ b/src/shell/instrument/reasampler_processor.cpp @@ -16,6 +16,8 @@ #include "pluginterfaces/vst/ivstmidicontrollers.h" // kCtrlAllNotesOff / kCtrlAllSoundsOff (panic) #include "pluginterfaces/vst/vstspeaker.h" +#include "core/instrument/engine/master_gain.h" // the automation write of the gain's own atomic +#include "core/instrument/param/param_live.h" // the RT-safe patch of one control into the block #include "shell/instrument/reasampler_editor.h" // createView hands the host our IPlugView editor #include "shell/instrument/reasampler_embed.h" // embed shell + IReaperUIEmbedInterface (its iid DEF'd there) @@ -200,6 +202,33 @@ tresult PLUGIN_API ReaSamplerProcessor::process(ProcessData& data) { (drain && drain->fullyIdle()) ? drain->installedAt : 0, std::memory_order_relaxed); + // Host automation, merged into the engine-facing block BEFORE the note marshalling below, so + // a note-on in this block latches this block's values. See automationLive_ (header) for why + // the merge is here rather than in the model's own publisher. + { + const bool dirty = drainInputParameterChanges(data.inputParameterChanges); + const std::uint32_t modelGen = liveParams_.generation(); + if (dirty || modelGen != seenModelGeneration_) { + // Declared INSIDE the branch: LiveValues carries default member initializers, so a + // block where nothing moved must not pay to construct one. + instrument::engine::LiveValues merged; + // Re-read the model's fold and re-apply every held automation value over it: without + // the re-apply, any knob move would revert an automated parameter until its lane's + // next point. + if (liveParams_.read(merged) != 0) { + seenModelGeneration_ = modelGen; + const int builtRate = builtSampleRate_.load(std::memory_order_relaxed); + for (std::size_t i = 0; i < kDeckParamSlots; ++i) { + if (!automationHeld_[i]) continue; + instrument::param::applyLiveParam( + merged, static_cast(i), automationNorm_[i], + builtRate); + } + automationLive_.publish(merged); + } + } + } + // Marshal MIDI note-on/off at block granularity (no per-event sample-offset split; // sample-accurate scheduling is a later tier). Note-offs also route to the drain // engine so a note held across a reload releases its old-snapshot voice too. diff --git a/src/shell/instrument/reasampler_processor.h b/src/shell/instrument/reasampler_processor.h index 92395d3..adea0f4 100644 --- a/src/shell/instrument/reasampler_processor.h +++ b/src/shell/instrument/reasampler_processor.h @@ -125,17 +125,12 @@ public: // Hands the host our LICE IPlugView editor. Steinberg::IPlugView* PLUGIN_API createView(Steinberg::FIDString name) override; - // A host write of one exposed parameter. Applies it to THE model through that control's - // existing commit tier — live publish, or the master-gain atomic — and never through a - // fourth route. Nothing reachable from here touches reloadInstrument or rebuildVoiceEngine, - // which is structural rather than careful: every reload- and rebuild-tier control is omitted - // from the parameter list, so no id maps to one. - // - // Parameter values reach the audio thread through the SAME block the editor's knobs publish - // into, which the engine observes ONCE per render() — i.e. at BLOCK BOUNDARIES, last write - // wins for that block. 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. - // process() reads no parameter queue and is unchanged by the parameter surface. + // The CONTROLLER-side write — a host GUI gesture on the generic panel, and whatever a host + // mirrors here for display. Applies it to THE model through that control's existing commit + // tier and never through a fourth route. Nothing reachable from here touches reloadInstrument + // or rebuildVoiceEngine, which is structural rather than careful: every reload- and + // rebuild-tier control is omitted from the parameter list, so no id maps to one. + // NOT the automation channel; this directory's CLAUDE.md owns which channel is which. Steinberg::tresult PLUGIN_API setParamNormalized( Steinberg::Vst::ParamID tag, Steinberg::Vst::ParamValue value) override; @@ -144,10 +139,20 @@ public: // through IComponentHandler. UI/main thread. void syncParamsFromModel(); - // A knob drag's host-edit bracket, so a host in touch or latch mode records ONE continuous - // edit rather than a burst of one-point gestures. Idempotent: a grab while one is open - // closes it first, and endParamGesture with none open does nothing. UI thread only. + // Folds what the audio thread took from the host's parameter queue back into the model and + // the controller cache, suppressing the host notification (those values came FROM it). + // UI/main thread; a no-op when nothing was automated. Called wherever the model is about to + // be READ as authoritative — getState, the bake, the editor's tick. + void drainAutomationToModel(); + + // A drag's host-edit bracket, so a host in touch or latch mode records ONE continuous edit + // per parameter rather than a burst of one-point ones. Every parameter the drag notifies + // opens its bracket on first touch and holds it until endParamGesture — LATCHING rather than + // declared up front, because an envelope-node drag moves a set the grab cannot name. The + // deck-knob form additionally opens its own id at once, which is the id the host sees a touch + // on even if the drag produces no move. Idempotent. UI thread only. void beginParamGesture(instrument::ui::DeckParam deck); + void beginParamGestureLatch(); void endParamGesture(); // Additionally exposes REAPER's IReaperUIEmbedInterface (queried by REAPER to drive the @@ -264,6 +269,9 @@ public: return static_cast(masterGain_.load(std::memory_order_relaxed)); } void setMasterGainLinear(double linear); // clamped to [0, masterGainMaxLinear()] + // The mirror alone, with no host notification: for the one writer that runs under + // reloadMutex_ and must arm rather than emit. True when the value actually moved. + bool publishMasterGainLinear(double linear); // The master-bus limiter's single enable (persisted in the parameter set). UI thread only: // a thin wrapper over setInstrumentParams, the one funnel that mirrors the flag onto the @@ -284,6 +292,11 @@ public: // same tick, so its arm would drain on the next one regardless). void flushLatencyRestart(); + // Delivers the master-gain host notification a reload deferred (the bake's reset gain lands + // under reloadMutex_, and performEdit may re-enter this object). Drained beside the latency + // restart, from the same sites and for the same reason. + void flushGainNotify(); + // Fires a one-shot preview note-on/off through the live VoiceEngine — the same // noteOn/noteOff host MIDI uses, so a preview is a real voice (counts against voice // count, can steal/be stolen, respects Poly/Mono + Retrigger/Legato). Off the audio @@ -306,18 +319,33 @@ private: // initialize(). void buildParameterList(); - // The normalized value a control reads at, from the model — the projection §6.1 calls a - // third surface. Master gain reads the processor's own atomic; everything else reads the - // parameter set through the deck's binding. + // THE automation read, on the audio thread, at the BLOCK BOUNDARY: the last point of each + // queue wins. RT-safe — relaxed atomic stores only. 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. True when at least one point landed, which is what makes the + // merge below it conditional. + bool drainInputParameterChanges(Steinberg::Vst::IParameterChanges* changes); + + // The normalized value a control reads at, from the model — a projection of it, never a + // cached shadow. Master gain reads the processor's own atomic, pitch key-track the scalar + // beside the play bundle; everything else reads the parameter set through the deck's binding. double modelParamNormalized(const InstrumentParams& params, instrument::ui::DeckParam deck) const; - // Notifies the host of every exposed control whose value differs between the two parameter - // sets. Called from setInstrumentParams — the ONE funnel every writer already goes through — - // so no internal write can leave the host displaying, and on next touch re-imposing, a - // superseded value. The bake's reset is the first non-gesture writer this covers. - void notifyParamsFromModel(const InstrumentParams& before, const InstrumentParams& after); + // The write peer of that read, and the ONE host-side write of a control's value: both the + // controller's setParamNormalized and the audio thread's automation fold go through it, so + // neither can miss a control whose value lives outside the parameter set. + static void writeDeckParamToModel(InstrumentParams& params, instrument::ui::DeckParam deck, + double normalized); + + // Notifies the host of every exposed control whose normalized value moved. `beforeNorms` is + // indexed by DeckParam ordinal. Called from setInstrumentParams — the ONE funnel every writer + // already goes through — so no internal write can leave the host displaying, and on next + // touch re-imposing, a superseded value. The bake's reset is the first non-gesture writer + // this covers, and the reason the emission is grouped. + void notifyParamsFromModel(const double* beforeNorms, const InstrumentParams& after); void notifyParamChanged(instrument::param::ParamId id, double normalized); + bool gestureIsOpen(instrument::param::ParamId id) const; // If process() published that the drain instrument is fully idle, move it into the // graveyard and prune — so an edited-away snapshot stops costing memory as soon as its @@ -376,8 +404,13 @@ private: ReaperBridge bridge_; - // The parameter whose drag bracket is currently open, 0 for none. UI thread only. - instrument::param::ParamId openGestureId_ = 0; + // The parameters whose drag bracket is currently open, and whether a drag is in flight at + // all. UI thread only. Past the cap a write degrades to a one-point gesture rather than + // dropping — the pre-bracket behaviour, not a new failure mode. + static constexpr std::size_t kMaxOpenGestures = 8; + instrument::param::ParamId openGestureIds_[kMaxOpenGestures] = {}; + std::size_t openGestureCount_ = 0; + bool gestureLatching_ = false; // Set across setState so a LOAD is not reflected back to the host as an edit. Main thread // only, and non-atomic on purpose: the SDK calls setState there and nowhere else. bool paramNotifySuppressed_ = false; @@ -387,6 +420,26 @@ private: // Both live_ and draining_ observe this same block — a block owned by a snapshot would // leave the drain's still-sounding voices deaf to the knob under them. instrument::engine::LiveParams liveParams_; + // The block the ENGINE reads, and the ONE thing SampleData::live points at. Written only by + // the audio thread, which merges liveParams_ with the host's automation points once per + // block and republishes ONLY when either side moved — so a block carrying neither costs one + // relaxed load and the engine's own read shape is unchanged. Two blocks because the seqlock's + // single-writer contract is load-bearing and the two writers differ in thread; this + // directory's CLAUDE.md owns the argument. + instrument::engine::LiveParams automationLive_; + // Audio thread only. The last liveParams_ generation merged, and the sticky automation values + // re-applied over every merge — without them a model republish (any knob move) would revert + // an automated parameter until its lane's next point. + std::uint32_t seenModelGeneration_ = 0; + static constexpr std::size_t kDeckParamSlots = + static_cast(instrument::ui::DeckParam::kCount); + double automationNorm_[kDeckParamSlots] = {}; + bool automationHeld_[kDeckParamSlots] = {}; + // The audio thread's publication of those values to the UI thread's fold. automationAny_ + // makes the idle drain a single load. + std::atomic automationPublished_[kDeckParamSlots] = {}; + std::atomic automationPending_[kDeckParamSlots] = {}; + std::atomic automationAny_{false}; // Serializes liveParams_.publish's two writer sites (reloadInstrument, publishLiveParams) // only — separate from reloadMutex_ so a knob drag's publish never blocks behind a // reload's WAV decode. The audio thread never takes this; process() only reads via @@ -435,6 +488,10 @@ private: // the bake's reset, which the old capture would be the wrong thing to apply it to. // Guarded by reloadMutex_, consumed by publishBuiltLocked. std::optional gainAtNextPublish_; + // Armed when that deferred gain lands, delivered by flushGainNotify once reloadMutex_ is + // released. Same shape and same reason as latencyRestartPending_ (see it): a host handler + // callback must never run inside this mutex. + std::atomic gainNotifyPending_{false}; // The decoded PCM parked across a deactivate, so an activation cycle costs no disk read // and no WAV decode: activation is "the audio thread may run", not "the sample is diff --git a/tests/test_bake_render.cpp b/tests/test_bake_render.cpp index 4be2fc1..915d3e4 100644 --- a/tests/test_bake_render.cpp +++ b/tests/test_bake_render.cpp @@ -210,7 +210,7 @@ int main() { { PlayParams slow = s.play; slow.trigAhd.attackFrames = 900; - block.publish(instrument::engine::foldLive(slow)); + block.publish(instrument::engine::foldLive(slow, s.keyTrack)); } const BakePlan plan = planOf(/*total=*/1000, /*noteOn=*/0, /*noteOff=*/1000); diff --git a/tests/test_deck_groups_state.cpp b/tests/test_deck_groups_state.cpp index 2703fc0..0fda436 100644 --- a/tests/test_deck_groups_state.cpp +++ b/tests/test_deck_groups_state.cpp @@ -49,9 +49,10 @@ static void testEveryDeckControlIsClassifiedIntoOneOfTheThreeCommitTiers() { // The note-on-latched tier: published like a live control, read only at note-on. Asserted as // its OWN state rather than as "not Reload" — the whole point of widening the predicate is - // that Rate must not fall back into either neighbour, and Γ-W4-T1 reads this classification - // to decide what it exposes to the host. - const DeckParam latched[] = {DeckParam::kRate}; + // that none of these falls back into either neighbour, and the VST3 parameter surface reads + // this classification to decide what it exposes to the host. + const DeckParam latched[] = {DeckParam::kRate, DeckParam::kKeyTrack, + DeckParam::kTrigLength}; for (DeckParam p : latched) CHECK(deckParamCommit(p) == LiveCommit::NoteOnLatched); // Everything else reloads or rebuilds; deck_groups.h is the home for why each exclusion @@ -60,7 +61,6 @@ static void testEveryDeckControlIsClassifiedIntoOneOfTheThreeCommitTiers() { DeckParam::kPlayMode, DeckParam::kPitchEngine, DeckParam::kPitchEnvEnable, DeckParam::kFilterEnable, DeckParam::kFilterLaw, DeckParam::kAmpVelCurve, DeckParam::kPitchVelCurve, DeckParam::kFilterVelCurve, - DeckParam::kKeyTrack, DeckParam::kTrigLength, DeckParam::kAmpEnvSelect, DeckParam::kPitchEnvSelect, DeckParam::kFilterEnvSelect, DeckParam::kAmpEnvMode, DeckParam::kPitchEnvMode, DeckParam::kFilterEnvMode, DeckParam::kVoiceCount, DeckParam::kVoiceMode, @@ -99,7 +99,10 @@ static void testOnlyALiveControlsDragTakesTheLiveTier() { // Rate keeps its own tier through the drag site: it must not arrive as Live (which would let // it move a sounding note) nor as Reload (which would re-decode the WAV under a swept knob). CHECK(knob(DeckParam::kRate) == LiveCommit::NoteOnLatched); - CHECK(knob(DeckParam::kTrigLength) == LiveCommit::Reload); + // Trigger length resolves playEnd_ and key-track the pitch ratio — both facts a voice fixes + // at note-on, which is the latched tier's own definition rather than the reload tier's. + CHECK(knob(DeckParam::kTrigLength) == LiveCommit::NoteOnLatched); + CHECK(knob(DeckParam::kKeyTrack) == LiveCommit::NoteOnLatched); // Master gain is Live and reaches the audio BESIDE the live block rather than through it — // one atomic the audio thread applies as a post-sum multiply. Classifying it Reload would // claim a gain move re-decodes the WAV, which it never did. diff --git a/tests/test_live_delivery.cpp b/tests/test_live_delivery.cpp index a580aa0..28875e9 100644 --- a/tests/test_live_delivery.cpp +++ b/tests/test_live_delivery.cpp @@ -96,7 +96,7 @@ static Run renderWithLive(SampleData& sample, LiveParams* block, int blockFrames int changeAfter, const LiveValues* changed, int noteOffBlock = -1, int velocity = 100) { sample.live = block; - if (block) block->publish(foldLive(sample.play)); + if (block) block->publish(foldLive(sample.play, sample.keyTrack)); VoiceEngine engine(1, sample); engine.noteOn(kTestNote, velocity); Run r; @@ -320,7 +320,7 @@ static void testANoteStartedAfterAPublishSoundsThePublishedEnvelope() { SampleData s = periodicSine(200000, 64.0); // adsr default: attack 0, sustain 1.0 LiveParams block; s.live = █ - LiveValues dialled = foldLive(s.play); + LiveValues dialled = foldLive(s.play, s.keyTrack); dialled.adsr.attackFrames = 24000; // half a second of attack, dialled before the note block.publish(dialled); VoiceEngine engine(1, s); @@ -345,7 +345,7 @@ static void testANoteStartedAfterAPublishSoundsThePublishedEnvelope() { slow.play.adsr.attackFrames = 24000; LiveParams block2; slow.live = &block2; - LiveValues snappy = foldLive(slow.play); + LiveValues snappy = foldLive(slow.play, slow.keyTrack); snappy.adsr.attackFrames = 0; block2.publish(snappy); VoiceEngine fast(1, slow); @@ -368,7 +368,7 @@ static void assertLiveFieldMovesTheSoundingNote(const char* name, void (*rig)(Sa rig(still); rig(moved); LiveParams blockA, blockB; - LiveValues target = foldLive(moved.play); + LiveValues target = foldLive(moved.play, moved.keyTrack); mutate(target); const Run baseline = renderWithLive(still, &blockA, 512, 24, -1, nullptr, noteOffBlock); @@ -616,7 +616,7 @@ static void testEveryLiveFilterControlMovesTheSoundingNote() { SampleData still = filteredSine(); SampleData moved = filteredSine(); LiveParams blockA, blockB; - LiveValues target = foldLive(moved.play); + LiveValues target = foldLive(moved.play, moved.keyTrack); c.mutate(target); const Run baseline = renderWithLive(still, &blockA, 512, 24, -1, nullptr); @@ -688,7 +688,7 @@ static void testOneBlockServesTwoIndependentObservers() { LiveParams block; liveSnapshot.live = █ drainSnapshot.live = █ - block.publish(foldLive(liveSnapshot.play)); + block.publish(foldLive(liveSnapshot.play, liveSnapshot.keyTrack)); VoiceEngine liveEngine(1, liveSnapshot); VoiceEngine drainEngine(1, drainSnapshot); @@ -696,7 +696,7 @@ static void testOneBlockServesTwoIndependentObservers() { drainEngine.noteOn(60, 100); std::vector a, b; - LiveValues moved = foldLive(liveSnapshot.play); + LiveValues moved = foldLive(liveSnapshot.play, liveSnapshot.keyTrack); moved.filterSettings.cutoffNorm = 0.2f; for (int blk = 0; blk < 24; ++blk) { if (blk == 8) block.publish(moved); @@ -746,7 +746,7 @@ static std::vector renderPreserveCapable(SampleData& s, LiveParams& const LiveValues* changed, int changeAfter, int note) { s.live = █ - block.publish(foldLive(s.play)); + block.publish(foldLive(s.play, s.keyTrack)); VoiceEngine engine(1, s, /*preserveVoiceCap=*/0, /*preserveWindowFrames=*/2048); engine.noteOn(note, 100); std::vector out; @@ -765,7 +765,7 @@ static void testARateChangeSpareTheSoundingNoteAndReachesTheNextOne() { moved.play.pitchEngine = eng; LiveParams blockA, blockB; - LiveValues halfRate = foldLive(moved.play); + LiveValues halfRate = foldLive(moved.play, moved.keyTrack); halfRate.playRate = 0.5; // At the ROOT note, so Preserve's shifter runs at shift 1.0 and never splices — the @@ -795,7 +795,7 @@ static void testARateChangeSpareTheSoundingNoteAndReachesTheNextOne() { fresh.play.pitchEngine = eng; LiveParams block; fresh.live = █ - LiveValues published = foldLive(fresh.play); + LiveValues published = foldLive(fresh.play, fresh.keyTrack); published.playRate = rate; block.publish(published); VoiceEngine engine(1, fresh, /*preserveVoiceCap=*/0, /*preserveWindowFrames=*/2048); @@ -827,7 +827,7 @@ static void testAPitchOffsetChangeMovesTheSoundingNoteInBothEngines() { moved.play.pitchEngine = eng; LiveParams blockA, blockB; - LiveValues target = foldLive(moved.play); + LiveValues target = foldLive(moved.play, moved.keyTrack); target.pitchOffsetSemitones = -12.0; const std::vector baseline = @@ -859,7 +859,7 @@ static void testAPitchOffsetChangeMovesTheSoundingNoteInBothEngines() { static std::size_t soundingBlocksWithPublishedPitch(SampleData& s, double offsetSemis, std::size_t capFrames) { LiveParams block; - LiveValues v = foldLive(s.play); // s.play keeps its own (zero) offset: the stale copy + LiveValues v = foldLive(s.play, s.keyTrack); // s.play keeps its own (zero) offset: the stale copy v.pitchOffsetSemitones = offsetSemis; block.publish(v); s.live = █ @@ -891,7 +891,7 @@ static void testAPublishedPitchOffsetLeavesTheStagedAttackWallClock() { s.play.trigAhd = AhdParams{kAttack, 0, 1.0, util::kCurveNeutral, util::kCurveNeutral}; LiveParams block; - LiveValues v = foldLive(s.play); + LiveValues v = foldLive(s.play, s.keyTrack); v.pitchOffsetSemitones = semis; block.publish(v); s.live = █ @@ -960,12 +960,12 @@ static void testPitchRatioAndVelocityGainStayLatched() { LiveParams block; s.live = █ - block.publish(foldLive(s.play)); + block.publish(foldLive(s.play, s.keyTrack)); VoiceEngine engine(1, s); engine.noteOn(72, 64); // an octave up: ratio 2.0 std::vector out; - LiveValues hostile = foldLive(s.play); + LiveValues hostile = foldLive(s.play, s.keyTrack); // Everything the block CAN carry, moved as far as it goes. None of it names velocity, the // note, the pitch ratio, or the PCM — that is the property under test. hostile.filterKeyTrack = 2.0; @@ -998,11 +998,11 @@ static void testPitchRatioAndVelocityGainStayLatched() { SampleData s2 = s; LiveParams block2; s2.live = &block2; - block2.publish(foldLive(s2.play)); + block2.publish(foldLive(s2.play, s2.keyTrack)); VoiceEngine engine2(1, s2); engine2.noteOn(72, 64); std::vector out2; - LiveValues quieter = foldLive(s2.play); + LiveValues quieter = foldLive(s2.play, s2.keyTrack); quieter.adsr.sustainLevel = 0.25; for (int blk = 0; blk < 8; ++blk) { if (blk == 2) block2.publish(quieter); @@ -1024,7 +1024,7 @@ static void testVelocityGainSurvivesAHostilePublishThatReallyLands() { rig.play.pitchEnv.shape.decayFrames = 24000; rig.play.pitchEnv.peakSemitones = 3.0; - LiveValues hostile = foldLive(rig.play); + LiveValues hostile = foldLive(rig.play, rig.keyTrack); hostile.filterKeyTrack = 2.0; hostile.filterSettings.cutoffNorm = 0.9f; hostile.filterModAmount = -1.0; diff --git a/tests/test_live_params.cpp b/tests/test_live_params.cpp index e9b1011..5bf556d 100644 --- a/tests/test_live_params.cpp +++ b/tests/test_live_params.cpp @@ -41,7 +41,9 @@ static void testFoldCarriesEveryContinuousControl() { p.pitchEnv.shape = AhdParams{7, 9, 0.4, 1.5, 0.75}; p.pitchEnv.peakSemitones = -3.5; - const LiveValues v = foldLive(p); + // Distinct from filter.keyTrack below on purpose: the two are different controls and a fold + // that crossed them would pass under a shared value. + const LiveValues v = foldLive(p, /*keyTrack=*/0.8); CHECK(v.adsr.attackFrames == 11); CHECK(v.adsr.holdFrames == 22); CHECK(v.adsr.decayFrames == 33); @@ -72,6 +74,21 @@ static void testFoldCarriesEveryContinuousControl() { CHECK(v.pitchEnv.shape.attackCurve == 1.5); CHECK(v.pitchEnv.shape.decayCurve == 0.75); CHECK(v.pitchEnv.peakSemitones == -3.5); + CHECK(v.keyTrack == 0.8); + // Spline-folded on the way in, so the block carries what the voice will actually play. + CHECK(!v.splineActive); + CHECK(v.lengthFraction == p.trigger.lengthFraction); +} + +// The fold, not the voice, is where a drawn contour pins the Trigger span — so the block a +// note-on latches already carries the folded value. +static void testADrawnEnvelopePinsTheFoldedTriggerLength() { + PlayParams p; + p.trigger.lengthFraction = 0.25; + p.ampSpline.mode = EnvMode::Spline; + const LiveValues v = foldLive(p, kKeyTrackDefault); + CHECK(v.splineActive); + CHECK(v.lengthFraction == 1.0); } static void testUnpublishedBlockReadsAsNothing() { @@ -170,6 +187,7 @@ static void testRampStepIsRateDerived() { int main() { testFoldCarriesEveryContinuousControl(); + testADrawnEnvelopePinsTheFoldedTriggerLength(); testUnpublishedBlockReadsAsNothing(); testConcurrentReaderNeverSeesAHalfAppliedEdit(); testRampTerminatesExactlyOnTheTarget(); diff --git a/tests/test_param_format.cpp b/tests/test_param_format.cpp index 2464271..7748ed6 100644 --- a/tests/test_param_format.cpp +++ b/tests/test_param_format.cpp @@ -1,17 +1,21 @@ // Standalone tests for the ONE formatter per unit category: the digit shapes each category // prints, and the property that makes the editor's knob label and the host's parameter string -// identical — both call THIS function, over a plain value derived the way each surface derives -// it. No VST3, no REAPER, no framework. +// identical — both call THIS function, each over the plain value ITS OWN surface derives (the +// editor's exponent read goes to the stored field, not through the knob law). No VST3, no +// REAPER, no framework. #include "../src/core/instrument/param/param_format.h" #include "../src/core/instrument/engine/master_gain.h" +#include "../src/core/instrument/map/sample_map.h" #include "../src/core/instrument/param/param_id.h" +#include "../src/core/instrument/param/param_units.h" #include "../src/core/instrument/ui/deck_values.h" #include #include #include +#include #include using namespace reasampler; @@ -61,34 +65,40 @@ static void testEachCategoryPrintsItsSpecifiedShape() { CHECK(digits(UnitKind::Dimensionless, 0.1) == "0.10"); } +// The EDITOR's derivation, spelled the way ReaSamplerEditor::deckPlainValue spells it — a curve +// exponent is read off its stored field, never round-tripped through the knob law. Driving the +// sweep through this rather than through toPlain(deckParamNorm(...)) is what makes the exponent +// half of it a real assertion instead of a round trip on both sides. +static double editorPlainValue(DeckParam deck, PlaySeconds& play) { + using reasampler::instrument::ui::UnitCategory; + if (reasampler::instrument::ui::deckParamUnit(deck) == UnitCategory::Exponent) { + const double* stored = reasampler::instrument::ui::deckDoubleField(deck, play); + return stored ? *stored : 0.0; + } + return toPlain(deck, reasampler::instrument::ui::deckParamNorm(deck, play)); +} + static void testTheEditorAndTheHostPrintTheSameDigitsAtTheSameStoredValue() { // The host derives its plain value from the normalized one it holds; the editor derives its - // from the STORED field, through the deck's own read. If those two derivations disagreed at - // any reachable value the two surfaces would print different numbers for one control — this - // is that property, swept over the whole travel of every exposed control. + // from the STORED field. If those two derivations disagreed at any reachable value the two + // surfaces would print different numbers for one control — this is that property, swept over + // the whole travel of every exposed control the parameter set carries. for (const ParamRow& row : exposedParams()) { - if (row.deck == DeckParam::kMasterGain) continue; // not stored in PlaySeconds + if (valueHomeFor(row.deck) == ValueHome::InstanceScalar) continue; // own tests below for (int step = 0; step <= 40; ++step) { const double norm = step / 40.0; PlaySeconds play; reasampler::instrument::ui::setDeckParam(row.deck, play, norm, /*segment=*/0); + const double storedNorm = reasampler::instrument::ui::deckParamNorm(row.deck, play); - char hostBuf[24]; - formatPlainFor(row.deck, toPlain(row.deck, norm), hostBuf, sizeof(hostBuf)); - - const double editorNorm = - reasampler::instrument::ui::deckParamNorm(row.deck, play); - char editorBuf[24]; - formatPlainFor(row.deck, toPlain(row.deck, editorNorm), editorBuf, sizeof(editorBuf)); - - if (storesNormalized(row.deck)) { - // The filter's four store their position as a FLOAT, so a norm the host has sent - // but we have not yet stored differs from the stored one by up to a float ulp. - // At a value landing exactly on a display rounding boundary that is worth one - // digit, so these four are held to the PLAIN value rather than to the string — - // the derivation is still asserted to be one derivation. + if (row.deck == DeckParam::kFilterMorph) { + // MORPH ALONE: its toPlain is full-double (`clamp01(n) * 100`), so the float the + // model stores and the double the host holds are genuinely different inputs, and + // at a value landing on a display rounding boundary that is worth one integer + // percent. Cutoff/Q/drive cast to float INSIDE toPlain, so they are bit-identical + // either way and are held to the string below like everything else. const double hostPlain = toPlain(row.deck, norm); - const double editorPlain = toPlain(row.deck, editorNorm); + const double editorPlain = toPlain(row.deck, storedNorm); const double tolerance = std::fabs(hostPlain) * 1e-6 + 1e-9; if (std::fabs(hostPlain - editorPlain) > tolerance) { std::printf("FAIL param %u at norm %.4f: host %.9g vs editor %.9g\n", @@ -97,6 +107,16 @@ static void testTheEditorAndTheHostPrintTheSameDigitsAtTheSameStoredValue() { } continue; } + // A write the model CLAMPED (Trigger length's (0,1] floor) is not a settled state: + // setParamNormalized caches what the model TOOK, so the host never holds the rejected + // value. Only the four float-stored positions differ by a ulp rather than a clamp. + if (!storesNormalized(row.deck) && storedNorm != norm) continue; + + char hostBuf[24]; + char editorBuf[24]; + formatPlainFor(row.deck, toPlain(row.deck, norm), hostBuf, sizeof(hostBuf)); + formatPlainFor(row.deck, editorPlainValue(row.deck, play), editorBuf, + sizeof(editorBuf)); if (std::strcmp(hostBuf, editorBuf) != 0) { std::printf("FAIL param %u at norm %.4f: host \"%s\" vs editor \"%s\"\n", row.id, norm, hostBuf, editorBuf); @@ -106,6 +126,48 @@ static void testTheEditorAndTheHostPrintTheSameDigitsAtTheSameStoredValue() { } } +// The one place the two surfaces GENUINELY diverge, asserted so it stays a known property rather +// than a surprise: an exponent inside curve_law's centre detent but not exactly neutral is +// reachable only through an overlay knot drag, and the host — which holds the norm and nothing +// else — reads it back as the neutral the knob law snaps to. +static void testAnOffDetentExponentReadsNeutralToTheHostAndTrueToTheEditor() { + PlaySeconds play; + // The detent is +/-0.01 in NORM, which is a ~+/-0.047 band in the exponent — so 1.04 is + // inside it and still prints as a distinct number. + play.adsr.attackCurve = 1.04; + char editorBuf[24]; + formatPlainFor(DeckParam::kAttackCurve, editorPlainValue(DeckParam::kAttackCurve, play), + editorBuf, sizeof(editorBuf)); + CHECK(std::string(editorBuf) == "1.04"); + const double hostNorm = + reasampler::instrument::ui::deckParamNorm(DeckParam::kAttackCurve, play); + char hostBuf[24]; + formatPlainFor(DeckParam::kAttackCurve, toPlain(DeckParam::kAttackCurve, hostNorm), hostBuf, + sizeof(hostBuf)); + CHECK(std::string(hostBuf) == "1.00"); +} + +static void testKeyTrackPrintsTheSameDigitsFromEitherSurface() { + using reasampler::instrument::map::InstrumentParams; + for (int step = 0; step <= 40; ++step) { + const double norm = step / 40.0; + InstrumentParams params; + params.keyTrack = reasampler::instrument::ui::keyTrackFromNorm(norm); + const double editorNorm = reasampler::instrument::ui::keyTrackNormFrom(params.keyTrack); + char hostBuf[24]; + char editorBuf[24]; + formatPlainFor(DeckParam::kKeyTrack, toPlain(DeckParam::kKeyTrack, norm), hostBuf, + sizeof(hostBuf)); + formatPlainFor(DeckParam::kKeyTrack, toPlain(DeckParam::kKeyTrack, editorNorm), editorBuf, + sizeof(editorBuf)); + if (std::strcmp(hostBuf, editorBuf) != 0) { + std::printf("FAIL key-track at norm %.4f: host \"%s\" vs editor \"%s\"\n", + norm, hostBuf, editorBuf); + ++g_fail; + } + } +} + static void testMasterGainPrintsTheSameDigitsFromEitherSurface() { using reasampler::instrument::engine::masterGainLinearFromNorm; using reasampler::instrument::engine::masterGainNormFromLinear; @@ -165,7 +227,9 @@ static void testEveryExposedParameterHasAFormatterThatWritesSomething() { int main() { testEachCategoryPrintsItsSpecifiedShape(); testTheEditorAndTheHostPrintTheSameDigitsAtTheSameStoredValue(); + testAnOffDetentExponentReadsNeutralToTheHostAndTrueToTheEditor(); testMasterGainPrintsTheSameDigitsFromEitherSurface(); + testKeyTrackPrintsTheSameDigitsFromEitherSurface(); testTypingBackADisplayedValueLandsOnIt(); testAShortBufferIsNeverOverrunAndAlwaysTerminates(); testEveryExposedParameterHasAFormatterThatWritesSomething(); diff --git a/tests/test_param_id.cpp b/tests/test_param_id.cpp index 58eefdd..200a70d 100644 --- a/tests/test_param_id.cpp +++ b/tests/test_param_id.cpp @@ -150,22 +150,23 @@ static void testTheExposedSetIsExactlyThePredicateAnswer() { } } -static void testAReservedRowIsNumberedButNotIssued() { - // Key-track (pitch) and Trigger length are note-on-latch candidates that still route through - // the reload tier, so they are NOT issued to the host today. Their numbers stay reserved - // rather than retired: nothing shipped under them, so a later promotion issues the same id - // and no other id moves. +static void testEveryNumberedRowIsIssued() { + // Key-track (pitch) and Trigger length were reserved-but-unissued while they routed through + // the reload tier; both are note-on-latched now, so both are issued under the numbers that + // were held for them and no other id moved — which is what the block-and-step scheme bought. CHECK(paramIdFor(DeckParam::kKeyTrack) == 1000); CHECK(paramIdFor(DeckParam::kTrigLength) == 1450); - CHECK(!isExposed(DeckParam::kKeyTrack)); - CHECK(!isExposed(DeckParam::kTrigLength)); - CHECK(exposedRowFor(1000) == nullptr); - CHECK(exposedRowFor(1450) == nullptr); + CHECK(isExposed(DeckParam::kKeyTrack)); + CHECK(isExposed(DeckParam::kTrigLength)); + CHECK(exposedRowFor(1000) != nullptr); + CHECK(exposedRowFor(1450) != nullptr); // Master gain IS issued: one atomic store the audio thread picks up next block is the live // tier by that tier's own definition. CHECK(isExposed(DeckParam::kMasterGain)); CHECK(exposedRowFor(1700) != nullptr); - CHECK(exposedParams().size() == paramTable().size() - 2); + // The whole table is issued today — 44 of 44. + CHECK(exposedParams().size() == paramTable().size()); + CHECK(exposedParams().size() == 44); } static void testEveryRowCarriesADistinctTitleAndShortTitle() { @@ -186,7 +187,7 @@ int main() { testAnInnerDialSitsBesideTheKnobItShapes(); testABlockCarriesOnlyItsOwnGroup(); testTheExposedSetIsExactlyThePredicateAnswer(); - testAReservedRowIsNumberedButNotIssued(); + testEveryNumberedRowIsIssued(); testEveryRowCarriesADistinctTitleAndShortTitle(); if (g_fail == 0) std::printf("param_id: all tests passed\n"); return g_fail == 0 ? 0 : 1; diff --git a/tests/test_param_live.cpp b/tests/test_param_live.cpp new file mode 100644 index 0000000..427608b --- /dev/null +++ b/tests/test_param_live.cpp @@ -0,0 +1,152 @@ +// Standalone tests for the audio thread's parameter patch. The load-bearing one is the +// EQUIVALENCE assertion: patching a control into the live block must produce, bit for bit, the +// block the model path would have folded — which is what makes a second routing table safe. + +#include "../src/core/instrument/param/param_live.h" + +#include "../src/core/instrument/param/param_id.h" +#include "../src/core/instrument/param/param_units.h" +#include "../src/core/instrument/map/sample_map.h" +#include "../src/core/instrument/ui/deck_values.h" + +#include +#include + +using namespace reasampler; +using namespace reasampler::instrument::param; +using reasampler::instrument::engine::LiveValues; +using reasampler::instrument::engine::foldLive; +using reasampler::instrument::map::InstrumentParams; +using reasampler::instrument::map::PlaySeconds; +using reasampler::instrument::map::resolvePlay; +using reasampler::instrument::ui::DeckParam; + +static int g_fail = 0; +#define CHECK_ID(cond, id) do { if(!(cond)) { \ + std::printf("FAIL line %d (param %u): %s\n", __LINE__, (id), #cond); ++g_fail; } } while(0) +#define CHECK(cond) do { if(!(cond)) { \ + std::printf("FAIL line %d: %s\n", __LINE__, #cond); ++g_fail; } } while(0) + +namespace { + +constexpr int kRate = 48000; + +// The MODEL path, verbatim: the write the editor makes, resolved and folded the way every +// publisher folds it. This is the reference the patch is measured against. +LiveValues modelBlock(const InstrumentParams& params) { + return foldLive(resolvePlay(params.play, kRate), params.keyTrack); +} + +// A parameter set deliberately away from its defaults, so an equivalence that only holds at the +// default cannot pass. Not every field — just enough that a mis-routed patch lands on a value +// that differs from the one it should have written. +InstrumentParams dialledParams() { + InstrumentParams p; + p.keyTrack = 1.5; + p.play.adsr.attackSeconds = 0.31; + p.play.adsr.holdSeconds = 0.07; + p.play.adsr.decaySeconds = 0.44; + p.play.adsr.sustainLevel = 0.62; + p.play.adsr.releaseSeconds = 0.9; + p.play.adsr.attackCurve = 2.5; + p.play.trigger.lengthFraction = 0.4; + p.play.trigAhd.attackSeconds = 0.12; + p.play.trigAhd.holdFraction = 0.3; + p.play.playRate = 1.2; + p.play.pitchOffsetSemitones = -5.0; + p.play.pitchEnv.enabled = true; + p.play.pitchEnv.peakSemitones = 7.0; + p.play.pitchEnv.shape.attackSeconds = 0.02; + p.play.filter.enabled = true; + p.play.filter.settings.cutoffNorm = 0.42f; + p.play.filter.settings.resonanceNorm = 0.66f; + p.play.filter.modAmount = -0.4; + p.play.filter.velAmount = 0.25; + p.play.filter.keyTrack = 0.75; + p.play.filter.env.attackSeconds = 0.05; + p.play.filter.trigEnv.decaySeconds = 0.6; + return p; +} + +} // namespace + +// THE assertion this module exists for. For every exposed control and several normalized +// positions: writing it through the model and folding must equal patching it into the folded +// block. Bytes, not fields — a member the patch forgot to route is caught as surely as one it +// routed to the wrong place. +static void testPatchingAControlEqualsFoldingTheModelAfterTheSameWrite() { + const double kPositions[] = {0.0, 0.137, 0.5, 0.813, 1.0}; + for (const ParamRow& row : exposedParams()) { + // Master gain is not carried by the block at all — the processor's own atomic is its + // route to the audio, and the patch reports that by refusing it. + if (row.deck == DeckParam::kMasterGain) { + LiveValues block = modelBlock(dialledParams()); + const LiveValues before = block; + CHECK_ID(!applyLiveParam(block, row.deck, 0.25, kRate), row.id); + CHECK_ID(std::memcmp(&before, &block, sizeof(LiveValues)) == 0, row.id); + continue; + } + for (double norm : kPositions) { + InstrumentParams written = dialledParams(); + if (row.deck == DeckParam::kKeyTrack) { + written.keyTrack = reasampler::instrument::ui::keyTrackFromNorm(norm); + } else { + reasampler::instrument::ui::setDeckParam(row.deck, written.play, norm, + /*segment=*/0); + } + const LiveValues expected = modelBlock(written); + + LiveValues patched = modelBlock(dialledParams()); + CHECK_ID(applyLiveParam(patched, row.deck, norm, kRate), row.id); + CHECK_ID(std::memcmp(&expected, &patched, sizeof(LiveValues)) == 0, row.id); + } + } +} + +// A drawn contour makes Trigger length inert — the editor's knob goes dead and the fold pins the +// fraction at 1.0. A host lane pointed at it must be equally inert, or automation would re-open a +// control the model says is closed. +static void testTriggerLengthIsInertUnderADrawnEnvelope() { + InstrumentParams p = dialledParams(); + p.play.ampSpline.mode = reasampler::EnvMode::Spline; + LiveValues block = modelBlock(p); + CHECK(block.splineActive); + CHECK(block.lengthFraction == 1.0); + CHECK(applyLiveParam(block, DeckParam::kTrigLength, 0.2, kRate)); + CHECK(block.lengthFraction == 1.0); +} + +// A control with no parameter row is refused rather than silently landing somewhere. +static void testAnUnexposedControlIsRefused() { + LiveValues block = modelBlock(dialledParams()); + const LiveValues before = block; + CHECK(!applyLiveParam(block, DeckParam::kPlayMode, 1.0, kRate)); + CHECK(!applyLiveParam(block, DeckParam::kVoiceCount, 1.0, kRate)); + CHECK(std::memcmp(&before, &block, sizeof(LiveValues)) == 0); +} + +// Every exposed control resolves to a home the host's read and write paths actually reach. The +// guard the promotion of pitch key-track needed: id 1000 was issued against a control whose value +// is not in PlaySeconds, and nothing failed to compile. +static void testEveryExposedControlHasAValueHome() { + for (const ParamRow& row : exposedParams()) { + CHECK_ID(valueHomeFor(row.deck) != ValueHome::None, row.id); + } + // And the instance-scalar set is exactly the two the shell branches on by name. + CHECK(valueHomeFor(DeckParam::kMasterGain) == ValueHome::InstanceScalar); + CHECK(valueHomeFor(DeckParam::kKeyTrack) == ValueHome::InstanceScalar); + int instanceScalars = 0; + for (const ParamRow& row : paramTable()) { + if (valueHomeFor(row.deck) == ValueHome::InstanceScalar) ++instanceScalars; + } + CHECK(instanceScalars == 2); +} + +int main() { + testPatchingAControlEqualsFoldingTheModelAfterTheSameWrite(); + testTriggerLengthIsInertUnderADrawnEnvelope(); + testAnUnexposedControlIsRefused(); + testEveryExposedControlHasAValueHome(); + if (g_fail == 0) std::printf("param_live: all tests passed\n"); + return g_fail == 0 ? 0 : 1; +} diff --git a/tests/test_param_units.cpp b/tests/test_param_units.cpp index 7ca378d..d6c6e52 100644 --- a/tests/test_param_units.cpp +++ b/tests/test_param_units.cpp @@ -6,7 +6,9 @@ #include "../src/core/instrument/param/param_id.h" #include "../src/core/instrument/engine/filter/filter_params.h" +#include "../src/core/instrument/engine/master_gain.h" #include "../src/core/instrument/map/play_seconds.h" +#include "../src/core/instrument/map/sample_map.h" #include "../src/core/instrument/ui/deck_values.h" #include @@ -29,6 +31,7 @@ static void testEveryUnitStringAndRangeMatchesTheSpecifiedTable() { // docs/product/parameter-automation.md 6.7.1, per parameter rather than per category, so a // control silently reclassified into the wrong category fails here. const Expect kExpected[] = { + {kParamKeyTrackPitch, "%", 0.0, 200.0}, {kParamRate, "%", 50.0, 200.0}, {kParamPitchOffset, "st", -24.0, 24.0}, {kParamPitchEnvAttack, "ms", 0.0, 10000.0}, @@ -65,6 +68,7 @@ static void testEveryUnitStringAndRangeMatchesTheSpecifiedTable() { {kParamAmpSustain, "%", 0.0, 100.0}, {kParamAmpRelease, "ms", 0.0, 10000.0}, {kParamAmpReleaseCurve, "", 0.1, 10.0}, + {kParamTriggerLength, "%", 0.0, 100.0}, {kParamAmpTrigAttack, "ms", 0.0, 10000.0}, {kParamAmpTrigAttackCurve, "", 0.1, 10.0}, {kParamAmpTrigHold, "%", 0.0, 100.0}, @@ -100,6 +104,38 @@ static void testEveryDefaultHasAnExactNormalizedPreimage() { } } +static void testTheHostAndTheEditorAgreeOnEveryDefaultPosition() { + // The criterion is that a host's reset-to-default and the editor's double-click land on the + // SAME value — and those are different code paths: ParameterInfo::defaultNormalizedValue + // comes from defaultNormalized (a per-CATEGORY switch), the editor's needle from + // deckParamNorm (a per-ID one). Asserting the param module against itself would not see the + // two disagree, and they carry three independently written full scales to disagree about. + using reasampler::instrument::ui::deckParamNorm; + const PlaySeconds defaults; + for (const ParamRow& row : exposedParams()) { + const double hostNorm = defaultNormalized(row.deck); + // The four that STORE their normalized position take it verbatim on both surfaces, and + // the two instance scalars are not in PlaySeconds at all — each read where it lives. + double editorNorm = 0.0; + switch (valueHomeFor(row.deck)) { + case ValueHome::InstanceScalar: + editorNorm = (row.deck == DeckParam::kMasterGain) + ? reasampler::instrument::engine::masterGainNormFromLinear(1.0) // unity + : reasampler::instrument::ui::keyTrackNormFrom( + reasampler::instrument::map::InstrumentParams{}.keyTrack); + break; + case ValueHome::ParamSetNorm: + case ValueHome::ParamSet: + editorNorm = deckParamNorm(row.deck, defaults); + break; + case ValueHome::None: + CHECK_ID(false, row.id); // an exposed control with no home reads nothing + continue; + } + CHECK_ID(hostNorm == editorNorm, row.id); + } +} + static void testTheFiltersFourTakeTheirStoredNormVerbatim() { // Their stored value IS the normalized one, so no taper may participate in their default: // this fails the moment someone routes them through toNormalized(toPlain(x)). @@ -181,6 +217,7 @@ static void testTheAddedDriveInverseUndoesTheFrozenLaw() { int main() { testEveryUnitStringAndRangeMatchesTheSpecifiedTable(); testEveryDefaultHasAnExactNormalizedPreimage(); + testTheHostAndTheEditorAgreeOnEveryDefaultPosition(); testTheFiltersFourTakeTheirStoredNormVerbatim(); testToPlainIsMonotoneAcrossTheWholeTravel(); testTheEndpointsAreTheDeclaredPlainRange();