From 3b2698f1de1d86c0753e3232530cd8f25058fb90 Mon Sep 17 00:00:00 2001 From: Shawn Wallace Date: Mon, 20 May 2024 18:31:16 -0400 Subject: [PATCH] Switch to using xwayland shell protocol over WL_SURFACE_ID Seems to fix #13. This means a hard requirement on Xwayland 23.1. --- README.md | 1 + src/server/dispatch.rs | 128 ++++++++++++++++++++++++++++++------ src/server/event.rs | 10 ++- src/server/mod.rs | 39 +++++------ src/server/tests.rs | 143 ++++++++++++++++++++++++----------------- src/xstate.rs | 7 +- tests/integration.rs | 9 +-- 7 files changed, 225 insertions(+), 112 deletions(-) diff --git a/README.md b/README.md index fb4ec82..151eeda 100644 --- a/README.md +++ b/README.md @@ -3,6 +3,7 @@ xwayland-satellite grants rootless Xwayland integration to any Wayland composito This is particularly useful for compositors that (understandably) do not want to go through implementing support for rootless Xwayland themselves. ## Dependencies +- Xwayland >=23.1 - xcb - xcb-util-cursor - clang (building only) diff --git a/src/server/dispatch.rs b/src/server/dispatch.rs index bd0d36d..7f17137 100644 --- a/src/server/dispatch.rs +++ b/src/server/dispatch.rs @@ -1,5 +1,5 @@ use super::*; -use log::{debug, trace, warn}; +use log::{debug, error, trace, warn}; use std::sync::{Arc, OnceLock}; use wayland_protocols::{ wp::{ @@ -34,6 +34,10 @@ use wayland_protocols::{ zxdg_output_v1::{self as s_xdgo, ZxdgOutputV1 as XdgOutputServer}, }, }, + xwayland::shell::v1::server::{ + xwayland_shell_v1::{self, XwaylandShellV1}, + xwayland_surface_v1::{self, XwaylandSurfaceV1}, + }, }; use wayland_server::{ protocol::{ @@ -128,9 +132,15 @@ impl Dispatch for ServerState { Request::::Destroy => { let mut object = state.objects.remove(*key).unwrap(); let surface: &mut SurfaceData = object.as_mut(); + if let Some(window_data) = surface.window.and_then(|w| state.windows.get_mut(&w)) { + window_data.surface_key.take(); + } surface.destroy_role(); surface.client.destroy(); - debug!("deleting key: {:?}", key); + debug!( + "deleting key: {key:?} (surface {:?})", + surface.server.id().protocol_id() + ); } Request::::SetBufferScale { scale } => { surface.client.set_buffer_scale(scale); @@ -181,37 +191,25 @@ impl Request::::CreateSurface { id } => { let mut surface_id = None; - let key = state.objects.insert_with_key(|key| { - debug!("new surface with key {key:?}"); + state.objects.insert_with_key(|key| { let client = client.create_surface(&state.qh, key); let server = data_init.init(id, key); surface_id = Some(server.id().protocol_id()); + debug!("new surface with key {key:?} ({surface_id:?})"); SurfaceData { client, server, key, + serial: Default::default(), attach: None, frame_callback: None, role: None, + xwl: None, + window: None, } .into() }); - - let surface_id = surface_id.unwrap(); - - if let Some((win, window_data)) = - state.windows.iter_mut().find_map(|(win, data)| { - Some(*win).zip((data.surface_id == surface_id).then_some(data)) - }) - { - window_data.surface_key = Some(key); - state.associated_windows.insert(key, win); - debug!("associate surface {surface_id} with window {win:?}"); - if window_data.mapped { - state.create_role_window(win, key); - } - } } Request::::CreateRegion { id } => { let c_region = client.create_region(&state.qh, ()); @@ -1075,3 +1073,95 @@ global_dispatch_no_events!(PointerConstraintsServer, PointerConstraintsClient); global_dispatch_with_events!(WlSeat, client::wl_seat::WlSeat); global_dispatch_with_events!(WlOutput, client::wl_output::WlOutput); global_dispatch_with_events!(WlDrmServer, WlDrmClient); + +impl GlobalDispatch for ServerState { + fn bind( + _: &mut Self, + _: &DisplayHandle, + _: &wayland_server::Client, + resource: wayland_server::New, + _: &(), + data_init: &mut wayland_server::DataInit<'_, Self>, + ) { + data_init.init(resource, ()); + } +} + +impl Dispatch for ServerState { + fn request( + state: &mut Self, + client: &wayland_server::Client, + _: &XwaylandShellV1, + request: ::Request, + _: &(), + dhandle: &DisplayHandle, + data_init: &mut wayland_server::DataInit<'_, Self>, + ) { + use xwayland_shell_v1::Request; + match request { + Request::GetXwaylandSurface { id, surface } => { + let key: ObjectKey = surface.data().copied().unwrap(); + let data: &mut SurfaceData = state.objects[key].as_mut(); + if data.xwl.is_some() { + error!("Surface {surface:?} already has the xwayland surface role!"); + client.kill( + dhandle, + wayland_client::backend::protocol::ProtocolError { + code: 0, + object_id: surface.id().protocol_id(), + object_interface: "wl_surface".to_string(), + message: "Surface already has role".to_string(), + }, + ); + return; + } + + let xwl = data_init.init(id, key); + data.xwl = Some(xwl); + } + Request::Destroy => {} + _ => unreachable!(), + } + } +} + +impl Dispatch for ServerState { + fn request( + state: &mut Self, + _: &wayland_server::Client, + _: &XwaylandSurfaceV1, + request: ::Request, + key: &ObjectKey, + _: &DisplayHandle, + _: &mut wayland_server::DataInit<'_, Self>, + ) { + use xwayland_surface_v1::Request; + let data: &mut SurfaceData = state.objects[*key].as_mut(); + match request { + Request::SetSerial { + serial_lo, + serial_hi, + } => { + let serial = [serial_lo, serial_hi]; + data.serial = Some(serial); + if let Some((win, window_data)) = + state.windows.iter_mut().find_map(|(win, data)| { + Some(*win) + .zip((data.surface_serial.is_some_and(|s| s == serial)).then_some(data)) + }) + { + debug!("associate surface {} with {:?}", data.server.id().protocol_id(), win); + window_data.surface_key = Some(*key); + state.associated_windows.insert(*key, win); + if window_data.mapped { + state.create_role_window(win, *key); + } + } + } + Request::Destroy => { + data.xwl.take(); + } + _ => unreachable!(), + } + } +} diff --git a/src/server/event.rs b/src/server/event.rs index 5b35e60..7217539 100644 --- a/src/server/event.rs +++ b/src/server/event.rs @@ -395,6 +395,9 @@ impl HandleEvent for Pointer { } } client::wl_pointer::Event::Leave { serial, surface } => { + if !surface.is_alive() { + return; + } debug!("leaving surface ({serial})"); self.pending_enter.0.take(); self.server @@ -504,7 +507,12 @@ impl HandleEvent for Keyboard { }, Leave { serial, - |surface| state.get_server_surface_from_client(surface) + |surface| { + if !surface.is_alive() { + return; + } + state.get_server_surface_from_client(surface) + } }, Key { serial, diff --git a/src/server/mod.rs b/src/server/mod.rs index 4240439..cd9be8a 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -33,6 +33,9 @@ use wayland_protocols::{ }, xdg_output::zv1::server::zxdg_output_manager_v1::ZxdgOutputManagerV1, }, + xwayland::shell::v1::server::{ + xwayland_shell_v1::XwaylandShellV1, xwayland_surface_v1::XwaylandSurfaceV1, + }, }; use wayland_server::{ protocol::{ @@ -81,7 +84,7 @@ pub struct WindowAttributes { #[derive(Debug)] struct WindowData { window: x::Window, - surface_id: u32, + surface_serial: Option<[u32; 2]>, surface_key: Option, mapped: bool, attrs: WindowAttributes, @@ -96,8 +99,8 @@ impl WindowData { ) -> Self { Self { window, - surface_id: 0, surface_key: None, + surface_serial: None, mapped: false, attrs: WindowAttributes { override_redirect, @@ -119,9 +122,12 @@ pub struct SurfaceData { client: client::wl_surface::WlSurface, server: WlSurface, key: ObjectKey, + serial: Option<[u32; 2]>, frame_callback: Option, attach: Option, role: Option, + xwl: Option, + window: Option, } impl SurfaceData { @@ -410,6 +416,8 @@ impl ServerState { .registry .bind::(data.name, XDG_WM_BASE_VERSION, &qh, ()); + dh.create_global::(1, ()); + let mut ret = Self { windows: HashMap::new(), clientside, @@ -558,27 +566,9 @@ impl ServerState { } } - pub fn associate_window(&mut self, window: x::Window, surface_id: u32) { + pub fn set_window_serial(&mut self, window: x::Window, serial: [u32; 2]) { let win = self.windows.get_mut(&window).unwrap(); - win.surface_id = surface_id; - - if let Some(key) = self - .objects - .iter_mut() - .filter_map(|(key, obj)| { - Some(key).zip(<&mut SurfaceData>::try_from(obj.0.as_mut().unwrap()).ok()) - }) - .find_map(|(key, surface)| { - (surface_id == surface.server.id().protocol_id()).then_some(key) - }) - { - win.surface_key = Some(key); - self.associated_windows.insert(key, window); - debug!("associate {:?} with surface {surface_id}", window); - if win.mapped { - self.create_role_window(window, key); - } - } + win.surface_serial = Some(serial); } pub fn reconfigure_window(&mut self, event: x::ConfigureNotifyEvent) { @@ -689,7 +679,8 @@ impl ServerState { } fn create_role_window(&mut self, window: x::Window, surface_key: ObjectKey) { - let surface: &SurfaceData = self.objects[surface_key].as_ref(); + let surface: &mut SurfaceData = self.objects[surface_key].as_mut(); + surface.window = Some(window); let client = &surface.client; client.attach(None, 0, 0); client.commit(); @@ -714,7 +705,7 @@ impl ServerState { window.window, parent, window.attrs.dims, - surface.client.id() + client.id() ); let parent_window = self.windows.get(&parent).unwrap(); diff --git a/src/server/tests.rs b/src/server/tests.rs index 84a52fc..dd2f164 100644 --- a/src/server/tests.rs +++ b/src/server/tests.rs @@ -31,6 +31,9 @@ use wayland_protocols::{ shell::server::{xdg_positioner, xdg_toplevel}, xdg_output::zv1::client::zxdg_output_manager_v1::ZxdgOutputManagerV1, }, + xwayland::shell::v1::client::{ + xwayland_shell_v1::XwaylandShellV1, xwayland_surface_v1::XwaylandSurfaceV1, + }, }; use wayland_server::{protocol as s_proto, Display, Resource}; use wl_drm::client::wl_drm::WlDrm; @@ -82,6 +85,7 @@ with_optional! { struct Compositor { compositor: TestObject, shm: TestObject, + shell: TestObject } } @@ -202,6 +206,7 @@ struct TestFixture { exwl_connection: Arc, /// Exwayland's display - must dispatch this for our server state to advance exwl_display: Display, + surface_serial: u64, } static INIT: std::sync::Once = std::sync::Once::new(); @@ -240,6 +245,7 @@ impl TestFixture { exwayland, exwl_connection: Connection::from_socket(fake_client).unwrap().into(), exwl_display: display, + surface_serial: 1, }; f.run(); f @@ -291,6 +297,12 @@ impl TestFixture { bind_req(name, WlShm::interface(), version), )); } + x if x == XwaylandShellV1::interface().name => { + ret.shell = Some(TestObject::from_request( + ®istry.obj, + bind_req(name, XwaylandShellV1::interface(), version), + )); + } _ => {} } } @@ -339,44 +351,68 @@ impl TestFixture { .insert(window, data); } - fn create_and_map_window( + fn new_window( + &mut self, + window: Window, + override_redirect: bool, + data: WindowData, + parent: Option, + ) { + let dims = data.dims; + self.register_window(window, data); + self.exwayland + .new_window(window, override_redirect, dims, parent); + } + + fn map_window( &mut self, comp: &Compositor, window: Window, - override_redirect: bool, - ) -> (TestObject, testwl::SurfaceId) { - let (_buffer, surface) = comp.create_surface(); - - let data = WindowData { - mapped: true, - dims: WindowDims { + surface: &WlSurface, + buffer: &TestObject, + ) { + self.exwayland.map_window(window); + self.associate_window(comp, window, surface); + self.run(); + surface + .send_request(Req::::Attach { + buffer: Some(buffer.obj.clone()), x: 0, y: 0, - width: 50, - height: 50, + }) + .unwrap(); + } + + fn associate_window(&mut self, comp: &Compositor, window: Window, surface: &WlSurface) { + let xwl = TestObject::::from_request( + &comp.shell.obj, + Req::::GetXwaylandSurface { + surface: surface.clone(), }, - fullscreen: false, - }; + ); - let dims = data.dims; + let serial = self.surface_serial; + let serial_lo = (serial & 0xFF) as u32; + let serial_hi = (serial >> 8 & 0xFF) as u32; + self.surface_serial += 1; - self.register_window(window, data); + xwl.send_request(Req::::SetSerial { + serial_lo, + serial_hi, + }) + .unwrap(); self.exwayland - .new_window(window, override_redirect, dims, None); - self.exwayland.map_window(window); - self.exwayland - .associate_window(window, surface.id().protocol_id()); + .set_window_serial(window, [serial_lo, serial_hi]); + } - self.run(); - - let testwl_id = self + #[track_caller] + fn check_new_surface(&mut self) -> testwl::SurfaceId { + let id = self .testwl .last_created_surface_id() .expect("Surface not created"); - - assert!(self.testwl.get_surface_data(testwl_id).is_some()); - - (surface, testwl_id) + assert!(self.testwl.get_surface_data(id).is_some()); + id } fn create_toplevel( @@ -384,7 +420,7 @@ impl TestFixture { comp: &Compositor, window: Window, ) -> (TestObject, testwl::SurfaceId) { - let (_buffer, surface) = comp.create_surface(); + let (buffer, surface) = comp.create_surface(); let data = WindowData { mapped: true, @@ -397,27 +433,18 @@ impl TestFixture { fullscreen: false, }; - let dims = data.dims; - - self.register_window(window, data); - self.exwayland.new_window(window, false, dims, None); - self.exwayland.map_window(window); - self.exwayland - .associate_window(window, surface.id().protocol_id()); - + self.new_window(window, false, data, None); + self.map_window(comp, window, &surface.obj, &buffer); self.run(); + let id = self.check_new_surface(); - let testwl_id = self - .testwl - .last_created_surface_id() - .expect("Toplevel surface not created"); { - let surface_data = self.testwl.get_surface_data(testwl_id).unwrap(); + let surface_data = self.testwl.get_surface_data(id).unwrap(); assert!( surface_data.surface == self .testwl - .get_object::(testwl_id) + .get_object::(id) .unwrap() ); assert!(surface_data.buffer.is_none()); @@ -429,11 +456,11 @@ impl TestFixture { } self.testwl - .configure_toplevel(testwl_id, 100, 100, vec![xdg_toplevel::State::Activated]); + .configure_toplevel(id, 100, 100, vec![xdg_toplevel::State::Activated]); self.run(); { - let surface_data = self.testwl.get_surface_data(testwl_id).unwrap(); + let surface_data = self.testwl.get_surface_data(id).unwrap(); assert!(surface_data.buffer.is_some()); } @@ -451,7 +478,7 @@ impl TestFixture { "Incorrect window geometry: {win_data:?}" ); - (surface, testwl_id) + (surface, id) } fn create_popup( @@ -460,7 +487,7 @@ impl TestFixture { window: Window, parent_id: testwl::SurfaceId, ) -> (TestObject, testwl::SurfaceId) { - let (_, popup_surface) = comp.create_surface(); + let (buffer, surface) = comp.create_surface(); let data = WindowData { mapped: true, dims: WindowDims { @@ -472,13 +499,11 @@ impl TestFixture { fullscreen: false, }; let dims = data.dims; - self.register_window(window, data); - self.exwayland.new_window(window, true, dims, None); - self.exwayland.map_window(window); - self.exwayland - .associate_window(window, popup_surface.id().protocol_id()); + self.new_window(window, true, data, None); + self.map_window(&comp, window, &surface.obj, &buffer); self.run(); - let popup_id = self.testwl.last_created_surface_id().unwrap(); + + let popup_id = self.check_new_surface(); assert_ne!(popup_id, parent_id); { @@ -533,7 +558,7 @@ impl TestFixture { assert!(surface_data.buffer.is_some()); } - (popup_surface, popup_id) + (surface, popup_id) } } @@ -597,8 +622,6 @@ type Ev = ::Event; // TODO: tests to add // - destroy window before surface -// - destroy surface before window -// - destroy popup and reassociate with new surface // - reconfigure window (popup) before mapping // - associate window after surface is already created @@ -701,7 +724,8 @@ fn pass_through_globals() { ZxdgOutputManagerV1, WpViewporter, WlDrm, - ZwpPointerConstraintsV1 + ZwpPointerConstraintsV1, + XwaylandShellV1 } let mut globals = SupportedGlobals::default(); @@ -777,8 +801,7 @@ fn popup_window_changes_surface() { assert!(f.testwl.get_surface_data(id).is_some()); f.exwayland.map_window(win); - f.exwayland - .associate_window(win, surface.id().protocol_id()); + f.associate_window(&comp, win, &surface.obj); f.run(); let data = f.testwl.get_surface_data(id).unwrap(); @@ -807,7 +830,10 @@ fn override_redirect_window_after_toplevel_close() { assert!(f.testwl.get_surface_data(first).is_none()); let win2 = unsafe { Window::new(2) }; - let (_, second) = f.create_and_map_window(&comp, win2, true); + let (buffer, surface) = comp.create_surface(); + f.new_window(win2, true, WindowData::default(), None); + f.map_window(&comp, win2, &surface.obj, &buffer); + let second = f.check_new_surface(); let data = f.testwl.get_surface_data(second).unwrap(); assert!( matches!(data.role, Some(testwl::SurfaceRole::Toplevel(_))), @@ -934,8 +960,7 @@ fn window_group_properties() { }, ); f.exwayland.map_window(win); - f.exwayland - .associate_window(win, surface.id().protocol_id()); + f.associate_window(&comp, win, &surface.obj); f.run(); let id = f.testwl.last_created_surface_id().unwrap(); diff --git a/src/xstate.rs b/src/xstate.rs index 4cb7560..e909c32 100644 --- a/src/xstate.rs +++ b/src/xstate.rs @@ -263,11 +263,13 @@ impl XState { } xcb::Event::X(x::Event::ClientMessage(e)) => match e.r#type() { x if x == self.atoms.wl_surface_id => { + panic!("Xserver should be using WL_SURFACE_SERIAL, not WL_SURFACE_ID"); + } + x if x == self.atoms.wl_surface_serial => { let x::ClientMessageData::Data32(data) = e.data() else { unreachable!(); }; - let id: u32 = (data[0] as u64 | ((data[1] as u64) << 32)) as u32; - server_state.associate_window(e.window(), id); + server_state.set_window_serial(e.window(), [data[0], data[1]]); } x if x == self.atoms.net_wm_state => { let x::ClientMessageData::Data32(data) = e.data() else { @@ -544,6 +546,7 @@ xcb::atoms_struct! { #[derive(Clone, Debug)] pub struct Atoms { pub wl_surface_id => b"WL_SURFACE_ID" only_if_exists = false, + pub wl_surface_serial => b"WL_SURFACE_SERIAL" only_if_exists = false, pub wm_protocols => b"WM_PROTOCOLS" only_if_exists = false, pub wm_delete_window => b"WM_DELETE_WINDOW" only_if_exists = false, pub wm_transient_for => b"WM_TRANSIENT_FOR" only_if_exists = false, diff --git a/tests/integration.rs b/tests/integration.rs index b074f1d..23212fa 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -300,11 +300,6 @@ impl Connection { .unwrap(); } - fn unmap_window(&self, window: x::Window) { - self.send_and_check_request(&x::UnmapWindow { window }) - .unwrap(); - } - fn set_property( &self, window: x::Window, @@ -404,6 +399,7 @@ fn toplevel_flow() { ); f.wait_and_dispatch(); let data = f.testwl.get_surface_data(surface).unwrap(); + let toplevel = data.toplevel().toplevel.clone(); assert_eq!(data.toplevel().title, Some("bindow".into())); assert_eq!(data.toplevel().app_id, Some("ssalc".into())); assert_eq!( @@ -421,8 +417,7 @@ fn toplevel_flow() { drop(connection); f.wait_and_dispatch(); - let data = f.testwl.get_surface_data(surface).expect("No surface data"); - assert!(!data.toplevel().toplevel.is_alive()); + assert!(!toplevel.is_alive()); } #[test]