diff --git a/docs/product/instrument-control-surface.md b/docs/product/instrument-control-surface.md index 6e3ec1a..449d270 100644 --- a/docs/product/instrument-control-surface.md +++ b/docs/product/instrument-control-surface.md @@ -94,31 +94,43 @@ contours are row 2's first two groups. ### 1.2 The measured layout +> **Updated for Ω-W1-T4's filter-mod move and toggle conversion.** The table below was +> re-measured against the shipped `deck_groups.cpp`/`knob_deck.cpp` after two changes this +> track made: the filter's mod-depth cell moved from FILTER to FILTER ENV (one cell each +> way), and the four toggles that used to be two-segment (`Off\|On`, `Staged\|Spline` on +> PITCH ENV/FILTER ENV/AMP ENVELOPE, `Off\|On` on FILTER) are now the single-button +> `kEnable`/`kMode` styles — see `knob_deck.h`'s `DeckToggleStyle`. The per-deck corner +> radio these three envelope decks carried is gone (superseded by the click-to-focus +> overlay border, `core/instrument/CLAUDE.md`'s envelope-overlay section); MASTER alone +> keeps the corner slot, for its passive gain-reduction lamp. + `deckGroupWidth(g) = max(captionRowWidth, knobRowWidth) + 2·kDeckGroupPadX`, with -`captionRowWidth = captionWidth + Σ(kDeckToggleGap + 2·segWidth) + (radio ? 4 + 12 : 0)` -and `knobRowWidth = |cellIds|·kDeckCellW (+ 4 + 2·segWidth for a rowToggle)`. Metrics: -`kDeckCellW 60`, `kDeckCellH 74`, `kDeckKnobSize 40`, `kDeckCellLabelH 16`, -`kDeckCaptionH 20`, `kDeckToggleH 18`, `kDeckGroupPadX 6`, `kDeckGroupPadY 4`, -`kDeckCaptionGap 2`, `kDeckToggleGap 4`, `kDeckGroupGap 12`, `kDeckRowGap 8`, -`kDeckRadioSize 12`, `kDeckGroupH 104`. +`captionRowWidth = captionWidth + Σ(kDeckToggleGap + toggleWidth) + (radio ? kDeckToggleGap + +kDeckRadioSize : 0)` and `knobRowWidth = |cellIds|·kDeckCellW + (rowToggle ? kDeckToggleGap + +rowToggleWidth : 0)`. `toggleWidth` is the WHOLE control either way — a `kSegmented` toggle +just halves it into two segments at that width; a single-button `kEnable`/`kMode` toggle +draws the whole span as one button. Metrics: `kDeckCellW 60`, `kDeckCellH 74`, `kDeckKnobSize +40`, `kDeckCellLabelH 16`, `kDeckCaptionH 20`, `kDeckToggleH 18`, `kDeckGroupPadX 6`, +`kDeckGroupPadY 4`, `kDeckCaptionGap 2`, `kDeckToggleGap 4`, `kDeckGroupGap 12`, `kDeckRowGap +8`, `kDeckRadioSize 12`, `kDeckGroupH 104`, `kEnableBtnW 52`, `kEnvModeW 46`. | Group | Row | Caption run | Knob run | **Width** | Δ | Control inventory | |---|---|---|---|---|---|---| -| **PITCH/RATE** | 1 | 70 + 4 + 2·48 = 170 | 3 × 60 = **180** | **192** | +42 | 3 cells `Key Trk` / `Rate` / `Pitch`; caption toggle `Varisp\|Presrv` (48) | -| **FILTER** | 1 | 46 + 4 + 2·32 + 4 + 2·44 = 206 | 7 × 60 = **420** | **432** | −92 | 7 cells (morph, cutoff, Q, drive, mod amt, vel, key trk); caption toggle `Off\|On` (32); **caption toggle 2 `Band\|Notch` (44) — moved from the knob row** | +| **PITCH/RATE** | 1 | 70 + 4 + 2·48 = 170 | 3 × 60 = **180** | **192** | +42 | 3 cells `Key Trk` / `Rate` / `Pitch`; caption toggle `Varisp\|Presrv` (96, segmented) | +| **FILTER** | 1 | 46 + 4 + 52 + 4 + 2·44 = 194 | 6 × 60 = **360** | **372** | −60 vs Γ | 6 cells (morph, cutoff, Q, drive, vel, key trk) — **mod amt left for FILTER ENV**; caption toggle `Filter` (52, single-button ENABLE — was `Off\|On`); caption toggle 2 `Band\|Notch` (88, segmented) | | **VELOCITY** | 1 | 54 | 3 × 60 = **180** | **192** | 0 | 3 curve-popup cells (amp, pitch, filter) | -| **VOICE** | 1 | 38 + 4 + 2·40 = 122 | 60 + 4 + 2·44 = **152** | **164** | 0 | 1 cell (voice count); caption toggle `Poly\|Mono` (40); **row toggle `Retrig\|Legato` (44) stays** — see note | -| **PITCH ENV** | 2 | 58 + 4 + 64 + 4 + 46 + 4 + 12 = 192 | 4 × 60 = **240** | **252** | 0 | 4 cells (A, H, D, Depth); caption toggle `Off\|On`; caption toggle 2 `Staged\|Spline`; corner radio | -| **FILTER ENV** | 2 | 66 + 4 + 46 + 4 + 12 = 132 | 5 × 60 = **300** | **312** | 0 | 5 slots (Gate: A,H,D,S,R / Trigger: A,H,D + 2 reserves); caption toggle 2; corner radio | -| **AMP ENVELOPE** | 2 | 78 + 4 + 88 + 4 + 46 + 4 + 12 = 236 | 5 × 60 = **300** | **312** | 0 | 5 slots (Gate: A,H,D,S,R / Trigger: Len,A,H,D + 1 reserve); caption toggle `Gate\|Trig` (44); caption toggle 2; corner radio | +| **VOICE** | 1 | 38 + 4 + 2·40 = 122 | 60 + 4 + 2·44 = **152** | **164** | 0 | 1 cell (voice count); caption toggle `Poly\|Mono` (80, segmented); **row toggle `Retrig\|Legato` (88, segmented) stays** — see note | +| **PITCH ENV** | 2 | 58 + 4 + 52 + 4 + 46 = 164 | 4 × 60 = **240** | **252** | 0 | 4 cells (A, H, D, Depth); caption toggle `Envelope` (52, single-button ENABLE — was `Off\|On`); caption toggle 2 reads `Stage`/`Spline` (46, single-button MODE — was `Staged\|Spline`); **no corner radio** | +| **FILTER ENV** | 2 | 66 + 4 + 46 = 116 | 6 × 60 = **360** | **372** | +60 vs Γ | 6 slots (Gate: A,H,D,S,R,**Mod** / Trigger: A,H,D + 2 reserves,**Mod**) — **mod amt arrives from FILTER**; caption toggle 2 reads `Stage`/`Spline` (46, single-button MODE); **no corner radio**; no enable toggle of its own (FILTER's governs both) | +| **AMP ENVELOPE** | 2 | 78 + 4 + 88 + 4 + 46 = 220 | 5 × 60 = **300** | **312** | 0 | 5 slots (Gate: A,H,D,S,R / Trigger: Len,A,H,D + 1 reserve); caption toggle `Gate\|Trig` (88, segmented); caption toggle 2 reads `Stage`/`Spline` (46, single-button MODE — was `Staged\|Spline`); **no corner radio** | | **MASTER** | **1+2** | 46 + 4 + 2·32 + 4 + 12 = **130** | 60 + 8 + 62 = **130** | **142** | +70 | 1 cell (gain, upper-left); 1 **reserved** lower-left slot; caption toggle `Limiter Off\|On` (32); corner **bubble** (12, passive); **meter column 62 px, full double height** | **Row totals.** | | Natural content | Gutters at floor | **Row width** | |---|---|---|---| -| Row 1 | 192 + 432 + 192 + 164 = **980** | 16 + 16 + 16 = 48 | **1028** | -| Row 2 | 252 + 312 + 312 = **876** | 76 + 76 = 152 | **1028** | +| Row 1 | 192 + 372 + 192 + 164 = **920** | 36 + 36 + 36 = 108 | **1028** | +| Row 2 | 252 + 372 + 312 = **936** | 46 + 46 = 92 | **1028** | **Window floor.** diff --git a/src/core/instrument/ui/deck_groups.cpp b/src/core/instrument/ui/deck_groups.cpp index 0702138..2a85d22 100644 --- a/src/core/instrument/ui/deck_groups.cpp +++ b/src/core/instrument/ui/deck_groups.cpp @@ -18,8 +18,9 @@ double clamp(double v, double lo, double hi) { return v < lo ? lo : (v > hi ? hi constexpr int kEnvModeW = 46; // The single-button enables ("Envelope" / "Filter" / "Limiter"). One width because they are one -// control grammar. MASTER is the group that BINDS it: its caption row and knob row both measure -// 130, so anything past 64 widens kDeckSpanningW and spends the editor's width budget. +// control grammar. MASTER is the group that BINDS it: its knob row measures 130 (the caption +// row, at this width, measures 118), so anything past 64 widens kDeckSpanningW and spends the +// editor's width budget. constexpr int kEnableBtnW = 52; DeckToggleDesc enableButton(DeckParam p) { diff --git a/src/core/instrument/ui/sample_chrome.cpp b/src/core/instrument/ui/sample_chrome.cpp index 6dd5da4..9851178 100644 --- a/src/core/instrument/ui/sample_chrome.cpp +++ b/src/core/instrument/ui/sample_chrome.cpp @@ -18,7 +18,7 @@ constexpr int kStripBandHeight = 30; constexpr int kRunGap = 6; // between adjacent items of the toolbar run constexpr int kChanBtnW = 60; // fits the longer of the two mode labels ("Stereo") -constexpr int kChanSegH = 18; +constexpr int kChanBtnH = 18; // shared height of the Loop and channel-mode single buttons // If the title slot ever fails to hold its text at the editor's floor, THESE narrow — the // floor does not move. constexpr int kLoopBtnW = 52; @@ -54,14 +54,14 @@ ChromeRects chromeRects(const Rect& chrome, int knobSize) { r.navBrowse = Rect::ltrb(leftOf(navRight, kNavButtonWidth), navTop, navRight, navTop + navH); - const int chanTop = topFor(kChanSegH); + const int chanTop = topFor(kChanBtnH); const int chanRight = leftOf(r.navBrowse.x, kRunGap); r.channel = Rect::ltrb(leftOf(chanRight, kChanBtnW), chanTop, chanRight, - chanTop + kChanSegH); + chanTop + kChanBtnH); const int loopRight = leftOf(r.channel.x, kRunGap); r.loop = Rect::ltrb(leftOf(loopRight, kLoopBtnW), chanTop, loopRight, - chanTop + kChanSegH); + chanTop + kChanBtnH); const int cellH = std::min(row.height, knobSize + kVelLabelH); const int cellTop = topFor(cellH); diff --git a/src/shell/instrument/editor_input_chrome.cpp b/src/shell/instrument/editor_input_chrome.cpp index 45a4826..bcfc12a 100644 --- a/src/shell/instrument/editor_input_chrome.cpp +++ b/src/shell/instrument/editor_input_chrome.cpp @@ -78,8 +78,9 @@ bool ReaSamplerEditor::mouseDownChrome(const FaceLayout& fl, int x, int y) { // still offer to reverse. One button, so the click's meaning is "the other state": read the // current enable off the markers, which is the same source the button's paint reads. if (loopControlsLive() && contains(cr.loop, x, y)) { + // setLoopEnabled owns the frames <= 0 guard (empty capture) — no need to repeat it here. const auto frames = static_cast(monoPcmFor(selectedId_).size()); - if (frames > 0) setLoopEnabled(!pickedMarkers(frames).hasLoop); + setLoopEnabled(!pickedMarkers(frames).hasLoop); invalidate(); return true; } diff --git a/src/shell/instrument/editor_paint_chrome.cpp b/src/shell/instrument/editor_paint_chrome.cpp index 1c2ea79..aab8688 100644 --- a/src/shell/instrument/editor_paint_chrome.cpp +++ b/src/shell/instrument/editor_paint_chrome.cpp @@ -226,15 +226,35 @@ void ReaSamplerEditor::paintChrome(LICE_IBitmap* bmp, const FaceLayout& fl, bool : (active ? InteractionState::Active : (hov ? InteractionState::Hover : InteractionState::Rest)); fillSurface(bmp, toKitBox(r), Role::BgCell, st); + // Disabled's wash over bg/cell is barely a shade off Rest's — the third state needs + // its own mark, not just a slightly-quieter fill, so it draws the one outline this + // control ever gets. See PLAN.md's Off-vs-Disabled ruling: Off is live and + // clickable, Disabled is not, and the two must not read as the same thing. + if (disabled) { + LICE_DrawRect(bmp, r.x, r.y, r.width - 1, r.height - 1, + toLice(roleColor(Role::LineHairline)), 1.0f, 0); + } kitTextCentered(bmp, r, label, kToolbarFont, active && !disabled ? Role::BgBase : (hov ? Role::TextPrimary : Role::TextDim)); }; drawChromeToggle(cr.loop, "Loop", HoverKind::kLoop, marks.hasLoop, !loopControlsLive()); + + // The channel button is a MODE selector like the deck's Stage|Spline buttons — always + // "on", so InteractionState::Active would otherwise swallow every hover. Filled by + // accent/primary rather than bg/cell so Hover's mix-toward-accent/hot actually moves the + // surface instead of nudging bg/cell by a few percent (roleColorState's Active case + // always answers accent/primary regardless of the role passed in, so Rest is unreachable + // here and this is purely which color Hover mixes FROM). + const auto drawModeChromeToggle = [&](const Rect& r, const char* label, HoverKind hk) { + const bool hov = isHovered(hk, -1); + const InteractionState st = hov ? InteractionState::Hover : InteractionState::Active; + fillSurface(bmp, toKitBox(r), Role::AccentPrimary, st); + kitTextCentered(bmp, r, label, kToolbarFont, Role::BgBase); + }; const bool isStereo = (channelMode_ == ChannelMode::Stereo); - drawChromeToggle(cr.channel, isStereo ? "Stereo" : "Mono", HoverKind::kChannel, - /*active=*/true, /*disabled=*/false); + drawModeChromeToggle(cr.channel, isStereo ? "Stereo" : "Mono", HoverKind::kChannel); } // The strip row: the full 128-key piano with the root lit. The loaded capture responds diff --git a/src/shell/instrument/editor_paint_deck.cpp b/src/shell/instrument/editor_paint_deck.cpp index 644be76..acfdce6 100644 --- a/src/shell/instrument/editor_paint_deck.cpp +++ b/src/shell/instrument/editor_paint_deck.cpp @@ -135,10 +135,13 @@ void ReaSamplerEditor::paintDeck(LICE_IBitmap* bmp, const FaceLayout& fl) { disabled ? Role::TextDim : (seg1Active ? Role::BgBase : Role::TextPrimary)); }; - // The single-button forms. ENABLE names what it controls and reads Primary on / dim gray - // off; MODE reads the current mode and has no off state. A control the MODE refuses draws - // Disabled, which is a third state and not a synonym for off: off is live and clickable, - // and the washed Disabled surface is what separates them. + // The single-button ENABLE form. Reads Primary on / dim gray off; a control the ENABLE + // gates draws Disabled, a third state and not a synonym for off (off is live and + // clickable). No deck ENABLE currently drives `disabled` true through this path — the + // deck's one Disabled control (FILTER's Band|Notch law when the filter itself is off) + // stays segmented and draws through `drawToggle` above — but the parameter stays for + // parity with that segmented form and because a future ENABLE could plausibly gate on + // something else the way Loop (chrome) already does outside this deck. const auto drawButtonToggle = [&](const DeckToggleLayout& t, const char* label, bool active, bool disabled) { const bool hov = !disabled && isHovered(HoverKind::kControl, t.id); @@ -151,9 +154,19 @@ void ReaSamplerEditor::paintDeck(LICE_IBitmap* bmp, const FaceLayout& fl) { active && !disabled ? Role::BgBase : (hov ? Role::TextPrimary : Role::TextDim)); }; + // The single-button MODE form: the label reads the current mode, so there is no off state + // and no dim-gray rest — it is always "on". Filled by accent/primary rather than routed + // through drawButtonToggle (which would force InteractionState::Active regardless of hov, + // leaving MODE with no hover cue at all): Hover mixes toward accent/hot FROM whatever base + // roleColorState is handed, so passing accent/primary here is what lets the already-lit + // button visibly brighten on hover instead of a hover mix nobody would notice against + // bg/cell's dark base. const auto drawModeToggle = [&](const DeckToggleLayout& t, EnvMode mode) { - drawButtonToggle(t, mode == EnvMode::Spline ? "Spline" : "Stage", /*active=*/true, - /*disabled=*/false); + const char* label = mode == EnvMode::Spline ? "Spline" : "Stage"; + const bool hov = isHovered(HoverKind::kControl, t.id); + const InteractionState st = hov ? InteractionState::Hover : InteractionState::Active; + fillSurface(bmp, toKitBox(t.seg0), Role::AccentPrimary, st); + kitTextCentered(bmp, t.seg0, label, Font::Micro, Role::BgBase); }; const bool anySpline = splineActive(play); @@ -224,8 +237,9 @@ void ReaSamplerEditor::paintDeck(LICE_IBitmap* bmp, const FaceLayout& fl) { } kitText(bmp, g.caption, caption, Font::Micro, Role::TextDim); - // The gain-reduction lamp. ROUND, where the overlay radios in this same slot are - // square, so it reads as a lamp rather than a control. + // The gain-reduction lamp — MASTER's own corner slot, the one the env decks' overlay + // radios used to share before focus-by-click replaced them. ROUND, not square, so it + // reads as a passive readout rather than a control. if (g.captionRadio.id >= 0 && g.captionRadio.passive) { const Rect& rb = g.captionRadio.box; const float r = rb.width / 2.0f - 0.5f; diff --git a/tests/test_deck_groups_measured.cpp b/tests/test_deck_groups_measured.cpp index d5c5f45..dd71071 100644 --- a/tests/test_deck_groups_measured.cpp +++ b/tests/test_deck_groups_measured.cpp @@ -422,8 +422,8 @@ static void testEnvModeCeilingsArePinnedForPitchEnvAndAmp() { // 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 -// contour row's 876 a constant rather than a Gate-only fact. +// reserve slots hold the two mode-dependent groups at 372 (FILTER ENV) and 312 (AMP ENVELOPE) +// either way, which is what makes the contour row's 936 a constant rather than a Gate-only fact. static void testEveryGroupWidthMatchesTheMeasuredLayout() { const struct { int id; int width; } want[] = { {kGroupPitch, 192}, {kGroupPitchEnv, 252}, {kGroupFilter, 372}, @@ -477,6 +477,7 @@ static void testTheGateFaceIsPixelIdenticalApartFromTheTwoFilterGroups() { {kGroupPitchEnv, 0, 252, 4}, // unchanged {kGroupFilterEnv, 298, 372, 6}, // was x=328 w=312 with 5 cells {kGroupAmpEnv, 716, 312, 5}, // unchanged + {kGroupMaster, 1040, 142, 1}, // unchanged — the claim above actually pins it }; for (const auto& w : want) { const DeckGroupLayout& l = lay(w.id); diff --git a/tests/test_deck_values.cpp b/tests/test_deck_values.cpp index dd31524..09ccadb 100644 --- a/tests/test_deck_values.cpp +++ b/tests/test_deck_values.cpp @@ -6,6 +6,7 @@ #include "../src/core/instrument/ui/deck_values.h" #include "../src/core/instrument/engine/master_gain.h" +#include "../src/core/instrument/ui/deck_groups.h" #include #include @@ -450,9 +451,34 @@ static void testASingleButtonsDerivedSegmentFlipsTheFieldItNames() { } } +// The cross-check the hand-maintained list above cannot catch: a control RE-STYLED to a +// single button (kEnable/kMode) with no nextToggleSegment entry silently commits segment -1, +// which setDeckParam reads as "off" — a latch, not a toggle. Swept over every group +// sampleDeckGroups actually ships, in both play modes, rather than a fixed id list, so a +// future re-style is caught the moment it lands here with no entry above. kLimiterEnable is +// the one shipped kEnable that is excluded: it lives on `InstrumentParams::limiterEnabled`, +// outside `PlaySeconds`, and commits through its own handler (editor_input_deck.cpp) rather +// than through nextToggleSegment/setDeckParam at all. +static void testEveryShippedSingleButtonToggleHasADerivedSegment() { + const PlaySeconds p; + for (PlayMode mode : {PlayMode::Gate, PlayMode::Trigger}) { + for (const DeckGroupDesc& g : sampleDeckGroups(mode)) { + for (const DeckToggleDesc* t : {&g.captionToggle, &g.captionToggle2, &g.rowToggle}) { + if (t->id < 0) continue; + if (t->style != DeckToggleStyle::kEnable && t->style != DeckToggleStyle::kMode) + continue; + const DeckParam id = static_cast(t->id); + if (id == DeckParam::kLimiterEnable) continue; + CHECK(nextToggleSegment(id, p) != -1); + } + } + } +} + int main() { testTheTwoCeilingNamesAreOneNumber(); testASingleButtonsDerivedSegmentFlipsTheFieldItNames(); + testEveryShippedSingleButtonToggleHasADerivedSegment(); testNormRoundTripsThroughEveryValueDomain(); testRateKnobEndsAreTheStretchersOwnBounds(); testRateAndPitchBindTheirOwnFields();