From f379ff5722a821212eb59ada9cf8e51cb3654aad Mon Sep 17 00:00:00 2001 From: Shawn Wallace Date: Fri, 21 Nov 2025 00:55:21 -0500 Subject: [PATCH] server: redraw decorations on window reconfigure Fixes #270 --- src/server/decoration.rs | 18 ++++++-------- src/server/event.rs | 54 +++++++++++++++++++++------------------- src/server/mod.rs | 9 ++++--- src/server/tests.rs | 54 ++++++++++++++++++++++++++++++++++++++++ tests/integration.rs | 1 - testwl/src/lib.rs | 24 +++++++++++++++--- 6 files changed, 117 insertions(+), 43 deletions(-) diff --git a/src/server/decoration.rs b/src/server/decoration.rs index 18b56ce..b98a763 100644 --- a/src/server/decoration.rs +++ b/src/server/decoration.rs @@ -138,21 +138,20 @@ impl DecorationsDataSatellite { self.surface.commit(); } - /// Returns true if decorations were actually drawn #[must_use] - pub fn draw_decorations( - &mut self, - world: &World, - width: i32, - parent_scale_factor: f32, - ) -> bool { - if width <= 0 || !self.should_draw { + pub fn will_draw_decorations(&self, width: i32) -> bool { + width > 0 && self.should_draw + } + + pub fn draw_decorations(&mut self, world: &World, width: i32, parent_scale_factor: f32) { + if !self.will_draw_decorations(width) { + println!("not drawing ({} {})", width, self.should_draw); if self.remove_buffer { self.surface.attach(None, 0, 0); self.surface.commit(); self.remove_buffer = false; } - return false; + return; } self.scale = parent_scale_factor; @@ -212,7 +211,6 @@ impl DecorationsDataSatellite { self.pixmap = bar; self.viewport.set_destination(width, Self::TITLEBAR_HEIGHT); self.update_buffer(world); - true } fn redraw_x_pixmap(&mut self, world: &World) { diff --git a/src/server/event.rs b/src/server/event.rs index ffc3a06..4ed2833 100644 --- a/src/server/event.rs +++ b/src/server/event.rs @@ -113,7 +113,10 @@ impl Event for SurfaceEvents { } } if entity.has::() { - update_surface_viewport(state.world.query_one(target).unwrap()); + update_surface_viewport( + &state.world, + state.world.query_one(target).unwrap(), + ); } } _ => unreachable!(), @@ -228,7 +231,10 @@ impl SurfaceEvents { let output_scale = output_data.get::<&OutputScaleFactor>().unwrap().get(); data.get::<&mut SurfaceScaleFactor>().unwrap().0 = output_scale; drop(query); - update_surface_viewport(state.world.query_one(target).unwrap()); + update_surface_viewport( + &state.world, + state.world.query_one(target).unwrap(), + ); } else { let scale = data.get::<&SurfaceScaleFactor>().unwrap(); if update_output_scale( @@ -310,10 +316,10 @@ impl SurfaceEvents { data.get::<&WlSurface>().unwrap().id(), ); - if let SurfaceRole::Toplevel(Some(toplevel)) = &mut *role { - if let Some(d) = &mut toplevel.decoration.satellite { + if let SurfaceRole::Toplevel(Some(toplevel)) = &*role { + if let Some(d) = &toplevel.decoration.satellite { let surface_width = (width as f64 / scale_factor.0) as i32; - if d.draw_decorations(&state.world, surface_width, scale_factor.0 as f32) { + if d.will_draw_decorations(surface_width) { height = height .saturating_sub( (DecorationsDataSatellite::TITLEBAR_HEIGHT as f64 * scale_factor.0) @@ -341,7 +347,7 @@ impl SurfaceEvents { }; drop(query); - update_surface_viewport(state.world.query_one(target).unwrap()); + update_surface_viewport(&state.world, state.world.query_one(target).unwrap()); } let (surface, attach, callback) = state @@ -457,15 +463,16 @@ impl SurfaceEvents { } pub(super) fn update_surface_viewport( + world: &World, mut surface_query: hecs::QueryOne<( &WindowData, &WpViewport, &SurfaceScaleFactor, - Option<&SurfaceRole>, + Option<&mut SurfaceRole>, &WlSurface, )>, ) { - let (window_data, viewport, scale_factor, role, surface) = surface_query.get().unwrap(); + let (window_data, viewport, scale_factor, mut role, surface) = surface_query.get().unwrap(); let dims = &window_data.attrs.dims; let size_hints = &window_data.attrs.size_hints; @@ -474,25 +481,22 @@ pub(super) fn update_surface_viewport( if width > 0 && height > 0 { viewport.set_destination(width, height); } + + let mut toplevel_data = match &mut role { + Some(SurfaceRole::Toplevel(Some(data))) => Some(data), + _ => None, + }; + if let Some(d) = toplevel_data + .as_mut() + .and_then(|d| d.decoration.satellite.as_deref_mut()) + { + d.draw_decorations(world, width, scale_factor.0 as f32); + } debug!("{} viewport: {width}x{height}", surface.id()); + if let Some(hints) = size_hints { - let data = match &role { - Some(SurfaceRole::Toplevel(Some(data))) => data, - Some(SurfaceRole::Toplevel(None)) => { - warn!( - "Trying to update size hints on {}, but toplevel role data is missing", - surface.id() - ); - return; - } - Some(SurfaceRole::Popup(_)) => { - // Popups don't have min/max size hints. - return; - } - None => { - warn!("No role set on {}.", surface.id()); - return; - } + let Some(data) = toplevel_data else { + return; }; if let Some(min) = hints.min_size { diff --git a/src/server/mod.rs b/src/server/mod.rs index 73e8ddd..4bead43 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -656,7 +656,10 @@ impl ServerState { drop(surface_query); for surface in surfaces { - update_surface_viewport(self.world.query_one(surface).unwrap()); + update_surface_viewport( + &self.world, + self.world.query_one(surface).unwrap(), + ); } } } @@ -952,7 +955,7 @@ impl InnerServerState { return; } - let mut query = data.query::<(&SurfaceRole, &SurfaceScaleFactor)>(); + let mut query = data.query::<(&mut SurfaceRole, &SurfaceScaleFactor)>(); let Some((role, scale_factor)) = query.get() else { return; }; @@ -974,7 +977,7 @@ impl InnerServerState { win.attrs.dims.height = dims.height; drop(query); drop(win); - update_surface_viewport(self.world.query_one(data.entity()).unwrap()); + update_surface_viewport(&self.world, self.world.query_one(data.entity()).unwrap()); } other => warn!("Non popup ({other:?}) being reconfigured, behavior may be off."), } diff --git a/src/server/tests.rs b/src/server/tests.rs index c651656..f40b6b5 100644 --- a/src/server/tests.rs +++ b/src/server/tests.rs @@ -2716,6 +2716,60 @@ fn client_side_decorations_no_global() { assert_eq!(toplevel.unwrap(), subsurface_parent.unwrap()); } +#[test] +fn resize_decorations_on_reconfigure() { + let (mut f, compositor) = TestFixture::new_with_compositor(); + let window = unsafe { Window::new(1) }; + let (_, id) = f.create_toplevel(&compositor, window); + f.testwl + .force_decoration_mode(id, zxdg_toplevel_decoration_v1::Mode::ClientSide); + f.testwl.configure_toplevel(id, 100, 100, vec![]); + f.run(); + + let data = f.connection().window(window); + assert_eq!( + data.dims, + WindowDims { + x: 0, + y: 0, + width: 100, + height: 75 + } + ); + let subsurface_id = f.testwl.last_created_surface_id().unwrap(); + assert_ne!(subsurface_id, id); + let data = f.testwl.get_surface_data(subsurface_id).unwrap(); + let buf_dims = f + .testwl + .get_buffer_dimensions(data.buffer.as_ref().expect("Missing buffer for subsurface")); + assert_eq!(buf_dims, testwl::Vec2 { x: 100, y: 25 }); + assert!( + matches!(data.role, Some(SurfaceRole::Subsurface(_))), + "surface was not a subsurface: {:?}", + data.role + ); + + f.satellite.reconfigure_window(x::ConfigureNotifyEvent::new( + window, + window, + x::WINDOW_NONE, + 0, + 0, + 200, + 200, + 0, + false, + )); + f.run(); + f.run(); + + let data = f.testwl.get_surface_data(subsurface_id).unwrap(); + let buf_dims = f + .testwl + .get_buffer_dimensions(data.buffer.as_ref().expect("Missing buffer for subsurface")); + assert_eq!(buf_dims, testwl::Vec2 { x: 200, y: 25 }); +} + /// See Pointer::handle_event for an explanation. #[test] fn popup_pointer_motion_workaround() {} diff --git a/tests/integration.rs b/tests/integration.rs index 93b3f18..4fe2c43 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -1362,7 +1362,6 @@ fn different_output_position() { f.testwl.move_pointer_to(surface, 150.0, 12.0); f.wait_and_dispatch(); let reply = connection.get_reply(&x::QueryPointer { window }); - println!("reply: {reply:?}"); assert!(reply.same_screen()); assert_eq!(reply.win_x(), 150); assert_eq!(reply.win_y(), 12); diff --git a/testwl/src/lib.rs b/testwl/src/lib.rs index 26e4642..b6c2e5b 100644 --- a/testwl/src/lib.rs +++ b/testwl/src/lib.rs @@ -260,7 +260,7 @@ struct State { surfaces: HashMap, outputs: HashMap, positioners: HashMap, - buffers: HashSet, + buffers: HashMap, begin: Instant, last_surface_id: Option, created_surfaces: Vec, @@ -559,7 +559,6 @@ impl Server { } pub fn get_surface_data(&self, surface_id: SurfaceId) -> Option<&SurfaceData> { - println!("{:?}", self.state.surfaces); self.state.surfaces.get(&surface_id) } @@ -936,6 +935,15 @@ impl Server { .handle() .remove_global::(self.decorations_global.clone()); } + + #[track_caller] + pub fn get_buffer_dimensions(&self, buffer: &WlBuffer) -> Vec2 { + *self + .state + .buffers + .get(buffer) + .expect("buffer does not exist!") + } } #[derive(Clone, Eq, PartialEq, Debug)] @@ -1861,9 +1869,17 @@ impl Dispatch for State { ) { use proto::wl_shm_pool::Request::*; match request { - CreateBuffer { id, .. } => { + CreateBuffer { + id, width, height, .. + } => { let buf = data_init.init(id, ()); - state.buffers.insert(buf); + state.buffers.insert( + buf, + Vec2 { + x: width, + y: height, + }, + ); } Resize { .. } => {} Destroy => {}