fix(instrument): bound the waveform zero-crossing snap to a pixel radius so single-cycle loop marks stop teleporting

Ctrl during a marker drag defeats the snap outright. nearestZeroCrossing keeps its unbounded contract; snapToZeroCrossing is the decision a drag applies.
2026-08-03 15:15:25 -04:00
parent 1159d364c2
commit b7b7e88195
5 changed files with 205 additions and 24 deletions
@@ -13,7 +13,10 @@
 // resolveDragFrame (drag lands on the frameToX/xToFrame column under the cursor, clamp to
 // [0,frameCount], zero-delta/zero-width no-ops); nearestZeroCrossing (nearest sign-change,
 // sample-on-zero, equidistant-tie-to-lower, no-crossing keeps target, target clamp, degenerate
-// buffers); the four marks (per-mark cap
+// buffers); snapToZeroCrossing + zeroCrossingSnapFrames (the radius: a single-cycle mark stays
+// where it was dropped, dense material answers exactly what the unbounded search did, the
+// boundary either side, the tie rule inside it, a sub-frame-per-pixel radius, clamps and
+// degenerate buffers); the four marks (per-mark cap
 // resolve, the reverse cap order that keeps a coincident pair separable, label sides/nudging,
 // the suppression rule and its promoted-first placement, the crossfade wedge ramp);
 // waveformSurface (two stacked
@@ -24,6 +27,7 @@
 #include "../src/core/instrument/ui/sample_bands.h"  // kWaveformMinHeight, kLaneGap
 #include "../src/core/ui/component_geometry.h"       // waveformColumnCount (the draw chain's own)

+#include <cmath>
 #include <cstddef>
 #include <cstdio>
 #include <vector>
@@ -376,6 +380,116 @@ static void testZeroCrossingDegenerate() {
    CHECK(nearestZeroCrossing(one.data(), 1, 0) == 0);  // <2 frames -> clamped target
 }

