From 41876674e4550de42f486c53aac75d543af4cf30 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sun, 2 Aug 2026 08:45:35 -0400 Subject: [PATCH] Fix deck-UI review findings: right-anchor MASTER's meter column, correct stale/overclaiming comments, split test_deck_groups.cpp on its commit-tier/overlay seam, and pin two width-ceiling assertions. --- src/core/instrument/ui/CMakeLists.txt | 4 + src/core/instrument/ui/knob_deck.cpp | 12 +- src/core/instrument/ui/knob_deck.h | 14 +- tests/test_deck_groups.cpp | 274 ++++++-------------------- tests/test_deck_groups_state.cpp | 231 ++++++++++++++++++++++ 5 files changed, 304 insertions(+), 231 deletions(-) create mode 100644 tests/test_deck_groups_state.cpp diff --git a/src/core/instrument/ui/CMakeLists.txt b/src/core/instrument/ui/CMakeLists.txt index 1ab5169..a005c02 100644 --- a/src/core/instrument/ui/CMakeLists.txt +++ b/src/core/instrument/ui/CMakeLists.txt @@ -79,6 +79,10 @@ reasampler_pure_library(deck_groups # assertion needs the band allocator, and the MASTER-reserve identity needs the column width the # PRIVATE edge above does not re-export. reasampler_test(deck_groups LINK deck_groups sample_bands master_meter) +# The commit-tier + overlay-selection state machine, split out of deck_groups_tests on the seam +# those fixtures already had: deckParamCommit/liveCommitFor and the overlay predicates are pure +# control-id/enum logic that touches no layout, so this target needs no sample_bands/master_meter. +reasampler_test(deck_groups_state LINK deck_groups) # The point-editing grammar both spline consumers share, so it links the curve itself (unlike # envelope_overlay/envelope_edit, which stay engine-free — the staged envelopes touch no curve). diff --git a/src/core/instrument/ui/knob_deck.cpp b/src/core/instrument/ui/knob_deck.cpp index 680dbb5..caf5dcf 100644 --- a/src/core/instrument/ui/knob_deck.cpp +++ b/src/core/instrument/ui/knob_deck.cpp @@ -8,8 +8,8 @@ namespace reasampler::instrument::ui { namespace { -// The knob-row width of a group: cells side by side (no inter-cell gap — the 48px cell -// already carries its own breathing room around the 28px knob), plus the optional row +// The knob-row width of a group: cells side by side (no inter-cell gap — the 60px cell +// already carries its own breathing room around the 40px knob), plus the optional row // toggle after a kDeckToggleGap. A spanning group's cells stack, so its knob row is one // cell wide plus whatever readout column sits beside it. int knobRowWidth(const DeckGroupDesc& g) { @@ -109,8 +109,10 @@ DeckGroupLayout layoutGroup(const DeckGroupDesc& g, const Rect& box) { slotTop += kDeckGroupH + kDeckRowGap; } if (g.column.id >= 0) { - // ONE rect spanning every slot, not a readout per row. - const int colX = innerLeft + (g.cellIds.empty() ? 0 : kDeckCellW + kDeckColumnGap); + // ONE rect spanning every slot, not a readout per row. Right-anchored off + // innerRight rather than measured past the cell slot, so a wider caption + // reserve on this group can never detach the column from the padding. + const int colX = innerRight - g.column.width; out.column = DeckColumnLayout{ g.column.id, Rect::ltrb(colX, cellTop, colX + g.column.width, box.bottom() - kDeckGroupPadY)}; @@ -156,7 +158,7 @@ std::vector justifyGutters(int count, int total, int blockW) { const int slack = blockW - total; if (slack < gutters * kDeckGroupGap) { // The block cannot hold the row: minimum gutters, and the row overruns to the right - // rather than wrapping. Unreachable in the editor — see layoutDeck's header note. + // rather than wrapping — see layoutDeck's header note for when this degrade applies. return std::vector(static_cast(gutters), kDeckGroupGap); } const int base = slack / gutters; diff --git a/src/core/instrument/ui/knob_deck.h b/src/core/instrument/ui/knob_deck.h index 194737d..3e7103e 100644 --- a/src/core/instrument/ui/knob_deck.h +++ b/src/core/instrument/ui/knob_deck.h @@ -1,12 +1,8 @@ // knob_deck.h — knob-deck layout + hit-test for the Sample-face knob deck. Engine-free // like param_slider: cells and toggles carry opaque shell-owned control ids. Mirror of // action_bar/param_slider; the knob primitive itself (value<->needle-angle, drag) is -// param_slider's — a knob cell here is just a rect the shell composes it into. -// -// A group is a fenced box: caption row (caption left, toggles and a corner radio -// right-anchored) over a knob row of equal-width cells, optionally followed by one row -// toggle. Row membership is a PROPERTY OF THE GROUP (DeckRow), never a wrap outcome — see -// the justification law at layoutDeck. +// param_slider's — a knob cell here is just a rect the shell composes it into. Group/row +// composition and the justification law are this directory's own CLAUDE.md's to describe. #pragma once @@ -31,7 +27,8 @@ inline constexpr int kDeckGroupPadY = 4; // group box vertical inner paddin inline constexpr int kDeckCaptionGap = 2; // caption row -> knob row gap inline constexpr int kDeckToggleGap = 4; // caption text -> toggle / cells -> row toggle gap inline constexpr int kDeckGroupGap = 12; // gap between groups on a row -inline constexpr int kDeckRowGap = 8; // gap between wrapped deck rows +inline constexpr int kDeckRowGap = 8; // gap between the deck's two categorical rows, + // and between the spanning deck's stacked slots inline constexpr int kDeckRadioSize = 12; // the caption-row corner radio square inline constexpr int kDeckColumnGap = 8; // the spanning deck's cell column -> its readout column // The knob cell's INNER dial: a concentric sub-disc that edits a second, related value while @@ -181,7 +178,8 @@ int deckHeight(const std::vector& groups); // divided equally with the integer residue going to the leftmost ones. Decks are never // stretched. Below the width the block needs, every gutter sits at kDeckGroupGap and the row // overflows right rather than wrapping — the shell clamps the window to a floor that fits -// (sample_bands' kEditorMinWidth), so that degrade is unreachable in the editor. +// (sample_bands' kEditorMinWidth) via checkSizeConstraint, a host-honoured clamp rather than a +// guarantee, so this degrade is defined and tested rather than assumed impossible. DeckLayout layoutDeck(const std::vector& groups, int left, int top, int availWidth); diff --git a/tests/test_deck_groups.cpp b/tests/test_deck_groups.cpp index 9574406..9b75b87 100644 --- a/tests/test_deck_groups.cpp +++ b/tests/test_deck_groups.cpp @@ -6,10 +6,9 @@ // floor width and its fit inside the floor window, the pinned Gate group widths, the editor // floor derived from the deck's width budget and each group's categorical row, // that no face leaves slack where its dropped controls were and that a Gate/Spline/Gate round -// trip restores the layout exactly, the hit-test reaching the new filter controls, the bipolar knob -// law's inverse pair, the commit-tier routing — which controls are live, and which drags take -// the live tier — and the overlay-selection state machine (exclusivity, the none resting state, -// and which selections are inert). +// trip restores the layout exactly, the hit-test reaching the new filter controls, and the +// bipolar knob law's inverse pair. The commit-tier routing and the overlay-selection state +// machine live in test_deck_groups_state.cpp — they touch no layout at all. #include "../src/core/instrument/ui/deck_groups.h" #include "../src/core/instrument/ui/master_meter.h" // kMeterColumnW: MASTER's reserve IS this @@ -428,21 +427,26 @@ static void testGutterArithmeticAndTheFilterTieLineAtTheFloor() { // Above the floor the tie-line DRIFTS, which is accepted and deliberate (§1.3): row 1 divides // its slack over three gutters and row 2 over two, so row 2's filter edge pulls right past // row 1's and the gap widens monotonically. Encoded as EXPECTED, not as a failure. +// +// Checked per ROW (tracking the last-seen box in each of the two categorical rows while +// walking dl.groups in deck order), not just deck-order neighbours: two same-row groups can +// sit apart in deck order with a different-row group between them, and a deck-order-only +// check would silently skip that gutter. static void testGuttersHoldTheirMinimumAndTheTieLineDriftsAboveTheFloor() { for (PlayMode mode : {PlayMode::Gate, PlayMode::Trigger}) { const std::vector g = sampleDeckGroups(mode); int lastDrift = 1 << 20; // sentinel above any real drift for (int avail = kAvailAtMinWidth; avail <= kAvailAtMinWidth + 600; avail += 37) { const DeckLayout dl = layoutDeck(g, kPad, 0, avail); - const DeckGroupLayout* prev = nullptr; - DeckRow prevRow = DeckRow::Spanning; + const DeckGroupLayout* prevInRow[2] = {nullptr, nullptr}; for (const DeckGroupLayout& gl : dl.groups) { const DeckRow row = deckRowFor(static_cast(gl.id)); - if (row != DeckRow::Spanning && prev && row == prevRow) { - CHECK(gl.box.x - prev->box.right() >= kDeckGroupGap); + if (row == DeckRow::Spanning) continue; + const int r = row == DeckRow::Contour ? 1 : 0; + if (prevInRow[r]) { + CHECK(gl.box.x - prevInRow[r]->box.right() >= kDeckGroupGap); } - prev = ≷ - prevRow = row; + prevInRow[r] = ≷ } const auto right = [&](int id) { return dl.groups[static_cast(indexOfGroup(g, id))].box.right(); @@ -532,6 +536,22 @@ static void testTheMasterColumnDoesNotDivideItsRunVertically() { CHECK(m2.column.box == m.column.box); } +// MASTER's caption row and knob row measure exactly equal (130 == 130) today, so a column +// derived from either edge lands in the same place — that balance is what let a left-derived +// offset masquerade as right-anchored. Widen the caption reserve alone (as a wider caption or +// a limiter-toggle change would) and the column must still land flush against the group's own +// right padding, derived from innerRight rather than measured past the cell slots. +static void testMasterColumnStaysRightAnchoredWhenCaptionRowOutgrowsTheKnobRow() { + const std::vector g = sampleDeckGroups(PlayMode::Gate); + DeckGroupDesc probe = g[static_cast(indexOfGroup(g, kGroupMaster))]; + probe.captionWidth += 40; // unbalances it: the caption row now measures past the knob row + const std::vector one = {probe}; + const DeckLayout dl = layoutDeck(one, kPad, 0, kAvailAtMinWidth); + const DeckGroupLayout& m = dl.groups[0]; + CHECK(m.box.width > 142); // the widen is real, not absorbed elsewhere + CHECK(m.column.box.right() == m.box.right() - kDeckGroupPadX); +} + static void testHitTestResolvesTheNewFilterControls() { const std::vector g = sampleDeckGroups(PlayMode::Gate); const DeckLayout dl = layoutDeck(g, kPad, 40, kAvailAtMinWidth); @@ -597,206 +617,6 @@ static void testBipolarKnobLawRoundTripsAndIsExactAtCentre() { CHECK(deckNormFromBipolar(3.0) == 1.0); } -static void testEveryDeckControlIsClassifiedIntoOneOfTheThreeCommitTiers() { - // The live set: the seven filter tone/modulation knobs, the baseline pitch offset, plus - // every stage time, stage level, hold fraction and curve exponent on all three envelopes — - // in BOTH mode shapes. - const DeckParam live[] = { - DeckParam::kPitch, - DeckParam::kFilterMorph, DeckParam::kFilterCutoff, DeckParam::kFilterQ, - DeckParam::kFilterDrive, DeckParam::kFilterModAmt, DeckParam::kFilterVel, - DeckParam::kFilterKeyTrack, - DeckParam::kAttack, DeckParam::kHold, DeckParam::kDecay, DeckParam::kSustain, - DeckParam::kRelease, - DeckParam::kTrigAttack, DeckParam::kTrigHold, DeckParam::kTrigDecay, - DeckParam::kFilterEnvAttack, DeckParam::kFilterEnvHold, DeckParam::kFilterEnvDecay, - DeckParam::kFilterEnvSustain, DeckParam::kFilterEnvRelease, - DeckParam::kFilterTrigAttack, DeckParam::kFilterTrigHold, DeckParam::kFilterTrigDecay, - DeckParam::kPitchEnvAttack, DeckParam::kPitchEnvHold, DeckParam::kPitchEnvDecay, - DeckParam::kPitchEnvDepth, - DeckParam::kAttackCurve, DeckParam::kDecayCurve, DeckParam::kReleaseCurve, - DeckParam::kTrigAttackCurve, DeckParam::kTrigDecayCurve, - DeckParam::kPitchEnvAttackCurve, DeckParam::kPitchEnvDecayCurve, - DeckParam::kFilterEnvAttackCurve, DeckParam::kFilterEnvDecayCurve, - DeckParam::kFilterEnvReleaseCurve, - DeckParam::kFilterTrigAttackCurve, DeckParam::kFilterTrigDecayCurve, - }; - for (DeckParam p : live) CHECK(deckParamCommit(p) == LiveCommit::Live); - - // The note-on-latched tier: published like a live control, read only at note-on. Asserted as - // its OWN state rather than as "not Reload" — the whole point of widening the predicate is - // that Rate must not fall back into either neighbour, and Γ-W4-T1 reads this classification - // to decide what it exposes to the host. - const DeckParam latched[] = {DeckParam::kRate}; - for (DeckParam p : latched) CHECK(deckParamCommit(p) == LiveCommit::NoteOnLatched); - - // Everything else reloads or rebuilds; deck_groups.h is the home for why each exclusion - // is excluded. - const DeckParam reloads[] = { - DeckParam::kPlayMode, DeckParam::kPitchEngine, DeckParam::kPitchEnvEnable, - DeckParam::kFilterEnable, DeckParam::kFilterLaw, - DeckParam::kAmpVelCurve, DeckParam::kPitchVelCurve, DeckParam::kFilterVelCurve, - DeckParam::kKeyTrack, DeckParam::kTrigLength, - DeckParam::kAmpEnvSelect, DeckParam::kPitchEnvSelect, DeckParam::kFilterEnvSelect, - DeckParam::kAmpEnvMode, DeckParam::kPitchEnvMode, DeckParam::kFilterEnvMode, - DeckParam::kVoiceCount, DeckParam::kVoiceMode, - DeckParam::kMonoTrigger, DeckParam::kMasterGain, DeckParam::kLimiterEnable, - DeckParam::kMasterMeter, DeckParam::kMasterGr, - }; - for (DeckParam p : reloads) CHECK(deckParamCommit(p) == LiveCommit::Reload); - - // COVERAGE, not cardinality: every id appears in EXACTLY ONE of the three lists. A sum check - // would stay green if an edit duplicated one id and dropped another, leaving that one - // unclassified. - for (int i = 0; i < static_cast(DeckParam::kCount); ++i) { - const DeckParam p = static_cast(i); - int seen = 0; - for (DeckParam q : live) if (q == p) ++seen; - for (DeckParam q : latched) if (q == p) ++seen; - for (DeckParam q : reloads) if (q == p) ++seen; - if (seen != 1) std::printf(" (deck id %d classified %d times)\n", i, seen); - CHECK(seen == 1); - } -} - -static void testOnlyALiveControlsDragTakesTheLiveTier() { - // deckParamCommit alone is not what a user experiences — liveCommitFor is, at the editor's - // commit site. Inverting it has to FAIL a test rather than merely read wrong. - const auto knob = [](DeckParam p) { - return liveCommitFor(LiveDragKind::kDeckKnob, static_cast(p)); - }; - CHECK(knob(DeckParam::kFilterCutoff) == LiveCommit::Live); - CHECK(knob(DeckParam::kAttack) == LiveCommit::Live); - CHECK(knob(DeckParam::kPitch) == LiveCommit::Live); - // The Trigger amp is live now that the fade pair folded into the AHD — the one behavioural - // consequence of that consolidation. - CHECK(knob(DeckParam::kTrigAttack) == LiveCommit::Live); - CHECK(knob(DeckParam::kTrigDecayCurve) == LiveCommit::Live); - // Rate keeps its own tier through the drag site: it must not arrive as Live (which would let - // it move a sounding note) nor as Reload (which would re-decode the WAV under a swept knob). - CHECK(knob(DeckParam::kRate) == LiveCommit::NoteOnLatched); - CHECK(knob(DeckParam::kTrigLength) == LiveCommit::Reload); - CHECK(knob(DeckParam::kMasterGain) == LiveCommit::Reload); - CHECK(knob(DeckParam::kAmpEnvSelect) == LiveCommit::Reload); - // The shell's processor-side sentinels (preview velocity is -2) and any out-of-range id - // are not parameter-set controls, so they must never reach the enum. - CHECK(liveCommitFor(LiveDragKind::kDeckKnob, -2) == LiveCommit::Reload); - CHECK(liveCommitFor(LiveDragKind::kDeckKnob, -1) == LiveCommit::Reload); - CHECK(knob(DeckParam::kCount) == LiveCommit::Reload); - // Every stage value an envelope node can reach is live, in either mode shape. - CHECK(liveCommitFor(LiveDragKind::kEnvNode, -1) == LiveCommit::Live); - // Every other drag (markers, scrollbar, curve nodes) commits through a reload. - CHECK(liveCommitFor(LiveDragKind::kOther, static_cast(DeckParam::kFilterCutoff)) == - LiveCommit::Reload); -} - -// --- The overlay selection state machine --------------------------------------- - -static int radio(DeckParam p) { return static_cast(p); } - -// EXCLUSIVITY: picking another deck's radio switches to it outright — two envelopes can never -// be overlay-active at once, whatever the previous selection was. -static void testOverlaySelectionIsExclusiveAcrossTheThreeEnvelopeDecks() { - const OverlayEnv states[] = {OverlayEnv::kNone, OverlayEnv::kAmp, OverlayEnv::kPitch, - OverlayEnv::kFilter}; - for (OverlayEnv from : states) { - if (from != OverlayEnv::kAmp) { - CHECK(nextOverlaySelection(from, radio(DeckParam::kAmpEnvSelect)) == OverlayEnv::kAmp); - } - if (from != OverlayEnv::kPitch) { - CHECK(nextOverlaySelection(from, radio(DeckParam::kPitchEnvSelect)) == - OverlayEnv::kPitch); - } - if (from != OverlayEnv::kFilter) { - CHECK(nextOverlaySelection(from, radio(DeckParam::kFilterEnvSelect)) == - OverlayEnv::kFilter); - } - } -} - -// kNone is a RESTING STATE the user can get back to: clicking the active radio clears it. -static void testClickingTheActiveOverlayRadioClearsToNone() { - CHECK(nextOverlaySelection(OverlayEnv::kAmp, radio(DeckParam::kAmpEnvSelect)) == - OverlayEnv::kNone); - CHECK(nextOverlaySelection(OverlayEnv::kPitch, radio(DeckParam::kPitchEnvSelect)) == - OverlayEnv::kNone); - CHECK(nextOverlaySelection(OverlayEnv::kFilter, radio(DeckParam::kFilterEnvSelect)) == - OverlayEnv::kNone); -} - -// A control that is not one of the three radios selects nothing and clears nothing. -static void testANonRadioIdLeavesTheOverlaySelectionAlone() { - CHECK(overlayEnvForRadio(radio(DeckParam::kFilterCutoff)) == OverlayEnv::kNone); - CHECK(overlayEnvForRadio(-1) == OverlayEnv::kNone); - CHECK(nextOverlaySelection(OverlayEnv::kFilter, radio(DeckParam::kFilterCutoff)) == - OverlayEnv::kFilter); - CHECK(nextOverlaySelection(OverlayEnv::kAmp, 9999) == OverlayEnv::kAmp); -} - -// The two group gates, spelled the way the predicates read them. Spline flags default off, so -// a case that says nothing about them is asserting the staged behaviour. -static DeckEnableState gates(bool pitchEnv, bool filter) { - DeckEnableState s; - s.pitchEnvEnabled = pitchEnv; - s.filterEnabled = filter; - return s; -} - -// An overlay whose deck group is switched OFF is inert, matching the drawn-but-dead knobs on -// the same params: a node drag must not reach a value the knob refuses. -static void testOverlayIsInertExactlyWhenItsGroupToggleIsOff() { - CHECK(overlayEnvInert(OverlayEnv::kPitch, gates(/*pitchEnv=*/false, /*filter=*/true))); - CHECK(!overlayEnvInert(OverlayEnv::kPitch, gates(true, true))); - CHECK(overlayEnvInert(OverlayEnv::kFilter, gates(true, /*filter=*/false))); - CHECK(!overlayEnvInert(OverlayEnv::kFilter, gates(true, true))); - // Amp has no enable toggle, so it is never inert; kNone draws nothing to grab. - CHECK(!overlayEnvInert(OverlayEnv::kAmp, gates(false, false))); - CHECK(!overlayEnvInert(OverlayEnv::kNone, gates(false, false))); - - // The enable gate alone, which the SPLINE overlay reads: it survives a mode switch, so a - // disabled group's contour is as dead as its knobs. - CHECK(!overlayEnvEnabled(OverlayEnv::kPitch, gates(false, true))); - CHECK(overlayEnvEnabled(OverlayEnv::kAmp, gates(false, false))); - // ...while the staged overlay additionally goes inert once the envelope is drawn: its - // nodes are no longer what the overlay is editing. - DeckEnableState drawn = gates(true, true); - drawn.ampSpline = true; - CHECK(overlayEnvInert(OverlayEnv::kAmp, drawn)); - CHECK(overlayEnvEnabled(OverlayEnv::kAmp, drawn)); -} - -// A deck knob goes inert exactly with its group's own enable toggle — including the filter's -// VELOCITY cell, which sits in the VELOCITY group visually but is a filter parameter and must -// go inert with the rest of the filter (the reachable-through-the-deck route mouseDownDeck -// checks before ever routing a curve-cell click to the popup). -static void testDeckKnobIsInertExactlyWithItsGroupsEnableToggle() { - CHECK(deckKnobInert(DeckParam::kFilterVelCurve, gates(/*pitchEnv=*/true, /*filter=*/false))); - CHECK(!deckKnobInert(DeckParam::kFilterVelCurve, gates(true, true))); - CHECK(deckKnobInert(DeckParam::kFilterCutoff, gates(true, false))); - CHECK(!deckKnobInert(DeckParam::kFilterCutoff, gates(true, true))); - CHECK(deckKnobInert(DeckParam::kPitchEnvDepth, gates(/*pitchEnv=*/false, true))); - CHECK(!deckKnobInert(DeckParam::kPitchEnvDepth, gates(true, true))); - // The amp's own velocity cell and every ordinary control are never inert here — inertness - // is a filter/pitch-env-group-only concept until an envelope is drawn. - CHECK(!deckKnobInert(DeckParam::kAmpVelCurve, gates(false, false))); - CHECK(!deckKnobInert(DeckParam::kAttack, gates(false, false))); -} - -// A drawn envelope's STAGED segment knobs go inert; the mode toggle itself and the depth knobs -// that scale either shape stay live. (Which segment knobs, per envelope, is pinned in -// spline_egs_tests alongside the rest of the spline rules.) -static void testAModeToggleIsNeitherLiveNorAnOverlayRadio() { - CHECK(deckParamCommit(DeckParam::kAmpEnvMode) == LiveCommit::Reload); - CHECK(deckParamCommit(DeckParam::kPitchEnvMode) == LiveCommit::Reload); - CHECK(deckParamCommit(DeckParam::kFilterEnvMode) == LiveCommit::Reload); - CHECK(overlayEnvForModeToggle(radio(DeckParam::kAmpEnvMode)) == OverlayEnv::kAmp); - CHECK(overlayEnvForModeToggle(radio(DeckParam::kPitchEnvMode)) == OverlayEnv::kPitch); - CHECK(overlayEnvForModeToggle(radio(DeckParam::kFilterEnvMode)) == OverlayEnv::kFilter); - // A mode toggle must not be mistaken for the overlay-select radio beside it. - CHECK(overlayEnvForRadio(radio(DeckParam::kAmpEnvMode)) == OverlayEnv::kNone); - CHECK(overlayEnvForModeToggle(radio(DeckParam::kAmpEnvSelect)) == OverlayEnv::kNone); -} - // The three mode toggles ride each env group's caption slack, so the deck's wrapped geometry // is unchanged by them: raising their segment width past the caption headroom would reflow the // first row and push the deck to a fourth one (see testDeckFitsInsideTheEnforcedMinimumWindow). @@ -876,6 +696,30 @@ static void testThePitchRateGroupIsKnobRowDrivenAtExactlyOneNinetyTwo() { CHECK(deckGroupWidth(probe) > 192); // one past it, the caption row takes over } +// The kEnvModeSegW ceilings recorded in deck_groups.cpp's own comment (PITCH ENV binds at 47, +// AMP at 55) pinned against the descriptors they derive from, the same way the Pitch/Rate +// caption ceiling above is: a change to either group's caption width or its enable toggle +// would otherwise invalidate the recorded numbers with nothing failing. +static void testEnvModeSegWCeilingsArePinnedForPitchEnvAndAmp() { + const std::vector g = sampleDeckGroups(PlayMode::Gate); + const DeckGroupDesc& penv = g[static_cast(indexOfGroup(g, kGroupPitchEnv))]; + const DeckGroupDesc& amp = g[static_cast(indexOfGroup(g, kGroupAmpEnv))]; + CHECK(deckGroupWidth(penv) == 252); + CHECK(deckGroupWidth(amp) == 312); + + DeckGroupDesc penvProbe = penv; + penvProbe.captionToggle2.segWidth = 47; + CHECK(deckGroupWidth(penvProbe) == 252); // at the ceiling, still knob-row-driven + penvProbe.captionToggle2.segWidth = 48; + CHECK(deckGroupWidth(penvProbe) > 252); // one past it, the caption row takes over + + DeckGroupDesc ampProbe = amp; + ampProbe.captionToggle2.segWidth = 55; + CHECK(deckGroupWidth(ampProbe) == 312); + ampProbe.captionToggle2.segWidth = 56; + CHECK(deckGroupWidth(ampProbe) > 312); +} + // Every group's width, in BOTH play modes, against the measured layout table // (instrument-control-surface.md §1.2). Mode-independence is the second half of the claim: the // reserve slots hold the two mode-dependent groups at 312 either way, which is what makes the @@ -970,15 +814,7 @@ static void testGateSplineGateRoundTripsToTheSameLayout() { } int main() { - testOverlaySelectionIsExclusiveAcrossTheThreeEnvelopeDecks(); - testClickingTheActiveOverlayRadioClearsToNone(); - testANonRadioIdLeavesTheOverlaySelectionAlone(); - testOverlayIsInertExactlyWhenItsGroupToggleIsOff(); - testDeckKnobIsInertExactlyWithItsGroupsEnableToggle(); - testAModeToggleIsNeitherLiveNorAnOverlayRadio(); testTheModeTogglesCostNoGroupWidth(); - testEveryDeckControlIsClassifiedIntoOneOfTheThreeCommitTiers(); - testOnlyALiveControlsDragTakesTheLiveTier(); testDeckReadsPitchThenFilterThenAmpLeftToRight(); testVelocityGroupOwnsTheThreeCurvesExclusively(); testCurveTargetNamesEachCellsOwnDestination(); @@ -992,6 +828,7 @@ int main() { testDeckFitsInsideTheEnforcedMinimumWindow(); testNoFaceLeavesSlackWhereItsDroppedControlsWere(); testThePitchRateGroupIsKnobRowDrivenAtExactlyOneNinetyTwo(); + testEnvModeSegWCeilingsArePinnedForPitchEnvAndAmp(); testEveryGroupWidthMatchesTheMeasuredLayout(); testGateSplineGateRoundTripsToTheSameLayout(); testTheEditorFloorIsDerivedFromTheDeckWidthBudget(); @@ -1001,6 +838,7 @@ int main() { testGuttersHoldTheirMinimumAndTheTieLineDriftsAboveTheFloor(); testTheMasterDeckInteriorLandsOnBothRowBaselines(); testTheMasterColumnDoesNotDivideItsRunVertically(); + testMasterColumnStaysRightAnchoredWhenCaptionRowOutgrowsTheKnobRow(); testHitTestResolvesTheNewFilterControls(); testBipolarKnobLawRoundTripsAndIsExactAtCentre(); if (g_fail == 0) std::printf("deck_groups: all tests passed\n"); diff --git a/tests/test_deck_groups_state.cpp b/tests/test_deck_groups_state.cpp new file mode 100644 index 0000000..9a7bb62 --- /dev/null +++ b/tests/test_deck_groups_state.cpp @@ -0,0 +1,231 @@ +// Standalone tests for reasampler::instrument::ui::deck_groups' commit-tier routing and +// overlay-selection state machine — no VST3, no REAPER, no framework. Split from +// test_deck_groups.cpp on the seam those fixtures already had: nothing here touches +// layoutDeck, DeckGroupWidth, or any other geometry API — deckParamCommit/liveCommitFor (which +// controls are live, and which drags take the live tier) and the overlay-selection state +// machine (exclusivity, the none resting state, and which selections are inert) are pure +// control-id/enum predicates. test_deck_groups.cpp keeps the geometry/row/width fixtures. + +#include "../src/core/instrument/ui/deck_groups.h" + +#include + +using namespace reasampler; +using namespace reasampler::instrument::ui; + +static int g_fail = 0; +#define CHECK(cond) do { if(!(cond)) { \ + std::printf("FAIL line %d: %s\n", __LINE__, #cond); ++g_fail; } } while(0) + +static void testEveryDeckControlIsClassifiedIntoOneOfTheThreeCommitTiers() { + // The live set: the seven filter tone/modulation knobs, the baseline pitch offset, plus + // every stage time, stage level, hold fraction and curve exponent on all three envelopes — + // in BOTH mode shapes. + const DeckParam live[] = { + DeckParam::kPitch, + DeckParam::kFilterMorph, DeckParam::kFilterCutoff, DeckParam::kFilterQ, + DeckParam::kFilterDrive, DeckParam::kFilterModAmt, DeckParam::kFilterVel, + DeckParam::kFilterKeyTrack, + DeckParam::kAttack, DeckParam::kHold, DeckParam::kDecay, DeckParam::kSustain, + DeckParam::kRelease, + DeckParam::kTrigAttack, DeckParam::kTrigHold, DeckParam::kTrigDecay, + DeckParam::kFilterEnvAttack, DeckParam::kFilterEnvHold, DeckParam::kFilterEnvDecay, + DeckParam::kFilterEnvSustain, DeckParam::kFilterEnvRelease, + DeckParam::kFilterTrigAttack, DeckParam::kFilterTrigHold, DeckParam::kFilterTrigDecay, + DeckParam::kPitchEnvAttack, DeckParam::kPitchEnvHold, DeckParam::kPitchEnvDecay, + DeckParam::kPitchEnvDepth, + DeckParam::kAttackCurve, DeckParam::kDecayCurve, DeckParam::kReleaseCurve, + DeckParam::kTrigAttackCurve, DeckParam::kTrigDecayCurve, + DeckParam::kPitchEnvAttackCurve, DeckParam::kPitchEnvDecayCurve, + DeckParam::kFilterEnvAttackCurve, DeckParam::kFilterEnvDecayCurve, + DeckParam::kFilterEnvReleaseCurve, + DeckParam::kFilterTrigAttackCurve, DeckParam::kFilterTrigDecayCurve, + }; + for (DeckParam p : live) CHECK(deckParamCommit(p) == LiveCommit::Live); + + // The note-on-latched tier: published like a live control, read only at note-on. Asserted as + // its OWN state rather than as "not Reload" — the whole point of widening the predicate is + // that Rate must not fall back into either neighbour, and Γ-W4-T1 reads this classification + // to decide what it exposes to the host. + const DeckParam latched[] = {DeckParam::kRate}; + for (DeckParam p : latched) CHECK(deckParamCommit(p) == LiveCommit::NoteOnLatched); + + // Everything else reloads or rebuilds; deck_groups.h is the home for why each exclusion + // is excluded. + const DeckParam reloads[] = { + DeckParam::kPlayMode, DeckParam::kPitchEngine, DeckParam::kPitchEnvEnable, + DeckParam::kFilterEnable, DeckParam::kFilterLaw, + DeckParam::kAmpVelCurve, DeckParam::kPitchVelCurve, DeckParam::kFilterVelCurve, + DeckParam::kKeyTrack, DeckParam::kTrigLength, + DeckParam::kAmpEnvSelect, DeckParam::kPitchEnvSelect, DeckParam::kFilterEnvSelect, + DeckParam::kAmpEnvMode, DeckParam::kPitchEnvMode, DeckParam::kFilterEnvMode, + DeckParam::kVoiceCount, DeckParam::kVoiceMode, + DeckParam::kMonoTrigger, DeckParam::kMasterGain, DeckParam::kLimiterEnable, + DeckParam::kMasterMeter, DeckParam::kMasterGr, + }; + for (DeckParam p : reloads) CHECK(deckParamCommit(p) == LiveCommit::Reload); + + // COVERAGE, not cardinality: every id appears in EXACTLY ONE of the three lists. A sum check + // would stay green if an edit duplicated one id and dropped another, leaving that one + // unclassified. + for (int i = 0; i < static_cast(DeckParam::kCount); ++i) { + const DeckParam p = static_cast(i); + int seen = 0; + for (DeckParam q : live) if (q == p) ++seen; + for (DeckParam q : latched) if (q == p) ++seen; + for (DeckParam q : reloads) if (q == p) ++seen; + if (seen != 1) std::printf(" (deck id %d classified %d times)\n", i, seen); + CHECK(seen == 1); + } +} + +static void testOnlyALiveControlsDragTakesTheLiveTier() { + // deckParamCommit alone is not what a user experiences — liveCommitFor is, at the editor's + // commit site. Inverting it has to FAIL a test rather than merely read wrong. + const auto knob = [](DeckParam p) { + return liveCommitFor(LiveDragKind::kDeckKnob, static_cast(p)); + }; + CHECK(knob(DeckParam::kFilterCutoff) == LiveCommit::Live); + CHECK(knob(DeckParam::kAttack) == LiveCommit::Live); + CHECK(knob(DeckParam::kPitch) == LiveCommit::Live); + // The Trigger amp is live now that the fade pair folded into the AHD — the one behavioural + // consequence of that consolidation. + CHECK(knob(DeckParam::kTrigAttack) == LiveCommit::Live); + CHECK(knob(DeckParam::kTrigDecayCurve) == LiveCommit::Live); + // Rate keeps its own tier through the drag site: it must not arrive as Live (which would let + // it move a sounding note) nor as Reload (which would re-decode the WAV under a swept knob). + CHECK(knob(DeckParam::kRate) == LiveCommit::NoteOnLatched); + CHECK(knob(DeckParam::kTrigLength) == LiveCommit::Reload); + CHECK(knob(DeckParam::kMasterGain) == LiveCommit::Reload); + CHECK(knob(DeckParam::kAmpEnvSelect) == LiveCommit::Reload); + // The shell's processor-side sentinels (preview velocity is -2) and any out-of-range id + // are not parameter-set controls, so they must never reach the enum. + CHECK(liveCommitFor(LiveDragKind::kDeckKnob, -2) == LiveCommit::Reload); + CHECK(liveCommitFor(LiveDragKind::kDeckKnob, -1) == LiveCommit::Reload); + CHECK(knob(DeckParam::kCount) == LiveCommit::Reload); + // Every stage value an envelope node can reach is live, in either mode shape. + CHECK(liveCommitFor(LiveDragKind::kEnvNode, -1) == LiveCommit::Live); + // Every other drag (markers, scrollbar, curve nodes) commits through a reload. + CHECK(liveCommitFor(LiveDragKind::kOther, static_cast(DeckParam::kFilterCutoff)) == + LiveCommit::Reload); +} + +// --- The overlay selection state machine --------------------------------------- + +static int radio(DeckParam p) { return static_cast(p); } + +// EXCLUSIVITY: picking another deck's radio switches to it outright — two envelopes can never +// be overlay-active at once, whatever the previous selection was. +static void testOverlaySelectionIsExclusiveAcrossTheThreeEnvelopeDecks() { + const OverlayEnv states[] = {OverlayEnv::kNone, OverlayEnv::kAmp, OverlayEnv::kPitch, + OverlayEnv::kFilter}; + for (OverlayEnv from : states) { + if (from != OverlayEnv::kAmp) { + CHECK(nextOverlaySelection(from, radio(DeckParam::kAmpEnvSelect)) == OverlayEnv::kAmp); + } + if (from != OverlayEnv::kPitch) { + CHECK(nextOverlaySelection(from, radio(DeckParam::kPitchEnvSelect)) == + OverlayEnv::kPitch); + } + if (from != OverlayEnv::kFilter) { + CHECK(nextOverlaySelection(from, radio(DeckParam::kFilterEnvSelect)) == + OverlayEnv::kFilter); + } + } +} + +// kNone is a RESTING STATE the user can get back to: clicking the active radio clears it. +static void testClickingTheActiveOverlayRadioClearsToNone() { + CHECK(nextOverlaySelection(OverlayEnv::kAmp, radio(DeckParam::kAmpEnvSelect)) == + OverlayEnv::kNone); + CHECK(nextOverlaySelection(OverlayEnv::kPitch, radio(DeckParam::kPitchEnvSelect)) == + OverlayEnv::kNone); + CHECK(nextOverlaySelection(OverlayEnv::kFilter, radio(DeckParam::kFilterEnvSelect)) == + OverlayEnv::kNone); +} + +// A control that is not one of the three radios selects nothing and clears nothing. +static void testANonRadioIdLeavesTheOverlaySelectionAlone() { + CHECK(overlayEnvForRadio(radio(DeckParam::kFilterCutoff)) == OverlayEnv::kNone); + CHECK(overlayEnvForRadio(-1) == OverlayEnv::kNone); + CHECK(nextOverlaySelection(OverlayEnv::kFilter, radio(DeckParam::kFilterCutoff)) == + OverlayEnv::kFilter); + CHECK(nextOverlaySelection(OverlayEnv::kAmp, 9999) == OverlayEnv::kAmp); +} + +// The two group gates, spelled the way the predicates read them. Spline flags default off, so +// a case that says nothing about them is asserting the staged behaviour. +static DeckEnableState gates(bool pitchEnv, bool filter) { + DeckEnableState s; + s.pitchEnvEnabled = pitchEnv; + s.filterEnabled = filter; + return s; +} + +// An overlay whose deck group is switched OFF is inert, matching the drawn-but-dead knobs on +// the same params: a node drag must not reach a value the knob refuses. +static void testOverlayIsInertExactlyWhenItsGroupToggleIsOff() { + CHECK(overlayEnvInert(OverlayEnv::kPitch, gates(/*pitchEnv=*/false, /*filter=*/true))); + CHECK(!overlayEnvInert(OverlayEnv::kPitch, gates(true, true))); + CHECK(overlayEnvInert(OverlayEnv::kFilter, gates(true, /*filter=*/false))); + CHECK(!overlayEnvInert(OverlayEnv::kFilter, gates(true, true))); + // Amp has no enable toggle, so it is never inert; kNone draws nothing to grab. + CHECK(!overlayEnvInert(OverlayEnv::kAmp, gates(false, false))); + CHECK(!overlayEnvInert(OverlayEnv::kNone, gates(false, false))); + + // The enable gate alone, which the SPLINE overlay reads: it survives a mode switch, so a + // disabled group's contour is as dead as its knobs. + CHECK(!overlayEnvEnabled(OverlayEnv::kPitch, gates(false, true))); + CHECK(overlayEnvEnabled(OverlayEnv::kAmp, gates(false, false))); + // ...while the staged overlay additionally goes inert once the envelope is drawn: its + // nodes are no longer what the overlay is editing. + DeckEnableState drawn = gates(true, true); + drawn.ampSpline = true; + CHECK(overlayEnvInert(OverlayEnv::kAmp, drawn)); + CHECK(overlayEnvEnabled(OverlayEnv::kAmp, drawn)); +} + +// A deck knob goes inert exactly with its group's own enable toggle — including the filter's +// VELOCITY cell, which sits in the VELOCITY group visually but is a filter parameter and must +// go inert with the rest of the filter (the reachable-through-the-deck route mouseDownDeck +// checks before ever routing a curve-cell click to the popup). +static void testDeckKnobIsInertExactlyWithItsGroupsEnableToggle() { + CHECK(deckKnobInert(DeckParam::kFilterVelCurve, gates(/*pitchEnv=*/true, /*filter=*/false))); + CHECK(!deckKnobInert(DeckParam::kFilterVelCurve, gates(true, true))); + CHECK(deckKnobInert(DeckParam::kFilterCutoff, gates(true, false))); + CHECK(!deckKnobInert(DeckParam::kFilterCutoff, gates(true, true))); + CHECK(deckKnobInert(DeckParam::kPitchEnvDepth, gates(/*pitchEnv=*/false, true))); + CHECK(!deckKnobInert(DeckParam::kPitchEnvDepth, gates(true, true))); + // The amp's own velocity cell and every ordinary control are never inert here — inertness + // is a filter/pitch-env-group-only concept until an envelope is drawn. + CHECK(!deckKnobInert(DeckParam::kAmpVelCurve, gates(false, false))); + CHECK(!deckKnobInert(DeckParam::kAttack, gates(false, false))); +} + +// A drawn envelope's STAGED segment knobs go inert; the mode toggle itself and the depth knobs +// that scale either shape stay live. (Which segment knobs, per envelope, is pinned in +// spline_egs_tests alongside the rest of the spline rules.) +static void testAModeToggleIsNeitherLiveNorAnOverlayRadio() { + CHECK(deckParamCommit(DeckParam::kAmpEnvMode) == LiveCommit::Reload); + CHECK(deckParamCommit(DeckParam::kPitchEnvMode) == LiveCommit::Reload); + CHECK(deckParamCommit(DeckParam::kFilterEnvMode) == LiveCommit::Reload); + CHECK(overlayEnvForModeToggle(radio(DeckParam::kAmpEnvMode)) == OverlayEnv::kAmp); + CHECK(overlayEnvForModeToggle(radio(DeckParam::kPitchEnvMode)) == OverlayEnv::kPitch); + CHECK(overlayEnvForModeToggle(radio(DeckParam::kFilterEnvMode)) == OverlayEnv::kFilter); + // A mode toggle must not be mistaken for the overlay-select radio beside it. + CHECK(overlayEnvForRadio(radio(DeckParam::kAmpEnvMode)) == OverlayEnv::kNone); + CHECK(overlayEnvForModeToggle(radio(DeckParam::kAmpEnvSelect)) == OverlayEnv::kNone); +} + +int main() { + testOverlaySelectionIsExclusiveAcrossTheThreeEnvelopeDecks(); + testClickingTheActiveOverlayRadioClearsToNone(); + testANonRadioIdLeavesTheOverlaySelectionAlone(); + testOverlayIsInertExactlyWhenItsGroupToggleIsOff(); + testDeckKnobIsInertExactlyWithItsGroupsEnableToggle(); + testAModeToggleIsNeitherLiveNorAnOverlayRadio(); + testEveryDeckControlIsClassifiedIntoOneOfTheThreeCommitTiers(); + testOnlyALiveControlsDragTakesTheLiveTier(); + if (g_fail == 0) std::printf("deck_groups_state: all tests passed\n"); + return g_fail == 0 ? 0 : 1; +}