Fix spline-drag rollback, bake contour leak, held-note live miss, amp clamp
Editor close mid-drag now rolls back published contours like capture-loss does; bake detaches liveContours so a printed capture can't observe a live drag; a held note now receives a live publish that lands in the same block as a note-on; amp spline read clamped to [0,1].
This commit is contained in:
@@ -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
|
// 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.
|
// this SampleData's own play params, which is what the bake is meant to print.
|
||||||
sample.live = nullptr;
|
sample.live = nullptr;
|
||||||
|
sample.liveContours = nullptr;
|
||||||
|
|
||||||
const int channels = sample.channelCount();
|
const int channels = sample.channelCount();
|
||||||
// The limiter delays its output by its lookahead, so the buffers carry that many extra
|
// The limiter delays its output by its lookahead, so the buffers carry that many extra
|
||||||
|
|||||||
@@ -225,8 +225,9 @@ private:
|
|||||||
// 128-point ceiling.
|
// 128-point ceiling.
|
||||||
//
|
//
|
||||||
// Holds a RAW POINTER into the bound point array: the caller guarantees the array outlives the
|
// 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
|
// cursor, or rebinds it first. The voice binds against its SampleData (that lifetime), or
|
||||||
// lifetime, or against its engine's live contour copy, which the engine rebinds before reusing.
|
// 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 {
|
class SplineCursor {
|
||||||
public:
|
public:
|
||||||
// Binds `c` if it has an evaluable segment; a shorter curve leaves the cursor inactive so
|
// Binds `c` if it has an evaluable segment; a shorter curve leaves the cursor inactive so
|
||||||
|
|||||||
@@ -335,8 +335,8 @@ void Voice::applyLive(const instrument::engine::LiveValues& live, bool snap) {
|
|||||||
|
|
||||||
namespace {
|
namespace {
|
||||||
|
|
||||||
// One EG's half of Voice::applyContours. `before` is read through the OLD binding, which is
|
// One EG's half of Voice::applyContours. `before` is read through the OLD binding — still
|
||||||
// still intact: the engine only overwrites a buffer no active voice is bound to.
|
// intact, by the double-buffer contract VoiceEngine::contours_ owns (voice_engine.h).
|
||||||
void rebindContour(SplineCursor& cursor, StepSmoother& smooth,
|
void rebindContour(SplineCursor& cursor, StepSmoother& smooth,
|
||||||
const instrument::engine::ContourPoints& pts, double phase, bool snap) {
|
const instrument::engine::ContourPoints& pts, double phase, bool snap) {
|
||||||
if (!cursor.active()) return;
|
if (!cursor.active()) return;
|
||||||
|
|||||||
@@ -302,10 +302,16 @@ private:
|
|||||||
// Early-free at a genuine permanent terminus (the spline analogue of a staged AHD's
|
// 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.
|
// finished()) — onFinalSegment()/segmentEndValue()'s own doc comments own the why.
|
||||||
amp = readSpline(ampSplineCur_, ampSplineSmooth_);
|
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() &&
|
if (amp == 0.0 && ampSplineCur_.onFinalSegment() &&
|
||||||
ampSplineCur_.segmentEndValue() == 0.0) {
|
ampSplineCur_.segmentEndValue() == 0.0) {
|
||||||
amplitudeDone_ = true;
|
amplitudeDone_ = true;
|
||||||
}
|
}
|
||||||
|
if (amp < 0.0) amp = 0.0;
|
||||||
|
else if (amp > 1.0) amp = 1.0;
|
||||||
} else if (playMode_ == PlayMode::Gate) {
|
} else if (playMode_ == PlayMode::Gate) {
|
||||||
amp = env_.tick();
|
amp = env_.tick();
|
||||||
if (env_.finished()) amplitudeDone_ = true;
|
if (env_.finished()) amplitudeDone_ = true;
|
||||||
|
|||||||
@@ -69,9 +69,13 @@ void VoiceEngine::applyContoursToActive() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
void VoiceEngine::startVoice(Voice& voice, int note, int velocity) {
|
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();
|
applyContoursToActive();
|
||||||
refreshLive();
|
applyLiveToActive();
|
||||||
// THE read of the note-on-latched commit class, and the only one: a published block outranks
|
// 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
|
// 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.
|
// touches none of these three — so a move reaches the next note and no sounding one.
|
||||||
|
|||||||
@@ -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
|
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
|
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
|
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
|
(add/delete/hard) and a restore all reach the voices through the reload; capture-lost and the
|
||||||
exit that commits nothing, re-couriers the restored `params_`.
|
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
|
**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
|
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
|
## Gotchas
|
||||||
|
|
||||||
- **`reasampler_processor.h` no longer needs a ceiling exception, and the one it had rested on a
|
- **`reasampler_processor.h` carries a small, documented ceiling exception.** It was described as
|
||||||
false premise.** It was described as ONE class declaration; it also carried two namespace-scope
|
ONE class declaration; it also carried two namespace-scope aggregates (`MasterBusMeter`,
|
||||||
aggregates (`MasterBusMeter`, `LoadedInstrument`) and the automation lane's own state. Both are
|
`LoadedInstrument`) and the automation lane's own state. All three are split out —
|
||||||
now split out — `processor_snapshot.h` and `automation_channel.h`, on the same grounds
|
`processor_snapshot.h` and `automation_channel.h`, on the same grounds `editor_interaction.h`
|
||||||
`editor_interaction.h` was split out of `reasampler_editor.h` in this directory: neither is
|
was split out of `reasampler_editor.h` in this directory: neither is behaviour. What remains is
|
||||||
behaviour. What remains is under the ceiling. Its bulk is the drain-slot proof and the
|
the class declaration itself, a handful of lines over the ceiling since the live-contour block
|
||||||
RT-discipline constraints, which the comment conventions name as keep-worthy.
|
(`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
|
- **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
|
`Main_OnCommandEx` inline from `WM_LBUTTONDOWN` would run the extension's whole landing
|
||||||
|
|||||||
@@ -162,6 +162,15 @@ void ReaSamplerEditor::removedFromParent() {
|
|||||||
// an edit forever and every later internal write to that parameter would emit a bare
|
// 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
|
// performEdit. The capture-lost path normally closes it; this does not rely on Windows
|
||||||
// delivering WM_CAPTURECHANGED before the window goes away.
|
// 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 (processor_) processor_->endParamGesture();
|
||||||
if (childHwnd_) {
|
if (childHwnd_) {
|
||||||
KillTimer(childHwnd_, kSyncTimerId); // stop the poll before the window goes away
|
KillTimer(childHwnd_, kSyncTimerId); // stop the poll before the window goes away
|
||||||
|
|||||||
@@ -394,9 +394,8 @@ private:
|
|||||||
// Both live_ and draining_ observe this same block — a block owned by a snapshot would
|
// 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.
|
// leave the drain's still-sounding voices deaf to the knob under them.
|
||||||
instrument::engine::LiveParams liveParams_;
|
instrument::engine::LiveParams liveParams_;
|
||||||
// The drawn-contour block, SampleData::liveContours' one target, under liveParams_' ordering
|
// SampleData::liveContours' one target, under liveParams_' ordering constraint above and for
|
||||||
// constraint and for its reason. No automation merge: no contour is a host parameter, so both
|
// its reason (this directory's CLAUDE.md owns why it needs no automation merge).
|
||||||
// engines read the model/editor publish directly.
|
|
||||||
instrument::engine::LiveContourBlock liveContours_;
|
instrument::engine::LiveContourBlock liveContours_;
|
||||||
// The block the ENGINE reads, and the ONE thing SampleData::live points at. Written only by
|
// 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
|
// the audio thread, which merges liveParams_ with the host's automation points once per block
|
||||||
|
|||||||
@@ -11,6 +11,7 @@
|
|||||||
#include "../src/core/instrument/bake/bake_render.h"
|
#include "../src/core/instrument/bake/bake_render.h"
|
||||||
|
|
||||||
#include "../src/core/instrument/engine/limiter.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 "../src/core/instrument/engine/live_params.h"
|
||||||
|
|
||||||
#include <cmath>
|
#include <cmath>
|
||||||
@@ -338,6 +339,36 @@ int main() {
|
|||||||
CHECK(sameSamples(audio, reference));
|
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 --------
|
// --- 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
|
// 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
|
// filter, so every printed frame equals its source frame PROVIDED the amp curve's gain at
|
||||||
|
|||||||
@@ -1167,6 +1167,38 @@ static void testVelocityGainSurvivesAHostilePublishThatReallyLands() {
|
|||||||
CHECK(proportional);
|
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<AudioSample> 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<double>(out.back()) - 0.4) < 0.01);
|
||||||
|
}
|
||||||
|
|
||||||
int main() {
|
int main() {
|
||||||
testStageDurationChangeHoldsPhase();
|
testStageDurationChangeHoldsPhase();
|
||||||
testShortenedStageStillLandsContinuously();
|
testShortenedStageStillLandsContinuously();
|
||||||
@@ -1189,6 +1221,7 @@ int main() {
|
|||||||
testAPublishedPitchOffsetRefitsThePitchEnvelopeSpan();
|
testAPublishedPitchOffsetRefitsThePitchEnvelopeSpan();
|
||||||
testPitchRatioAndVelocityGainStayLatched();
|
testPitchRatioAndVelocityGainStayLatched();
|
||||||
testVelocityGainSurvivesAHostilePublishThatReallyLands();
|
testVelocityGainSurvivesAHostilePublishThatReallyLands();
|
||||||
|
testHeldVoiceReceivesALivePublishThatCoincidesWithANoteOn();
|
||||||
if (g_fail == 0) std::printf("live_delivery tests passed\n");
|
if (g_fail == 0) std::printf("live_delivery tests passed\n");
|
||||||
return g_fail == 0 ? 0 : 1;
|
return g_fail == 0 ? 0 : 1;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user