diff options
Diffstat (limited to 'crates/core')
| -rw-r--r-- | crates/core/Cargo.toml | 1 | ||||
| -rw-r--r-- | crates/core/src/event.rs | 110 | ||||
| -rw-r--r-- | crates/core/src/geometry.rs | 146 | ||||
| -rw-r--r-- | crates/core/src/keysyms.rs | 4 | ||||
| -rw-r--r-- | crates/core/src/lib.rs | 7 | ||||
| -rw-r--r-- | crates/core/src/manager.rs | 706 | ||||
| -rw-r--r-- | crates/core/src/monitor.rs | 20 | ||||
| -rw-r--r-- | crates/core/src/placement.rs | 22 | ||||
| -rw-r--r-- | crates/core/src/rules.rs | 110 | ||||
| -rw-r--r-- | crates/core/src/theme.rs | 77 | ||||
| -rw-r--r-- | crates/core/src/window.rs | 236 |
11 files changed, 1406 insertions, 33 deletions
diff --git a/crates/core/Cargo.toml b/crates/core/Cargo.toml index 7a2fb39..d06905c 100644 --- a/crates/core/Cargo.toml +++ b/crates/core/Cargo.toml @@ -8,5 +8,6 @@ description = "Platform-independent window/workspace/layout state for srdwm" [dependencies] log.workspace = true bitflags = "2" +regex = "1" [dev-dependencies] diff --git a/crates/core/src/event.rs b/crates/core/src/event.rs index eb2d8aa..5822d3f 100644 --- a/crates/core/src/event.rs +++ b/crates/core/src/event.rs @@ -44,6 +44,53 @@ pub fn key_combo_string(modifiers: Modifiers, key_name: &str) -> String { format!("{modifiers}{key_name}") } +/// Parses a `"Mod4+Shift+Return"`-style combo string into modifiers plus the +/// bare key name, accepting the modifier tokens in *any* order. +/// +/// This matters because [`key_combo_string`]/[`Modifiers`]'s `Display` only +/// ever produce one fixed order (Ctrl, Shift, Alt, Mod4) - but every +/// shipped keybinding is written the conventional "Mod4+Shift+x" way +/// (Super first, matching Hyprland's own `SUPER, SHIFT, x` convention). +/// `srd.bind` used to store the combo string exactly as the Lua config +/// wrote it, and dispatch always looked it up by the canonical +/// Ctrl/Shift/Alt/Mod4 order built from the real keypress - so any +/// binding combining more than one modifier in a different order than that +/// fixed one could never fire: X11 grabbed the physical key correctly +/// (`grab_keybindings` already parsed order-independently, duplicating +/// this logic) but dispatch found nothing to run, and on Wayland the combo +/// was not even recognized as bound at all, so the keypress was forwarded +/// straight to the focused client instead of reaching srdwm. Confirmed +/// against the shipped `keybindings.lua`: every multi-modifier binding +/// there (`Mod4+Shift+*`, `Mod4+Ctrl+k`, `Alt+Shift+Tab`, ...) is written +/// Super/Alt-first, which never matched the canonical order. Returns +/// `None` for an empty combo (no key name at all). +pub fn parse_key_combo(combo: &str) -> Option<(Modifiers, &str)> { + let parts: Vec<&str> = combo.split('+').collect(); + let (key_name, mod_parts) = parts.split_last()?; + let mut modifiers = Modifiers::empty(); + for m in mod_parts { + modifiers |= match *m { + "Ctrl" => Modifiers::CTRL, + "Shift" => Modifiers::SHIFT, + "Alt" => Modifiers::ALT, + "Mod4" | "Super" => Modifiers::SUPER, + _ => Modifiers::empty(), + }; + } + Some((modifiers, key_name)) +} + +/// Re-orders a combo string into the canonical form [`key_combo_string`] +/// produces, regardless of what order its modifiers were written in. +/// Unparseable input (empty string) is returned unchanged, so a caller that +/// can't do anything better with it still has *something* to store/log. +pub fn canonicalize_key_combo(combo: &str) -> String { + match parse_key_combo(combo) { + Some((modifiers, key_name)) => key_combo_string(modifiers, key_name), + None => combo.to_string(), + } +} + #[derive(Debug, Clone)] pub enum Event { WindowCreated(WindowId), @@ -67,4 +114,67 @@ pub enum Event { /// can lock and suspend, which is otherwise impossible: a laptop that /// does nothing on lid-close is a real problem, not a nicety. LidSwitch { closed: bool }, + /// `WindowManager::current_workspace` changed. Carries no id: every + /// consumer that cares (`main.rs`'s `sync()`) re-reads whichever + /// workspace is current now rather than trusting a stale snapshot from + /// whenever this event was queued. + /// + /// Exists purely so `sync()` actually runs after a switch - without a + /// `dirty`-setting event, `WindowManager::switch_workspace` alone only + /// changes core's own bookkeeping; nothing shows or hides a single + /// window for the new workspace until `sync()` runs, which only + /// happens when a polled event sets `dirty`. Every switch path (a + /// keybinding, `SUPER`+scroll, the `ext_workspace_v1` protocol's + /// `activate` request) needs this pushed after it changes + /// `current_workspace`, or the switch is invisible. + WorkspaceChanged, +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn super_first_combo_canonicalizes_to_dispatch_order() { + // The shipped keybindings.lua writes every combo Super-first + // ("Mod4+Shift+m"), matching Hyprland's own convention - but + // `key_combo_string`'s Display order is fixed Ctrl/Shift/Alt/Mod4. + // A binding registered under its literal Lua string could never be + // found by a real keypress, which always dispatches through the + // canonical order. This is exactly the bug `canonicalize_key_combo` + // exists to close. + assert_eq!(canonicalize_key_combo("Mod4+Shift+m"), "Shift+Mod4+m"); + assert_eq!(canonicalize_key_combo("Mod4+Ctrl+k"), "Ctrl+Mod4+k"); + assert_eq!(canonicalize_key_combo("Alt+Shift+Tab"), "Shift+Alt+Tab"); + } + + #[test] + fn already_canonical_combo_is_unchanged() { + assert_eq!(canonicalize_key_combo("Ctrl+Mod4+k"), "Ctrl+Mod4+k"); + } + + #[test] + fn single_modifier_combo_is_unaffected() { + // No ordering ambiguity with one modifier - this case always + // worked, before and after the fix. + assert_eq!(canonicalize_key_combo("Mod4+Return"), "Mod4+Return"); + } + + #[test] + fn parse_key_combo_accepts_modifiers_in_any_order() { + let (mods, key) = parse_key_combo("Mod4+Shift+m").unwrap(); + assert_eq!(key, "m"); + assert!(mods.contains(Modifiers::SUPER) && mods.contains(Modifiers::SHIFT)); + + let (mods2, key2) = parse_key_combo("Shift+Mod4+m").unwrap(); + assert_eq!(key2, "m"); + assert_eq!(mods, mods2); + } + + #[test] + fn parse_key_combo_with_no_modifiers_is_bare_key() { + let (mods, key) = parse_key_combo("Return").unwrap(); + assert_eq!(key, "Return"); + assert_eq!(mods, Modifiers::empty()); + } } diff --git a/crates/core/src/geometry.rs b/crates/core/src/geometry.rs index 3dd205a..62d4369 100644 --- a/crates/core/src/geometry.rs +++ b/crates/core/src/geometry.rs @@ -31,6 +31,66 @@ impl Rect { || other.bottom() <= self.y) } + /// The overlapping region of two rects, or `None` if they don't + /// overlap at all (matches `overlaps`' own half-open semantics: a rect + /// that only touches another along an edge or corner does not count). + pub fn intersection(&self, other: &Rect) -> Option<Rect> { + if !self.overlaps(other) { + return None; + } + let x = self.x.max(other.x); + let y = self.y.max(other.y); + let right = self.right().min(other.right()); + let bottom = self.bottom().min(other.bottom()); + Some(Rect::new(x, y, (right - x) as u32, (bottom - y) as u32)) + } + + /// `self` minus `other`, as the (up to 4) axis-aligned pieces left + /// over - the standard top/bottom/left/right sliver decomposition + /// around the intersection. An empty `Vec` means `other` fully covers + /// `self`; a one-element `Vec` equal to `self` means they don't + /// overlap at all. + fn subtract_one(&self, other: &Rect) -> Vec<Rect> { + let Some(ix) = self.intersection(other) else { return vec![*self] }; + let mut out = Vec::with_capacity(4); + // Top sliver: full width, above the intersection. + if ix.y > self.y { + out.push(Rect::new(self.x, self.y, self.width, (ix.y - self.y) as u32)); + } + // Bottom sliver: full width, below the intersection. + if ix.bottom() < self.bottom() { + out.push(Rect::new(self.x, ix.bottom(), self.width, (self.bottom() - ix.bottom()) as u32)); + } + // Left/right slivers are constrained to the intersection's own + // y-range (not self's full height), so the top/bottom slivers + // above don't get double-counted at the corners. + if ix.x > self.x { + out.push(Rect::new(self.x, ix.y, (ix.x - self.x) as u32, ix.height)); + } + if ix.right() < self.right() { + out.push(Rect::new(ix.right(), ix.y, (self.right() - ix.right()) as u32, ix.height)); + } + out + } + + /// `self` minus every rect in `occluders` that overlaps it, as the + /// disjoint pieces still left over. Used to keep a window's border + /// from rendering on top of another window's content that's actually + /// stacked in front of it - see `crates/wayland/src/elements.rs`'s + /// `visible_border_fragments` doc comment for the fuller story on why + /// that's needed at all. An empty result means `occluders` between + /// them fully cover `self`. + pub fn subtract_all(&self, occluders: &[Rect]) -> Vec<Rect> { + let mut remaining = vec![*self]; + for occluder in occluders { + if remaining.is_empty() { + break; + } + remaining = remaining.iter().flat_map(|r| r.subtract_one(occluder)).collect(); + } + remaining + } + /// Shrinks the rect on all sides by `margin`, saturating at zero size. pub fn inset(&self, margin: u32) -> Rect { let m = margin as i32; @@ -91,4 +151,90 @@ mod tests { assert!(!r.contains_point(10, 10)); assert!(r.contains_point(9, 9)); } + + #[test] + fn intersection_of_non_overlapping_rects_is_none() { + let a = Rect::new(0, 0, 10, 10); + let b = Rect::new(20, 20, 10, 10); + assert_eq!(a.intersection(&b), None); + } + + #[test] + fn intersection_is_the_overlapping_region() { + let a = Rect::new(0, 0, 100, 100); + let b = Rect::new(50, 50, 100, 100); + assert_eq!(a.intersection(&b), Some(Rect::new(50, 50, 50, 50))); + } + + #[test] + fn subtract_all_with_no_occluders_returns_the_rect_unchanged() { + let r = Rect::new(0, 0, 100, 100); + assert_eq!(r.subtract_all(&[]), vec![r]); + } + + #[test] + fn subtract_all_with_a_non_overlapping_occluder_returns_the_rect_unchanged() { + let r = Rect::new(0, 0, 100, 100); + let occluder = Rect::new(200, 200, 10, 10); + assert_eq!(r.subtract_all(&[occluder]), vec![r]); + } + + #[test] + fn subtract_all_with_a_fully_covering_occluder_returns_nothing() { + let r = Rect::new(10, 10, 20, 20); + let occluder = Rect::new(0, 0, 100, 100); + assert!(r.subtract_all(&[occluder]).is_empty()); + } + + /// This is the exact bug this whole mechanism exists to fix, found live: + /// a tall vertical border strip on a background window (e.g. its right + /// edge) with a foreground window's content covering its middle, + /// leaving only a sliver above and below visible - rather than the + /// border rendering straight through the foreground window's content. + #[test] + fn subtract_all_splits_a_tall_strip_around_a_covering_window_into_two_slivers() { + // A 3px-wide, 630px-tall right border strip... + let border = Rect::new(890, 126, 3, 630); + // ...with a foreground window covering its middle vertically. + let foreground = Rect::new(240, 277, 800, 630); + let pieces = border.subtract_all(&[foreground]); + // Only the sliver above the foreground window's top edge and the + // sliver below its bottom edge should remain - the foreground + // window's own height (630) exceeds the border's, so in this case + // the whole thing is covered from y=277 down; only the top sliver + // (126..277) survives. + assert_eq!(pieces, vec![Rect::new(890, 126, 3, 277 - 126)]); + } + + #[test] + fn subtract_all_leaves_a_gap_when_the_occluder_only_covers_the_middle() { + let strip = Rect::new(0, 0, 5, 100); + let occluder = Rect::new(0, 30, 5, 20); // covers y in [30, 50) + let pieces = strip.subtract_all(&[occluder]); + assert_eq!(pieces.len(), 2); + assert!(pieces.contains(&Rect::new(0, 0, 5, 30))); + assert!(pieces.contains(&Rect::new(0, 50, 5, 50))); + } + + #[test] + fn subtract_all_handles_multiple_occluders_in_sequence() { + let strip = Rect::new(0, 0, 5, 100); + let a = Rect::new(0, 10, 5, 10); // [10,20) + let b = Rect::new(0, 40, 5, 10); // [40,50) + let pieces = strip.subtract_all(&[a, b]); + assert_eq!(pieces.len(), 3); + assert!(pieces.contains(&Rect::new(0, 0, 5, 10))); + assert!(pieces.contains(&Rect::new(0, 20, 5, 20))); + assert!(pieces.contains(&Rect::new(0, 50, 5, 50))); + } + + #[test] + fn subtract_all_handles_a_partial_side_overlap_without_losing_area() { + // Occluder only covers the left half of the rect - the right half + // (a "right sliver") must survive intact. + let r = Rect::new(0, 0, 100, 50); + let occluder = Rect::new(-10, -10, 60, 70); // covers x in [0,50) + let pieces = r.subtract_all(&[occluder]); + assert_eq!(pieces, vec![Rect::new(50, 0, 50, 50)]); + } } diff --git a/crates/core/src/keysyms.rs b/crates/core/src/keysyms.rs index b166159..e610ecb 100644 --- a/crates/core/src/keysyms.rs +++ b/crates/core/src/keysyms.rs @@ -21,6 +21,7 @@ pub fn keysym_to_name(keysym: u32) -> Option<String> { 0xff56 => "Next".to_string(), 0xff50 => "Home".to_string(), 0xff57 => "End".to_string(), + 0xff61 => "Print".to_string(), 0xffbe..=0xffc9 => format!("F{}", keysym - 0xffbe + 1), // Laptop/media keys. Values taken from the system's own // <X11/XF86keysym.h>, not guessed - a wrong constant here fails @@ -61,6 +62,7 @@ pub fn name_to_keysym(name: &str) -> Option<u32> { "next" | "pagedown" => return Some(0xff56), "home" => return Some(0xff50), "end" => return Some(0xff57), + "print" => return Some(0xff61), // Must stay in sync with `keysym_to_name` above: the X11 backend // resolves names through here to pass to `XGrabKey`, so a key // missing from *this* direction can be pressed but never grabbed. @@ -127,7 +129,7 @@ mod tests { #[test] fn named_keys_roundtrip() { - for name in ["Return", "Escape", "Tab", "Left", "Right", "Up", "Down", "F1", "F12"] { + for name in ["Return", "Escape", "Tab", "Left", "Right", "Up", "Down", "Print", "F1", "F12"] { let ks = name_to_keysym(name).unwrap(); assert_eq!(keysym_to_name(ks), Some(name.to_string())); } diff --git a/crates/core/src/lib.rs b/crates/core/src/lib.rs index 0fc2fad..471fece 100644 --- a/crates/core/src/lib.rs +++ b/crates/core/src/lib.rs @@ -6,15 +6,18 @@ pub mod manager; pub mod monitor; pub mod placement; pub mod rules; +pub mod theme; pub mod window; pub mod workspace; -pub use event::{key_combo_string, Event, MouseButton, Modifiers}; +pub use event::{canonicalize_key_combo, key_combo_string, parse_key_combo, Event, MouseButton, Modifiers}; pub use geometry::Rect; pub use layout::{Layout, MasterStackLayout, NoOpLayout, TilingConfig}; pub use manager::{Direction, WindowManager}; pub use monitor::{Monitor, MonitorId}; pub use placement::{PlacementConfig, SmartPlacement}; +pub use regex::Regex; pub use rules::{WindowMatch, WindowRule, WindowRuleActions}; -pub use window::{ResizeEdge, TitlebarHit, Window, WindowId, RESIZE_MARGIN, TITLEBAR_HEIGHT}; +pub use theme::{parse_hex_color, ThemeConfig}; +pub use window::{GlobalMenu, MenuSource, ResizeEdge, TitlebarHit, Window, WindowId, RESIZE_MARGIN, TITLEBAR_HEIGHT}; pub use workspace::{Workspace, WorkspaceId}; diff --git a/crates/core/src/manager.rs b/crates/core/src/manager.rs index 3856a25..8696508 100644 --- a/crates/core/src/manager.rs +++ b/crates/core/src/manager.rs @@ -3,6 +3,7 @@ use crate::layout::{Layout, MasterStackLayout, NoOpLayout, TilingConfig}; use crate::monitor::{Monitor, MonitorId}; use crate::placement::{PlacementConfig, SmartPlacement, MIN_WINDOW_HEIGHT, MIN_WINDOW_WIDTH}; use crate::rules::WindowRule; +use crate::theme::ThemeConfig; use crate::window::{ResizeEdge, TitlebarHit, Window, WindowId}; use crate::workspace::{Workspace, WorkspaceId}; use std::collections::HashMap; @@ -41,14 +42,36 @@ pub struct WindowManager { monitors: Vec<Monitor>, workspaces: Vec<Workspace>, current_workspace: WorkspaceId, + /// Whichever workspace was current immediately before the current one + /// became current - see `switch_workspace`'s doc comment. + previous_workspace: WorkspaceId, + /// Read from `workspace.auto_back_and_forth`. When set, switching to + /// the workspace that's already active switches to `previous_workspace` + /// instead - sway's `workspace_auto_back_and_forth` behavior, a quick + /// "jump back to whatever I was just on" toggle on a single keybinding. + pub auto_back_and_forth: bool, next_workspace_id: WorkspaceId, next_window_id: WindowId, layouts: HashMap<String, Box<dyn Layout>>, pub tiling: TilingConfig, pub placement: PlacementConfig, + /// Whether geometry changes made via `toggle_maximize`/`toggle_fullscreen` + /// should be animated. Read from `general.animations`; a backend's open + /// animation is gated on this too, since core has no notion of "open". + pub animations_enabled: bool, + /// Tween duration in milliseconds, read from `general.animation_duration`. + pub animation_duration_ms: u32, + /// Default decoration colours and border width, read from `theme.colors.*`/ + /// `theme.decorations.*`. See `ThemeConfig`'s own doc comment. + pub theme: ThemeConfig, drag: Option<DragState>, resize: Option<ResizeState>, rules: Vec<WindowRule>, + /// Windows a client-close was requested for, drained once per tick by + /// `main.rs`'s event loop and forwarded to `Platform::close`. Needed + /// because `WindowManager` is platform-agnostic and has no way to send + /// a client its close request directly - see `close_window`. + close_requests: Vec<WindowId>, } impl Default for WindowManager { @@ -71,14 +94,20 @@ impl WindowManager { monitors: Vec::new(), workspaces: vec![Workspace::new(0, "1", "dynamic")], current_workspace: 0, + previous_workspace: 0, + auto_back_and_forth: false, next_workspace_id: 1, next_window_id: 1, layouts, tiling: TilingConfig::default(), placement: PlacementConfig::default(), + animations_enabled: true, + animation_duration_ms: 200, + theme: ThemeConfig::default(), drag: None, resize: None, rules: Vec::new(), + close_requests: Vec::new(), } } @@ -147,6 +176,28 @@ impl WindowManager { } } } + // A maximized/fullscreen window's geometry was set to a snapshot of + // its monitor's usable/full rect at the moment it was toggled on -- + // it is not live-bound to that rect afterward. Without this, a bar + // or dock changing its exclusive zone while a window is maximized + // (the live case: a dock dropping its reservation to 0 so a + // maximized window can cover its area) grows or shrinks `Monitor:: + // geometry`/`full_geometry` here, but the already-maximized window + // keeps its stale pre-change size until manually un-maximized and + // re-maximized - reported as "maximize does not extend past the + // dock" even though the dock's own zone change took effect + // immediately in every other respect (new windows placed correctly, + // `Monitor::geometry` itself correct if queried fresh). + for window in self.windows.values_mut() { + if !window.maximized && !window.fullscreen { + continue; + } + let Some(monitor) = live.iter().find(|m| m.id == window.monitor) else { continue }; + let target = if window.fullscreen { monitor.full_geometry } else { monitor.geometry }; + if window.geometry != target { + window.geometry = target; + } + } } pub fn monitors(&self) -> &[Monitor] { @@ -175,7 +226,16 @@ impl WindowManager { /// it's left for the next `arrange_workspace` call to place. pub fn add_window(&mut self, mut window: Window) -> WindowId { let id = window.id; + // Applied before rule matching below, which still wins when a rule + // sets its own `border_color`/`border_width` - this only replaces + // whatever a backend's `Window::new` happened to hardcode. + window.border_color = self.theme.default_border_color; + window.border_width = self.theme.default_border_width; let actions = self.rules.iter().find(|r| r.matcher.matches(&window)).map(|r| r.actions.clone()); + // See `Window::rules_applied`'s doc comment: a native Wayland window + // still has empty title/app_id at this point, so a real (if + // inconclusive) match attempt needs to wait for `reapply_rules_if_pending`. + window.rules_applied = actions.is_some() || !(window.title.is_empty() && window.app_id.is_empty()); let workspace = actions.as_ref().and_then(|a| a.workspace).unwrap_or(self.current_workspace); window.workspace = workspace; @@ -222,6 +282,63 @@ impl WindowManager { id } + /// Retries rule matching for a window `add_window` couldn't conclusively + /// match yet (see `Window::rules_applied`'s doc comment) - a backend + /// calls this once a native Wayland window's real `title`/`app_id` + /// become known, typically on its first real commit. A no-op once + /// `rules_applied` is already `true`, so this is safe to call on every + /// subsequent metadata change without rules re-applying repeatedly. + /// + /// Returns whether a rule actually matched and was applied - distinct + /// from simply "ran" (this is a no-op past the first call regardless). + /// A backend uses this to decide whether a follow-up geometry/decoration + /// sync is warranted: `sync_geometry` re-stacks the window to the top + /// via smithay's `Space::map_element` as a side effect of updating its + /// tracked position (`map_element` always does this, `activate` or + /// not - there is no "move without restacking" in this smithay + /// version), so calling it on *every* title/app_id change - which + /// happens constantly for perfectly ordinary reasons (a browser tab + /// finishing a page load) long after the window's own creation - would + /// silently yank an unfocused, unrelated window back to the front any + /// time its title happened to update. Reported live as exactly that: + /// an older window jumping in front of a newer, focused one with no + /// user action to explain it. + pub fn reapply_rules_if_pending(&mut self, id: WindowId) -> bool { + let Some(window) = self.windows.get(&id) else { return false }; + if window.rules_applied || (window.title.is_empty() && window.app_id.is_empty()) { + return false; + } + let actions = self.rules.iter().find(|r| r.matcher.matches(window)).map(|r| r.actions.clone()); + let Some(window) = self.windows.get_mut(&id) else { return false }; + window.rules_applied = true; + let Some(actions) = actions else { return false }; + if let Some(floating) = actions.floating { + window.floating = floating; + } + if let Some(decorated) = actions.decorated { + window.decorated = decorated; + } + if let Some(color) = actions.border_color { + window.border_color = color; + } + if let Some(width) = actions.border_width { + window.border_width = width; + } + if let Some(pinned) = actions.pinned { + window.always_on_top = pinned; + } + if let Some(geometry) = actions.geometry { + window.geometry = geometry; + } + if let Some(workspace) = actions.workspace { + self.move_window_to_workspace(id, workspace); + } + if actions.maximized.unwrap_or(false) { + self.toggle_maximize(id); + } + true + } + pub fn remove_window(&mut self, id: WindowId) -> Option<Window> { self.order.retain(|&w| w != id); if self.focused == Some(id) { @@ -259,6 +376,22 @@ impl WindowManager { self.restack_pinned(); } + /// Sends a window to the back of the stack - the middle-click-titlebar + /// convention most X11 WMs (twm, fvwm, IceWM) have always had and this + /// one never did. Doesn't touch focus: lowering the window you're + /// currently looking at out from under the pointer without also moving + /// keyboard focus elsewhere would leave input going to a window that's + /// no longer visible under the cursor, which is more surprising than + /// useful. `restack_pinned` still runs afterward so a pinned window + /// can't accidentally end up buried by this either. + pub fn lower_window(&mut self, id: WindowId) { + if let Some(pos) = self.order.iter().position(|&w| w == id) { + let id = self.order.remove(pos); + self.order.insert(0, id); + } + self.restack_pinned(); + } + /// Toggles "always on top" for a window (Hyprland's `pin`), used for /// picture-in-picture and small HUD overlays that must stay visible /// while you work in something else. @@ -430,6 +563,14 @@ impl WindowManager { pub fn close_window(&mut self, id: WindowId) { log::info!("close_window({id})"); + self.close_requests.push(id); + } + + /// Drains windows queued by `close_window` since the last call. Core + /// has no way to reach a client itself - the caller (`main.rs`) is + /// expected to forward each id to `Platform::close`. + pub fn take_close_requests(&mut self) -> Vec<WindowId> { + std::mem::take(&mut self.close_requests) } pub fn minimize_window(&mut self, id: WindowId) { @@ -447,9 +588,68 @@ impl WindowManager { } } + /// Moves a window into the scratchpad pool, hiding it immediately -- + /// sway's `move scratchpad`. The single most-used "quick terminal" + /// pattern in tiling window managers, and srdwm had no equivalent at + /// all before this. + /// + /// Also floats the window: tiling something that's meant to pop in and + /// out on demand doesn't make sense, and would otherwise fight + /// `arrange_workspace` every time it's shown. Reuses `minimized` for + /// the actual show/hide gating rather than introducing a second + /// visibility flag - `scratchpad` here is purely a marker of *pool + /// membership*, kept separate so `scratchpad_show` knows which hidden + /// windows are its own to bring back, as opposed to an ordinarily + /// minimized one. + pub fn scratchpad_add(&mut self, id: WindowId) { + if let Some(w) = self.windows.get_mut(&id) { + w.scratchpad = true; + w.floating = true; + } + self.minimize_window(id); + } + + /// Removes a window from the scratchpad pool without changing its + /// current visibility - for a rule or script that wants to opt a + /// window back into ordinary window management. + pub fn scratchpad_remove(&mut self, id: WindowId) { + if let Some(w) = self.windows.get_mut(&id) { + w.scratchpad = false; + } + } + + /// Toggles the scratchpad - sway's `scratchpad show`, meant for one + /// keybinding a user presses repeatedly. If the focused window is + /// itself a currently-shown scratchpad window, hides it; otherwise + /// shows (and focuses) the most recently added hidden scratchpad + /// window, if any, moving it onto whichever workspace is current so it + /// follows the user rather than staying pinned to wherever it was + /// added from - sway's own behavior. "Most recently added" is `id` + /// order, since ids are allocated monotonically and no separate + /// timestamp is tracked; only ever one window is shown/hidden per + /// call, deliberately not sway's full multi-window cycling, which + /// needs its own remembered order and is a rarer need than a single + /// scratchpad window covers. + pub fn scratchpad_show(&mut self) { + if let Some(id) = self.focused { + if self.windows.get(&id).is_some_and(|w| w.scratchpad && !w.minimized) { + self.minimize_window(id); + return; + } + } + let Some(id) = self.windows.values().filter(|w| w.scratchpad && w.minimized).map(|w| w.id).max() else { return }; + if let Some(w) = self.windows.get_mut(&id) { + w.workspace = self.current_workspace; + } + self.restore_window(id); + self.focus_window(id); + } + pub fn toggle_maximize(&mut self, id: WindowId) { let monitor_geom = self.windows.get(&id).and_then(|w| self.monitor_for(w.monitor)).map(|m| m.geometry); + let animations_enabled = self.animations_enabled; let Some(w) = self.windows.get_mut(&id) else { return }; + let from = w.geometry; if w.maximized { if let Some(restore) = w.restore_geometry.take() { w.geometry = restore; @@ -460,6 +660,9 @@ impl WindowManager { w.geometry = geom; w.maximized = true; } + if animations_enabled && w.geometry != from { + w.anim_from = Some(from); + } } /// Fullscreen: the window covers its whole monitor with no decoration. @@ -469,15 +672,38 @@ impl WindowManager { /// they are mutually exclusive - toggling one off restores whatever the /// window's geometry was before *either* was applied, and entering /// fullscreen from a maximised window doesn't lose the original size. + /// + /// `decorated` is saved and restored the same way, via + /// `restore_decorated` - exiting used to hardcode `w.decorated = true` + /// unconditionally, which is only correct for a window that was + /// decorated to begin with. Any window a rule sets `decorated = false` + /// for (client-side-decorated apps like Firefox, matched via + /// `srd.rule({ class = "firefox" }, { decorated = false })`) that ever + /// goes fullscreen - an HTML5 video, a PDF presentation, plain F11 -- + /// came back from it permanently `decorated = true`, with no further + /// event to ever set it back. Since border/titlebar redraw fresh from + /// live `Window.decorated` every frame but the *hit-testing* band this + /// wrongly turned on doesn't correspond to anything the client is + /// actually drawing there, every click in what srdwm now (incorrectly) + /// treats as the titlebar band got swallowed as a drag/button hit + /// instead of ever reaching the client - reported live as a click on + /// Firefox's back button minimizing the window instead. pub fn toggle_fullscreen(&mut self, id: WindowId) { - let monitor_geom = self.windows.get(&id).and_then(|w| self.monitor_for(w.monitor)).map(|m| m.geometry); + // Unlike `toggle_maximize`, fullscreen uses the monitor's true + // full rect, not the exclusive-zone-shrunk usable area - a + // fullscreen window should cover (or go under) a bar/dock like + // everywhere else, not stop short of it. See `Monitor:: + // full_geometry`'s doc comment. + let monitor_geom = self.windows.get(&id).and_then(|w| self.monitor_for(w.monitor)).map(|m| m.full_geometry); + let animations_enabled = self.animations_enabled; let Some(w) = self.windows.get_mut(&id) else { return }; + let from = w.geometry; if w.fullscreen { if let Some(restore) = w.restore_geometry.take() { w.geometry = restore; } w.fullscreen = false; - w.decorated = true; + w.decorated = w.restore_decorated.take().unwrap_or(true); } else if let Some(geom) = monitor_geom { // Only remember the pre-fullscreen geometry if we aren't already // maximised, otherwise the monitor rect would overwrite the real @@ -488,8 +714,12 @@ impl WindowManager { w.maximized = false; w.geometry = geom; w.fullscreen = true; + w.restore_decorated = Some(w.decorated); w.decorated = false; } + if animations_enabled && w.geometry != from { + w.anim_from = Some(from); + } } pub fn is_fullscreen(&self, id: WindowId) -> bool { @@ -529,7 +759,7 @@ impl WindowManager { if w.minimized { continue; } - if let Some(hit) = ResizeEdge::hit_test(w.geometry, x, y) { + if let Some(hit) = ResizeEdge::hit_test(w.geometry, x, y, w.decorated, w.border_width) { return Some((w.id, hit)); } } @@ -578,7 +808,13 @@ impl WindowManager { new_geom.x += dx; new_geom.y += dy; - let monitor_bounds = self.windows.get(&drag.window).and_then(|w| self.monitor_for(w.monitor)).map(|m| m.geometry); + // `full_geometry`, not `geometry`: a floating window being dragged + // must be able to cross into (or land under/over) the strip a + // bar/dock reserves - only *placement* of a brand-new window and + // maximize avoid it. Clamping a drag to the shrunk usable area + // made it physically impossible to ever drag a window past a + // dock, at any speed or angle. + let monitor_bounds = self.windows.get(&drag.window).and_then(|w| self.monitor_for(w.monitor)).map(|m| m.full_geometry); if let Some(bounds) = monitor_bounds { new_geom.x = new_geom.x.clamp(bounds.x - new_geom.width as i32 + 40, bounds.right() - 40); new_geom.y = new_geom.y.clamp(bounds.y, bounds.bottom() - 40); @@ -630,6 +866,15 @@ impl WindowManager { self.resize.is_some() } + /// The edge currently being dragged, if a resize is in progress - so a + /// backend can keep showing the matching resize cursor for the whole + /// drag, not just while the pointer happens to still be hovering that + /// exact edge (which it usually isn't, once the drag is actually + /// underway). + pub fn resize_edge(&self) -> Option<ResizeEdge> { + self.resize.as_ref().map(|r| r.edge) + } + // ---- Workspaces ----------------------------------------------------- pub fn add_workspace(&mut self, name: impl Into<String>, layout: impl Into<String>) -> WorkspaceId { @@ -639,6 +884,17 @@ impl WindowManager { id } + /// Sets a workspace's display name - used to apply `workspace.names` + /// at startup (`crates/srdwm/src/main.rs`'s `apply_workspace_count`), + /// since `WindowManager::new`/`add_workspace` otherwise leave every + /// workspace named after its own 1-based index regardless of what a + /// config asked for. A no-op if `id` doesn't exist. + pub fn rename_workspace(&mut self, id: WorkspaceId, name: impl Into<String>) { + if let Some(w) = self.workspaces.iter_mut().find(|w| w.id == id) { + w.name = name.into(); + } + } + pub fn remove_workspace(&mut self, id: WorkspaceId) { if self.workspaces.len() <= 1 { return; @@ -653,9 +909,19 @@ impl WindowManager { } } + /// Switches to `id`, unless `auto_back_and_forth` is set and `id` is + /// already the current workspace - in which case this jumps to + /// `previous_workspace` instead, sway's `workspace_auto_back_and_forth` + /// behavior. `previous_workspace` itself always tracks "whatever was + /// current right before this call changed it", updated on every real + /// switch regardless of the setting, so turning the setting on later + /// (or a client-driven switch, e.g. `ext_workspace_v1`'s `activate`) + /// doesn't need its own separate bookkeeping. pub fn switch_workspace(&mut self, id: WorkspaceId) { - if self.workspaces.iter().any(|w| w.id == id) { - self.current_workspace = id; + let target = if self.auto_back_and_forth && id == self.current_workspace { self.previous_workspace } else { id }; + if self.workspaces.iter().any(|w| w.id == target) && target != self.current_workspace { + self.previous_workspace = self.current_workspace; + self.current_workspace = target; } } @@ -683,6 +949,19 @@ impl WindowManager { self.windows.values().filter(|w| w.workspace == self.current_workspace && !w.minimized) } + /// Same windows as [`Self::visible_windows`], but in real front-to-back + /// stacking order (topmost first) instead of arbitrary `HashMap` + /// iteration order. Needed anywhere a backend composites more than one + /// window's elements (content, decoration, border) together and their + /// relative order across *different* windows actually matters - unlike + /// `visible_windows`, which is fine for anything per-window in + /// isolation (border color, geometry) where order never came up. + /// `self.order` reversed is the same "topmost first" convention + /// `hit_test`/`window_at` already use. + pub fn visible_windows_front_to_back(&self) -> impl Iterator<Item = &Window> { + self.order.iter().rev().filter_map(|id| self.windows.get(id)).filter(|w| w.workspace == self.current_workspace && !w.minimized) + } + // ---- Layout ----------------------------------------------------------- pub fn set_layout(&mut self, workspace: WorkspaceId, layout_name: impl Into<String>) { @@ -854,7 +1133,7 @@ mod tests { w.geometry = Rect::new(500, 500, 400, 300); wm.add_window(w); wm.start_drag(a, 510, 510); - wm.update_drag(20, 510); // drag far left, within snap threshold of edge 0 + wm.update_drag(15, 510); // drag far left, landing within snap_threshold (8px) of edge 0 wm.end_drag(); let g = wm.window(a).unwrap().geometry; assert_eq!(g, Rect::new(0, 0, 960, 1080)); @@ -891,6 +1170,42 @@ mod tests { } #[test] + fn maximize_records_anim_from_when_animations_enabled() { + let mut wm = wm_with_monitor(); + let a = wm.alloc_window_id(); + let mut w = Window::new(a, "a"); + w.geometry = Rect::new(50, 50, 300, 200); + wm.add_window(w); + let placed = wm.window(a).unwrap().geometry; + wm.toggle_maximize(a); + assert_eq!(wm.window(a).unwrap().anim_from, Some(placed)); + } + + #[test] + fn maximize_does_not_record_anim_from_when_animations_disabled() { + let mut wm = wm_with_monitor(); + wm.animations_enabled = false; + let a = wm.alloc_window_id(); + let mut w = Window::new(a, "a"); + w.geometry = Rect::new(50, 50, 300, 200); + wm.add_window(w); + wm.toggle_maximize(a); + assert_eq!(wm.window(a).unwrap().anim_from, None); + } + + #[test] + fn fullscreen_records_anim_from_covering_the_full_monitor() { + let mut wm = wm_with_monitor(); + let a = wm.alloc_window_id(); + let mut w = Window::new(a, "a"); + w.geometry = Rect::new(50, 50, 300, 200); + wm.add_window(w); + let placed = wm.window(a).unwrap().geometry; + wm.toggle_fullscreen(a); + assert_eq!(wm.window(a).unwrap().anim_from, Some(placed)); + } + + #[test] fn directional_focus_picks_nearest_window_in_that_direction() { let mut wm = wm_with_monitor(); wm.set_layout(wm.current_workspace(), "tiling"); @@ -950,7 +1265,7 @@ mod tests { let mut wm = wm_with_monitor(); wm.set_layout(wm.current_workspace(), "tiling"); wm.add_rule(WindowRule { - matcher: crate::rules::WindowMatch { title_contains: Some("calculator".into()), class: None }, + matcher: crate::rules::WindowMatch { title_contains: Some("calculator".into()), ..Default::default() }, actions: crate::rules::WindowRuleActions { floating: Some(true), ..Default::default() }, }); let id = wm.alloc_window_id(); @@ -962,7 +1277,7 @@ mod tests { fn non_matching_rule_leaves_window_untouched() { let mut wm = wm_with_monitor(); wm.add_rule(WindowRule { - matcher: crate::rules::WindowMatch { title_contains: Some("calculator".into()), class: None }, + matcher: crate::rules::WindowMatch { title_contains: Some("calculator".into()), ..Default::default() }, actions: crate::rules::WindowRuleActions { floating: Some(true), ..Default::default() }, }); let id = wm.alloc_window_id(); @@ -975,7 +1290,7 @@ mod tests { let mut wm = wm_with_monitor(); let target = wm.add_workspace("scratch", "dynamic"); wm.add_rule(WindowRule { - matcher: crate::rules::WindowMatch { title_contains: None, class: Some("scratchpad".into()) }, + matcher: crate::rules::WindowMatch { class: Some("scratchpad".into()), ..Default::default() }, actions: crate::rules::WindowRuleActions { workspace: Some(target), ..Default::default() }, }); let id = wm.alloc_window_id(); @@ -997,6 +1312,150 @@ mod tests { assert!(wm.workspace(ws2).is_none()); } + #[test] + fn rename_workspace_changes_the_display_name() { + let mut wm = wm_with_monitor(); + let ws2 = wm.add_workspace("2", "dynamic"); + wm.rename_workspace(ws2, "code"); + assert_eq!(wm.workspace(ws2).unwrap().name, "code"); + } + + #[test] + fn auto_back_and_forth_jumps_to_the_previous_workspace_when_reselecting_the_active_one() { + let mut wm = wm_with_monitor(); + wm.auto_back_and_forth = true; + let ws2 = wm.add_workspace("2", "dynamic"); + wm.switch_workspace(ws2); + assert_eq!(wm.current_workspace(), ws2); + // Re-selecting the already-active workspace jumps back to 0, the + // one that was active right before. + wm.switch_workspace(ws2); + assert_eq!(wm.current_workspace(), 0); + } + + #[test] + fn without_auto_back_and_forth_reselecting_the_active_workspace_is_a_plain_no_op() { + let mut wm = wm_with_monitor(); + let ws2 = wm.add_workspace("2", "dynamic"); + wm.switch_workspace(ws2); + wm.switch_workspace(ws2); + assert_eq!(wm.current_workspace(), ws2); + } + + #[test] + fn switching_to_a_nonexistent_workspace_does_not_move_or_touch_previous() { + let mut wm = wm_with_monitor(); + let ws2 = wm.add_workspace("2", "dynamic"); + wm.switch_workspace(ws2); + wm.switch_workspace(9999); + assert_eq!(wm.current_workspace(), ws2); + // The failed switch must not have overwritten `previous_workspace` + // either - auto_back_and_forth would otherwise jump to a + // workspace id that was never really visited. + wm.auto_back_and_forth = true; + wm.switch_workspace(ws2); + assert_eq!(wm.current_workspace(), 0); + } + + #[test] + fn rename_workspace_is_a_no_op_for_an_id_that_does_not_exist() { + let mut wm = wm_with_monitor(); + wm.rename_workspace(9999, "ghost"); + assert!(wm.workspaces().iter().all(|w| w.name != "ghost")); + } + + // ---- Scratchpad -------------------------------------------------------- + + #[test] + fn scratchpad_add_hides_the_window_and_marks_pool_membership() { + let mut wm = wm_with_monitor(); + let a = wm.alloc_window_id(); + wm.add_window(Window::new(a, "term")); + wm.scratchpad_add(a); + let w = wm.window(a).unwrap(); + assert!(w.scratchpad); + assert!(w.minimized); + assert!(w.floating); + assert!(!wm.visible_windows().any(|w| w.id == a)); + } + + #[test] + fn scratchpad_show_brings_back_the_hidden_window_and_focuses_it() { + let mut wm = wm_with_monitor(); + let a = wm.alloc_window_id(); + wm.add_window(Window::new(a, "term")); + wm.scratchpad_add(a); + wm.scratchpad_show(); + let w = wm.window(a).unwrap(); + assert!(!w.minimized); + assert_eq!(wm.focused_id(), Some(a)); + assert!(wm.visible_windows().any(|w| w.id == a)); + } + + #[test] + fn scratchpad_show_hides_again_when_the_shown_scratchpad_window_is_focused() { + let mut wm = wm_with_monitor(); + let a = wm.alloc_window_id(); + wm.add_window(Window::new(a, "term")); + wm.scratchpad_add(a); + wm.scratchpad_show(); // shows + focuses + wm.scratchpad_show(); // toggles back off + assert!(wm.window(a).unwrap().minimized); + assert!(!wm.visible_windows().any(|w| w.id == a)); + } + + #[test] + fn scratchpad_show_moves_the_window_onto_the_current_workspace() { + let mut wm = wm_with_monitor(); + let a = wm.alloc_window_id(); + wm.add_window(Window::new(a, "term")); + wm.scratchpad_add(a); + let ws2 = wm.add_workspace("2", "dynamic"); + wm.switch_workspace(ws2); + wm.scratchpad_show(); + assert_eq!(wm.window(a).unwrap().workspace, ws2); + assert!(wm.visible_windows().any(|w| w.id == a)); + } + + #[test] + fn scratchpad_show_with_no_scratchpad_windows_is_a_no_op() { + let mut wm = wm_with_monitor(); + let a = wm.alloc_window_id(); + wm.add_window(Window::new(a, "normal")); + wm.scratchpad_show(); + assert_eq!(wm.focused_id(), Some(a)); + assert!(!wm.window(a).unwrap().minimized); + } + + #[test] + fn scratchpad_show_picks_the_most_recently_added_hidden_window() { + let mut wm = wm_with_monitor(); + let a = wm.alloc_window_id(); + wm.add_window(Window::new(a, "old")); + wm.scratchpad_add(a); + let b = wm.alloc_window_id(); + wm.add_window(Window::new(b, "new")); + wm.scratchpad_add(b); + wm.scratchpad_show(); + assert_eq!(wm.focused_id(), Some(b)); + assert!(wm.window(a).unwrap().minimized); + } + + #[test] + fn scratchpad_remove_leaves_current_visibility_untouched_but_drops_pool_membership() { + let mut wm = wm_with_monitor(); + let a = wm.alloc_window_id(); + wm.add_window(Window::new(a, "term")); + wm.scratchpad_add(a); + wm.scratchpad_remove(a); + assert!(!wm.window(a).unwrap().scratchpad); + assert!(wm.window(a).unwrap().minimized); + // No longer scratchpad-managed, so a later `scratchpad_show` must + // not touch it. + wm.scratchpad_show(); + assert!(wm.window(a).unwrap().minimized); + } + // ---- Monitor hotplug ------------------------------------------------- fn two_monitors() -> Vec<Monitor> { @@ -1140,6 +1599,203 @@ mod tests { } #[test] + fn fullscreen_round_trip_restores_a_client_side_decorated_window_to_undecorated() { + // Regression test: exiting fullscreen used to hardcode + // `decorated = true` unconditionally, which is only correct for a + // window that was decorated to begin with. A window a rule sets + // `decorated = false` for (client-side-decorated apps like + // Firefox) that goes fullscreen and back used to come back + // permanently `decorated = true` - with nothing to ever set it + // back, since the client only negotiates its decoration mode once. + // Since border/titlebar hit-testing is keyed off `Window.decorated` + // directly, this made srdwm swallow every click near the top of + // the window as a fake titlebar hit instead of forwarding it to + // the client. + let mut wm = WindowManager::new(); + wm.set_monitors(two_monitors()); + let id = wm.alloc_window_id(); + let mut w = Window::new(id, "firefox"); + w.geometry = Rect::new(100, 100, 400, 300); + w.decorated = false; + wm.add_window(w); + + wm.toggle_fullscreen(id); + assert!(!wm.window(id).unwrap().decorated, "fullscreen itself must still drop the titlebar"); + + wm.toggle_fullscreen(id); + assert!(!wm.window(id).unwrap().decorated, "must restore the pre-fullscreen decorated=false, not default to true"); + } + + /// A monitor whose usable `geometry` is shrunk by a bottom dock's + /// exclusive zone, distinct from its true `full_geometry` - the shape + /// every real backend reports once a bar/dock has claimed space (see + /// `Monitor::full_geometry`'s doc comment). + fn monitor_with_dock() -> Monitor { + let mut m = Monitor::new(0, "primary", Rect::new(0, 0, 1920, 1020)); + m.full_geometry = Rect::new(0, 0, 1920, 1080); + m.primary = true; + m + } + + #[test] + fn fullscreen_covers_the_full_monitor_ignoring_a_dock_reservation() { + // Regression test: fullscreen used to target `Monitor::geometry` + // (the usable, exclusive-zone-shrunk area), the same field maximize + // correctly uses - so a fullscreened window stopped short of a + // dock's reserved strip instead of covering (or going under) it + // like fullscreen does everywhere else. `full_geometry` is what + // fixes that; `geometry` must stay untouched so maximize keeps + // respecting the dock. + let mut wm = WindowManager::new(); + wm.set_monitors(vec![monitor_with_dock()]); + let id = wm.alloc_window_id(); + wm.add_window(Window::new(id, "a")); + + wm.toggle_fullscreen(id); + assert_eq!(wm.window(id).unwrap().geometry, Rect::new(0, 0, 1920, 1080), "fullscreen must reach the true monitor edge, past the dock"); + } + + #[test] + fn maximize_still_respects_the_dock_reservation() { + let mut wm = WindowManager::new(); + wm.set_monitors(vec![monitor_with_dock()]); + let id = wm.alloc_window_id(); + wm.add_window(Window::new(id, "a")); + + wm.toggle_maximize(id); + assert_eq!(wm.window(id).unwrap().geometry, Rect::new(0, 0, 1920, 1020), "maximize must still stop at the dock, unlike fullscreen"); + } + + #[test] + fn maximized_window_grows_when_the_dock_drops_its_reservation_live() { + // Regression test: a dock that hides/reduces its exclusive zone + // while a window is already maximized (an auto-hide dock reacting + // to monocle/maximize, exactly the scenario an AGS peer session hit + // live) used to leave that window stuck at its stale, dock-shrunk + // size - `set_monitors` updated `Monitor::geometry` correctly but + // never touched already-maximized/fullscreen windows' `geometry`, + // so nothing re-grew until the window was manually un-maximized and + // re-maximized. + let mut wm = WindowManager::new(); + wm.set_monitors(vec![monitor_with_dock()]); + let id = wm.alloc_window_id(); + wm.add_window(Window::new(id, "a")); + wm.toggle_maximize(id); + assert_eq!(wm.window(id).unwrap().geometry, Rect::new(0, 0, 1920, 1020)); + + // The dock drops its exclusive zone to 0. + let mut freed = Monitor::new(0, "primary", Rect::new(0, 0, 1920, 1080)); + freed.full_geometry = Rect::new(0, 0, 1920, 1080); + freed.primary = true; + wm.set_monitors(vec![freed]); + + assert_eq!( + wm.window(id).unwrap().geometry, + Rect::new(0, 0, 1920, 1080), + "an already-maximized window must live-track a monitor geometry change, not just windows placed afterward" + ); + } + + #[test] + fn fullscreen_window_also_live_tracks_a_monitor_geometry_change() { + let mut wm = WindowManager::new(); + wm.set_monitors(vec![monitor_with_dock()]); + let id = wm.alloc_window_id(); + wm.add_window(Window::new(id, "a")); + wm.toggle_fullscreen(id); + assert_eq!(wm.window(id).unwrap().geometry, Rect::new(0, 0, 1920, 1080)); + + let mut resized = Monitor::new(0, "primary", Rect::new(0, 0, 2560, 1420)); + resized.full_geometry = Rect::new(0, 0, 2560, 1440); + resized.primary = true; + wm.set_monitors(vec![resized]); + + assert_eq!(wm.window(id).unwrap().geometry, Rect::new(0, 0, 2560, 1440), "fullscreen must live-track the true full rect, not the usable one"); + } + + #[test] + fn a_non_maximized_window_is_left_alone_by_a_monitor_geometry_change() { + // set_monitors' new re-sync pass is gated on maximized/fullscreen -- + // must not clobber an ordinary floating/tiled window's geometry just + // because the monitor rect changed underneath it. + let mut wm = WindowManager::new(); + wm.set_monitors(vec![monitor_with_dock()]); + let id = wm.alloc_window_id(); + let mut w = Window::new(id, "a"); + w.geometry = Rect::new(100, 100, 400, 300); + wm.add_window(w); + wm.window_mut(id).unwrap().geometry = Rect::new(100, 100, 400, 300); + + let mut freed = Monitor::new(0, "primary", Rect::new(0, 0, 1920, 1080)); + freed.full_geometry = Rect::new(0, 0, 1920, 1080); + freed.primary = true; + wm.set_monitors(vec![freed]); + + assert_eq!(wm.window(id).unwrap().geometry, Rect::new(100, 100, 400, 300)); + } + + #[test] + fn dragging_a_window_can_cross_into_the_dock_reserved_strip() { + // Regression test: `update_drag`'s clamp used to also use + // `Monitor::geometry` (the shrunk usable area), which made it + // physically impossible to ever drag a floating window into the + // strip a dock reserves - not just discouraged, genuinely + // unreachable at any drag speed or angle. `full_geometry` is what + // makes that space reachable again; the dock still renders on top + // as an overlay, same as it does everywhere else. + let mut wm = WindowManager::new(); + wm.set_monitors(vec![monitor_with_dock()]); + let id = wm.alloc_window_id(); + let mut w = Window::new(id, "a"); + w.geometry = Rect::new(500, 500, 200, 200); + wm.add_window(w); + + wm.start_drag(id, 600, 600); + // Drag far down - past the old usable-area bottom (1020) and + // toward the true monitor bottom (1080). + wm.update_drag(600, 5000); + let g = wm.window(id).unwrap().geometry; + // Old behavior (clamped to `geometry`, bottom 1020) would stop at + // y=980; clamped to `full_geometry` (bottom 1080), it reaches 1040. + assert_eq!(g.y, 1040, "must clamp against the true monitor bottom, not the dock-shrunk usable area"); + } + + #[test] + fn class_rule_applies_once_app_id_is_known_after_creation() { + // Regression test: `add_window` matches rules against whatever + // `app_id`/`title` the window already has - for a native Wayland + // client those are still empty at that moment (the real values + // only arrive on a later commit, well after `new_toplevel`), so + // every class-based rule - including `srd.rule({ class = + // "firefox" }, { decorated = false })`, meant to stop srdwm + // drawing a second titlebar over Firefox's own - silently never + // matched. `reapply_rules_if_pending` is the retry a backend calls + // once the real app_id is known. + let mut wm = wm_with_monitor(); + wm.add_rule(WindowRule { + matcher: crate::rules::WindowMatch { class: Some("firefox".into()), ..Default::default() }, + actions: crate::rules::WindowRuleActions { decorated: Some(false), ..Default::default() }, + }); + let id = wm.alloc_window_id(); + // Empty app_id, exactly as a fresh native Wayland toplevel has it. + wm.add_window(Window::new(id, "")); + assert!(wm.window(id).unwrap().decorated, "no app_id yet, so no match - must not have flipped early"); + + let w = wm.window_mut(id).unwrap(); + w.app_id = "firefox".into(); + wm.reapply_rules_if_pending(id); + assert!(!wm.window(id).unwrap().decorated, "app_id now known - the rule must apply on retry"); + + // A later, unrelated title change (e.g. a browser tab switching) + // must not re-match and re-apply - rule actions apply once. + let w = wm.window_mut(id).unwrap(); + w.decorated = true; + w.title = "a new tab title".into(); + wm.reapply_rules_if_pending(id); + assert!(wm.window(id).unwrap().decorated, "rules_applied is already true - must not re-run the match"); + } + + #[test] fn fullscreen_from_maximized_still_restores_the_pre_maximize_size() { // Both share `restore_geometry`; entering fullscreen from a // maximised window must not overwrite it with the monitor rect, or @@ -1305,4 +1961,34 @@ mod tests { wm.raise_window(b); assert_eq!(wm.stacking_order().last().map(|w| w.id), Some(b)); } + + #[test] + fn lower_window_sends_it_to_the_back_of_the_stack() { + let mut wm = wm_with_monitor(); + let a = wm.alloc_window_id(); + wm.add_window(Window::new(a, "a")); + let b = wm.alloc_window_id(); + wm.add_window(Window::new(b, "b")); + let c = wm.alloc_window_id(); + wm.add_window(Window::new(c, "c")); + assert_eq!(wm.stacking_order().last().map(|w| w.id), Some(c), "precondition: c is on top after being added last"); + + wm.lower_window(c); + let order: Vec<_> = wm.stacking_order().map(|w| w.id).collect(); + assert_eq!(order, vec![c, a, b], "c must be at the very back, a/b unchanged relative to each other"); + } + + #[test] + fn lower_window_never_buries_a_pinned_window() { + let mut wm = wm_with_monitor(); + let a = wm.alloc_window_id(); + wm.add_window(Window::new(a, "a")); + let pinned = wm.alloc_window_id(); + wm.add_window(Window::new(pinned, "pinned")); + wm.toggle_always_on_top(pinned); + assert_eq!(wm.stacking_order().last().map(|w| w.id), Some(pinned)); + + wm.lower_window(a); + assert_eq!(wm.stacking_order().last().map(|w| w.id), Some(pinned), "a pinned window must stay on top even after an unrelated lower_window call"); + } } diff --git a/crates/core/src/monitor.rs b/crates/core/src/monitor.rs index aa25fba..b04ee15 100644 --- a/crates/core/src/monitor.rs +++ b/crates/core/src/monitor.rs @@ -5,14 +5,30 @@ pub type MonitorId = u32; #[derive(Debug, Clone)] pub struct Monitor { pub id: MonitorId, - pub name: String, + /// Usable area: the output rect shrunk by any layer-shell exclusive + /// zone (a bar/dock). What placement, tiling and maximize target -- + /// see `full_geometry`'s doc comment for the one thing that + /// deliberately does *not* use this field. pub geometry: Rect, + /// The output's true full rect, ignoring any exclusive zone. + /// + /// Kept separate from `geometry` because "respects the dock" and + /// "doesn't" are two genuinely different behaviors a window needs, + /// not one setting: fullscreen (and a window being interactively + /// dragged) should be able to cover or cross the strip a bar/dock + /// reserves - the bar just renders on top, as an overlay, the same + /// way it does everywhere else - while a *new* window's placement, + /// tiling and maximize should keep avoiding that strip, same as + /// before. Defaults to `geometry` (no reservation) for any backend + /// that hasn't been taught the distinction yet. + pub full_geometry: Rect, + pub name: String, pub refresh_rate_mhz: u32, pub primary: bool, } impl Monitor { pub fn new(id: MonitorId, name: impl Into<String>, geometry: Rect) -> Self { - Self { id, name: name.into(), geometry, refresh_rate_mhz: 60_000, primary: false } + Self { id, name: name.into(), geometry, full_geometry: geometry, refresh_rate_mhz: 60_000, primary: false } } } diff --git a/crates/core/src/placement.rs b/crates/core/src/placement.rs index ed9e410..15a5ad5 100644 --- a/crates/core/src/placement.rs +++ b/crates/core/src/placement.rs @@ -21,13 +21,33 @@ pub const MIN_WINDOW_HEIGHT: u32 = 150; pub struct PlacementConfig { pub grid_margin: u32, pub cascade_offset: i32, + /// How close (in logical pixels) a dragged window's edge has to end up + /// to a monitor edge on release before `snap_zone` triggers a + /// half/quarter/maximize. A single edge match with no corner match + /// (e.g. top-only) maximizes the *whole* window - see `snap_zone`'s + /// `(false, false, true, false) => area` arm - so this value directly + /// controls how easy it is to accidentally full-maximize a window while + /// just repositioning it near the top of the screen, not only how + /// generous the corner/half-snap zones are. pub snap_threshold: i32, pub max_grid: u32, } impl Default for PlacementConfig { fn default() -> Self { - Self { grid_margin: 10, cascade_offset: 30, snap_threshold: 50, max_grid: 4 } + // `snap_threshold` was 50, then 20 - both live-tested and reported + // as still snapping from an ordinary "move it near an edge" drag, + // not just a deliberate release-at-the-edge one. `update_drag`'s + // clamp used to also cap a dragged window's reach to the + // exclusive-zone-shrunk usable area rather than the monitor's true + // edge (see `Monitor::full_geometry`), which made this worse than + // the number alone suggests: the window could get within 20px of + // `snap_zone`'s comparison edge well before the cursor was + // anywhere near the real screen edge. 8 keeps snapping reachable + // (a window's own edge, not the cursor, is what's measured) while + // requiring it to actually be at the edge, not just closer to it + // than to the middle of the screen. + Self { grid_margin: 10, cascade_offset: 30, snap_threshold: 8, max_grid: 4 } } } diff --git a/crates/core/src/rules.rs b/crates/core/src/rules.rs index 642179d..148b627 100644 --- a/crates/core/src/rules.rs +++ b/crates/core/src/rules.rs @@ -1,22 +1,46 @@ +use regex::Regex; + use crate::geometry::Rect; use crate::window::Window; use crate::workspace::WorkspaceId; /// Match criteria for a [`WindowRule`]. A matcher with every field `None` /// matches nothing (an accidental `srd.rule({}, {...})` in config should be a -/// silent no-op, not "apply to every window"). +/// silent no-op, not "apply to every window"). Every field that is `Some` +/// must match (AND semantics) - matching the convention i3's multi-criteria +/// rules and bspwm's `class:instance:title` rules both already use. +/// +/// `title_contains`/`class` (plain substring/exact match) are kept +/// alongside the regex fields below rather than folded into them: they +/// cover the large majority of real rules (`srd.rule({ class = "firefox" }, +/// ...)`) with no regex syntax to get right, and are cheaper to evaluate. +/// `title_regex`/`class_regex`/`instance` exist for the cases that need +/// more precision - disambiguating a specific dialog by title while +/// leaving an app's main window alone, the concrete example that motivated +/// adding these - without forcing every simple rule to write one. #[derive(Debug, Clone, Default)] pub struct WindowMatch { /// Case-insensitive substring match against `Window::title`. pub title_contains: Option<String>, - /// Case-insensitive exact match against `Window::app_id` (X11 `WM_CLASS` - /// / Wayland `app_id`). + /// Case-insensitive exact match against `Window::app_id` (X11 `WM_CLASS`'s + /// *class* half / Wayland `app_id`). pub class: Option<String>, + /// Regex match against `Window::title`. Case-sensitive by default -- + /// write `(?i)` at the start of the pattern for case-insensitive, + /// the same convention i3's own criteria use. + pub title_regex: Option<Regex>, + /// Regex match against `Window::app_id`. + pub class_regex: Option<Regex>, + /// Case-insensitive exact match against `Window::instance` (X11 + /// `WM_CLASS`'s *instance* half). Always fails to match on Wayland-native + /// windows, which have no equivalent (`Window::instance` is always + /// empty there) - an X11/XWayland-only criterion, same as bspwm's. + pub instance: Option<String>, } impl WindowMatch { pub fn is_empty(&self) -> bool { - self.title_contains.is_none() && self.class.is_none() + self.title_contains.is_none() && self.class.is_none() && self.title_regex.is_none() && self.class_regex.is_none() && self.instance.is_none() } pub fn matches(&self, window: &Window) -> bool { @@ -33,6 +57,21 @@ impl WindowMatch { return false; } } + if let Some(re) = &self.title_regex { + if !re.is_match(&window.title) { + return false; + } + } + if let Some(re) = &self.class_regex { + if !re.is_match(&window.app_id) { + return false; + } + } + if let Some(i) = &self.instance { + if !window.instance.eq_ignore_ascii_case(i) { + return false; + } + } true } } @@ -70,7 +109,7 @@ mod tests { #[test] fn title_match_is_case_insensitive_substring() { let w = Window::new(1, "Mozilla Firefox"); - let m = WindowMatch { title_contains: Some("firefox".into()), class: None }; + let m = WindowMatch { title_contains: Some("firefox".into()), ..Default::default() }; assert!(m.matches(&w)); } @@ -78,10 +117,69 @@ mod tests { fn class_match_is_case_insensitive_exact() { let mut w = Window::new(1, ""); w.app_id = "Firefox".into(); - let m = WindowMatch { title_contains: None, class: Some("firefox".into()) }; + let m = WindowMatch { class: Some("firefox".into()), ..Default::default() }; assert!(m.matches(&w)); let mut w2 = Window::new(2, ""); w2.app_id = "firefoxx".into(); assert!(!m.matches(&w2)); } + + #[test] + fn title_regex_matches_a_specific_dialog_without_matching_the_main_window() { + // The concrete case that motivated adding regex support at all: + // disambiguating a specific dialog by title while leaving an app's + // main window alone - not reliably possible with substring-only + // matching if the dialog's title is a substring-superset situation + // (or vice versa) that plain `contains` can't express. + let m = WindowMatch { title_regex: Some(Regex::new(r"^Save File$").unwrap()), ..Default::default() }; + let dialog = Window::new(1, "Save File"); + let main = Window::new(2, "Save File - GNU Image Manipulation Program"); + assert!(m.matches(&dialog)); + assert!(!m.matches(&main)); + } + + #[test] + fn class_regex_is_case_sensitive_unless_the_pattern_opts_in() { + let mut w = Window::new(1, ""); + w.app_id = "firefox".into(); + let sensitive = WindowMatch { class_regex: Some(Regex::new(r"^Firefox$").unwrap()), ..Default::default() }; + assert!(!sensitive.matches(&w)); + let insensitive = WindowMatch { class_regex: Some(Regex::new(r"(?i)^Firefox$").unwrap()), ..Default::default() }; + assert!(insensitive.matches(&w)); + } + + #[test] + fn instance_match_is_case_insensitive_exact_and_independent_of_class() { + let mut w = Window::new(1, ""); + w.app_id = "Navigator".into(); + w.instance = "firefox".into(); + let m = WindowMatch { instance: Some("Firefox".into()), ..Default::default() }; + assert!(m.matches(&w)); + let mut w2 = Window::new(2, ""); + w2.instance = "firefoxdeveloperedition".into(); + assert!(!m.matches(&w2)); + } + + #[test] + fn multiple_criteria_are_combined_with_and() { + let mut w = Window::new(1, "Preferences"); + w.app_id = "firefox".into(); + let m = WindowMatch { class: Some("firefox".into()), title_contains: Some("preferences".into()), ..Default::default() }; + assert!(m.matches(&w)); + // Same class, different title - must not match once title is + // also a criterion. + let mut w2 = Window::new(2, "Mozilla Firefox"); + w2.app_id = "firefox".into(); + assert!(!m.matches(&w2)); + } + + #[test] + fn a_nonempty_instance_criterion_does_not_match_a_wayland_native_window() { + // `Window::instance` is always empty on Wayland (no equivalent + // concept), so a real `instance` rule (never an empty-string one -- + // nobody writes `instance = ""`) must not match there. + let w = Window::new(1, ""); + let m = WindowMatch { instance: Some("firefox".into()), ..Default::default() }; + assert!(!m.matches(&w)); + } } diff --git a/crates/core/src/theme.rs b/crates/core/src/theme.rs new file mode 100644 index 0000000..1717fe9 --- /dev/null +++ b/crates/core/src/theme.rs @@ -0,0 +1,77 @@ +/// Default decoration colours and border width, applied to every window at +/// creation (before rules run, so a rule's own `border_color`/`border_width` +/// still wins - see `WindowManager::add_window`) and read live by a +/// backend's titlebar rendering. +/// +/// Read from `theme.colors.*`/`theme.decorations.*` in `crates/srdwm/src/ +/// main.rs`'s `apply_general_settings`. Before that wiring existed, these +/// were hardcoded Rust constants scattered across `crates/wayland` (the +/// Nord palette every default here still matches, so an unconfigured +/// session looks identical to before) - found the same way `window_gap` +/// and `general.animations` were: config already validated/defaulted these +/// keys, nothing ever read them. +#[derive(Debug, Clone, Copy, PartialEq)] +pub struct ThemeConfig { + pub titlebar_bg: (u8, u8, u8), + pub titlebar_fg_focused: (u8, u8, u8), + pub titlebar_fg_unfocused: (u8, u8, u8), + pub default_border_color: (u8, u8, u8), + pub default_border_width: u32, +} + +impl Default for ThemeConfig { + fn default() -> Self { + Self { + titlebar_bg: (0x2e, 0x34, 0x40), + titlebar_fg_focused: (0x88, 0xc0, 0xd0), + titlebar_fg_unfocused: (0x4c, 0x56, 0x6a), + default_border_color: (136, 192, 208), // Nord accent, matches legacy theme default + default_border_width: 2, + } + } +} + +/// Parses a `"#rrggbb"` string into its channels. Returns `None` for +/// anything else - `crates/config` already validates this shape at load +/// time (`is_valid_hex_color`) and logs a warning for a malformed value, so +/// a caller here can fall back to a default silently rather than erroring +/// a second time. +pub fn parse_hex_color(s: &str) -> Option<(u8, u8, u8)> { + if s.len() != 7 || !s.starts_with('#') { + return None; + } + let r = u8::from_str_radix(&s[1..3], 16).ok()?; + let g = u8::from_str_radix(&s[3..5], 16).ok()?; + let b = u8::from_str_radix(&s[5..7], 16).ok()?; + Some((r, g, b)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses_a_well_formed_hex_color() { + assert_eq!(parse_hex_color("#88c0d0"), Some((0x88, 0xc0, 0xd0))); + } + + #[test] + fn rejects_missing_hash_or_wrong_length() { + assert_eq!(parse_hex_color("88c0d0"), None); + assert_eq!(parse_hex_color("#88c0d"), None); + assert_eq!(parse_hex_color("#88c0d00"), None); + } + + #[test] + fn rejects_non_hex_digits() { + assert_eq!(parse_hex_color("#zzzzzz"), None); + } + + #[test] + fn default_matches_the_legacy_hardcoded_nord_palette() { + let t = ThemeConfig::default(); + assert_eq!(t.titlebar_bg, (0x2e, 0x34, 0x40)); + assert_eq!(t.titlebar_fg_focused, (0x88, 0xc0, 0xd0)); + assert_eq!(t.default_border_color, (136, 192, 208)); + } +} diff --git a/crates/core/src/window.rs b/crates/core/src/window.rs index 9484709..1a4cab5 100644 --- a/crates/core/src/window.rs +++ b/crates/core/src/window.rs @@ -2,6 +2,61 @@ use crate::geometry::Rect; pub type WindowId = u64; +/// A window's exported application/window menu, as a D-Bus *address* -- +/// bus name plus object paths - never the menu's actual content. +/// +/// The content is a `GMenuModel` already exported over `org.gtk.Menus`/ +/// `org.gtk.Actions`, which GTK4 consumes natively (`Gio.DBusMenuModel`, +/// `Gtk.PopoverMenuBar.new_from_model()`) with full submenus, toggles, +/// accelerators and icons - carrying the model itself over a Wayland +/// protocol instead would mean hand-marshalling and hand-rendering it for +/// strictly worse fidelity. These four strings are the only part a +/// compositor can supply that a client-side global-menu shell can't get +/// any other way: on XWayland this is `_GTK_UNIQUE_BUS_NAME`/ +/// `_GTK_MENUBAR_OBJECT_PATH`/`_GTK_APPLICATION_OBJECT_PATH`/ +/// `_GTK_WINDOW_OBJECT_PATH`; on Wayland-native surfaces it's GTK's own +/// private `gtk_shell1` protocol's `gtk_surface1.set_dbus_properties` +/// request, which carries the identical four fields under different +/// names. See `crates/wayland/src/xwayland.rs` and `gtk_shell.rs` for +/// where each backend actually populates this. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct GlobalMenu { + pub bus_name: String, + /// The app or window's menu bar, whichever the client exported -- + /// `menubar_path` (a full menu bar) if set, else `app_menu_path` (just + /// the single app-level menu older/simpler clients export instead). + /// Which of the two (or the pre-`_GTK_*` Unity path) actually won is + /// [`Self::source`] - load-bearing, not cosmetic: see its own doc + /// comment. + pub menu_path: Option<String>, + pub app_path: Option<String>, + pub window_path: Option<String>, + pub source: MenuSource, +} + +/// Which export flavour [`GlobalMenu::menu_path`] actually came from -- +/// the two address their actions under different D-Bus action-group +/// prefixes, and getting this wrong doesn't fail loudly: the menu still +/// renders, every item just comes up permanently insensitive, which reads +/// exactly like an app that exported a broken menu rather than a +/// consumer that resolved the wrong prefix. +/// +/// - [`MenuSource::Gtk`]: a real `GMenuModel`. Items reference actions as +/// `app.xxx`/`win.xxx`; a consumer must insert two action groups, under +/// prefixes `"app"` and `"win"`, from [`GlobalMenu::app_path`]/ +/// [`GlobalMenu::window_path`] respectively. +/// - [`MenuSource::Unity`]: the older Ubuntu Unity-era export +/// (`_UNITY_OBJECT_PATH`, still relevant for some Qt platform-theme +/// builds). Items reference actions as `unity.xxx`, all against one +/// group at the menu's own path - a consumer inserts a single group +/// under prefix `"unity"` instead. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub enum MenuSource { + #[default] + Gtk, + Unity, +} + /// State of a single managed window. This is platform-independent: backends /// (X11, Wayland, ...) own the real surface/client handle and keep a `Window` /// in sync with it via `srdwm_core::WindowManager`. @@ -10,12 +65,26 @@ pub struct Window { pub id: WindowId, pub title: String, pub app_id: String, + /// X11 `WM_CLASS`'s *instance* half (`WM_CLASS` is `"instance\0class\0"`; + /// `app_id` above holds the class half, matching Wayland's `app_id` + /// concept). Always empty on Wayland/XWayland, which has no equivalent. + pub instance: String, pub geometry: Rect, /// Geometry to restore to when un-maximizing. pub restore_geometry: Option<Rect>, pub decorated: bool, + /// `decorated`'s value from just before entering fullscreen, restored + /// on exit - see `WindowManager::toggle_fullscreen`'s doc comment on + /// why this can't just hardcode `true` back. + pub restore_decorated: Option<bool>, pub floating: bool, pub minimized: bool, + /// Whether this window belongs to the scratchpad pool - see + /// `WindowManager::scratchpad_add`/`scratchpad_show`'s doc comments. + /// Persists across show/hide toggles (`minimized` is what actually + /// gates visibility); a window never sets this itself, only `srd.window. + /// scratchpad()`/the equivalent keybinding does. + pub scratchpad: bool, pub maximized: bool, pub fullscreen: bool, pub always_on_top: bool, @@ -23,6 +92,28 @@ pub struct Window { pub border_width: u32, pub workspace: usize, pub monitor: u32, + /// Whether `WindowManager`'s class/title-matched rules have already + /// been evaluated (and, if matched, applied) for this window. + /// + /// `add_window` matches rules once, at creation, but a native Wayland + /// client's `title`/`app_id` are still empty at that moment (they + /// arrive on a later commit - see the Wayland backend's + /// `sync_toplevel_metadata` doc comment); matching then would silently + /// fail every class-based rule. Left `false` so a backend can retry + /// the match once real identity is known, without ever re-matching + /// after that (rule actions apply once, not on every subsequent title + /// change). + pub rules_applied: bool, + /// Set by `WindowManager::toggle_maximize`/`toggle_fullscreen` to the + /// geometry `self.geometry` just moved *from*, whenever that move + /// should be animated. A backend's `sync_geometry` takes (reads and + /// clears) this once per change to start a tween toward the new + /// `geometry`; left `None` for changes that must track 1:1 instead + /// (interactive drag/resize), which never set it. + pub anim_from: Option<Rect>, + /// This window's global-menu D-Bus address, if the client has exported + /// one. See [`GlobalMenu`]'s own doc comment. + pub global_menu: Option<GlobalMenu>, } impl Window { @@ -31,11 +122,14 @@ impl Window { id, title: title.into(), app_id: String::new(), + instance: String::new(), geometry: Rect::new(0, 0, 640, 480), restore_geometry: None, decorated: true, + restore_decorated: None, floating: false, minimized: false, + scratchpad: false, maximized: false, fullscreen: false, always_on_top: false, @@ -43,6 +137,9 @@ impl Window { border_width: 2, workspace: 0, monitor: 0, + rules_applied: false, + anim_from: None, + global_menu: None, } } } @@ -59,6 +156,25 @@ pub const TITLEBAR_HEIGHT: u32 = 30; /// costs a few pixels of client edge; that is the right trade for making /// resize reliably grabbable without a keyboard. pub const RESIZE_MARGIN: i32 = 10; +/// Top-edge resize margin for an *undecorated* window specifically -- +/// narrower than [`RESIZE_MARGIN`] on purpose. +/// +/// An undecorated (client-side-decorated) window has no titlebar band for +/// srdwm to treat as a drag handle - Firefox's own tab strip, concretely -- +/// so the client's own header area sits directly at `frame.y` with nothing +/// srdwm-drawn to grab. The client detects a drag on its own header and +/// asks to be moved via `xdg_toplevel.move`, but only for clicks that +/// actually reach it as a normal button press; the full 10px `RESIZE_MARGIN` +/// swallowed every click within the first 10 rows of the window - including +/// most of a typical natural grab point near the top of a tab strip - as a +/// top-edge resize instead, so the client's own move request never fired. +/// Reported live as "can't drag-move Firefox from its own top bar." +/// Resize-from-the-top-edge still works (a deliberate earlier trade-off -- +/// see `undecorated_window_still_resizes_from_every_edge_including_top`'s +/// own comment - since an undecorated window is still a window), just from +/// a much narrower band that a click meant to grab the tab strip is very +/// unlikely to land in by accident. +pub const UNDECORATED_TOP_RESIZE_MARGIN: i32 = 3; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ResizeEdge { @@ -75,11 +191,38 @@ pub enum ResizeEdge { impl ResizeEdge { /// Determine which titlebar button (if any) a point within the titlebar /// band falls on. Buttons are laid out right-aligned: close, maximize, minimize. - pub fn hit_test(frame: Rect, x: i32, y: i32) -> Option<TitlebarHit> { - if !frame.contains_point(x, y) { + /// + /// `decorated` must reflect the window's *actual* current state, not + /// just whether it usually draws one: `frame` always reserves + /// `TITLEBAR_HEIGHT` at the top regardless of whether anything is drawn + /// there (placement never shrinks a window's allocated geometry just + /// because a rule or CSD negotiation later turns decoration off - see + /// `sync_geometry`'s own doc comment on that split). Applying the + /// titlebar-band/button logic unconditionally meant an *undecorated* + /// window's own content in that top band - Firefox's tab strip and URL + /// bar, concretely, once `decorated = false` actually started applying + /// to it - silently ate every click there as a phantom + /// drag/close/maximize/minimize hit instead of ever reaching the + /// client. Resize-from-edge still applies either way: an undecorated + /// window is still a window, and dragging its (invisible) edge to + /// resize is still expected to work. + pub fn hit_test(frame: Rect, x: i32, y: i32, decorated: bool, border_width: u32) -> Option<TitlebarHit> { + // Border strips render *outside* `frame` (`decoration:: + // border_strips`, `border_width` pixels past each edge) - without + // widening the containment check to match, those visible pixels + // were a dead zone: `frame.contains_point` rejected them outright, + // so hovering the border itself (not just just inside it) showed no + // resize cursor and couldn't be grabbed, even though it's what + // visually reads as the window's actual edge. `resize_edge_at` + // itself needs no matching change - its margin comparisons + // (`x <= frame.x + m`, etc.) already treat anything at or outside + // `frame`'s own edge as maximally "near", border pixels included. + let bw = border_width as i32; + let outer = Rect::new(frame.x - bw, frame.y - bw, frame.width + 2 * border_width, frame.height + 2 * border_width); + if !outer.contains_point(x, y) { return None; } - if y < frame.y + TITLEBAR_HEIGHT as i32 { + if decorated && y < frame.y + TITLEBAR_HEIGHT as i32 { const BUTTON: i32 = TITLEBAR_HEIGHT as i32; let right = frame.right(); if x >= right - BUTTON { @@ -93,15 +236,16 @@ impl ResizeEdge { } return Some(TitlebarHit::Drag); } - let edge = Self::resize_edge_at(frame, x, y)?; + let edge = Self::resize_edge_at(frame, x, y, decorated)?; Some(TitlebarHit::Resize(edge)) } - fn resize_edge_at(frame: Rect, x: i32, y: i32) -> Option<ResizeEdge> { + fn resize_edge_at(frame: Rect, x: i32, y: i32, decorated: bool) -> Option<ResizeEdge> { let m = RESIZE_MARGIN; + let top_m = if decorated { m } else { UNDECORATED_TOP_RESIZE_MARGIN }; let near_left = x <= frame.x + m; let near_right = x >= frame.right() - m; - let near_top = y <= frame.y + m; + let near_top = y <= frame.y + top_m; let near_bottom = y >= frame.bottom() - m; Some(match (near_left, near_right, near_top, near_bottom) { (true, _, true, _) => ResizeEdge::TopLeft, @@ -110,6 +254,7 @@ impl ResizeEdge { (_, true, _, true) => ResizeEdge::BottomRight, (true, false, false, false) => ResizeEdge::Left, (false, true, false, false) => ResizeEdge::Right, + (false, false, true, false) => ResizeEdge::Top, (false, false, false, true) => ResizeEdge::Bottom, _ => return None, }) @@ -173,14 +318,14 @@ mod tests { #[test] fn close_button_is_top_right_corner_of_titlebar() { let f = frame(); - let hit = ResizeEdge::hit_test(f, f.right() - 5, f.y + 5); + let hit = ResizeEdge::hit_test(f, f.right() - 5, f.y + 5, true, 0); assert_eq!(hit, Some(TitlebarHit::Close)); } #[test] fn maximize_is_left_of_close() { let f = frame(); - let hit = ResizeEdge::hit_test(f, f.right() - TITLEBAR_HEIGHT as i32 - 5, f.y + 5); + let hit = ResizeEdge::hit_test(f, f.right() - TITLEBAR_HEIGHT as i32 - 5, f.y + 5, true, 0); assert_eq!(hit, Some(TitlebarHit::Maximize)); } @@ -188,21 +333,90 @@ mod tests { fn middle_of_titlebar_is_drag() { let f = frame(); let (cx, _) = f.center(); - let hit = ResizeEdge::hit_test(f, cx, f.y + 5); + let hit = ResizeEdge::hit_test(f, cx, f.y + 5, true, 0); assert_eq!(hit, Some(TitlebarHit::Drag)); } #[test] fn bottom_right_corner_is_resize() { let f = frame(); - let hit = ResizeEdge::hit_test(f, f.right() - 1, f.bottom() - 1); + let hit = ResizeEdge::hit_test(f, f.right() - 1, f.bottom() - 1, true, 0); assert_eq!(hit, Some(TitlebarHit::Resize(ResizeEdge::BottomRight))); } + /// The bug this guards against: an undecorated window's own content in + /// its top `TITLEBAR_HEIGHT` band (Firefox's tab strip/URL bar, once + /// `decorated = false` actually applies to it) was silently swallowed + /// as a phantom drag hit instead of ever reaching the client, since the + /// titlebar-band check used to run unconditionally. + #[test] + fn undecorated_window_has_no_titlebar_band() { + let f = frame(); + let (cx, _) = f.center(); + // Inside the old phantom titlebar band (< TITLEBAR_HEIGHT) but + // outside RESIZE_MARGIN, so a real resize edge can't also explain a + // `None` here - undecorated, this must not be treated as + // decoration (or a resize edge) at all, just plain content. + let hit = ResizeEdge::hit_test(f, cx, f.y + 20, false, 0); + assert_eq!(hit, None); + } + + #[test] + fn undecorated_window_still_resizes_from_every_edge_including_top() { + let f = frame(); + let (cx, _) = f.center(); + let hit = ResizeEdge::hit_test(f, cx, f.y + 1, false, 0); + assert_eq!(hit, Some(TitlebarHit::Resize(ResizeEdge::Top))); + } + + #[test] + fn undecorated_top_resize_band_is_much_narrower_than_decorated() { + // Regression test: an undecorated window's own header (Firefox's + // tab strip, concretely) has no srdwm-drawn titlebar to grab, so a + // click meant to drag-move it via the client's own `xdg_toplevel. + // move` has to actually reach the client - the full `RESIZE_MARGIN` + // (10px) swallowed most of a natural grab point near the top as a + // resize instead. A *decorated* window's top band is unaffected -- + // it already has TITLEBAR_HEIGHT worth of unambiguous drag space + // above where `RESIZE_MARGIN` even starts to matter. + let f = frame(); + let (cx, _) = f.center(); + assert_eq!(ResizeEdge::hit_test(f, cx, f.y + 5, false, 0), None, "5px in: past the narrow undecorated band, must reach the client"); + assert_eq!( + ResizeEdge::hit_test(f, cx, f.y + 5, true, 0), + Some(TitlebarHit::Drag), + "decorated: 5px in is still well inside the titlebar band, not a resize edge" + ); + } + #[test] fn outside_frame_is_none() { let f = frame(); - assert_eq!(ResizeEdge::hit_test(f, 0, 0), None); + assert_eq!(ResizeEdge::hit_test(f, 0, 0, true, 0), None); + } + + #[test] + fn border_pixels_are_hoverable_not_a_dead_zone() { + // Regression test: `decoration::border_strips` draws the border + // `border_width` pixels *outside* `frame`, but hit-testing only + // checked `frame` itself - so the visible border was a dead zone + // that showed no resize cursor and couldn't be grabbed, even + // though it's what visually reads as the window's edge. + let f = frame(); + let (_, cy) = f.center(); + let border_width = 2; + // One pixel into the border strip, past the left edge. + let x = f.x - 1; + assert_eq!(ResizeEdge::hit_test(f, x, cy, true, 0), None, "sanity check: with no border, this point really is outside the window"); + assert_eq!( + ResizeEdge::hit_test(f, x, cy, true, border_width), + Some(TitlebarHit::Resize(ResizeEdge::Left)), + "one pixel into the actual drawn border must still register as the left edge" + ); + // Just past the border entirely (border_width + 1 outside frame) is + // still nothing - the fix widens the dead zone's boundary, it + // doesn't remove it. + assert_eq!(ResizeEdge::hit_test(f, f.x - border_width as i32 - 1, cy, true, border_width), None); } #[test] |