+// --- snapToZeroCrossing: the radius -------------------------------------------
+
+// The radius in frames is the frame span kZeroCrossingSnapPx pixels cover, so it tracks the
+// capture's length against a fixed band — read off xToFrame, never a second ratio.
+static void testSnapRadiusIsThePixelBandsOwnFrameSpan() {
+    const Rect a = wideArea();  // width 1000
+    CHECK(zeroCrossingSnapFrames(overlayOf(a), 100000) == kZeroCrossingSnapPx * 100);
+    CHECK(zeroCrossingSnapFrames(overlayOf(a), 1000) == kZeroCrossingSnapPx);  // 1 frame per px
+    // Below one frame per pixel the radius is 0: the user is placing individual frames.
+    CHECK(zeroCrossingSnapFrames(overlayOf(a), 100) == 0);
+    CHECK(zeroCrossingSnapFrames(overlayOf(a), 0) == 0);
+    CHECK(zeroCrossingSnapFrames(overlayOf(Rect::ltrb(0, 0, 0, 60)), 1000) == 0);  // zero width
+}
+
+// One cycle of a 60 Hz sine at 48 kHz — 800 frames, and exactly ONE interior sign change, at the
+// midpoint (frame 0 is on zero, which is not a crossing, and the up-crossing is the wrap). That
+// single crossing IS the reported defect: an unbounded search resolves every drop in the buffer
+// to it, so the loop can only ever be half a cycle.
+static std::vector<AudioSample> singleCycleSine() {
+    constexpr std::int64_t n = 800;
+    constexpr double kTwoPi = 6.283185307179586;
+    std::vector<AudioSample> pcm(static_cast<std::size_t>(n));
+    for (std::int64_t i = 0; i < n; ++i) {
+        const double phase = kTwoPi * static_cast<double>(i) / static_cast<double>(n);
+        pcm[static_cast<std::size_t>(i)] = static_cast<AudioSample>(std::sin(phase));
+    }
+    return pcm;
+}
+
+static void testSnapLeavesASingleCycleMarkWhereItWasDropped() {
+    const std::vector<AudioSample> pcm = singleCycleSine();
+    const std::int64_t n = static_cast<std::int64_t>(pcm.size());
+    // The fixture really does teleport under the unbounded search — both quadrant peaks land on
+    // the one midpoint crossing, hundreds of frames away.
+    CHECK(nearestZeroCrossing(pcm.data(), n, 200) == 401);
+    CHECK(nearestZeroCrossing(pcm.data(), n, 600) == 401);
+
+    const Rect a = Rect::ltrb(20, 10, 820, 90);  // 800 px for 800 frames -> 1 frame per px
+    const std::int64_t r = zeroCrossingSnapFrames(overlayOf(a), n);
+    CHECK(r == kZeroCrossingSnapPx);
+    // ...and with the radius the marks stay put, which is what makes the loop draggable at all.
+    CHECK(snapToZeroCrossing(pcm.data(), n, 200, r) == 200);
+    CHECK(snapToZeroCrossing(pcm.data(), n, 600, r) == 600);
+    // The snap is not dead here — aimed at the crossing it still takes it.
+    CHECK(snapToZeroCrossing(pcm.data(), n, 401 - r, r) == 401);
+    CHECK(snapToZeroCrossing(pcm.data(), n, 401 - r - 1, r) == 401 - r - 1);
+}
+
+// Dense material: 48000 frames flipping sign every 24 (a 1 kHz square), drawn 1000 px wide, so
+// the radius is 240 frames and every crossing is within 12. The snap must therefore answer
+// exactly what the unbounded search always did, at every target — long material does not change.
+static void testSnapIsUnchangedOnDenseMaterial() {
+    constexpr std::int64_t n = 48000, kHalfPeriod = 24;
+    std::vector<AudioSample> pcm(static_cast<std::size_t>(n));
+    for (std::int64_t i = 0; i < n; ++i) {
+        pcm[static_cast<std::size_t>(i)] = ((i / kHalfPeriod) % 2 == 0) ? 1.0f : -1.0f;
+    }
+    const Rect a = wideArea();  // width 1000 -> 48 frames per px
+    const std::int64_t r = zeroCrossingSnapFrames(overlayOf(a), n);
+    CHECK(r == kZeroCrossingSnapPx * 48);
+    // A mark dropped one frame off a crossing still snaps onto it.
+    CHECK(snapToZeroCrossing(pcm.data(), n, kHalfPeriod + 1, r) == kHalfPeriod);
+    CHECK(snapToZeroCrossing(pcm.data(), n, kHalfPeriod - 1, r) == kHalfPeriod);
+    for (std::int64_t t = 0; t < n; t += 7) {
+        CHECK(snapToZeroCrossing(pcm.data(), n, t, r) == nearestZeroCrossing(pcm.data(), n, t));
+    }
+}
+
+// The boundary, both sides: exactly at the radius is inside it, one past it is not. A lone 0.0
+// sample is its own isolated crossing (the sample-on-zero rule), so each buffer has exactly one.
+static void testSnapTakesACrossingAtTheRadiusAndRefusesOnePastIt() {
+    const std::int64_t n = 400, t = 200, r = 10;
+    for (const std::int64_t at : {t + r, t + r + 1, t - r, t - r - 1}) {
+        std::vector<AudioSample> pcm(static_cast<std::size_t>(n), 1.0f);
+        pcm[static_cast<std::size_t>(at)] = 0.0f;
+        const std::int64_t want = (at == t + r || at == t - r) ? at : t;
+        CHECK(snapToZeroCrossing(pcm.data(), n, t, r) == want);
+    }
+}
+
+// The tie rule is the radius's too: inside it, the fan-out order still decides, and it still
+// resolves to the LOWER frame at every distance.
+static void testSnapKeepsTheTieRuleInsideTheRadius() {
+    const std::int64_t n = 200, t = 100, r = 40;
+    for (std::int64_t d = 1; d <= r; ++d) {
+        std::vector<AudioSample> pcm(static_cast<std::size_t>(n), 1.0f);
+        pcm[static_cast<std::size_t>(t - d)] = 0.0f;
+        pcm[static_cast<std::size_t>(t + d)] = 0.0f;
+        CHECK(snapToZeroCrossing(pcm.data(), n, t, r) == t - d);
+    }
+}
+
+static void testSnapAtZeroRadiusMovesNothingButAnExactHit() {
+    std::vector<AudioSample> pcm = {1, 1, -1, -1};  // crossing at 2
+    CHECK(snapToZeroCrossing(pcm.data(), 4, 2, 0) == 2);
+    CHECK(snapToZeroCrossing(pcm.data(), 4, 1, 0) == 1);
+    CHECK(snapToZeroCrossing(pcm.data(), 4, 3, 0) == 3);
+}
+
+static void testSnapClampsAndTakesDegenerateInputs() {
+    std::vector<AudioSample> pcm = {1, -1, 1, -1};  // crossings at 1,2,3
+    CHECK(snapToZeroCrossing(pcm.data(), 4, 999, 100) == 3);   // clamped, then found
+    CHECK(snapToZeroCrossing(pcm.data(), 4, -999, 100) == 1);
+    CHECK(snapToZeroCrossing(pcm.data(), 4, 0, 100) == 1);     // frame 0 is never a crossing
+    CHECK(snapToZeroCrossing(pcm.data(), 4, 0, -1) == 0);      // negative radius -> no snap
+    CHECK(snapToZeroCrossing(nullptr, 0, 5, 100) == 0);
+    std::vector<AudioSample> one = {1};
+    CHECK(snapToZeroCrossing(one.data(), 1, 0, 100) == 0);     // <2 frames -> clamped target
+}
+
 // --- waveformSurface: the lane split + the overlay contract --------------------

 // A realistic waveform band: full-width, taller than the two-lane floor.
@@ -829,6 +943,13 @@ int main() {
    testZeroCrossingNoneKeepsTarget();
    testZeroCrossingClampsTarget();
    testZeroCrossingDegenerate();
+    testSnapRadiusIsThePixelBandsOwnFrameSpan();
+    testSnapLeavesASingleCycleMarkWhereItWasDropped();
+    testSnapIsUnchangedOnDenseMaterial();
+    testSnapTakesACrossingAtTheRadiusAndRefusesOnePastIt();
+    testSnapKeepsTheTieRuleInsideTheRadius();
+    testSnapAtZeroRadiusMovesNothingButAnExactHit();
+    testSnapClampsAndTakesDegenerateInputs();

    testSurfaceStereoStacksTwoLanes();
    testSurfaceMonoIsOneLane();