From 98a81c2668e2ccd79bde9927c7191b2d85f0eabc Mon Sep 17 00:00:00 2001 From: Shawn Wallace Date: Thu, 4 Jul 2024 16:02:35 -0400 Subject: [PATCH] Support repositioning mapped popups Requires xdg_wm_base version 3+. Helps make some Steam popups act more consistently. --- src/server/mod.rs | 65 +++++++++++++++++++++++++++++++------- src/server/tests.rs | 77 ++++++++++++++++++++++++++++++++++++++++----- src/xstate/mod.rs | 7 ++++- testwl/src/lib.rs | 54 ++++++++++++++++++++++--------- 4 files changed, 169 insertions(+), 34 deletions(-) diff --git a/src/server/mod.rs b/src/server/mod.rs index 7e55600..0898dd0 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -486,7 +486,12 @@ impl ServerState { let xdg_wm_base = clientside .global_list .bind::(&qh, 2..=6, ()) - .expect("Could not bind XdgWmBase"); + .expect("Could not bind xdg_wm_base"); + + if xdg_wm_base.version() < 3 { + warn!("xdg_wm_base version 2 detected. Popup repositioning will not work, and some popups may not work correctly."); + } + let manager = DataDeviceManagerState::bind(&clientside.global_list, &qh) .inspect_err(|e| { warn!("Could not bind data device manager ({e:?}). Clipboard will not work.") @@ -541,15 +546,6 @@ impl ServerState { handle_globals::(&self.dh, globals.iter()); } - fn get_object_from_client_object(&self, proxy: &P) -> Option<&T> - where - for<'a> &'a T: TryFrom<&'a Object, Error = String>, - Globals: wayland_client::Dispatch, - { - let key: ObjectKey = proxy.data().copied().unwrap(); - Some(self.objects.get(key)?.as_ref()) - } - pub fn new_window( &mut self, window: x::Window, @@ -645,14 +641,61 @@ impl ServerState { win.surface_serial = Some(serial); } + pub fn can_reconfigure_window(&mut self, window: x::Window) -> bool { + let Some(win) = self.windows.get_mut(&window) else { + return true; + }; + + if win.mapped && !win.attrs.override_redirect { + false + } else { + true + } + } + pub fn reconfigure_window(&mut self, event: x::ConfigureNotifyEvent) { let win = self.windows.get_mut(&event.window()).unwrap(); - win.attrs.dims = WindowDims { + let dims = WindowDims { x: event.x(), y: event.y(), width: event.width(), height: event.height(), }; + if dims == win.attrs.dims { + return; + } + debug!("Reconfiguring {win:?} {:?}", dims); + if !win.mapped { + win.attrs.dims = dims; + return; + } + + if self.xdg_wm_base.version() < 3 { + return; + } + + let Some(key) = win.surface_key else { + return; + }; + + let Some(data): Option<&mut SurfaceData> = self.objects.get_mut(key).map(|o| o.as_mut()) + else { + return; + }; + + match &data.role { + Some(SurfaceRole::Popup(Some(popup))) => { + popup.positioner.set_offset( + event.x() as i32 - win.output_offset.x, + event.y() as i32 - win.output_offset.y, + ); + popup + .positioner + .set_size(event.width().into(), event.height().into()); + popup.popup.reposition(&popup.positioner, 0); + } + other => warn!("Non popup ({other:?}) being reconfigured, behavior may be off."), + } } pub fn map_window(&mut self, window: x::Window) { diff --git a/src/server/tests.rs b/src/server/tests.rs index 43dbc9b..534a1fc 100644 --- a/src/server/tests.rs +++ b/src/server/tests.rs @@ -40,7 +40,7 @@ use wayland_protocols::{ }; use wayland_server::{protocol as s_proto, Display, Resource}; use wl_drm::client::wl_drm::WlDrm; -use xcb::x::Window; +use xcb::x::{self, Window}; use xcb::XidNew; @@ -685,11 +685,6 @@ where type Req<'a, T> = ::Request<'a>; type Ev = ::Event; -// TODO: tests to add -// - destroy window before surface -// - reconfigure window (popup) before mapping -// - associate window after surface is already created - // Matches Xwayland flow. #[test] fn toplevel_flow() { @@ -1249,7 +1244,8 @@ fn output_offset() { data.role ); } - f.testwl.configure_toplevel(t_id, 100, 100, vec![xdg_toplevel::State::Activated]); + f.testwl + .configure_toplevel(t_id, 100, 100, vec![xdg_toplevel::State::Activated]); f.run(); { @@ -1305,7 +1301,74 @@ fn output_offset_change() { let data = &f.connection().windows[&window]; assert_eq!(data.dims.x, 600); assert_eq!(data.dims.y, 200); +} +#[test] +fn reposition_popup() { + let (mut f, comp) = TestFixture::new_with_compositor(); + let toplevel = unsafe { Window::new(1) }; + let (_, t_id) = f.create_toplevel(&comp, toplevel); + + let popup = unsafe { Window::new(2) }; + let (_, p_id) = f.create_popup(&comp, popup, toplevel, t_id, 20, 40); + + f.exwayland.reconfigure_window(x::ConfigureNotifyEvent::new( + popup, + popup, + x::WINDOW_NONE, + 40, // x + 60, // y + 80, // width + 100, // height + 0, + true, + )); + f.run(); + f.run(); + let data = f.testwl.get_surface_data(p_id).unwrap(); + assert_eq!( + data.popup().positioner_state.offset, + testwl::Vec2 { x: 40, y: 60 } + ); + assert_eq!( + data.popup().positioner_state.size, + Some(testwl::Vec2 { x: 80, y: 100 }) + ); + let win_data = &f.connection().windows[&popup]; + assert_eq!(win_data.dims, WindowDims { + x: 40, + y: 60, + width: 80, + height: 100 + }); +} + +#[test] +fn ignore_toplevel_reconfigure() { + let (mut f, comp) = TestFixture::new_with_compositor(); + let toplevel = unsafe { Window::new(1) }; + let _ = f.create_toplevel(&comp, toplevel); + + f.exwayland.reconfigure_window(x::ConfigureNotifyEvent::new( + toplevel, + toplevel, + x::WINDOW_NONE, + 40, // x + 60, // y + 80, // width + 100, // height + 0, + true, + )); + + f.run(); + let win_data = &f.connection().windows[&toplevel]; + assert_eq!(win_data.dims, WindowDims { + x: 0, + y: 0, + width: 100, + height: 100 + }); } /// See Pointer::handle_event for an explanation. diff --git a/src/xstate/mod.rs b/src/xstate/mod.rs index aeb25ff..1be1da8 100644 --- a/src/xstate/mod.rs +++ b/src/xstate/mod.rs @@ -332,7 +332,12 @@ impl XState { self.handle_property_change(e, server_state); } xcb::Event::X(x::Event::ConfigureRequest(e)) => { + if !server_state.can_reconfigure_window(e.window()) { + debug!("ignoring reconfigure request for {:?}", e.window()); + continue; + } debug!("{:?} request: {:?}", e.window(), e.value_mask()); + let mut list = Vec::new(); let mask = e.value_mask(); @@ -670,7 +675,7 @@ xcb::atoms_struct! { } } -#[derive(Clone, Copy, Debug, Default)] +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] pub struct WindowDims { pub x: i16, pub y: i16, diff --git a/testwl/src/lib.rs b/testwl/src/lib.rs index fb8ce8e..6a4f522 100644 --- a/testwl/src/lib.rs +++ b/testwl/src/lib.rs @@ -219,6 +219,20 @@ impl State { self.configure_serial += 1; } + #[track_caller] + pub fn configure_popup(&mut self, surface_id: SurfaceId) { + let surface = self.surfaces.get_mut(&surface_id).unwrap(); + let Some(SurfaceRole::Popup(p)) = &mut surface.role else { + panic!("Surface does not have popup role: {:?}", surface.role); + }; + let PositionerState { size, offset, .. } = &p.positioner_state; + let size = size.unwrap(); + p.popup.configure(offset.x, offset.y, size.x, size.y); + p.xdg.configure(self.configure_serial); + self.configure_serial += 1; + } + + #[track_caller] fn get_toplevel(&mut self, surface_id: SurfaceId) -> &mut Toplevel { let surface = self @@ -254,7 +268,6 @@ pub struct Server { dh: DisplayHandle, state: State, client: Option, - configure_serial: u32, } impl Server { @@ -348,7 +361,6 @@ impl Server { dh, state: State::default(), client: None, - configure_serial: 1, } } @@ -415,16 +427,8 @@ impl Server { #[track_caller] pub fn configure_popup(&mut self, surface_id: SurfaceId) { - let surface = self.state.surfaces.get_mut(&surface_id).unwrap(); - let Some(SurfaceRole::Popup(p)) = &mut surface.role else { - panic!("Surface does not have popup role: {:?}", surface.role); - }; - let PositionerState { size, offset, .. } = &p.positioner_state; - let size = size.unwrap(); - p.popup.configure(offset.x, offset.y, size.x, size.y); - p.xdg.configure(self.configure_serial); - self.configure_serial += 1; - self.dispatch(); + self.state.configure_popup(surface_id); + self.display.flush_clients().unwrap(); } #[track_caller] @@ -503,7 +507,16 @@ impl Server { } pub fn move_output(&mut self, output: &WlOutput, x: i32, y: i32) { - output.geometry(x, y, 0, 0, wl_output::Subpixel::None, "".into(), "".into(), wl_output::Transform::Normal); + output.geometry( + x, + y, + 0, + 0, + wl_output::Subpixel::None, + "".into(), + "".into(), + wl_output::Transform::Normal, + ); output.done(); self.display.flush_clients().unwrap(); } @@ -800,16 +813,27 @@ impl Dispatch for State { impl Dispatch for State { fn request( - _: &mut Self, + state: &mut Self, _: &Client, _: &XdgPopup, request: ::Request, - _: &SurfaceId, + surface_id: &SurfaceId, _: &DisplayHandle, _: &mut wayland_server::DataInit<'_, Self>, ) { match request { xdg_popup::Request::Destroy => {} + xdg_popup::Request::Reposition { positioner, token } => { + let data = state.surfaces.get_mut(surface_id).unwrap(); + let Some(SurfaceRole::Popup(p)) = &mut data.role else { + unreachable!(); + }; + let positioner_data = + &state.positioners[&PositionerId(positioner.id().protocol_id())]; + p.positioner_state = positioner_data.clone(); + p.popup.repositioned(token); + state.configure_popup(*surface_id); + } other => todo!("unhandled request {other:?}"), } }