From beb7c3ebe09fe50f66765bddbdfb02b9b03f1738 Mon Sep 17 00:00:00 2001 From: Shawn Wallace Date: Sun, 16 Mar 2025 15:55:56 -0400 Subject: [PATCH] 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 | 176 ++++++++++++++++++++++++++++++++++--------- src/server/mod.rs | 79 ++++++++++++++++++- src/server/tests.rs | 147 +++++++++++++++++++++++++++++++++++- tests/integration.rs | 36 +++++++++ 4 files changed, 397 insertions(+), 41 deletions(-) diff --git a/src/server/event.rs b/src/server/event.rs index f107b35..0ebfeb3 100644 --- a/src/server/event.rs +++ b/src/server/event.rs @@ -131,8 +131,8 @@ impl SurfaceData { win_data.update_output_offset( key, WindowOutputOffset { - x: output.dimensions.x, - y: output.dimensions.y, + x: output.dimensions.x - state.global_output_offset.x.value, + 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, + make: String, + model: String, + transform: WEnum, + }, Xdg, } -#[derive(Copy, Clone)] pub(super) struct OutputDimensions { source: OutputDimensionsSource, - x: i32, - y: i32, + pub x: 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( - &mut self, - source: OutputDimensionsSource, - x: i32, - y: i32, - state: &mut ServerState, - ) { - if matches!(source, OutputDimensionsSource::Wl) - && matches!(self.dimensions.source, OutputDimensionsSource::Xdg) - { - return; - } + pub(super) fn global_offset_updated(&mut self, state: &mut ServerState) { + let x = self.dimensions.x - state.global_output_offset.x.value; + let y = self.dimensions.y - state.global_output_offset.y.value; - self.dimensions.source = source; - 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}"); + match &self.dimensions.source { + OutputDimensionsSource::Wl { + physical_width, + physical_height, + subpixel, + make, + 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); + } + + fn update_window_offsets(&mut self, state: &mut ServerState) { 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( + &mut self, + source: OutputDimensionsSource, + x: i32, + y: i32, + state: &mut ServerState, + ) { + 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( &mut self, event: client::wl_output::Event, state: &mut ServerState, ) { - if let client::wl_output::Event::Geometry { x, y, .. } = event { - self.update_offset(OutputDimensionsSource::Wl, x, y, state); + if let client::wl_output::Event::Geometry { + 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, - y, + |x| { + 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 }, diff --git a/src/server/mod.rs b/src/server/mod.rs index f24d11e..b360ee4 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -87,7 +87,7 @@ pub struct WindowAttributes { pub group: Option, } -#[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, } +#[derive(Copy, Clone, Default)] +struct GlobalOutputOffsetDimension { + owner: Option, + value: i32, +} + +#[derive(Copy, Clone)] +struct GlobalOutputOffset { + x: GlobalOutputOffsetDimension, + y: GlobalOutputOffsetDimension, +} + pub struct ServerState { dh: DisplayHandle, clientside: ClientState, @@ -492,6 +504,8 @@ pub struct ServerState { xdg_wm_base: XdgWmBase, clipboard_data: Option>, last_kb_serial: Option, + global_output_offset: GlobalOutputOffset, + global_offset_updated: bool, } impl ServerState { @@ -542,6 +556,17 @@ impl ServerState { 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 ServerState { 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 ServerState { } } + 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] diff --git a/src/server/tests.rs b/src/server/tests.rs index 47f51ec..68f5289 100644 --- a/src/server/tests.rs +++ b/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, output: WlOutput) { - TestObject::::from_request( + fn create_xdg_output( + &mut self, + man: &TestObject, + output: WlOutput, + ) -> TestObject { + let xdg = TestObject::::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, 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() {} diff --git a/tests/integration.rs b/tests/integration.rs index f54af29..c1df0cd 100644 --- a/tests/integration.rs +++ b/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); +}