diff options
| author | srdusr <[email protected]> | 2025-02-15 14:56:00 +0200 |
|---|---|---|
| committer | srdusr <[email protected]> | 2025-02-15 14:56:00 +0200 |
| commit | 0a4c4b5941fe982ccb3d3175e26d9d83f0025ffd (patch) | |
| tree | 7672d0af277664f457c6c9462925c0005fe35dcf /crates/x11/src | |
| parent | 413daa7ba2ea0ebd1424c024fd0566423aaea3f8 (diff) | |
| download | srdwm-0a4c4b5941fe982ccb3d3175e26d9d83f0025ffd.tar.gz srdwm-0a4c4b5941fe982ccb3d3175e26d9d83f0025ffd.zip | |
Checkpoint: preserve all uncommitted rust-rewrite worktree work
Safety commit before reconciling this worktree with main, which has
diverged with its own separate fixes today. Nothing here is reviewed
or curated yet - this exists purely so none of this work can be lost
to a git operation, disk issue, or worktree cleanup while that
reconciliation happens.
Diffstat (limited to 'crates/x11/src')
| -rw-r--r-- | crates/x11/src/platform/connect.rs | 3 | ||||
| -rw-r--r-- | crates/x11/src/platform/events.rs | 39 | ||||
| -rw-r--r-- | crates/x11/src/platform/mod.rs | 52 | ||||
| -rw-r--r-- | crates/x11/src/platform/struts.rs | 222 | ||||
| -rw-r--r-- | crates/x11/src/platform/tests.rs | 49 | ||||
| -rw-r--r-- | crates/x11/src/platform/trait_impl.rs | 27 |
6 files changed, 388 insertions, 4 deletions
diff --git a/crates/x11/src/platform/connect.rs b/crates/x11/src/platform/connect.rs index 2d2f726..060b3d2 100644 --- a/crates/x11/src/platform/connect.rs +++ b/crates/x11/src/platform/connect.rs @@ -19,6 +19,8 @@ impl X11Platform { atoms._NET_WM_STATE_MAXIMIZED_HORZ, atoms._NET_CLIENT_LIST, atoms._NET_ACTIVE_WINDOW, + atoms._NET_WM_STRUT, + atoms._NET_WM_STRUT_PARTIAL, ]).map_err(err)?; let font = conn.generate_id().map_err(err)?; @@ -95,6 +97,7 @@ impl X11Platform { numlock_mask, ipc, appmenu_registrar: Some(srdwm_platform::AppmenuRegistrarState::new()), + struts: HashMap::new(), }) } diff --git a/crates/x11/src/platform/events.rs b/crates/x11/src/platform/events.rs index ef970f1..30e3f8d 100644 --- a/crates/x11/src/platform/events.rs +++ b/crates/x11/src/platform/events.rs @@ -31,8 +31,43 @@ impl X11Platform { self.conn.flush().map_err(err)?; Ok(None) } - XEvent::UnmapNotify(ev) => Ok(self.unmanage(ev.window)), - XEvent::DestroyNotify(ev) => Ok(self.unmanage(ev.window)), + XEvent::UnmapNotify(ev) => { + let struts_changed = self.forget_strut_window(ev.window); + let unmanaged = self.unmanage(ev.window); + // `unmanage`'s own `Event::WindowDestroyed` wins if this + // window was somehow both a managed client *and* a tracked + // strut window (shouldn't happen in practice - see + // `track_strut_window`'s own doc comment - but `unmanage` + // returning `Some` either way takes priority, since a + // closed window matters more to core than a reservation + // change on the very same event). + Ok(unmanaged.or_else(|| struts_changed.then(struts::monitors_changed_event))) + } + XEvent::DestroyNotify(ev) => { + let struts_changed = self.forget_strut_window(ev.window); + let unmanaged = self.unmanage(ev.window); + Ok(unmanaged.or_else(|| struts_changed.then(struts::monitors_changed_event))) + } + // Only a window we aren't already managing as a regular + // top-level client - see `track_strut_window`'s own doc + // comment for why a real panel/dock typically needs this path + // specifically (override-redirect, so it never goes through + // `manage_new_window`/`MapRequest` at all). Fires for *every* + // other window that maps too (a client's own subwindows, an + // override-redirect tooltip/menu popup), but `read_strut` + // inside it is a cheap couple of `GetProperty` round-trips + // that come back empty for anything that never sets the + // property, so this costs nothing beyond that for the common + // case of "not a bar". + XEvent::MapNotify(ev) => { + if self.xid_to_core.contains_key(&ev.window) { + return Ok(None); + } + Ok(self.track_strut_window(ev.window).then(struts::monitors_changed_event)) + } + XEvent::PropertyNotify(ev) if ev.atom == self.atoms._NET_WM_STRUT || ev.atom == self.atoms._NET_WM_STRUT_PARTIAL => { + Ok(self.update_strut_property(ev.window).then(struts::monitors_changed_event)) + } XEvent::ButtonPress(ev) => { let (x, y) = (ev.root_x as i32, ev.root_y as i32); let hit = self.wm.borrow().hit_test(x, y); diff --git a/crates/x11/src/platform/mod.rs b/crates/x11/src/platform/mod.rs index 04c918c..8ca2299 100644 --- a/crates/x11/src/platform/mod.rs +++ b/crates/x11/src/platform/mod.rs @@ -54,6 +54,8 @@ x11rb::atom_manager! { _NET_WM_STATE_MAXIMIZED_HORZ, _NET_CLIENT_LIST, _NET_ACTIVE_WINDOW, + _NET_WM_STRUT, + _NET_WM_STRUT_PARTIAL, UTF8_STRING, // Global-menu properties - see `read_global_menu`'s doc comment. // A native X11 client is exactly the same GTK/Qt app the Wayland @@ -84,6 +86,36 @@ struct Frame { supports_delete: bool, } +/// One window's own `_NET_WM_STRUT_PARTIAL` (or the older, span-free +/// `_NET_WM_STRUT`) reservation - the X11 equivalent of a Wayland +/// layer-shell surface's exclusive zone (`zwlr_layer_surface_v1::set_ +/// exclusive_zone`, read on the Wayland backends via `layer_map_for_ +/// output(...).non_exclusive_zone()`). At most one edge is ever nonzero +/// for a real panel/dock (a bar reserves *one* strip, not several), but +/// the property itself allows all four at once, so all four are kept. +/// +/// Values are in root-window (screen-global) pixels, matching every other +/// X11 geometry value in this backend - there is no separate logical/ +/// physical scale to convert between the way the Wayland backends' own +/// `zone_physical` conversion needs (`udev/platform.rs`'s `monitors()`), +/// since this compositor's X11 backend has no independent output-scale +/// concept at all. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +struct Strut { + left: u32, + right: u32, + top: u32, + bottom: u32, + left_start_y: i32, + left_end_y: i32, + right_start_y: i32, + right_end_y: i32, + top_start_x: i32, + top_end_x: i32, + bottom_start_x: i32, + bottom_end_x: i32, +} + fn err(e: impl std::fmt::Display) -> PlatformError { PlatformError::Other(e.to_string()) } @@ -150,6 +182,22 @@ pub struct X11Platform { /// D-Bus service itself can independently fail (see that module's own /// `None` handling). appmenu_registrar: Option<srdwm_platform::AppmenuRegistrarState>, + /// Every currently-mapped window's own `_NET_WM_STRUT_PARTIAL`/`_NET_ + /// WM_STRUT` reservation, keyed by its X window id - populated on + /// `MapNotify` (see `events.rs`) and kept fresh via `PropertyNotify` + /// on the same two atoms, removed on `UnmapNotify`/`DestroyNotify`. + /// Read by `monitors()` to shrink each monitor's own usable rect the + /// same way the Wayland backends' layer-shell exclusive zones already + /// do - see that method's own doc comment. Deliberately keyed on + /// *any* mapped window that sets the property, not gated on `_NET_WM_ + /// WINDOW_TYPE_DOCK` specifically: the EWMH spec's actual reservation + /// mechanism is the strut property itself, and real struts also come + /// from override-redirect panels that never go through `manage_new_ + /// window`/`xid_to_core` at all (confirmed live by a peer session's + /// own aegis bar, an override-redirect `_NET_WM_WINDOW_TYPE_DOCK` + /// window) - so this can't be folded into the existing managed- + /// window bookkeeping. + struts: HashMap<XWindow, Strut>, } @@ -169,8 +217,12 @@ mod actions; mod connect; mod events; mod global_menu; +mod struts; mod trait_impl; mod window; #[cfg(test)] +use struts::usable_rect; + +#[cfg(test)] mod tests; diff --git a/crates/x11/src/platform/struts.rs b/crates/x11/src/platform/struts.rs new file mode 100644 index 0000000..1f58192 --- /dev/null +++ b/crates/x11/src/platform/struts.rs @@ -0,0 +1,222 @@ +//! `_NET_WM_STRUT_PARTIAL`/`_NET_WM_STRUT` tracking - the X11 half of +//! monitor usable-area reservation, matching what the Wayland backends +//! already do for a `zwlr_layer_shell_v1` bar/dock's exclusive zone (see +//! `udev/platform.rs`'s `monitors()` and its own `non_exclusive_zone` +//! doc comment). Confirmed missing entirely, live, by a peer session +//! (`aegis`) building its own X11-backend bar: an override-redirect +//! `_NET_WM_WINDOW_TYPE_DOCK` window setting a correct `_NET_WM_STRUT_ +//! PARTIAL` never shrank `srd monitors`' own reported usable rect at +//! all, so a real tiled client would have had its window placed right +//! underneath the bar. `grep -rl STRUT crates/x11/src` found nothing at +//! all before this file - the feature had simply never been built for +//! this backend, not a narrower bug in existing logic. + +use super::*; + +/// A strut reservation changed, so whatever `monitors()` would report has +/// too - `crates/srdwm/src/main.rs`'s own event loop already re-queries +/// `Platform::monitors()` and refreshes `WindowManager`'s cached list on +/// exactly this event (it's how output hotplug is handled), so reusing it +/// here is what actually makes a fresh strut reservation visible to `srd +/// monitors`/tiling/placement at all - the cached list otherwise only +/// changes on a real hotplug, never on a dock mapping or resizing itself. +/// The zero-sized placeholder `Monitor` is never read: that event handler +/// discards the payload and re-queries the real list unconditionally, the +/// same "cheap sentinel, ignored on arrival" shape the Wayland backends' +/// own equivalent trigger already uses (`layer_shell.rs`'s `layer_ +/// destroyed`, on an exclusive-zone change). +pub(super) fn monitors_changed_event() -> Event { + Event::MonitorAdded(Monitor::new(0, "", Rect::new(0, 0, 0, 0))) +} + +impl X11Platform { + /// Reads `window`'s current strut reservation, preferring `_NET_WM_ + /// STRUT_PARTIAL` (12 `CARDINAL`s: left, right, top, bottom, then each + /// edge's own start/end span) and falling back to the older, span-free + /// `_NET_WM_STRUT` (4 `CARDINAL`s: left, right, top, bottom) for a + /// client that only sets that - per the EWMH spec, a plain `_STRUT` + /// with no `_PARTIAL` reserves its margin across the *entire* length + /// of that edge, which is what defaulting its span to `0..root extent` + /// below encodes. `None` if neither property is set, or both are + /// present but empty/malformed - the same "nothing reserved" answer + /// either way. + pub(super) fn read_strut(&self, window: XWindow) -> Option<Strut> { + if let Ok(cookie) = self.conn.get_property(false, window, self.atoms._NET_WM_STRUT_PARTIAL, x11rb::protocol::xproto::AtomEnum::CARDINAL, 0, 12) { + if let Ok(reply) = cookie.reply() { + if let Some(v) = reply.value32().map(|it| it.collect::<Vec<u32>>()) { + if v.len() >= 12 { + let s = Strut { + left: v[0], + right: v[1], + top: v[2], + bottom: v[3], + left_start_y: v[4] as i32, + left_end_y: v[5] as i32, + right_start_y: v[6] as i32, + right_end_y: v[7] as i32, + top_start_x: v[8] as i32, + top_end_x: v[9] as i32, + bottom_start_x: v[10] as i32, + bottom_end_x: v[11] as i32, + }; + return if s == Strut::default() { None } else { Some(s) }; + } + } + } + } + let Ok(cookie) = self.conn.get_property(false, window, self.atoms._NET_WM_STRUT, x11rb::protocol::xproto::AtomEnum::CARDINAL, 0, 4) else { + return None; + }; + let reply = cookie.reply().ok()?; + let v: Vec<u32> = reply.value32()?.collect(); + if v.len() < 4 || v[..4] == [0, 0, 0, 0] { + return None; + } + let screen = &self.conn.setup().roots[0]; + let (w, h) = (screen.width_in_pixels as i32, screen.height_in_pixels as i32); + Some(Strut { + left: v[0], + right: v[1], + top: v[2], + bottom: v[3], + left_start_y: 0, + left_end_y: h, + right_start_y: 0, + right_end_y: h, + top_start_x: 0, + top_end_x: w, + bottom_start_x: 0, + bottom_end_x: w, + }) + } + + /// Called on `MapNotify` for any window this backend isn't already + /// managing as a regular client (see that call site's own comment for + /// why: a real panel/dock is typically override-redirect specifically + /// to skip window management entirely, so it never reaches `manage_ + /// new_window`/`xid_to_core` the way an ordinary top-level does). + /// Selecting `PROPERTY_CHANGE` here is what makes a later live resize + /// of the bar (`update_strut_property`, on `PropertyNotify`) actually + /// get noticed - without it, only the reservation this window had at + /// the moment it first mapped would ever be seen. Returns `true` when + /// a real reservation was found, so the caller can fire the same + /// `Event::MonitorAdded` re-query trigger `monitors()`'s own live + /// hotplug path already uses (see that call site's own comment for + /// why this can't just call `monitors()` again itself - the cached + /// list `srd monitors` actually reads lives in `WindowManager`, one + /// layer up, not in this struct). + pub(super) fn track_strut_window(&mut self, window: XWindow) -> bool { + let Some(strut) = self.read_strut(window) else { return false }; + self.struts.insert(window, strut); + let _ = self.conn.change_window_attributes(window, &ChangeWindowAttributesAux::new().event_mask(EventMask::PROPERTY_CHANGE)); + let _ = self.conn.flush(); + true + } + + /// `PropertyNotify` on `_NET_WM_STRUT`/`_NET_WM_STRUT_PARTIAL` for a + /// window already being watched (`track_strut_window` selected the + /// event mask that makes this fire at all) - re-reads and either + /// updates or drops the reservation, matching whatever the client's + /// new property value actually says (a bar shrinking its own reserved + /// strip live, or clearing the property to reserve nothing at all). + /// Returns `true` only when the reservation actually changed (not + /// merely present) - see `track_strut_window`'s own doc comment for + /// what the caller does with that. + pub(super) fn update_strut_property(&mut self, window: XWindow) -> bool { + let new = self.read_strut(window); + let changed = self.struts.get(&window).copied() != new; + match new { + Some(strut) => { + self.struts.insert(window, strut); + } + None => { + self.struts.remove(&window); + } + } + changed + } + + /// `UnmapNotify`/`DestroyNotify` for a tracked strut window - a no-op + /// `HashMap::remove` for every other window, so this is safe to call + /// unconditionally from both handlers rather than needing its own + /// "was this actually a strut window" check first. Returns `true` + /// only when a real reservation actually existed and was removed -- + /// see `track_strut_window`'s own doc comment for what the caller + /// does with that. + pub(super) fn forget_strut_window(&mut self, window: XWindow) -> bool { + self.struts.remove(&window).is_some() + } + + /// `full`, shrunk by every tracked strut whose reserved band actually + /// overlaps it - the X11 equivalent of `udev/platform.rs`'s `non_ + /// exclusive_zone`-based `usable` computation, called once per + /// monitor from `monitors()`. See the free [`usable_rect`] function + /// below (the actual math, kept separate so it's unit-testable + /// without a live X11 connection) for exactly how. + pub(super) fn usable_rect_for(&self, full: Rect) -> Rect { + let screen = &self.conn.setup().roots[0]; + let screen_size = (screen.width_in_pixels as i32, screen.height_in_pixels as i32); + usable_rect(full, screen_size, self.struts.values().copied()) + } +} + +/// The actual shrink math behind [`X11Platform::usable_rect_for`], pulled +/// out as a free function of plain values (no live X11 connection needed) +/// specifically so it can be unit tested directly - `usable_rect_for` +/// itself needs `self.conn.setup()` for the root screen's own dimensions, +/// which only a real connected server can answer. +/// +/// Every strut value is a distance in from the *screen's* own edge +/// (ICCCM/EWMH `_NET_WM_STRUT_PARTIAL`: `top` reserves root-absolute +/// `y ∈ [0, top)`, `bottom` reserves `y ∈ [screen_height - bottom, +/// screen_height)`, and so on) - not from this monitor's own edge, so +/// the reserved boundary is computed in root-absolute coordinates first +/// (needing `screen_size` for the bottom/right cases) and only then +/// intersected against `full`. A strut anchored on an edge this monitor +/// doesn't border at all, or whose own start/end span doesn't overlap +/// this monitor's extent on the perpendicular axis (a bar on a different +/// monitor entirely, in a multi-monitor setup), correctly contributes no +/// shrink either way, since its reserved boundary then falls outside +/// `full` on that axis. +pub(super) fn usable_rect(full: Rect, screen_size: (i32, i32), struts: impl Iterator<Item = Strut>) -> Rect { + let (screen_w, screen_h) = screen_size; + let (full_left, full_top) = (full.x, full.y); + let (full_right, full_bottom) = (full.x + full.width as i32, full.y + full.height as i32); + let (mut left, mut top, mut right, mut bottom) = (full_left, full_top, full_right, full_bottom); + for strut in struts { + if strut.top > 0 && strut.top_end_x > full_left && strut.top_start_x < full_right { + top = top.max(strut.top as i32); + } + if strut.bottom > 0 && strut.bottom_end_x > full_left && strut.bottom_start_x < full_right { + bottom = bottom.min(screen_h - strut.bottom as i32); + } + if strut.left > 0 && strut.left_end_y > full_top && strut.left_start_y < full_bottom { + left = left.max(strut.left as i32); + } + if strut.right > 0 && strut.right_end_y > full_top && strut.right_start_y < full_bottom { + right = right.min(screen_w - strut.right as i32); + } + } + // Clamped against the monitor's own opposite edge, not just `0`: a + // strut reservation declared against the *whole screen* can still + // exceed a single monitor's own extent in a multi-monitor layout + // (e.g. a bar `top=32` on a screen where this particular monitor's + // usable band is otherwise smaller than that) - without this, an + // overshoot on one axis could invert `left > right`/`top > bottom` + // into a negative-size rect instead of clamping to "no usable space + // left on this monitor". Each axis's two edges are clamped as + // separate statements, in order (`left` before `right`, `top` before + // `bottom`), not one combined tuple `let` - a first version of this + // used `let (top, bottom) = (top.min(full_bottom), bottom.max(top))` + // in one statement, which reads `top` on the right-hand side of both + // tuple elements from the *pre-clamp* binding (a `let` only shadows + // once the whole statement finishes), silently clamping `bottom` + // against the wrong, oversized `top` - caught by a test asserting + // `usable.height == 0` for a strut taller than the monitor, which + // instead came back as `400`, not `0`. + let left = left.min(full_right); + let right = right.max(full_left).max(left); + let top = top.min(full_bottom); + let bottom = bottom.max(full_top).max(top); + Rect::new(left, top, (right - left).max(0) as u32, (bottom - top).max(0) as u32) +} diff --git a/crates/x11/src/platform/tests.rs b/crates/x11/src/platform/tests.rs index 61d8124..7af5455 100644 --- a/crates/x11/src/platform/tests.rs +++ b/crates/x11/src/platform/tests.rs @@ -46,3 +46,52 @@ let keycodes = modmap(2, &[(3, 50)]); assert_eq!(modmask_for_keycode_in_mod_slots(99, 2, &keycodes), ModMask::from(0u16)); } + + fn top_strut(height: u32, start_x: i32, end_x: i32) -> Strut { + Strut { top: height, top_start_x: start_x, top_end_x: end_x, ..Strut::default() } + } + + #[test] + fn a_top_bar_shrinks_the_monitor_it_actually_spans() { + let full = Rect::new(0, 0, 1920, 1080); + let strut = top_strut(32, 0, 1920); + let usable = usable_rect(full, (1920, 1080), std::iter::once(strut)); + assert_eq!(usable, Rect::new(0, 32, 1920, 1048)); + } + + #[test] + fn a_bar_confined_to_a_different_monitor_leaves_this_one_alone() { + // A 1920-wide bar sitting entirely over the first monitor + // (x 0..1920) must not shrink a second monitor placed to its + // right (x 1920..3840) - struts are screen-global, not + // monitor-relative, so this is the only thing that tells them + // apart. + let second_monitor = Rect::new(1920, 0, 1920, 1080); + let strut = top_strut(32, 0, 1920); + let usable = usable_rect(second_monitor, (3840, 1080), std::iter::once(strut)); + assert_eq!(usable, second_monitor); + } + + #[test] + fn a_bottom_strut_is_measured_from_the_screen_bottom_not_the_monitor() { + let full = Rect::new(0, 0, 1920, 1080); + let strut = Strut { bottom: 40, bottom_start_x: 0, bottom_end_x: 1920, ..Strut::default() }; + let usable = usable_rect(full, (1920, 1080), std::iter::once(strut)); + assert_eq!(usable, Rect::new(0, 0, 1920, 1040)); + } + + #[test] + fn a_strut_larger_than_the_monitor_clamps_to_zero_size_not_a_negative_one() { + let full = Rect::new(0, 0, 800, 600); + let strut = top_strut(1000, 0, 800); + let usable = usable_rect(full, (800, 600), std::iter::once(strut)); + assert_eq!(usable.height, 0); + assert!(usable.width > 0, "only the top edge was reserved, the sides must stay untouched"); + } + + #[test] + fn no_struts_at_all_leaves_the_monitor_exactly_as_reported() { + let full = Rect::new(100, 50, 1024, 768); + let usable = usable_rect(full, (1920, 1080), std::iter::empty()); + assert_eq!(usable, full); + } diff --git a/crates/x11/src/platform/trait_impl.rs b/crates/x11/src/platform/trait_impl.rs index 1ff7845..b45a4aa 100644 --- a/crates/x11/src/platform/trait_impl.rs +++ b/crates/x11/src/platform/trait_impl.rs @@ -56,14 +56,37 @@ impl Platform for X11Platform { continue; } let name = String::from_utf8_lossy(&info.name).to_string(); - let mut m = Monitor::new(i as u32, name, Rect::new(crtc.x as i32, crtc.y as i32, crtc.width as u32, crtc.height as u32)); + let full = Rect::new(crtc.x as i32, crtc.y as i32, crtc.width as u32, crtc.height as u32); + // Shrunk by whatever `_NET_WM_STRUT`/`_NET_WM_STRUT_PARTIAL` + // a panel/dock has reserved - see `struts::usable_rect_for`'s + // own doc comment; this is the X11 mirror of `udev/platform. + // rs`'s `monitors()` shrinking by `non_exclusive_zone()`. + // Reporting the full head size here otherwise means core's + // placement/tiling treats a bar's own reserved strip as + // ordinary free space, exactly the gap a peer session (aegis) + // found and reported live building its own X11-backend bar. + let usable = self.usable_rect_for(full); + let mut m = Monitor::new(i as u32, name, usable); + m.full_geometry = full; + // Same as `geometry`, not top-only - see `udev/platform.rs`'s + // own `maximize_geometry_for` doc comment for why a dock on + // any edge, not just a top menu bar, should keep maximize + // from covering it: no edge actually benefits from the + // top-only distinction the way a plain top menu bar once did, + // and respecting every edge here is what every mainstream + // desktop's own maximize convention already does. + m.maximize_geometry = usable; m.primary = i == 0; monitors.push(m); } if monitors.is_empty() { let screen = &self.conn.setup().roots[0]; + let full = Rect::new(0, 0, screen.width_in_pixels as u32, screen.height_in_pixels as u32); + let usable = self.usable_rect_for(full); monitors.push({ - let mut m = Monitor::new(0, "default", Rect::new(0, 0, screen.width_in_pixels as u32, screen.height_in_pixels as u32)); + let mut m = Monitor::new(0, "default", usable); + m.full_geometry = full; + m.maximize_geometry = usable; m.primary = true; m }); |