diff --git a/src/core/instrument/CLAUDE.md b/src/core/instrument/CLAUDE.md index d726358..9bee9e1 100644 --- a/src/core/instrument/CLAUDE.md +++ b/src/core/instrument/CLAUDE.md @@ -255,7 +255,7 @@ anything for a trigger shape. - `play_params.h` — the value layer: `PlayParams`/`AdsrParams`/`TriggerParams`/`PitchEnvParams`/`FilterParams`, the per-instance mode enums (`ChannelMode`/`VoiceMode`/`MonoTrigger`), and `SampleData` (the ONE loaded capture: decoded PCM + root + loop + start + keyTrack + velocity curve + play params). Shared by the engine, the codec, and the editor, so a UI/codec TU reading a param struct doesn't recompile when a `Voice` member changes. `FilterParams` stores the filter module's own `FilterSettings` by value rather than a parallel copy of its normalized positions. - `envelopes.h` — the three per-frame evaluators (`AdsrEnvelope` AHDSR, `AhdEnvelope` the sustain-less Attack/Hold/Decay, `PitchEnvelope` the AHD pitch offset), CONCRETE and fully header-inline. Never give them a common base or a virtual `tick()`: they are called per-voice-per-sample. Also home to `fitAhd`/`ahdLevelAt`, THE span split and shape every sustain-less envelope shares. A voice carries two of each shape — the amp's and the filter's — and its play mode picks which pair it reads. `AdsrEnvelope`/`PitchEnvelope` own `applyLive` (the φ-holding mid-stage rule), its fresh-note peer `snapLive`, and `StepSmoother`, the bounded offset that absorbs the level steps φ cannot cover; `AhdEnvelope` is POSITIONAL (evaluated at a source offset, not ticked), so it has no phase to hold and smooths a live reshape instead. - `live_params.h` / `live_params.cpp` — the live-parameter block: `LiveValues` (the plain, trivially-copyable bundle the audio thread observes), the single-writer `LiveParams` seqlock that publishes it without a lock or a torn read, `foldLive` (the ONE derivation from `PlayParams` — every publisher goes through it so the two representations cannot drift), and `ValueRamp`, the per-frame glide whose EXACT termination is what lets the filter's equality-compare cutoff skip re-engage. Links no engine: the block is a value the voice observes, not a thing the engine owns. - - `voice.h` / `voice.cpp` — one voice. The per-SAMPLE render half (`advanceFrame` and everything it calls) is INLINE IN THE HEADER by RT constraint; the per-NOTE half (note-on setup incl. the Preserve ring prime, legato retune, gate-off, the off-thread shifter presize) is out of line in the TU. The voice owns its own `VoiceFilter` and filter envelope, run between the pitch stage and the amp multiply — see `engine/filter/CLAUDE.md`. + - `voice.h` / `voice.cpp` — one voice. The per-SAMPLE render half (`advanceFrame` and everything it calls) is INLINE IN THE HEADER by RT constraint; the per-NOTE half (note-on setup incl. the Preserve ring prime, legato retune, gate-off, the off-thread shifter presize) is out of line in the TU. The voice owns its own `VoiceFilter` and filter envelope, run between the pitch stage and the amp multiply — see `engine/filter/CLAUDE.md`. **Documented ~600-line-ceiling exception** (root `CLAUDE.md` structural heuristic 1): `voice.h` sits over the ceiling because `advanceFrame`'s RT-inline constraint forbids the seam a split would need — a documented exception, not silent overshoot. - `voice_engine.h` / `voice_engine.cpp` — `VoiceEngine`: note routing, bounded-stealing allocation, user-parameterized voice count (1–32, default 16), `VoiceMode` Poly/Mono (last-note held-note stack, `MonoTrigger` Retrigger/Legato), two-tier panic (CC 123 = all-notes-off release, CC 120 = immediate hard-stop including Trigger one-shots), and the block render loops. Preview injects a synthetic note-on at the loaded capture's root note into the main `VoiceEngine` — no dedicated `PreviewCard`; preview obeys polyphony/mono/voice-stealing/envelopes. - `pitch_shift` — hand-rolled **correlation-aligned SOLA** (splice-overlap-add) pitch shifter for the Preserve playback mode: one active read tap chases the write head at the shift ratio; each splice jump is refined by a cross-correlation search so the new read point is waveform-aligned, then old and new taps are crossfaded (raised-cosine, amplitude-complementary). Replaces the prior dual-tap OLA whose fixed half-window tap offset caused anti-phase cancellation on many source frequencies. **GA2:** ring buffer **primed with the actual upcoming source** at note-on (was zero-filled) → gap-free frame-0 onset, ~25 ms Preserve onset latency eliminated (Preserve now speaks on frame 0, matching Varispeed), and real-content-bounded tail (last-window tail-truncation gone). No third-party dependencies; RT-discipline: no allocation in `process()`. - `velocity_curve` — pure velocity→amp transfer curve: `VelocityCurve` evaluated by a Fritsch–Carlson monotone cubic Hermite spline (no overshoot outside [0,1]). `eval(velocity)` called once per note-on. `flat()` default (y=1, every velocity→unity) replaces the prior fixed `velocity/127` path — a deliberate non-back-compat behavior change (Daniel-approved). @@ -264,7 +264,7 @@ anything for a trigger shape. ### `map/` - `sample_map` — the bank blob → selected capture resolve, the channel policy (downmix / dual-mono / L-R split), `InstrumentParams` (the ONE parameter set: root/loop/start overrides, keyTrack, velocity curve, `PlaySeconds`), the single override-beats-intrinsic fold (`resolveCapture`, shared by the bank and refs paths so they cannot drift), and the `SampleData` build. **Wall-clock times stored as rate-free SECONDS, resolved against the live project rate — NO hardcoded sample rates in `src/`** (Daniel's standing ruling, load-bearing). Deliberately does NOT link the voice engine: the build's product is plain `SampleData`. -- `component_state_io` (`core/instrument/map`) — the `ComponentState` envelope + params-payload binary codec (envelope v1…v11, params payload v1…v9), split out of `sample_map` (Q-W2v, T4-13 ≡ T2-07) so BOTH artifacts can link the codec without the extension pulling in the whole voice engine to serialize one preset blob — the extension's `instrument_drop` and the instrument's processor read/write the identical bytes, so the cross-artifact contract cannot drift. Payload v1…v7 are the RETIRED per-zone lists: still read, lifting by adopting zone one's capture + parameters (that first zone is what the old first-match resolve actually played, so it is also what supersedes the envelope's stored selection id). Payload v9 appends the per-voice filter tail; a v8 blob is a strict prefix of it and lifts to the off/neutral filter default. +- `component_state_io` (`core/instrument/map`) — the `ComponentState` envelope + params-payload binary codec (envelope v1…v11, params payload v1…v10), split out of `sample_map` (Q-W2v, T4-13 ≡ T2-07) so BOTH artifacts can link the codec without the extension pulling in the whole voice engine to serialize one preset blob — the extension's `instrument_drop` and the instrument's processor read/write the identical bytes, so the cross-artifact contract cannot drift. Payload v1…v7 are the RETIRED per-zone lists: still read, lifting by adopting zone one's capture + parameters (that first zone is what the old first-match resolve actually played, so it is also what supersedes the envelope's stored selection id). Payload v9 appends the per-voice filter tail; a v8 blob is a strict prefix of it and lifts to the off/neutral filter default. - `params_payload` — the PARAMS-PAYLOAD half of that codec, split from the envelope half on the axis the format already has: the payload carries its own version and grows independently, so the two version ladders are two responsibilities. An INTERNAL seam — the public entry points stay `serialize`/`deserializeComponentState`. The prose ladder and every version constant stay in `component_state_io.h`, their one home. - `bank_sync` — generation change-detection + assignment-request consume: owns the yes/no decision logic so the rules are provable without a host. The processor shell owns cadence and side effects. - `bridge_marshal` — pure marshalling helper for the REAPER VST-host bridge read: interprets the `GetProjExtState` int return against its filled buffer. diff --git a/src/core/instrument/engine/CMakeLists.txt b/src/core/instrument/engine/CMakeLists.txt index 43b0781..a0b7620 100644 --- a/src/core/instrument/engine/CMakeLists.txt +++ b/src/core/instrument/engine/CMakeLists.txt @@ -20,7 +20,7 @@ add_subdirectory(filter) # engine — the block is a plain value the voice observes, not a thing the engine owns. reasampler_pure_library(live_params SOURCES live_params.cpp - LINK PUBLIC peaks velocity_curve filter) + LINK PUBLIC peaks velocity_curve filter curve_law) reasampler_test(live_params LINK live_params) # Two TUs on the engine's own responsibility seam (per-note setup vs. note routing and @@ -28,7 +28,7 @@ reasampler_test(live_params LINK live_params) # boundary costs the hot path nothing. reasampler_pure_library(sampler_core SOURCES voice.cpp voice_engine.cpp - LINK PUBLIC peaks pitch_shift velocity_curve filter live_params) + LINK PUBLIC peaks pitch_shift velocity_curve filter live_params curve_law) # Links only sampler_core: linking more would break the plain-data-boundary proof — a VST3 # or REAPER type reaching the core would fail to compile or link here. reasampler_test(sampler_core LINK sampler_core) diff --git a/src/core/instrument/engine/envelopes.h b/src/core/instrument/engine/envelopes.h index 2a5fc0c..7818a96 100644 --- a/src/core/instrument/engine/envelopes.h +++ b/src/core/instrument/engine/envelopes.h @@ -346,14 +346,14 @@ public: // Trigger amp and filter envelopes. A zero/negative span finishes immediately. void configure(std::int64_t spanFrames, const AhdParams& params) { span_ = spanFrames > 0 ? spanFrames : 0; - fit(params); + fit(params, /*latchFinished=*/false); // a fresh note starts from a clean read smooth_.clear(); } // Peer of AdsrEnvelope::snapLive: a voice that has rendered nothing takes the new shape // outright, with no step to absorb. void snapLive(const AhdParams& params) { - fit(params); + fit(params, /*latchFinished=*/false); smooth_.clear(); } @@ -361,7 +361,10 @@ public: // why this smooths rather than holding a normalized position. void applyLive(double sourceOffset, const AhdParams& params) { const double before = ahdLevelAt(sourceOffset, fit_, attackCurve_, decayCurve_); - fit(params); + // LATCHED: a voice already read past its fitted total must never resurge because a + // later live move reopened the total. Reachable on any active() voice, including one + // ringing out past its own end (voice.h) where tickAmplitude() still runs. + fit(params, /*latchFinished=*/true); const double after = ahdLevelAt(sourceOffset, fit_, attackCurve_, decayCurve_); if (after != before) smooth_.absorb(before - after); } @@ -381,11 +384,15 @@ public: const AhdSpan& stages() const { return fit_; } private: - void fit(const AhdParams& p) { + // `latchFinished`: once true, a re-fit can only ever KEEP finished_ true, never clear it — + // see applyLive above for why. configure()/snapLive() pass false: those are a fresh read + // (new note or a not-yet-rendered voice), which must compute finished_ from scratch. + void fit(const AhdParams& p, bool latchFinished) { fit_ = fitAhd(span_, p); attackCurve_ = p.attackCurve; decayCurve_ = p.decayCurve; - finished_ = (fit_.total <= 0); + const bool empty = (fit_.total <= 0); + finished_ = latchFinished ? (finished_ || empty) : empty; } std::int64_t span_ = 0; diff --git a/src/core/instrument/engine/play_params.h b/src/core/instrument/engine/play_params.h index ad12272..b9d4b84 100644 --- a/src/core/instrument/engine/play_params.h +++ b/src/core/instrument/engine/play_params.h @@ -55,8 +55,8 @@ struct AdsrParams { // Attack -> Hold -> Decay over a bounded span: the shape every SUSTAIN-LESS envelope takes // (the Trigger amp, the Trigger filter envelope, the pitch envelope). Hold is a FRACTION of -// the span left after attack and decay, never a time of its own — that is what makes -// A + H + D <= span structural rather than clamped (see fitAhd in envelopes.h). +// the span left after attack and decay, never a time of its own — fitAhd (envelopes.h) owns +// why a fraction, not a time. struct AhdParams { std::int64_t attackFrames = 0; std::int64_t decayFrames = 0; diff --git a/src/core/instrument/engine/voice.h b/src/core/instrument/engine/voice.h index 2d7460b..8bdb48f 100644 --- a/src/core/instrument/engine/voice.h +++ b/src/core/instrument/engine/voice.h @@ -6,6 +6,11 @@ // configured an out-of-line render would put a call — and the envelope ticks behind it — // across a TU boundary on the hottest path in the program. The per-NOTE half (start / // retune / release / hardStop / presize) is cold enough to live in voice.cpp. +// +// DOCUMENTED ~600-line-ceiling EXCEPTION (root CLAUDE.md structural heuristic 1): this file +// is over the ceiling because of the constraint above, not silent overshoot. A responsibility +// seam here would move part of advanceFrame's inline body out of this header, reintroducing +// the cross-TU call the header-inlining exists to avoid — worse than the overshoot. #include #include diff --git a/src/core/instrument/map/CMakeLists.txt b/src/core/instrument/map/CMakeLists.txt index 532e891..4f409d1 100644 --- a/src/core/instrument/map/CMakeLists.txt +++ b/src/core/instrument/map/CMakeLists.txt @@ -21,7 +21,7 @@ reasampler_test(bank_sync LINK bank_sync) # the format already keeps on independent version axes (see component_state_io.h). reasampler_pure_library(component_state_io SOURCES component_state_io.cpp params_payload.cpp - LINK PUBLIC velocity_curve master_gain) + LINK PUBLIC velocity_curve master_gain curve_law) # Links only component_state_io, deliberately no sampler_core/pitch_shift: the structural # proof the codec is engine-free, which is what keeps engine object code out of the extension. reasampler_test(component_state_io LINK component_state_io) @@ -29,7 +29,7 @@ reasampler_test(component_state_io LINK component_state_io) # The mapping's product is plain SampleData, so the voice engine is not a dependency. reasampler_pure_library(sample_map SOURCES sample_map.cpp - LINK PUBLIC bank_book wav_codec velocity_curve peaks) + LINK PUBLIC bank_book wav_codec velocity_curve peaks curve_law) # Links only sample_map + component_state_io: the same plain-data-boundary proof, spanning # both halves of the mapping/codec split where the frozen-format assertions live. reasampler_test(sample_map LINK sample_map component_state_io) diff --git a/src/core/instrument/map/component_state_io.cpp b/src/core/instrument/map/component_state_io.cpp index 2a3b71b..ab72530 100644 --- a/src/core/instrument/map/component_state_io.cpp +++ b/src/core/instrument/map/component_state_io.cpp @@ -17,15 +17,13 @@ namespace reasampler::instrument::map { using engine::masterGainMaxLinear; using reasampler::wire::ByteReader; +using reasampler::wire::asU64; using reasampler::wire::bitsToDouble; using reasampler::wire::doubleToBits; using reasampler::wire::putLE; namespace { -// Signed 64-bit values ride the wire as their two's-complement unsigned image. -std::uint64_t asU64(std::int64_t v) { return static_cast(v); } - // Apply a payload read to the state: the adoption rule (a retired payload's first zone // supersedes the envelope's selection id) lives here, once. void applyPayload(ComponentState& out, PayloadRead read) { diff --git a/src/core/instrument/map/params_payload.cpp b/src/core/instrument/map/params_payload.cpp index 2a21c36..9600993 100644 --- a/src/core/instrument/map/params_payload.cpp +++ b/src/core/instrument/map/params_payload.cpp @@ -14,15 +14,13 @@ namespace reasampler::instrument::map { using reasampler::wire::ByteReader; +using reasampler::wire::asU64; using reasampler::wire::bitsToDouble; using reasampler::wire::doubleToBits; using reasampler::wire::putLE; namespace { -// Signed 64-bit values ride the wire as their two's-complement unsigned image. -std::uint64_t asU64(std::int64_t v) { return static_cast(v); } - // Emit the OVERRIDE trio shared by the v2..v7 per-zone record and the v8 single record, so // the two shapes cannot drift byte-for-byte. void putOverrides(std::vector& out, const InstrumentParams& p) { @@ -149,8 +147,14 @@ double readCurveExponent(ByteReader& r) { } void readAhd(ByteReader& r, AhdSeconds& a) { - a.attackSeconds = bitsToDouble(r.u64()); - a.decaySeconds = bitsToDouble(r.u64()); + // attackSeconds/decaySeconds reach resolvePlay's static_cast (sample_map.cpp) + // unguarded — UB on NaN, and on a large-enough finite value — so a corrupt/non-finite wire + // value degrades to 0 seconds rather than reaching that cast, the same guard readSecondsPlayTail + // and the v9 filter tail already apply to their own wall-clock fields. + const double attack = bitsToDouble(r.u64()); + const double decay = bitsToDouble(r.u64()); + a.attackSeconds = std::isfinite(attack) ? attack : 0.0; + a.decaySeconds = std::isfinite(decay) ? decay : 0.0; const double frac = bitsToDouble(r.u64()); a.holdFraction = std::isfinite(frac) ? frac : 0.0; a.attackCurve = readCurveExponent(r); diff --git a/src/core/instrument/map/sample_map.cpp b/src/core/instrument/map/sample_map.cpp index 0eac216..abf1469 100644 --- a/src/core/instrument/map/sample_map.cpp +++ b/src/core/instrument/map/sample_map.cpp @@ -203,8 +203,8 @@ DecodedPcm decodeChannels(const std::vector& interleaved, } PlayParams resolvePlay(const PlaySeconds& stored, int sampleRate) { - // seconds -> frames at the LIVE rate; source-timeline quantities (trigger %-length + - // fades) carry through untouched, already frames/fractions. + // seconds -> frames at the LIVE rate; the source-timeline quantity (trigger %-length) + // 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) { diff --git a/src/core/instrument/ui/CMakeLists.txt b/src/core/instrument/ui/CMakeLists.txt index 36749d3..2e090a3 100644 --- a/src/core/instrument/ui/CMakeLists.txt +++ b/src/core/instrument/ui/CMakeLists.txt @@ -35,7 +35,7 @@ reasampler_test(browser_scroll LINK browser_scroll) reasampler_pure_library(param_slider SOURCES param_slider.cpp LINK PUBLIC editor_geometry) reasampler_test(param_slider LINK param_slider) -reasampler_pure_library(envelope_overlay SOURCES envelope_overlay.cpp LINK PUBLIC editor_geometry) +reasampler_pure_library(envelope_overlay SOURCES envelope_overlay.cpp LINK PUBLIC editor_geometry curve_law) reasampler_test(envelope_overlay LINK envelope_overlay) reasampler_pure_library(envelope_edit SOURCES envelope_edit.cpp LINK PUBLIC envelope_overlay) diff --git a/src/core/instrument/ui/envelope_edit.cpp b/src/core/instrument/ui/envelope_edit.cpp index 35c1368..1d77279 100644 --- a/src/core/instrument/ui/envelope_edit.cpp +++ b/src/core/instrument/ui/envelope_edit.cpp @@ -5,6 +5,7 @@ #include "core/util/clamp01.h" #include +#include // std::fabs #include // std::abs namespace reasampler::instrument::ui { @@ -91,15 +92,20 @@ SegmentLevels segmentLevels(const StageEnvelope& env, EnvNode knot) { } // A knot drag: the grab-time mid-level shifted by the pixel delta, read back through -// curve_law's inverse. Both directions go through the ONE law, which is why the knot and the -// inner dial cannot express different exponents. +// curve_law's inverse (curve_law.h owns why the knot and the inner dial share this one law). double curveFromKnotDrag(const StageEnvelope& grabEnv, EnvNode knot, double grabExponent, const Rect& area, int dyPixels) { const SegmentLevels seg = segmentLevels(grabEnv, knot); if (!seg.ok) return grabExponent; - const double grabLevel = seg.start + (seg.end - seg.start) * curveMidLevel(grabExponent); + const double span = seg.end - seg.start; + // segmentLevels only rejects an EXACTLY level segment; a near-level one (e.g. sustain + // 0.99) still passes with a tiny divisor here, so one pixel of drag can swing `u` by + // ~1.0 and saturate the exponent. Floor the magnitude at a couple of pixels' worth of + // level travel — a segment thinner than that is visually a no-op drag anyway. + if (std::fabs(span) < 2.0 * levelPerPixel(area)) return grabExponent; + const double grabLevel = seg.start + span * curveMidLevel(grabExponent); const double newLevel = grabLevel - static_cast(dyPixels) * levelPerPixel(area); - return curveFromMidLevel((newLevel - seg.start) / (seg.end - seg.start)); + return curveFromMidLevel((newLevel - seg.start) / span); } } // namespace @@ -196,10 +202,21 @@ StageEnvelope resolveNodeDrag(const StageEnvelope& grabEnv, EnvNode node, const out.holdFraction = clamp01((s.hold + dSec) / rem); break; } - case EnvNode::DecayEnd: - out.decaySeconds = - std::clamp(grabEnv.decaySeconds + dSec, 0.0, bounds.maxDecaySeconds); + case EnvNode::DecayEnd: { + // DecayEnd is DRAWN at t0 + total, and total = attack + decay + (span-attack-decay) + // * holdFraction, so d(total)/d(decay) = 1 - holdFraction: Hold eats a holdFraction + // share of whatever decay gives up. Scaling by 1/(1-frac) makes the drawn endpoint + // track the cursor 1:1, matching every other node. At frac == 1.0 (the Trigger + // default) Hold consumes the WHOLE remainder regardless of decay's value, so the + // derivative is exactly 0 — no scale recovers motion there, and decaySeconds is left + // unchanged rather than divided by zero. + const double denom = 1.0 - clamp01(grabEnv.holdFraction); + if (denom > 1e-9) { + out.decaySeconds = + std::clamp(grabEnv.decaySeconds + dSec / denom, 0.0, bounds.maxDecaySeconds); + } break; + } case EnvNode::AttackCurve: out.attackCurve = curveFromKnotDrag(grabEnv, node, grabEnv.attackCurve, rect, dyPixels); diff --git a/src/core/instrument/ui/envelope_edit.h b/src/core/instrument/ui/envelope_edit.h index 705d250..c4f104f 100644 --- a/src/core/instrument/ui/envelope_edit.h +++ b/src/core/instrument/ui/envelope_edit.h @@ -1,21 +1,13 @@ // envelope_edit.h — node hit-test + pixel-delta -> clamped-param inverse map for the draggable -// envelope nodes and their mid-segment curve knots. Mirror of card_drag/waveform_view: drag -// arithmetic lives here, unit-tested outside the DAW; the shell draws handles, captures the -// grab, and feeds pixel deltas back in. -// -// envelope_overlay owns the params->polyline forward (draw) map; this module owns the inverse -// (edit) map + hit-test. Both read/write the same StageEnvelope fields (the shell re-reads the -// one parameter set every paint), so a node drag, a knot drag, and a knob edit are three views -// on one source of truth — structurally, not through a listener chain. -// -// A drag can never produce a param a knob couldn't: time nodes are range-clamped to the same -// per-param [min,max] the knobs enforce (EnvClampBounds, caller-supplied since those maxima -// live shell-side), and a knot resolves through curve_law's own exponent domain. +// envelope nodes and their mid-segment curve knots. The inverse of envelope_overlay's forward +// (draw) map; both read/write the same StageEnvelope fields, so node drag / knot drag / knob +// edit are one model — see core/instrument/CLAUDE.md's "envelope overlay" section for why. +// Mirror of card_drag/waveform_view: drag arithmetic lives here, unit-tested outside the DAW. // // Time-only nodes drag on X; DecayEnd in an AHDSR drags on both axes (X = decay time, Y = -// sustain level); a curve knot drags on Y alone. Origin is never draggable, and neither is an -// AHDSR's ReleaseEnd — it is anchored to the right edge, and release is dragged from -// ReleaseStart instead. A node is only editable in its own kind. +// sustain level); a curve knot drags on Y alone (curve_law.h owns the exponent domain). Origin +// is never draggable, and neither is an AHDSR's ReleaseEnd — anchored to the right edge, with +// release dragged from ReleaseStart instead. A node is only editable in its own kind. #pragma once diff --git a/src/core/instrument/ui/envelope_overlay.cpp b/src/core/instrument/ui/envelope_overlay.cpp index 924ba4d..99a0964 100644 --- a/src/core/instrument/ui/envelope_overlay.cpp +++ b/src/core/instrument/ui/envelope_overlay.cpp @@ -90,8 +90,7 @@ EnvVertex gateVtx(EnvNode node, const Rect& area, double px, double level, bool } // The knot for a segment running from `startLevel` to `endLevel`, placed at the segment's -// pixel midpoint. Its level is the curve's own value at the segment midpoint, which is what -// makes the knot's height and the inner dial two readings of one exponent. +// pixel midpoint, its level read through curve_law.h's own law (the knot/dial pairing's home). EnvVertex knotVtx(EnvNode node, const Rect& area, int x0, int x1, double startLevel, double endLevel, double exponent) { const double u = curveMidLevel(exponent); @@ -178,8 +177,19 @@ std::vector ahdPolyline(const StageEnvelope& env, const Rect& area, pts.reserve(6); pts.push_back(vtx(EnvNode::Origin, area, totalSeconds, t0, 0.0)); pts.push_back(vtx(EnvNode::AttackEnd, area, totalSeconds, t0 + s.attack, 1.0)); - pts.push_back(vtx(EnvNode::HoldEnd, area, totalSeconds, t0 + s.attack + s.hold, 1.0)); - pts.push_back(vtx(EnvNode::DecayEnd, area, totalSeconds, t0 + s.total, 0.0)); + EnvVertex holdVtx = vtx(EnvNode::HoldEnd, area, totalSeconds, t0 + s.attack + s.hold, 1.0); + EnvVertex decayVtx = vtx(EnvNode::DecayEnd, area, totalSeconds, t0 + s.total, 0.0); + // A hold that consumes the WHOLE post-attack/decay remainder (holdFraction == 1.0, the + // Trigger AHD default) puts HoldEnd and DecayEnd on the same wall-clock instant, and + // nodeAtPoint's earlier-draw-order tie-break then hides DecayEnd behind HoldEnd forever. + // Nudge apart, clamped to the canvas — the same minimum-separation rationale + // kGateNodeSepPx exists for on the AHDSR schematic, applied to this coincidence instead. + if (decayVtx.x - holdVtx.x < kGateNodeSepPx) { + const int right = area.x + std::max(1, area.width) - 1; + decayVtx.x = std::min(holdVtx.x + kGateNodeSepPx, right); + } + pts.push_back(holdVtx); + pts.push_back(decayVtx); if (s.attack > 0.0) { pts.push_back(knotVtx(EnvNode::AttackCurve, area, pts[0].x, pts[1].x, 0.0, 1.0, env.attackCurve)); diff --git a/src/core/instrument/ui/envelope_overlay.h b/src/core/instrument/ui/envelope_overlay.h index fe93e10..474784e 100644 --- a/src/core/instrument/ui/envelope_overlay.h +++ b/src/core/instrument/ui/envelope_overlay.h @@ -13,10 +13,9 @@ namespace reasampler::instrument::ui { -// Which LAYOUT POLICY an envelope takes, decided by whether it has a sustain stage rather -// than by which processor it modulates. A gated (AHDSR) envelope right-anchors its release so -// the sustain plateau reads full-width; a sustain-less (AHD) one maps 1:1 onto the waveform's -// own time axis, which only means anything for a trigger shape. The two policies coexist. +// Which LAYOUT POLICY an envelope takes — see core/instrument/CLAUDE.md's "envelope overlay" +// section for why (decided by sustain-stage presence, not by which processor it modulates). +// Ahdsr right-anchors its release; Ahd maps 1:1 onto the waveform's own time axis. enum class EnvKind { Ahdsr, Ahd }; // Gate nodes: Origin -> AttackEnd -> HoldEnd -> DecayEnd(sustain) -> ReleaseStart -> ReleaseEnd. diff --git a/src/core/wire/bytes.h b/src/core/wire/bytes.h index 0350191..514eb00 100644 --- a/src/core/wire/bytes.h +++ b/src/core/wire/bytes.h @@ -30,6 +30,11 @@ inline void putLE(std::vector& out, T v) { } } +// Signed 64-bit values ride the wire as their two's-complement unsigned image (the putLE +// call site's u64 cast). The one home for that cast — component_state_io and params_payload +// both need it and must not carry their own copies. +inline std::uint64_t asU64(std::int64_t v) { return static_cast(v); } + // IEEE-754 double <-> u64 bit-cast for the wire (memcpy is the only defined // type-pun in C++17). Doubles ride the wire as their u64 bit image via putLE. inline std::uint64_t doubleToBits(double d) { diff --git a/src/shell/instrument/CMakeLists.txt b/src/shell/instrument/CMakeLists.txt index 2ebb1c0..56c85a8 100644 --- a/src/shell/instrument/CMakeLists.txt +++ b/src/shell/instrument/CMakeLists.txt @@ -85,7 +85,7 @@ if(WIN32 AND EXISTS "${VST3_SDK}/public.sdk/source/main/pluginfactory.cpp") capture_browser keyboard_strip sample_bands sample_chrome waveform_view bank_sync browser_scroll param_slider tooltip theme component_geometry bank_grid trigger_seam envelope_overlay envelope_edit - knob_deck deck_groups curve_popup master_gain sample_usage file_bytes) + knob_deck deck_groups curve_popup master_gain sample_usage file_bytes curve_law) # SDK_INC gives the REAPER VST3 interfaces + API header for the bridge; WDL_INC gives # LICE for the editor. The VST3 SDK headers arrive via vst3_sdk PUBLIC. target_include_directories(reasampler_vst PRIVATE ${REASAMPLER_SRC_DIR} ${SDK_INC} ${WDL_INC}) diff --git a/src/shell/instrument/editor_controls.cpp b/src/shell/instrument/editor_controls.cpp index 6cefd1a..5496665 100644 --- a/src/shell/instrument/editor_controls.cpp +++ b/src/shell/instrument/editor_controls.cpp @@ -54,6 +54,29 @@ namespace { constexpr double kEnvTimeMaxSeconds = instrument::ui::kGateStageMaxSeconds; constexpr double kPitchDepthMaxSemis = 24.0; // pitch depth throw: +/-24 st, centered constexpr double kKeyTrackMax = 2.0; // key-track slider ceiling (0..200%) + +// The raw stored curve exponent for a curve-dial control id, read DIRECTLY off the field — +// never round-tripped through curveFromKnobNorm(knobNormFromCurve(x)): the knob-norm law's +// centre detent (curve_law.h) snaps anything near-neutral back to exactly 1.0, so a round trip +// can misreport a stored exponent that isn't neutral as "^1.00". +double curveExponentFor(int id, const PlaySeconds& play) { + using DeckParam = instrument::ui::DeckParam; // ReaSamplerEditor::ParamControl is an alias + switch (static_cast(id)) { + case DeckParam::kAttackCurve: return play.adsr.attackCurve; + case DeckParam::kDecayCurve: return play.adsr.decayCurve; + case DeckParam::kReleaseCurve: return play.adsr.releaseCurve; + case DeckParam::kTrigAttackCurve: return play.trigAhd.attackCurve; + case DeckParam::kTrigDecayCurve: return play.trigAhd.decayCurve; + case DeckParam::kPitchEnvAttackCurve: return play.pitchEnv.shape.attackCurve; + case DeckParam::kPitchEnvDecayCurve: return play.pitchEnv.shape.decayCurve; + case DeckParam::kFilterEnvAttackCurve: return play.filter.env.attackCurve; + case DeckParam::kFilterEnvDecayCurve: return play.filter.env.decayCurve; + case DeckParam::kFilterEnvReleaseCurve: return play.filter.env.releaseCurve; + case DeckParam::kFilterTrigAttackCurve: return play.filter.trigEnv.attackCurve; + case DeckParam::kFilterTrigDecayCurve: return play.filter.trigEnv.decayCurve; + default: return util::kCurveNeutral; + } +} } // namespace ReaSamplerEditor::FaceLayout ReaSamplerEditor::faceLayout(int w, int h) const { @@ -373,7 +396,7 @@ std::string ReaSamplerEditor::deckValueLabel(int id) const { case ParamControl::kFilterEnvReleaseCurve: case ParamControl::kFilterTrigAttackCurve: case ParamControl::kFilterTrigDecayCurve: - snprintf(buf, sizeof(buf), "^%.2f", util::curveFromKnobNorm(controlValue(id, play))); + snprintf(buf, sizeof(buf), "^%.2f", curveExponentFor(id, play)); break; default: // -2 (preview velocity) is labeled at its chrome call site; nothing else here. @@ -461,10 +484,11 @@ StageEnvelope ReaSamplerEditor::packEnvelope(OverlayEnv which, const PlaySeconds else packAhdsr(play.filter.env, env); break; case OverlayEnv::kAmp: - case OverlayEnv::kNone: if (trigger) packAhd(play.trigAhd, t0, playSpan, env); else packAhdsr(play.adsr, env); break; + case OverlayEnv::kNone: + break; // nothing is overlay-active; a default-constructed StageEnvelope, not amp } return env; } diff --git a/src/shell/instrument/editor_input_chrome.cpp b/src/shell/instrument/editor_input_chrome.cpp index bb7cd36..57940fe 100644 --- a/src/shell/instrument/editor_input_chrome.cpp +++ b/src/shell/instrument/editor_input_chrome.cpp @@ -44,6 +44,7 @@ bool ReaSamplerEditor::mouseDownChrome(const FaceLayout& fl, int x, int y) { if (contains(cr.velCell, x, y)) { drag_ = DragKind::kDeckKnob; dragParamId_ = -2; // sentinel: the preview velocity knob (a processor param) + dragInnerCellId_ = -1; // peer of mouseDownDeck's own grab-time init, for symmetry dragKnobStartValue_ = previewVelocity01(); dragStartX_ = x; dragStartY_ = y; diff --git a/src/shell/instrument/editor_internal.h b/src/shell/instrument/editor_internal.h index c8cdfb9..bcb493e 100644 --- a/src/shell/instrument/editor_internal.h +++ b/src/shell/instrument/editor_internal.h @@ -158,8 +158,8 @@ inline void drawKnobFace(LICE_IBitmap* bmp, const instrument::ui::Rect& knobRect // The concentric INNER dial: a second value on the same cell, drawn in the categorical // tertiary accent so it reads as a different KIND of control rather than a louder one — the -// same purple the overlay traces the envelope in, which is what ties a segment's knot to its -// dial by eye. Shares the outer knob's value<->angle map (param_slider's), so both needles +// same purple the overlay traces the envelope in (curve_law.h owns the knot/dial pairing this +// ties into). Shares the outer knob's value<->angle map (param_slider's), so both needles // point the same way for the same normalized value. inline void drawInnerDial(LICE_IBitmap* bmp, const instrument::ui::Rect& innerRect, double value01, ui::InteractionState st) { diff --git a/src/shell/instrument/editor_paint_deck.cpp b/src/shell/instrument/editor_paint_deck.cpp index 634c693..14df8f2 100644 --- a/src/shell/instrument/editor_paint_deck.cpp +++ b/src/shell/instrument/editor_paint_deck.cpp @@ -111,7 +111,11 @@ void ReaSamplerEditor::paintDeck(LICE_IBitmap* bmp, const FaceLayout& fl) { // The overlay-select radio: filled in the tertiary accent (the colour the overlay // traces in) when this group's envelope is the one on the waveform, hollow otherwise. if (g.captionRadio.id >= 0) { - const bool on = (overlayEnv_ == overlayEnvForRadio(g.captionRadio.id)); + // overlayEnvForRadio returns kNone for BOTH "not a radio id" and "no selection" — + // a non-radio id must never read as lit just because nothing is selected, so the + // picked env has to be checked against kNone itself, not just matched by equality. + const OverlayEnv picked = overlayEnvForRadio(g.captionRadio.id); + const bool on = picked != OverlayEnv::kNone && overlayEnv_ == picked; const bool hov = isHovered(HoverKind::kEnvRadio, g.captionRadio.id); const Rect& rb = g.captionRadio.box; LICE_DrawRect(bmp, rb.x, rb.y, rb.width - 1, rb.height - 1, diff --git a/tests/test_component_state_io.cpp b/tests/test_component_state_io.cpp index 572c60a..7d8c7c2 100644 --- a/tests/test_component_state_io.cpp +++ b/tests/test_component_state_io.cpp @@ -681,6 +681,26 @@ static void testNonFiniteFilterFieldsLiftToTheNeutralDefault() { CHECK(g.enabled); } +// A non-finite attackSeconds/decaySeconds on a stored AHD (a corrupt blob) must lift to 0 +// seconds rather than reach resolvePlay's static_cast (sample_map.cpp) — UB on +// NaN, and on a large-enough finite value. Mirrors +// testNonFiniteFilterFieldsLiftToTheNeutralDefault's per-field precedent, on the v10 AHD tail. +static void testNonFiniteAhdSecondsLiftToZero() { + ComponentState in; + in.selectionId = "pad"; + in.params.play.trigAhd.attackSeconds = std::numeric_limits::quiet_NaN(); + in.params.play.trigAhd.decaySeconds = std::numeric_limits::infinity(); + in.params.play.filter.trigEnv.attackSeconds = -std::numeric_limits::infinity(); + in.params.play.filter.trigEnv.decaySeconds = std::numeric_limits::quiet_NaN(); + + const ComponentState out = + deserializeComponentState(serializeComponentState(in), 48000.0); + CHECK(out.params.play.trigAhd.attackSeconds == 0.0); + CHECK(out.params.play.trigAhd.decaySeconds == 0.0); + CHECK(out.params.play.filter.trigEnv.attackSeconds == 0.0); + CHECK(out.params.play.filter.trigEnv.decaySeconds == 0.0); +} + // The WRITER emits the CURRENT payload version, and the marker + version sit at the head of // the payload — the self-describing property every legacy branch depends on. Asserted // against the semantic constants, not literals. @@ -1273,6 +1293,7 @@ int main() { testV8RecordLiftsToTheOffNeutralFilter(); testFilterTailRoundTripsLosslessly(); testNonFiniteFilterFieldsLiftToTheNeutralDefault(); + testNonFiniteAhdSecondsLiftToZero(); if (failures == 0) { std::printf("component_state_io_tests: all tests passed\n"); return 0; diff --git a/tests/test_envelope_edit.cpp b/tests/test_envelope_edit.cpp index 048cb96..824b1a3 100644 --- a/tests/test_envelope_edit.cpp +++ b/tests/test_envelope_edit.cpp @@ -193,9 +193,20 @@ static void testAhdStageTimesTrackTheWallClockScale() { const StageEnvelope attack = resolveNodeDrag(e, EnvNode::AttackEnd, overlayOf(a), kTotal, bounds(), 100, 0); CHECK(std::fabs(attack.attackSeconds - (e.attackSeconds + 100 * secPerPx)) < 1e-9); + + // DecayEnd's underlying param (decaySeconds) does NOT move 1:1 with the cursor: the drawn + // endpoint is t0 + total, and Hold eats a holdFraction share of whatever decay gives up + // (d(total)/d(decay) = 1 - holdFraction), so decaySeconds itself has to move faster than + // the cursor to make the DRAWN node track it. Assert on the RENDERED position, not the + // raw param — that is the property a drag actually has to deliver, and asserting the old + // 1:1 param delta here is exactly what let the node-tracking defect through undetected. + EnvVertex before; + CHECK(findNode(buildEnvelopePolyline(e, overlayOf(a), kTotal), EnvNode::DecayEnd, before)); const StageEnvelope decay = resolveNodeDrag(e, EnvNode::DecayEnd, overlayOf(a), kTotal, bounds(), 100, 0); - CHECK(std::fabs(decay.decaySeconds - (e.decaySeconds + 100 * secPerPx)) < 1e-9); + EnvVertex after; + CHECK(findNode(buildEnvelopePolyline(decay, overlayOf(a), kTotal), EnvNode::DecayEnd, after)); + CHECK(std::abs((after.x - before.x) - 100) <= 1); // 1:1 with the cursor, to rounding } // Hold is a fraction of what attack and decay left, so the node's pixel motion converts through @@ -270,6 +281,19 @@ static void testKnotOnALevelSegmentIsANoOp() { CHECK(out.decayCurve == 2.5); } +// A NEAR-level segment (sustain 0.99) is not caught by the exact-equality guard above, but its +// tiny divisor turns a one-pixel drag into a saturating swing of the exponent — the drag must +// still be a no-op rather than slam to a domain endpoint. +static void testKnotOnANearLevelSegmentIsANoOp() { + const Rect a = wideArea(); + StageEnvelope e = ahdsrEnv(); + e.sustainLevel = 0.99; + e.decayCurve = 2.5; + const StageEnvelope out = + resolveNodeDrag(e, EnvNode::DecayCurve, overlayOf(a), kTotal, bounds(), 0, -1); + CHECK(out.decayCurve == 2.5); +} + // --- degenerate ---------------------------------------------------------------- static void testDegenerateInputsAreNoOps() { @@ -299,6 +323,7 @@ int main() { testKnotDragMovesTheExponentWithinItsDomain(); testKnotAndModelCannotDiverge(); testKnotOnALevelSegmentIsANoOp(); + testKnotOnANearLevelSegmentIsANoOp(); testDegenerateInputsAreNoOps(); diff --git a/tests/test_envelope_overlay.cpp b/tests/test_envelope_overlay.cpp index 60fcc87..8f91d49 100644 --- a/tests/test_envelope_overlay.cpp +++ b/tests/test_envelope_overlay.cpp @@ -97,6 +97,18 @@ static void testDegenerateAreaAndDuration() { CHECK(gatePxPerSecond(Rect{}) == 0.0); } +// The literal PARAM-DOMAIN scale, independent of any sample duration: usable px = canvas width +// minus the last column minus 4 node-separation bases, spread over 4 x kGateStageMaxSeconds. +// This is what makes a dragged handle track the cursor 1:1 (envelope_edit's own inverse reads +// this same function) — a scale regression here is exactly what a relational-only check misses. +static void testGatePxPerSecond() { + const double expected = + (1000.0 - 1.0 - 4.0 * kGateNodeSepPx) / (4.0 * kGateStageMaxSeconds); // 967/8 px/s + CHECK(gatePxPerSecond(wideArea()) == expected); + CHECK(gatePxPerSecond(Rect::ltrb(5, 5, 5, 45)) == 0.0); // zero-width area -> 0 + CHECK(gatePxPerSecond(Rect::ltrb(0, 0, 10, 10)) > 0.0); // tiny area: usable floors at 1px, > 0 +} + // --- the AHDSR schematic ------------------------------------------------------ static void testAhdsrNodeOrderAndLevels() { @@ -122,6 +134,33 @@ static void testAhdsrNodeOrderAndLevels() { CHECK(v.x == a.right() - 1); // ANCHORED, whatever the release is } +// The literal per-node x placement, hand-derived from the documented formula (pps = 120.875 +// px/s per testGatePxPerSecond; each timed stage is prefixed by the kGateNodeSepPx=8 base): +// attack .2s -> raw 8+24.175=32.175 -> px 32; hold .1s -> raw 32.175+8+12.0875=52.2625 -> px 52; +// decay .3s -> raw 52.2625+8+36.2625=96.525 -> px 97; plateau -> raw 999-8-48.35=942.65 -> px +// 943; release end pinned at the last column, 999. A literal regression pin — no relational or +// bounds-only check catches a formula-shape change the way an exact pixel count does. +static void testAhdsrSchematicPlacement() { + const Rect a = wideArea(); + const std::vector poly = + buildEnvelopePolyline(ahdsr(0.2, 0.1, 0.3, 0.5, 0.4), overlayOf(a), 4.0); + EnvVertex v; + CHECK(findNode(poly, EnvNode::AttackEnd, v) && v.x == a.x + 32); + CHECK(findNode(poly, EnvNode::HoldEnd, v) && v.x == a.x + 52); + CHECK(findNode(poly, EnvNode::DecayEnd, v) && v.x == a.x + 97); + CHECK(findNode(poly, EnvNode::ReleaseStart, v) && v.x == a.x + 943); + CHECK(findNode(poly, EnvNode::ReleaseEnd, v) && v.x == a.x + 999); +} + +// The AHDSR schematic is scaled by the PARAM domain, NOT the capture length: the same params +// produce the SAME polyline whether totalSeconds is 0.3 or 10 (gatePolyline doesn't even take +// totalSeconds — only the sustain-less AHD's x-axis is wall-clock/PCM-aligned). +static void testGateLayoutIndependentOfSampleDuration() { + const Rect a = wideArea(); + const StageEnvelope e = ahdsr(0.2, 0.1, 0.3, 0.5, 0.06); + CHECK(buildEnvelopePolyline(e, overlayOf(a), 0.3) == buildEnvelopePolyline(e, overlayOf(a), 10.0)); +} + // The layout failure this policy exists to fix: at zero release the sustain plateau must run to // (near) the right edge instead of the figure bunching left. static void testZeroReleasePutsTheSustainPlateauAtTheRightEdge() { @@ -176,12 +215,29 @@ static void testMaxedStagesCompressWithoutOverrunning() { for (std::size_t i = 1; i < 6; ++i) { CHECK(poly[i].x >= poly[i - 1].x); CHECK(poly[i].x <= a.right() - 1); + // The compression exists to preserve MINIMUM gaps under overrun, not merely + // non-strict monotonicity — a compression that let two nodes collapse onto one + // pixel would still pass a `>=` check but defeat the whole point of kGateNodeSepPx. + CHECK(poly[i].x - poly[i - 1].x >= kGateNodeSepPx - 1); } EnvVertex end; CHECK(findNode(poly, EnvNode::ReleaseEnd, end)); CHECK(end.x == a.right() - 1); } +// Absurd stage values must clamp in double space, not overflow the integer cast (32-bit long +// on Windows would wrap negative and land on the WRONG edge) — a regression named for the bug +// it once was. gateVtx's own double-space clamp is what this exercises. +static void testAbsurdReleaseValueStaysInBounds() { + const Rect a = wideArea(); + StageEnvelope huge = ahdsr(0.1, 0.1, 0.1, 0.5, 0.1); + huge.releaseSeconds = 1e12; + for (const EnvVertex& v : buildEnvelopePolyline(huge, overlayOf(a), 4.0)) { + CHECK(v.x >= a.x && v.x < a.right()); + CHECK(v.y >= a.y && v.y < a.bottom()); + } +} + // --- the AHD split ------------------------------------------------------------ // The combined-time bound, asserted structurally across the full domains: no (attack, decay, @@ -322,12 +378,16 @@ int main() { testTimeToXClampsBothEnds(); testLevelToY(); testDegenerateAreaAndDuration(); + testGatePxPerSecond(); testAhdsrNodeOrderAndLevels(); + testAhdsrSchematicPlacement(); + testGateLayoutIndependentOfSampleDuration(); testZeroReleasePutsTheSustainPlateauAtTheRightEdge(); testReleaseGrowsLeftwardFromTheAnchor(); testTierZeroDefaultsKeepEveryNodeDistinct(); testMaxedStagesCompressWithoutOverrunning(); + testAbsurdReleaseValueStaysInBounds(); testAhdSplitNeverExceedsTheSpan(); testHoldFractionEndpoints(); diff --git a/tests/test_live_delivery.cpp b/tests/test_live_delivery.cpp index 189e938..c7448a8 100644 --- a/tests/test_live_delivery.cpp +++ b/tests/test_live_delivery.cpp @@ -474,6 +474,62 @@ static void testEveryEnvelopeStageTimeAndLevelMovesTheSoundingNote() { s.play.pitchEnv.peakSemitones = 12.0; }, [](LiveValues& v) { v.pitchEnv.peakSemitones = 0.0; }, -1}, + // A small hold that finishes the envelope well inside the render window (Hold ends at + // frame ~5090, comfortably short of the window) vs. a live move that opens the hold out + // near the whole span: with the fraction alone unmoved, the boundary the two renders + // cross (or don't) inside the observed tail is what makes this audible, not a level + // change — Hold's own output is flat regardless of exactly where inside it pos_ sits. + {"pitch env hold fraction", + [](SampleData& s) { + s.play.pitchEnv.enabled = true; + s.play.pitchEnv.peakSemitones = 12.0; + s.play.pitchEnv.shape.attackFrames = 100; + s.play.pitchEnv.shape.decayFrames = 100; + s.play.pitchEnv.shape.holdFraction = 0.05; + }, + [](LiveValues& v) { v.pitchEnv.shape.holdFraction = 1.0; }, -1}, + + // The Trigger AHDs — Voice::applyLive's ampAhd_/filterAhd_ branches, otherwise + // unexercised by this table (every case above is Gate/AdsrEnvelope). + {"trigger amp attack (AHD)", + [](SampleData& s) { + s.play.playMode = PlayMode::Trigger; + s.play.trigger.lengthFraction = 1.0; + s.play.trigAhd.attackFrames = 48000; + }, + [](LiveValues& v) { v.ampAhd.attackFrames = 4000; }, -1}, + {"trigger filter attack (AHD)", + [](SampleData& s) { + s.play.playMode = PlayMode::Trigger; + s.play.trigger.lengthFraction = 1.0; + filterSweep(s); + s.play.filter.trigEnv.attackFrames = 48000; + }, + [](LiveValues& v) { v.filterAhd.attackFrames = 4000; }, -1}, + + // One curve exponent per envelope (amp/pitch/filter), reusing each envelope's own + // attack/decay rig above so only the mutated field differs. + {"amp attack curve", + [](SampleData& s) { s.play.adsr.attackFrames = 48000; }, + [](LiveValues& v) { v.adsr.attackCurve = 5.0; }, -1}, + {"amp release curve", + [](SampleData& s) { s.play.adsr.releaseFrames = 48000; }, + [](LiveValues& v) { v.adsr.releaseCurve = 5.0; }, 2}, + {"pitch env attack curve", + [](SampleData& s) { + s.play.pitchEnv.enabled = true; + s.play.pitchEnv.shape.attackFrames = 48000; + s.play.pitchEnv.shape.decayFrames = 48000; + s.play.pitchEnv.peakSemitones = 12.0; + }, + [](LiveValues& v) { v.pitchEnv.shape.attackCurve = 5.0; }, -1}, + {"filter env decay curve", + [](SampleData& s) { + filterSweep(s); + s.play.filter.env.decayFrames = 48000; + s.play.filter.env.sustainLevel = 0.0; + }, + [](LiveValues& v) { v.filterEnv.decayCurve = 5.0; }, -1}, }; for (const Case& c : cases) { assertLiveFieldMovesTheSoundingNote(c.name, c.rig, c.mutate, c.noteOffBlock); diff --git a/tests/test_sampler_core.cpp b/tests/test_sampler_core.cpp index dadbca7..4b24e0d 100644 --- a/tests/test_sampler_core.cpp +++ b/tests/test_sampler_core.cpp @@ -1068,21 +1068,24 @@ static void testTriggerLengthWithStart() { } // --- Trigger fade-in / fade-out ramp shape (equal-power default). --- -static void testTriggerFadeShape() { - // 100 frames, 100% length, fadeIn 20, fadeOut 20. Head ramps 0->1, tail ramps 1->0, unity - // between. Equal-power: sin/cos ramps, monotonic, endpoints ~0 and ~1. +// 100 frames, 100% length, attack 20 / decay 20, holdFraction 1.0 — triggerSample() sets no +// curve exponent, so both stages default to util::kCurveNeutral (1.0): the ramps are LINEAR, +// not the retired fade pair's equal-power sin/cos. Asserted against the closed form rather +// than monotonicity alone — a monotonicity-only check is blind to exactly this shape change. +static void testTriggerAhdFadeShape() { SampleData km = (triggerSample(100, 1.0, 20, 20)); VoiceEngine eng(1, km); eng.noteOn(60, 127); std::vector out; eng.render(out, 120); - CHECK(approx(out[0], 0.0, 1e-3)); // fade-in starts at 0 - // Fade-in monotonic non-decreasing. - for (std::size_t i = 1; i < 20; ++i) CHECK(out[i] >= out[i - 1] - 1e-4); - // Unity plateau in the middle. - for (std::size_t i = 25; i < 75; ++i) CHECK(approx(out[i], 1.0, 1e-3)); - // Fade-out monotonic non-increasing over [80,100). - for (std::size_t i = 81; i < 100; ++i) CHECK(out[i] <= out[i - 1] + 1e-4); + for (std::size_t i = 0; i < 20; ++i) { + CHECK(approx(out[i], static_cast(i) / 20.0, 1e-3)); + } + // Hold plateau at unity. + for (std::size_t i = 20; i < 80; ++i) CHECK(approx(out[i], 1.0, 1e-3)); + for (std::size_t i = 80; i < 100; ++i) { + CHECK(approx(out[i], 1.0 - static_cast(i - 80) / 20.0, 1e-3)); + } // Past playEnd = silence. for (std::size_t i = 100; i < 120; ++i) CHECK(approx(out[i], 0.0, 1e-6)); } @@ -1099,7 +1102,7 @@ static void testTriggerEdgeCases() { for (float v : out) CHECK(approx(v, 0.0, 1e-6)); CHECK(eng.activeVoiceCount() == 0); } - // Fades that sum beyond the play length are clamped (no crash, no negative gain, amp in [0,1]). + // AHD attack + decay beyond the play length are fitted by fitAhd, not overflowed (no crash, no negative gain, amp in [0,1]). { // 40 frames, 100% -> playLen 40; fadeIn 30 + fadeOut 30 = 60 > 40 -> clamped. SampleData km = (triggerSample(40, 1.0, 30, 30)); @@ -2578,7 +2581,7 @@ int main() { testAhdsrHoldZeroEqualsAdsr(); testTriggerLengthFractionFrames(); testTriggerLengthWithStart(); - testTriggerFadeShape(); + testTriggerAhdFadeShape(); testTriggerEdgeCases(); testTriggerIgnoresNoteOff(); diff --git a/tests/test_staged_envelopes.cpp b/tests/test_staged_envelopes.cpp index a4fdd9c..ba3897d 100644 --- a/tests/test_staged_envelopes.cpp +++ b/tests/test_staged_envelopes.cpp @@ -249,27 +249,36 @@ static void testEachModePlaysItsOwnStageValuesAndTheOtherSurvives() { s.play.trigAhd.attackFrames = 0; s.play.trigAhd.decayFrames = 2000; s.play.trigAhd.holdFraction = 0.0; + const AdsrParams originalAdsr = s.play.adsr; - const auto renderFirst = [&](PlayMode mode) { - SampleData copy = s; - copy.play.playMode = mode; - VoiceEngine eng(1, copy); + const auto render = [&]() { + VoiceEngine eng(1, s); eng.noteOn(60, 127); std::vector out; eng.render(out, 1000); return out; }; - const std::vector gate = renderFirst(PlayMode::Gate); + s.play.playMode = PlayMode::Gate; + const std::vector gate = render(); CHECK(gate[0] < 0.01f); // halfway up a 2000-frame attack CHECK(std::fabs(gate[999] - 999.0f / 2000.0f) < 1e-3f); - const std::vector trig = renderFirst(PlayMode::Trigger); + // The actual excursion: flip the SAME SampleData to Trigger and render through it — not a + // fresh copy. A prior revision of this test copied `s` per render and never wrote it, so + // "back to Gate" compared two renders of an object nothing had touched: it would have + // passed with trigAhd deleted from the struct. + s.play.playMode = PlayMode::Trigger; + const std::vector trig = render(); CHECK(trig[0] > 0.99f); // instant onset CHECK(std::fabs(trig[999] - (1.0f - 999.0f / 2000.0f)) < 1e-3f); - // Back to Gate: the AHDSR values were never touched by the excursion. - const std::vector again = renderFirst(PlayMode::Gate); + // Flip back to Gate on the SAME object: the AHDSR fields must have survived the round trip + // and the render must reproduce the pre-excursion output exactly. + s.play.playMode = PlayMode::Gate; + CHECK(s.play.adsr.attackFrames == originalAdsr.attackFrames); + CHECK(s.play.adsr.sustainLevel == originalAdsr.sustainLevel); + const std::vector again = render(); for (std::size_t i = 0; i < gate.size(); ++i) CHECK(again[i] == gate[i]); } @@ -366,7 +375,10 @@ static void testTriggerPreserveEndsWithoutATerminalDiscontinuity() { const double kSourceSlope = 0.8 * 2.0 * kPi / 40.0; for (VoiceMode vm : {VoiceMode::Poly, VoiceMode::Mono}) { const TailMeasure m = renderTail(vm, PitchEngine::Preserve, 8000); - CHECK(m.soundingFrames > 0); + // > 0 alone would pass on a hard cut with no blend at all; the ring-out is ~185 frames + // past the kRingSpanFrames=4000 span, so requiring it past the span is what confirms + // the blend actually engaged rather than merely stopping somewhere. + CHECK(m.soundingFrames > 4000); CHECK(m.worstStep <= kSourceSlope * 1.5); // And the voice genuinely reaches silence rather than being left ringing. CHECK(m.lastLevel < 1e-3); @@ -411,9 +423,9 @@ static void testTriggerPreserveAhdEndingEarlyAlsoRingsOut() { } // Varispeed is not implicated and must be left exactly as it was: its terminal sample is real -// source content at its natural end, so no ring-out is armed there. Asserted as byte-identity -// between two renders of the same rig, one of which would differ if the Preserve-only guard -// were ever widened. +// source content at its natural end, so no ring-out is armed there. Asserted directly against +// the closed-form source waveform (not by comparing two renders): the read head reproduces the +// source exactly through its span, then stops dead with no ring-out appended. static void testVarispeedTailIsUntouched() { const auto render = [](std::size_t frames) { SampleData s; @@ -597,6 +609,30 @@ static void testPitchEnvelopeSpanIsOutputFramesUnderVarispeed() { CHECK(soundingFrames(84, -24.0) == 1750); } +// --- Live delivery must not resurrect a finished AHD --------------------------- + +// A live move must never UN-finish a completed AHD. Reachable on a sounding voice past its +// fitted total but still active() (the ring-out past a Trigger AHD's own early end, voice.h) — +// VoiceEngine::applyLiveToActive runs on any active() voice, so a live move there that reopens +// the total could otherwise make a finished one-shot briefly resurge. +static void testApplyLiveNeverUnfinishesACompletedAhd() { + AhdParams p; + p.attackFrames = 10; + p.decayFrames = 10; + p.holdFraction = 0.0; // total = 20, well short of the 1000-frame span + AhdEnvelope ahd; + ahd.configure(1000, p); + CHECK(ahd.amplitudeAt(500.0) == 0.0); // past the fitted total: latched finished + CHECK(ahd.finished()); + + // A live move that would, on a FRESH fit, reopen the total out across the whole span. + AhdParams reopened = p; + reopened.holdFraction = 1.0; + ahd.applyLive(500.0, reopened); // same sourceOffset the voice is sitting at + CHECK(ahd.finished()); // must stay finished — no resurge + CHECK(ahd.amplitudeAt(500.0) == 0.0); +} + // --- Migration shape ---------------------------------------------------------- // A prior ZERO fade-out migrates to Decay = 0 and keeps the abrupt end the old controls could @@ -634,6 +670,8 @@ int main() { testPitchEnvelopeSpanIsOutputFramesUnderVarispeed(); + testApplyLiveNeverUnfinishesACompletedAhd(); + testZeroFadeOutMigratesToAnAbruptEnd(); if (g_fail == 0) { diff --git a/tests/test_theme.cpp b/tests/test_theme.cpp index 8ca4460..69dc5db 100644 --- a/tests/test_theme.cpp +++ b/tests/test_theme.cpp @@ -132,13 +132,18 @@ static void testOverlayAccentSeparatesFromTheWaveformAccent() { const KitColor overlay = roleColor(Role::AccentTertiary); const KitColor prior = roleColor(Role::AccentSecondary); CHECK(contrastRatio(overlay, wave) > contrastRatio(prior, wave)); + // Pin the MEASURED value, not just the relational improvement: today's pair sits at + // ~1.37:1 — BELOW the 3:1 indicator floor every other assertion in this file uses. That + // is a known, held deviation (Daniel's palette call, tracked separately) rather than a + // silently accepted one; a regression toward the old ~1.12:1 secondary pairing must fail. + CHECK(contrastRatio(overlay, wave) >= 1.35); // Hue divergence against the waveform: the waveform's green dominates its red, the // overlay's red dominates its green — opposite balances, not two shades of one. CHECK(wave.g > wave.r); CHECK(overlay.r > overlay.g); const int delta = std::abs(int(wave.r) - int(overlay.r)) + std::abs(int(wave.g) - int(overlay.g)) + - std::abs(int(wave.b) - int(overlay.b)) ; + std::abs(int(wave.b) - int(overlay.b)); CHECK(delta >= 60); }