diff --git a/src/core/instrument/engine/voice.cpp b/src/core/instrument/engine/voice.cpp index 4e75c63..e7cad8f 100644 --- a/src/core/instrument/engine/voice.cpp +++ b/src/core/instrument/engine/voice.cpp @@ -95,6 +95,9 @@ void Voice::start(int note, int velocity, const SampleData& sample, bool declick // baseRatio_ source frames per output frame — needs the span converted, or a transposed // note's envelope outruns (or outlives) the note it shapes. Preserve reads at the source // rate, so its two domains already coincide. + // Divides by baseRatio_ alone, though the actual Varispeed read rate is baseRatio_ x + // envFactor — a deep pitch envelope makes this a first-order approximation, not exact. + // Strictly better than the un-converted source-frame span it replaced. const double pitchSpan = (pitchEngine_ == PitchEngine::Preserve || !(baseRatio_ > 0.0)) ? static_cast(postStart) @@ -268,6 +271,9 @@ void Voice::retune(int note) { // legato phrase is one gesture, one strike (classic mono-synth behavior). if (!active_ || sample_ == nullptr) return; note_ = note; + // Changes baseRatio_ without re-converting pitchEnv_'s already-configured span (the + // baseRatio_ division in the note-on setup above), so a slide leaves that envelope on the + // first note's domain — consistent with "touch nothing else," but the drift lives here. baseRatio_ = keyTrackedRatio(note, sample_->rootNote, sample_->keyTrack); // Filter key-tracking follows the pitch: it is a function of the note, so a slide moves it // too. The velocity offset deliberately stays the first note's, matching velocityGain_. diff --git a/src/core/instrument/engine/voice.h b/src/core/instrument/engine/voice.h index 8bdb48f..2d7460b 100644 --- a/src/core/instrument/engine/voice.h +++ b/src/core/instrument/engine/voice.h @@ -6,11 +6,6 @@ // 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/ui/CMakeLists.txt b/src/core/instrument/ui/CMakeLists.txt index 2e090a3..03d8ee8 100644 --- a/src/core/instrument/ui/CMakeLists.txt +++ b/src/core/instrument/ui/CMakeLists.txt @@ -51,7 +51,7 @@ reasampler_test(knob_deck LINK knob_deck) # MorphLaw) for the v9 filter tail -- plain value types, no filter symbol linked. reasampler_pure_library(deck_groups SOURCES deck_groups.cpp - LINK PUBLIC knob_deck velocity_curve peaks) + LINK PUBLIC knob_deck velocity_curve peaks curve_law) # sample_bands is linked directly for the test only: the deck-fits-the-floor-window assertion # needs the band allocator deck_groups itself has no reason to depend on. reasampler_test(deck_groups LINK deck_groups sample_bands) diff --git a/src/core/instrument/ui/envelope_edit.cpp b/src/core/instrument/ui/envelope_edit.cpp index 1d77279..01cece9 100644 --- a/src/core/instrument/ui/envelope_edit.cpp +++ b/src/core/instrument/ui/envelope_edit.cpp @@ -108,11 +108,29 @@ double curveFromKnotDrag(const StageEnvelope& grabEnv, EnvNode knot, double grab return curveFromMidLevel((newLevel - seg.start) / span); } +// An AHD's DecayEnd moves decaySeconds via X, scaled by 1/(1 - holdFraction) — see +// resolveNodeDrag's DecayEnd case. At holdFraction == 1.0 that derivative is exactly 0, so a +// drag there can never change anything; when it ALSO coincides with HoldEnd (decay ~ 0) it is a +// dead handle sitting on top of a live one. Excluded from the grabbable set in that exact case +// only — a functional DecayEnd (holdFraction < 1) stays grabbable even when it coincides. +bool ahdDecayEndIsDead(const StageEnvelope& env, const std::vector& poly) { + if (env.kind != EnvKind::Ahd) return false; + if (1.0 - clamp01(env.holdFraction) > 1e-9) return false; + EnvVertex hold, decay; + bool haveHold = false, haveDecay = false; + for (const EnvVertex& v : poly) { + if (v.node == EnvNode::HoldEnd) { hold = v; haveHold = true; } + else if (v.node == EnvNode::DecayEnd) { decay = v; haveDecay = true; } + } + return haveHold && haveDecay && hold.x == decay.x; +} + } // namespace NodeHit nodeAtPoint(const StageEnvelope& env, const OverlayArea& area, double totalSeconds, int x, int y) { const std::vector poly = buildEnvelopePolyline(env, area, totalSeconds); + const bool dropDeadDecayEnd = ahdDecayEndIsDead(env, poly); // Nearest draggable, kind-matching node within the pick radius wins (Chebyshev distance); // ties go to the earlier draw-order node. Knots are appended last, so a knot coincident // with an endpoint handle loses — a drag there stays a time edit. @@ -120,6 +138,7 @@ NodeHit nodeAtPoint(const StageEnvelope& env, const OverlayArea& area, double to int bestDist = kNodeGrabRadius + 1; for (const EnvVertex& v : poly) { if (!isDraggable(v.node) || !nodeInKind(v.node, env.kind)) continue; + if (dropDeadDecayEnd && v.node == EnvNode::DecayEnd) continue; const int dist = std::max(std::abs(x - v.x), std::abs(y - v.y)); if (dist < bestDist) { // strict-less-than keeps ties at the earlier draw order bestDist = dist; diff --git a/src/core/instrument/ui/envelope_edit.h b/src/core/instrument/ui/envelope_edit.h index c4f104f..b36bd05 100644 --- a/src/core/instrument/ui/envelope_edit.h +++ b/src/core/instrument/ui/envelope_edit.h @@ -37,7 +37,9 @@ struct EnvClampBounds { // inputs buildEnvelopePolyline drew from). `hit` is false for a point off every draggable node. // Nearest node within the radius wins (Chebyshev distance); an exact tie goes to the earlier // draw-order node, and since knots are appended last, a coincident endpoint handle wins over a -// knot rather than the drag silently becoming a curve edit. +// knot rather than the drag silently becoming a curve edit. An AHD's DecayEnd is excluded +// entirely when it coincides with HoldEnd AND holdFraction == 1.0 — there it cannot move +// (resolveNodeDrag's derivative is 0), so it is dropped rather than won by draw order. struct NodeHit { bool hit = false; EnvNode node = EnvNode::Origin; // meaningful only when hit == true diff --git a/src/core/instrument/ui/envelope_overlay.cpp b/src/core/instrument/ui/envelope_overlay.cpp index 99a0964..a611249 100644 --- a/src/core/instrument/ui/envelope_overlay.cpp +++ b/src/core/instrument/ui/envelope_overlay.cpp @@ -177,19 +177,14 @@ 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)); - 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); + pts.push_back(vtx(EnvNode::HoldEnd, area, totalSeconds, t0 + s.attack + s.hold, 1.0)); + // DecayEnd is drawn at t0 + total, which coincides with HoldEnd exactly when decay ~ 0 — + // independent of holdFraction (total = attack + hold + decay always). Left at its true + // instant rather than nudged: the 1:1 axis this policy exists to keep honest must hold even + // at a shared instant, including the Trigger default's abrupt (zero-decay) cutoff. + // envelope_edit's nodeAtPoint handles the coincidence instead, by dropping DecayEnd from the + // grabbable set when it also cannot move (holdFraction == 1.0). + pts.push_back(vtx(EnvNode::DecayEnd, area, totalSeconds, t0 + s.total, 0.0)); 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/tests/test_curve_law.cpp b/tests/test_curve_law.cpp index 46789f7..a5e7f48 100644 --- a/tests/test_curve_law.cpp +++ b/tests/test_curve_law.cpp @@ -156,8 +156,11 @@ static void testKnobLawRoundTripsOutsideTheDetent() { const double back = curveFromKnobNorm(knobNormFromCurve(e)); CHECK(std::fabs(back - e) < 1e-9); } - CHECK(curveFromKnobNorm(0.0) == kCurveMin); - CHECK(curveFromKnobNorm(-3.0) == kCurveMin); // out-of-range norm saturates + // exp(-log(10)) is not guaranteed bit-exact to kCurveMin's literal 0.1 (1-2 ulp either way); + // both norms below collapse to the same t == 0.0 computation, so both get the same + // tolerance rather than leaning on clampCurve's floor to land on it by luck. + CHECK(std::fabs(curveFromKnobNorm(0.0) - kCurveMin) < 1e-12); + CHECK(std::fabs(curveFromKnobNorm(-3.0) - kCurveMin) < 1e-12); // out-of-range norm saturates CHECK(std::fabs(curveFromKnobNorm(1.0) - kCurveMax) < 1e-12); // The ENDS need only land on the domain, not on an exact norm — 0.1 is not exactly 1/10 in // binary, so log(kCurveMin) is a hair off -log(kCurveMax). Only the centre carries an diff --git a/tests/test_envelope_edit.cpp b/tests/test_envelope_edit.cpp index 824b1a3..add5ff9 100644 --- a/tests/test_envelope_edit.cpp +++ b/tests/test_envelope_edit.cpp @@ -4,7 +4,8 @@ // pixel delta produces exactly the param a knob would have. // // Covers: nodeAtPoint (every drawn handle grabbable, the anchored ReleaseEnd and the Origin -// never grabbed, other-kind nodes rejected, misses outside the radius); resolveNodeDrag +// never grabbed, other-kind nodes rejected, misses outside the radius, a dead coincident AHD +// DecayEnd excluded while a functional one stays grabbable); resolveNodeDrag // (AHDSR stage times at the schematic scale, the sustain level on Y, the release dragged from // its START with the inverted sign, the caller's clamp domain, AHD stage times at the 1:1 // scale, the hold FRACTION); curve-knot drags (the exponent domain, its endpoints, and the @@ -47,6 +48,18 @@ static StageEnvelope ahdsrEnv() { return e; } +// F1's coincidence cases need attack/decay/fraction/span combinations ahdEnv() doesn't cover. +static StageEnvelope ahd(double a, double d, double frac, double origin, double span) { + StageEnvelope e; + e.kind = EnvKind::Ahd; + e.attackSeconds = a; + e.decaySeconds = d; + e.holdFraction = frac; + e.originSeconds = origin; + e.spanSeconds = span; + return e; +} + static StageEnvelope ahdEnv() { StageEnvelope e; e.kind = EnvKind::Ahd; @@ -117,6 +130,26 @@ static void testAhdHasNoSustainNodes() { CHECK(out.attackSeconds == e.attackSeconds); } +// F1: at the Trigger default (decay 0, holdFraction 1.0) DecayEnd sits on HoldEnd's own instant +// AND cannot move there (resolveNodeDrag's decay branch has derivative 0 — see the denom guard). +// A grab at its true (now un-nudged) position must miss rather than resolve to a dead handle; +// HoldEnd, the live node underneath, stays fully grabbable. +static void testDeadCoincidentDecayEndIsNotGrabbable() { + const StageEnvelope e = ahd(0.5, 0.0, 1.0, 0.0, 3.0); + CHECK(!grabAt(e, EnvNode::DecayEnd).hit); + CHECK(grabAt(e, EnvNode::HoldEnd).hit); + CHECK(grabAt(e, EnvNode::HoldEnd).node == EnvNode::HoldEnd); +} + +// Coincidence alone does not drop DecayEnd — only holdFraction == 1.0 makes it truly dead. With +// holdFraction < 1 the X-drag still moves decaySeconds (denom > 0), so it stays grabbable even +// when decay is 0 and it starts out coincident with HoldEnd. +static void testFunctionalCoincidentDecayEndStaysGrabbable() { + const StageEnvelope e = ahd(0.5, 0.0, 0.5, 0.0, 3.0); + CHECK(grabAt(e, EnvNode::DecayEnd).hit); + CHECK(grabAt(e, EnvNode::DecayEnd).node == EnvNode::DecayEnd); +} + static void testMissOutsideTheRadius() { const StageEnvelope e = ahdsrEnv(); const Rect a = wideArea(); @@ -310,6 +343,8 @@ int main() { testEveryDrawnHandleIsGrabbable(); testAnchoredEndAndOriginAreNotGrabbable(); testAhdHasNoSustainNodes(); + testDeadCoincidentDecayEndIsNotGrabbable(); + testFunctionalCoincidentDecayEndStaysGrabbable(); testMissOutsideTheRadius(); testAhdsrStageTimesTrackTheSchematicScale(); diff --git a/tests/test_envelope_overlay.cpp b/tests/test_envelope_overlay.cpp index 8f91d49..b16b323 100644 --- a/tests/test_envelope_overlay.cpp +++ b/tests/test_envelope_overlay.cpp @@ -102,9 +102,10 @@ static void testDegenerateAreaAndDuration() { // 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); + // 967 / 8 px/s, pinned as a literal — restating the formula with the same named constants + // would let a change to kGateNodeSepPx or kGateStageMaxSeconds move both sides and pass + // silently. + CHECK(gatePxPerSecond(wideArea()) == 120.875); 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 } @@ -304,6 +305,36 @@ static void testAhdIsOneToOneWithTheTimeAxis() { CHECK(!hasNode(poly, EnvNode::ReleaseCurve)); } +// F1 regression: DecayEnd stays at its true wall-clock instant even when that instant coincides +// with HoldEnd's (decay ~ 0) — the 1:1 AHD axis promises N seconds -> N seconds, and a nudge +// away from that instant lies about the shape, including the Trigger default's abrupt cutoff. +// Fails against the prior nudge, which moved DecayEnd right whenever the gap was under +// kGateNodeSepPx. +static void testDecayEndStaysAtItsTrueInstantEvenWhenCoincidentWithHoldEnd() { + const Rect a = wideArea(); + const double total = 4.0; + + // Short but nonzero decay: the true gap to HoldEnd is a few px, under kGateNodeSepPx, so + // the retired nudge would have fired here too. + const StageEnvelope shortDecay = ahd(0.5, 0.02, 1.0, 0.0, 3.0); + const AhdSplit sShort = splitAhdSeconds(shortDecay); + EnvVertex decayShort; + CHECK(findNode(buildEnvelopePolyline(shortDecay, overlayOf(a), total), EnvNode::DecayEnd, + decayShort)); + CHECK(decayShort.x == timeToX(a, total, sShort.total)); + + // Zero decay (the Trigger AHD default's shape): DecayEnd and HoldEnd share the exact same + // instant — the abrupt cutoff — and DecayEnd must not be nudged off it. + const StageEnvelope zeroDecay = ahd(0.5, 0.0, 1.0, 0.0, 3.0); + const AhdSplit sZero = splitAhdSeconds(zeroDecay); + const std::vector polyZero = buildEnvelopePolyline(zeroDecay, overlayOf(a), total); + EnvVertex decayZero, holdZero; + CHECK(findNode(polyZero, EnvNode::DecayEnd, decayZero)); + CHECK(findNode(polyZero, EnvNode::HoldEnd, holdZero)); + CHECK(decayZero.x == timeToX(a, total, sZero.total)); + CHECK(decayZero.x == holdZero.x); // truly coincident, not nudged apart +} + // --- curve knots -------------------------------------------------------------- // A knot rides every sloped stage that has a duration, and none that does not — a zero-length @@ -392,6 +423,7 @@ int main() { testAhdSplitNeverExceedsTheSpan(); testHoldFractionEndpoints(); testAhdIsOneToOneWithTheTimeAxis(); + testDecayEndStaysAtItsTrueInstantEvenWhenCoincidentWithHoldEnd(); testKnotsRideOnlySlopedNonZeroSegments(); testKnotHeightTracksTheExponent(); diff --git a/tests/test_sample_map.cpp b/tests/test_sample_map.cpp index 90a173f..4cd78b6 100644 --- a/tests/test_sample_map.cpp +++ b/tests/test_sample_map.cpp @@ -703,8 +703,10 @@ static void testMigratedFadeStretchesWhenTheDecodeRateDiffersFromTheProjectRate( CHECK(matched.trigAhd.attackFrames == 441); CHECK(matched.trigAhd.decayFrames == 882); - // A 44.1 kHz file opened in a 48 kHz project: 441 * 48000/44100 = 480 source frames, ~8.8% - // longer than the fade the saved instance actually had. + // resolvePlay's second argument is the DECODE rate; the seconds above were lifted (divided) + // at the PROJECT rate 44100 — so this is a 48 kHz file opened in a 44.1 kHz project (the + // mirror of component_state_io.h's worked example): 441 * 48000/44100 = 480 source frames, + // ~8.8% longer than the fade the saved instance actually had. const PlayParams stretched = resolvePlay(st, 48000); CHECK(stretched.trigAhd.attackFrames == 480); CHECK(stretched.trigAhd.decayFrames == 960);