From abb27f08f2e9b05e4ebb6b545cece2ce6e0b3172 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sat, 1 Aug 2026 18:44:01 -0400 Subject: [PATCH] Raise the editor floor to 1190x680, derived from the deck's declared width budget, and make row membership a property of the group --- src/core/instrument/CLAUDE.md | 2 +- src/core/instrument/ui/deck_groups.cpp | 23 +++++ src/core/instrument/ui/deck_groups.h | 10 ++ src/core/instrument/ui/knob_deck.h | 18 +++- src/core/instrument/ui/sample_bands.h | 7 +- tests/test_deck_groups.cpp | 122 +++++++++++++++++++++---- 6 files changed, 159 insertions(+), 23 deletions(-) diff --git a/src/core/instrument/CLAUDE.md b/src/core/instrument/CLAUDE.md index 272c14a..d682f10 100644 --- a/src/core/instrument/CLAUDE.md +++ b/src/core/instrument/CLAUDE.md @@ -314,7 +314,7 @@ anything for a trigger shape. - `browser_scroll` — scroll + type-to-filter layered over `capture_browser`: vertical scroll offset, scrollbar thumb, thumb-drag mapping, and name-substring search. - `param_slider` — parameter control-panel: vertical stack of TOGGLE (two-segment selector) and SLIDER (horizontal track) rows; maps normalized value to/from handle pixel. - `embed_strip` — compact single-row control layout for embed mode in the track FX chain. -- `knob_deck` — pure knob-deck layout + hit-test (FB1): group-box / caption-row / compact-toggle / knob-cell geometry, deterministic whole-group wrap, `DeckLayout` / `DeckHit`. Mirror of `action_bar`/`param_slider`; no LICE or REAPER types. Carries a SECOND hit-test, `hitTestKnobFace`, resolved against the drawn CIRCLES rather than the cell: a double-click reset is aimed at a dial, so the label band and the cell margins must miss where a drag grab deliberately does not, and only a radial resolve can tell the inner curve dial from the outer ring it sits inside. **The cell/knob/label sizes and `sample_bands`' editor floor move as a pair** — wider cells need a wider floor width or the deck wraps to a fourth row. A group carries TWO caption-toggle slots, laid right-to-left: the second exists because a group whose knob row is wider than its caption row has caption slack a toggle can occupy for free, and the deck has fourteen pixels of headroom on its first row at the editor's floor width — a `rowToggle` would widen the GROUP and wrap the deck to a fourth row, past what the minimum window holds. **A group's cell run is a RESERVED WIDTH, not a fixed cell size**: a `-1` id reserves one cell's width without a cell, and the cells present divide the whole run between them at one uniform integer width (residue in symmetric end margins). That is what lets a mode flip drop controls from a face — Trigger's AMP and FILTER ENV lose their Sustain/Release stages — without either reflowing the deck or leaving dead slots in the box; a face with fewer controls simply gets roomier cells. Do not reintroduce fixed-width cells with blank slots. +- `knob_deck` — pure knob-deck layout + hit-test (FB1): group-box / caption-row / compact-toggle / knob-cell geometry, deterministic whole-group wrap, `DeckLayout` / `DeckHit`. Mirror of `action_bar`/`param_slider`; no LICE or REAPER types. Carries a SECOND hit-test, `hitTestKnobFace`, resolved against the drawn CIRCLES rather than the cell: a double-click reset is aimed at a dial, so the label band and the cell margins must miss where a drag grab deliberately does not, and only a radial resolve can tell the inner curve dial from the outer ring it sits inside. The deck's width budget at the editor's floor — the row block, the spanning deck's reserve, the ceiling, and what drives the floor — is declared and reasoned at the constants themselves (`knob_deck.h`); every group's categorical row is `deck_groups`' `deckRowFor`. A group carries TWO caption-toggle slots, laid right-to-left: the second exists because a group whose knob row is wider than its caption row has caption slack a toggle can occupy for free, where a `rowToggle` widens the GROUP and is charged against that budget — which is why the env decks' mode toggles ride the caption row. **A group's cell run is a RESERVED WIDTH, not a fixed cell size**: a `-1` id reserves one cell's width without a cell, and the cells present divide the whole run between them at one uniform integer width (residue in symmetric end margins). That is what lets a mode flip drop controls from a face — Trigger's AMP and FILTER ENV lose their Sustain/Release stages — without either reflowing the deck or leaving dead slots in the box; a face with fewer controls simply gets roomier cells. Do not reintroduce fixed-width cells with blank slots. - `deck_values` — the deck's control-id ↔ parameter-set BINDING and its display units, split from the editor shell on the same axis `deck_groups` was split from `knob_deck`: `deck_groups` says which controls exist, this says what each one's value MEANS. Holds `deckParamNorm` / diff --git a/src/core/instrument/ui/deck_groups.cpp b/src/core/instrument/ui/deck_groups.cpp index 28de21f..a1401cf 100644 --- a/src/core/instrument/ui/deck_groups.cpp +++ b/src/core/instrument/ui/deck_groups.cpp @@ -130,6 +130,29 @@ std::vector sampleDeckGroups(PlayMode playMode) { return out; } +DeckRow deckRowFor(DeckGroupId group) { + // Every enumerator listed and no default, on the same gate isLiveDeckParam below relies on. + switch (group) { + case kGroupPitch: + case kGroupFilter: + case kGroupVelocity: + case kGroupVoice: + return DeckRow::Sound; + case kGroupPitchEnv: + case kGroupFilterEnv: + case kGroupAmpEnv: + return DeckRow::Contour; + case kGroupMaster: + return DeckRow::Spanning; + } + // Unreachable for a valid enumerator, and Spanning rather than Sound ON PURPOSE: the + // -Wswitch gate is compiler-dependent, so on a toolchain that does not raise it a dropped + // case arm falls here instead. Sound is what a new group most plausibly IS, which would + // make the fall-through invisible; Spanning is the one row nothing may silently join, so + // the tests' partition count catches it. + return DeckRow::Spanning; +} + CurveTarget curveTargetFor(int controlId) { switch (static_cast(controlId)) { case DeckParam::kAmpVelCurve: return CurveTarget::kAmp; diff --git a/src/core/instrument/ui/deck_groups.h b/src/core/instrument/ui/deck_groups.h index f338102..6a76cbf 100644 --- a/src/core/instrument/ui/deck_groups.h +++ b/src/core/instrument/ui/deck_groups.h @@ -101,6 +101,16 @@ enum DeckGroupId { kGroupMaster, }; +// The deck's two categorical rows, plus the row-spanning bus deck. Sound is what the voice +// IS, Contour is how it moves over time, Spanning is what happens after the mixer. +enum class DeckRow { Sound, Contour, Spanning }; + +// Which row a group belongs to. Membership is a property of the GROUP; width is a property of +// its descriptor — separating them is what lets the row law be settled while the descriptors +// are still moving. Total over DeckGroupId by an exhaustive switch with no default, so a group +// added without a row cannot silently become Sound. +DeckRow deckRowFor(DeckGroupId group); + // Which velocity curve a deck cell edits, or kNone when the control is an ordinary knob. THE // one place a control id resolves to a curve target — paint (draw a curve thumbnail, not a // dial) and hit-test (open a popup, not start a drag) both read this predicate rather than diff --git a/src/core/instrument/ui/knob_deck.h b/src/core/instrument/ui/knob_deck.h index f13b995..44fd38f 100644 --- a/src/core/instrument/ui/knob_deck.h +++ b/src/core/instrument/ui/knob_deck.h @@ -23,8 +23,9 @@ namespace reasampler::instrument::ui { // Fixed deck metrics, exposed so the shell and tests agree. The cell/knob/label sizes were -// raised together for legibility at high pixel densities; the editor's floor width -// (sample_bands) is what absorbs the wider cells, so the two move as a pair. +// raised together for legibility at high pixel densities. The deck's cell metrics AND its +// group/row composition BOTH drive sample_bands' kEditorMinWidth; none of the three may move +// alone. inline constexpr int kDeckCellW = 60; // one knob cell inline constexpr int kDeckCellH = 74; inline constexpr int kDeckKnobSize = 40; // knob diameter inside the cell @@ -46,6 +47,19 @@ inline constexpr int kDeckInnerDialSize = 20; inline constexpr int kDeckGroupH = kDeckGroupPadY + kDeckCaptionH + kDeckCaptionGap + kDeckCellH + kDeckGroupPadY; +// --- The deck's width budget at the editor's floor ------------------------------------ +// DECLARATIONS of budget, not measurements: nothing here is computed from a descriptor, and a +// group inventory that overruns one is what fails. sample_bands' kEditorMinWidth is derived +// from the first two — kDeckRowBlockW + kDeckGroupGap + kDeckSpanningW + 2*kPad — and the +// identity is asserted in test_deck_groups.cpp rather than coded, so the allocator keeps no +// include edge to this header. +inline constexpr int kDeckRowBlockW = 1020; // the block both categorical rows justify inside +inline constexpr int kDeckSpanningW = 142; // the right-anchored spanning deck, outside the block +// The hard ceiling the FLOOR may not exceed; the window itself still grows freely above it. +// The gap between it and kEditorMinWidth is the whole width budget for the life of this +// layout — see instrument-control-surface.md §1.6 before spending any of it. +inline constexpr int kEditorCeilingWidth = 1280; + // A two-segment compact toggle (always 2 segments — the Mono/Stereo grammar). id -1 = absent. struct DeckToggleDesc { int id = -1; // shell control id returned by the hit-test; -1 = no toggle diff --git a/src/core/instrument/ui/sample_bands.h b/src/core/instrument/ui/sample_bands.h index 3210520..564c0ae 100644 --- a/src/core/instrument/ui/sample_bands.h +++ b/src/core/instrument/ui/sample_bands.h @@ -16,7 +16,12 @@ inline constexpr int kPad = 8; // exactly this, and there is no scroll, so anything smaller pushes the deck band off the // window bottom (computeSampleBands' waveform-floor-wins degrade). Growing is fine — the // waveform band is the elastic one. Both the enforced minimum and the opening rect read this. -inline constexpr int kEditorMinWidth = 980; +// +// The width is a LITERAL here on purpose, though it is derived from knob_deck's width budget: +// this allocator is deliberately independent of the deck (it takes deckHeight as a parameter +// for exactly that reason), so the derivation is asserted in test_deck_groups.cpp — the one +// place that already includes both headers — rather than coded as an include edge. +inline constexpr int kEditorMinWidth = 1190; inline constexpr int kEditorMinHeight = 680; // Chrome band: the toolbar row (title + nav) stacked over the control row (piano strip, diff --git a/tests/test_deck_groups.cpp b/tests/test_deck_groups.cpp index 2c4c30b..5069705 100644 --- a/tests/test_deck_groups.cpp +++ b/tests/test_deck_groups.cpp @@ -3,7 +3,8 @@ // descriptors the Sample face carries: the signal-flow group order (pitch -> filter -> amp), // the Filter group's contents, the VELOCITY group's exclusive ownership of the three curve // cells and its placement immediately left of VOICE, the wrapped deck height at the editor's -// floor width and its fit inside the floor window, the pinned Gate widths and row assignment, +// 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 @@ -238,13 +239,14 @@ static void testAmpGroupWidthSurvivesAGateTriggerFlip() { static void testWrappedDeckHeightAtTheEditorFloorWidth() { const std::vector g = sampleDeckGroups(PlayMode::Gate); - // At the floor (== default) 980 the deck takes three rows: PITCH + PITCH ENV + FILTER fill - // the first (950 of the 964 available — fourteen px of headroom, so one more FILTER cell - // would wrap the group and reflow everything under it), FILTER ENV + AMP + VELOCITY the - // second, VOICE + MASTER the third. Two rows cannot hold the eight groups in ANY order at - // this width: 1978 px of group plus 72 px of gaps against a 1928 px two-row capacity. - CHECK(deckRowCount(g, kAvailAtMinWidth) == 3); - CHECK(deckHeight(g, kAvailAtMinWidth) == 3 * kDeckGroupH + 2 * kDeckRowGap); + // An UPPER BOUND, not an equality. The greedy whole-group wrap is still what decides row + // membership until the reflow replaces it with the categorical partition, and at this width + // it happens to pack two ragged rows with the wrong composition. Bounding it is a real + // regression canary — a third row would cost the waveform 112 px again — without turning a + // wrap outcome into a claim. + const int rows = deckRowCount(g, kAvailAtMinWidth); + CHECK(rows <= 2); + CHECK(deckHeight(g, kAvailAtMinWidth) == rows * kDeckGroupH + (rows - 1) * kDeckRowGap); // Whole groups only, never split: every group's box lies inside the available width or is // the first of its row. @@ -265,8 +267,13 @@ static void testDeckFitsInsideTheEnforcedMinimumWindow() { const std::vector g = sampleDeckGroups(mode); const int h = deckHeight(g, kAvailAtMinWidth); const SampleBands b = computeSampleBands(kEditorMinWidth, kEditorMinHeight, h); - CHECK(deckRowCount(g, kAvailAtMinWidth) == 3); // either face, three rows at the floor + CHECK(deckRowCount(g, kAvailAtMinWidth) <= 2); // either face; see the bound above CHECK(b.decks.height == h); + // The raised floor hands the waveform the reflow's 112 px two waves early: at two rows + // the deck band is 216 and the waveform 358, against 328/246 before. Bounded rather + // than pinned for the same reason the row count is. + CHECK(b.decks.height <= 2 * kDeckGroupH + kDeckRowGap); + CHECK(b.waveform.height >= 358); // Bottom-anchored INSIDE the pad is the whole assertion: the degrade path pushes the // deck down until the waveform hits its floor, so any deck too tall to fit stops // landing on this exact line. A `<= kEditorMinHeight` bound would not catch it — the @@ -276,6 +283,75 @@ static void testDeckFitsInsideTheEnforcedMinimumWindow() { } } +// The floor is a DERIVED number, and this is the one place the derivation is written down — +// sample_bands stays independent of knob_deck, so neither header can hold it. This fixture is +// the only one that includes both. +static void testTheEditorFloorIsDerivedFromTheDeckWidthBudget() { + CHECK(kDeckRowBlockW + kDeckGroupGap + kDeckSpanningW + 2 * kPad == kEditorMinWidth); + // The budget: what is left between the derived floor and the hard ceiling, and it is spent + // once. A cell costs 60 of it. + CHECK(kEditorCeilingWidth - kEditorMinWidth == 90); + // The reflow's 112 px goes entirely to the waveform, so the height does not move. + CHECK(kEditorMinHeight == 680); +} + +static void testEveryDeckGroupBelongsToExactlyOneRow() { + CHECK(deckRowFor(kGroupPitch) == DeckRow::Sound); + CHECK(deckRowFor(kGroupFilter) == DeckRow::Sound); + CHECK(deckRowFor(kGroupVelocity) == DeckRow::Sound); + CHECK(deckRowFor(kGroupVoice) == DeckRow::Sound); + CHECK(deckRowFor(kGroupPitchEnv) == DeckRow::Contour); + CHECK(deckRowFor(kGroupFilterEnv) == DeckRow::Contour); + CHECK(deckRowFor(kGroupAmpEnv) == DeckRow::Contour); + CHECK(deckRowFor(kGroupMaster) == DeckRow::Spanning); + + // Totality against the descriptor list the deck actually carries, not just against the + // enum: a group that shipped without a row would land here as a miscount. + for (PlayMode mode : {PlayMode::Gate, PlayMode::Trigger}) { + int sound = 0, contour = 0, spanning = 0; + for (const DeckGroupDesc& d : sampleDeckGroups(mode)) { + switch (deckRowFor(static_cast(d.id))) { + case DeckRow::Sound: ++sound; break; + case DeckRow::Contour: ++contour; break; + case DeckRow::Spanning: ++spanning; break; + } + } + CHECK(sound == 4 && contour == 3 && spanning == 1); + } +} + +// What the budget can already be measured against. The contour row fits today and MASTER has +// not touched its reserve; the SOUND row does not fit yet and must not be forced to — it is +// 1030 against the 1020 block, and the 50 px deficit is exactly what two later descriptor +// changes buy: PITCH becoming PITCH/RATE (+42) and FILTER's Band|Notch moving from the knob +// row to the caption corner (−92), netting 980. The fit is asserted when they land, not here. +static void testTheContourRowAndTheSpanningDeckFitTheBudget() { + for (PlayMode mode : {PlayMode::Gate, PlayMode::Trigger}) { + const std::vector g = sampleDeckGroups(mode); + int contourWidth = 0, contourGroups = 0, spanningWidth = 0; + for (const DeckGroupDesc& d : g) { + const DeckRow row = deckRowFor(static_cast(d.id)); + if (row == DeckRow::Contour) { + contourWidth += deckGroupWidth(d); + ++contourGroups; + } else if (row == DeckRow::Spanning) { + spanningWidth += deckGroupWidth(d); + } + } + // 252 + 312 + 312. Mode-stable because FILTER ENV's and AMP's reserve slots hold them + // at 312 in Trigger as well as Gate. + CHECK(contourGroups == 3); + CHECK(contourWidth == 876); + CHECK(contourWidth <= kDeckRowBlockW); + // Slack enough that neither of the row's two gutters falls under the minimum. + CHECK(kDeckRowBlockW - contourWidth >= (contourGroups - 1) * kDeckGroupGap); + // MASTER is 72 today against a 142 reserve: the double-height interior it grows into is + // budgeted for, not yet spent. + CHECK(spanningWidth == 72); + CHECK(spanningWidth <= kDeckSpanningW); + } +} + static void testHitTestResolvesTheNewFilterControls() { const std::vector g = sampleDeckGroups(PlayMode::Gate); const DeckLayout dl = layoutDeck(g, kPad, 40, kAvailAtMinWidth); @@ -568,15 +644,17 @@ static void testNoFaceLeavesSlackWhereItsDroppedControlsWere() { // The "residue lands in symmetric end margins" rule is knob_deck's own (layoutGroup), pinned // once by its synthetic residue>=2 fixture in test_knob_deck.cpp rather than restated here. -// Gate is the common face and it already packs correctly: pin its group widths and row -// assignment at the floor so a later edit anywhere in the deck cannot reflow it silently. -// (Measured from the shipped descriptors, not copied out of a failing run.) -static void testGateModeWidthsAndRowAssignmentAreUnchanged() { +// Gate is the common face and its group widths are what the width budget is spent against: +// pin them at the floor so a later edit anywhere in the deck cannot move one silently. +// (Measured from the shipped descriptors, not copied out of a failing run.) The WRAP row a +// group lands on is deliberately NOT pinned — that is the interim greedy pack the reflow +// replaces, and deckRowFor is where row membership is asserted. +static void testGateModeGroupWidthsAreUnchanged() { const std::vector g = sampleDeckGroups(PlayMode::Gate); - const struct { int id; int width; int row; } want[] = { - {kGroupPitch, 150, 0}, {kGroupPitchEnv, 252, 0}, {kGroupFilter, 524, 0}, - {kGroupFilterEnv, 312, 1}, {kGroupAmpEnv, 312, 1}, {kGroupVelocity, 192, 1}, - {kGroupVoice, 164, 2}, {kGroupMaster, 72, 2}, + const struct { int id; int width; } want[] = { + {kGroupPitch, 150}, {kGroupPitchEnv, 252}, {kGroupFilter, 524}, + {kGroupFilterEnv, 312}, {kGroupAmpEnv, 312}, {kGroupVelocity, 192}, + {kGroupVoice, 164}, {kGroupMaster, 72}, }; CHECK(g.size() == sizeof(want) / sizeof(want[0])); const DeckLayout dl = layoutDeck(g, kPad, 0, kAvailAtMinWidth); @@ -584,7 +662,10 @@ static void testGateModeWidthsAndRowAssignmentAreUnchanged() { CHECK(dl.groups[i].id == want[i].id); CHECK(deckGroupWidth(g[i]) == want[i].width); CHECK(dl.groups[i].box.width == want[i].width); - CHECK(dl.groups[i].box.y == want[i].row * (kDeckGroupH + kDeckRowGap)); + // Every box lands on a row line, and no lower than the second — the same two-row + // bound the deck height carries. + CHECK(dl.groups[i].box.y % (kDeckGroupH + kDeckRowGap) == 0); + CHECK(dl.groups[i].box.y <= kDeckGroupH + kDeckRowGap); // Gate carries no reserves, so its cells are the deck's base size. for (const DeckCellLayout& c : dl.groups[i].cells) CHECK(c.cell.width == kDeckCellW); } @@ -669,8 +750,11 @@ int main() { testWrappedDeckHeightAtTheEditorFloorWidth(); testDeckFitsInsideTheEnforcedMinimumWindow(); testNoFaceLeavesSlackWhereItsDroppedControlsWere(); - testGateModeWidthsAndRowAssignmentAreUnchanged(); + testGateModeGroupWidthsAreUnchanged(); testGateSplineGateRoundTripsToTheSameLayout(); + testTheEditorFloorIsDerivedFromTheDeckWidthBudget(); + testEveryDeckGroupBelongsToExactlyOneRow(); + testTheContourRowAndTheSpanningDeckFitTheBudget(); testHitTestResolvesTheNewFilterControls(); testBipolarKnobLawRoundTripsAndIsExactAtCentre(); if (g_fail == 0) std::printf("deck_groups: all tests passed\n");