server: redraw decorations on window reconfigure

Fixes #270

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(
+    pub fn will_draw_decorations(&self, width: i32) -> bool {
-        &mut self,
+        width > 0 && self.should_draw
-        world: &World,
+    }
-        width: i32,
+
-        parent_scale_factor: f32,
+    pub fn draw_decorations(&mut self, world: &World, width: i32, parent_scale_factor: f32) {
-    ) -> bool {
+        if !self.will_draw_decorations(width) {
-        if width <= 0 || !self.should_draw {
+            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) {

src/server/event.rs

@@ -113,7 +113,10 @@ impl Event for SurfaceEvents {
                         }
                     }
                     if entity.has::<WindowData>() {
-                        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 SurfaceRole::Toplevel(Some(toplevel)) = &*role {
-                if let Some(d) = &mut toplevel.decoration.satellite {
+                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 {
+        let Some(data) = toplevel_data else {
-            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;
-            }
         };
 
         if let Some(min) = hints.min_size {

src/server/mod.rs

@@ -656,7 +656,10 @@ impl<C: XConnection> ServerState<C> {
 
                     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<S: X11Selection + 'static> InnerServerState<S> {
             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<S: X11Selection + 'static> InnerServerState<S> {
                 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."),
         }

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() {}

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);

testwl/src/lib.rs

@@ -260,7 +260,7 @@ struct State {
     surfaces: HashMap<SurfaceId, SurfaceData>,
     outputs: HashMap<WlOutput, Output>,
     positioners: HashMap<PositionerId, PositionerState>,
-    buffers: HashSet<WlBuffer>,
+    buffers: HashMap<WlBuffer, Vec2>,
     begin: Instant,
     last_surface_id: Option<SurfaceId>,
     created_surfaces: Vec<SurfaceId>,
@@ -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::<State>(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<WlShmPool, ()> 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 => {}