Update Smithay (last_acked refactor, Qt layer-shell popup grab fix, popup ordering fix)

Cargo.lock

@@ -3602,7 +3602,7 @@ checksum = "b7c388c1b5e93756d0c740965c41e8822f866621d41acbdf6336a6a168f8840c"
 [[package]]
 name = "smithay"
 version = "0.7.0"
-source = "git+https://github.com/Smithay/smithay.git#20d2dacd71394b5f96f6ace0a70a6f20dc62c0c6"
+source = "git+https://github.com/Smithay/smithay.git#47c2d2de18697a63ddf9e828212510e0349c9d27"
 dependencies = [
  "aliasable",
  "appendlist",
@@ -3676,7 +3676,7 @@ dependencies = [
 [[package]]
 name = "smithay-drm-extras"
 version = "0.1.0"
-source = "git+https://github.com/Smithay/smithay.git#20d2dacd71394b5f96f6ace0a70a6f20dc62c0c6"
+source = "git+https://github.com/Smithay/smithay.git#47c2d2de18697a63ddf9e828212510e0349c9d27"
 dependencies = [
  "drm",
  "libdisplay-info",

src/handlers/compositor.rs

@@ -14,7 +14,7 @@ use smithay::wayland::compositor::{
     SurfaceAttributes,
 };
 use smithay::wayland::dmabuf::get_dmabuf;
-use smithay::wayland::shell::xdg::XdgToplevelSurfaceData;
+use smithay::wayland::shell::xdg::ToplevelCachedState;
 use smithay::wayland::shm::{ShmHandler, ShmState};
 use smithay::{delegate_compositor, delegate_shm};
 
@@ -286,13 +286,14 @@ impl CompositorHandler for State {
                         .buffer_delta
                         .take();
 
-                    let role = states
+                    let serial = states
-                        .data_map
+                        .cached_state
-                        .get::<XdgToplevelSurfaceData>()
+                        .get::<ToplevelCachedState>()
-                        .unwrap()
+                        .current()
-                        .lock()
+                        .last_acked
-                        .unwrap();
+                        .as_ref()
-                    (role.configure_serial, buffer_delta)
+                        .map(|c| c.serial);
+                    (serial, buffer_delta)
                 });
                 if serial.is_none() {
                     error!("commit on a mapped surface without a configured serial");

src/handlers/layer_shell.rs

@@ -174,24 +174,8 @@ impl State {
                         self.niri.layer_shell_on_demand_focus = Some(layer.clone());
                     }
                 } else {
-                    let was_mapped = self.niri.mapped_layer_surfaces.remove(layer).is_some();
+                    self.niri.mapped_layer_surfaces.remove(layer);
                     self.niri.unmapped_layer_surfaces.insert(surface.clone());
-
-                    // After layer surface unmaps it has to perform the initial commit-configure
-                    // sequence again. This is a workaround until Smithay properly resets
-                    // initial_configure_sent upon the surface unmapping itself as it does for
-                    // toplevels.
-                    if was_mapped {
-                        with_states(surface, |states| {
-                            let mut data = states
-                                .data_map
-                                .get::<LayerSurfaceData>()
-                                .unwrap()
-                                .lock()
-                                .unwrap();
-                            data.initial_configure_sent = false;
-                        });
-                    }
                 }
             } else {
                 let scale = output.current_scale();

src/handlers/mod.rs

@@ -19,7 +19,6 @@ use smithay::input::pointer::{
 use smithay::input::{keyboard, Seat, SeatHandler, SeatState};
 use smithay::output::Output;
 use smithay::reexports::rustix::fs::{fcntl_setfl, OFlags};
-use smithay::reexports::wayland_protocols::xdg::shell::server::xdg_toplevel;
 use smithay::reexports::wayland_protocols_wlr::screencopy::v1::server::zwlr_screencopy_manager_v1::ZwlrScreencopyManagerV1;
 use smithay::reexports::wayland_server::protocol::wl_data_source::WlDataSource;
 use smithay::reexports::wayland_server::protocol::wl_output::WlOutput;
@@ -92,7 +91,7 @@ use crate::protocols::virtual_pointer::{
     VirtualPointerInputBackend, VirtualPointerManagerState, VirtualPointerMotionAbsoluteEvent,
     VirtualPointerMotionEvent,
 };
-use crate::utils::{output_size, send_scale_transform, with_toplevel_role};
+use crate::utils::{output_size, send_scale_transform};
 use crate::{
     delegate_ext_workspace, delegate_foreign_toplevel, delegate_gamma_control,
     delegate_mutter_x11_interop, delegate_output_management, delegate_screencopy,
@@ -543,15 +542,6 @@ impl ForeignToplevelHandler for State {
     fn set_fullscreen(&mut self, wl_surface: WlSurface, wl_output: Option<WlOutput>) {
         if let Some((mapped, current_output)) = self.niri.layout.find_window_and_output(&wl_surface)
         {
-            let has_fullscreen_cap = with_toplevel_role(mapped.toplevel(), |role| {
-                role.current
-                    .capabilities
-                    .contains(xdg_toplevel::WmCapabilities::Fullscreen)
-            });
-            if !has_fullscreen_cap {
-                return;
-            }
-
             let window = mapped.window.clone();
 
             if let Some(requested_output) = wl_output.as_ref().and_then(Output::from_resource) {

src/handlers/xdg_shell.rs

@@ -1259,8 +1259,9 @@ pub fn add_mapped_toplevel_pre_commit_hook(toplevel: &ToplevelSurface) -> HookId
                 .unwrap()
                 .lock()
                 .unwrap();
+            let serial = role.last_acked.as_ref().map(|c| c.serial);
 
-            (got_unmapped, dmabuf, role.configure_serial)
+            (got_unmapped, dmabuf, serial)
         });
 
         let mut transaction_for_dmabuf = None;
@@ -1305,7 +1306,7 @@ pub fn add_mapped_toplevel_pre_commit_hook(toplevel: &ToplevelSurface) -> HookId
             }
 
             animate = mapped.should_animate_commit(serial);
-        } else {
+        } else if !got_unmapped {
             error!("commit on a mapped surface without a configured serial");
         };
 

src/protocols/foreign_toplevel.rs

@@ -11,7 +11,9 @@ use smithay::reexports::wayland_server::protocol::wl_surface::WlSurface;
 use smithay::reexports::wayland_server::{
     Client, DataInit, Dispatch, DisplayHandle, GlobalDispatch, New, Resource,
 };
-use smithay::wayland::shell::xdg::{ToplevelStateSet, XdgToplevelSurfaceRoleAttributes};
+use smithay::wayland::shell::xdg::{
+    ToplevelState, ToplevelStateSet, XdgToplevelSurfaceRoleAttributes,
+};
 use wayland_protocols_wlr::foreign_toplevel::v1::server::{
     zwlr_foreign_toplevel_handle_v1, zwlr_foreign_toplevel_manager_v1,
 };
@@ -19,7 +21,7 @@ use zwlr_foreign_toplevel_handle_v1::ZwlrForeignToplevelHandleV1;
 use zwlr_foreign_toplevel_manager_v1::ZwlrForeignToplevelManagerV1;
 
 use crate::niri::State;
-use crate::utils::with_toplevel_role;
+use crate::utils::with_toplevel_role_and_current;
 
 const VERSION: u32 = 3;
 
@@ -96,11 +98,16 @@ pub fn refresh(state: &mut State) {
     state.niri.layout.with_windows(|mapped, output, _, _| {
         let toplevel = mapped.toplevel();
         let wl_surface = toplevel.wl_surface();
-        with_toplevel_role(toplevel, |role| {
+        with_toplevel_role_and_current(toplevel, |role, cur| {
+            let Some(cur) = cur else {
+                error!("mapped must have had initial commit");
+                return;
+            };
+
             if state.niri.keyboard_focus.surface() == Some(wl_surface) {
                 focused = Some((mapped.window.clone(), output.cloned()));
             } else {
-                refresh_toplevel(protocol_state, wl_surface, role, output, false);
+                refresh_toplevel(protocol_state, wl_surface, role, cur, output, false);
             }
         });
     });
@@ -109,8 +116,13 @@ pub fn refresh(state: &mut State) {
     if let Some((window, output)) = focused {
         let toplevel = window.toplevel().expect("no X11 support");
         let wl_surface = toplevel.wl_surface();
-        with_toplevel_role(toplevel, |role| {
+        with_toplevel_role_and_current(toplevel, |role, cur| {
-            refresh_toplevel(protocol_state, wl_surface, role, output.as_ref(), true);
+            let Some(cur) = cur else {
+                error!("mapped must have had initial commit");
+                return;
+            };
+
+            refresh_toplevel(protocol_state, wl_surface, role, cur, output.as_ref(), true);
         });
     }
 }
@@ -144,10 +156,11 @@ fn refresh_toplevel(
     protocol_state: &mut ForeignToplevelManagerState,
     wl_surface: &WlSurface,
     role: &XdgToplevelSurfaceRoleAttributes,
+    current: &ToplevelState,
     output: Option<&Output>,
     has_focus: bool,
 ) {
-    let states = to_state_vec(&role.current.states, has_focus);
+    let states = to_state_vec(&current.states, has_focus);
 
     match protocol_state.toplevels.entry(wl_surface.clone()) {
         Entry::Occupied(entry) => {

src/tests/window_opening.rs

@@ -65,6 +65,7 @@ fn simple() {
 }
 
 #[test]
+#[should_panic(expected = "Protocol error 3 on object xdg_surface")]
 fn dont_ack_initial_configure() {
     let mut f = Fixture::new();
     f.add_output(1, (1920, 1080));
@@ -80,19 +81,6 @@ fn dont_ack_initial_configure() {
     // Don't ack the configure.
     window.commit();
     f.double_roundtrip(id);
-
-    // FIXME: Technically this is a protocol violation but uh. Smithay currently doesn't check it,
-    // and I'm not sure if it can be done generically in Smithay (because a compositor may not use
-    // its rendering helpers). I might add a check in niri itself sometime; I'm just not sure if
-    // there might be clients that this could break.
-    let window = f.client(id).window(&surface);
-    assert_snapshot!(
-        window.format_recent_configures(),
-        @r"
-    size: 936 × 1048, bounds: 1888 × 1048, states: []
-    size: 936 × 1048, bounds: 1888 × 1048, states: [Activated]
-    "
-    );
 }
 
 #[derive(Clone, Copy)]

src/utils/mod.rs

@@ -25,7 +25,8 @@ use smithay::utils::{Coordinate, Logical, Point, Rectangle, Size, Transform};
 use smithay::wayland::compositor::{send_surface_state, with_states, SurfaceData};
 use smithay::wayland::fractional_scale::with_fractional_scale;
 use smithay::wayland::shell::xdg::{
-    ToplevelSurface, XdgToplevelSurfaceData, XdgToplevelSurfaceRoleAttributes,
+    ToplevelCachedState, ToplevelState, ToplevelSurface, XdgToplevelSurfaceData,
+    XdgToplevelSurfaceRoleAttributes,
 };
 use wayland_backend::server::Credentials;
 
@@ -279,6 +280,25 @@ pub fn with_toplevel_role<T>(
     })
 }
 
+pub fn with_toplevel_role_and_current<T>(
+    toplevel: &ToplevelSurface,
+    f: impl FnOnce(&mut XdgToplevelSurfaceRoleAttributes, Option<&ToplevelState>) -> T,
+) -> T {
+    with_states(toplevel.wl_surface(), |states| {
+        let mut role = states
+            .data_map
+            .get::<XdgToplevelSurfaceData>()
+            .unwrap()
+            .lock()
+            .unwrap();
+
+        let mut guard = states.cached_state.get::<ToplevelCachedState>();
+        let current = guard.current().last_acked.as_ref().map(|c| &c.state);
+
+        f(&mut role, current)
+    })
+}
+
 pub fn update_tiled_state(
     toplevel: &ToplevelSurface,
     prefer_no_csd: bool,

src/window/mapped.rs

@@ -15,7 +15,10 @@ use smithay::reexports::wayland_server::Resource as _;
 use smithay::utils::{Logical, Point, Rectangle, Scale, Serial, Size, Transform};
 use smithay::wayland::compositor::{remove_pre_commit_hook, with_states, HookId, SurfaceData};
 use smithay::wayland::seat::WaylandFocus;
-use smithay::wayland::shell::xdg::{SurfaceCachedState, ToplevelSurface};
+use smithay::wayland::shell::xdg::{
+    SurfaceCachedState, ToplevelCachedState, ToplevelConfigure, ToplevelSurface,
+    XdgToplevelSurfaceData,
+};
 use wayland_backend::server::Credentials;
 
 use super::{ResolvedWindowRules, WindowRef};
@@ -36,7 +39,7 @@ use crate::utils::id::IdCounter;
 use crate::utils::transaction::Transaction;
 use crate::utils::{
     get_credentials_for_surface, send_scale_transform, update_tiled_state, with_toplevel_role,
-    ResizeEdge,
+    with_toplevel_role_and_current, ResizeEdge,
 };
 
 #[derive(Debug)]
@@ -720,28 +723,35 @@ impl LayoutElement for Mapped {
         // configure, whereas what we potentially want is to unfullscreen the window into its
         // fullscreen size.
         let already_sent = with_toplevel_role(self.toplevel(), |role| {
-            let (last_sent, last_serial) = if let Some(configure) = role.pending_configures().last()
+            let last_sent = if let Some(configure) = role.pending_configures().last() {
-            {
                 // FIXME: it would be more optimal to find the *oldest* pending configure that
                 // has the same size and fullscreen state to the last pending configure.
-                (&configure.state, configure.serial)
+                configure
             } else {
-                (
+                role.last_acked.as_ref().unwrap()
-                    role.last_acked.as_ref().unwrap(),
-                    role.configure_serial.unwrap(),
-                )
             };
+            let ToplevelConfigure {
+                serial: last_serial,
+                state: last_sent,
+            } = last_sent;
 
             let same_size = last_sent.size.unwrap_or_default() == size;
             let has_fullscreen = last_sent.states.contains(xdg_toplevel::State::Fullscreen);
             let same_fullscreen = has_fullscreen == self.is_pending_windowed_fullscreen;
-            (same_size && same_fullscreen).then_some(last_serial)
+            (same_size && same_fullscreen).then_some(*last_serial)
         });
 
         if let Some(serial) = already_sent {
-            if let Some(current_serial) =
+            let current_serial = with_states(self.toplevel().wl_surface(), |states| {
-                with_toplevel_role(self.toplevel(), |role| role.current_serial)
+                states
-            {
+                    .cached_state
+                    .get::<ToplevelCachedState>()
+                    .current()
+                    .last_acked
+                    .as_ref()
+                    .map(|c| c.serial)
+            });
+            if let Some(current_serial) = current_serial {
                 // God this triple negative...
                 if !current_serial.is_no_older_than(&serial) {
                     // We have already sent a request for the new size, but the surface has not
@@ -805,7 +815,9 @@ impl LayoutElement for Mapped {
 
     fn has_ssd(&self) -> bool {
         let toplevel = self.toplevel();
-        let mode = with_toplevel_role(self.toplevel(), |role| role.current.decoration_mode);
+        let mode = self
+            .toplevel()
+            .with_committed_state(|current| current.and_then(|s| s.decoration_mode));
 
         match mode {
             Some(zxdg_toplevel_decoration_v1::Mode::ServerSide) => true,
@@ -892,10 +904,10 @@ impl LayoutElement for Mapped {
             return ConfigureIntent::ShouldSend;
         }
 
-        with_toplevel_role(self.toplevel(), |attributes| {
+        with_toplevel_role_and_current(self.toplevel(), |attributes, current_committed| {
             if let Some(server_pending) = &attributes.server_pending {
                 let current_server = attributes.current_server_state();
-                if server_pending != current_server {
+                if *server_pending != current_server {
                     // Something changed. Check if the only difference is the size, and if the
                     // current server size matches the current committed size.
                     let mut current_server_same_size = current_server.clone();
@@ -903,12 +915,17 @@ impl LayoutElement for Mapped {
                     if current_server_same_size == *server_pending {
                         // Only the size changed. Check if the window committed our previous size
                         // request.
-                        if attributes.current.size == current_server.size {
+                        let Some(current_committed) = current_committed else {
+                            error!("mapped must have had initial commit");
+                            return ConfigureIntent::ShouldSend;
+                        };
+
+                        if current_committed.size == current_server.size {
                             // The window had committed for our previous size change, so we can
                             // change the size again.
                             trace!(
                                 "current size matches server size: {:?}",
-                                attributes.current.size
+                                current_committed.size
                             );
                             ConfigureIntent::CanSend
                         } else {
@@ -960,7 +977,7 @@ impl LayoutElement for Mapped {
                 }
 
                 let server_pending = role.server_pending.as_ref().unwrap();
-                server_pending != role.current_server_state()
+                *server_pending != role.current_server_state()
             });
 
         if has_pending_changes {
@@ -1031,10 +1048,8 @@ impl LayoutElement for Mapped {
             return false;
         }
 
-        with_toplevel_role(self.toplevel(), |role| {
+        self.toplevel().with_committed_state(|current| {
-            role.current
+            current.is_some_and(|s| s.states.contains(xdg_toplevel::State::Fullscreen))
-                .states
-                .contains(xdg_toplevel::State::Fullscreen)
         })
     }
 
@@ -1076,7 +1091,14 @@ impl LayoutElement for Mapped {
             return current_size;
         }
 
-        let pending = with_toplevel_role(self.toplevel(), |role| {
+        let pending = with_states(self.toplevel().wl_surface(), |states| {
+            let role = states
+                .data_map
+                .get::<XdgToplevelSurfaceData>()
+                .unwrap()
+                .lock()
+                .unwrap();
+
             // If we have a server-pending size change that we haven't sent yet, use that size.
             if let Some(server_pending) = &role.server_pending {
                 let current_server = role.current_server_state();
@@ -1091,18 +1113,18 @@ impl LayoutElement for Mapped {
             }
 
             // If we have a sent-but-not-committed-to size, use that.
-            let (last_sent, last_serial) = if let Some(configure) = role.pending_configures().last()
+            let last_sent = role
-            {
+                .pending_configures()
-                (&configure.state, configure.serial)
+                .last()
-            } else {
+                .unwrap_or_else(|| role.last_acked.as_ref().unwrap());
-                (
+            let ToplevelConfigure {
-                    role.last_acked.as_ref().unwrap(),
+                state: last_sent,
-                    role.configure_serial.unwrap(),
+                serial: last_serial,
-                )
+            } = last_sent;
-            };
 
-            if let Some(current_serial) = role.current_serial {
+            let mut guard = states.cached_state.get::<ToplevelCachedState>();
-                if !current_serial.is_no_older_than(&last_serial) {
+            if let Some(current) = guard.current().last_acked.as_ref() {
+                if !current.serial.is_no_older_than(last_serial) {
                     return Some((
                         last_sent.size.unwrap_or_default(),
                         last_sent.states.contains(xdg_toplevel::State::Fullscreen),