Actually properly offset popups on offset outputs

Also update window positions when output positon changes.

src/server/event.rs

@@ -147,33 +147,44 @@ impl SurfaceData {
         event: client::wl_surface::Event,
         state: &mut ServerState<C>,
     ) {
-        let surface = &self.server;
+        use client::wl_surface::Event;
-        simple_event_shunt! {
+
-            surface, event: client::wl_surface::Event => [
+        match event {
-                Enter { |output| {
+            Event::Enter { output } => {
                 let key: ObjectKey = output.data().copied().unwrap();
                 let Some(object) = state.objects.get_mut(key) else {
                     return;
                 };
                 let output: &mut Output = object.as_mut();
-                    if let Some(win) = self.window {
+
-                        if let Some(data) = state.windows.get_mut(&win) {
+                if let Some(win_data) = self
-                            output.add_surface(self, &mut data.attrs.dims, state.connection.as_mut().unwrap());
+                    .window
+                    .as_ref()
+                    .map(|win| state.windows.get_mut(&win).unwrap())
+                {
+                    win_data.update_output_offset(
+                        key,
+                        WindowOutputOffset {
+                            x: output.x,
+                            y: output.y,
+                        },
+                        state.connection.as_mut().unwrap(),
+                    );
+                    output.windows.insert(win_data.window);
                 }
+                self.server.enter(&output.server);
+                debug!("{} entered {}", self.server.id(), output.server.id());
             }
-                    &output.server
+            Event::Leave { output } => {
-                }},
-                Leave { |output| {
                 let key: ObjectKey = output.data().copied().unwrap();
                 let Some(object) = state.objects.get_mut(key) else {
                     return;
                 };
                 let output: &mut Output = object.as_mut();
-                    output.surfaces.remove(&self.client);
+                self.server.leave(&output.server);
-                    &output.server
+            }
-                }},
+            Event::PreferredBufferScale { factor } => self.server.preferred_buffer_scale(factor),
-                PreferredBufferScale { factor }
+            other => warn!("unhandled surface request: {other:?}"),
-            ]
         }
     }
 
@@ -190,14 +201,8 @@ impl SurfaceData {
         if let Some(pending) = xdg.pending.take() {
             let window = state.associated_windows[self.key];
             let window = state.windows.get_mut(&window).unwrap();
-            let x = match pending.x {
+            let x = pending.x + window.output_offset.x;
-                Some(x) => x as i16,
+            let y = pending.y + window.output_offset.y;
-                None => 0,
-            };
-            let y = match pending.y {
-                Some(y) => y as i16,
-                None => 0,
-            };
             let width = if pending.width > 0 {
                 pending.width as u16
             } else {
@@ -212,14 +217,15 @@ impl SurfaceData {
             connection.set_window_dims(
                 window.window,
                 PendingSurfaceState {
+                    x,
+                    y,
                     width: width as _,
                     height: height as _,
-                    ..pending
                 },
             );
             window.attrs.dims = WindowDims {
-                x,
+                x: x as i16,
-                y,
+                y: y as i16,
                 width,
                 height,
             };
@@ -249,8 +255,7 @@ impl SurfaceData {
                 let activated = states.contains(&(u32::from(xdg_toplevel::State::Activated) as u8));
 
                 if activated {
-                    let window = state.associated_windows[self.key];
+                    state.to_focus = Some(self.window.unwrap());
-                    state.to_focus = Some(window);
                 }
 
                 if let Some(SurfaceRole::Toplevel(Some(toplevel))) = &mut self.role {
@@ -292,8 +297,8 @@ impl SurfaceData {
             } => {
                 trace!("popup configure: {x}x{y}, {width}x{height}");
                 self.xdg_mut().unwrap().pending = Some(PendingSurfaceState {
-                    x: Some(x),
+                    x,
-                    y: Some(y),
+                    y,
                     width,
                     height,
                 });
@@ -642,9 +647,9 @@ impl HandleEvent for Touch {
 pub struct Output {
     pub client: client::wl_output::WlOutput,
     pub server: WlOutput,
-    surfaces: HashSet<client::wl_surface::WlSurface>,
+    pub windows: HashSet<x::Window>,
-    x: i32,
+    pub x: i32,
-    y: i32,
+    pub y: i32,
 }
 
 impl Output {
@@ -652,64 +657,36 @@ impl Output {
         Self {
             client,
             server,
-            surfaces: HashSet::new(),
+            windows: HashSet::new(),
             x: 0,
             y: 0,
         }
     }
-
-    fn add_surface<C: XConnection>(
-        &mut self,
-        surface: &SurfaceData,
-        dims: &mut WindowDims,
-        connection: &mut C,
-    ) {
-        self.surfaces.insert(surface.client.clone());
-        let window = surface.window.unwrap();
-        dims.x = self.x as _;
-        dims.y = self.y as _;
-
-        debug!("moving surface to {}x{}", self.x, self.y);
-        connection.set_window_dims(
-            window,
-            PendingSurfaceState {
-                x: Some(self.x),
-                y: Some(self.y),
-                width: dims.width as _,
-                height: dims.height as _,
-            },
-        );
-    }
 }
 impl HandleEvent for Output {
     type Event = client::wl_output::Event;
 
     fn handle_event<C: XConnection>(&mut self, event: Self::Event, state: &mut ServerState<C>) {
         if let client::wl_output::Event::Geometry { x, y, .. } = event {
-            debug!("moving output to {x}x{y}");
+            debug!("moving {} to {x}x{y}", self.server.id());
             self.x = x;
             self.y = y;
-            self.surfaces.retain(|surface| {
+
-                let Some(data) = state.get_object_from_client_object::<SurfaceData, _>(surface)
+            self.windows.retain(|window| {
-                else {
+                let Some(data): Option<&mut WindowData> = state.windows.get_mut(window) else {
                     return false;
                 };
 
-                let window = data.window.as_ref().copied().unwrap();
+                data.update_output_offset(
-                let Some(win_data) = state.windows.get(&window) else {
+                    self.server.data().copied().unwrap(),
-                    return false;
+                    WindowOutputOffset {
-                };
+                        x: self.x,
-
+                        y: self.y,
-                state.connection.as_mut().unwrap().set_window_dims(
-                    window,
-                    PendingSurfaceState {
-                        x: Some(x),
-                        y: Some(y),
-                        width: win_data.attrs.dims.width as _,
-                        height: win_data.attrs.dims.height as _,
                     },
+                    state.connection.as_mut().unwrap(),
                 );
-                true
+
+                return true;
             });
         }
 

src/server/mod.rs

@@ -88,6 +88,12 @@ pub struct WindowAttributes {
     pub group: Option<x::Window>,
 }
 
+#[derive(Debug, Default, PartialEq, Eq)]
+struct WindowOutputOffset {
+    x: i32,
+    y: i32,
+}
+
 #[derive(Debug)]
 struct WindowData {
     window: x::Window,
@@ -95,6 +101,8 @@ struct WindowData {
     surface_key: Option<ObjectKey>,
     mapped: bool,
     attrs: WindowAttributes,
+    output_offset: WindowOutputOffset,
+    output_key: Option<ObjectKey>,
 }
 
 impl WindowData {
@@ -115,8 +123,43 @@ impl WindowData {
                 popup_for: parent,
                 ..Default::default()
             },
+            output_offset: WindowOutputOffset::default(),
+            output_key: None,
         }
     }
+
+    fn update_output_offset<C: XConnection>(
+        &mut self,
+        output_key: ObjectKey,
+        offset: WindowOutputOffset,
+        connection: &mut C,
+    ) {
+        debug!("offset: {offset:?}");
+        if self.output_key != Some(output_key) {
+            self.output_key = Some(output_key);
+        }
+
+        if offset == self.output_offset {
+            return;
+        }
+
+        let dims = &mut self.attrs.dims;
+        dims.x += (offset.x - self.output_offset.x) as i16;
+        dims.y += (offset.y - self.output_offset.y) as i16;
+        self.output_offset = offset;
+
+        connection.set_window_dims(
+            self.window,
+            PendingSurfaceState {
+                x: dims.x as i32,
+                y: dims.y as i32,
+                width: self.attrs.dims.width as _,
+                height: self.attrs.dims.height as _,
+            },
+        );
+
+        debug!("set {:?} offset to {:?}", self.window, self.output_offset);
+    }
 }
 
 struct SurfaceAttach {
@@ -950,8 +993,8 @@ impl<C: XConnection> ServerState<C> {
 
 #[derive(Default, Debug)]
 pub struct PendingSurfaceState {
-    pub x: Option<i32>,
+    pub x: i32,
-    pub y: Option<i32>,
+    pub y: i32,
     pub width: i32,
     pub height: i32,
 }

src/server/tests.rs

@@ -193,8 +193,8 @@ impl super::XConnection for FakeXConnection {
     #[track_caller]
     fn set_window_dims(&mut self, window: Window, state: super::PendingSurfaceState) {
         self.window(window).dims = WindowDims {
-            x: state.x.unwrap_or(0) as _,
+            x: state.x as _,
-            y: state.y.unwrap_or(0) as _,
+            y: state.y as _,
             width: state.width as _,
             height: state.height as _,
         };
@@ -732,7 +732,8 @@ fn popup_flow_simple() {
     let (_, toplevel_id) = f.create_toplevel(&compositor, win_toplevel);
 
     let win_popup = unsafe { Window::new(2) };
-    let (popup_surface, popup_id) = f.create_popup(&compositor, win_popup, win_toplevel, toplevel_id, 10, 10);
+    let (popup_surface, popup_id) =
+        f.create_popup(&compositor, win_popup, win_toplevel, toplevel_id, 10, 10);
 
     f.exwayland.unmap_window(win_popup);
     f.exwayland.destroy_window(win_popup);
@@ -1216,20 +1217,68 @@ fn override_redirect_choose_hover_window() {
 }
 
 #[test]
-fn offset_output() {
+fn output_offset() {
     let (mut f, comp) = TestFixture::new_with_compositor();
     let output = f.new_output(500, 100);
-
     let window = unsafe { Window::new(1) };
-    let (_, surface_id) = f.create_toplevel(&comp, window);
+
-    f.testwl.move_surface_to_output(surface_id, output);
+    {
+        let (surface, surface_id) = f.create_toplevel(&comp, window);
+        f.testwl.move_surface_to_output(surface_id, &output);
         f.run();
         let data = &f.connection().windows[&window];
         assert_eq!(data.dims.x, 500);
         assert_eq!(data.dims.y, 100);
 
+        f.exwayland.unmap_window(window);
+        surface.obj.destroy();
+        f.run();
+    }
+
+    let (t_buffer, t_surface) = comp.create_surface();
+    f.map_window(&comp, window, &t_surface.obj, &t_buffer);
+    f.run();
+    let t_id = f.testwl.last_created_surface_id().unwrap();
+    f.testwl.move_surface_to_output(t_id, &output);
+    f.run();
+    {
+        let data = f.testwl.get_surface_data(t_id).unwrap();
+        assert!(
+            matches!(data.role, Some(testwl::SurfaceRole::Toplevel(_))),
+            "surface role: {:?}",
+            data.role
+        );
+    }
+    f.testwl.configure_toplevel(t_id, 100, 100, vec![xdg_toplevel::State::Activated]);
+    f.run();
+
+    {
+        let data = &f.connection().windows[&window];
+        assert_eq!(data.dims.x, 500);
+        assert_eq!(data.dims.y, 100);
+    }
+
     let popup = unsafe { Window::new(2) };
-    let (_, p_id) = f.create_popup(&comp, popup, window, surface_id, 510, 110);
+    let (p_surface, p_id) = f.create_popup(&comp, popup, window, t_id, 510, 110);
+    f.testwl.move_surface_to_output(p_id, &output);
+    f.run();
+    let data = f.testwl.get_surface_data(p_id).unwrap();
+    assert_eq!(
+        data.popup().positioner_state.offset,
+        testwl::Vec2 { x: 10, y: 10 }
+    );
+
+    f.exwayland.unmap_window(popup);
+    p_surface.obj.destroy();
+    f.run();
+
+    let (buffer, surface) = comp.create_surface();
+    f.map_window(&comp, popup, &surface.obj, &buffer);
+    f.run();
+    let p_id = f.testwl.last_created_surface_id().unwrap();
+    f.testwl.move_surface_to_output(p_id, &output);
+    f.testwl.configure_popup(p_id);
+    f.run();
     let data = f.testwl.get_surface_data(p_id).unwrap();
     assert_eq!(
         data.popup().positioner_state.offset,
@@ -1237,6 +1286,28 @@ fn offset_output() {
     );
 }
 
+#[test]
+fn output_offset_change() {
+    let (mut f, comp) = TestFixture::new_with_compositor();
+    let output = f.new_output(500, 100);
+    let window = unsafe { Window::new(1) };
+    let (_, id) = f.create_toplevel(&comp, window);
+    f.testwl.move_surface_to_output(id, &output);
+    f.run();
+
+    let data = &f.connection().windows[&window];
+    assert_eq!(data.dims.x, 500);
+    assert_eq!(data.dims.y, 100);
+
+    f.testwl.move_output(&output, 600, 200);
+    f.run();
+    f.run();
+    let data = &f.connection().windows[&window];
+    assert_eq!(data.dims.x, 600);
+    assert_eq!(data.dims.y, 200);
+
+}
+
 /// See Pointer::handle_event for an explanation.
 #[test]
 fn popup_pointer_motion_workaround() {}

src/xstate/mod.rs

@@ -422,13 +422,13 @@ impl XState {
         let size_hints = self.get_wm_size_hints(window);
 
         let geometry = self.connection.wait_for_reply(geometry)?;
+        debug!("{window:?} geometry: {geometry:?}");
         let attrs = self.connection.wait_for_reply(attrs)?;
         let mut title = name.resolve()?;
         if title.is_none() {
             title = self.get_wm_name(window).resolve()?;
         }
 
-        debug!("got title: {title:?}");
         let class = class.resolve()?;
         let wm_hints = wm_hints.resolve()?;
         let size_hints = size_hints.resolve()?;
@@ -781,22 +781,15 @@ impl super::XConnection for Arc<xcb::Connection> {
     }
 
     fn set_window_dims(&mut self, window: x::Window, dims: crate::server::PendingSurfaceState) {
-        trace!("reconfiguring window {window:?}");
+        trace!("set window dimensions {window:?} {dims:?}");
-        let mut vals = vec![
-            x::ConfigWindow::Width(dims.width as _),
-            x::ConfigWindow::Height(dims.height as _),
-        ];
-        if let Some(x) = dims.x {
-            vals.push(x::ConfigWindow::X(x));
-        }
-        if let Some(y) = dims.y {
-            vals.push(x::ConfigWindow::Y(y));
-        }
-        vals.sort();
-
         unwrap_or_skip_bad_window!(self.send_and_check_request(&x::ConfigureWindow {
             window,
-            value_list: &vals
+            value_list: &[
+                x::ConfigWindow::X(dims.x),
+                x::ConfigWindow::Y(dims.y),
+                x::ConfigWindow::Width(dims.width as _),
+                x::ConfigWindow::Height(dims.height as _),
+            ]
         }));
     }
 

tests/integration.rs

@@ -889,7 +889,7 @@ fn different_output_position() {
     f.testwl.new_output(0, 0);
     f.wait_and_dispatch();
     let output = f.testwl.last_created_output();
-    f.testwl.move_surface_to_output(surface, output);
+    f.testwl.move_surface_to_output(surface, &output);
     f.testwl.move_pointer_to(surface, 10.0, 10.0);
     f.wait_and_dispatch();
     let reply = connection.get_reply(&x::QueryPointer { window });
@@ -900,7 +900,7 @@ fn different_output_position() {
     f.testwl.new_output(100, 0);
     f.wait_and_dispatch();
     let output = f.testwl.last_created_output();
-    f.testwl.move_surface_to_output(surface, output);
+    f.testwl.move_surface_to_output(surface, &output);
     f.testwl.move_pointer_to(surface, 150.0, 12.0);
     f.wait_and_dispatch();
     let reply = connection.get_reply(&x::QueryPointer { window });

testwl/src/lib.rs

@@ -502,9 +502,15 @@ impl Server {
         self.display.flush_clients().unwrap();
     }
 
-    pub fn move_surface_to_output(&mut self, surface: SurfaceId, output: WlOutput) {
+    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.done();
+        self.display.flush_clients().unwrap();
+    }
+
+    pub fn move_surface_to_output(&mut self, surface: SurfaceId, output: &WlOutput) {
         let data = self.state.surfaces.get(&surface).expect("No such surface");
-        data.surface.enter(&output);
+        data.surface.enter(output);
         self.display.flush_clients().unwrap();
     }
 }