diff --git a/src/core/instrument/bake/bake_render.cpp b/src/core/instrument/bake/bake_render.cpp index d0b1696..cc1bebc 100644 --- a/src/core/instrument/bake/bake_render.cpp +++ b/src/core/instrument/bake/bake_render.cpp @@ -36,6 +36,7 @@ BakeAudio renderBake(SampleData sample, const BakePlan& plan, double masterGainL // depend on what the user happened to be dragging. The dialed values are already in // this SampleData's own play params, which is what the bake is meant to print. sample.live = nullptr; + sample.liveContours = nullptr; const int channels = sample.channelCount(); // The limiter delays its output by its lookahead, so the buffers carry that many extra diff --git a/src/core/instrument/engine/velocity_curve.h b/src/core/instrument/engine/velocity_curve.h index dc1bb75..1f49790 100644 --- a/src/core/instrument/engine/velocity_curve.h +++ b/src/core/instrument/engine/velocity_curve.h @@ -225,8 +225,9 @@ private: // 128-point ceiling. // // Holds a RAW POINTER into the bound point array: the caller guarantees the array outlives the -// cursor, or rebinds it first. The voice binds against its SampleData, which has exactly that -// lifetime, or against its engine's live contour copy, which the engine rebinds before reusing. +// cursor, or rebinds it first. The voice binds against its SampleData (that lifetime), or +// against its engine's live contour copy — the double-buffer contract VoiceEngine::contours_ +// owns (voice_engine.h) is what makes that second binding safe. class SplineCursor { public: // Binds `c` if it has an evaluable segment; a shorter curve leaves the cursor inactive so diff --git a/src/core/instrument/engine/voice.cpp b/src/core/instrument/engine/voice.cpp index ded44cc..335ff13 100644 --- a/src/core/instrument/engine/voice.cpp +++ b/src/core/instrument/engine/voice.cpp @@ -335,8 +335,8 @@ void Voice::applyLive(const instrument::engine::LiveValues& live, bool snap) { namespace { -// One EG's half of Voice::applyContours. `before` is read through the OLD binding, which is -// still intact: the engine only overwrites a buffer no active voice is bound to. +// One EG's half of Voice::applyContours. `before` is read through the OLD binding — still +// intact, by the double-buffer contract VoiceEngine::contours_ owns (voice_engine.h). void rebindContour(SplineCursor& cursor, StepSmoother& smooth, const instrument::engine::ContourPoints& pts, double phase, bool snap) { if (!cursor.active()) return; diff --git a/src/core/instrument/engine/voice.h b/src/core/instrument/engine/voice.h index 6e225c1..435e187 100644 --- a/src/core/instrument/engine/voice.h +++ b/src/core/instrument/engine/voice.h @@ -302,10 +302,16 @@ private: // Early-free at a genuine permanent terminus (the spline analogue of a staged AHD's // finished()) — onFinalSegment()/segmentEndValue()'s own doc comments own the why. amp = readSpline(ampSplineCur_, ampSplineSmooth_); + // A swap-step absorbed by ampSplineSmooth_ can briefly push the sum outside [0,1] + // (a contour's own points are drawn in-range, but the smoother's addend isn't); the + // filter clamps its own read for the same reason (below). The finished-check reads + // the PRE-clamp value so a genuine zero-terminus isn't masked by the clamp itself. if (amp == 0.0 && ampSplineCur_.onFinalSegment() && ampSplineCur_.segmentEndValue() == 0.0) { amplitudeDone_ = true; } + if (amp < 0.0) amp = 0.0; + else if (amp > 1.0) amp = 1.0; } else if (playMode_ == PlayMode::Gate) { amp = env_.tick(); if (env_.finished()) amplitudeDone_ = true; diff --git a/src/core/instrument/engine/voice_engine.cpp b/src/core/instrument/engine/voice_engine.cpp index eed4e4c..2e9dffa 100644 --- a/src/core/instrument/engine/voice_engine.cpp +++ b/src/core/instrument/engine/voice_engine.cpp @@ -69,9 +69,13 @@ void VoiceEngine::applyContoursToActive() { } void VoiceEngine::startVoice(Voice& voice, int note, int velocity) { - // FIRST, so no sounding voice is left bound to a buffer the next read overwrites. + // FIRST, so no sounding voice is left bound to a buffer the next read overwrites. A held + // note sharing this generation with the note-on must see it too — refreshLive() alone + // would consume the generation without applying it, and since applyLiveToActive() only + // acts on a NEW generation, the held voice would then never see this publish at all + // (not merely late a block), only catching up whenever the next one arrives. applyContoursToActive(); - refreshLive(); + applyLiveToActive(); // 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 // touches none of these three — so a move reaches the next note and no sounding one. diff --git a/src/shell/instrument/CLAUDE.md b/src/shell/instrument/CLAUDE.md index 1e34d86..9b5b8d7 100644 --- a/src/shell/instrument/CLAUDE.md +++ b/src/shell/instrument/CLAUDE.md @@ -91,8 +91,9 @@ else — no parameter set, no generation, no host callback — and its release i host-facing reader needs the model to carry it mid-gesture; it is what keeps a motion event off the host entirely (an unlatched `setInstrumentParams` opens a group edit even when nothing exposed moved). `reloadInstrument` republishes the model's contours, so a release, a click-edit -(add/delete/hard) and a restore all reach the voices through the reload; capture-lost, the one -exit that commits nothing, re-couriers the restored `params_`. +(add/delete/hard) and a restore all reach the voices through the reload; capture-lost and the +editor closing mid-drag (`removedFromParent`) are the two exits that commit nothing — both +re-courier the restored `params_`. **The VST3 parameter surface is a THIRD surface onto the one model, never a second copy.** The pure half — the frozen id table, the exposed set, the plain-value layer, the formatter — is @@ -241,13 +242,19 @@ against a performance budget — they are there because `VoiceEngine::applyLiveT ## Gotchas -- **`reasampler_processor.h` no longer needs a ceiling exception, and the one it had rested on a - false premise.** It was described as ONE class declaration; it also carried two namespace-scope - aggregates (`MasterBusMeter`, `LoadedInstrument`) and the automation lane's own state. Both are - now split out — `processor_snapshot.h` and `automation_channel.h`, on the same grounds - `editor_interaction.h` was split out of `reasampler_editor.h` in this directory: neither is - behaviour. What remains is under the ceiling. Its bulk is the drain-slot proof and the - RT-discipline constraints, which the comment conventions name as keep-worthy. +- **`reasampler_processor.h` carries a small, documented ceiling exception.** It was described as + ONE class declaration; it also carried two namespace-scope aggregates (`MasterBusMeter`, + `LoadedInstrument`) and the automation lane's own state. All three are split out — + `processor_snapshot.h` and `automation_channel.h`, on the same grounds `editor_interaction.h` + was split out of `reasampler_editor.h` in this directory: neither is behaviour. What remains is + the class declaration itself, a handful of lines over the ceiling since the live-contour block + (`liveContours_` + `publishLiveContours`) landed beside `liveParams_` under the same ordering + constraint. No further seam exists that isn't behaviour: every remaining aggregate-shaped chunk + (the preview mailbox, the drain-slot triple) is read and written inline by `process()` on the + audio thread, so splitting it into its own header would either duplicate the RT-discipline + comments at a second site or turn a plain member access into a cross-header one for no gain. + Its bulk is the drain-slot proof and the RT-discipline constraints, which the comment + conventions name as keep-worthy. - **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 diff --git a/src/shell/instrument/editor_platform.cpp b/src/shell/instrument/editor_platform.cpp index 5631266..b465139 100644 --- a/src/shell/instrument/editor_platform.cpp +++ b/src/shell/instrument/editor_platform.cpp @@ -162,6 +162,15 @@ void ReaSamplerEditor::removedFromParent() { // 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 (drag_ == DragKind::kSplineNode) { + // A contour drag reaches the voices without the model (see publishContoursLive) — the + // window closing mid-drag is a second exit alongside WM_CAPTURECHANGED that must roll + // the voices back the same way, or an abandoned drag stays audible after the editor + // that started it is gone. + params_ = dragStartParams_; + publishContoursLive(); + drag_ = DragKind::kNone; + } if (processor_) processor_->endParamGesture(); if (childHwnd_) { KillTimer(childHwnd_, kSyncTimerId); // stop the poll before the window goes away diff --git a/src/shell/instrument/reasampler_processor.h b/src/shell/instrument/reasampler_processor.h index 71361ae..fab1b55 100644 --- a/src/shell/instrument/reasampler_processor.h +++ b/src/shell/instrument/reasampler_processor.h @@ -394,9 +394,8 @@ 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 drawn-contour block, SampleData::liveContours' one target, under liveParams_' ordering - // constraint and for its reason. No automation merge: no contour is a host parameter, so both - // engines read the model/editor publish directly. + // SampleData::liveContours' one target, under liveParams_' ordering constraint above and for + // its reason (this directory's CLAUDE.md owns why it needs no automation merge). instrument::engine::LiveContourBlock liveContours_; // 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 diff --git a/tests/test_bake_render.cpp b/tests/test_bake_render.cpp index 7a382be..0cff496 100644 --- a/tests/test_bake_render.cpp +++ b/tests/test_bake_render.cpp @@ -11,6 +11,7 @@ #include "../src/core/instrument/bake/bake_render.h" #include "../src/core/instrument/engine/limiter.h" +#include "../src/core/instrument/engine/live_contours.h" #include "../src/core/instrument/engine/live_params.h" #include @@ -338,6 +339,36 @@ int main() { CHECK(sameSamples(audio, reference)); } + // --- The bake detaches liveContours too, not just live ------------------------------- + // A contour drag in flight publishes into liveContours (see reasampler_processor.h); a + // render that observed it would print whatever the user happens to be dragging instead + // of the dialed shape already sitting in play.ampSpline.contour. + { + instrument::engine::LiveContourBlock block; + SampleData s = makeSample(/*stereo=*/false, /*frames=*/1000); + s.play.playMode = PlayMode::Trigger; + s.play.ampSpline.mode = EnvMode::Spline; + s.play.ampSpline.contour = instrument::engine::VelocityCurve::flat(); // dialed: unity + + // Publish a very different contour — ramping down to silence — into the block. + s.liveContours = █ + { + PlayParams dragged = s.play; + dragged.ampSpline.contour = instrument::engine::VelocityCurve::rampDown(); + block.publish(instrument::engine::foldContours(dragged)); + } + + const BakePlan plan = planOf(/*total=*/1000, /*noteOn=*/0, /*noteOff=*/1000); + const BakeAudio audio = renderBake(s, plan, kUnity, kNoLimiter); + + CHECK(s.liveContours == &block); // the caller's own snapshot was not detached + + SampleData detached = s; + detached.liveContours = nullptr; + const BakeAudio reference = renderBake(detached, plan, kUnity, kNoLimiter); + CHECK(sameSamples(audio, reference)); + } + // --- Regression baseline: the neutral render is the source, sample for sample -------- // A Trigger voice at its own root under Varispeed reads at ratio exactly 1 and hits no // filter, so every printed frame equals its source frame PROVIDED the amp curve's gain at diff --git a/tests/test_live_delivery.cpp b/tests/test_live_delivery.cpp index e8b4ac6..d0227b5 100644 --- a/tests/test_live_delivery.cpp +++ b/tests/test_live_delivery.cpp @@ -1167,6 +1167,38 @@ static void testVelocityGainSurvivesAHostilePublishThatReallyLands() { CHECK(proportional); } +// A note-on that lands in the same block boundary as a publish must not let the already- +// sounding voice miss it: startVoice used to call refreshLive() bare, which consumes the +// generation stamp without applying it to the voices already active — a held note that shared +// a block with a note-on then never saw the publish at all (not merely delayed a block). +static void testHeldVoiceReceivesALivePublishThatCoincidesWithANoteOn() { + SampleData s; + s.frames.assign(200000, 1.0f); // DC: the envelope alone shapes the output + s.sampleRate = kRate; + s.rootNote = 60; + s.play.adsr.attackFrames = 0; + s.play.adsr.sustainLevel = 1.0; + s.play.adsr.releaseFrames = 480000; // long enough that release never intrudes + + LiveParams block; + s.live = █ + VoiceEngine engine(2, s); + engine.noteOn(60, 100); // voice A, sustaining at the dialed 1.0 + + LiveValues quieter = foldLive(s.play, s.keyTrack); + quieter.adsr.sustainLevel = 0.2; + block.publish(quieter); + + engine.noteOn(61, 100); // voice B starts in the SAME generation as the publish + + std::vector out; + for (int i = 0; i < 20; ++i) engine.render(out, 512); // let voice A's glide settle + + // Both voices settle at 0.2 — a held voice that missed the publish would still sit at 1.0, + // for a combined 1.2 rather than 0.4. + CHECK(std::fabs(static_cast(out.back()) - 0.4) < 0.01); +} + int main() { testStageDurationChangeHoldsPhase(); testShortenedStageStillLandsContinuously(); @@ -1189,6 +1221,7 @@ int main() { testAPublishedPitchOffsetRefitsThePitchEnvelopeSpan(); testPitchRatioAndVelocityGainStayLatched(); testVelocityGainSurvivesAHostilePublishThatReallyLands(); + testHeldVoiceReceivesALivePublishThatCoincidesWithANoteOn(); if (g_fail == 0) std::printf("live_delivery tests passed\n"); return g_fail == 0 ? 0 : 1; }