From e2c73e30d602de98dc4f682016cea03902654cbc Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Mon, 27 Jul 2026 23:30:27 -0400 Subject: [PATCH] fix(vst): master-gain per-sample ramp (no zipper/click); voice-count commits once on release; minor review items --- src/vst/master_gain.h | 12 ++++--- src/vst/reasampler_editor.cpp | 56 +++++++++++++++++++------------ src/vst/reasampler_editor.h | 4 ++- src/vst/reasampler_processor.cpp | 57 +++++++++++++++++++++++++------- src/vst/reasampler_processor.h | 9 +++-- src/vst/sample_map.cpp | 5 +-- tests/test_master_gain.cpp | 16 +++++++++ 7 files changed, 116 insertions(+), 43 deletions(-) diff --git a/src/vst/master_gain.h b/src/vst/master_gain.h index a1163b7..9ba20aa 100644 --- a/src/vst/master_gain.h +++ b/src/vst/master_gain.h @@ -33,8 +33,10 @@ double masterGainMaxLinear(); // [kMasterGainMinDb, kMasterGainMaxDb]. norm is clamped to [0,1]. Pure. double masterGainDbFromNorm(double norm); -// Inverse taper: dB -> normalized [0,1]. -infinity (or any dB at/below kMasterGainMinDb) -// maps to the bottom of the finite sweep (0 for -inf, else the clamped floor); +24 -> 1. Pure. +// Inverse taper: dB -> normalized [0,1]. -infinity (or any dB at or below kMasterGainMinDb, +// including below-floor values like -80 dB) maps to norm 0 (the -inf bottom detent) — the +// finite sweep only covers the range above kMasterGainMinDb; everything at or below it collapses +// to the same true-zero bottom. +24 -> 1. Pure. double masterGainNormFromDb(double db); // Knob taper composed with dB->ratio: normalized [0,1] -> LINEAR gain. norm 0 -> exactly @@ -42,8 +44,10 @@ double masterGainNormFromDb(double db); double masterGainLinearFromNorm(double norm); // Inverse: LINEAR gain -> normalized [0,1]. linear <= 0 -> 0 (the -inf bottom); a linear at -// or below the kMasterGainMinDb floor also reads ~0+ (the taper's finite bottom); unity -> -// ~0.714; masterGainMaxLinear() -> 1. Out-of-range/non-finite input clamps. Pure. +// or below the kMasterGainMinDb floor (e.g. 0.001 = -60 dB, or anything below) also maps to 0 +// — the floor IS the -inf detent; values between true-zero and the floor cannot be represented +// on the knob and collapse to the bottom. unity -> ~0.714; masterGainMaxLinear() -> 1. +// Out-of-range/non-finite input clamps. Pure. double masterGainNormFromLinear(double linear); // The knob's hover/drag value label for a normalized value: "-inf" at the bottom, else a diff --git a/src/vst/reasampler_editor.cpp b/src/vst/reasampler_editor.cpp index 700c6fe..6de3798 100644 --- a/src/vst/reasampler_editor.cpp +++ b/src/vst/reasampler_editor.cpp @@ -577,16 +577,14 @@ void ReaSamplerEditor::applyDeckKnob(int zoneIndex, int id, double norm) { } switch (static_cast(id)) { case ParamControl::kVoiceCount: { - // Stepped: quantize the continuous drag to the integer count and only fire the - // setter on a CHANGE (each fire is an off-thread engine rebuild — cheap, but not - // free; per-step is the right cadence). + // Stepped: quantize the continuous drag to the integer count and track it live + // for the label/needle. The actual engine rebuild (setVoiceCount) fires ONCE on + // WM_LBUTTONUP — not per step — so a full drag (~31 steps) costs one rebuild, + // not thirty. const int count = kMinVoiceCount + static_cast(norm * (kMaxVoiceCount - kMinVoiceCount) + 0.5); - if (count != voiceCount_) { - voiceCount_ = count; - processor_->setVoiceCount(count); - } + voiceCount_ = count; return; } case ParamControl::kMasterGain: @@ -1421,16 +1419,19 @@ void ReaSamplerEditor::paintEnvelopeOverlay(LICE_IBitmap* bmp, const Rect& waveA } void ReaSamplerEditor::paintVelocityCurve(LICE_IBitmap* bmp, const Rect& r, - const PerformanceZone& zone) { + const PerformanceZone& zone, + bool drawCaption) { if (r.width() <= 0 || r.height() <= 0) return; // suppressed (window too narrow) // The bordered box: a panel surface + hairline border, drawn by palette role. fillSurface(bmp, toKitBox(r), Role::BgPanel, InteractionState::Rest); LICE_DrawRect(bmp, r.left, r.top, r.width() - 1, r.height() - 1, toLice(roleColor(Role::LineHairline)), 1.0f, 0); - // A corner caption (decorative — the axes are velocity -> amp). - kitText(bmp, Rect{r.left + 4, r.top + 1, r.right - 4, r.top + 12}, "Vel curve", - Font::Micro, Role::TextDim); + // A corner caption (decorative — the axes are velocity -> amp). Suppressed when the caller + // (e.g. the curve popup) renders its own sheet title so the label doesn't double. + if (drawCaption) + kitText(bmp, Rect{r.left + 4, r.top + 1, r.right - 4, r.top + 12}, "Vel curve", + Font::Micro, Role::TextDim); const VelocityCurve::Box box = curveBoxFromRect(r); if (box.width <= 0 || box.height <= 1) return; @@ -1611,8 +1612,9 @@ void ReaSamplerEditor::paintCurvePopup(LICE_IBitmap* bmp, int w, int h) { } // The full-size editor: the SAME draw path as the Zone inline box (paintVelocityCurve + // the one curveBoxFromRect mapping formula), so trace/handles/drag-off cues cannot drift - // between the two surfaces. The popup edits the picked capture's one-zone site. - paintVelocityCurve(bmp, pl.curveBox, effectiveSampleZone()); + // between the two surfaces. The popup edits the picked capture's one-zone site. Caption + // suppressed (the sheet's own "VELOCITY -> AMP" title above is the label for this context). + paintVelocityCurve(bmp, pl.curveBox, effectiveSampleZone(), /*drawCaption=*/false); } void ReaSamplerEditor::handleCurveMouseDown(const Rect& r, int zoneIndex, int x, int y) { @@ -2627,9 +2629,6 @@ void ReaSamplerEditor::onMouseMove(int x, int y) { GetClientRect(childHwnd_, &rc); const int w = rc.right - rc.left; const int h = rc.bottom - rc.top; - // r11: the Sample bands derive from the deck height (mode-independent width math). - const std::vector deckDescs = deckGroupDescs(effectiveSampleZone().play); - const SampleBands bands = computeSampleBands(w, h, deckHeight(deckDescs, w - 2 * kPad)); const int dx = x - dragStartX_; if (drag_ == DragKind::kDeckKnob) { @@ -2642,6 +2641,11 @@ void ReaSamplerEditor::onMouseMove(int x, int y) { return; } + // r11: the Sample bands derive from the deck height (mode-independent width math). Hoisted + // below the kDeckKnob early-return — that branch uses neither deckDescs nor bands. + const std::vector deckDescs = deckGroupDescs(effectiveSampleZone().play); + const SampleBands bands = computeSampleBands(w, h, deckHeight(deckDescs, w - 2 * kPad)); + if (drag_ == DragKind::kRootMarker) { // The fenced root strip on the Sample cluster band. Setting the root materializes a // full-keyboard zone carrying the override on the picked id (the D-B override vehicle) — @@ -2831,15 +2835,19 @@ void ReaSamplerEditor::onMouseUp(int x, int y) { curvePointIndex_ = -1; dragCurveZone_ = -1; // A scrollbar drag is transient UI (no map change), and the processor-side knobs (the - // preview-velocity -2 sentinel, voice count, master gain) are per-instance settings already - // applied live — none reloads the instrument here (voice count rebuilds per step in its - // setter; master gain is an atomic the audio thread reads directly). Every other drag is a - // coherent map edit: publish the in-flight map + reload off-thread. + // preview-velocity -2 sentinel, voice count, master gain) are per-instance settings that + // don't reload the instrument via the map path. Master gain is an atomic the audio thread + // reads directly. Voice count: the label/needle tracks live during the drag but the engine + // rebuild (setVoiceCount) fires ONCE here on release — not per integer step. const bool deckTransient = kind == DragKind::kDeckKnob && (paramId == -2 || paramId == static_cast(ParamControl::kVoiceCount) || paramId == static_cast(ParamControl::kMasterGain)); if (kind == DragKind::kScrollThumb || deckTransient) { + // Commit the voice count now that the drag is complete (one rebuild per full drag). + if (deckTransient && processor_ && + paramId == static_cast(ParamControl::kVoiceCount)) + processor_->setVoiceCount(voiceCount_); invalidate(); return; } @@ -2873,11 +2881,15 @@ void ReaSamplerEditor::onMouseRDown(int x, int y) { GetClientRect(childHwnd_, &rc); const CurvePopupLayout pl = computeCurvePopup(rc.right - rc.left, rc.bottom - rc.top); if (!contains(pl.curveBox, x, y)) return; + // Hit-test first (read-only, via effectiveSampleZone) so a right-click that lands between + // nodes does not materialize an uncommitted zone in map_. Materialize only on an actual hit. + const VelocityCurve::Box box = curveBoxFromRect(pl.curveBox); + const int idx = effectiveSampleZone().velocityCurve.pointAtPixel(box, x, y); + if (idx < 0) return; const int zi = ensureSampleZone(); if (zi < 0) return; PerformanceZone& z = map_.zones[static_cast(zi)]; - const int idx = z.velocityCurve.pointAtPixel(curveBoxFromRect(pl.curveBox), x, y); - if (idx >= 0 && z.velocityCurve.deletePoint(static_cast(idx))) { + if (z.velocityCurve.deletePoint(static_cast(idx))) { selectedZone_ = zi; hover_ = HoverTarget{}; // a stale kCurveNode index would light a shifted node commitAndReload(); diff --git a/src/vst/reasampler_editor.h b/src/vst/reasampler_editor.h index 1b89a1d..862ecae 100644 --- a/src/vst/reasampler_editor.h +++ b/src/vst/reasampler_editor.h @@ -179,7 +179,9 @@ private: // Y = amp 0-1), the monotone spline traced by eval, one draggable node handle per control // point. Shared by the Sample face (beside the hero) and the Zone param panel; all mapping / // hit-test / clamp math lives in the pure velocity_curve module. `r` empty -> draws nothing. - void paintVelocityCurve(LICE_IBitmap* bmp, const Rect& r, const PerformanceZone& zone); + // drawCaption: false suppresses the "Vel curve" corner label (the popup draws its own title). + void paintVelocityCurve(LICE_IBitmap* bmp, const Rect& r, const PerformanceZone& zone, + bool drawCaption = true); // Route a mouse-down inside curve-editor box `r` editing map_.zones[zoneIndex]: a node grab // starts a kCurveNode drag; Alt-click on an interior node deletes it (committed at once); diff --git a/src/vst/reasampler_processor.cpp b/src/vst/reasampler_processor.cpp index aae1407..6233378 100644 --- a/src/vst/reasampler_processor.cpp +++ b/src/vst/reasampler_processor.cpp @@ -43,6 +43,13 @@ namespace { // FIXED so raising the voice count never multiplies shifter CPU past the profiled budget. constexpr std::size_t kPreserveVoiceCap = 8; +// FB1 post-mixer gain ramp rate (per sample). gainCurrent_ converges to masterGain_ at this +// linear step; it ramps from 0 to unity (or vice versa) in ~20 ms at 48 kHz. The early-out +// (|current - target| < threshold) snaps to the target and avoids the ramp loop on idle blocks. +// kGainRampSnap is the threshold below which we snap to the target (avoids long sub-LSB creep). +constexpr float kGainRampRate = 1.0f / 960.0f; // 960 samples @ 48 kHz ≈ 20 ms +constexpr float kGainRampSnap = kGainRampRate * 0.5f; + // Read a whole file into a byte buffer. Off-thread only (blocking file I/O). Empty on // any failure — the caller treats an unreadable WAV as "nothing to play". std::vector readFileBytes(const std::string& path) { @@ -893,14 +900,28 @@ tresult PLUGIN_API ReaSamplerProcessor::process(ProcessData& data) { drain->engine.render(ch0, ch1, static_cast(frames)); drain->preview.render(ch0, ch1, static_cast(frames)); } - // FB1 post-mixer master gain: ONE relaxed load per block, applied AFTER the voice sum - // (engine + drain + preview) and BEFORE the extra-channel mirror + peak, so the mirror - // and the level indicator both see the actual output. A cheap multiply — no per-voice - // cost, no alloc, no lock (RT discipline). + // FB1 post-mixer master gain: ramp gainCurrent_ toward the atomic target per-sample so + // continuous knob drags produce no zipper noise and the true-zero bottom causes no click. + // Applied AFTER the voice sum and BEFORE the extra-channel mirror + peak so both see the + // actual output. Branch-free inner loop; early-out when already at target. RT-safe. { - const float g = masterGain_.load(std::memory_order_relaxed); - if (g != 1.f) { - for (int32 i = 0; i < frames; ++i) { ch0[i] *= g; ch1[i] *= g; } + const float gTarget = masterGain_.load(std::memory_order_relaxed); + const float diff = gTarget - gainCurrent_; + if (diff < -kGainRampSnap || diff > kGainRampSnap) { + // Ramp toward target: step per sample, then apply the per-sample gain. + for (int32 i = 0; i < frames; ++i) { + const float d = gTarget - gainCurrent_; + if (d > kGainRampRate) gainCurrent_ += kGainRampRate; + else if (d < -kGainRampRate) gainCurrent_ -= kGainRampRate; + else gainCurrent_ = gTarget; + ch0[i] *= gainCurrent_; + ch1[i] *= gainCurrent_; + } + } else { + gainCurrent_ = gTarget; + if (gTarget != 1.f) { + for (int32 i = 0; i < frames; ++i) { ch0[i] *= gTarget; ch1[i] *= gTarget; } + } } } // Any channels beyond the first two mirror ch0 (defensive — REAPER negotiates 1 or 2). @@ -930,12 +951,24 @@ tresult PLUGIN_API ReaSamplerProcessor::process(ProcessData& data) { drain->engine.render(ch0, static_cast(frames)); drain->preview.render(ch0, static_cast(frames)); } - // FB1 post-mixer master gain (mono path) — same contract as the stereo branch above: - // post-sum, pre-peak/replicate, one relaxed load, RT-safe. + // FB1 post-mixer master gain (mono path) — same ramp contract as the stereo branch: + // post-sum, pre-peak/replicate, per-sample gainCurrent_ ramp toward target, RT-safe. { - const float g = masterGain_.load(std::memory_order_relaxed); - if (g != 1.f) { - for (int32 i = 0; i < frames; ++i) ch0[i] *= g; + const float gTarget = masterGain_.load(std::memory_order_relaxed); + const float diff = gTarget - gainCurrent_; + if (diff < -kGainRampSnap || diff > kGainRampSnap) { + for (int32 i = 0; i < frames; ++i) { + const float d = gTarget - gainCurrent_; + if (d > kGainRampRate) gainCurrent_ += kGainRampRate; + else if (d < -kGainRampRate) gainCurrent_ -= kGainRampRate; + else gainCurrent_ = gTarget; + ch0[i] *= gainCurrent_; + } + } else { + gainCurrent_ = gTarget; + if (gTarget != 1.f) { + for (int32 i = 0; i < frames; ++i) ch0[i] *= gTarget; + } } } float peak = 0.f; diff --git a/src/vst/reasampler_processor.h b/src/vst/reasampler_processor.h index 858e9b0..14f5f6b 100644 --- a/src/vst/reasampler_processor.h +++ b/src/vst/reasampler_processor.h @@ -367,9 +367,14 @@ private: MonoTrigger monoTrigger_ = MonoTrigger::Retrigger; // FB1 post-mixer master gain (LINEAR; persisted in component state v8). A lock-free - // atomic — the ONE voice-param the audio thread reads directly (a single relaxed load - // per block, applied as a post-sum multiply). Default unity = pre-FB1 output. + // atomic — the target the UI thread writes; the audio thread ramps gainCurrent_ toward + // it per-sample each block (linear interpolation, ~20 ms at 48 kHz / 256-frame block) + // so sudden knob moves produce no zipper noise and the true-zero bottom causes no click. std::atomic masterGain_{1.0f}; + // The audio-thread running gain value: tracks masterGain_ across blocks, stepping at + // most kGainRampRate per sample toward the target. Starts at unity (pre-FB1 default). + // Written and read exclusively on the audio thread — no atomics needed. + float gainCurrent_ = 1.0f; // --- S-VIEW-4 preview-trigger mailbox (off-thread -> audio thread, lock-free) --------- // The editor's preview-trigger button posts a note-on/off request from the UI thread; process() diff --git a/src/vst/sample_map.cpp b/src/vst/sample_map.cpp index 882fb3f..82ef9a1 100644 --- a/src/vst/sample_map.cpp +++ b/src/vst/sample_map.cpp @@ -745,8 +745,9 @@ ComponentState deserializeComponentState(const std::vector& bytes, // default (unity) holds, reproducing pre-FB1 output exactly. A non-finite, negative, or // above-cap value (a corrupt blob) falls back to unity rather than silencing/blasting. if (version == kComponentStateVersion) { - const double g = bitsToDouble(asU64(r.i64())); - if (!r.ok) return out; // truncated inside the gain double -> empty (unity holds) + const double g = bitsToDouble(r.u64()); + if (!r.ok) return out; // truncated inside the gain double — unity holds (out already + // carries mode/marker/velocity/voice fields from above) out.masterGainLinear = (std::isfinite(g) && g >= 0.0 && g <= vst::masterGainMaxLinear() * (1.0 + 1e-9)) ? g diff --git a/tests/test_master_gain.cpp b/tests/test_master_gain.cpp index e13d4ac..16be668 100644 --- a/tests/test_master_gain.cpp +++ b/tests/test_master_gain.cpp @@ -74,6 +74,21 @@ static void testNonFiniteLinearClamps() { CHECK(near(masterGainNormFromLinear(1e9), 1.0)); // above the cap clamps to 1 } +static void testBelowFloorCollapsesToBottom() { + // Any linear gain at or below the kMasterGainMinDb floor collapses to norm 0 (the -inf + // bottom detent). The floor IS the bottom of the finite sweep, so -61 dB, -80 dB, and + // a vanishingly small positive linear all map to the same place as true zero. + const double floorLinear = std::pow(10.0, kMasterGainMinDb / 20.0); // 10^(-60/20) = 0.001 + CHECK(masterGainNormFromLinear(floorLinear) == 0.0); // exactly at the floor -> bottom + CHECK(masterGainNormFromLinear(floorLinear * 0.5) == 0.0); // below the floor -> bottom + CHECK(masterGainNormFromLinear(1e-9) == 0.0); // tiny positive -> bottom + // The dB path agrees: anything at or below kMasterGainMinDb maps to norm 0. + CHECK(masterGainNormFromDb(kMasterGainMinDb) == 0.0); + CHECK(masterGainNormFromDb(kMasterGainMinDb - 20.0) == 0.0); // -80 dB -> bottom + // Just ABOVE the floor (by epsilon) returns a non-zero norm. + CHECK(masterGainNormFromDb(kMasterGainMinDb + 1e-9) > 0.0); +} + static void testLabels() { char buf[24]; formatMasterGainLabel(0.0, buf, sizeof(buf)); @@ -93,6 +108,7 @@ int main() { testNormLinearRoundTripAcrossTravel(); testMonotonic(); testNonFiniteLinearClamps(); + testBelowFloorCollapsesToBottom(); testLabels(); if (g_fail) { std::printf("%d FAILURE(S)\n", g_fail);