diff options
| -rw-r--r-- | crates/core/src/context_menu.rs | 238 | ||||
| -rw-r--r-- | crates/wayland/src/decoration.rs | 110 | ||||
| -rw-r--r-- | crates/wayland/src/decoration/tests.rs | 68 | ||||
| -rw-r--r-- | crates/wayland/src/input/pointer.rs | 17 | ||||
| -rw-r--r-- | crates/wayland/src/state/desktop_icons.rs | 18 | ||||
| -rw-r--r-- | crates/wayland/src/state/menu.rs | 63 | ||||
| -rw-r--r-- | crates/x11/src/platform/actions.rs | 2 | ||||
| -rw-r--r-- | crates/x11/src/platform/context_menu.rs | 43 | ||||
| -rw-r--r-- | crates/x11/src/platform/events.rs | 2 | ||||
| -rw-r--r-- | docs/TODO.md | 16 |
10 files changed, 463 insertions, 114 deletions
diff --git a/crates/core/src/context_menu.rs b/crates/core/src/context_menu.rs index 30b1e8f..f56c7b9 100644 --- a/crates/core/src/context_menu.rs +++ b/crates/core/src/context_menu.rs @@ -10,8 +10,18 @@ //! drifting copy of the same logic - this data has no Wayland-specific //! content at all, so duplicating it would just be two copies of the same //! bug waiting to happen. +//! +//! Redesigned after being reported live as "looks very ugly and some of +//! it doesn't make sense": every row, including a separator, used to be a +//! full `TITLEBAR_HEIGHT`-tall slot (a hairline sitting in the middle of +//! 32px of empty space), and "Move to Workspace" faked a section header +//! by embedding box-drawing characters directly in a clickable-looking +//! item label (`"─── Move to Workspace ───"`) rather than being a real, +//! non-interactive row. Both are fixed below: separators and headers get +//! their own, much smaller row heights, and [`MenuAction::Header`] is a +//! real non-clickable row type instead of a label-text hack. -use crate::{WindowManager, WindowId, WorkspaceId, TITLEBAR_HEIGHT}; +use crate::{WindowManager, WindowId, WorkspaceId}; #[derive(Clone, Copy)] pub enum MenuAction { @@ -21,13 +31,41 @@ pub enum MenuAction { ToggleFloating, ToggleAlwaysOnTop, MoveToWorkspace(WorkspaceId), + /// Cycles `ThemeConfig::traffic_light_buttons` - the same live knob + /// `srd set button_style` flips, but applied here with an immediate + /// redraw of every open window's titlebar (see `run_context_menu_ + /// action`'s own doc comment for why that's only possible from + /// backend-specific code, not the generic IPC path). A theme setting + /// is shared across every window by definition, not a per-window + /// property - cycling it from one window's menu restyles all of + /// them, the same way changing it in a settings app would. + CycleButtonStyle, + /// Cycles `ThemeConfig::buttons_left` - same shape as `CycleButtonStyle`. + CycleButtonSide, Close, - /// Not a real action - a purely visual divider row. `row_at` still - /// resolves a click on one to `Some(index)` (it occupies a real row, - /// same as any other), so the dispatch site is what actually no-ops - /// on it, same "the row exists but does nothing" contract a real - /// desktop's own menu separators have. + /// A purely visual divider row - no label, no click behaviour. `row_ + /// at` still resolves a click on one to `Some(index)` (it occupies + /// real space, same as any other row), so the dispatch site is what + /// actually no-ops on it, same "the row exists but does nothing" + /// contract a real desktop's own menu separators have. Separator, + /// A non-interactive section label (`"Move to Workspace"`, `"Customize"`) + /// - dimmer, smaller text, never highlighted, click is a no-op just + /// like [`Self::Separator`]. Replaces an earlier hack that embedded + /// box-drawing characters directly in an ordinary item's label, which + /// rendered (and behaved, right up until the dispatch site's own + /// special-case) exactly like a clickable row that happened to do + /// nothing - confusing on both counts. + Header, +} + +impl MenuAction { + /// Whether this row can ever be the target of a real click - shared + /// by both backends' click-dispatch sites so `Separator`/`Header` + /// can't drift out of sync with each other on which rows are inert. + pub fn is_interactive(&self) -> bool { + !matches!(self, MenuAction::Separator | MenuAction::Header) + } } pub struct ContextMenu { @@ -36,11 +74,29 @@ pub struct ContextMenu { /// `Window.geometry` and every other rendered element's position uses. pub pos: (i32, i32), pub width: u32, + /// A real, clickable item's own row height. `Separator`/`Header` rows + /// use [`SEPARATOR_HEIGHT`]/[`HEADER_HEIGHT`] instead, regardless of + /// this value - see [`Self::row_height_for`]. pub row_height: u32, pub items: Vec<(&'static str, MenuAction)>, } const MENU_WIDTH: u32 = 170; +/// A real item's own row height, in logical pixels. Its own constant +/// rather than reusing `TITLEBAR_HEIGHT`: this is a stand-alone popup, not +/// a titlebar, and the two heights never needed to match - they just +/// happened to, which made a hairline separator take a full 32px slot to +/// draw a 1px line in. Sized against GNOME/KDE's own popover menu row +/// height (32-36px and 28-30px respectively), not this compositor's own +/// titlebar band. +pub const MENU_ROW_HEIGHT: u32 = 28; +/// A divider's own row height - just enough room for the hairline plus a +/// little breathing room on each side, not a full item row. +pub const SEPARATOR_HEIGHT: u32 = 9; +/// A section-label row's own height - shorter than a real item (it holds +/// smaller, dimmer text with nothing to click), taller than a plain +/// separator (it has to fit that text). +pub const HEADER_HEIGHT: u32 = 22; impl ContextMenu { /// Builds the menu for `window`, opening with its top-left corner at @@ -53,15 +109,23 @@ impl ContextMenu { let w = wm.window(window)?; let maximize_label = if w.maximized { "Restore" } else { "Maximize" }; let fullscreen_label = if w.fullscreen { "Exit Fullscreen" } else { "Fullscreen" }; - let floating_label = if w.floating { "\u{2713} Floating" } else { "Floating" }; let pin_label = if w.always_on_top { "\u{2713} Always on Top" } else { "Always on Top" }; - let mut items = vec![ - ("Minimize", MenuAction::Minimize), - (maximize_label, MenuAction::ToggleMaximize), - (fullscreen_label, MenuAction::ToggleFullscreen), - (floating_label, MenuAction::ToggleFloating), - (pin_label, MenuAction::ToggleAlwaysOnTop), - ]; + let mut items = vec![("Minimize", MenuAction::Minimize), (maximize_label, MenuAction::ToggleMaximize), (fullscreen_label, MenuAction::ToggleFullscreen)]; + // "Floating" only means something under a layout that actually + // tiles - `arrange_workspace` is the only reader of `Window:: + // floating` anywhere in this compositor, and it only runs for the + // "tiling" layout (`"dynamic"`/`"floating"` never reposition + // anyone regardless of this flag). Reported live as "doesn't make + // sense": toggling it while running this project's own default + // dynamic layout visibly does nothing at all, which reads as a + // broken menu item rather than an inapplicable one. Shown only + // when it would actually change something on screen. + let layout_is_tiling = wm.workspace(w.workspace).map(|ws| ws.layout == "tiling").unwrap_or(false); + if layout_is_tiling { + let floating_label = if w.floating { "\u{2713} Floating" } else { "Floating" }; + items.push((floating_label, MenuAction::ToggleFloating)); + } + items.push((pin_label, MenuAction::ToggleAlwaysOnTop)); // One row per *other* workspace - skips the window's own current // one, since "move to the workspace it's already on" isn't a real // action. Flattened rather than a real submenu - `workspace.count` @@ -69,7 +133,8 @@ impl ContextMenu { // so the menu stays a reasonable height without one. let others: Vec<&crate::Workspace> = wm.workspaces().iter().filter(|ws| ws.id != w.workspace).collect(); if !others.is_empty() { - items.push(("\u{2500}\u{2500}\u{2500} Move to Workspace \u{2500}\u{2500}\u{2500}", MenuAction::Separator)); + items.push(("", MenuAction::Separator)); + items.push(("Move to Workspace", MenuAction::Header)); for ws in others { // `Workspace.name` is `&'static`-incompatible (a real // `String`, user-configurable via `workspace.names`) -- @@ -79,13 +144,50 @@ impl ContextMenu { items.push((label, MenuAction::MoveToWorkspace(ws.id))); } } - items.push(("\u{2500}\u{2500}\u{2500}\u{2500}\u{2500}\u{2500}\u{2500}\u{2500}\u{2500}\u{2500}\u{2500}\u{2500}\u{2500}\u{2500}\u{2500}", MenuAction::Separator)); + // Quick-access theme customization - reported live as a direct + // ask ("allow customizing from there as well"), scoped to the two + // knobs the same live request already named by name (traffic- + // light vs. traditional buttons, which side they sit on): both + // are shared theme settings, not per-window ones, so cycling + // either restyles every open window's titlebar at once, same as + // changing it in a config reload would - see `CycleButtonStyle`/ + // `CycleButtonSide`'s own doc comments for why that's still an + // *immediate* redraw here rather than "takes effect next time you + // open a window" the way the plain `srd set` path is scoped to. + items.push(("", MenuAction::Separator)); + items.push(("Customize", MenuAction::Header)); + let button_style_label: &'static str = if wm.theme.traffic_light_buttons { "Button Style: Traffic Lights" } else { "Button Style: Traditional" }; + let button_side_label: &'static str = if wm.theme.buttons_left { "Button Side: Left" } else { "Button Side: Right" }; + items.push((button_style_label, MenuAction::CycleButtonStyle)); + items.push((button_side_label, MenuAction::CycleButtonSide)); + items.push(("", MenuAction::Separator)); items.push(("Close", MenuAction::Close)); - Some(Self { window, pos, width: MENU_WIDTH, row_height: TITLEBAR_HEIGHT, items }) + Some(Self { window, pos, width: MENU_WIDTH, row_height: MENU_ROW_HEIGHT, items }) + } + + /// The height of row `index`, or `self.row_height` (a real item's own + /// height) for an out-of-range index - callers that already know + /// `index` is valid (every real one does) never hit that fallback; it + /// exists so this can't panic if a future caller ever gets it wrong. + pub fn row_height_for(&self, index: usize) -> u32 { + match self.items.get(index) { + Some((_, MenuAction::Separator)) => SEPARATOR_HEIGHT, + Some((_, MenuAction::Header)) => HEADER_HEIGHT, + _ => self.row_height, + } + } + + /// The y-offset row `index` starts at, relative to the menu's own top + /// - every row height up to (not including) `index`, summed. `row_ + /// at`/rendering both walk rows this same way, so a mismatch between + /// "where a row is drawn" and "where a click resolves to" can't creep + /// in from computing the two differently. + pub fn row_y(&self, index: usize) -> i32 { + (0..index.min(self.items.len())).map(|i| self.row_height_for(i)).sum::<u32>() as i32 } pub fn height(&self) -> i32 { - self.row_height as i32 * self.items.len() as i32 + self.row_y(self.items.len()) } /// Which row (if any) global-space point `(x, y)` falls on. @@ -97,7 +199,15 @@ impl ContextMenu { if rel_y < 0 || rel_y >= self.height() { return None; } - Some((rel_y / self.row_height as i32) as usize) + let mut top = 0; + for (i, _) in self.items.iter().enumerate() { + let h = self.row_height_for(i) as i32; + if rel_y < top + h { + return Some(i); + } + top += h; + } + None } } @@ -114,6 +224,15 @@ mod tests { (wm, id) } + fn wm_with_tiling_window() -> (WindowManager, WindowId) { + let mut wm = WindowManager::new(); + wm.set_monitors(vec![crate::Monitor::new(0, "primary", crate::Rect::new(0, 0, 1920, 1080))]); + wm.set_layout(wm.current_workspace(), "tiling"); + let id = wm.alloc_window_id(); + wm.add_window(Window::new(id, "a")); + (wm, id) + } + #[test] fn open_labels_maximize_action_by_current_state() { let (mut wm, id) = wm_with_window(); @@ -126,15 +245,29 @@ mod tests { } #[test] + fn floating_row_is_hidden_outside_tiling_layout() { + let (wm, id) = wm_with_window(); + let menu = ContextMenu::open(&wm, id, (100, 100)).unwrap(); + assert!(!menu.items.iter().any(|(label, _)| label.contains("Floating")), "dynamic layout: toggling floating has no visible effect, so it must not be offered"); + } + + #[test] + fn floating_row_is_shown_under_tiling_layout() { + let (wm, id) = wm_with_tiling_window(); + let menu = ContextMenu::open(&wm, id, (100, 100)).unwrap(); + assert!(menu.items.iter().any(|(label, _)| label.contains("Floating")), "tiling layout: toggling floating genuinely changes the window's geometry"); + } + + #[test] fn open_marks_pinned_state_on_the_always_on_top_row() { let (mut wm, id) = wm_with_window(); let menu = ContextMenu::open(&wm, id, (100, 100)).unwrap(); - // Minimize, Maximize, Fullscreen, Floating, then Always on Top. - assert_eq!(menu.items[4].0, "Always on Top"); + // Minimize, Maximize, Fullscreen (no Floating - dynamic layout), Always on Top. + assert_eq!(menu.items[3].0, "Always on Top"); wm.toggle_always_on_top(id); let menu = ContextMenu::open(&wm, id, (100, 100)).unwrap(); - assert!(menu.items[4].0.starts_with('\u{2713}'), "pinned state must be visible on the label itself"); + assert!(menu.items[3].0.starts_with('\u{2713}'), "pinned state must be visible on the label itself"); } #[test] @@ -148,7 +281,6 @@ mod tests { let (wm, id) = wm_with_window(); let menu = ContextMenu::open(&wm, id, (100, 100)).unwrap(); assert!(!menu.items.iter().any(|(label, _)| label.contains("Workspace"))); - assert_eq!(menu.items.len(), 7, "Minimize, Maximize, Fullscreen, Floating, Always on Top, one separator, Close"); } #[test] @@ -165,22 +297,68 @@ mod tests { } #[test] - fn clicking_a_separator_row_is_distinguishable_from_a_real_action() { + fn header_rows_are_not_interactive() { + let (wm, id) = wm_with_window(); + let menu = ContextMenu::open(&wm, id, (100, 100)).unwrap(); + let headers: Vec<_> = menu.items.iter().filter(|(_, a)| matches!(a, MenuAction::Header)).collect(); + assert!(!headers.is_empty()); + for (_, action) in headers { + assert!(!action.is_interactive()); + } + } + + #[test] + fn customize_section_reflects_current_theme_state() { let (mut wm, id) = wm_with_window(); - wm.add_workspace("2", "dynamic"); + wm.theme.traffic_light_buttons = true; + wm.theme.buttons_left = false; let menu = ContextMenu::open(&wm, id, (100, 100)).unwrap(); - let separator_count = menu.items.iter().filter(|(_, a)| matches!(a, MenuAction::Separator)).count(); - assert_eq!(separator_count, 2, "one before the workspace section, one before Close"); + assert!(menu.items.iter().any(|(label, _)| *label == "Button Style: Traffic Lights")); + assert!(menu.items.iter().any(|(label, _)| *label == "Button Side: Right")); + + wm.theme.traffic_light_buttons = false; + wm.theme.buttons_left = true; + let menu = ContextMenu::open(&wm, id, (100, 100)).unwrap(); + assert!(menu.items.iter().any(|(label, _)| *label == "Button Style: Traditional")); + assert!(menu.items.iter().any(|(label, _)| *label == "Button Side: Left")); } #[test] - fn row_at_maps_a_point_to_the_right_row() { + fn separators_and_headers_are_shorter_than_a_real_item_row() { + let (wm, id) = wm_with_window(); + let menu = ContextMenu::open(&wm, id, (100, 100)).unwrap(); + for (i, (_, action)) in menu.items.iter().enumerate() { + let h = menu.row_height_for(i); + match action { + MenuAction::Separator => assert_eq!(h, SEPARATOR_HEIGHT), + MenuAction::Header => assert_eq!(h, HEADER_HEIGHT), + _ => assert_eq!(h, MENU_ROW_HEIGHT), + } + } + } + + #[test] + fn height_is_the_sum_of_every_rows_own_height() { + let (wm, id) = wm_with_window(); + let menu = ContextMenu::open(&wm, id, (100, 100)).unwrap(); + let expected: i32 = (0..menu.items.len()).map(|i| menu.row_height_for(i) as i32).sum(); + assert_eq!(menu.height(), expected); + } + + #[test] + fn row_at_maps_a_point_to_the_right_row_with_variable_heights() { let (wm, id) = wm_with_window(); let menu = ContextMenu::open(&wm, id, (100, 100)).unwrap(); assert_eq!(menu.row_at(150, 100), Some(0), "top of the first row"); - assert_eq!(menu.row_at(150, 100 + TITLEBAR_HEIGHT as i32 - 1), Some(0), "bottom of the first row"); - assert_eq!(menu.row_at(150, 100 + TITLEBAR_HEIGHT as i32), Some(1), "top of the second row"); + assert_eq!(menu.row_at(150, 100 + menu.row_height as i32 - 1), Some(0), "bottom of the first row"); + assert_eq!(menu.row_at(150, 100 + menu.row_height as i32), Some(1), "top of the second row"); assert_eq!(menu.row_at(150, 100 + menu.height() - 1), Some(menu.items.len() - 1), "last row, last pixel"); + // Every row boundary in between must resolve to exactly one row -- + // the real regression this test guards against is a gap or an + // overlap between two rows of different heights. + for y in 0..menu.height() { + assert!(menu.row_at(150, 100 + y).is_some(), "no gap at offset {y}"); + } } #[test] diff --git a/crates/wayland/src/decoration.rs b/crates/wayland/src/decoration.rs index aa12a54..d24c035 100644 --- a/crates/wayland/src/decoration.rs +++ b/crates/wayland/src/decoration.rs @@ -87,20 +87,61 @@ pub(crate) const CORNER_RADIUS: u32 = 12; /// Still no submenus/icons - real gaps beyond this pass' own scope, not /// attempted blind. /// -/// A row whose label is *entirely* the box-drawing character `─` -/// (`\u{2500}`, one or more, no other content) renders as a real thin -/// divider line instead of text glyphs - a label that *mixes* `─` with -/// real text (`"─── Move to Workspace ───"`, a deliberate section-header -/// convention `core::ContextMenu` already uses) is untouched and still -/// renders as text, since that dual purpose (divider *and* caption) is -/// the actual design, not a plain separator. A pure divider is drawn as -/// pixels rather than characters because Unicode box-drawing glyphs -/// render inconsistently thin/dotted across fonts at small sizes - a -/// real 1px anti-aliased line, inset from both edges and blended low- -/// opacity against the panel (matching the AGS reference dropdown's own -/// `separator.menu-sep`: `color-mix(in srgb, var(--fg) 12%, transparent)`), -/// reads as an intentional divider rather than a run of stray dashes. -pub fn render_context_menu(width: u32, row_height: u32, items: &[(&str, bool)], bg: (u8, u8, u8), fg: (u8, u8, u8), highlight_bg: (u8, u8, u8), border: (u8, u8, u8)) -> Vec<u8> { +/// A row's shape, alongside its own label/height - `render_context_menu` +/// used to detect a divider by checking whether a label was made entirely +/// of the box-drawing character `─`, and had no representation for a +/// section-header row at all (`"─── Move to Workspace ───"` rendered, and +/// behaved, as an ordinary clickable-looking item that merely did +/// nothing). Reported live as looking wrong on both counts - a real, +/// distinct row kind is what `srdwm_core::context_menu::MenuAction`'s own +/// `Separator`/`Header` variants exist for; this mirrors that split here +/// without this crate needing to depend on that enum itself. +#[derive(Clone, Copy, PartialEq, Eq)] +pub enum MenuRowKind { + Item, + /// A thin divider line, no label. + Separator, + /// A non-interactive section caption - dimmer, smaller text, never + /// highlighted. + Header, +} + +/// Rounded floating panel with a per-row rounded hover highlight, matching +/// the reference this project's own AGS panel already settled on for its +/// global-menu dropdown (`widget/Bar/components/GlobalMenu/style.scss`'s +/// `popover box.menu-list`): flat rows with no border/outline at rest, a +/// soft tinted fill (not a frame) on the highlighted one, inset padding so +/// rows don't touch the panel's own edge, real gaps between rows. Reported +/// live as looking "squished, no spacing/padding/margining, not at all +/// polished" - the previous version drew edge-to-edge square rows with a +/// single hard 1px border around the whole menu, exactly what that +/// complaint (raised about the AGS dropdown, fixed there first) describes. +/// Still no submenus/icons - real gaps beyond this pass' own scope, not +/// attempted blind. +/// +/// `rows` carries each row's own height alongside its label/kind - a +/// [`MenuRowKind::Separator`]/[`MenuRowKind::Header`] row is much shorter +/// than a real item (see `srdwm_core::context_menu`'s own `SEPARATOR_ +/// HEIGHT`/`HEADER_HEIGHT`), reported live as looking wrong when every +/// row, divider included, took the same full item height: a hairline +/// sitting in the middle of a mostly-empty 32px slot. The caller is what +/// actually knows each row's real height (`ContextMenu::row_height_for`); +/// this function just draws whatever it's told, at the offsets implied by +/// summing those heights in order - the same summing `ContextMenu::row_ +/// at`/`row_y` do, so a click and a pixel can never disagree about where a +/// row actually is. +/// +/// A [`MenuRowKind::Separator`] is drawn as a real 1px anti-aliased line, +/// inset from both edges and blended low-opacity against the panel +/// (matching the AGS reference dropdown's own `separator.menu-sep`: +/// `color-mix(in srgb, var(--fg) 12%, transparent)`) - pixels, not +/// Unicode box-drawing characters, which render inconsistently thin/ +/// dotted across fonts at small sizes. A [`MenuRowKind::Header`] draws its +/// label dimmed (the same mix ratio a separator's own line uses) and +/// never highlighted, regardless of what the caller passes for that row's +/// `highlighted` field - section captions aren't clickable, so nothing +/// should ever visually suggest they are. +pub fn render_context_menu(width: u32, rows: &[(&str, bool, u32, MenuRowKind)], bg: (u8, u8, u8), fg: (u8, u8, u8), highlight_bg: (u8, u8, u8), border: (u8, u8, u8)) -> Vec<u8> { let _ = border; // No outline anywhere now - see this function's own doc comment. Kept as a parameter so callers/themes don't need updating for a look this function no longer draws. const PANEL_RADIUS: f32 = 10.0; const ROW_INSET: i32 = 4; @@ -114,17 +155,13 @@ pub fn render_context_menu(width: u32, row_height: u32, items: &[(&str, bool)], // accent to tint toward", not "the literal pixel colour". const HIGHLIGHT_MIX: f32 = 0.22; // Matches the reference's own `separator.menu-sep` background -- - // barely-there, a hairline rather than a visible bar. - const SEPARATOR_MIX: f32 = 0.12; + // barely-there, a hairline rather than a visible bar. Reused as the + // header caption's own dim ratio - both exist to read as "present, + // but deliberately not the main content" against the same panel. + const DIM_MIX: f32 = 0.12; - let (width, row_height) = (width.max(1) as usize, row_height.max(1) as usize); - // Exactly `row_height * items.len()`, same as before this pass -- - // `ContextMenu`/`DesktopMenu`'s own `height()` and `row_at()` (which - // this function has no access to and mustn't get out of sync with) - // assume row `i` starts at `i * row_height` with no extra top/bottom - // inset, so all of this rework happens *inside* that unchanged canvas - // rather than by growing it. - let height = (row_height * items.len().max(1)).max(1); + let width = width.max(1) as usize; + let height = rows.iter().map(|(_, _, h, _)| *h).sum::<u32>().max(1) as usize; let mut buf = vec![0u8; width * height * 4]; // The panel itself: one flat rounded-rect fill on an otherwise fully @@ -133,15 +170,19 @@ pub fn render_context_menu(width: u32, row_height: u32, items: &[(&str, bool)], fill_rounded_rect(&mut buf, width, height, 0, 0, width as i32, height as i32, PANEL_RADIUS, bg, bg); let highlight_fill = mix_rgb(bg, highlight_bg, HIGHLIGHT_MIX); - let separator_color = mix_rgb(bg, fg, SEPARATOR_MIX); + let dim_color = mix_rgb(bg, fg, DIM_MIX); let font = find_system_font(); - for (i, (label, highlighted)) in items.iter().enumerate() { - let row_top = (i * row_height) as i32; - if !label.is_empty() && label.chars().all(|c| c == '\u{2500}') { - let y = row_top + row_height as i32 / 2; - fill_rounded_rect_over(&mut buf, width, height, ROW_INSET * 2, y, width as i32 - ROW_INSET * 2, y + 1, 0.0, separator_color); + let mut row_top: i32 = 0; + for (label, highlighted, row_height, kind) in rows.iter().copied() { + let row_height = row_height as i32; + if kind == MenuRowKind::Separator { + let y = row_top + row_height / 2; + fill_rounded_rect_over(&mut buf, width, height, ROW_INSET * 2, y, width as i32 - ROW_INSET * 2, y + 1, 0.0, dim_color); + row_top += row_height; continue; } + let highlighted = highlighted && kind != MenuRowKind::Header; + let label_color = if kind == MenuRowKind::Header { dim_color } else { fg }; // The background text actually sits on, for `blit_glyph`'s own // blend-toward-a-known-solid-colour contract - the row's own // highlight fill (already baked into `buf` by this point, above) @@ -153,9 +194,9 @@ pub fn render_context_menu(width: u32, row_height: u32, items: &[(&str, bool)], // comment) - used correctly, it would leave a visible dark // fringe around every character's anti-aliased edge instead of a // clean blend into the row's real colour. - let row_bg = if *highlighted { highlight_fill } else { bg }; - if *highlighted { - fill_rounded_rect_over(&mut buf, width, height, ROW_INSET, row_top, width as i32 - ROW_INSET, row_top + row_height as i32, ROW_RADIUS, highlight_fill); + let row_bg = if highlighted { highlight_fill } else { bg }; + if highlighted { + fill_rounded_rect_over(&mut buf, width, height, ROW_INSET, row_top, width as i32 - ROW_INSET, row_top + row_height, ROW_RADIUS, highlight_fill); } if let Some(font) = &font { let baseline = row_top as f32 + row_height as f32 * 0.72; @@ -168,7 +209,7 @@ pub fn render_context_menu(width: u32, row_height: u32, items: &[(&str, bool)], if metrics.width > 0 && metrics.height > 0 { let glyph_x = pen_x + metrics.xmin as f32; let glyph_y = baseline - metrics.height as f32 - metrics.ymin as f32; - blit_glyph(&mut buf, width, height, glyph_x.round() as i32, glyph_y.round() as i32, &metrics, &coverage, row_bg, fg); + blit_glyph(&mut buf, width, height, glyph_x.round() as i32, glyph_y.round() as i32, &metrics, &coverage, row_bg, label_color); } pen_x += metrics.advance_width; if pen_x as usize >= width { @@ -176,6 +217,7 @@ pub fn render_context_menu(width: u32, row_height: u32, items: &[(&str, bool)], } } } + row_top += row_height; } buf } diff --git a/crates/wayland/src/decoration/tests.rs b/crates/wayland/src/decoration/tests.rs index b14cbca..f2a9ded 100644 --- a/crates/wayland/src/decoration/tests.rs +++ b/crates/wayland/src/decoration/tests.rs @@ -790,18 +790,18 @@ fn border_bottom_extra_rows_are_transparent_outside_the_corners() { } #[test] -fn context_menu_is_one_row_tall_per_item() { - let items = [("Minimize", false), ("Maximize", false), ("Always on Top", false), ("Close", false)]; - let buf = render_context_menu(160, 28, &items, (0x2e, 0x34, 0x40), (0xff, 0xff, 0xff), (0x4c, 0x56, 0x6a), (0x10, 0x10, 0x10)); +fn context_menu_is_the_sum_of_each_rows_own_height() { + let items = [("Minimize", false, 28, MenuRowKind::Item), ("Maximize", false, 28, MenuRowKind::Item), ("Always on Top", false, 28, MenuRowKind::Item), ("Close", false, 28, MenuRowKind::Item)]; + let buf = render_context_menu(160, &items, (0x2e, 0x34, 0x40), (0xff, 0xff, 0xff), (0x4c, 0x56, 0x6a), (0x10, 0x10, 0x10)); assert_eq!(buf.len(), 160 * (28 * 4) * 4); } #[test] fn context_menu_highlighted_row_has_a_different_background_than_the_rest() { - let items = [("Minimize", false), ("Close", true)]; + let items = [("Minimize", false, 28, MenuRowKind::Item), ("Close", true, 28, MenuRowKind::Item)]; let bg = (0x2e, 0x34, 0x40); let highlight = (0x4c, 0x56, 0x6a); - let buf = render_context_menu(160, 28, &items, bg, (0xff, 0xff, 0xff), highlight, (0x10, 0x10, 0x10)); + let buf = render_context_menu(160, &items, bg, (0xff, 0xff, 0xff), highlight, (0x10, 0x10, 0x10)); let width = 160usize; // Sample a background pixel from each row, away from the text/border. let px_at = |x: usize, y: usize| -> [u8; 3] { @@ -830,8 +830,8 @@ fn context_menu_panel_is_opaque_in_the_middle_but_rounded_at_the_corners() { // opaque - the opposite of what this test used to assert - while an // edge's midpoint (away from any corner's curve) and the panel's own // interior stay fully opaque either way. - let items = [("Close", false)]; - let buf = render_context_menu(100, 28, &items, (0, 0, 0), (0xff, 0xff, 0xff), (0, 0, 0), (0x99, 0x99, 0x99)); + let items = [("Close", false, 28, MenuRowKind::Item)]; + let buf = render_context_menu(100, &items, (0, 0, 0), (0xff, 0xff, 0xff), (0, 0, 0), (0x99, 0x99, 0x99)); let alpha_at = |x: usize, y: usize| buf[(y * 100 + x) * 4 + 3]; assert_eq!(alpha_at(0, 0), 0, "the exact corner pixel is now outside the rounded curve, not a hard square"); // 2px in from the flat top/bottom edges, at the midpoint (far enough @@ -844,15 +844,18 @@ fn context_menu_panel_is_opaque_in_the_middle_but_rounded_at_the_corners() { } #[test] -fn a_pure_separator_row_draws_a_narrow_line_not_a_full_text_row() { - // A label of only `\u{2500}` renders as a thin graphical line instead - // of text glyphs - see `render_context_menu`'s own doc comment for - // why (Unicode box-drawing glyphs render inconsistently at small +fn a_separator_row_draws_a_narrow_line_not_a_full_text_row() { + // `MenuRowKind::Separator` renders as a thin graphical line regardless + // of its (empty) label - see `render_context_menu`'s own doc comment + // for why (Unicode box-drawing glyphs render inconsistently at small // sizes; a real line reads as an intentional divider). Scans every // pixel in the row (not one fixed column, which could accidentally // land in a font glyph's own hollow spot) - a hairline should touch // only a couple of the row's own pixel rows, nowhere near a real text - // row's spread (see the sibling test just below). + // row's spread (see the sibling test just below). Given its own much + // smaller `SEPARATOR_HEIGHT`-sized slot (9px here), not the item rows' + // 28px, so this also exercises variable row heights, not just the + // divider's own look. // // Three items, separator in the middle: `fill_rounded_rect`'s own // distance field softens alpha within `PANEL_RADIUS` of *any* of the @@ -868,8 +871,9 @@ fn a_pure_separator_row_draws_a_narrow_line_not_a_full_text_row() { // so only the separator itself can account for a non-`bg` pixel here. let bg = (0x2e, 0x34, 0x40); let width = 160usize; - let items = [("Open", false), ("\u{2500}\u{2500}\u{2500}\u{2500}\u{2500}", false), ("Close", false)]; - let buf = render_context_menu(width as u32, 28, &items, bg, (0xff, 0xff, 0xff), (0x4c, 0x56, 0x6a), (0x10, 0x10, 0x10)); + const SEPARATOR_HEIGHT: u32 = 9; + let items = [("Open", false, 28, MenuRowKind::Item), ("", false, SEPARATOR_HEIGHT, MenuRowKind::Separator), ("Close", false, 28, MenuRowKind::Item)]; + let buf = render_context_menu(width as u32, &items, bg, (0xff, 0xff, 0xff), (0x4c, 0x56, 0x6a), (0x10, 0x10, 0x10)); let px_at = |x: usize, y: usize| -> [u8; 3] { let i = (y * width + x) * 4; [buf[i + 2], buf[i + 1], buf[i]] @@ -878,31 +882,43 @@ fn a_pure_separator_row_draws_a_narrow_line_not_a_full_text_row() { let safe_x = (PANEL_RADIUS + 2)..(width - PANEL_RADIUS - 2); let row_has_any_non_bg = |y: usize| safe_x.clone().any(|x| px_at(x, y) != [bg.0, bg.1, bg.2]); let separator_row_top = 28; - let non_bg_rows = (separator_row_top..separator_row_top + 28).filter(|&y| row_has_any_non_bg(y)).count(); + let non_bg_rows = (separator_row_top..separator_row_top + SEPARATOR_HEIGHT as usize).filter(|&y| row_has_any_non_bg(y)).count(); assert!(non_bg_rows <= 3, "a hairline separator should touch only a couple of the row's pixel rows, got {non_bg_rows}"); assert!(non_bg_rows >= 1, "the separator must actually draw something, not vanish entirely"); } #[test] -fn a_labeled_section_header_separator_still_renders_as_text() { - // `"─── Move to Workspace ───"` deliberately mixes the divider - // character with real text (`core::ContextMenu`'s own section-header - // convention) - it must keep rendering as text, not collapse into a - // plain hairline just because it contains `\u{2500}` characters too. - // Same whole-row scan and same "sandwich away from the panel's own - // rounded-edge antialiasing" shape as the sibling test above. +fn a_header_row_renders_dimmed_text_not_a_divider_and_ignores_highlight() { + // `MenuRowKind::Header` replaced an earlier hack that faked a section + // caption by embedding box-drawing characters directly in an ordinary + // item's label (`"─── Move to Workspace ───"`) - indistinguishable + // from a real, clickable row except for the dashes, and reported live + // as looking exactly like that: a menu item that does nothing. A real + // header row must still render as text (not collapse into a hairline + // the way `Separator` does), and must never show a hover highlight + // even if the caller passes `highlighted: true` for it by mistake -- + // a section caption isn't a target, so nothing should ever suggest it + // is one. let bg = (0x2e, 0x34, 0x40); let width = 200usize; - let items = [("Open", false), ("\u{2500}\u{2500}\u{2500} Move to Workspace \u{2500}\u{2500}\u{2500}", false), ("Close", false)]; - let buf = render_context_menu(width as u32, 28, &items, bg, (0xff, 0xff, 0xff), (0x4c, 0x56, 0x6a), (0x10, 0x10, 0x10)); + const HEADER_HEIGHT: u32 = 22; + let items = [("Open", false, 28, MenuRowKind::Item), ("Move to Workspace", true, HEADER_HEIGHT, MenuRowKind::Header), ("Close", false, 28, MenuRowKind::Item)]; + let buf = render_context_menu(width as u32, &items, bg, (0xff, 0xff, 0xff), (0x4c, 0x56, 0x6a), (0x10, 0x10, 0x10)); let px_at = |x: usize, y: usize| -> [u8; 3] { let i = (y * width + x) * 4; [buf[i + 2], buf[i + 1], buf[i]] }; let row_has_any_non_bg = |y: usize| (0..width).any(|x| px_at(x, y) != [bg.0, bg.1, bg.2]); let header_row_top = 28; - let non_bg_rows = (header_row_top..header_row_top + 28).filter(|&y| row_has_any_non_bg(y)).count(); - assert!(non_bg_rows > 3, "a text row (mixed divider+caption) should paint well more than a hairline's worth of rows, got {non_bg_rows}"); + let non_bg_rows = (header_row_top..header_row_top + HEADER_HEIGHT as usize).filter(|&y| row_has_any_non_bg(y)).count(); + assert!(non_bg_rows > 3, "a header's own caption text should paint well more than a hairline's worth of rows, got {non_bg_rows}"); + // No highlight fill anywhere in the header's own row band, even though + // `highlighted: true` was passed for it above - a flat highlight fill + // would show up as a large, uniform block of `highlight_bg`-tinted + // pixels well away from the text glyphs themselves; sampling a point + // near the row's own right edge (past where "Move to Workspace" 's + // text reaches) catches that without depending on exact glyph shapes. + assert_eq!(px_at(width - 10, header_row_top + 10), [bg.0, bg.1, bg.2], "a header row must never show the hover highlight fill"); } #[test] diff --git a/crates/wayland/src/input/pointer.rs b/crates/wayland/src/input/pointer.rs index 2ecc15c..c6ed5ba 100644 --- a/crates/wayland/src/input/pointer.rs +++ b/crates/wayland/src/input/pointer.rs @@ -514,14 +514,15 @@ pub(crate) fn handle_pointer_button(state: &mut CompState, pos: Point<f64, Logic if let Some(menu) = state.context_menu.take() { if let Some(row) = menu.row_at(pos.x as i32, pos.y as i32) { let (_, action) = menu.items[row]; - // A separator row occupies real space (`row_at` resolves a - // click on it same as any other) but isn't a real action -- - // same "click does nothing, menu stays open" convention any - // native menu's own divider follows, rather than either - // running a no-op action or dismissing the whole menu on - // what was very possibly a slightly-off click at a real - // item just above/below it. - if matches!(action, crate::context_menu::MenuAction::Separator) { + // A separator or section-header row occupies real space + // (`row_at` resolves a click on it same as any other) but + // isn't a real action - same "click does nothing, menu + // stays open" convention any native menu's own divider/ + // caption follows, rather than either running a no-op + // action or dismissing the whole menu on what was very + // possibly a slightly-off click at a real item just + // above/below it. + if !action.is_interactive() { state.context_menu = Some(menu); return; } diff --git a/crates/wayland/src/state/desktop_icons.rs b/crates/wayland/src/state/desktop_icons.rs index 8ed22a5..9d2dd91 100644 --- a/crates/wayland/src/state/desktop_icons.rs +++ b/crates/wayland/src/state/desktop_icons.rs @@ -593,8 +593,22 @@ impl CompState { fn build_desktop_menu_buffer(&mut self, menu: DesktopMenu) { let theme = self.wm.borrow().theme; - let items: Vec<(&str, bool)> = menu.items.iter().map(|&(label, _)| (label, false)).collect(); - let data = decoration::render_context_menu(menu.width, menu.row_height, &items, theme.titlebar_bg, theme.titlebar_fg_focused, theme.titlebar_fg_unfocused, theme.default_border_color); + // Not redesigned the way the titlebar menu was (`srdwm_core:: + // context_menu`'s own module doc comment) - this menu's rows are + // still all one uniform height, `Separator` included, matching + // its behaviour before `render_context_menu` grew a per-row + // height/kind. Every row here is real content or `DesktopMenuAction + // ::Separator`, never a non-interactive caption, so there is no + // `MenuRowKind::Header` case to map to. + let rows: Vec<(&str, bool, u32, decoration::MenuRowKind)> = menu + .items + .iter() + .map(|(label, action)| { + let kind = if matches!(action, crate::desktop_menu::DesktopMenuAction::Separator) { decoration::MenuRowKind::Separator } else { decoration::MenuRowKind::Item }; + (*label, false, menu.row_height, kind) + }) + .collect(); + let data = decoration::render_context_menu(menu.width, &rows, theme.titlebar_bg, theme.titlebar_fg_focused, theme.titlebar_fg_unfocused, theme.default_border_color); let buffer = MemoryRenderBuffer::from_slice(&data, Fourcc::Argb8888, (menu.width as i32, menu.height()), 1, Transform::Normal, None); self.desktop_menu_buffer = Some(buffer); self.desktop_menu = Some(menu); diff --git a/crates/wayland/src/state/menu.rs b/crates/wayland/src/state/menu.rs index b8cf094..b0b0a6e 100644 --- a/crates/wayland/src/state/menu.rs +++ b/crates/wayland/src/state/menu.rs @@ -14,8 +14,20 @@ impl CompState { return; }; let theme = self.wm.borrow().theme; - let items: Vec<(&str, bool)> = menu.items.iter().map(|&(label, _)| (label, false)).collect(); - let data = decoration::render_context_menu(menu.width, menu.row_height, &items, theme.titlebar_bg, theme.titlebar_fg_focused, theme.titlebar_fg_unfocused, theme.default_border_color); + let rows: Vec<(&str, bool, u32, decoration::MenuRowKind)> = menu + .items + .iter() + .enumerate() + .map(|(i, &(label, action))| { + let kind = match action { + crate::context_menu::MenuAction::Separator => decoration::MenuRowKind::Separator, + crate::context_menu::MenuAction::Header => decoration::MenuRowKind::Header, + _ => decoration::MenuRowKind::Item, + }; + (label, false, menu.row_height_for(i), kind) + }) + .collect(); + let data = decoration::render_context_menu(menu.width, &rows, theme.titlebar_bg, theme.titlebar_fg_focused, theme.titlebar_fg_unfocused, theme.default_border_color); let buffer = MemoryRenderBuffer::from_slice(&data, Fourcc::Argb8888, (menu.width as i32, menu.height()), 1, Transform::Normal, None); self.context_menu_buffer = Some(buffer); self.context_menu = Some(menu); @@ -60,17 +72,54 @@ impl CompState { MenuAction::MoveToWorkspace(workspace) => { self.wm.borrow_mut().move_window_to_workspace(window, workspace); } + MenuAction::CycleButtonStyle => { + let mut wm = self.wm.borrow_mut(); + wm.theme.traffic_light_buttons = !wm.theme.traffic_light_buttons; + drop(wm); + self.redraw_every_decoration(); + } + MenuAction::CycleButtonSide => { + let mut wm = self.wm.borrow_mut(); + wm.theme.buttons_left = !wm.theme.buttons_left; + drop(wm); + self.redraw_every_decoration(); + } MenuAction::Close => { if let Some(w) = self.id_to_window.get(&window) { crate::input::close_dwindow(w); } } // Never actually reached: the click-dispatch site - // (`input/pointer.rs`) intercepts `Separator` before calling - // this function at all. Handled here too so this match stays - // exhaustive without a catch-all that would silently swallow a - // real future variant added without updating this function. - MenuAction::Separator => {} + // (`input/pointer.rs`) intercepts `Separator`/`Header` before + // calling this function at all. Handled here too so this + // match stays exhaustive without a catch-all that would + // silently swallow a real future variant added without + // updating this function. + MenuAction::Separator | MenuAction::Header => {} + } + } + + /// Rebuilds every open window's titlebar/border bitmap against + /// whatever `wm.theme` currently holds - what `CycleButtonStyle`/ + /// `CycleButtonSide` need to actually show their effect immediately. + /// + /// `srd set button_style`/`button_side` (`crates/platform/src/ipc/ + /// dispatch.rs`) change the exact same `ThemeConfig` fields but are + /// deliberately scoped to "only affects windows created (or + /// redecorated) after this call" - that crate is backend-agnostic + /// and has no way to reach into a Wayland-specific redraw. A titlebar + /// menu action has no such excuse: it's *this* backend's own code, + /// already holding `&mut self`, and a "customize" action that doesn't + /// visibly change the very titlebar you clicked would be exactly the + /// kind of "doesn't make sense" this menu was reported for in the + /// first place. `redraw_decoration_buffer` already no-ops on any + /// window whose decoration signature didn't actually change, so + /// calling it for every window here costs nothing for the (common) + /// case where most of them don't use server-side decoration at all. + fn redraw_every_decoration(&mut self) { + let ids: Vec<WindowId> = self.wm.borrow().windows().map(|w| w.id).collect(); + for id in ids { + self.redraw_decoration_buffer(id); } } diff --git a/crates/x11/src/platform/actions.rs b/crates/x11/src/platform/actions.rs index 9d31d01..4847399 100644 --- a/crates/x11/src/platform/actions.rs +++ b/crates/x11/src/platform/actions.rs @@ -35,7 +35,7 @@ impl X11Platform { Ok(()) } - fn redraw_all_decorations(&mut self) -> PlatformResult<()> { + pub(super) fn redraw_all_decorations(&mut self) -> PlatformResult<()> { let focused = self.wm.borrow().focused_id(); let ids: Vec<WindowId> = self.frames.keys().copied().collect(); for id in ids { diff --git a/crates/x11/src/platform/context_menu.rs b/crates/x11/src/platform/context_menu.rs index 9238f6a..e6b4b7f 100644 --- a/crates/x11/src/platform/context_menu.rs +++ b/crates/x11/src/platform/context_menu.rs @@ -80,7 +80,7 @@ impl X11Platform { pub(super) fn redraw_context_menu(&mut self) -> PlatformResult<()> { let Some((menu, popup)) = &self.context_menu else { return Ok(()) }; let popup = *popup; - let (width, row_height, total_height) = (menu.width, menu.row_height, menu.height()); + let (width, total_height) = (menu.width, menu.height()); let theme = self.wm.borrow().theme; let bg = rgb_to_pixel(theme.titlebar_bg); let fg = rgb_to_pixel(theme.titlebar_fg_focused); @@ -89,14 +89,27 @@ impl X11Platform { self.conn.poly_fill_rectangle(popup, self.gc, &[Rectangle { x: 0, y: 0, width: width as u16, height: total_height as u16 }]).map_err(err)?; self.conn.change_gc(self.gc, &ChangeGCAux::new().foreground(fg).font(self.font)).map_err(err)?; + // Per-row heights, not a uniform `row_height * i` - a `Separator`/ + // `Header` row is shorter than a real item (`ContextMenu::row_ + // height_for`), same variable-height model the Wayland backend's + // own `render_context_menu` now uses; `menu.row_y(i)` is exactly + // how that side computes each row's own top too, so the two + // backends can't drift onto different geometry for the same menu + // data. No dimmed-text treatment for a `Header` row here (this + // backend draws everything through one single-colour `GC`, no + // per-pixel blending the way the Wayland renderer's own `mix_rgb` + // has) - feature parity (non-interactive, correctly sized) over + // pixel parity, matching this backend's existing bar elsewhere. for (i, (label, action)) in menu.items.iter().enumerate() { - let row_y = i as i32 * row_height as i32; + let row_y = menu.row_y(i); + let row_height = menu.row_height_for(i) as i32; if matches!(action, MenuAction::Separator) { - let mid = row_y + row_height as i32 / 2; + let mid = row_y + row_height / 2; self.conn.poly_line(CoordMode::ORIGIN, popup, self.gc, &[Point { x: 8, y: mid as i16 }, Point { x: width as i16 - 8, y: mid as i16 }]).map_err(err)?; continue; } - self.conn.image_text8(popup, self.gc, 10, (row_y + 20) as i16, label.as_bytes()).map_err(err)?; + let baseline = row_y + row_height * 3 / 4; + self.conn.image_text8(popup, self.gc, 10, baseline as i16, label.as_bytes()).map_err(err)?; } self.conn.flush().map_err(err)?; Ok(()) @@ -144,8 +157,28 @@ impl X11Platform { MenuAction::MoveToWorkspace(workspace) => { self.wm.borrow_mut().move_window_to_workspace(window, workspace); } + // Same reasoning as the Wayland backend's own `redraw_every_ + // decoration` (`crates/wayland/src/state/menu.rs`): `srd set + // button_style`/`button_side` are deliberately scoped to + // "takes effect on windows created after this call" because + // `crates/platform` is backend-agnostic and has no redraw hook + // to call - this menu action runs as this backend's own + // code, already holding `&mut self`, so it can and should + // repaint every open titlebar immediately instead. + MenuAction::CycleButtonStyle => { + let mut wm = self.wm.borrow_mut(); + wm.theme.traffic_light_buttons = !wm.theme.traffic_light_buttons; + drop(wm); + self.redraw_all_decorations()?; + } + MenuAction::CycleButtonSide => { + let mut wm = self.wm.borrow_mut(); + wm.theme.buttons_left = !wm.theme.buttons_left; + drop(wm); + self.redraw_all_decorations()?; + } MenuAction::Close => self.request_close(window)?, - MenuAction::Separator => {} + MenuAction::Separator | MenuAction::Header => {} } self.conn.flush().map_err(err)?; Ok(()) diff --git a/crates/x11/src/platform/events.rs b/crates/x11/src/platform/events.rs index 10f96a9..545e8f3 100644 --- a/crates/x11/src/platform/events.rs +++ b/crates/x11/src/platform/events.rs @@ -89,7 +89,7 @@ impl X11Platform { (menu.row_at(x, y).map(|r| menu.items[r].1), menu.window) }; match row_action { - Some(srdwm_core::context_menu::MenuAction::Separator) => {} + Some(action) if !action.is_interactive() => {} Some(action) => { self.close_context_menu()?; self.run_context_menu_action(menu_window, action)?; diff --git a/docs/TODO.md b/docs/TODO.md index 2cace23..691e63b 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -1,5 +1,21 @@ # TODO / planned features - master checklist +## Titlebar right-click menu redesigned: real separators/headers, an inapplicable item hidden, live customization added (2026-08-28) + +Reported live: "looks very ugly currently and some of it doesn't make sense." Both were real, found by reading `srdwm_core::context_menu` and `decoration::render_context_menu` directly rather than guessing at what "ugly" meant. + +**Ugly, root-caused:** every row - a real item or a bare divider - occupied one full `TITLEBAR_HEIGHT` (32px) slot, so a separator was a 1px hairline sitting in the middle of 32px of mostly empty space, and the "Move to Workspace" section divider faked a caption by embedding literal box-drawing characters directly in an item's own label (`"─── Move to Workspace ───"`) - which rendered, and behaved (right up until the click-dispatch site's own special-cased check), exactly like a normal clickable row that happened to do nothing. + +**Doesn't make sense, root-caused:** "Floating" was always offered, but `Window::floating` is only ever read by `arrange_workspace`, which only runs anything for the `"tiling"` layout - toggling it under this project's own default `"dynamic"` layout (and the user's own stated preference for dynamic/floating day to day) visibly changes nothing at all, which reads as a broken control rather than an inapplicable one. + +Fixed all three properly rather than patched around: `ContextMenu` (`crates/core/src/context_menu.rs`) gained two new row kinds, `Separator` (now genuinely small, `SEPARATOR_HEIGHT` = 9px) and a real `Header` (`HEADER_HEIGHT` = 22px, non-interactive, dimmed text, never highlighted) that replaces the old label-hack entirely - both backends' rendering (`decoration::render_context_menu`'s new `MenuRowKind`, and X11's own `redraw_context_menu`) now sum each row's own real height (`ContextMenu::row_height_for`/`row_y`) instead of assuming one uniform height, so a hit-test and a pixel can never disagree about where a row actually is. "Floating" is now omitted entirely when the window's own workspace isn't running the `"tiling"` layout. + +**New, in direct response to "allow customizing from there as well":** a "Customize" section with two live toggle rows - Button Style (traffic-lights / traditional) and Button Side (left / right), the exact two knobs already named in an earlier live request this session. Clicking either flips the matching `ThemeConfig` field and immediately redraws every open window's titlebar (`redraw_every_decoration` on Wayland, `redraw_all_decorations` on X11) - deliberately *not* routed through the same path `srd set button_style`/`button_side` use, since that path (`crates/platform`, backend-agnostic) is scoped to "only affects windows created after this call" for lack of any redraw hook it can reach; a menu action that didn't visibly change the very titlebar you clicked would be exactly the "doesn't make sense" complaint this whole redesign was about. Decoration mode (server/client) was considered and deliberately left out of this section: it's negotiated once at map time, so changing it can never affect an already-open window either, and there's no clean way to make it make sense here the way the redraw trick does for button style/side. + +Both backends stay in exact sync on the underlying data (`srdwm_core::context_menu` is genuinely shared, not two drifting copies) - X11's own rendering keeps its existing "feature parity, not pixel parity" stance (no per-pixel colour blending for a dimmed header the way the Wayland renderer's `mix_rgb` gives it; the header row is still correctly sized and non-interactive, just not visually dimmed there). + +Verified via the rendering function's own pixel-level unit tests (rounded panel, correct per-row heights, hairline separator, dimmed non-highlighted header text) plus the full `ContextMenu` row-construction test suite (Floating hidden/shown by layout, customize labels reflecting live theme state, no gaps/overlaps across variable row heights). Full workspace build/test/clippy clean (242 core tests, +8 for this menu's own new behaviour; 152 wayland, net-even after rewriting the old label-hack tests into real `MenuRowKind` ones). **Not independently screenshotted interactively** - opening the real menu needs a physical right-click, and synthetic input (`ydotool`) operates at the uinput level, not scoped to any one nested test instance, so it isn't safe to fire blind the way this file's own standing caution about it already establishes; the pixel-level tests are what stand in for that here. + ## Root cause found and fixed: tiled windows tinted dark along a shared edge (2026-08-28) Reported live as "some windows are dark tinted" (via a peer session, `dotfiles-1a`, who diagnosed the actual cause and handed over a concrete fix rather than a symptom). Root cause verified by reading the rasteriser before touching anything: `shadow_bitmap` never tints a window's own interior, and no rule sets `opacity` on the affected windows - the tint was never a content property. It was a shadow-versus-gap-size mismatch. `SHADOW_SIZE` is 24px; `gap_inner` on this session's live config is 1px. `redraw_decoration_buffer`'s shadow gate (`crates/wayland/src/state/lifecycle.rs`) only excluded a maximized or fullscreen window, so every *tiled* window got the same 24px shadow too - with only 1px of real gap for it to fall into, it landed almost entirely on the neighbouring tile instead, darkening it by up to `SHADOW_MAX_ALPHA` (~35%). The focused window is raised above its neighbours, so this showed up as the *unfocused* side of a shared tile edge reading tinted. |