Support repositioning mapped popups

Requires xdg_wm_base version 3+. Helps make some Steam popups act more
consistently.

src/server/mod.rs

@@ -486,7 +486,12 @@ impl<C: XConnection> ServerState<C> {
         let xdg_wm_base = clientside
             .global_list
             .bind::<XdgWmBase, _, _>(&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<C: XConnection> ServerState<C> {
         handle_globals::<C>(&self.dh, globals.iter());
     }
 
-    fn get_object_from_client_object<T, P: Proxy>(&self, proxy: &P) -> Option<&T>
-    where
-        for<'a> &'a T: TryFrom<&'a Object, Error = String>,
-        Globals: wayland_client::Dispatch<P, ObjectKey>,
-    {
-        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<C: XConnection> ServerState<C> {
         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) {

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> = <T as Proxy>::Request<'a>;
 type Ev<T> = <T as Proxy>::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.

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,

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<Client>,
-    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();
+        self.state.configure_popup(surface_id);
-        let Some(SurfaceRole::Popup(p)) = &mut surface.role else {
+        self.display.flush_clients().unwrap();
-            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();
     }
 
     #[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<WlKeyboard, ()> for State {
 
 impl Dispatch<XdgPopup, SurfaceId> for State {
     fn request(
-        _: &mut Self,
+        state: &mut Self,
         _: &Client,
         _: &XdgPopup,
         request: <XdgPopup as Resource>::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:?}"),
         }
     }