2 files changed, 138 insertions, 8 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);