diff --git a/docs/VERIFICATION.md b/docs/VERIFICATION.md index 0330528..4ad21bb 100644 --- a/docs/VERIFICATION.md +++ b/docs/VERIFICATION.md @@ -1,6 +1,6 @@ # DAW verification — post-1.0 work on `dev` -Checks for Θ, Ξ, Ψ, Ε, Ρ, and Γ work that no unit test can close. Build **Release**, install into +Checks for Θ, Ξ, Ψ, Ε, Ρ, Γ, and Ω work that no unit test can close. Build **Release**, install into `UserPlugins/`, restart REAPER. Panel tail toggle = **None**, project rate 48000, unless a check says otherwise. ## Precision invariants @@ -167,6 +167,20 @@ Checks for Θ, Ξ, Ψ, Ε, Ρ, and Γ work that no unit test can close. Build ** - [ ] Open a project saved before the zone retirement — the instance reopens on its first zone and sounds the same (`docs/COMPLETED.md` §"Θ-W1-T1") - [ ] Such an instance with implicit channel mode + a stereo capture reopens **Stereo** — confirm that is acceptable by ear (`docs/COMPLETED.md` §"Θ-W1-T1") +## Waveform marker zero-crossing snap (Ω-W2-T5) + +- [ ] `[verify — DAW]` Drag a Gate loop mark on a single-cycle capture — the felt radius the mark + may be pulled to reach a crossing genuinely reads as imperceptible, i.e. a displacement inside + the pointer's own grab neighbourhood is not one the eye catches (`src/core/instrument/ui/waveform_view.h`'s + `kZeroCrossingSnapPx` comment) +- [ ] `[verify — DAW]` Hold Ctrl, then drag a waveform marker (start/loop start/loop end) off a + zero crossing — confirm it lands exactly where dropped, with no snap, in a live editor window + (`editor_input_waveform.cpp`'s `dragWaveform`, `GetKeyState`-driven, no CI test target) +- [ ] `[verify — DAW]` Press and hold Ctrl BEFORE mouse-down on a mark that coincides with a + contour node or another mark's cap — confirm the mark is grabbed (not the node dragged, not a + hard/smooth toggle fired), matching the fix in `resolveWaveformClaim` + (`src/core/instrument/ui/spline_edit.cpp`) + ## Look and feel - [ ] Sign off by eye in a live editor window: knob arcs, needles, envelope splines, waveform outline (`docs/COMPLETED.md` §"Θ-W7-T1", `docs/COMPLETED.md` §"Θ-W6-T1" — "Antialiasing pass") diff --git a/docs/product/instrument-control-surface.md b/docs/product/instrument-control-surface.md index ff25e97..e2d0649 100644 --- a/docs/product/instrument-control-surface.md +++ b/docs/product/instrument-control-surface.md @@ -829,6 +829,19 @@ model"* — a snap available on one and not the others would be a divergence. > "whole number" above the frame. Overloading Shift there would collide with a shipped > gesture. Recorded as a deliberate exclusion. +> **AMENDED (Ω-W2-T5, Daniel 2026-08-03) — Ctrl now has a marker meaning too, and it is not +> this table's Ctrl row.** Dragging a waveform marker snaps it to the nearest zero crossing +> within a small pixel radius (`core/instrument/ui/waveform_view`'s `snapToZeroCrossing`); +> holding Ctrl defeats that snap outright rather than scaling the drag by 0.05. This does not +> reopen the exclusion above or the Shift question it answers: the exclusion is about **Shift** +> colliding with the shipped zero-crossing gesture on a domain with no meaningful whole number +> to snap to, and that reasoning is untouched — Ctrl's marker meaning is a different modifier +> defeating a different gesture (an existing snap, not this section's taper), not a second +> "whole number" law for frames. The two Ctrl meanings (defeat-the-snap here, fine-drag in +> §4.1's own table) do not collide in practice because they land on disjoint control families — +> a marker is never also a unit-valued knob — but the marker's own Ctrl is a genuinely separate +> semantic and is recorded as such, not folded into the row above. + ### 4.2 The snap unit, by category | Unit category | Controls | Shift snaps to | diff --git a/src/core/instrument/CLAUDE.md b/src/core/instrument/CLAUDE.md index a0ef4e6..629b23d 100644 --- a/src/core/instrument/CLAUDE.md +++ b/src/core/instrument/CLAUDE.md @@ -340,7 +340,7 @@ anything for a trigger shape. - `sample_chrome` — the CHROME band's interior: the toolbar row (title + the whole right-anchored control run — bake Hold cell, bake, preview, velocity knob cell, loop enable, channel toggle, Browse) over the strip row, which the piano strip owns outright. The title takes what the run leaves; the strip takes its whole row, inset only by the shared band pad so it lines up with the waveform band beneath. Every run member's width is RESERVED unconditionally, the Hold cell included — the only conditionally-drawn one, and the leftmost, so what its reservation buys is a title slot that does not re-measure when a loop is dialled in or out (`sample_chrome.h` records the cost). Also `previewGlyph`, the preview button's play triangle — three vertices for one filled-triangle draw, so the button's label needs no font metric and no image asset. - `bake_hold` — the Hold knob's value domain and nothing else: the knob's normalized [0,1] mapped onto the note-length ladder and back, ordered by LENGTH rather than by the ladder's presentation order. Split from `sample_chrome` on the same axis `deck_values` was split from `knob_deck` — that says where the cell is, this says what its position means. - `keyboard_strip` — piano-keyboard strip: true white/black key geometry (whites tiled at one width, blacks overlaid at one width and height, straddling their boundary), hit-test resolving black-over-white by zone, root-marker rect, the absolute-position drag resolver, and MIDI note naming under the C4 convention. **Same-class keys are one integer width by construction; the residue of an indivisible band width (`w % 75`, up to 74 px) lands in symmetric end margins, never in a key** — uniform widths and gap-free edge-to-edge tiling cannot both hold, and uniformity wins. -- `waveform_view` — the WAVEFORM band's interior: `resolveLaneSplit` is THE lane-split decision (two lanes only when the mode is stereo AND the source has a second channel — a mono source under stereo mode is dual-mono and draws one lane), free of any pixel geometry so the meter's bar count can ask the same question without a band rect; `waveformSurface` folds it and then measures it against the band, which is why its `laneCount` can still report 1 for a Stereo split on a band too thin to divide. It also yields **the** overlay area, and `laneEnvelope` splits one multi-channel envelope pass per lane. Also maps frame span linearly across a rect; generic named draggable markers with drag-delta resolver, clamp, and zero-crossing snap, plus `markerHandleRect` — a top-strip grab tab distinct from a marker's full-height column, so two markers that share a frame stay independently grabbable (the column goes to the first in draw order; the tab, asked first, resolves the other). +- `waveform_view` — the WAVEFORM band's interior: `resolveLaneSplit` is THE lane-split decision (two lanes only when the mode is stereo AND the source has a second channel — a mono source under stereo mode is dual-mono and draws one lane), free of any pixel geometry so the meter's bar count can ask the same question without a band rect; `waveformSurface` folds it and then measures it against the band, which is why its `laneCount` can still report 1 for a Stereo split on a band too thin to divide. It also yields **the** overlay area, and `laneEnvelope` splits one multi-channel envelope pass per lane. Also maps frame span linearly across a rect, plus generic named draggable markers with a drag-delta resolver, clamp, and the zero-crossing snap. The snap is RADIUS-BOUNDED (`snapToZeroCrossing`), the radius denominated in PIXELS and converted through `xToFrame`; `nearestZeroCrossing` beside it is the unbounded primitive and is NOT what a drag applies. Ctrl defeats the snap outright at the shell (`editor_input_waveform.cpp`), the same "get out of the way, I want exactness" role it plays on the knobs' fine drag. `markerHandleRect` is a top-strip grab tab distinct from a marker's full-height column, so two markers that share a frame stay independently grabbable — the column goes to the first in draw order, the tab, asked first, resolves the other. - **Overlay contract (consumed by later waveform work).** `WaveformSurface::overlay` — equivalently the standalone `waveformOverlayArea(band)` — is the band's DRAWN COLUMN SPAN at the band's full height, in both modes: same inset and same width as `component_geometry`'s `waveformColumnCount`, so overlay pixel and waveform column are one pixel and every overlay lands on the column that draws the frame it names. Deriving that width instead of restating the inset is what keeps the two chains from drifting apart again. Everything riding the waveform (the amp-envelope trace and its node handles, the start/loop markers, the loop region) draws ONCE into it, spanning both stacked lanes; hit-testing resolves against the same area so a grab in the lower lane reaches them. Anything drawn or hit-tested per lane is a duplicate and a defect — structurally enforced: `overlay` is the distinct `OverlayArea` type (`editor_geometry`), not `Rect`, so every overlay-consuming API (`frameToX`/`markerAtPoint`/`resolveDragFrame`, `envelope_edit`'s `nodeAtPoint`/`resolveNodeDrag`, `envelope_overlay`'s `buildEnvelopePolyline`) rejects a lane rect at compile time rather than silently accepting one. - **The four marks.** One grammar — line + shaped cap + label — over START / LOOP / END / XFADE. Which of them are PRESENT is the mode's answer, and drawn iff grabbable: Trigger carries START alone (the loop pair and the fade are absent, not dim), Gate always carries the pair so the drag-to-set-loop gesture survives the enable being off, and the fade needs an active loop to have a seam. `markerHandleRect` IS the cap: every mark's is the same rect shape, only the glyph inside differs, which is what keeps the claim arbitration seeing one nominal cap area. `capAtPoint` resolves caps in the REVERSE of the column order, so any coincident PAIR stays separable (one answers its cap, the other its column) and the crossfade — the one mark with no column — can never be shadowed. `layoutMarkLabels` places the promoted (grabbed/hovered) mark first and suppresses any box that would overlap one already placed. `crossfadeWedgeHeight` is the ONE ramp both the audible region and the ingredient ghost draw, because they are the same fade weight over the two spans it mixes. - `loop_marks` — the loop enable's state machine, split from the geometry above on the axis the surface already has: that says where a mark is, this says what the loop IS. `SampleLoop::hasLoop` is the single authority and `resolveLoopMarks`/`applyLoopMarks` are its only two folds — the resolve re-parks on `defaultLoopBounds` only when the span is one `resolveLoop` would refuse (so a user's off keeps its positions and `parked` separates the two OFF states), and the write folds collapse-to-off in and ties the crossfade to the SPAN rather than to the enable. Links `loop_span` so the span the user is offered and the span the engine accepts stay one definition. diff --git a/src/core/instrument/ui/spline_edit.cpp b/src/core/instrument/ui/spline_edit.cpp index 6356814..1f00d3a 100644 --- a/src/core/instrument/ui/spline_edit.cpp +++ b/src/core/instrument/ui/spline_edit.cpp @@ -28,7 +28,12 @@ VelocityCurve::Box splineOverlayBox(const OverlayArea& area) { WaveformClaimant resolveWaveformClaim(const WaveformClaim& node, const WaveformClaim& tab, const WaveformClaim& marker, SplineGesture gesture) { - if (gesture == SplineGesture::kControlLeft && node.hit) return WaveformClaimant::kNode; + // The override is for a node with NOTHING else in the running: a cap or column also hit + // must fall through to the ordinary smallest-area arbitration below, exactly like a plain + // left-click, so Ctrl pressed before the button can never steal a mark grab out from under + // the cursor. + if (gesture == SplineGesture::kControlLeft && node.hit && !tab.hit && !marker.hit) + return WaveformClaimant::kNode; if (node.hit && (!tab.hit || node.area <= tab.area) && (!marker.hit || node.area <= marker.area)) return WaveformClaimant::kNode; if (tab.hit && (!marker.hit || tab.area <= marker.area)) return WaveformClaimant::kTab; diff --git a/src/core/instrument/ui/spline_edit.h b/src/core/instrument/ui/spline_edit.h index f70799a..deee507 100644 --- a/src/core/instrument/ui/spline_edit.h +++ b/src/core/instrument/ui/spline_edit.h @@ -75,8 +75,11 @@ enum class WaveformClaimant { kNone, kNode, kTab, kMarker }; // editor_input_waveform.cpp's mouseDownWaveform for the live constants). Every mark's cap is // one markerHandleRect, so the `tab` slot carries ONE nominal area however many marks feed it; // which mark it resolves to is waveform_view's capAtPoint, not this. A control-click has no -// cap/column meaning (they answer plain grabs only), so it resolves to the node whenever the -// node is in the running, regardless of area. +// cap/column meaning (only the node's hard/smooth toggle answers it), so it claims the node +// outright ONLY when no cap or column also hits — a cap or column in the running instead falls +// through to the ordinary smallest-area arbitration below, same as a plain click. Ctrl must +// never be able to steal a mark grab out from under the cursor merely because it was pressed +// before the button. WaveformClaimant resolveWaveformClaim(const WaveformClaim& node, const WaveformClaim& tab, const WaveformClaim& marker, SplineGesture gesture); diff --git a/src/core/instrument/ui/waveform_view.cpp b/src/core/instrument/ui/waveform_view.cpp index f026edf..b914481 100644 --- a/src/core/instrument/ui/waveform_view.cpp +++ b/src/core/instrument/ui/waveform_view.cpp @@ -178,17 +178,15 @@ std::int64_t resolveDragFrame(const OverlayArea& area, std::int64_t frameCount, return xToFrame(area, frameCount, grabX + dxPixels); } -std::int64_t nearestZeroCrossing(const AudioSample* pcm, std::int64_t frames, - std::int64_t target) { - if (pcm == nullptr || frames < 2) return clampFrame(target, frames > 0 ? frames - 1 : 0); - // Clamp target into a valid sample index [0, frames). - std::int64_t t = target; - if (t < 0) t = 0; - if (t > frames - 1) t = frames - 1; +namespace { +// The fan-out both entry points below share, so the two cannot grow two search orders: at each +// distance d probe t-d before t+d, which is what resolves an equidistant tie to the LOWER frame. +// Answers -1 when no crossing lies within `maxDistance` — the caller decides what that means. +std::int64_t crossingWithin(const AudioSample* pcm, std::int64_t frames, std::int64_t t, + std::int64_t maxDistance) { // A crossing lives at frame i (1 <= i < frames) when sign(pcm[i-1]) != sign(pcm[i]) OR - // pcm[i] == 0. isCrossing(i) tests exactly that. We fan out from t: at each distance d we - // probe t-d before t+d, so an equidistant tie resolves to the LOWER frame (deterministic). + // pcm[i] == 0. auto isCrossing = [&](std::int64_t i) -> bool { if (i < 1 || i >= frames) return false; const AudioSample a = pcm[i - 1]; @@ -198,7 +196,7 @@ std::int64_t nearestZeroCrossing(const AudioSample* pcm, std::int64_t frames, }; if (isCrossing(t)) return t; - for (std::int64_t d = 1; d < frames; ++d) { + for (std::int64_t d = 1; d <= maxDistance; ++d) { const std::int64_t lo = t - d; if (lo >= 1 && isCrossing(lo)) return lo; // lower side wins the tie const std::int64_t hi = t + d; @@ -206,7 +204,36 @@ std::int64_t nearestZeroCrossing(const AudioSample* pcm, std::int64_t frames, // Stop once both probes have run off both ends — no crossing anywhere. if (lo < 1 && hi >= frames) break; } - return t; // no sign change in the whole buffer -> keep the raw (clamped) target + return -1; +} + +// Clamp into a valid sample index [0, frames). +std::int64_t clampSampleIndex(std::int64_t target, std::int64_t frames) { + return clampFrame(target, frames > 0 ? frames - 1 : 0); +} + +} // namespace + +std::int64_t nearestZeroCrossing(const AudioSample* pcm, std::int64_t frames, + std::int64_t target) { + const std::int64_t t = clampSampleIndex(target, frames); + if (pcm == nullptr || frames < 2) return t; + // `frames` bounds every reachable distance from any t in [0, frames), so this is the whole + // buffer. + const std::int64_t c = crossingWithin(pcm, frames, t, frames); + return c >= 0 ? c : t; // no sign change anywhere -> keep the raw (clamped) target +} + +std::int64_t zeroCrossingSnapFrames(const OverlayArea& area, std::int64_t frameCount) { + return xToFrame(area, frameCount, area.rect.x + kZeroCrossingSnapPx); +} + +std::int64_t snapToZeroCrossing(const AudioSample* pcm, std::int64_t frames, std::int64_t target, + std::int64_t maxDistanceFrames) { + const std::int64_t t = clampSampleIndex(target, frames); + if (pcm == nullptr || frames < 2 || maxDistanceFrames < 0) return t; + const std::int64_t c = crossingWithin(pcm, frames, t, maxDistanceFrames); + return c >= 0 ? c : t; // nothing that near -> the mark stays where it was dropped } } // namespace reasampler::instrument::ui diff --git a/src/core/instrument/ui/waveform_view.h b/src/core/instrument/ui/waveform_view.h index 5bb84fe..c2f1d7c 100644 --- a/src/core/instrument/ui/waveform_view.h +++ b/src/core/instrument/ui/waveform_view.h @@ -185,12 +185,48 @@ int markerAtPoint(const OverlayArea& area, std::int64_t frameCount, const std::i std::int64_t resolveDragFrame(const OverlayArea& area, std::int64_t frameCount, std::int64_t startFrame, int dxPixels); -// Nearest zero-crossing frame to `target` in the mono PCM, for loop/start snap. A crossing is a -// frame i (1 <= i < frames) where pcm[i-1] and pcm[i] differ in sign (pcm[i] == 0 snaps to i). -// Search fans out symmetrically from the clamped target; an equidistant tie resolves to the -// lower frame. No sign change anywhere (or fewer than 2 frames) returns the clamped target -// unchanged. +// --- The zero-crossing snap ---------------------------------------------------------------- + +// Nearest zero-crossing frame to `target` in the mono PCM. A crossing is a frame i +// (1 <= i < frames) where pcm[i-1] and pcm[i] differ in sign (pcm[i] == 0 snaps to i). Search +// fans out symmetrically from the clamped target; an equidistant tie resolves to the lower +// frame. No sign change anywhere (or fewer than 2 frames) returns the clamped target unchanged. +// +// This answers WHERE the nearest crossing is, over the whole buffer. It is not what a drag +// should apply — see snapToZeroCrossing. No production caller: the drag applies the bounded +// entry. Keep this one anyway — it is the differential oracle the radius tests check the bounded +// entry against (dense material must equal it exactly; sparse and single-cycle material must +// not), and re-adopting it as a drag snap is the exact defect snapToZeroCrossing's own header +// warns against. std::int64_t nearestZeroCrossing(const AudioSample* pcm, std::int64_t frames, std::int64_t target); +// How far, in PIXELS, a mark may be pulled to reach a crossing. Aliases the mark's own grab band +// on purpose: the snap may move a mark only as far as the pointer could have been from it and +// still have grabbed it. `[verify — DAW]` a displacement inside the neighbourhood the user was +// already pointing at is one they cannot see — asserted from the geometry, not observed by eye +// in a live editor window. +// +// PIXELS rather than frames because the drag itself resolves in pixels — resolveDragFrame answers +// the column's own first frame, so a frame radius under one column's span would be unreachable on +// a long capture, and a fixed frame radius means a different VISIBLE distance at every sample +// length. +inline constexpr int kZeroCrossingSnapPx = kMarkerGrabWidth; + +// That radius in frames for this overlay, read off THE mapping rather than a second ratio. +// Answers 0 below one frame per pixel, which is right: at that zoom the user is placing +// individual frames, and only a mark dropped exactly on a crossing snaps. +std::int64_t zeroCrossingSnapFrames(const OverlayArea& area, std::int64_t frameCount); + +// THE snap a marker drag applies: the nearest crossing WITHIN maxDistanceFrames of the clamped +// target, or the target itself when none is that near. Same fan-out and same tie rule as +// nearestZeroCrossing. +// +// The radius is what makes this a snap rather than a constraint. Unbounded, the search always +// finds something: on single-cycle material, whose whole ~800-frame buffer holds one or two +// crossings, it pinned the loop marks hundreds of frames from where they were dropped — halving +// the loop and doubling the pitch. Do not remove the bound. +std::int64_t snapToZeroCrossing(const AudioSample* pcm, std::int64_t frames, std::int64_t target, + std::int64_t maxDistanceFrames); + } // namespace reasampler::instrument::ui diff --git a/src/shell/instrument/CLAUDE.md b/src/shell/instrument/CLAUDE.md index 68fb23c..d21cd82 100644 --- a/src/shell/instrument/CLAUDE.md +++ b/src/shell/instrument/CLAUDE.md @@ -311,3 +311,11 @@ against a performance budget — they are there because `VoiceEngine::applyLiveT "channel identity derives from `app_version` accessors, no scattered `#ifdef`s" — `INLINE_UID` needs literal brace-init tokens, so it can't route through a runtime string accessor. +- **Ctrl held during a waveform marker drag defeats the zero-crossing snap outright** + (`editor_input_waveform.cpp`'s `dragWaveform`, reading `dragModifiers().ctrl` — the same + "get out of the way, I want exactness" role Ctrl plays as the knobs' fine drag). It is + undiscoverable by design — no cursor cue, no label — so this note is its only home; the + radius itself (`core/instrument/ui/waveform_view`'s `snapToZeroCrossing`) is documented there, + not here. `resolveWaveformClaim` (`core/instrument/ui/spline_edit`) is the reason Ctrl held + BEFORE mouse-down still reaches the mark rather than stealing a coincident envelope-node grab + or hard/smooth toggle. diff --git a/src/shell/instrument/editor_input_waveform.cpp b/src/shell/instrument/editor_input_waveform.cpp index 7b2860b..27332cb 100644 --- a/src/shell/instrument/editor_input_waveform.cpp +++ b/src/shell/instrument/editor_input_waveform.cpp @@ -282,13 +282,16 @@ void ReaSamplerEditor::dragWaveform(const FaceLayout& fl, int x, int y) { dragStartMarkers_.crossfade}; std::int64_t newFrame = resolveDragFrame(overlay, frames, startVals[idx], dx); - // Snap to the nearest zero crossing in the decoded PCM. Pure over the cached mono - // frames — no host types, no file I/O. The crossfade handle is exempt: it sets a fade - // LENGTH, and the whole point of the fade is that its edges need no zero crossing. + // Snap to a zero crossing in the decoded PCM, but only one within kZeroCrossingSnapPx of + // where the mark was dropped. Pure over the cached mono frames — no host types, no file I/O. + // The crossfade handle is exempt: it sets a fade LENGTH, and the whole point of the fade is + // that its edges need no zero crossing. Ctrl defeats the snap outright, the same "get out of + // the way, I want exactness" role it plays as the knobs' fine drag. `[verify — DAW]` this + // `GetKeyState` read has no test target — nothing in CI observes the defeat actually landing. const std::vector& pcm = monoPcmFor(selectedId_); - if (!pcm.empty() && waveMarker_ != WaveMarker::kCrossfade) { - newFrame = nearestZeroCrossing(pcm.data(), static_cast(pcm.size()), - newFrame); + if (!pcm.empty() && waveMarker_ != WaveMarker::kCrossfade && !dragModifiers().ctrl) { + newFrame = snapToZeroCrossing(pcm.data(), frames, newFrame, + zeroCrossingSnapFrames(overlay, frames)); } // Build the edited marker set from the snapshot, moving only the grabbed marker, then diff --git a/tests/test_spline_edit.cpp b/tests/test_spline_edit.cpp index c9b6447..9c25529 100644 --- a/tests/test_spline_edit.cpp +++ b/tests/test_spline_edit.cpp @@ -280,15 +280,27 @@ static void testAMissedCandidateNeverWinsOnADegenerateZeroArea() { WaveformClaimant::kTab); } -// A control-click has no tab/marker meaning (only the node's hard/smooth toggle answers it), so -// it resolves to the node whenever the node is in the running, even where a plain left-click at -// the same pixel would hand the tab or marker the win on area alone. -static void testControlClickAlwaysTakesTheNodeOverASmallerTabOrMarker() { +// A control-click has no tab/marker meaning (only the node's hard/smooth toggle answers it), but +// that must not let Ctrl steal a mark grab out from under the cursor: when a cap or column is +// ALSO in the running, control-click defers to the ordinary smallest-area arbitration exactly +// like a plain left-click would, so pressing Ctrl before or after the button gives the same +// answer. Only with no cap/column in the running at all does control-click claim the node +// outright regardless of area. +static void testControlClickDefersToACoincidentCapOrColumnLikeAPlainClick() { const WaveformClaim node{true, kNodeArea}; const WaveformClaim smallerTab{true, 50}; // would beat the node on a plain left-click CHECK(resolveWaveformClaim(node, smallerTab, WaveformClaim{}, SplineGesture::kLeft) == WaveformClaimant::kTab); + // Ctrl pressed before the click must not out-rank the cap that a plain click already gives + // the win — the exact regression this pins. CHECK(resolveWaveformClaim(node, smallerTab, WaveformClaim{}, SplineGesture::kControlLeft) == + WaveformClaimant::kTab); + const WaveformClaim smallerMarker{true, 80}; // still smaller than the node, no tab present + CHECK(resolveWaveformClaim(node, WaveformClaim{}, smallerMarker, + SplineGesture::kControlLeft) == WaveformClaimant::kMarker); + // No cap or column at all: control-click still claims the node outright, regardless of area + // — there is nothing else for it to defer to. + CHECK(resolveWaveformClaim(node, WaveformClaim{}, WaveformClaim{}, SplineGesture::kControlLeft) == WaveformClaimant::kNode); // No node in the running: control-click has nothing to fall back to, so the tab still wins. CHECK(resolveWaveformClaim(WaveformClaim{}, smallerTab, WaveformClaim{}, @@ -311,7 +323,7 @@ int main() { testTabWinsAGenuineTabVersusMarkerTie(); testNoHitAnywhereFallsThroughToNone(); testAMissedCandidateNeverWinsOnADegenerateZeroArea(); - testControlClickAlwaysTakesTheNodeOverASmallerTabOrMarker(); + testControlClickDefersToACoincidentCapOrColumnLikeAPlainClick(); if (g_fail == 0) std::printf("spline_edit: all tests passed\n"); return g_fail == 0 ? 0 : 1; diff --git a/tests/test_waveform_view.cpp b/tests/test_waveform_view.cpp index df0f467..f1e3f03 100644 --- a/tests/test_waveform_view.cpp +++ b/tests/test_waveform_view.cpp @@ -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 #include #include #include @@ -376,6 +380,155 @@ 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 +} + +// A narrower-than-kZeroCrossingSnapPx overlay pushes `area.x + kZeroCrossingSnapPx` past +// area.right(), so xToFrame answers frameCount (its own past-the-edge clamp) and the "radius" +// becomes the WHOLE buffer — the original unbounded-snap defect, on a width the band-stack +// allocator's kEditorMinWidth floor never actually produces in the shipped editor. Documented as +// a fixture rather than left implicit, since this is a public pure API and the width sweep +// elsewhere in this file jumps straight from 0 to 1000. +static void testNarrowOverlayLosesTheBoundBelowTheSnapWidth() { + const Rect a = Rect::ltrb(0, 0, 3, 60); // narrower than kZeroCrossingSnapPx (5) + CHECK(zeroCrossingSnapFrames(overlayOf(a), 100000) == 100000); +} + +// Long SPARSE material: a 1 s / 48 kHz 40 Hz square wave, crossings ~600 frames apart, drawn +// 1000 px wide (r = 240). The dense sweep above holds every crossing well inside the radius by +// construction, so it can never observe the bound; this is the only fixture where the radius +// sits strictly BETWEEN two crossings on genuinely long material, so the bounded and unbounded +// searches can actually disagree. +static void testSnapBoundsALongSparseCaptureBetweenCrossings() { + constexpr std::int64_t n = 48000, kHalfPeriod = 600; // 40 Hz square wave at 48 kHz + std::vector pcm(static_cast(n)); + for (std::int64_t i = 0; i < n; ++i) { + pcm[static_cast(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); // 240 + CHECK(r < kHalfPeriod); // strictly between two crossings, not covering either + + // Equidistant midpoint between the crossings at 600 and 1200: the unbounded search still + // finds one (the tie rule picks the lower, 600), while the bounded snap correctly leaves the + // mark where it was dropped — this pair IS the observable difference on long material. + const std::int64_t crossing = kHalfPeriod, midpoint = crossing + kHalfPeriod / 2; + CHECK(nearestZeroCrossing(pcm.data(), n, midpoint) == crossing); + CHECK(snapToZeroCrossing(pcm.data(), n, midpoint, r) == midpoint); + + // Inside the radius the snap still reaches its crossing, same as ever. + CHECK(snapToZeroCrossing(pcm.data(), n, crossing + r, r) == crossing); + CHECK(snapToZeroCrossing(pcm.data(), n, crossing - r, r) == crossing); +} + +// 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 singleCycleSine() { + constexpr std::int64_t n = 800; + constexpr double kTwoPi = 6.283185307179586; + std::vector pcm(static_cast(n)); + for (std::int64_t i = 0; i < n; ++i) { + const double phase = kTwoPi * static_cast(i) / static_cast(n); + pcm[static_cast(i)] = static_cast(std::sin(phase)); + } + return pcm; +} + +static void testSnapLeavesASingleCycleMarkWhereItWasDropped() { + const std::vector pcm = singleCycleSine(); + const std::int64_t n = static_cast(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 pcm(static_cast(n)); + for (std::int64_t i = 0; i < n; ++i) { + pcm[static_cast(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 pcm(static_cast(n), 1.0f); + pcm[static_cast(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 pcm(static_cast(n), 1.0f); + pcm[static_cast(t - d)] = 0.0f; + pcm[static_cast(t + d)] = 0.0f; + CHECK(snapToZeroCrossing(pcm.data(), n, t, r) == t - d); + } +} + +static void testSnapAtZeroRadiusMovesNothingButAnExactHit() { + std::vector 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 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 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 +982,15 @@ int main() { testZeroCrossingNoneKeepsTarget(); testZeroCrossingClampsTarget(); testZeroCrossingDegenerate(); + testSnapRadiusIsThePixelBandsOwnFrameSpan(); + testNarrowOverlayLosesTheBoundBelowTheSnapWidth(); + testSnapLeavesASingleCycleMarkWhereItWasDropped(); + testSnapIsUnchangedOnDenseMaterial(); + testSnapBoundsALongSparseCaptureBetweenCrossings(); + testSnapTakesACrossingAtTheRadiusAndRefusesOnePastIt(); + testSnapKeepsTheTieRuleInsideTheRadius(); + testSnapAtZeroRadiusMovesNothingButAnExactHit(); + testSnapClampsAndTakesDegenerateInputs(); testSurfaceStereoStacksTwoLanes(); testSurfaceMonoIsOneLane();