srdusr
aboutsummaryrefslogtreecommitdiffstats
path: root/crates/x11
diff options
context:
space:
mode:
authorsrdusr <[email protected]>2025-08-25 11:47:00 +0200
committersrdusr <[email protected]>2025-08-25 11:47:00 +0200
commitbd7141718901c6f41511137e4b34a3bd9e2705b1 (patch)
tree672811318065eb97a822c1f7cec97b001a336900 /crates/x11
parent72e787a706d372c9a92e424e9b13ece929cb2bce (diff)
downloadsrdwm-bd7141718901c6f41511137e4b34a3bd9e2705b1.tar.gz
srdwm-bd7141718901c6f41511137e4b34a3bd9e2705b1.zip
X11 backend: right-click titlebar window menu, matching Wayland's own
Closes the one real gap an X11/Wayland feature-parity audit found this session (desktop icons, window-position memory, and static exclusive-zone reservation were already shared or Wayland-only by nature - see docs/TODO.md's own audit entry for the full breakdown). MenuAction/ContextMenu (row set, labels, row_at hit-testing) move from crates/wayland/src/context_menu.rs into crates/core/src/context_menu.rs -- pure state and geometry with nothing Wayland-specific in it, so X11 needing the same rows is shared data, not duplicated logic. The Wayland crate's own context_menu.rs is now a one-line re-export so every existing crate::context_menu::... call site keeps working unchanged. X11 has no compositor-level input dispatch to intercept every click the way Wayland's input/pointer.rs does, so the X11 side (crates/x11/src/platform/context_menu.rs, new) draws the menu into its own small override-redirect popup window and grabs the pointer for the duration so a click anywhere dismisses it, matching the Wayland backend's own convention. events.rs's ButtonPress handler now reads the real button number instead of hardcoding every press as a left click - a real latent bug (right-clicking a titlebar button would have silently performed its left-click action). Live-verified end to end in an isolated Xvfb + srdwm --x11 instance: full row set including the workspace picker, Minimize runs and closes the menu, a second window's menu dismisses cleanly on outside click, normal focus/click behaviour continues working afterward. See docs/TODO.md for the full investigation and verification narrative.
Diffstat (limited to 'crates/x11')
-rw-r--r--crates/x11/src/platform/connect.rs1
-rw-r--r--crates/x11/src/platform/context_menu.rs153
-rw-r--r--crates/x11/src/platform/events.rs61
-rw-r--r--crates/x11/src/platform/mod.rs7
-rw-r--r--crates/x11/src/platform/window.rs7
5 files changed, 222 insertions, 7 deletions
diff --git a/crates/x11/src/platform/connect.rs b/crates/x11/src/platform/connect.rs
index 060b3d2..ee8906d 100644
--- a/crates/x11/src/platform/connect.rs
+++ b/crates/x11/src/platform/connect.rs
@@ -98,6 +98,7 @@ impl X11Platform {
ipc,
appmenu_registrar: Some(srdwm_platform::AppmenuRegistrarState::new()),
struts: HashMap::new(),
+ context_menu: None,
})
}
diff --git a/crates/x11/src/platform/context_menu.rs b/crates/x11/src/platform/context_menu.rs
new file mode 100644
index 0000000..9238f6a
--- /dev/null
+++ b/crates/x11/src/platform/context_menu.rs
@@ -0,0 +1,153 @@
+//! Right-click titlebar window menu - the X11 half of `srdwm_core::
+//! context_menu`. The Wayland backend renders this into a shared
+//! compositor buffer; X11 has no equivalent (there is no single "every
+//! surface" compositor here, just ordinary X windows), so this draws it
+//! into its own small override-redirect popup window with the same GC/
+//! font `redraw_decoration` already uses for the titlebar itself.
+//!
+//! Row set and hit-testing come from `srdwm_core::context_menu`, unchanged
+//! from the Wayland backend - see that module's own doc comment for why
+//! it moved there. Foreign-toplevel state broadcasting
+//! (`foreign_toplevel::send_state` on the Wayland side) has no X11
+//! equivalent - that protocol is Wayland-only - so `run_context_menu_
+//! action` below is otherwise the same action set with that one line
+//! dropped.
+
+use super::*;
+use srdwm_core::context_menu::{ContextMenu, MenuAction};
+use x11rb::protocol::xproto::{ChangeGCAux, CoordMode, Point};
+
+impl X11Platform {
+ /// Opens the menu for `window` with its top-left corner at `pos`
+ /// (root-window pixels, wherever the right-click landed), clamped so
+ /// it never opens off the right/bottom edge of the screen. Grabs the
+ /// pointer for the duration: X11 has no single compositor-level input
+ /// dispatch to intercept every click the way the Wayland backend's
+ /// `input/pointer.rs` does, so an active grab on the root window is
+ /// what makes "click anywhere else dismisses the menu" work at all --
+ /// without it, a click over some other client's window would go
+ /// straight to that client and never reach us.
+ pub(super) fn open_context_menu(&mut self, window: WindowId, pos: (i32, i32)) -> PlatformResult<()> {
+ let Some(mut menu) = ({
+ let wm = self.wm.borrow();
+ ContextMenu::open(&wm, window, pos)
+ }) else {
+ return Ok(());
+ };
+
+ let screen = &self.conn.setup().roots[0];
+ let (sw, sh) = (screen.width_in_pixels as i32, screen.height_in_pixels as i32);
+ let x = menu.pos.0.max(0).min((sw - menu.width as i32).max(0));
+ let y = menu.pos.1.max(0).min((sh - menu.height()).max(0));
+ menu.pos = (x, y);
+
+ let popup = self.conn.generate_id().map_err(err)?;
+ let aux = CreateWindowAux::new()
+ .override_redirect(1)
+ .event_mask(EventMask::EXPOSURE)
+ .background_pixel(screen.white_pixel);
+ self.conn
+ .create_window(COPY_DEPTH_FROM_PARENT, popup, self.root, x as i16, y as i16, menu.width as u16, menu.height() as u16, 0, WindowClass::INPUT_OUTPUT, 0, &aux)
+ .map_err(err)?;
+ self.conn.map_window(popup).map_err(err)?;
+ self.conn.configure_window(popup, &ConfigureWindowAux::new().stack_mode(StackMode::ABOVE)).map_err(err)?;
+
+ // `owner_events: false` - every button event while this grab is
+ // active is reported against `grab_window` (root) regardless of
+ // which window the pointer is physically over, carrying the same
+ // absolute `root_x`/`root_y` the normal per-window path already
+ // uses for hit-testing. Best-effort: a failed grab (another
+ // client already holds one, vanishingly rare in practice) still
+ // leaves the menu visible and closeable by re-clicking its own
+ // rows, just without the "click elsewhere" dismissal.
+ let _ = self
+ .conn
+ .grab_pointer(false, self.root, EventMask::BUTTON_PRESS, GrabMode::ASYNC, GrabMode::ASYNC, x11rb::NONE, x11rb::NONE, x11rb::CURRENT_TIME)
+ .map_err(err)?
+ .reply();
+ self.conn.flush().map_err(err)?;
+
+ self.context_menu = Some((menu, popup));
+ self.redraw_context_menu()
+ }
+
+ /// Repaints every row into the popup window - called once on open
+ /// (there's no live hover highlight here either, same as the Wayland
+ /// backend's own `render_context_menu`) and again on `Expose` (the
+ /// popup has no backing store, so anything that uncovers it needs a
+ /// real repaint, unlike the Wayland side where the buffer is already
+ /// composited).
+ pub(super) fn redraw_context_menu(&mut self) -> PlatformResult<()> {
+ let Some((menu, popup)) = &self.context_menu else { return Ok(()) };
+ let popup = *popup;
+ let (width, row_height, total_height) = (menu.width, menu.row_height, menu.height());
+ let theme = self.wm.borrow().theme;
+ let bg = rgb_to_pixel(theme.titlebar_bg);
+ let fg = rgb_to_pixel(theme.titlebar_fg_focused);
+
+ self.conn.change_gc(self.gc, &ChangeGCAux::new().foreground(bg)).map_err(err)?;
+ self.conn.poly_fill_rectangle(popup, self.gc, &[Rectangle { x: 0, y: 0, width: width as u16, height: total_height as u16 }]).map_err(err)?;
+ self.conn.change_gc(self.gc, &ChangeGCAux::new().foreground(fg).font(self.font)).map_err(err)?;
+
+ for (i, (label, action)) in menu.items.iter().enumerate() {
+ let row_y = i as i32 * row_height as i32;
+ if matches!(action, MenuAction::Separator) {
+ let mid = row_y + row_height as i32 / 2;
+ self.conn.poly_line(CoordMode::ORIGIN, popup, self.gc, &[Point { x: 8, y: mid as i16 }, Point { x: width as i16 - 8, y: mid as i16 }]).map_err(err)?;
+ continue;
+ }
+ self.conn.image_text8(popup, self.gc, 10, (row_y + 20) as i16, label.as_bytes()).map_err(err)?;
+ }
+ self.conn.flush().map_err(err)?;
+ Ok(())
+ }
+
+ /// Closes the currently-open menu, if any - a no-op otherwise, so
+ /// every dismissal path (a row picked, a click outside, the window it
+ /// belongs to closing underneath it) can call this unconditionally.
+ pub(super) fn close_context_menu(&mut self) -> PlatformResult<()> {
+ if let Some((_, popup)) = self.context_menu.take() {
+ self.conn.ungrab_pointer(x11rb::CURRENT_TIME).map_err(err)?;
+ self.conn.destroy_window(popup).map_err(err)?;
+ self.conn.flush().map_err(err)?;
+ }
+ Ok(())
+ }
+
+ /// Runs whichever action a click on `row` selected - the X11
+ /// equivalent of the Wayland backend's `state/menu.rs::run_context_
+ /// menu_action`, same action set minus the Wayland-only foreign-
+ /// toplevel broadcast.
+ pub(super) fn run_context_menu_action(&mut self, window: WindowId, action: MenuAction) -> PlatformResult<()> {
+ match action {
+ MenuAction::Minimize => {
+ self.wm.borrow_mut().minimize_window(window);
+ if let Some(frame) = self.frame_for(window) {
+ self.conn.unmap_window(frame).map_err(err)?;
+ }
+ }
+ MenuAction::ToggleMaximize => {
+ self.wm.borrow_mut().toggle_maximize(window);
+ self.sync_geometry(window)?;
+ }
+ MenuAction::ToggleFullscreen => {
+ self.wm.borrow_mut().toggle_fullscreen(window);
+ self.sync_geometry(window)?;
+ }
+ MenuAction::ToggleFloating => {
+ self.wm.borrow_mut().toggle_floating(window);
+ self.sync_geometry(window)?;
+ }
+ MenuAction::ToggleAlwaysOnTop => {
+ self.wm.borrow_mut().toggle_always_on_top(window);
+ }
+ MenuAction::MoveToWorkspace(workspace) => {
+ self.wm.borrow_mut().move_window_to_workspace(window, workspace);
+ }
+ MenuAction::Close => self.request_close(window)?,
+ MenuAction::Separator => {}
+ }
+ self.conn.flush().map_err(err)?;
+ Ok(())
+ }
+}
diff --git a/crates/x11/src/platform/events.rs b/crates/x11/src/platform/events.rs
index 30e3f8d..10f96a9 100644
--- a/crates/x11/src/platform/events.rs
+++ b/crates/x11/src/platform/events.rs
@@ -70,30 +70,70 @@ impl X11Platform {
}
XEvent::ButtonPress(ev) => {
let (x, y) = (ev.root_x as i32, ev.root_y as i32);
+ let button = match ev.detail {
+ 1 => MouseButton::Left,
+ 2 => MouseButton::Middle,
+ 3 => MouseButton::Right,
+ other => MouseButton::Other(other),
+ };
+
+ // The context menu, if open, captures every press: a click
+ // inside resolves whichever row it landed on, a click
+ // anywhere else just dismisses it - same "one click, one
+ // action" rule the Wayland backend's own input/pointer.rs
+ // follows, and why `open_context_menu` grabs the pointer
+ // (see that method's own doc comment for why X11 needs to).
+ if self.context_menu.is_some() {
+ let (row_action, menu_window) = {
+ let (menu, _) = self.context_menu.as_ref().unwrap();
+ (menu.row_at(x, y).map(|r| menu.items[r].1), menu.window)
+ };
+ match row_action {
+ Some(srdwm_core::context_menu::MenuAction::Separator) => {}
+ Some(action) => {
+ self.close_context_menu()?;
+ self.run_context_menu_action(menu_window, action)?;
+ }
+ None => self.close_context_menu()?,
+ }
+ return Ok(Some(Event::MouseButtonPress { button, x, y }));
+ }
+
let hit = self.wm.borrow().hit_test(x, y);
if let Some((id, hit)) = hit {
self.raise_and_focus(id)?;
- match hit {
- TitlebarHit::Drag => self.wm.borrow_mut().start_drag(id, x, y),
- TitlebarHit::Close => self.request_close(id)?,
- TitlebarHit::Maximize => {
+ match (button, hit) {
+ (MouseButton::Left, TitlebarHit::Drag) => self.wm.borrow_mut().start_drag(id, x, y),
+ (MouseButton::Left, TitlebarHit::Close) => self.request_close(id)?,
+ (MouseButton::Left, TitlebarHit::Maximize) => {
self.wm.borrow_mut().toggle_maximize(id);
self.sync_geometry(id)?;
}
- TitlebarHit::Minimize => {
+ (MouseButton::Left, TitlebarHit::Minimize) => {
self.wm.borrow_mut().minimize_window(id);
if let Some(frame) = self.frame_for(id) {
self.conn.unmap_window(frame).map_err(err)?;
}
}
- TitlebarHit::Resize(edge) => self.wm.borrow_mut().start_resize(id, edge, x, y),
+ (MouseButton::Left, TitlebarHit::Resize(edge)) => self.wm.borrow_mut().start_resize(id, edge, x, y),
+ // Right-click on the titlebar's own drag area: the
+ // window menu (minimize/maximize/fullscreen/
+ // floating/always-on-top/move-to-workspace/close).
+ // Any other button/hit combination - right-click
+ // on a button, middle-click anywhere - just
+ // raises/focuses (already done above) and does
+ // nothing further, matching real desktops (a
+ // right-click on a titlebar button is not itself
+ // a button action).
+ (MouseButton::Right, TitlebarHit::Drag) => self.open_context_menu(id, (x, y))?,
+ _ => {}
}
self.conn.flush().map_err(err)?;
}
// Let the click through to the client (we grabbed it SYNC).
self.conn.allow_events(x11rb::protocol::xproto::Allow::REPLAY_POINTER, ev.time).map_err(err)?;
self.conn.flush().map_err(err)?;
- Ok(Some(Event::MouseButtonPress { button: MouseButton::Left, x, y }))
+ Ok(Some(Event::MouseButtonPress { button, x, y }))
}
XEvent::ButtonRelease(ev) => {
let mut wm = self.wm.borrow_mut();
@@ -141,6 +181,13 @@ impl X11Platform {
Ok(Some(Event::KeyRelease { key_name, modifiers: Self::modifiers_from_state(ev.state.into()) }))
}
XEvent::Expose(ev) => {
+ // The popup has no backing store, unlike the compositor-
+ // side Wayland menu - anything that uncovers it needs a
+ // real repaint.
+ if self.context_menu.as_ref().is_some_and(|(_, popup)| *popup == ev.window) {
+ let _ = self.redraw_context_menu();
+ return Ok(None);
+ }
let target = self.frames.iter().find(|(_, f)| f.frame == ev.window).map(|(&id, _)| id);
if let Some(id) = target {
let w = self.wm.borrow().window(id).cloned_for_render();
diff --git a/crates/x11/src/platform/mod.rs b/crates/x11/src/platform/mod.rs
index 8ca2299..e22b3f7 100644
--- a/crates/x11/src/platform/mod.rs
+++ b/crates/x11/src/platform/mod.rs
@@ -198,6 +198,12 @@ pub struct X11Platform {
/// window) - so this can't be folded into the existing managed-
/// window bookkeeping.
struts: HashMap<XWindow, Strut>,
+ /// The currently-open right-click titlebar window menu, if any --
+ /// the built row set/geometry alongside the override-redirect popup
+ /// window it's drawn into. See `context_menu.rs`'s module doc comment
+ /// for why X11 needs its own popup window and pointer grab where the
+ /// Wayland backend just reads a compositor-space struct.
+ context_menu: Option<(srdwm_core::context_menu::ContextMenu, XWindow)>,
}
@@ -215,6 +221,7 @@ impl ClonedForRender for Option<&CoreWindow> {
mod actions;
mod connect;
+mod context_menu;
mod events;
mod global_menu;
mod struts;
diff --git a/crates/x11/src/platform/window.rs b/crates/x11/src/platform/window.rs
index 997981f..bce71ca 100644
--- a/crates/x11/src/platform/window.rs
+++ b/crates/x11/src/platform/window.rs
@@ -156,6 +156,13 @@ impl X11Platform {
if let Some(frame) = self.frames.remove(&id) {
let _ = self.conn.destroy_window(frame.frame);
}
+ // A closed window's own context menu (opened right before, say, a
+ // client that immediately quits) would otherwise dangle - its
+ // `MenuAction::Close`/etc. would target a `WindowId` `remove_
+ // window` below has already forgotten.
+ if self.context_menu.as_ref().is_some_and(|(menu, _)| menu.window == id) {
+ let _ = self.close_context_menu();
+ }
self.wm.borrow_mut().remove_window(id);
let _ = self.conn.flush();
Some(Event::WindowDestroyed(id))