diff options
| -rw-r--r-- | crates/wayland/src/lib.rs | 46 | ||||
| -rw-r--r-- | crates/wayland/src/xwayland.rs | 100 | ||||
| -rw-r--r-- | docs/IMPLEMENTATION_STATUS.md | 92 |
3 files changed, 197 insertions, 41 deletions
diff --git a/crates/wayland/src/lib.rs b/crates/wayland/src/lib.rs index b2fa9dc..507102e 100644 --- a/crates/wayland/src/lib.rs +++ b/crates/wayland/src/lib.rs @@ -562,6 +562,39 @@ fn handle_pointer_position(state: &mut CompState, pos: Point<f64, Logical>, time } } +/// The underlying `wl_surface` for a mapped window, regardless of whether +/// it's a native `xdg-shell` toplevel or an XWayland `X11Surface` -- +/// `desktop::Window` exposes these as two separate accessors with no +/// shared one. +fn dwindow_wl_surface(w: &DWindow) -> Option<WlSurface> { + if let Some(top) = w.toplevel() { + return Some(top.wl_surface().clone()); + } + w.x11_surface().and_then(|x| x.wl_surface()) +} + +/// Requests a client close its window, whichever kind it is. +fn close_dwindow(w: &DWindow) { + if let Some(top) = w.toplevel() { + top.send_close(); + } else if let Some(x11) = w.x11_surface() { + let _ = x11.close(); + } +} + +/// Focuses `id` in our own `WindowManager` *and* gives its surface real +/// Wayland/X11 keyboard focus - without this, a window can be raised and +/// tiled correctly yet never receive a single keystroke. +fn focus_window(state: &mut CompState, id: WindowId) { + state.wm.borrow_mut().focus_window(id); + state.pending.borrow_mut().push(CoreEvent::WindowFocused(id)); + let surface = state.id_to_window.get(&id).and_then(dwindow_wl_surface); + if let Some(keyboard) = state.seat.get_keyboard() { + let serial = SERIAL_COUNTER.next_serial(); + keyboard.set_focus(state, surface, serial); + } +} + fn handle_pointer_button(state: &mut CompState, pos: Point<f64, Logical>, button: u32, pressed: bool, time: u32) { const BTN_LEFT: u32 = 0x110; let serial = SERIAL_COUNTER.next_serial(); @@ -569,13 +602,12 @@ fn handle_pointer_button(state: &mut CompState, pos: Point<f64, Logical>, button if pressed && button == BTN_LEFT { let hit = state.wm.borrow().hit_test(pos.x as i32, pos.y as i32); if let Some((id, hit)) = hit { - state.wm.borrow_mut().focus_window(id); - state.pending.borrow_mut().push(CoreEvent::WindowFocused(id)); + focus_window(state, id); match hit { TitlebarHit::Drag => state.wm.borrow_mut().start_drag(id, pos.x as i32, pos.y as i32), TitlebarHit::Close => { - if let Some(w) = state.id_to_window.get(&id).and_then(|w| w.toplevel()) { - w.send_close(); + if let Some(w) = state.id_to_window.get(&id) { + close_dwindow(w); } } TitlebarHit::Maximize => { @@ -586,7 +618,11 @@ fn handle_pointer_button(state: &mut CompState, pos: Point<f64, Logical>, button TitlebarHit::Resize(edge) => state.wm.borrow_mut().start_resize(id, edge, pos.x as i32, pos.y as i32), } } else if let Some((window, _loc)) = state.space.element_under(pos) { - state.space.raise_element(&window.clone(), true); + let window = window.clone(); + state.space.raise_element(&window, true); + if let Some(&id) = dwindow_wl_surface(&window).and_then(|s| state.surface_to_id.get(&s)) { + focus_window(state, id); + } } } else if !pressed { let mut wm = state.wm.borrow_mut(); diff --git a/crates/wayland/src/xwayland.rs b/crates/wayland/src/xwayland.rs index dc7a88b..ca54f87 100644 --- a/crates/wayland/src/xwayland.rs +++ b/crates/wayland/src/xwayland.rs @@ -40,7 +40,33 @@ pub(crate) type X11Window = smithay::xwayland::xwm::X11Window; /// internal X11-connection source. Both are owned by the event loop after /// `insert_source`, not by any struct here - dropping the loop (or the /// `X11Wm` on disconnect) is what shuts things down. +/// +/// Before spawning, arranges for XWayland to run with `-shm`: this +/// compositor only ever supports `wl_shm` (see `udev.rs`'s module docs on +/// why it's deliberately software-only, no GBM/DMA-BUF), and XWayland's +/// default behavior of trying `glamor` first and falling back to +/// shared-memory buffers on failure does *not* fall back to the +/// `xwayland_shell_v1` protocol for associating X11 windows with +/// `wl_surface`s - confirmed by tracing the actual Wayland protocol +/// exchange with `WAYLAND_DEBUG=1`. Starting with `-shm` from the outset +/// avoids the failed glamor attempt entirely, which keeps XWayland on the +/// code path that does use `xwayland_shell_v1` correctly. +/// +/// `smithay::xwayland::XWayland::spawn` builds its `Xwayland` command line +/// internally with a fixed argument list (no way to add `-shm` directly), +/// and can't be bypassed either: the `XWaylandClientData` type it inserts +/// as the spawned client's data has private fields, so nothing outside +/// smithay can construct one, and `X11Wm`/the internal surface-association +/// commit hook both depend on the client's data specifically being that +/// type. Instead, a tiny wrapper script shadows `Xwayland` on `PATH` +/// (`Command::new("Xwayland")`'s lookup honors the `PATH` smithay copies +/// from this process's own environment) and always re-execs the real +/// binary with `-shm` prepended. pub(crate) fn spawn(handle: &LoopHandle<'static, CompState>, display_handle: &smithay::reexports::wayland_server::DisplayHandle) -> std::io::Result<()> { + if let Err(e) = ensure_shm_wrapper_on_path() { + log::warn!("could not set up an -shm wrapper for XWayland ({e}); XWayland windows will likely fail to render - see xwayland.rs's `spawn` docs"); + } + let (xwayland, client) = XWayland::spawn(display_handle, None, std::iter::empty::<(String, String)>(), true, std::process::Stdio::null(), std::process::Stdio::null(), |_| ())?; let handle_for_ready = handle.clone(); @@ -59,6 +85,60 @@ pub(crate) fn spawn(handle: &LoopHandle<'static, CompState>, display_handle: &sm Ok(()) } +/// Writes a small shell script named `Xwayland` to a private directory and +/// prepends that directory to this process's own `PATH` - the next +/// `Command::new("Xwayland")` (namely `XWayland::spawn`'s, which copies +/// `PATH` from this process's environment into the child's) resolves to +/// the wrapper instead of the real binary. The wrapper always re-execs the +/// real `Xwayland` with `-shm` prepended to whatever arguments it was +/// given, so it's transparent to everything else `spawn` sets up. +fn ensure_shm_wrapper_on_path() -> std::io::Result<()> { + use std::os::unix::fs::PermissionsExt; + + let real_xwayland = find_on_path("Xwayland").ok_or_else(|| std::io::Error::new(std::io::ErrorKind::NotFound, "Xwayland not found on PATH"))?; + + let wrapper_dir = std::env::var_os("XDG_RUNTIME_DIR").map(std::path::PathBuf::from).unwrap_or_else(std::env::temp_dir).join("srdwm-xwayland-shm-wrapper"); + std::fs::create_dir_all(&wrapper_dir)?; + + let wrapper_path = wrapper_dir.join("Xwayland"); + let quoted = shell_single_quote(&real_xwayland.to_string_lossy()); + std::fs::write(&wrapper_path, format!("#!/bin/sh\nexec {quoted} -shm \"$@\"\n"))?; + let mut perms = std::fs::metadata(&wrapper_path)?.permissions(); + perms.set_mode(0o755); + std::fs::set_permissions(&wrapper_path, perms)?; + + let old_path = std::env::var_os("PATH").unwrap_or_default(); + let mut new_path = wrapper_dir.into_os_string(); + new_path.push(":"); + new_path.push(old_path); + // SAFETY: called once, synchronously, before any XWayland process (or + // any other thread) is spawned. + unsafe { std::env::set_var("PATH", new_path) }; + Ok(()) +} + +fn find_on_path(name: &str) -> Option<std::path::PathBuf> { + let path_var = std::env::var_os("PATH")?; + std::env::split_paths(&path_var).map(|dir| dir.join(name)).find(|candidate| candidate.is_file()) +} + +/// POSIX single-quoting: safe for any byte sequence, including embedded +/// single quotes (`'` -> `'\''`). +fn shell_single_quote(s: &str) -> String { + format!("'{}'", s.replace('\'', r"'\''")) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn shell_single_quote_handles_embedded_quotes() { + assert_eq!(shell_single_quote("/usr/bin/Xwayland"), "'/usr/bin/Xwayland'"); + assert_eq!(shell_single_quote("/it's/here"), r"'/it'\''s/here'"); + } +} + fn to_core_resize_edge(edge: X11ResizeEdge) -> ResizeEdge { match edge { X11ResizeEdge::Top => ResizeEdge::Top, @@ -112,7 +192,6 @@ impl CompState { let geom = self.wm.borrow().window(id).map(|w| w.geometry).unwrap_or_default(); let dwindow = DWindow::new_x11_window(surface.clone()); - let _ = surface.set_mapped(true); let _ = surface.configure(Rectangle::new((geom.x, geom.y + TITLEBAR_HEIGHT as i32).into(), (geom.width as i32, (geom.height - TITLEBAR_HEIGHT) as i32).into())); self.space.map_element(dwindow.clone(), (geom.x, geom.y + TITLEBAR_HEIGHT as i32), true); @@ -168,12 +247,27 @@ impl XwmHandler for CompState { let id = wm.alloc_window_id(); let mut w = CoreWindow::new(id, window.title()); w.app_id = window.class(); - let size = window.geometry().size; - w.geometry = srdwm_core::Rect::new(0, 0, size.w.max(1) as u32, size.h.max(1) as u32 + TITLEBAR_HEIGHT); + // Not `window.geometry()`: at `MapRequest` time this can still + // be whatever tiny/default size the X11 window was *created* + // with, before XWayland ever applies a `ConfigureRequest` -- + // and our own `configure_request` handler is deliberately a + // no-op (we own layout for managed windows, matching + // `new_managed_window`'s xdg-shell path below, which doesn't + // trust the client's initial size either). + w.geometry = srdwm_core::Rect::new(0, 0, 800, 600 + TITLEBAR_HEIGHT); wm.add_window(w); id }; self.xwayland_windows.insert(window.window_id(), id); + // Grant the map request *now*, unconditionally: per `X11Surface`'s + // docs this is what tells XWayland the window may proceed, and it + // does so before ever finishing our own wl_surface-dependent setup + // (`finish_x11_window_setup` bails out until `wl_surface()` + // resolves). Deferring `set_mapped` until after that check would + // deadlock - XWayland doesn't seem to advance the window past + // surface creation (no `get_xwayland_surface`/`set_serial`, no + // buffer attach) until the map is granted. + let _ = window.set_mapped(true); self.finish_x11_window_setup(&window); if !self.id_to_window.contains_key(&id) { self.xwayland_pending.push(window); diff --git a/docs/IMPLEMENTATION_STATUS.md b/docs/IMPLEMENTATION_STATUS.md index 576b3dc..e8b53e8 100644 --- a/docs/IMPLEMENTATION_STATUS.md +++ b/docs/IMPLEMENTATION_STATUS.md @@ -145,37 +145,64 @@ What's here is a genuine from-scratch `smithay`-based compositor, not a stub: renders. No client-side visual check yet (the VM has no Wayland-native client installed to test against, only X11 ones - see below). No hotplug (connectors or GPUs) after startup. -- 🔄 XWayland integration (`crates/wayland/src/xwayland.rs`), udev/DRM +- ✅ XWayland integration (`crates/wayland/src/xwayland.rs`), udev/DRM backend only (the winit backend would need its own `calloop::EventLoop` added first - see the module's doc comment): spawns XWayland, starts `X11Wm`, and implements `XwmHandler`/`XWaylandShellHandler` to bridge X11-only clients into the same `WindowManager`/`Space` pipeline xdg-shell windows use (`CreateNotify`/`MapRequest` create a real `srdwm_core::Window`, matched by rules via `class()`; unmap/destroy - clean up the same way). **Verified working up through window creation - and event routing, then found a real architectural blocker**: XWayland - tries `glamor` (GBM-based rendering) first; since this compositor is - deliberately software-only (no GBM/DMA-BUF support - the whole point of - the dumb-buffer approach above), glamor fails, and XWayland's - post-failure fallback path skips the `xwayland_shell_v1` protocol - entirely, so `X11Surface::wl_surface()` never resolves and windows never - render. Confirmed by tracing the actual Wayland protocol exchange - (`WAYLAND_DEBUG=1` on the spawned XWayland process): it binds - `xwayland_shell_v1` at startup, then a second, window-creation-time - registry pass sees the global but never binds it, and - `get_xwayland_surface`/`set_serial` never appear at all. `Xwayland - -help` confirms a `-shm` flag exists that forces shared-memory buffers - from the start (matching this compositor's `wl_shm`/`ImportMem`-only - renderer) instead of trying and falling back from glamor - but - `smithay::xwayland::XWayland::spawn` builds its `Xwayland` command line - internally with a fixed argument list and has no way to add `-shm`. - Fixing this for real means either bypassing `XWayland::spawn` with a - custom implementation (reimplementing its X11 lock-file/socket-pair/ - readiness-detection logic, which is intentionally private to smithay -- - `mod x11_sockets;`, not `pub mod`) or giving the compositor real - GBM/DMA-BUF import support, undoing the earlier deliberate low-spec/ - no-GPU-required design. Left as a documented gap rather than rushing a - low-level reimplementation with no cheap way to iterate on it. + clean up the same way). **Verified live end-to-end**: an `xterm` + launched against the spawned XWayland renders as a correctly-sized, + server-managed window, and typing at it (via a real synthetic + QEMU-level keyboard, not a shortcut) reaches the shell inside it -- + `ls` produced a new prompt line. Getting there surfaced three real bugs, + each root-caused with evidence rather than guessed at: + - XWayland tries `glamor` (GBM-based rendering) first; since this + compositor is deliberately software-only, glamor fails and previously + left XWayland on a rendering path that never used the + `xwayland_shell_v1` protocol at all (confirmed via `WAYLAND_DEBUG=1` + tracing: the global was bound but `get_xwayland_surface`/`set_serial` + were never called). Fixed by shadowing `Xwayland` on `PATH` with a + tiny wrapper script that always re-execs the real binary with `-shm` + - `smithay::xwayland::XWayland::spawn` builds its own fixed argument + list with no way to pass this directly, and its `XWaylandClientData` + has private fields so the spawn call itself can't be bypassed either. + - Even with `-shm`, `set_mapped(true)` was only ever called from inside + `finish_x11_window_setup`, itself gated on `X11Surface::wl_surface()` + already resolving - but XWayland doesn't appear to advance a window + past surface creation (no buffer attach, no further protocol traffic + at all) until the map is granted. A real deadlock, found by tracing + the *same* `WAYLAND_DEBUG=1` output before and after the `-shm` fix + and seeing identical behavior either way. Fixed by calling + `set_mapped(true)` unconditionally in `map_window_request`, before + checking whether `wl_surface()` is available. + - The window then rendered as a ~1px sliver: `map_window_request` seeded + the initial `srdwm_core::Window` geometry from + `X11Surface::geometry()`, which at `MapRequest` time can still be + whatever tiny default the X11 window was *created* with (our own + `configure_request` handler is deliberately a no-op - this compositor + owns layout for managed windows). Fixed by using the same fixed + 800x600 default `new_managed_window`'s xdg-shell path already uses, + instead of trusting the client's initial size. + - Typing didn't reach the window at all until a fourth, broader bug was + found and fixed *outside* the XWayland code: nothing in the whole + Wayland backend ever called `KeyboardHandle::set_focus` - clicking a + window only updated `srdwm_core::WindowManager`'s own focus tracking, + never Wayland/X11 keyboard focus. This affected xdg-shell windows too, + not just XWayland ones. Fixed in `lib.rs`'s `handle_pointer_button` + (both the decoration-click and click-through-to-content-area paths, + the latter of which also never focused a window at all, only raised + it). `TitlebarHit::Close` was also X11-surface-blind (only called + `ToplevelSurface::send_close()`), fixed alongside. + - No font is installed in the test VM at all (a gap in the VM's package + set, not the code), so the titlebar band renders with no title text in + this environment - `decoration.rs`'s font-search fallback is working + exactly as designed; see its own section above for where actual text + rendering was verified. + - Not implemented: selections/clipboard, XSETTINGS, RandR + primary-output sync, override-redirect window geometry beyond initial + placement (all have harmless no-op default `XwmHandler` methods). **Why the visual verification stopped short of a screenshot**: the winit window opens on the *host* compositor, and the only available display in @@ -237,17 +264,16 @@ built them: `xterm`, which is X11-only) - the compositor/socket/render-pipeline side is confirmed, but no real Wayland app has been shown on-screen yet. - **XWayland**: `xterm` launched with `DISPLAY` pointed at the udev - backend's spawned XWayland connected successfully and stayed alive - (`CreateNotify`/`MapRequest` both reached `XwmHandler`, logged and - handled with no crash), but never rendered - this is the - glamor/`-shm` blocker documented above, root-caused via - `WAYLAND_DEBUG=1` protocol tracing on the XWayland process rather than - guessed at. + backend's spawned XWayland renders as a correctly-sized, decorated + (background-only in this VM - no font installed) window, and receives + real keyboard input end-to-end: a synthetic QEMU-level keypress sequence + (`ls` + Enter) executed inside the shell and produced a new prompt line, + screendump-confirmed. Getting there took four rounds of root-causing via + `WAYLAND_DEBUG=1` protocol tracing and fixing real bugs - see the + Wayland backend section above for the full account. ## Not implemented anywhere yet -- XWayland actually rendering a window (blocked on the glamor/`-shm` - issue above; the spawn/protocol/window-tracking plumbing is real). - Animations (`general.animations`/`animation_duration` config keys exist and are read into defaults, but nothing consumes them yet). - A native GUI settings app (the legacy project's `GUI_SETTINGS.md` was |