diff --git a/src/core/instrument/engine/play_params.h b/src/core/instrument/engine/play_params.h index 0309223..0620566 100644 --- a/src/core/instrument/engine/play_params.h +++ b/src/core/instrument/engine/play_params.h @@ -195,12 +195,10 @@ bool splineActive(const Play& p) { (p.filter.enabled && p.filterSpline.mode == EnvMode::Spline); } -// The one enforcement of the rule above: forces `p` to Trigger the instant it goes -// splineActive. Every path that can leave `p` in a state splineActive() would newly return true -// for — resolving stored params toward the engine, or an editor control edit that can flip a -// mode/enable toggle — routes through this, so none of them can reopen the Gate+spline hole -// splineActive's own doc comment describes. Header-inline and allocation-free: play_params.h -// sits on the per-voice-per-sample include path. +// The one enforcement of splineActive's rule (see its doc above). Header-inline and +// allocation-free: play_params.h sits on the per-voice-per-sample include path. Both callers — +// resolvePlay (sample_map.cpp) and the editor's applyControl — route through here, so the two +// cannot drift apart. template void enforceGateUnavailableWhileDrawn(Play& p) { if (splineActive(p)) p.playMode = PlayMode::Trigger; diff --git a/src/core/instrument/ui/CMakeLists.txt b/src/core/instrument/ui/CMakeLists.txt index 5c0de91..cff632f 100644 --- a/src/core/instrument/ui/CMakeLists.txt +++ b/src/core/instrument/ui/CMakeLists.txt @@ -24,10 +24,8 @@ reasampler_pure_library(waveform_view SOURCES waveform_view.cpp LINK PUBLIC editor_geometry peaks PRIVATE sample_bands) # sample_bands is linked directly here because the test exercises the lane metrics that -# waveform_view does not re-export. velocity_curve is linked for the smallest-target-first -# arbitration tests, which pin the geometric facts editor_input_waveform.cpp's node/tab/marker -# resolution depends on (the shell itself has no test target). -reasampler_test(waveform_view LINK waveform_view sample_bands velocity_curve) +# waveform_view does not re-export. +reasampler_test(waveform_view LINK waveform_view sample_bands) reasampler_pure_library(browser_scroll SOURCES browser_scroll.cpp @@ -63,7 +61,12 @@ reasampler_test(deck_groups LINK deck_groups sample_bands) reasampler_pure_library(spline_edit SOURCES spline_edit.cpp LINK PUBLIC editor_geometry velocity_curve) -reasampler_test(spline_edit LINK spline_edit) +# waveform_view and sample_bands are linked for the test only: resolveWaveformClaim's +# smallest-target-first tests build the real node/tab/marker geometry +# editor_input_waveform.cpp's mouseDownWaveform composes (the shell that calls it has no test +# target of its own), which needs waveform_view's marker/tab primitives and sample_bands' +# kWaveformMinHeight floor. +reasampler_test(spline_edit LINK spline_edit waveform_view sample_bands) reasampler_pure_library(curve_popup SOURCES curve_popup.cpp LINK PUBLIC editor_geometry) # velocity_curve is linked for the test only: the sheet's geometry is domain-agnostic, and diff --git a/src/core/instrument/ui/spline_edit.cpp b/src/core/instrument/ui/spline_edit.cpp index d5c62af..6356814 100644 --- a/src/core/instrument/ui/spline_edit.cpp +++ b/src/core/instrument/ui/spline_edit.cpp @@ -26,4 +26,14 @@ VelocityCurve::Box splineOverlayBox(const OverlayArea& area) { return VelocityCurve::Box{area.rect.x, area.rect.y, area.rect.width, area.rect.height}; } +WaveformClaimant resolveWaveformClaim(const WaveformClaim& node, const WaveformClaim& tab, + const WaveformClaim& marker, SplineGesture gesture) { + if (gesture == SplineGesture::kControlLeft && node.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; + if (marker.hit) return WaveformClaimant::kMarker; + return WaveformClaimant::kNone; +} + } // namespace reasampler::instrument::ui diff --git a/src/core/instrument/ui/spline_edit.h b/src/core/instrument/ui/spline_edit.h index e139d04..246fa9b 100644 --- a/src/core/instrument/ui/spline_edit.h +++ b/src/core/instrument/ui/spline_edit.h @@ -1,11 +1,13 @@ // spline_edit.h — the point-editing grammar's CLICK resolution: add/grab/delete/toggle from a -// single (x, y). Both spline consumers — the velocity-curve popup and the spline EG overlay — -// route their mouse-down through it, so the two cannot drift into two click grammars. Two more -// gesture rules complete the grammar but live in the shell (see the note near the bottom of this -// file). Mirror of envelope_edit otherwise: decision logic only, no host types, no drawing. +// single (x, y), plus resolveWaveformClaim, the waveform overlay's cross-affordance arbitration +// (node vs. crossfade tab vs. marker). Both spline consumers — the velocity-curve popup and the +// spline EG overlay — route their mouse-down through the click grammar, so the two cannot drift +// apart. Decision logic only, no host types, no drawing; mirror of envelope_edit otherwise. #pragma once +#include + #include "core/instrument/engine/velocity_curve.h" #include "core/instrument/ui/editor_geometry.h" // Rect / OverlayArea @@ -51,4 +53,28 @@ VelocityCurve::Box splineOverlayBox(const OverlayArea& area); // strictly in-box, since splineOverlayBox has no inset — editor_input_waveform.cpp's // splineOverlayClick. +// One arbitration candidate: whether the affordance was hit under the cursor, and its own +// pick-target area (nominal, per its own module's constants — not the actual clipped pixel +// count; see resolveWaveformClaim). +struct WaveformClaim { + bool hit = false; + std::int64_t area = 0; +}; + +// Which affordance a waveform-overlay click claims. +enum class WaveformClaimant { kNone, kNode, kTab, kMarker }; + +// The overlay's cross-affordance arbitration: a contour node (or, mutually exclusively, a +// staged envelope's drag node — both feed the same `node` slot), the loop crossfade tab, and a +// marker's full-height column can all claim the same pixel. Hit gates a candidate out +// entirely; among the ones that hit, the SMALLEST nominal area wins — the marker column is the +// odd one out (its target is the whole overlay height), so it only wins where nothing narrower +// also claims the click. Ties go to whichever is checked first: node, then tab, then marker — +// no live geometry produces a tie except tab-vs-marker, which the tab correctly wins (see +// editor_input_waveform.cpp's mouseDownWaveform for the live constants). A control-click has no +// tab/marker meaning (they answer plain grabs only), so it resolves to the node whenever the +// node is in the running, regardless of area. +WaveformClaimant resolveWaveformClaim(const WaveformClaim& node, const WaveformClaim& tab, + const WaveformClaim& marker, SplineGesture gesture); + } // namespace reasampler::instrument::ui diff --git a/src/shell/instrument/editor_input_waveform.cpp b/src/shell/instrument/editor_input_waveform.cpp index bdaba5a..2de7461 100644 --- a/src/shell/instrument/editor_input_waveform.cpp +++ b/src/shell/instrument/editor_input_waveform.cpp @@ -39,88 +39,88 @@ bool ReaSamplerEditor::mouseDownWaveform(const FaceLayout& fl, int x, int y) { ? SplineGesture::kControlLeft : SplineGesture::kLeft; - // Envelope nodes first (they sit on top of the markers), then the wave markers. With no - // envelope overlay-active — or with its deck group's enable toggle off, which makes the - // same params' knobs inert — there are no grabbable nodes and the markers take every grab. + // The staged envelope's draggable node and the drawn contour's node are mutually exclusive + // (overlayEnvInert flips the staged one inert exactly when its envelope is in Spline mode), + // so at most one of the two hit-tests below is ever live for the same click — both feed the + // SAME arbitration slot below rather than either one getting its own check-order return. const double rate = liveSampleRate(); const bool nodesLive = overlayEnv_ != OverlayEnv::kNone && !overlayEnvInert(overlayEnv_, gates); + NodeHit envNodeHit; + StageEnvelope env; if (rate > 0.0 && nodesLive) { const std::int64_t startFrame = params_.startPoint.value_or(0); - const StageEnvelope env = packEnvelope(overlayEnv_, params_.play, frames, startFrame); + env = packEnvelope(overlayEnv_, params_.play, frames, startFrame); const double totalSeconds = static_cast(frames) / rate; - const NodeHit nh = nodeAtPoint(env, overlay, totalSeconds, x, y); - if (nh.hit) { - drag_ = DragKind::kEnvNode; - envNode_ = nh.node; - dragStartX_ = x; - dragStartY_ = y; - dragStartEnv_ = env; - dragSampleFrames_ = frames; - dragStartParams_ = params_; - return true; // node moves once the cursor drags - } + envNodeHit = nodeAtPoint(env, overlay, totalSeconds, x, y); } + const SetupMarkers m = pickedMarkers(frames); const std::int64_t markerFrames[3] = {m.start, m.loopStart, m.loopEnd}; - // Three affordances can claim the same pixel: a contour node (a small fixed pick box), the - // crossfade tab (a small clipped top-strip tab), and a marker's full-height grab column - // (waveform_view.h's tab-vs-column split already keeps the tab apart from ITS OWN column; - // this is the cross-affordance case on top of that). Resolving by any fixed check order - // shadows whichever one loses the tie — this seam regressed twice from exactly that fix. - // Instead measure each claimant's own target area and let the SMALLEST hit win: the marker - // column is the odd one out (its target is the whole band height), so it only wins where - // nothing narrower also claims the pixel. Never add here (kAdd is only tried once nothing - // else has claimed the click, below). - struct Candidate { - bool hit = false; - std::int64_t area = 0; - }; - - Candidate node; - if (splineLive) { + // Three affordances can claim the same pixel: a node (the staged envelope's or the drawn + // contour's — a small fixed pick box either way), the crossfade tab (a small clipped + // top-strip tab), and a marker's full-height grab column (waveform_view.h's tab-vs-column + // split already keeps the tab apart from ITS OWN column; this is the cross-affordance case + // on top of that). resolveWaveformClaim (spline_edit.h) is the ONE arbitration: it measures + // each claimant's own NOMINAL target area and lets the smallest hit win, since a fixed check + // order shadows whichever one loses the tie — this seam regressed twice from exactly that + // fix. Never add here (kAdd is only tried once nothing else has claimed the click, below). + WaveformClaim node; + if (envNodeHit.hit) { + constexpr std::int64_t side = 2 * kNodeGrabRadius + 1; + node = {true, side * side}; + } else if (splineLive) { const VelocityCurve::Box box = splineOverlayBox(overlay); // Also require strict in-box, matching splineOverlayClick's own narrowing (spline_edit.h's // grammar note) — otherwise this candidate could "win" the arbitration below for a click // splineOverlayClick would then refuse, silently swallowing it instead of falling through // to the tab/marker checks. if (contains(overlay.rect, x, y) && splineFor(overlayEnv_).pointAtPixel(box, x, y) >= 0) { - node.hit = true; constexpr std::int64_t side = 2 * kCurveNodeGrabRadius + 1; - node.area = side * side; + node = {true, side * side}; } } - Candidate tab; const Rect tabRect = m.hasLoop ? markerHandleRect(overlay, frames, m.loopStart - m.crossfade) : Rect{}; - if (m.hasLoop && contains(tabRect, x, y)) { - tab.hit = true; - tab.area = static_cast(tabRect.width) * tabRect.height; - } + const WaveformClaim tab = (m.hasLoop && contains(tabRect, x, y)) + ? WaveformClaim{true, static_cast(tabRect.width) * + tabRect.height} + : WaveformClaim{}; + // Nominal, not actual: markerAtPoint clips the column at the overlay edges (a marker at + // frame 0 has 6 usable columns, not 11) and the node's fixed side clips too at a pick-box + // corner. Both overestimate in the direction that already produces the intended winner, so + // the arbitration runs on NOMINAL area, not the measured hit-testable pixel count. const int markerHit = markerAtPoint(overlay, frames, markerFrames, 3, x, y); - Candidate marker; - if (markerHit >= 0) { - marker.hit = true; - marker.area = static_cast(2 * kMarkerGrabWidth + 1) * overlay.rect.height; - } + const WaveformClaim marker = + (markerHit >= 0) + ? WaveformClaim{true, static_cast(2 * kMarkerGrabWidth + 1) * + overlay.rect.height} + : WaveformClaim{}; - // Node checked before marker on an area tie so a degenerate (zero-height) overlay still - // prefers the node — unreachable in practice (sample_bands floors the band well above it), - // kept only so this arbitration has one well-defined answer for every input, not just the - // ones the current geometry constants happen to produce. - if (node.hit && (!tab.hit || node.area <= tab.area) && (!marker.hit || node.area <= marker.area)) { - return splineOverlayClick(overlay, x, y, gesture, /*addOnEmptySpace=*/false); - } - if (tab.hit && (!marker.hit || tab.area <= marker.area)) { - beginMarkerDrag(WaveMarker::kLoopXfade, m, frames, x); - return true; - } - if (marker.hit) { - beginMarkerDrag(static_cast(markerHit), m, frames, x); - return true; + switch (resolveWaveformClaim(node, tab, marker, gesture)) { + case WaveformClaimant::kNode: + if (envNodeHit.hit) { + drag_ = DragKind::kEnvNode; + envNode_ = envNodeHit.node; + dragStartX_ = x; + dragStartY_ = y; + dragStartEnv_ = env; + dragSampleFrames_ = frames; + dragStartParams_ = params_; + return true; // node moves once the cursor drags + } + return splineOverlayClick(overlay, x, y, gesture, /*addOnEmptySpace=*/false); + case WaveformClaimant::kTab: + beginMarkerDrag(WaveMarker::kLoopXfade, m, frames, x); + return true; + case WaveformClaimant::kMarker: + beginMarkerDrag(static_cast(markerHit), m, frames, x); + return true; + case WaveformClaimant::kNone: + break; } // Nothing else wanted the click: now the drawn contour may take the empty space. if (splineLive) return splineOverlayClick(overlay, x, y, gesture, /*addOnEmptySpace=*/true); diff --git a/src/shell/instrument/editor_session.cpp b/src/shell/instrument/editor_session.cpp index d93890c..6414cef 100644 --- a/src/shell/instrument/editor_session.cpp +++ b/src/shell/instrument/editor_session.cpp @@ -186,8 +186,12 @@ ReaSamplerEditor::SetupMarkers ReaSamplerEditor::pickedMarkers(std::int64_t fram // Seed from the bank's intrinsic loop (fact about the file), then let the parameter set's // override win (the instrument's performance choice). Read the loop intrinsic from the // live bank blob (the same path selectSample uses); when that is not readable (extension - // absent / not yet parsed) the instance-owned ref carries the same intrinsics. - if (processor_) { + // absent / not yet parsed) the instance-owned ref carries the same intrinsics. Skipped + // entirely once an override is already set — it would just be overwritten below, and the + // bridge read + JSON parse it costs is real (mouseDownWaveform's arbitration calls this on + // every waveform click, not just marker grabs, to know whether a tab or marker candidate + // hits at all). + if (processor_ && !params_.loopOverride) { std::optional sel; auto banksJson = processor_->bridge().readReasamplerExtState(reasampler::kProjExtBanksKey); diff --git a/tests/test_deck_groups.cpp b/tests/test_deck_groups.cpp index cf9f4a4..dbbb202 100644 --- a/tests/test_deck_groups.cpp +++ b/tests/test_deck_groups.cpp @@ -619,14 +619,14 @@ static bool sameLayout(const DeckLayout& a, const DeckLayout& b) { // A Spline excursion is fully reversible at the layout level: the mode forcing swaps the amp // and filter faces onto their wider cells and back, leaving no residue in the geometry. Driven -// through splineActive and the editor's own forcing rule, so the deck cannot agree with a -// forcing rule the shell does not use. +// through the shared enforceGateUnavailableWhileDrawn helper, so the deck cannot agree with a +// forcing rule the real callers do not use. static void testGateSplineGateRoundTripsToTheSameLayout() { PlayParams p; // Gate, all three envelopes staged const DeckLayout before = layoutDeck(sampleDeckGroups(p.playMode), kPad, 0, kAvailAtMinWidth); p.ampSpline.mode = EnvMode::Spline; - if (splineActive(p)) p.playMode = PlayMode::Trigger; // editor_controls' forcing, verbatim + enforceGateUnavailableWhileDrawn(p); // the shared helper both real callers route through CHECK(p.playMode == PlayMode::Trigger); const DeckLayout drawn = layoutDeck(sampleDeckGroups(p.playMode), kPad, 0, kAvailAtMinWidth); // The excursion is real: the amp face's cells are strictly wider than Gate's. diff --git a/tests/test_spline_edit.cpp b/tests/test_spline_edit.cpp index d9ba2e7..5346e1f 100644 --- a/tests/test_spline_edit.cpp +++ b/tests/test_spline_edit.cpp @@ -2,15 +2,22 @@ // framework. Asserts the ONE point-editing grammar both spline consumers route through: // left-click grabs a node and adds in empty space, right-click deletes, control-click toggles // hard/smooth, and a click outside the mapping box resolves to nothing unless it lands on a -// node's pick radius (so the popup's inset ring can grab but never add). +// node's pick radius (so the popup's inset ring can grab but never add). Also +// resolveWaveformClaim, the waveform overlay's node/tab/marker cross-affordance arbitration — +// waveform_view is a test-only link so those tests can build the real geometry +// editor_input_waveform.cpp's mouseDownWaveform composes. #include "../src/core/instrument/ui/spline_edit.h" +#include "../src/core/instrument/ui/waveform_view.h" +#include "../src/core/instrument/ui/sample_bands.h" // kWaveformMinHeight #include using namespace reasampler::instrument::ui; using namespace reasampler::instrument::engine; +static OverlayArea overlayOf(const Rect& r) { return OverlayArea{r}; } + static int g_fail = 0; #define CHECK(cond) do { if(!(cond)) { \ std::printf("FAIL line %d: %s\n", __LINE__, #cond); ++g_fail; } } while(0) @@ -97,6 +104,159 @@ static void testOverlayBoxIsTheWholeArea() { CHECK(box.height == 200); } +// --- Smallest-target-first: resolveWaveformClaim, the shell's own comparison chain ----- +// +// editor_input_waveform.cpp's mouseDownWaveform resolves a click among a contour node (a fixed +// pick box), the crossfade tab, and a marker's full-height column by calling +// resolveWaveformClaim with each candidate's own target area; the smallest hit wins. These +// tests build the real geometry over the pure primitives the shell composes, then feed it into +// resolveWaveformClaim itself, so a reverted node-first/marker-first/tab-first ordering fails +// them — pinning the mechanism, not just the input geometry it acts on. A realistic band height +// (kWaveformMinHeight, the product's own floor) is used throughout so these numbers are the +// worst case for the node, not a favourable one. +static VelocityCurve::Box boxOf(const Rect& r) { return VelocityCurve::Box{r.x, r.y, r.width, r.height}; } + +constexpr std::int64_t kNodeSide = 2 * kCurveNodeGrabRadius + 1; +constexpr std::int64_t kNodeArea = kNodeSide * kNodeSide; // 169, fixed + +// Case (a): a fresh Spline default (rampDown) puts its endpoint 0 at (box.left, box.top) — the +// exact pixel the start marker draws at frame 0. The node's fixed 169px pick box is far smaller +// than a kWaveformMinHeight-tall marker column, so the endpoint stays reachable. +static void testFreshRampDownEndpointBeatsTheStartMarkerAtFrameZero() { + const Rect a = Rect{20, 10, 1000, kWaveformMinHeight}; + const OverlayArea overlay = overlayOf(a); + const std::int64_t frames = 100000; + const VelocityCurve contour = VelocityCurve::rampDown(); + const VelocityCurve::Box box = boxOf(a); + + CHECK(contour.pointAtPixel(box, a.x, a.y) == 0); // endpoint 0 sits at (box.left, box.top) + const std::int64_t markers[1] = {0}; + CHECK(markerAtPoint(overlay, frames, markers, 1, a.x, a.y) == 0); // the coincidence + + const std::int64_t markerArea = + static_cast(2 * kMarkerGrabWidth + 1) * a.height; // 11 * band height + CHECK(kNodeArea < markerArea); // the node wins: the endpoint stays a genuine grab target + + const WaveformClaim node{true, kNodeArea}; + const WaveformClaim marker{true, markerArea}; + CHECK(resolveWaveformClaim(node, WaveformClaim{}, marker, SplineGesture::kLeft) == + WaveformClaimant::kNode); +} + +// Case (b): the crossfade tab at zero crossfade sits on loopStart's own pixel column; a node +// dragged to value ~0.98 lands a couple of rows below the box top — inside the tab's own +// top-strip band, where the review found the tab fully shadowed by a node-first pass. +static void testCrossfadeTabBeatsAContourNodeNearItsTopStrip() { + const Rect a = Rect{20, 10, 1000, kWaveformMinHeight}; + const OverlayArea overlay = overlayOf(a); + const std::int64_t frames = 100000; + const std::int64_t loopStart = 40000, crossfade = 0; // zero crossfade -> tab sits on loopStart + const int mx = frameToX(overlay, frames, loopStart - crossfade); + const Rect tabRect = markerHandleRect(overlay, frames, loopStart - crossfade); + CHECK(!tabRect.empty()); + + const VelocityCurve::Box box = boxOf(a); + const int ny = a.y + 3; // ~0.98 up a kWaveformMinHeight-tall box; inside the tab's top strip + VelocityCurve c = VelocityCurve::flat(); + const VelocityPoint p = c.pointFromPixel(box, mx, ny); + c.addPoint(p.velocity, p.value); + CHECK(c.pointAtPixel(box, mx, ny) >= 0); + CHECK(contains(tabRect, mx, ny)); // the coincidence: both claim the same pixel + + const std::int64_t tabArea = static_cast(tabRect.width) * tabRect.height; // <= 110 + CHECK(tabArea < kNodeArea); // the tab wins: it stays the only affordance at zero crossfade + + const WaveformClaim node{true, kNodeArea}; + const WaveformClaim tab{true, tabArea}; + CHECK(resolveWaveformClaim(node, tab, WaveformClaim{}, SplineGesture::kLeft) == + WaveformClaimant::kTab); + + // The residual the review names: the node keeps its OUTER columns, one pixel past the tab's + // clipped edge but still inside its own pick radius. + const int outerX = mx + kMarkerHandleHalfWidth + 1; + CHECK(!contains(tabRect, outerX, ny)); + CHECK(c.pointAtPixel(box, outerX, ny) >= 0); +} + +// Case (c): a contour node coincident with a loop marker. At kWaveformMinHeight (the product's +// own floor) the column is already an order of magnitude larger than the node's fixed pick box, +// so the node wins the shared pixel while the column stays reachable everywhere the node isn't. +static void testContourNodeBeatsALoopMarkerAtTheirSharedPixelButNotElsewhere() { + const Rect a = Rect{20, 10, 1000, kWaveformMinHeight}; + const OverlayArea overlay = overlayOf(a); + const std::int64_t frames = 100000; + const std::int64_t loopEnd = 70000; + const int mx = frameToX(overlay, frames, loopEnd); + const std::int64_t markers[1] = {loopEnd}; + + const VelocityCurve::Box box = boxOf(a); + const int ny = a.y + a.height / 2; // mid-height, well clear of any tab + VelocityCurve c = VelocityCurve::flat(); + const VelocityPoint p = c.pointFromPixel(box, mx, ny); + c.addPoint(p.velocity, p.value); + CHECK(c.pointAtPixel(box, mx, ny) >= 0); + CHECK(markerAtPoint(overlay, frames, markers, 1, mx, ny) == 0); // the coincidence + + const std::int64_t markerArea = + static_cast(2 * kMarkerGrabWidth + 1) * a.height; // 11 * band height + CHECK(kNodeArea < markerArea); // the node wins the shared pixel + + const WaveformClaim node{true, kNodeArea}; + const WaveformClaim marker{true, markerArea}; + CHECK(resolveWaveformClaim(node, WaveformClaim{}, marker, SplineGesture::kLeft) == + WaveformClaimant::kNode); + + // A few rows clear of the node (outside its 13px pick box, still on the marker's column) + // the marker alone claims the click. + const int farY = ny + kCurveNodeGrabRadius + 4; + CHECK(c.pointAtPixel(box, mx, farY) < 0); + CHECK(markerAtPoint(overlay, frames, markers, 1, mx, farY) == 0); + CHECK(resolveWaveformClaim(WaveformClaim{}, WaveformClaim{}, marker, SplineGesture::kLeft) == + WaveformClaimant::kMarker); +} + +// The only live tie: the crossfade tab (<=110) can equal the node (169) only off-geometry, but +// tab-vs-marker ties at overlay height 10 (kMarkerHandleHeight), where the tab's 11x10 strip +// (110) equals a marker column's 11 * 10 (110) — the tab wins, matching check order. +static void testTabWinsAGenuineTabVersusMarkerTie() { + CHECK(resolveWaveformClaim(WaveformClaim{}, WaveformClaim{true, 110}, WaveformClaim{true, 110}, + SplineGesture::kLeft) == WaveformClaimant::kTab); +} + +// No claimant hit at all falls through to kNone — the caller's cue to let the drawn contour take +// empty space (addOnEmptySpace) rather than starting any drag. +static void testNoHitAnywhereFallsThroughToNone() { + CHECK(resolveWaveformClaim(WaveformClaim{}, WaveformClaim{}, WaveformClaim{}, SplineGesture::kLeft) == + WaveformClaimant::kNone); +} + +// A candidate that reports hit == false must never win merely because its (unused, default) +// area of 0 looks "smallest" — hit gates a candidate out before its area is ever compared. Real +// call sites never produce hit == false with area != 0, but the arbitration still owes one +// well-defined answer to every input, not just the ones live geometry happens to produce. +static void testAMissedCandidateNeverWinsOnADegenerateZeroArea() { + const WaveformClaim missedNode{false, 0}; + const WaveformClaim tab{true, 50}; + const WaveformClaim marker{true, 100}; + CHECK(resolveWaveformClaim(missedNode, tab, marker, SplineGesture::kLeft) == + 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() { + 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); + CHECK(resolveWaveformClaim(node, smallerTab, 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{}, + SplineGesture::kControlLeft) == WaveformClaimant::kTab); +} + int main() { testLeftClickOnANodeGrabsIt(); testLeftClickInEmptySpaceAdds(); @@ -105,6 +265,15 @@ int main() { testOutsideTheBoxGrabsButNeverAdds(); testDegenerateBoxResolvesToNothing(); testOverlayBoxIsTheWholeArea(); + + testFreshRampDownEndpointBeatsTheStartMarkerAtFrameZero(); + testCrossfadeTabBeatsAContourNodeNearItsTopStrip(); + testContourNodeBeatsALoopMarkerAtTheirSharedPixelButNotElsewhere(); + testTabWinsAGenuineTabVersusMarkerTie(); + testNoHitAnywhereFallsThroughToNone(); + testAMissedCandidateNeverWinsOnADegenerateZeroArea(); + testControlClickAlwaysTakesTheNodeOverASmallerTabOrMarker(); + if (g_fail == 0) std::printf("spline_edit: all tests passed\n"); return g_fail == 0 ? 0 : 1; } diff --git a/tests/test_spline_egs.cpp b/tests/test_spline_egs.cpp index 16d78a3..e977d85 100644 --- a/tests/test_spline_egs.cpp +++ b/tests/test_spline_egs.cpp @@ -363,6 +363,33 @@ static void testFlatZeroFinalSegmentStillFreesTheVoiceEarly() { CHECK(!v.soundingNote()); // a genuine permanent terminus, not a mid-contour dip } +// The timing, not just the fact: the case above is trivially "early" (a wholly-flat contour +// frees on frame 0), which can't distinguish "frees early" from "frees at the right frame." This +// fixture's final segment starts MID-sample, so the free must land there, not at frame 0 and not +// at the sample's natural end. The breakpoint (phase 0.5495) is deliberately off every sampled +// frame's exact phase (k/1000), so no sampled frame lands on the segment boundary itself and +// which segment "owns" that frame is never ambiguous. +static void testFlatZeroFinalSegmentFreesTheVoiceWhereItBeginsNotAtFrameZero() { + const double breakpointPhase = 0.5495; + const VelocityCurve contour = VelocityCurve::fromPoints( + {{xAt(0.0), 1.0}, {xAt(breakpointPhase), 0.0}, {xAt(1.0), 0.0}}, CurveDomain::Unipolar); + const std::size_t frames = 1000; + const SampleData s = splineAmpSample(frames, contour); + + Voice v; + v.start(60, 100, s); + // Frames 0..549 (phase < breakpoint) sit on the declining first segment: still sounding. + for (std::size_t i = 0; i < 550; ++i) { + v.renderFrame(); + CHECK(v.soundingNote()); + } + // Frame 550 (phase 0.55) is the first sampled frame past the breakpoint, on the flat-zero + // final segment — this is where the early-free fires. + const double y550 = v.renderFrame(); + CHECK(near(y550, 0.0, 1e-9)); + CHECK(!v.soundingNote()); +} + // --- 9. A fresh spline EG opens on the smooth y = 1 - x ---------------------- static void testAFreshSplineEgDefaultsToTheSmoothDownwardSlope() { @@ -532,6 +559,7 @@ int main() { testAContourReplaysProportionallyOnADifferentLengthSample(); testTwoPointContourRisingFromZeroSoundsForItsFullSpan(); testFlatZeroFinalSegmentStillFreesTheVoiceEarly(); + testFlatZeroFinalSegmentFreesTheVoiceWhereItBeginsNotAtFrameZero(); testAFreshSplineEgDefaultsToTheSmoothDownwardSlope(); testGateIsUnavailableWhileASplineEgIsActiveAndReturnsAfterwards(); testEnforceGateUnavailableWhileDrawnForcesTriggerOnBothRepresentations(); diff --git a/tests/test_waveform_view.cpp b/tests/test_waveform_view.cpp index bab64a8..d74627f 100644 --- a/tests/test_waveform_view.cpp +++ b/tests/test_waveform_view.cpp @@ -13,7 +13,6 @@ // stacked height, grabs reaching the lower lane); laneEnvelope (per-lane channel split). #include "../src/core/instrument/ui/waveform_view.h" -#include "../src/core/instrument/engine/velocity_curve.h" // kCurveNodeGrabRadius, VelocityCurve::pointAtPixel #include "../src/core/instrument/ui/sample_bands.h" // kWaveformMinHeight, kLaneGap #include @@ -22,7 +21,6 @@ using namespace reasampler; using namespace reasampler::instrument::ui; -using namespace reasampler::instrument::engine; using reasampler::audio::AudioSample; static int g_fail = 0; @@ -386,102 +384,6 @@ static void testStartMarkerSharesTheHandleStripWhenItSitsAtTheFadeEdge() { CHECK(contains(markerHandleRect(overlayOf(a), 1000, fadeEdge), mx, topY)); } -// --- Smallest-target-first: the three-way coincidence with a spline contour node ------- -// -// editor_input_waveform.cpp's mouseDownWaveform resolves a click among a contour node (a fixed -// pick box), the crossfade tab, and a marker's full-height column by measuring each candidate's -// own target area and letting the smallest win — this module can't exercise the shell's -// arbitration itself (no shell test target wraps the editor), but it can pin the geometric facts -// that arbitration depends on, over the pure primitives it composes. A realistic band height -// (kWaveformMinHeight, the product's own floor) is used throughout so these numbers are the -// worst case for the node, not a favourable one. -static VelocityCurve::Box boxOf(const Rect& r) { return VelocityCurve::Box{r.x, r.y, r.width, r.height}; } - -// Case (a): a fresh Spline default (rampDown) puts its endpoint 0 at (box.left, box.top) — the -// exact pixel the start marker draws at frame 0. The node's fixed 169px pick box is far smaller -// than a kWaveformMinHeight-tall marker column, so the endpoint stays reachable. -static void testFreshRampDownEndpointBeatsTheStartMarkerAtFrameZero() { - const Rect a = Rect{20, 10, 1000, kWaveformMinHeight}; - const OverlayArea overlay = overlayOf(a); - const std::int64_t frames = 100000; - const VelocityCurve contour = VelocityCurve::rampDown(); - const VelocityCurve::Box box = boxOf(a); - - CHECK(contour.pointAtPixel(box, a.x, a.y) == 0); // endpoint 0 sits at (box.left, box.top) - const std::int64_t markers[1] = {0}; - CHECK(markerAtPoint(overlay, frames, markers, 1, a.x, a.y) == 0); // the coincidence - - constexpr std::int64_t nodeSide = 2 * kCurveNodeGrabRadius + 1; - constexpr std::int64_t nodeArea = nodeSide * nodeSide; // 169, fixed - const std::int64_t markerArea = - static_cast(2 * kMarkerGrabWidth + 1) * a.height; // 11 * band height - CHECK(nodeArea < markerArea); // the node wins: the endpoint stays a genuine grab target -} - -// Case (b): the crossfade tab at zero crossfade sits on loopStart's own pixel column; a node -// dragged to value ~0.98 lands a couple of rows below the box top — inside the tab's own -// top-strip band, where the review found the tab fully shadowed by a node-first pass. -static void testCrossfadeTabBeatsAContourNodeNearItsTopStrip() { - const Rect a = Rect{20, 10, 1000, kWaveformMinHeight}; - const OverlayArea overlay = overlayOf(a); - const std::int64_t frames = 100000; - const std::int64_t loopStart = 40000, crossfade = 0; // zero crossfade -> tab sits on loopStart - const int mx = frameToX(overlay, frames, loopStart - crossfade); - const Rect tabRect = markerHandleRect(overlay, frames, loopStart - crossfade); - CHECK(!tabRect.empty()); - - const VelocityCurve::Box box = boxOf(a); - const int ny = a.y + 3; // ~0.98 up a kWaveformMinHeight-tall box; inside the tab's top strip - VelocityCurve c = VelocityCurve::flat(); - const VelocityPoint p = c.pointFromPixel(box, mx, ny); - c.addPoint(p.velocity, p.value); - CHECK(c.pointAtPixel(box, mx, ny) >= 0); - CHECK(contains(tabRect, mx, ny)); // the coincidence: both claim the same pixel - - constexpr std::int64_t nodeSide = 2 * kCurveNodeGrabRadius + 1; - constexpr std::int64_t nodeArea = nodeSide * nodeSide; // 169, fixed - const std::int64_t tabArea = static_cast(tabRect.width) * tabRect.height; // <= 110 - CHECK(tabArea < nodeArea); // the tab wins: it stays the only affordance at zero crossfade - - // The residual the review names: the node keeps its OUTER columns, one pixel past the tab's - // clipped edge but still inside its own pick radius. - const int outerX = mx + kMarkerHandleHalfWidth + 1; - CHECK(!contains(tabRect, outerX, ny)); - CHECK(c.pointAtPixel(box, outerX, ny) >= 0); -} - -// Case (c): a contour node coincident with a loop marker. At kWaveformMinHeight (the product's -// own floor) the column is already an order of magnitude larger than the node's fixed pick box, -// so the node wins the shared pixel while the column stays reachable everywhere the node isn't. -static void testContourNodeBeatsALoopMarkerAtTheirSharedPixelButNotElsewhere() { - const Rect a = Rect{20, 10, 1000, kWaveformMinHeight}; - const OverlayArea overlay = overlayOf(a); - const std::int64_t frames = 100000; - const std::int64_t loopEnd = 70000; - const int mx = frameToX(overlay, frames, loopEnd); - const std::int64_t markers[1] = {loopEnd}; - - const VelocityCurve::Box box = boxOf(a); - const int ny = a.y + a.height / 2; // mid-height, well clear of any tab - VelocityCurve c = VelocityCurve::flat(); - const VelocityPoint p = c.pointFromPixel(box, mx, ny); - c.addPoint(p.velocity, p.value); - CHECK(c.pointAtPixel(box, mx, ny) >= 0); - CHECK(markerAtPoint(overlay, frames, markers, 1, mx, ny) == 0); // the coincidence - - constexpr std::int64_t nodeSide = 2 * kCurveNodeGrabRadius + 1; - constexpr std::int64_t nodeArea = nodeSide * nodeSide; // 169, fixed - const std::int64_t markerArea = - static_cast(2 * kMarkerGrabWidth + 1) * a.height; // 11 * band height - CHECK(nodeArea < markerArea); // the node wins the shared pixel - - // A few rows clear of the node (outside its 13px pick box, still on the marker's column) - // the marker alone claims the click. - const int farY = ny + kCurveNodeGrabRadius + 4; - CHECK(c.pointAtPixel(box, mx, farY) < 0); - CHECK(markerAtPoint(overlay, frames, markers, 1, mx, farY) == 0); -} - // --- Per-lane envelope content ------------------------------------------------- static void testAsymmetricStereoLanesCarryDifferentContent() { @@ -557,9 +459,6 @@ int main() { testMarkerHandleClipsIntoTheArea(); testMarkerHandleOnDegenerateAreas(); testStartMarkerSharesTheHandleStripWhenItSitsAtTheFadeEdge(); - testFreshRampDownEndpointBeatsTheStartMarkerAtFrameZero(); - testCrossfadeTabBeatsAContourNodeNearItsTopStrip(); - testContourNodeBeatsALoopMarkerAtTheirSharedPixelButNotElsewhere(); testAsymmetricStereoLanesCarryDifferentContent(); testLaneEnvelopeRejectsOutOfRangeLane();