From ffdae252782d047a6b2d76c9bb4efeb92c391db3 Mon Sep 17 00:00:00 2001 From: srdusr <99972264+srdusr@users.noreply.github.com> Date: Sun, 28 Sep 2025 21:25:00 +0200 Subject: Context/desktop menu polish: real hover tint, real separator line, Select All Reported live: "looks weird and unpolished... need a lot more items". Compared directly against the exact AGS reference this project's own menu rebuild already targets rather than guessing: - Highlighted rows used a flat, fully-saturated fill instead of the reference's subtle 22%-accent-into-background wash. New decoration:: color::mix_rgb (channel-wise linear blend, generalizing brighten/ darken's fixed-target blends to an arbitrary second colour/ratio) lets render_context_menu reproduce that same ratio. - Every separator row was a label string of Unicode box-drawing characters rendered as text glyphs, which render inconsistently at small sizes - a label that's entirely U+2500 now draws a real 1px hairline instead; a label that mixes it with real text ("--- Move to Workspace ---", a deliberate section-header convention) still renders as text, unchanged. - "Select All" added to the bare-desktop menu, the one action every mainstream desktop's own menu offers that this one lacked. New tests needed real care: the panel's own rounded-corner distance field softens alpha within its radius of any canvas edge, not just the visible corners, so a naive full-row pixel scan against bg picked that up as a false positive on the first attempt - fixed by scanning only rows/columns confirmed (via a throwaway debug dump) to sit inside the panel's genuinely flat interior. Full workspace build/test/clippy clean, built and installed. Real submenus and per-row icons remain real, separate scope - this project's floating-menu UI has no nested-panel concept yet. --- crates/wayland/src/decoration.rs | 42 +++++++++++++++--- crates/wayland/src/decoration/color.rs | 14 ++++++ crates/wayland/src/decoration/tests.rs | 72 ++++++++++++++++++++++++++++++- crates/wayland/src/desktop_menu.rs | 8 +++- crates/wayland/src/state/desktop_icons.rs | 19 ++++++++ docs/TODO.md | 12 ++++++ 6 files changed, 160 insertions(+), 7 deletions(-) diff --git a/crates/wayland/src/decoration.rs b/crates/wayland/src/decoration.rs index 56852cb..ee3f34c 100644 --- a/crates/wayland/src/decoration.rs +++ b/crates/wayland/src/decoration.rs @@ -40,7 +40,7 @@ mod titlebar; pub use border::{border_strips, render_border_bottom, render_border_top}; pub(crate) use border::{border_bottom_visible_rows, border_top_visible_rows}; pub(crate) use buttons::HOVER_GLYPH_DURATION; -pub(crate) use color::rgb_to_bgra; +pub(crate) use color::{mix_rgb, rgb_to_bgra}; pub(crate) use corners::{round_bottom_corners, round_top_corners}; pub(crate) use font::{blit_glyph, find_system_font, FONT_PIXELS, TEXT_LEFT_PADDING}; pub use shadow::{shadow_bitmap, shadow_rect}; @@ -84,13 +84,38 @@ pub(crate) const CORNER_RADIUS: u32 = 12; /// 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/separators - real gaps beyond this pass' own -/// scope, not attempted blind. +/// 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 { 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; const ROW_RADIUS: f32 = 6.0; + // AGS reference's own measured/settled ratio (`popover box.menu-list + // button:hover`'s `color-mix(in srgb, var(--primary-bg) 22%, var( + // --widget-bg))`) - a subtle tinted wash, not a flat, fully- + // saturated fill of whatever colour the caller passes as `highlight_ + // bg`. Applied here rather than changing what callers pass in, so + // every existing call site's own colour choice still means "the + // 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; 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 -- @@ -107,9 +132,16 @@ pub fn render_context_menu(width: u32, row_height: u32, items: &[(&str, bool)], // the menu (desktop/window content) rather than a hard-edged square. 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 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); + continue; + } // 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) @@ -121,9 +153,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_bg } else { bg }; + 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_bg); + 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); } if let Some(font) = &font { let baseline = row_top as f32 + row_height as f32 * 0.72; diff --git a/crates/wayland/src/decoration/color.rs b/crates/wayland/src/decoration/color.rs index 7ea9d80..5262cec 100644 --- a/crates/wayland/src/decoration/color.rs +++ b/crates/wayland/src/decoration/color.rs @@ -32,3 +32,17 @@ pub(crate) fn darken(color: (u8, u8, u8)) -> (u8, u8, u8) { let mix = |c: u8| (c as f32 * (1.0 - AMOUNT)).round() as u8; (mix(color.0), mix(color.1), mix(color.2)) } + +/// Linear channel-wise blend of `a` toward `b` by `t` (`0.0` is pure `a`, +/// `1.0` is pure `b`) - unlike `brighten`/`darken`, which blend toward a +/// fixed white/black, this blends toward an arbitrary second colour, at +/// an arbitrary caller-chosen ratio. `render_context_menu`'s own row +/// highlight uses this to reproduce the AGS reference dropdown's own +/// subtle wash (`color-mix(in srgb, var(--primary-bg) 22%, var(--widget- +/// bg))`) instead of a flat, fully-saturated fill - the same reference +/// this project's own menu rebuild already targets elsewhere. +pub(crate) fn mix_rgb(a: (u8, u8, u8), b: (u8, u8, u8), t: f32) -> (u8, u8, u8) { + let t = t.clamp(0.0, 1.0); + let mix = |ac: u8, bc: u8| (ac as f32 + (bc as f32 - ac as f32) * t).round() as u8; + (mix(a.0, b.0), mix(a.1, b.1), mix(a.2, b.2)) +} diff --git a/crates/wayland/src/decoration/tests.rs b/crates/wayland/src/decoration/tests.rs index 5cbc12f..b14cbca 100644 --- a/crates/wayland/src/decoration/tests.rs +++ b/crates/wayland/src/decoration/tests.rs @@ -809,7 +809,15 @@ fn context_menu_highlighted_row_has_a_different_background_than_the_rest() { [buf[i + 2], buf[i + 1], buf[i]] // BGRA -> RGB }; assert_eq!(px_at(100, 5), [bg.0, bg.1, bg.2], "row 0 (not highlighted) should use bg"); - assert_eq!(px_at(100, 33), [highlight.0, highlight.1, highlight.2], "row 1 (highlighted) should use highlight_bg"); + // Not `highlight` at full strength: the AGS reference dropdown's own + // hover fill is a 22%-mixed wash of the accent over the panel + // background (`color-mix(in srgb, var(--primary-bg) 22%, var( + // --widget-bg))`), not a flat, fully-saturated fill - see `render_ + // context_menu`'s own doc comment. `mix_rgb(bg, highlight, 0.22)`, + // computed by hand here rather than imported, so this test would + // actually catch the ratio drifting. + let expected = (53, 59, 73); // (46,52,64) blended 22% toward (76,86,106) + assert_eq!(px_at(100, 33), [expected.0, expected.1, expected.2], "row 1 (highlighted) should use a subtle tinted wash toward highlight_bg, not a flat fill of it"); } #[test] @@ -835,6 +843,68 @@ fn context_menu_panel_is_opaque_in_the_middle_but_rounded_at_the_corners() { assert_eq!(alpha_at(50, 14), 255, "the panel's interior stays opaque"); } +#[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 + // 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). + // + // Three items, separator in the middle: `fill_rounded_rect`'s own + // distance field softens alpha within `PANEL_RADIUS` of *any* of the + // panel's four edges, not just the visible corner curves (a rounded + // rect's SDF clamps the comparison point toward whichever edge is + // nearest along each axis independently, so a point that's flat-on + // to one edge but still within `radius` of another edge still gets + // partial coverage). Sandwiching the separator row between two + // others, and scanning only rows/columns comfortably past `PANEL_ + // RADIUS` in from every canvas edge, keeps this test inside the + // panel's genuinely flat, fully-opaque interior - confirmed by + // measurement (a debug dump of raw pixel deltas), not assumed -- + // 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)); + let px_at = |x: usize, y: usize| -> [u8; 3] { + let i = (y * width + x) * 4; + [buf[i + 2], buf[i + 1], buf[i]] + }; + const PANEL_RADIUS: usize = 10; + 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(); + 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. + 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)); + 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}"); +} + #[test] fn snap_flyout_is_sized_for_a_full_grid_of_labels() { let labels = ["Left Half", "Right Half", "Top Left", "Top Right", "Bottom Left", "Bottom Right"]; diff --git a/crates/wayland/src/desktop_menu.rs b/crates/wayland/src/desktop_menu.rs index 3cb90ed..dbbf957 100644 --- a/crates/wayland/src/desktop_menu.rs +++ b/crates/wayland/src/desktop_menu.rs @@ -30,6 +30,11 @@ pub(crate) enum DesktopMenuAction { /// concrete path to a real file manager's own richer menu (cut/copy/ /// paste, properties, ...), deliberately not reimplemented here. OpenInFileManager, + /// Selects every desktop icon at once - the one bare-desktop menu + /// action every mainstream file manager/desktop offers (Explorer, + /// Nautilus, Finder) that this menu had no equivalent for at all, + /// reported live as this menu needing "a lot more items". + SelectAll, Refresh, /// A purely visual divider row - see `context_menu::MenuAction:: /// Separator`'s own doc comment (same shape, same reason, separate @@ -78,6 +83,7 @@ impl DesktopMenu { ("Open Terminal Here", DesktopMenuAction::OpenTerminalHere), ("Open in File Manager", DesktopMenuAction::OpenInFileManager), ("\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}", DesktopMenuAction::Separator), + ("Select All", DesktopMenuAction::SelectAll), ("Refresh", DesktopMenuAction::Refresh), ]; Self { pos, width: MENU_WIDTH, row_height: ROW_HEIGHT, items } @@ -144,7 +150,7 @@ mod tests { let menu = DesktopMenu::open_for_desktop((10, 10)); let real_actions: Vec<&str> = menu.items.iter().filter(|(_, a)| !matches!(a, DesktopMenuAction::Separator)).map(|(l, _)| *l).collect(); - assert_eq!(real_actions, vec!["New Folder", "New Text Document", "Open Terminal Here", "Open in File Manager", "Refresh"]); + assert_eq!(real_actions, vec!["New Folder", "New Text Document", "Open Terminal Here", "Open in File Manager", "Select All", "Refresh"]); } #[test] diff --git a/crates/wayland/src/state/desktop_icons.rs b/crates/wayland/src/state/desktop_icons.rs index 81a8ed8..a1b1968 100644 --- a/crates/wayland/src/state/desktop_icons.rs +++ b/crates/wayland/src/state/desktop_icons.rs @@ -218,6 +218,24 @@ impl CompState { } } + /// Selects every desktop icon at once - the bare-desktop menu's own + /// "Select All" action (see `DesktopMenuAction::SelectAll`'s own doc + /// comment). Same "only rebuild the buffers that actually changed" + /// shape as `select_desktop_icon`. + pub(crate) fn select_all_desktop_icons(&mut self) { + let Some(icons) = &mut self.desktop_icons else { return }; + let mut changed = Vec::new(); + for icon in &mut icons.icons { + if !icon.selected { + icon.selected = true; + changed.push(icon.id.clone()); + } + } + for id in changed { + self.rebuild_icon_buffer(&id); + } + } + /// Starts a rubber-band selection at `pos` (global space) - clears /// whatever was selected before, matching real desktop convention /// (Windows/GNOME/macOS all start a fresh marquee selection, not an @@ -603,6 +621,7 @@ impl CompState { DesktopMenuAction::NewTextFile => self.new_desktop_text_file(), DesktopMenuAction::OpenTerminalHere => self.open_terminal_here(), DesktopMenuAction::OpenInFileManager => self.open_desktop_in_file_manager(), + DesktopMenuAction::SelectAll => self.select_all_desktop_icons(), DesktopMenuAction::Refresh => self.refresh_desktop_icons(), // Never actually reached - the click-dispatch site intercepts // `Separator` first, same as `context_menu::MenuAction:: diff --git a/docs/TODO.md b/docs/TODO.md index e539ffb..82bb2dc 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -13,6 +13,18 @@ that has the full story. Keep this list current as items close or open; update the source doc's own entry too, don't let this drift into a second stale copy the way `PANEL_SUPPORT_TODO.md` did. +## Context/desktop menu polish: a real hover-tint ratio, a real separator line, and "Select All" (2026-08-27) + +Reported live: "looks weird and unpolished... should be smooth... need a lot more items." Compared the current renderer directly against the exact reference this project's own menu rebuild already targets (`~/dotfiles/ags_project/widget/Bar/components/GlobalMenu/style.scss`'s `popover box.menu-list`) rather than guessing at what "polished" means: + +- The highlighted-row fill was a flat, fully-saturated `highlight_bg` at 100% opacity. The reference's own hover fill is a *subtle tinted wash* - `color-mix(in srgb, var(--primary-bg) 22%, var(--widget-bg))`, only 22% accent mixed into the panel's own background. New `decoration::color::mix_rgb` (channel-wise linear blend, generalizing `brighten`/`darken`'s fixed-target blends to an arbitrary second colour and ratio) lets `render_context_menu` reproduce that same 22% ratio instead of a flat fill. +- Every "separator" row was a label string made entirely of the Unicode box-drawing character `─`, rendered through the ordinary text-glyph path - box-drawing glyphs render inconsistently thin/dotted across fonts at small sizes, unlike the reference's own real 1px hairline (`separator.menu-sep`, `color-mix(in srgb, var(--fg) 12%, transparent)`). A label that's *entirely* `─` now draws a real, low-opacity horizontal line instead of glyphs; a label that *mixes* `─` with real text (`"─── Move to Workspace ───"`, the deliberate section-header convention `core::ContextMenu` already uses) is untouched and still renders as text - that dual purpose is the actual design, not a plain separator to collapse. +- "Select All" added to the bare-desktop menu - the one action every mainstream desktop's own right-click menu offers that this one had no equivalent for at all, and a direct, useful complement to this session's own multi-select-drag fix just above. + +New tests needed real care to get right: the panel's own rounded-corner distance field softens alpha within `PANEL_RADIUS` of *any* canvas edge, not just the visible corner curves, so a naive "scan every pixel of the row" comparison against `bg` picked up that pre-existing antialiasing as a false positive on the first attempt - fixed by scanning only rows/columns confirmed (via a throwaway debug dump, not assumed) to sit inside the panel's genuinely flat interior. + +Full workspace build/test/clippy clean (223 core / 142 wayland / 29 platform / 24 ctl / 28 config / 10 x11 tests). Still not attempted: real submenus and per-row icons - both real, separate scope (this project's floating-menu UI has no nested-panel concept at all yet), not attempted blind alongside a live-feedback pass. + ## Real bug, root-caused and fixed: dragging a multi-selected desktop icon only ever moved that one icon (2026-08-27) Reported live: "try move desktop items all at once somewhere else" didn't work. Confirmed by reading the actual data, not guessed: `CompState::desktop_icon_drag` only ever held one icon id, and - the real, compounding bug - the click handler that starts a drag (`input/pointer.rs`) called `select_desktop_icon(Some(&id))` *unconditionally* before starting the drag, which collapses any existing multi-selection down to just the one icon being grabbed. Even if the drag itself had supported multiple icons, that call site would have destroyed the selection before it ever got the chance. -- cgit v1.2.3