diff --git a/src/core/instrument/ui/envelope_edit.cpp b/src/core/instrument/ui/envelope_edit.cpp index dc5e2f0..5c76c61 100644 --- a/src/core/instrument/ui/envelope_edit.cpp +++ b/src/core/instrument/ui/envelope_edit.cpp @@ -11,8 +11,8 @@ namespace reasampler::instrument::ui { using util::clamp01; -using util::curveFromMidLevel; -using util::curveMidLevel; +using util::curveFromLevelAt; +using util::curveLevelAt; namespace { @@ -105,10 +105,35 @@ SegmentLevels segmentLevels(const StageEnvelope& env, EnvNode knot) { return s; } -// A knot drag: the grab-time mid-level shifted by the pixel delta, read back through -// curve_law's inverse (curve_law.h owns why the knot and the inner dial share this one law). +// The pixel bounds of the segment a curve knot rides, by node — read off the SAME polyline the +// draw built (never re-derived), so the drag's phi can never disagree with knotVtx's. +struct SegmentPixels { + int x0 = 0; + int x1 = 0; + bool ok = false; +}; +SegmentPixels segmentPixels(const std::vector& poly, EnvNode knot) { + EnvNode startNode, endNode; + switch (knot) { + case EnvNode::AttackCurve: startNode = EnvNode::Origin; endNode = EnvNode::AttackEnd; break; + case EnvNode::DecayCurve: startNode = EnvNode::HoldEnd; endNode = EnvNode::DecayEnd; break; + case EnvNode::ReleaseCurve: startNode = EnvNode::ReleaseStart; endNode = EnvNode::ReleaseEnd; break; + default: return {}; + } + SegmentPixels s; + bool haveStart = false, haveEnd = false; + for (const EnvVertex& v : poly) { + if (v.node == startNode) { s.x0 = v.x; haveStart = true; } + else if (v.node == endNode) { s.x1 = v.x; haveEnd = true; } + } + s.ok = haveStart && haveEnd; + return s; +} + +// A knot drag: the grab-time level at `phi` (the phi the knot's own drawn x implies — see +// knotPhi) shifted by the pixel delta, read back through curve_law's inverse at that same phi. double curveFromKnotDrag(const StageEnvelope& grabEnv, EnvNode knot, double grabExponent, - const Rect& area, double dyPixels) { + double phi, const Rect& area, double dyPixels) { const SegmentLevels seg = segmentLevels(grabEnv, knot); if (!seg.ok) return grabExponent; const double span = seg.end - seg.start; @@ -117,9 +142,9 @@ double curveFromKnotDrag(const StageEnvelope& grabEnv, EnvNode knot, double grab // ~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 grabLevel = seg.start + span * curveLevelAt(phi, grabExponent); const double newLevel = grabLevel - dyPixels * levelPerPixel(area); - return curveFromMidLevel((newLevel - seg.start) / span); + return curveFromLevelAt(phi, (newLevel - seg.start) / span); } // An AHD's DecayEnd moves decaySeconds via X, scaled by 1/(1 - holdFraction) — see @@ -178,6 +203,16 @@ StageEnvelope resolveNodeDrag(const StageEnvelope& grabEnv, EnvNode node, const const double dy = static_cast(dyPixels) * scale; const double dSec = dx * secPerPx; + // A curve knot's phi is read off the same polyline knotVtx drew, so the drag inverts the + // exact phi the knot is sitting at rather than assuming the segment midpoint. + double curvePhi = 0.5; + if (node == EnvNode::AttackCurve || node == EnvNode::DecayCurve || + node == EnvNode::ReleaseCurve) { + const std::vector poly = buildEnvelopePolyline(grabEnv, area, totalSeconds); + const SegmentPixels sp = segmentPixels(poly, node); + if (sp.ok) curvePhi = knotPhi(sp.x0, sp.x1); + } + if (grabEnv.kind == EnvKind::Ahdsr) { switch (node) { // Each cumulative-time node edits its own segment duration. Non-negative durations @@ -212,15 +247,15 @@ StageEnvelope resolveNodeDrag(const StageEnvelope& grabEnv, EnvNode node, const break; case EnvNode::AttackCurve: out.attackCurve = snappedExponent( - curveFromKnotDrag(grabEnv, node, grabEnv.attackCurve, rect, dy), mods); + curveFromKnotDrag(grabEnv, node, grabEnv.attackCurve, curvePhi, rect, dy), mods); break; case EnvNode::DecayCurve: out.decayCurve = snappedExponent( - curveFromKnotDrag(grabEnv, node, grabEnv.decayCurve, rect, dy), mods); + curveFromKnotDrag(grabEnv, node, grabEnv.decayCurve, curvePhi, rect, dy), mods); break; case EnvNode::ReleaseCurve: out.releaseCurve = snappedExponent( - curveFromKnotDrag(grabEnv, node, grabEnv.releaseCurve, rect, dy), mods); + curveFromKnotDrag(grabEnv, node, grabEnv.releaseCurve, curvePhi, rect, dy), mods); break; default: break; @@ -261,11 +296,11 @@ StageEnvelope resolveNodeDrag(const StageEnvelope& grabEnv, EnvNode node, const } case EnvNode::AttackCurve: out.attackCurve = snappedExponent( - curveFromKnotDrag(grabEnv, node, grabEnv.attackCurve, rect, dy), mods); + curveFromKnotDrag(grabEnv, node, grabEnv.attackCurve, curvePhi, rect, dy), mods); break; case EnvNode::DecayCurve: out.decayCurve = snappedExponent( - curveFromKnotDrag(grabEnv, node, grabEnv.decayCurve, rect, dy), mods); + curveFromKnotDrag(grabEnv, node, grabEnv.decayCurve, curvePhi, rect, dy), mods); break; default: break; diff --git a/src/core/instrument/ui/envelope_overlay.cpp b/src/core/instrument/ui/envelope_overlay.cpp index 4648aa8..ccd2066 100644 --- a/src/core/instrument/ui/envelope_overlay.cpp +++ b/src/core/instrument/ui/envelope_overlay.cpp @@ -9,8 +9,7 @@ namespace reasampler::instrument::ui { using util::clamp01; -using util::curveMap; -using util::curveMidLevel; +using util::curveLevelAt; int timeToX(const Rect& area, double totalSeconds, double t) { const int w = std::max(0, area.width); @@ -46,6 +45,12 @@ int levelToY(const Rect& area, double level) { return area.y + static_cast(dy); } +double knotPhi(int x0, int x1) { + if (x1 == x0) return 0.5; + const int mid = (x0 + x1) / 2; + return static_cast(mid - x0) / static_cast(x1 - x0); +} + AhdSplit splitAhdSeconds(const StageEnvelope& env) { AhdSplit out; const double span = std::max(0.0, env.spanSeconds); @@ -90,14 +95,16 @@ 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 read through curve_law.h's own law (the knot/dial pairing's home). +// pixel midpoint. Its level is read at the phi that midpoint's TRUNCATED x actually implies +// (knotPhi), not always phi = 0.5 — an odd-pixel span would otherwise draw the knot a half +// pixel off the curve its own vertices trace. curve_law.h owns the knot/dial pairing. EnvVertex knotVtx(EnvNode node, const Rect& area, int x0, int x1, double startLevel, double endLevel, double exponent) { - const double u = curveMidLevel(exponent); - const double level = startLevel + (endLevel - startLevel) * u; EnvVertex v; v.node = node; v.x = (x0 + x1) / 2; + const double u = curveLevelAt(knotPhi(x0, x1), exponent); + const double level = startLevel + (endLevel - startLevel) * u; v.y = levelToY(area, level); v.level = level; v.knot = true; diff --git a/src/core/instrument/ui/envelope_overlay.h b/src/core/instrument/ui/envelope_overlay.h index 2cc1f0d..aae605c 100644 --- a/src/core/instrument/ui/envelope_overlay.h +++ b/src/core/instrument/ui/envelope_overlay.h @@ -115,6 +115,13 @@ int timeToX(const Rect& area, double totalSeconds, double t); // clamped. Shared with envelope_edit's node hit-test. int levelToY(const Rect& area, double level); +// The normalized phi a curve knot's TRUNCATED integer x actually lands at within its bounding +// segment [x0, x1] — exactly 0.5 only when the span is even. Shared with envelope_edit's knot +// drag so the draw and its inverse read the same phi off the same formula rather than two +// copies that could drift apart. x0 == x1 (no interior) returns 0.5; callers never place a knot +// there. +double knotPhi(int x0, int x1); + // The A/H/D split of an AHD's span, in seconds — the pure-UI mirror of the engine's fitAhd, so // the drawn stage boundaries land where the voice actually puts them. Attack takes at most the // span and Decay at most what Attack left, so Hold's fraction of the remainder can never push diff --git a/src/core/util/curve_law.h b/src/core/util/curve_law.h index b8382b5..27bb25f 100644 --- a/src/core/util/curve_law.h +++ b/src/core/util/curve_law.h @@ -1,8 +1,8 @@ #pragma once // curve_law — the ONE per-segment envelope curve law: the exponent domain, the map from a -// stage's normalized position to its normalized level, and the mid-segment inverse the -// overlay knot drags through. Header-only and dependency-free so the engine evaluator, the -// overlay's forward map, and its inverse all read the same law rather than three copies. +// stage's normalized position to its normalized level, and that map's inverse (mid-segment is +// the special case). Header-only and dependency-free so the engine evaluator, the overlay's +// forward map, and its inverse all read the same law rather than three copies. #include @@ -58,19 +58,25 @@ inline double knobNormFromCurve(double exponent) { return t < 0.0 ? 0.0 : (t > 1.0 ? 1.0 : t); } -// The normalized level at a segment's MIDPOINT (phi = 0.5) — where the overlay places the -// draggable curve knot — and its inverse. The pair is what keeps knot-drag and inner dial on -// one value: both resolve through this law, not through each other. -inline double curveMidLevel(double exponent) { return curveMap(0.5, clampCurve(exponent)); } +// The normalized level at an arbitrary segment position phi in (0,1), and its inverse. A +// knot's DRAWN x truncates to an integer, which lands it off phi = 0.5 whenever its segment's +// pixel span is odd; reading the knot's y through the phi its own x actually implies (rather +// than assuming 0.5) is what keeps the knot on the trace its own vertices draw. +inline double curveLevelAt(double phi, double exponent) { return curveMap(phi, clampCurve(exponent)); } -// Mid-level -> exponent: u = 0.5^p, so p = ln(u)/ln(0.5). Out-of-domain u clamps to the +// Level -> exponent at phi: u = phi^p, so p = ln(u)/ln(phi). Out-of-domain u clamps to the // exponent endpoints rather than producing a non-finite exponent. -inline double curveFromMidLevel(double midLevel) { - const double lo = curveMidLevel(kCurveMax); // smallest reachable mid-level - const double hi = curveMidLevel(kCurveMin); // largest - if (!(midLevel > lo)) return kCurveMax; // also catches NaN - if (midLevel >= hi) return kCurveMin; - return clampCurve(std::log(midLevel) / std::log(0.5)); +inline double curveFromLevelAt(double phi, double level) { + const double lo = curveLevelAt(phi, kCurveMax); // smallest reachable level at this phi + const double hi = curveLevelAt(phi, kCurveMin); // largest + if (!(level > lo)) return kCurveMax; // also catches NaN + if (level >= hi) return kCurveMin; + return clampCurve(std::log(level) / std::log(phi)); } +// The segment-MIDPOINT (phi = 0.5) case — the knot's placement whenever its pixel span is +// even. Kept under its own name for the existing callers/tests that assume that case. +inline double curveMidLevel(double exponent) { return curveLevelAt(0.5, exponent); } +inline double curveFromMidLevel(double midLevel) { return curveFromLevelAt(0.5, midLevel); } + } // namespace reasampler::util diff --git a/tests/test_curve_law.cpp b/tests/test_curve_law.cpp index a5e7f48..10c90ba 100644 --- a/tests/test_curve_law.cpp +++ b/tests/test_curve_law.cpp @@ -14,6 +14,7 @@ #include #include +#include using namespace reasampler::util; @@ -107,6 +108,39 @@ static void testMidLevelInverseSaturates() { CHECK(std::fabs(curveFromMidLevel(0.5) - kCurveNeutral) < 1e-12); } +// curveLevelAt/curveFromLevelAt is the general form a knot's own (possibly off-centre) phi +// needs — curveMidLevel/curveFromMidLevel is the phi = 0.5 case, not a second law. +static void testMidLevelIsThePhiHalfSpecialCase() { + for (double e : {kCurveMin, 0.3, kCurveNeutral, 2.0, kCurveMax}) { + CHECK(curveLevelAt(0.5, e) == curveMidLevel(e)); + } + for (double u : {0.0, 0.2, 0.5, 0.8, 1.0}) { + CHECK(curveFromLevelAt(0.5, u) == curveFromMidLevel(u)); + } +} + +// The round trip must hold at an arbitrary phi, not only 0.5 — this is what a knot whose +// integer x lands off its segment's true midpoint (an odd pixel span) actually exercises. +static void testLevelAtRoundTripsAtArbitraryPhi() { + for (double phi : {0.1, 0.3, 0.42, 0.5, 0.63, 0.9}) { + for (int i = 0; i <= 50; ++i) { + const double e = kCurveMin + (kCurveMax - kCurveMin) * (i / 50.0); + const double level = curveLevelAt(phi, e); + CHECK(level > 0.0 && level < 1.0); + CHECK(std::fabs(curveFromLevelAt(phi, level) - e) < 1e-9); + } + } +} + +// Saturation holds at an arbitrary phi too, not only the mid-level special case. +static void testLevelAtInverseSaturatesAtArbitraryPhi() { + for (double phi : {0.2, 0.5, 0.8}) { + CHECK(curveFromLevelAt(phi, 0.0) == kCurveMax); + CHECK(curveFromLevelAt(phi, 1.0) == kCurveMin); + CHECK(curveFromLevelAt(phi, std::nan("")) == kCurveMax); + } +} + // --- The inner dial's travel --------------------------------------------------- // The knob drag delivers `start - dy/kKnobDragRangePixels`. param_slider owns that constant and @@ -184,6 +218,9 @@ int main() { testClampCurveHoldsTheDomain(); testMidLevelRoundTripsAgainstTheExponent(); testMidLevelInverseSaturates(); + testMidLevelIsThePhiHalfSpecialCase(); + testLevelAtRoundTripsAtArbitraryPhi(); + testLevelAtInverseSaturatesAtArbitraryPhi(); testKnobLawIsExactAtTheNeutralCentre(); testADialSweptThroughNeutralLandsOnTheIdentity(); testKnobLawRoundTripsOutsideTheDetent(); diff --git a/tests/test_envelope_edit.cpp b/tests/test_envelope_edit.cpp index 6effc07..5ff0c1a 100644 --- a/tests/test_envelope_edit.cpp +++ b/tests/test_envelope_edit.cpp @@ -373,6 +373,40 @@ static void testKnotOnANearLevelSegmentIsANoOp() { CHECK(out.decayCurve == 2.5); } +// The knot drag must read the SAME phi the draw used even off the segment midpoint (an odd +// pixel span), not the fixed phi = 0.5 wideArea()'s AttackCurve span happens to land on above. +// Checked two ways: a zero-delta grab reproduces the stored exponent, and a real one-pixel drag +// moves the knot's own drawn y by the same one pixel every other node axis tracks 1:1. +static void testKnotDragTracksTheDrawOnAnOddPixelSpan() { + bool found = false; + for (int width = 24; width <= 260 && !found; ++width) { + const Rect a = Rect::ltrb(0, 0, width, 100); + StageEnvelope e = ahdsrEnv(); + e.attackCurve = 3.0; + EnvVertex origin, attackEnd, knot; + const std::vector poly = buildEnvelopePolyline(e, overlayOf(a), kTotal); + if (!findNode(poly, EnvNode::Origin, origin)) continue; + if (!findNode(poly, EnvNode::AttackEnd, attackEnd)) continue; + if (!findNode(poly, EnvNode::AttackCurve, knot)) continue; + const int span = attackEnd.x - origin.x; + if (span <= 0 || span % 2 == 0) continue; + found = true; + + const StageEnvelope same = + resolveNodeDrag(e, EnvNode::AttackCurve, overlayOf(a), kTotal, bounds(), 0, 0); + CHECK(std::fabs(same.attackCurve - e.attackCurve) < 1e-9); + + const StageEnvelope dragged = + resolveNodeDrag(e, EnvNode::AttackCurve, overlayOf(a), kTotal, bounds(), 0, 1); + EnvVertex knotAfter; + CHECK(findNode(buildEnvelopePolyline(dragged, overlayOf(a), kTotal), EnvNode::AttackCurve, + knotAfter)); + CHECK(knotAfter.x == knot.x); // a curve drag never moves the knot's x + CHECK(std::abs(knotAfter.y - (knot.y + 1)) <= 1); + } + CHECK(found); // the sweep must actually land on an odd span +} + // --- the interaction law on the overlay ---------------------------------------- // Ctrl scales the PIXEL delta, so it composes with every axis — the tapered schematic, the 1:1 @@ -476,6 +510,7 @@ int main() { testKnotAndModelCannotDiverge(); testKnotOnALevelSegmentIsANoOp(); testKnotOnANearLevelSegmentIsANoOp(); + testKnotDragTracksTheDrawOnAnOddPixelSpan(); testDegenerateInputsAreNoOps(); diff --git a/tests/test_envelope_overlay.cpp b/tests/test_envelope_overlay.cpp index 6c527f2..269b440 100644 --- a/tests/test_envelope_overlay.cpp +++ b/tests/test_envelope_overlay.cpp @@ -10,7 +10,9 @@ // per-segment separation at the tier-0 defaults, overrun compression, every vertex in-bounds); // splitAhdSeconds (A+H+D never exceeds the span, hold at 0% and 100%); the AHD polyline (1:1 with // the time axis, origin offset); curve knots (present only on sloped non-zero segments, height -// following the exponent); the degenerate flat baseline. +// following the exponent, and — swept across ODD and EVEN pixel spans, not one fixture's width — +// sitting on the curve its own vertices imply rather than always the segment's exact midpoint); +// the degenerate flat baseline. #include "../src/core/instrument/ui/envelope_overlay.h" @@ -422,6 +424,95 @@ static void testKnotHeightTracksTheExponent() { CHECK(steep.y >= a.y && steep.y <= a.bottom() - 1); } +// --- the knot sits ON its own curve (the reported defect, stated as the gate) ------------- + +// The general (non-truncated-phi) reading of a knot's level, computed from the vertices +// `buildEnvelopePolyline` actually returned — x0/x1/knotX are all int pixels a caller can read +// off the polyline, so this is a check ON the output, not a restatement of knotVtx's own +// formula. x0 == x1 has no interior (no knot is ever built there). +static double expectedKnotLevel(int x0, int x1, int knotX, double startLevel, double endLevel, + double exponent) { + const double phi = (x1 != x0) + ? static_cast(knotX - x0) / static_cast(x1 - x0) + : 0.5; + return startLevel + (endLevel - startLevel) * reasampler::util::curveMap(phi, exponent); +} + +// The reported defect, stated as the gate: at every exponent the knot's centre lies on the +// trace, within 1 px. Swept over a range of canvas widths (down to a few pixels of stage span) +// so the check actually exercises ODD pixel spans, where the segment's true midpoint falls +// between two pixels — testKnotHeightTracksTheExponent above sits at a width whose span happens +// to be even, which is exactly the kind of fixture that missed this defect. +static void testKnotSitsOnItsOwnCurveAcrossOddAndEvenSpans() { + bool sawOdd = false, sawEven = false; + int worstAhdsr = 0, worstAhd = 0; + for (int width = 24; width <= 260; width += 3) { + const Rect a = Rect::ltrb(0, 0, width, 100); + for (double exp : {util::kCurveMin, 0.3, 1.0, 3.0, util::kCurveMax}) { + StageEnvelope e = ahdsr(0.4, 0.0, 0.0, 1.0, 0.0); + e.attackCurve = exp; + EnvVertex origin, attackEnd, knot; + const std::vector poly = buildEnvelopePolyline(e, overlayOf(a), 4.0); + if (findNode(poly, EnvNode::Origin, origin) && + findNode(poly, EnvNode::AttackEnd, attackEnd) && + findNode(poly, EnvNode::AttackCurve, knot)) { + const int span = attackEnd.x - origin.x; + if (span > 0) { + if (span % 2 == 0) sawEven = true; else sawOdd = true; + const double expected = + expectedKnotLevel(origin.x, attackEnd.x, knot.x, 0.0, 1.0, exp); + const int expectedY = levelToY(a, expected); + worstAhdsr = (std::max)(worstAhdsr, std::abs(knot.y - expectedY)); + CHECK(std::abs(knot.y - expectedY) <= 1); + } + } + + StageEnvelope f = ahd(0.4, 0.6, 0.5, 0.0, 3.0); + f.attackCurve = exp; + EnvVertex originAhd, attackEndAhd, knotAhd; + const std::vector polyAhd = buildEnvelopePolyline(f, overlayOf(a), 4.0); + if (findNode(polyAhd, EnvNode::Origin, originAhd) && + findNode(polyAhd, EnvNode::AttackEnd, attackEndAhd) && + findNode(polyAhd, EnvNode::AttackCurve, knotAhd)) { + const int span = attackEndAhd.x - originAhd.x; + if (span > 0) { + if (span % 2 == 0) sawEven = true; else sawOdd = true; + const double expected = expectedKnotLevel(originAhd.x, attackEndAhd.x, + knotAhd.x, 0.0, 1.0, exp); + const int expectedY = levelToY(a, expected); + worstAhd = (std::max)(worstAhd, std::abs(knotAhd.y - expectedY)); + CHECK(std::abs(knotAhd.y - expectedY) <= 1); + } + } + } + } + CHECK(sawOdd); // the sweep actually exercised an odd-pixel span... + CHECK(sawEven); // ...and an even one, so this isn't resting on one fixture's luck. + std::printf(" worst knot/curve separation: AHDSR %d px, AHD %d px\n", worstAhdsr, worstAhd); +} + +// Exponent 1.0 is still a plain straight line even off the segment's exact midpoint — checked +// at a deliberately ODD span so the linear case isn't only proven at the symmetric one. +static void testNeutralExponentIsAStraightLineOffCentre() { + bool found = false; + for (int width = 24; width <= 200 && !found; ++width) { + const Rect a = Rect::ltrb(0, 0, width, 100); + StageEnvelope e = ahdsr(0.4, 0.0, 0.0, 1.0, 0.0); + e.attackCurve = util::kCurveNeutral; + EnvVertex origin, attackEnd, knot; + const std::vector poly = buildEnvelopePolyline(e, overlayOf(a), 4.0); + if (!findNode(poly, EnvNode::Origin, origin)) continue; + if (!findNode(poly, EnvNode::AttackEnd, attackEnd)) continue; + if (!findNode(poly, EnvNode::AttackCurve, knot)) continue; + const int span = attackEnd.x - origin.x; + if (span <= 0 || span % 2 == 0) continue; + found = true; + const double phi = static_cast(knot.x - origin.x) / static_cast(span); + CHECK(std::fabs(knot.level - phi) < 1e-12); // linear: level == phi, exactly + } + CHECK(found); // the sweep must actually land on an odd span +} + // --- degenerate --------------------------------------------------------------- static void testDegenerateSurfaceYieldsFlatBaseline() { @@ -458,6 +549,8 @@ int main() { testKnotsRideOnlySlopedNonZeroSegments(); testKnotHeightTracksTheExponent(); + testKnotSitsOnItsOwnCurveAcrossOddAndEvenSpans(); + testNeutralExponentIsAStraightLineOffCentre(); testDegenerateSurfaceYieldsFlatBaseline();