Offset output positions to always have positive coordinates

Honestly, this is something that should probably be fixed in Xwayland itself,
but they don't seem interested in fixing it:
https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/395#note_555613

Fixes #15

src/server/event.rs

@@ -131,8 +131,8 @@ impl SurfaceData {
                     win_data.update_output_offset(
                         key,
                         WindowOutputOffset {
-                            x: output.dimensions.x,
+                            x: output.dimensions.x - state.global_output_offset.x.value,
-                            y: output.dimensions.y,
+                            y: output.dimensions.y - state.global_output_offset.y.value,
                         },
                         state.connection.as_mut().unwrap(),
                     );
@@ -665,17 +665,23 @@ pub struct XdgOutput {
     pub server: ServerXdgOutput,
 }
 
-#[derive(Copy, Clone)]
 enum OutputDimensionsSource {
-    Wl,
+    // The data in this variant is the values needed for the wl_output.geometry event.
+    Wl {
+        physical_width: i32,
+        physical_height: i32,
+        subpixel: WEnum<client::wl_output::Subpixel>,
+        make: String,
+        model: String,
+        transform: WEnum<client::wl_output::Transform>,
+    },
     Xdg,
 }
 
-#[derive(Copy, Clone)]
 pub(super) struct OutputDimensions {
     source: OutputDimensionsSource,
-    x: i32,
+    pub x: i32,
-    y: i32,
+    pub y: i32,
     pub width: i32,
     pub height: i32,
 }
@@ -697,7 +703,14 @@ impl Output {
             xdg: None,
             windows: HashSet::new(),
             dimensions: OutputDimensions {
-                source: OutputDimensionsSource::Wl,
+                source: OutputDimensionsSource::Wl {
+                    physical_height: 0,
+                    physical_width: 0,
+                    subpixel: WEnum::Value(client::wl_output::Subpixel::Unknown),
+                    make: String::new(),
+                    model: String::new(),
+                    transform: WEnum::Value(client::wl_output::Transform::Normal),
+                },
                 x: 0,
                 y: 0,
                 width: 0,
@@ -738,27 +751,40 @@ impl HandleEvent for Output {
 }
 
 impl Output {
-    fn update_offset<C: XConnection>(
+    pub(super) fn global_offset_updated(&mut self, state: &mut ServerState<impl XConnection>) {
-        &mut self,
+        let x = self.dimensions.x - state.global_output_offset.x.value;
-        source: OutputDimensionsSource,
+        let y = self.dimensions.y - state.global_output_offset.y.value;
-        x: i32,
+
-        y: i32,
+        match &self.dimensions.source {
-        state: &mut ServerState<C>,
+            OutputDimensionsSource::Wl {
-    ) {
+                physical_width,
-        if matches!(source, OutputDimensionsSource::Wl)
+                physical_height,
-            && matches!(self.dimensions.source, OutputDimensionsSource::Xdg)
+                subpixel,
-        {
+                make,
-            return;
+                model,
+                transform,
+            } => {
+                self.server.geometry(
+                    x,
+                    y,
+                    *physical_width,
+                    *physical_height,
+                    convert_wenum(*subpixel),
+                    make.clone(),
+                    model.clone(),
+                    convert_wenum(*transform),
+                );
+            }
+            OutputDimensionsSource::Xdg => {
+                self.xdg.as_ref().unwrap().server.logical_position(x, y);
+            }
+        }
+        self.server.done();
+
+        self.update_window_offsets(state);
     }
 
-        self.dimensions.source = source;
+    fn update_window_offsets(&mut self, state: &mut ServerState<impl XConnection>) {
-        self.dimensions.x = x;
-        self.dimensions.y = y;
-        let id = match source {
-            OutputDimensionsSource::Xdg => self.xdg.as_ref().unwrap().server.id(),
-            OutputDimensionsSource::Wl => 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;
@@ -766,24 +792,93 @@ impl Output {
 
             data.update_output_offset(
                 self.server.data().copied().unwrap(),
-                WindowOutputOffset { x, y },
+                WindowOutputOffset {
+                    x: self.dimensions.x - state.global_output_offset.x.value,
+                    y: self.dimensions.y - state.global_output_offset.y.value,
+                },
                 state.connection.as_mut().unwrap(),
             );
 
             true
         });
     }
+
+    fn update_offset<C: XConnection>(
+        &mut self,
+        source: OutputDimensionsSource,
+        x: i32,
+        y: i32,
+        state: &mut ServerState<C>,
+    ) {
+        if matches!(source, OutputDimensionsSource::Wl { .. })
+            && matches!(self.dimensions.source, OutputDimensionsSource::Xdg)
+        {
+            return;
+        }
+
+        let key: ObjectKey = self.server.data().copied().unwrap();
+        let global_offset = &mut state.global_output_offset;
+        let mut maybe_update_dimension = |value, dim: &mut GlobalOutputOffsetDimension| {
+            if value < dim.value {
+                *dim = GlobalOutputOffsetDimension {
+                    owner: Some(key),
+                    value,
+                };
+                state.global_offset_updated = true;
+            } else if dim.owner == Some(key) && value > dim.value {
+                *dim = Default::default();
+                state.global_offset_updated = true;
+            }
+        };
+
+        maybe_update_dimension(x, &mut global_offset.x);
+        maybe_update_dimension(y, &mut global_offset.y);
+
+        self.dimensions.source = source;
+        self.dimensions.x = x;
+        self.dimensions.y = y;
+        let id = match self.dimensions.source {
+            OutputDimensionsSource::Xdg => self.xdg.as_ref().unwrap().server.id(),
+            OutputDimensionsSource::Wl { .. } => self.server.id(),
+        };
+        debug!("moving {id} to {x}x{y}");
+
+        self.update_window_offsets(state);
+    }
+
     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 {
+        if let client::wl_output::Event::Geometry {
-            self.update_offset(OutputDimensionsSource::Wl, x, y, state);
+            x,
+            y,
+            physical_width,
+            physical_height,
+            subpixel,
+            make,
+            model,
+            transform,
+        } = &event
+        {
+            self.update_offset(
+                OutputDimensionsSource::Wl {
+                    physical_width: *physical_width,
+                    physical_height: *physical_height,
+                    subpixel: *subpixel,
+                    make: make.clone(),
+                    model: model.clone(),
+                    transform: *transform,
+                },
+                *x,
+                *y,
+                state,
+            );
         }
 
         if let client::wl_output::Event::Mode { width, height, .. } = event {
-            if matches!(self.dimensions.source, OutputDimensionsSource::Wl) {
+            if matches!(self.dimensions.source, OutputDimensionsSource::Wl { .. }) {
                 self.dimensions.width = width;
                 self.dimensions.height = height;
                 debug!("{} dimensions: {width}x{height} (wl)", self.server.id());
@@ -807,8 +902,12 @@ impl Output {
                 },
                 Scale { factor },
                 Geometry {
-                    x,
+                    |x| {
-                    y,
+                        x - state.global_output_offset.x.value
+                    },
+                    |y| {
+                        y - state.global_output_offset.y.value
+                    },
                     physical_width,
                     physical_height,
                     |subpixel| convert_wenum(subpixel),
@@ -838,7 +937,14 @@ impl Output {
         }
         simple_event_shunt! {
             xdg, event: zxdg_output_v1::Event => [
-                LogicalPosition { x, y },
+                LogicalPosition {
+                    |x| {
+                        x - state.global_output_offset.x.value
+                    },
+                    |y| {
+                        y - state.global_output_offset.y.value
+                    }
+                },
                 LogicalSize { width, height },
                 Done,
                 Name { name },

src/server/mod.rs

@@ -87,7 +87,7 @@ pub struct WindowAttributes {
     pub group: Option<x::Window>,
 }
 
-#[derive(Debug, Default, PartialEq, Eq)]
+#[derive(Debug, Default, PartialEq, Eq, Copy, Clone)]
 struct WindowOutputOffset {
     x: i32,
     y: i32,
@@ -473,6 +473,18 @@ struct FocusData {
     output_name: Option<String>,
 }
 
+#[derive(Copy, Clone, Default)]
+struct GlobalOutputOffsetDimension {
+    owner: Option<ObjectKey>,
+    value: i32,
+}
+
+#[derive(Copy, Clone)]
+struct GlobalOutputOffset {
+    x: GlobalOutputOffsetDimension,
+    y: GlobalOutputOffsetDimension,
+}
+
 pub struct ServerState<C: XConnection> {
     dh: DisplayHandle,
     clientside: ClientState,
@@ -492,6 +504,8 @@ pub struct ServerState<C: XConnection> {
     xdg_wm_base: XdgWmBase,
     clipboard_data: Option<ClipboardData<C::X11Selection>>,
     last_kb_serial: Option<u32>,
+    global_output_offset: GlobalOutputOffset,
+    global_offset_updated: bool,
 }
 
 impl<C: XConnection> ServerState<C> {
@@ -542,6 +556,17 @@ impl<C: XConnection> ServerState<C> {
             xdg_wm_base,
             clipboard_data,
             last_kb_serial: None,
+            global_output_offset: GlobalOutputOffset {
+                x: GlobalOutputOffsetDimension {
+                    owner: None,
+                    value: 0,
+                },
+                y: GlobalOutputOffsetDimension {
+                    owner: None,
+                    value: 0,
+                },
+            },
+            global_offset_updated: false,
         }
     }
 
@@ -844,15 +869,43 @@ impl<C: XConnection> ServerState<C> {
 
         for (key, event) in self.clientside.read_events() {
             let Some(object) = &mut self.objects.get_mut(key) else {
-                warn!("could not handle clientside event: stale surface");
+                warn!("could not handle clientside event: stale object");
                 continue;
             };
             let mut object = object.0.take().unwrap();
             object.handle_event(event, self);
+
             let ret = self.objects[key].0.replace(object); // safe indexed access?
             debug_assert!(ret.is_none());
         }
 
+        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!(
+                "updated global output offset: {}x{}",
+                self.global_output_offset.x.value, self.global_output_offset.y.value
+            );
+            for (key, _) in self.output_keys.clone() {
+                let Some(object) = &mut self.objects.get_mut(key) else {
+                    continue;
+                };
+                let mut output: Output = object
+                    .0
+                    .take()
+                    .expect("Output object missing?")
+                    .try_into()
+                    .expect("Not an output?");
+                output.global_offset_updated(self);
+                self.objects[key].0.replace(output.into());
+            }
+            self.global_offset_updated = false;
+        }
+
         {
             if let Some(FocusData {
                 window,
@@ -918,6 +971,28 @@ impl<C: XConnection> ServerState<C> {
         }
     }
 
+    fn calc_global_output_offset(&mut self) {
+        for (key, _) in &self.output_keys {
+            let Some(object) = &self.objects.get(key) else {
+                continue;
+            };
+
+            let output: &Output = object.as_ref();
+            if output.dimensions.x < self.global_output_offset.x.value {
+                self.global_output_offset.x = GlobalOutputOffsetDimension {
+                    owner: Some(key),
+                    value: output.dimensions.x,
+                }
+            }
+            if output.dimensions.y < self.global_output_offset.y.value {
+                self.global_output_offset.y = GlobalOutputOffsetDimension {
+                    owner: Some(key),
+                    value: output.dimensions.y,
+                }
+            }
+        }
+    }
+
     fn create_role_window(&mut self, window: x::Window, surface_key: ObjectKey) {
         // Temporarily remove surface to placate borrow checker
         let mut surface: SurfaceData = self.objects[surface_key]

src/server/tests.rs

@@ -14,7 +14,7 @@ use wayland_client::{
         wl_compositor::WlCompositor,
         wl_display::WlDisplay,
         wl_keyboard::WlKeyboard,
-        wl_output::WlOutput,
+        wl_output::{self, WlOutput},
         wl_pointer::WlPointer,
         wl_registry::WlRegistry,
         wl_seat::{self, WlSeat},
@@ -51,7 +51,7 @@ use wayland_protocols::{
         shell::server::{xdg_positioner, xdg_toplevel},
         xdg_output::zv1::client::{
             zxdg_output_manager_v1::{self, ZxdgOutputManagerV1},
-            zxdg_output_v1::ZxdgOutputV1,
+            zxdg_output_v1::{self, ZxdgOutputV1},
         },
     },
     xwayland::shell::v1::client::{
@@ -487,13 +487,18 @@ impl TestFixture {
         man
     }
 
-    fn create_xdg_output(&mut self, man: &TestObject<ZxdgOutputManagerV1>, output: WlOutput) {
+    fn create_xdg_output(
-        TestObject::<ZxdgOutputV1>::from_request(
+        &mut self,
+        man: &TestObject<ZxdgOutputManagerV1>,
+        output: WlOutput,
+    ) -> TestObject<ZxdgOutputV1> {
+        let xdg = TestObject::<ZxdgOutputV1>::from_request(
             &man.obj,
             zxdg_output_manager_v1::Request::GetXdgOutput { output },
         );
         self.run();
         self.run();
+        xdg
     }
 
     fn register_window(&mut self, window: Window, data: WindowData) {
@@ -1704,6 +1709,140 @@ 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 positon 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);
+}
+
 /// See Pointer::handle_event for an explanation.
 #[test]
 fn popup_pointer_motion_workaround() {}

tests/integration.rs

@@ -1428,3 +1428,39 @@ fn popup_done() {
 
     assert_eq!(reply.map_state(), x::MapState::Unmapped);
 }
+
+#[test]
+fn negative_output_coordinates() {
+    let mut f = Fixture::new();
+    let output = f.create_output(-500, -500);
+    let mut connection = Connection::new(&f.display);
+
+    let window = connection.new_window(connection.root, 0, 0, 200, 200, false);
+    let surface = f.map_as_toplevel(&mut connection, window);
+    f.testwl.move_surface_to_output(surface, &output);
+    f.testwl.move_pointer_to(surface, 30.0, 40.0);
+    f.wait_and_dispatch();
+
+    let tree = connection.get_reply(&x::QueryTree { window });
+    let geo = connection.get_reply(&x::GetGeometry {
+        drawable: x::Drawable::Window(window),
+    });
+    let reply = connection.get_reply(&x::TranslateCoordinates {
+        src_window: tree.parent(),
+        dst_window: connection.root,
+        src_x: geo.x(),
+        src_y: geo.y(),
+    });
+
+    assert!(reply.same_screen());
+    assert_eq!(reply.dst_x(), 0);
+    assert_eq!(reply.dst_y(), 0);
+
+    let ptr_reply = connection.get_reply(&x::QueryPointer {
+        window: connection.root,
+    });
+    assert!(ptr_reply.same_screen());
+    assert_eq!(ptr_reply.child(), window);
+    assert_eq!(ptr_reply.win_x(), 30);
+    assert_eq!(ptr_reply.win_y(), 40);
+}