fix: align global output offset with screen edges

Previously, global output offset only moved outputs if they were in
negative coordinate space. Doing so guaranteed an output on the top-most
and left-most border of the screen. This commit offsets outputs to achieve
the same goal on outputs in positive coordinate space, which matches the
output placement of `xrandr`, a placement assumption made by Krita's
tablet tool logic.

The new definition of global output offset can be thought of as drawing
the smallest possible rectangle with contains every monitor, and moving
that bounding box to (0, 0).

ARCHITECTURE.md

@@ -34,13 +34,16 @@ The code for the X11 portion of satellite lives in `src/xstate`. Satellite must
 as any other standard X11 window manager. This includes:
 
 - Setting SubstructureRedirect and SubstructureNotify on the root window, to get notifications for when new windows are being created
-- Follwing (most of) the [ICCCM](https://www.x.org/releases/X11R7.6/doc/xorg-docs/specs/ICCCM/icccm.html) and [EWMH](https://specifications.freedesktop.org/wm-spec/latest/) specs
+- Following (most of) the [ICCCM](https://www.x.org/releases/X11R7.6/doc/xorg-docs/specs/ICCCM/icccm.html) and [EWMH](https://specifications.freedesktop.org/wm-spec/latest/) specs
 
 In addition, satellite must do some other things that a normal X11 window manager wouldn't - but a compositor integrating
 Xwayland would - such as synchronize X11 and Wayland selections. This is explained further in the Wayland server section.
 
 The way that satellite manages windows from the X11 point of view is as follows:
 
+- All monitors maintain their relative positions to one another. Their absolute position is such that
+  the top-most monitor's top edge is on the X-axis and the left-most monitor's left edge is on the Y-axis.
+    - All monitors are on non-negative coordinates with no gaps between the screen and any monitor, matching what `xrandr` does.
 - All toplevels on a monitor are positioned at 0x0 on that monitor. So if you have one monitor at 0x0,
 all the windows are located at 0x0. If you have a monitor at 300x600, all the windows on that monitor are at 300x600.
     - This offset is needed because all monitors rest in the same coordinate plane in X11, so missing this offset would

src/server/event.rs

@@ -212,7 +212,9 @@ impl SurfaceEvents {
 
                 let mut query = data.query::<(&x::Window, &mut WindowData)>();
                 if let Some((window, win_data)) = query.get() {
-                    let dimensions = output_data.get::<&OutputDimensions>().unwrap();
+                    let Some(dimensions) = output_data.get::<&OutputDimensions>() else {
+                        return;
+                    };
                     win_data.update_output_offset(
                         *window,
                         WindowOutputOffset {
@@ -1077,7 +1079,9 @@ fn update_output_offset(
     let connection = &mut state.connection;
     let state = &mut state.inner;
     {
-        let mut dimensions = state.world.get::<&mut OutputDimensions>(output).unwrap();
+        let Ok(mut dimensions) = state.world.get::<&mut OutputDimensions>(output) else {
+            return;
+        };
         if matches!(source, OutputDimensionsSource::Wl { .. })
             && matches!(dimensions.source, OutputDimensionsSource::Xdg)
         {
@@ -1093,7 +1097,8 @@ fn update_output_offset(
                 };
                 state.global_offset_updated = true;
             } else if dim.owner == Some(output) && value > dim.value {
-                *dim = Default::default();
+                // Another output's position could be less than the new value, so recalculate
+                dim.owner = None;
                 state.global_offset_updated = true;
             }
         };
@@ -1124,7 +1129,9 @@ fn update_window_output_offsets(
     world: &World,
     connection: &mut impl XConnection,
 ) {
-    let dimensions = world.get::<&OutputDimensions>(output).unwrap();
+    let Ok(dimensions) = world.get::<&OutputDimensions>(output) else {
+        return;
+    };
     let mut query = world.query::<(&x::Window, &mut WindowData, &OnOutput)>();
 
     for (_, (window, data, _)) in query
@@ -1150,7 +1157,9 @@ pub(super) fn update_global_output_offset(
 ) {
     let entity = world.entity(output).unwrap();
     let mut query = entity.query::<(&OutputDimensions, &WlOutput)>();
-    let (dimensions, server) = query.get().unwrap();
+    let Some((dimensions, server)) = query.get() else {
+        return;
+    };
 
     let x = dimensions.x - global_output_offset.x.value;
     let y = dimensions.y - global_output_offset.y.value;
@@ -1249,24 +1258,28 @@ impl OutputEvent {
                     state,
                 );
                 let global_output_offset = state.global_output_offset;
+                let global_offset_updated = state.global_offset_updated;
 
-                let (output, dimensions, xdg) = state
+                let Ok((output, dimensions, xdg)) = state.world.query_one_mut::<(
-                    .world
+                    &WlOutput,
-                    .query_one_mut::<(&WlOutput, &mut OutputDimensions, Option<&XdgOutputServer>)>(
+                    &mut OutputDimensions,
-                        target,
+                    Option<&XdgOutputServer>,
-                    )
+                )>(target) else {
-                    .unwrap();
+                    return;
+                };
 
-                output.geometry(
+                if !global_offset_updated {
-                    x - global_output_offset.x.value,
+                    output.geometry(
-                    y - global_output_offset.y.value,
+                        x - global_output_offset.x.value,
-                    physical_width,
+                        y - global_output_offset.y.value,
-                    physical_height,
+                        physical_width,
-                    convert_wenum(subpixel),
+                        physical_height,
-                    make,
+                        convert_wenum(subpixel),
-                    model,
+                        make,
-                    convert_wenum(transform),
+                        model,
-                );
+                        convert_wenum(transform),
+                    );
+                }
                 dimensions.rotated_90 = transform.into_result().is_ok_and(|t| {
                     matches!(
                         t,
@@ -1290,10 +1303,12 @@ impl OutputEvent {
                 height,
                 refresh,
             } => {
-                let (output, dimensions) = state
+                let Ok((output, dimensions)) = state
                     .world
                     .query_one_mut::<(&WlOutput, &mut OutputDimensions)>(target)
-                    .unwrap();
+                else {
+                    return;
+                };
 
                 if flags
                     .into_result()
@@ -1348,20 +1363,24 @@ impl OutputEvent {
         match event {
             Event::LogicalPosition { x, y } => {
                 update_output_offset(target, OutputDimensionsSource::Xdg, x, y, state);
-                state
+                if !state.global_offset_updated {
-                    .world
+                    state
-                    .get::<&XdgOutputServer>(target)
+                        .world
-                    .unwrap()
+                        .get::<&XdgOutputServer>(target)
-                    .logical_position(
+                        .unwrap()
-                        x - state.global_output_offset.x.value,
+                        .logical_position(
-                        y - state.global_output_offset.y.value,
+                            x - state.global_output_offset.x.value,
-                    );
+                            y - state.global_output_offset.y.value,
+                        );
+                }
             }
             Event::LogicalSize { .. } => {
-                let (xdg, dimensions) = state
+                let Ok((xdg, dimensions)) = state
                     .world
                     .query_one_mut::<(&XdgOutputServer, &OutputDimensions)>(target)
-                    .unwrap();
+                else {
+                    return;
+                };
                 if dimensions.rotated_90 {
                     xdg.logical_size(dimensions.height, dimensions.width);
                 } else {

src/server/mod.rs

@@ -143,7 +143,7 @@ impl WindowData {
         offset: WindowOutputOffset,
         connection: &mut C,
     ) {
-        log::trace!("offset: {offset:?}");
+        log::trace!(target: "output_offset", "offset: {offset:?}");
         if offset == self.output_offset {
             return;
         }
@@ -162,7 +162,7 @@ impl WindowData {
                 height: self.attrs.dims.height as _,
             },
         ) {
-            debug!("set {:?} offset to {:?}", window, self.output_offset);
+            debug!(target: "output_offset", "set {:?} offset to {:?}", window, self.output_offset);
         }
     }
 }
@@ -644,14 +644,15 @@ impl<C: XConnection> ServerState<C> {
                 .unwrap();
         }
 
+        if self.global_output_offset.x.owner.is_none()
+            || self.global_output_offset.y.owner.is_none()
+        {
+            self.calc_global_output_offset();
+            self.global_offset_updated = true;
+        }
         if self.global_offset_updated {
-            if self.global_output_offset.x.owner.is_none()
-                || self.global_output_offset.y.owner.is_none()
-            {
-                self.calc_global_output_offset();
-            }
-
             debug!(
+                target: "output_offset",
                 "updated global output offset: {}x{}",
                 self.global_output_offset.x.value, self.global_output_offset.y.value
             );
@@ -814,7 +815,9 @@ impl<S: X11Selection + 'static> InnerServerState<S> {
         for (entity, name) in query.iter() {
             if *name == global {
                 self.updated_outputs.push(*entity);
-                self.world.remove_one::<OutputScaleFactor>(*entity).unwrap();
+                self.world
+                    .remove::<(OutputScaleFactor, OutputDimensions)>(*entity)
+                    .unwrap();
                 let query = self
                     .world
                     .query_mut::<&OnOutput>()
@@ -826,6 +829,14 @@ impl<S: X11Selection + 'static> InnerServerState<S> {
                         self.world.remove_one::<OnOutput>(*e).unwrap();
                     }
                 }
+                if self.global_output_offset.x.owner == Some(*entity) {
+                    self.global_offset_updated = true;
+                    self.global_output_offset.x.owner = None;
+                }
+                if self.global_output_offset.y.owner == Some(*entity) {
+                    self.global_offset_updated = true;
+                    self.global_output_offset.y.owner = None;
+                }
                 break;
             }
         }
@@ -1323,6 +1334,8 @@ impl<S: X11Selection + 'static> InnerServerState<S> {
     }
 
     fn calc_global_output_offset(&mut self) {
+        self.global_output_offset.x.value = i32::MAX;
+        self.global_output_offset.y.value = i32::MAX;
         for (entity, dimensions) in self.world.query_mut::<&OutputDimensions>() {
             if dimensions.x < self.global_output_offset.x.value {
                 self.global_output_offset.x = GlobalOutputOffsetDimension {

src/server/tests.rs

@@ -864,13 +864,6 @@ impl TestFixture<FakeXConnection> {
         let data = self.testwl.get_surface_data(surface_id).unwrap();
         match data.role {
             Some(SurfaceRole::Popup(_)) => {
-                assert_eq!(
-                    data.popup().positioner_state.offset,
-                    testwl::Vec2 {
-                        x: dims.x as _,
-                        y: dims.y as _
-                    }
-                );
                 assert_eq!(
                     data.popup().positioner_state.size,
                     Some(testwl::Vec2 {
@@ -1620,86 +1613,246 @@ fn override_redirect_choose_hover_window() {
     assert_eq!(&popup_data.popup().parent, win1_xdg);
 }
 
-#[test]
+#[track_caller]
-fn output_offset() {
+fn check_output_position_event(output: &TestObject<WlOutput>, pos: (i32, i32)) {
-    let (mut f, comp) = TestFixture::new_with_compositor();
+    let mut geo = None;
-    let (output_obj, output) = f.new_output(0, 0);
+    let events = std::mem::take(&mut *output.data.events.lock().unwrap());
-    let man = f.enable_xdg_output();
+    log::debug!("events: {events:?}");
-    f.create_xdg_output(&man, output_obj.obj);
+    for event in events {
-    f.testwl.move_xdg_output(&output, 500, 100);
+        match event {
-    f.run();
+            wl_output::Event::Geometry { x, y, .. } => {
-    let window = Window::new(1);
+                geo = Some((x, y));
-
+            }
-    {
+            wl_output::Event::Done => {
-        let (surface, surface_id) = f.create_toplevel(&comp, window);
+                if let Some(geo) = geo {
-        f.testwl.move_surface_to_output(surface_id, &output);
+                    assert_eq!(geo, pos);
-        f.run();
+                    return;
-        let data = &f.connection().windows[&window];
+                }
-        assert_eq!(data.dims.x, 500);
+            }
-        assert_eq!(data.dims.y, 100);
+            _ => {}
-
+        }
-        f.satellite.unmap_window(window);
-        surface.obj.destroy();
-        f.run();
     }
-
+    if geo.is_none() {
-    let (t_buffer, t_surface) = comp.create_surface();
+        panic!("Did not receive any geometry events");
-    f.map_window(&comp, window, &t_surface.obj, &t_buffer);
+    } else {
-    f.run();
+        panic!("Did not receive a done event");
-    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![]);
+}
-    f.testwl.focus_toplevel(t_id);
-    f.run();
 
-    {
+#[track_caller]
-        let data = &f.connection().windows[&window];
+fn check_output_position_event_xdg(
-        assert_eq!(data.dims.x, 500);
+    xdg_out: &TestObject<ZxdgOutputV1>,
-        assert_eq!(data.dims.y, 100);
+    out: &TestObject<WlOutput>,
+    pos: (i32, i32),
+    goo_updated: bool,
+) {
+    let mut done = false;
+    let events = std::mem::take(&mut *xdg_out.data.events.lock().unwrap())
+        .into_iter()
+        .rev();
+    for event in events {
+        if let zxdg_output_v1::Event::LogicalPosition { x, y } = event {
+            assert_eq!(pos, (x, y));
+            done = true;
+            break;
+        }
     }
-
+    assert!(done, "Did not get zxdg_output_v1 logical_position");
-    let popup = Window::new(2);
+    let events = std::mem::take(&mut *out.data.events.lock().unwrap());
-    let (p_surface, p_id) =
-        f.create_popup(&comp, PopupBuilder::new(popup, window, t_id).x(510).y(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,
+        events
-        testwl::Vec2 { x: 10, y: 10 }
+            .into_iter()
-    );
+            .filter(|e| matches!(*e, wl_output::Event::Done))
-
+            .count(),
-    f.satellite.unmap_window(popup);
+        goo_updated as usize,
-    p_surface.obj.destroy();
+        "Did not get expected wl_output done event"
-    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,
-        testwl::Vec2 { x: 10, y: 10 }
     );
 }
 
 #[test]
-fn output_offset_change() {
+fn output_offset_one_output() {
+    // If there is only one output, that output is always positioned at 0x0
+    let (mut f, _) = TestFixture::new_with_compositor();
+    let (output_obj, output) = f.new_output(0, 0);
+    f.run();
+    f.run();
+    check_output_position_event(&output_obj, (0, 0));
+
+    f.testwl.move_output(&output, 500, 100);
+    f.run();
+    f.run();
+    check_output_position_event(&output_obj, (0, 0));
+
+    f.testwl.move_output(&output, -500, -100);
+    f.run();
+    f.run();
+    check_output_position_event(&output_obj, (0, 0));
+}
+
+#[test]
+fn output_offset_multi_output() {
+    // With multiple outputs, the top-most output is on the X-axis, the left-most output is on the
+    // Y-axis, and they always maintain relative positioning.
+    let (mut f, _) = TestFixture::new_with_compositor();
+
+    let (output_obj_1, output_1) = f.new_output(1000, 0);
+    f.run();
+    check_output_position_event(&output_obj_1, (0, 0));
+
+    let (output_obj_2, _) = f.new_output(0, 1000);
+    f.run();
+    check_output_position_event(&output_obj_1, (1000, 0));
+    check_output_position_event(&output_obj_2, (0, 1000));
+
+    f.testwl.move_output(&output_1, 1000, 2000);
+    f.run();
+    f.run();
+    check_output_position_event(&output_obj_1, (1000, 1000));
+    check_output_position_event(&output_obj_2, (0, 0));
+
+    // Global output offset does not change
+    f.testwl.move_output(&output_1, 1000, 1000);
+    f.run();
+    f.run();
+    check_output_position_event(&output_obj_1, (1000, 0));
+    assert!(&output_obj_2.data.events.lock().unwrap().is_empty());
+}
+
+#[test]
+fn output_offset_multi_output_xdg() {
+    let (mut f, _) = TestFixture::new_with_compositor();
+    let man = f.enable_xdg_output();
+
+    let (output_obj_1, output_1) = f.new_output(0, 0);
+    f.run();
+    std::mem::take(&mut *output_obj_1.data.events.lock().unwrap());
+    let output_xdg_1 = f.create_xdg_output(&man, output_obj_1.obj.clone());
+    f.testwl.move_xdg_output(&output_1, 1000, 0);
+    f.run();
+    f.run();
+    check_output_position_event_xdg(&output_xdg_1, &output_obj_1, (0, 0), true);
+
+    let (output_obj_2, output_2) = f.new_output(1000, 1000);
+    f.run();
+    std::mem::take(&mut *output_obj_2.data.events.lock().unwrap());
+    let output_xdg_2 = f.create_xdg_output(&man, output_obj_2.obj.clone());
+    f.testwl.move_xdg_output(&output_2, 0, 1000);
+    f.run();
+    f.run();
+    check_output_position_event_xdg(&output_xdg_1, &output_obj_1, (1000, 0), true);
+    check_output_position_event_xdg(&output_xdg_2, &output_obj_2, (0, 1000), true);
+
+    f.testwl.move_xdg_output(&output_1, 1000, 2000);
+    f.run();
+    f.run();
+    check_output_position_event_xdg(&output_xdg_1, &output_obj_1, (1000, 1000), true);
+    check_output_position_event_xdg(&output_xdg_2, &output_obj_2, (0, 0), true);
+
+    f.testwl.move_xdg_output(&output_1, 1000, 1000);
+    f.run();
+    f.run();
+    check_output_position_event_xdg(&output_xdg_1, &output_obj_1, (1000, 0), false);
+    assert!(output_xdg_2.data.events.lock().unwrap().is_empty());
+    assert!(output_obj_2.data.events.lock().unwrap().is_empty());
+}
+
+#[test]
+fn output_offset_remove_output() {
+    let (mut f, _) = TestFixture::new_with_compositor();
+
+    let (output_ext_c, output_ext) = f.new_output(0, 0);
+    let (output_main_c, _) = f.new_output(1000, 500);
+    f.run();
+
+    check_output_position_event(&output_ext_c, (0, 0));
+    check_output_position_event(&output_main_c, (1000, 500));
+
+    f.remove_output(output_ext);
+    f.run();
+    f.run();
+    check_output_position_event(&output_main_c, (0, 0));
+}
+
+#[test]
+fn output_offset_surface_positioning() {
     let (mut f, comp) = TestFixture::new_with_compositor();
 
+    f.new_output(0, 0);
+    let (_, output) = f.new_output(500, 100);
+    f.run();
+
+    let window = Window::new(1);
+    let (_, toplevel_id) = f.create_toplevel(&comp, window);
+    f.testwl.move_surface_to_output(toplevel_id, &output);
+    f.run();
+
+    let mut toplevel_pos = WindowDims {
+        x: 500,
+        y: 100,
+        width: 100,
+        height: 100,
+    };
+    f.assert_window_dimensions(window, toplevel_id, toplevel_pos);
+
+    let popup = Window::new(2);
+    let (_, p_id) = f.create_popup(
+        &comp,
+        PopupBuilder::new(popup, window, toplevel_id).x(510).y(110),
+    );
+    let mut popup_dims = WindowDims {
+        x: 510,
+        y: 110,
+        width: 50,
+        height: 50,
+    };
+    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.assert_window_dimensions(popup, p_id, popup_dims);
+
+    f.testwl.move_output(&output, 600, 200);
+    f.run();
+    f.run();
+
+    toplevel_pos.x = 600;
+    toplevel_pos.y = 200;
+    f.assert_window_dimensions(window, toplevel_id, toplevel_pos);
+    let data = f.testwl.get_surface_data(p_id).unwrap();
+    assert_eq!(
+        data.popup().positioner_state.offset,
+        testwl::Vec2 { x: 10, y: 10 }
+    );
+    popup_dims.x = 610;
+    popup_dims.y = 210;
+    f.assert_window_dimensions(popup, p_id, popup_dims);
+
+    f.testwl.move_output(&output, -100, -200);
+    f.run();
+    f.run();
+
+    toplevel_pos.x = 0;
+    toplevel_pos.y = 0;
+    f.assert_window_dimensions(window, toplevel_id, toplevel_pos);
+    let data = f.testwl.get_surface_data(p_id).unwrap();
+    assert_eq!(
+        data.popup().positioner_state.offset,
+        testwl::Vec2 { x: 10, y: 10 }
+    );
+    popup_dims.x = 10;
+    popup_dims.y = 10;
+    f.assert_window_dimensions(popup, p_id, popup_dims);
+}
+
+#[test]
+fn output_offset_xdg_override() {
+    let (mut f, comp) = TestFixture::new_with_compositor();
+    f.new_output(0, 0);
+    f.run();
+
     let (output_obj, output) = f.new_output(500, 100);
     let window = Window::new(1);
     let (_, id) = f.create_toplevel(&comp, window);
@@ -1713,13 +1866,8 @@ fn output_offset_change() {
     };
     test_position(&f, 500, 100);
 
-    f.testwl.move_output(&output, 600, 200);
-    f.run();
-    f.run();
-    test_position(&f, 600, 200);
-
     let man = f.enable_xdg_output();
-    f.create_xdg_output(&man, output_obj.obj);
+    f.create_xdg_output(&man, output_obj.obj.clone());
     // testwl inits xdg output position to 0, and it should take priority over wl_output position
     test_position(&f, 0, 0);
 
@@ -1733,6 +1881,82 @@ fn output_offset_change() {
     test_position(&f, 1000, 22);
 }
 
+#[test]
+fn output_offset_negative_position() {
+    let mut f = TestFixture::new();
+    std::mem::take(&mut *f.registry.data.events.lock().unwrap());
+    let (output, _) = f.new_output(-500, -500);
+    f.run();
+    f.run();
+    check_output_position_event(&output, (0, 0));
+
+    let (output2, _) = f.new_output(0, 0);
+    f.run();
+    f.run();
+    check_output_position_event(&output2, (500, 500));
+    assert!(output.data.events.lock().unwrap().is_empty());
+
+    let (output3, _) = f.new_output(500, 500);
+    f.run();
+    f.run();
+    check_output_position_event(&output3, (1000, 1000));
+    assert!(output.data.events.lock().unwrap().is_empty());
+    assert!(output2.data.events.lock().unwrap().is_empty());
+}
+
+#[test]
+fn output_offset_negative_position_update() {
+    let mut f = TestFixture::new();
+    std::mem::take(&mut *f.registry.data.events.lock().unwrap());
+
+    let (output, _) = f.new_output(-500, -500);
+    f.run();
+    f.run();
+    check_output_position_event(&output, (0, 0));
+
+    let (output2, _) = f.new_output(0, -1000);
+    f.run();
+    f.run();
+    check_output_position_event(&output, (0, 500));
+    check_output_position_event(&output2, (500, 0));
+
+    let (output3, o3) = f.new_output(-1000, 0);
+    f.run();
+    f.run();
+    check_output_position_event(&output, (500, 500));
+    check_output_position_event(&output2, (1000, 0));
+    check_output_position_event(&output3, (0, 1000));
+
+    f.testwl.move_output(&o3, 0, 0);
+    f.run();
+    f.run();
+    check_output_position_event(&output, (0, 500));
+    check_output_position_event(&output2, (500, 0));
+    check_output_position_event(&output3, (500, 1000));
+}
+
+#[test]
+fn output_offset_negative_position_update_xdg() {
+    let mut f = TestFixture::new();
+    std::mem::take(&mut *f.registry.data.events.lock().unwrap());
+    let xdg = f.enable_xdg_output();
+
+    let (output, _) = f.new_output(-500, -500);
+    f.run();
+    f.run();
+    check_output_position_event(&output, (0, 0));
+
+    let (output2, output_s) = f.new_output(0, 0);
+    f.run();
+    std::mem::take(&mut *output2.data.events.lock().unwrap());
+    let xdg_output = f.create_xdg_output(&xdg, output2.obj.clone());
+    f.testwl.move_xdg_output(&output_s, 0, -1000);
+    f.run();
+    f.run();
+    check_output_position_event(&output, (0, 500));
+    check_output_position_event_xdg(&xdg_output, &output2, (500, 0), true);
+}
+
 #[test]
 fn reconfigure_popup() {
     let (mut f, comp) = TestFixture::new_with_compositor();
@@ -2037,140 +2261,6 @@ fn fullscreen_heuristic() {
     check_fullscreen(3, true);
 }
 
-#[track_caller]
-fn check_output_position_event(output: &TestObject<WlOutput>, x: i32, y: i32) {
-    let events = std::mem::take(&mut *output.data.events.lock().unwrap());
-    assert!(!events.is_empty());
-    let mut done = false;
-    let mut geo = false;
-    for event in events {
-        match event {
-            wl_output::Event::Geometry {
-                x: geo_x, y: geo_y, ..
-            } => {
-                assert_eq!(geo_x, x);
-                assert_eq!(geo_y, y);
-                geo = true;
-            }
-            wl_output::Event::Done => {
-                done = true;
-            }
-            _ => {}
-        }
-    }
-
-    assert!(geo, "Didn't get geometry event");
-    assert!(done, "Didn't get done event");
-}
-
-#[test]
-fn negative_output_position() {
-    let mut f = TestFixture::new();
-    std::mem::take(&mut *f.registry.data.events.lock().unwrap());
-    let (output, _) = f.new_output(-500, -500);
-    f.run();
-    f.run();
-    check_output_position_event(&output, 0, 0);
-
-    let (output2, _) = f.new_output(0, 0);
-    f.run();
-    f.run();
-    check_output_position_event(&output2, 500, 500);
-    assert!(output.data.events.lock().unwrap().is_empty());
-
-    let (output3, _) = f.new_output(500, 500);
-    f.run();
-    f.run();
-    check_output_position_event(&output3, 1000, 1000);
-    assert!(output.data.events.lock().unwrap().is_empty());
-    assert!(output2.data.events.lock().unwrap().is_empty());
-}
-
-#[test]
-fn negative_output_position_update_offset() {
-    let mut f = TestFixture::new();
-    std::mem::take(&mut *f.registry.data.events.lock().unwrap());
-
-    let (output, _) = f.new_output(-500, -500);
-    f.run();
-    f.run();
-    check_output_position_event(&output, 0, 0);
-
-    let (output2, _) = f.new_output(0, -1000);
-    f.run();
-    f.run();
-    check_output_position_event(&output, 0, 500);
-    check_output_position_event(&output2, 500, 0);
-
-    let (output3, _) = f.new_output(-1000, 0);
-    f.run();
-    f.run();
-    check_output_position_event(&output, 500, 500);
-    check_output_position_event(&output2, 1000, 0);
-    check_output_position_event(&output3, 0, 1000);
-}
-
-#[test]
-fn negative_output_xdg_position_update_offset() {
-    let mut f = TestFixture::new();
-    std::mem::take(&mut *f.registry.data.events.lock().unwrap());
-    let xdg = f.enable_xdg_output();
-
-    let (output, _) = f.new_output(-500, -500);
-    f.run();
-    f.run();
-    check_output_position_event(&output, 0, 0);
-
-    let (output2, output_s) = f.new_output(0, 0);
-    let xdg_output = f.create_xdg_output(&xdg, output2.obj);
-    f.testwl.move_xdg_output(&output_s, 0, -1000);
-    f.run();
-    f.run();
-    check_output_position_event(&output, 0, 500);
-
-    let mut found = false;
-    let mut first = false;
-    for event in std::mem::take(&mut *xdg_output.data.events.lock().unwrap()) {
-        if let zxdg_output_v1::Event::LogicalPosition { x, y } = event {
-            // Testwl sends a logical position event when the output is first created
-            // We are interested in the second one generated by satellite
-            if !first {
-                first = true;
-                continue;
-            }
-            assert_eq!(x, 500);
-            assert_eq!(y, 0);
-            found = true;
-            break;
-        }
-    }
-    assert!(found, "Did not get xdg output logical position");
-    found = false;
-    for event in std::mem::take(&mut *output2.data.events.lock().unwrap()) {
-        if let wl_output::Event::Done = event {
-            found = true;
-            break;
-        }
-    }
-    assert!(found, "Did not get done event");
-}
-
-#[test]
-fn negative_output_position_remove_offset() {
-    let mut f = TestFixture::new();
-    std::mem::take(&mut *f.registry.data.events.lock().unwrap());
-
-    let (c_output, s_output) = f.new_output(-500, -500);
-    f.run();
-    f.run();
-    check_output_position_event(&c_output, 0, 0);
-
-    f.testwl.move_output(&s_output, 500, 500);
-    f.run();
-    f.run();
-    check_output_position_event(&c_output, 500, 500);
-}
-
 #[test]
 fn scaled_output_popup() {
     let (mut f, comp) = TestFixture::new_with_compositor();