Prioritize xdg_output for updating window positions

Fixes #46

src/server/dispatch.rs

@@ -815,9 +815,9 @@ impl<C: XConnection> Dispatch<XdgOutputServer, ObjectKey> for ServerState<C> {
             unreachable!();
         };
 
-        let output: &XdgOutput = state.objects[*key].as_ref();
-        output.client.destroy();
-        state.objects.remove(*key);
+        let output: &mut Output = state.objects[*key].as_mut();
+        let xdg_output = output.xdg.take().unwrap();
+        xdg_output.client.destroy();
     }
 }
 
@@ -836,14 +836,10 @@ impl<C: XConnection> Dispatch<OutputManServer, ClientGlobalWrapper<OutputManClie
         match request {
             s_output_man::Request::GetXdgOutput { id, output } => {
                 let output_key: ObjectKey = output.data().copied().unwrap();
-                state
-                    .objects
-                    .insert_from_other_objects([output_key], |[output_obj], key| {
-                        let output: &Output = output_obj.try_into().unwrap();
-                        let client = client.get_xdg_output(&output.client, &state.qh, key);
-                        let server = data_init.init(id, key);
-                        XdgOutput { server, client }.into()
-                    });
+                let output: &mut Output = state.objects[output_key].as_mut();
+                let client = client.get_xdg_output(&output.client, &state.qh, output_key);
+                let server = data_init.init(id, output_key);
+                output.xdg = Some(XdgOutput { client, server });
             }
             s_output_man::Request::Destroy => {}
             _ => unreachable!(),

src/server/event.rs

@@ -162,12 +162,13 @@ impl SurfaceData {
                     .as_ref()
                     .map(|win| state.windows.get_mut(&win).unwrap())
                 {
+                    let (x, y) = match output.position {
+                        OutputPosition::Xdg { x, y } => (x, y),
+                        OutputPosition::Wl { x, y } => (x, y),
+                    };
                     win_data.update_output_offset(
                         key,
-                        WindowOutputOffset {
-                            x: output.x,
-                            y: output.y,
-                        },
+                        WindowOutputOffset { x, y },
                         state.connection.as_mut().unwrap(),
                     );
                     output.windows.insert(win_data.window);
@@ -326,22 +327,6 @@ impl HandleEvent for Buffer {
     }
 }
 
-pub type XdgOutput = GenericObject<ServerXdgOutput, ClientXdgOutput>;
-impl HandleEvent for XdgOutput {
-    type Event = zxdg_output_v1::Event;
-    fn handle_event<C: XConnection>(&mut self, event: Self::Event, _: &mut ServerState<C>) {
-        simple_event_shunt! {
-            self.server, event: zxdg_output_v1::Event => [
-                LogicalPosition { x, y },
-                LogicalSize { width, height },
-                Done,
-                Name { name },
-                Description { description }
-            ]
-        }
-    }
-}
-
 pub type Seat = GenericObject<WlSeat, client::wl_seat::WlSeat>;
 impl HandleEvent for Seat {
     type Event = client::wl_seat::Event;
@@ -647,12 +632,23 @@ impl HandleEvent for Touch {
     }
 }
 
+pub struct XdgOutput {
+    pub client: ClientXdgOutput,
+    pub server: ServerXdgOutput,
+}
+
+#[derive(Copy, Clone)]
+pub enum OutputPosition {
+    Wl { x: i32, y: i32 },
+    Xdg { x: i32, y: i32 },
+}
+
 pub struct Output {
     pub client: client::wl_output::WlOutput,
     pub server: WlOutput,
-    pub windows: HashSet<x::Window>,
-    pub x: i32,
-    pub y: i32,
+    pub xdg: Option<XdgOutput>,
+    windows: HashSet<x::Window>,
+    position: OutputPosition,
 }
 
 impl Output {
@@ -660,37 +656,81 @@ impl Output {
         Self {
             client,
             server,
+            xdg: None,
             windows: HashSet::new(),
-            x: 0,
-            y: 0,
+            position: OutputPosition::Wl { x: 0, y: 0 },
         }
     }
 }
+
+#[derive(Debug)]
+pub enum OutputEvent {
+    Wl(client::wl_output::Event),
+    Xdg(zxdg_output_v1::Event),
+}
+
+impl From<client::wl_output::Event> for ObjectEvent {
+    fn from(value: client::wl_output::Event) -> Self {
+        Self::Output(OutputEvent::Wl(value))
+    }
+}
+
+impl From<zxdg_output_v1::Event> for ObjectEvent {
+    fn from(value: zxdg_output_v1::Event) -> Self {
+        Self::Output(OutputEvent::Xdg(value))
+    }
+}
+
 impl HandleEvent for Output {
-    type Event = client::wl_output::Event;
+    type Event = OutputEvent;
 
     fn handle_event<C: XConnection>(&mut self, event: Self::Event, state: &mut ServerState<C>) {
+        match event {
+            OutputEvent::Xdg(event) => self.xdg_event(event, state),
+            OutputEvent::Wl(event) => self.wl_event(event, state),
+        }
+    }
+}
+
+impl Output {
+    fn update_offset<C: XConnection>(
+        &mut self,
+        offset: OutputPosition,
+        state: &mut ServerState<C>,
+    ) {
+        if matches!(offset, OutputPosition::Wl { .. })
+            && matches!(self.position, OutputPosition::Xdg { .. })
+        {
+            return;
+        }
+
+        self.position = offset;
+        let (x, y, id) = match offset {
+            OutputPosition::Xdg { x, y } => (x, y, self.xdg.as_ref().unwrap().server.id()),
+            OutputPosition::Wl { x, y } => (x, y, self.server.id()),
+        };
+        debug!("moving {id} to {x}x{y}");
+        self.windows.retain(|window| {
+            let Some(data): Option<&mut WindowData> = state.windows.get_mut(window) else {
+                return false;
+            };
+
+            data.update_output_offset(
+                self.server.data().copied().unwrap(),
+                WindowOutputOffset { x, y },
+                state.connection.as_mut().unwrap(),
+            );
+
+            return true;
+        });
+    }
+    fn wl_event<C: XConnection>(
+        &mut self,
+        event: client::wl_output::Event,
+        state: &mut ServerState<C>,
+    ) {
         if let client::wl_output::Event::Geometry { x, y, .. } = event {
-            debug!("moving {} to {x}x{y}", self.server.id());
-            self.x = x;
-            self.y = y;
-
-            self.windows.retain(|window| {
-                let Some(data): Option<&mut WindowData> = state.windows.get_mut(window) else {
-                    return false;
-                };
-
-                data.update_output_offset(
-                    self.server.data().copied().unwrap(),
-                    WindowOutputOffset {
-                        x: self.x,
-                        y: self.y,
-                    },
-                    state.connection.as_mut().unwrap(),
-                );
-
-                return true;
-            });
+            self.update_offset(OutputPosition::Wl { x, y }, state);
         }
 
         simple_event_shunt! {
@@ -718,6 +758,26 @@ impl HandleEvent for Output {
             ]
         }
     }
+
+    fn xdg_event<C: XConnection>(
+        &mut self,
+        event: zxdg_output_v1::Event,
+        state: &mut ServerState<C>,
+    ) {
+        if let zxdg_output_v1::Event::LogicalPosition { x, y } = event {
+            self.update_offset(OutputPosition::Xdg { x, y }, state);
+        }
+        let xdg = &self.xdg.as_ref().unwrap().server;
+        simple_event_shunt! {
+            xdg, event: zxdg_output_v1::Event => [
+                LogicalPosition { x, y },
+                LogicalSize { width, height },
+                Done,
+                Name { name },
+                Description { description }
+            ]
+        }
+    }
 }
 
 pub type Drm = GenericObject<WlDrmServer, WlDrmClient>;

src/server/mod.rs

@@ -134,7 +134,7 @@ impl WindowData {
         offset: WindowOutputOffset,
         connection: &mut C,
     ) {
-        debug!("offset: {offset:?}");
+        log::trace!("offset: {offset:?}");
         if self.output_key != Some(output_key) {
             self.output_key = Some(output_key);
         }
@@ -360,7 +360,6 @@ pub(crate) enum Object {
     RelativePointer(RelativePointer),
     DmabufFeedback(DmabufFeedback),
     Drm(Drm),
-    XdgOutput(XdgOutput),
     Touch(Touch),
     ConfinedPointer(ConfinedPointer),
     LockedPointer(LockedPointer)

src/server/tests.rs

@@ -32,7 +32,10 @@ use wayland_protocols::{
     },
     xdg::{
         shell::server::{xdg_positioner, xdg_toplevel},
-        xdg_output::zv1::client::zxdg_output_manager_v1::ZxdgOutputManagerV1,
+        xdg_output::zv1::client::{
+            zxdg_output_manager_v1::{self, ZxdgOutputManagerV1},
+            zxdg_output_v1::ZxdgOutputV1,
+        },
     },
     xwayland::shell::v1::client::{
         xwayland_shell_v1::XwaylandShellV1, xwayland_surface_v1::XwaylandSurfaceV1,
@@ -371,7 +374,14 @@ impl TestFixture {
         }
     }
 
-    fn new_output(&mut self, x: i32, y: i32) -> wayland_server::protocol::wl_output::WlOutput {
+    fn new_output(
+        &mut self,
+        x: i32,
+        y: i32,
+    ) -> (
+        TestObject<WlOutput>,
+        wayland_server::protocol::wl_output::WlOutput,
+    ) {
         self.testwl.new_output(x, y);
         self.run();
         self.run();
@@ -388,7 +398,7 @@ impl TestFixture {
         };
 
         assert_eq!(interface, WlOutput::interface().name);
-        TestObject::<WlOutput>::from_request(
+        let output = TestObject::<WlOutput>::from_request(
             &self.registry.obj,
             Req::<WlRegistry>::Bind {
                 name,
@@ -396,7 +406,49 @@ impl TestFixture {
             },
         );
         self.run();
-        self.testwl.last_created_output()
+        (output, self.testwl.last_created_output())
+    }
+
+    fn enable_xdg_output(&mut self) -> TestObject<ZxdgOutputManagerV1> {
+        self.testwl.enable_xdg_output_manager();
+        self.run();
+        self.run();
+
+        let mut events = std::mem::take(&mut *self.registry.data.events.lock().unwrap());
+        assert_eq!(
+            events.len(),
+            1,
+            "Unexpected number of global events after enabling xdg output"
+        );
+        let event = events.pop().unwrap();
+        let Ev::<WlRegistry>::Global {
+            name,
+            interface,
+            version,
+        } = event
+        else {
+            panic!("Unexpected event: {event:?}");
+        };
+
+        assert_eq!(interface, ZxdgOutputManagerV1::interface().name);
+        let man = TestObject::<ZxdgOutputManagerV1>::from_request(
+            &self.registry.obj,
+            Req::<WlRegistry>::Bind {
+                name,
+                id: (ZxdgOutputManagerV1::interface(), version),
+            },
+        );
+        self.run();
+        man
+    }
+
+    fn create_xdg_output(&mut self, man: &TestObject<ZxdgOutputManagerV1>, output: WlOutput) {
+        TestObject::<ZxdgOutputV1>::from_request(
+            &man.obj,
+            zxdg_output_manager_v1::Request::GetXdgOutput { output },
+        );
+        self.run();
+        self.run();
     }
 
     fn register_window(&mut self, window: Window, data: WindowData) {
@@ -744,6 +796,8 @@ fn pass_through_globals() {
 
     let mut f = TestFixture::new();
     f.testwl.new_output(0, 0);
+    f.testwl.enable_xdg_output_manager();
+    f.run();
     f.run();
 
     const fn check<T: Proxy>() {}
@@ -1214,7 +1268,11 @@ fn override_redirect_choose_hover_window() {
 #[test]
 fn output_offset() {
     let (mut f, comp) = TestFixture::new_with_compositor();
-    let output = f.new_output(500, 100);
+    let (output_obj, output) = f.new_output(0, 0);
+    let man = f.enable_xdg_output();
+    f.create_xdg_output(&man, output_obj.obj);
+    f.testwl.move_xdg_output(&output, 500, 100);
+    f.run();
     let window = unsafe { Window::new(1) };
 
     {
@@ -1285,22 +1343,38 @@ fn output_offset() {
 #[test]
 fn output_offset_change() {
     let (mut f, comp) = TestFixture::new_with_compositor();
-    let output = f.new_output(500, 100);
+
+    let (output_obj, 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);
+    let test_position = |f: &TestFixture, x, y| {
+        let data = &f.connection().windows[&window];
+        assert_eq!(data.dims.x, x);
+        assert_eq!(data.dims.y, y);
+    };
+    test_position(&f, 500, 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);
+    test_position(&f, 600, 200);
+
+    let man = f.enable_xdg_output();
+    f.create_xdg_output(&man, output_obj.obj);
+    // testwl inits xdg output position to 0, and it should take priority over wl_output position
+    test_position(&f, 0, 0);
+
+    f.testwl.move_xdg_output(&output, 1000, 22);
+    f.run();
+    test_position(&f, 1000, 22);
+
+    f.testwl.move_output(&output, 600, 200);
+    f.run();
+    f.run();
+    test_position(&f, 1000, 22);
 }
 
 #[test]