fix(instrument-ui): distinguish Disabled from Off, add hover cue to mode toggles, close review minors

Adds a hairline outline for Disabled chrome buttons, resolves Hover on the four
mode-selector single-buttons, re-measures the control-surface doc's 1.2 layout
table post-reflow, and adds a structural test pinning every enable/mode toggle
to a derived segment.

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.**
 

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) {

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);

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<std::int64_t>(monoPcmFor(selectedId_).size());
-        if (frames > 0) setLoopEnabled(!pickedMarkers(frames).hasLoop);
+        setLoopEnabled(!pickedMarkers(frames).hasLoop);
         invalidate();
         return true;
     }

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

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;

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);

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 <cmath>
 #include <cstdio>
@@ -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<DeckParam>(t->id);
+                if (id == DeckParam::kLimiterEnable) continue;
+                CHECK(nextToggleSegment(id, p) != -1);
+            }
+        }
+    }
+}
+
 int main() {
     testTheTwoCeilingNamesAreOneNumber();
     testASingleButtonsDerivedSegmentFlipsTheFieldItNames();
+    testEveryShippedSingleButtonToggleHasADerivedSegment();
     testNormRoundTripsThroughEveryValueDomain();
     testRateKnobEndsAreTheStretchersOwnBounds();
     testRateAndPitchBindTheirOwnFields();