daniel/reasampler
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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.

Browse Source
This commit is contained in:
Daniel Harvey
2026-08-02 08:45:35 -04:00
parent df10ddacc2
commit 41876674e4
5 changed files with 304 additions and 231 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/test_deck_groups.cpp
+56 -218
View File
@@ -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<DeckGroupDesc> 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<DeckGroupId>(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 = &gl;
prevRow = row;
prevInRow[r] = &gl;
}
const auto right = [&](int id) {
return dl.groups[static_cast<std::size_t>(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<DeckGroupDesc> g = sampleDeckGroups(PlayMode::Gate);
DeckGroupDesc probe = g[static_cast<std::size_t>(indexOfGroup(g, kGroupMaster))];
probe.captionWidth += 40; // unbalances it: the caption row now measures past the knob row
const std::vector<DeckGroupDesc> 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<DeckGroupDesc> 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<int>(DeckParam::kCount); ++i) {
const DeckParam p = static_cast<DeckParam>(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<int>(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<int>(DeckParam::kFilterCutoff)) ==
LiveCommit::Reload);
}
// --- The overlay selection state machine ---------------------------------------
static int radio(DeckParam p) { return static_cast<int>(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<DeckGroupDesc> g = sampleDeckGroups(PlayMode::Gate);
const DeckGroupDesc& penv = g[static_cast<std::size_t>(indexOfGroup(g, kGroupPitchEnv))];
const DeckGroupDesc& amp = g[static_cast<std::size_t>(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");