Merge pull request #689 from mahkoh/jorth/xdg-initial-commit-state

mahkoh · web-flow · commit 3a4ca4defebd · 2025-12-14T23:26:49.000+01:00
xdg-shell: make acked serial part of the pending state
diff --git a/src/ifs/wl_surface.rs b/src/ifs/wl_surface.rs
@@ -486,6 +486,7 @@ struct PendingState {
     commit_time: Option<u64>,
     tray_item_ack_serial: Option<u32>,
     color_description: Option<Option<Rc<ColorDescription>>>,
+    serial: Option<u64>,
 }
 
 struct AttachedSubsurfaceState {
@@ -539,6 +540,7 @@ impl PendingState {
         opt!(commit_time);
         opt!(tray_item_ack_serial);
         opt!(color_description);
+        opt!(serial);
         {
             let (dx1, dy1) = self.offset;
             let (dx2, dy2) = mem::take(&mut next.offset);
diff --git a/src/ifs/wl_surface/xdg_surface.rs b/src/ifs/wl_surface/xdg_surface.rs
@@ -87,8 +87,9 @@ pub struct XdgSurface {
     base: Rc<XdgWmBase>,
     role: Cell<XdgSurfaceRole>,
     pub surface: Rc<WlSurface>,
-    requested_serial: NumCell<u32>,
-    acked_serial: Cell<Option<u32>>,
+    requested_serial: NumCell<u64>,
+    acked_serial: Cell<u64>,
+    applied_serial: Cell<u64>,
     geometry: Cell<Option<Rect>>,
     extents: Cell<Rect>,
     effective_geometry: Cell<Rect>,
@@ -99,11 +100,19 @@ pub struct XdgSurface {
     popups: CopyHashMap<XdgPopupId, Rc<Popup>>,
     pub workspace: CloneCell<Option<Rc<WorkspaceNode>>>,
     pub tracker: Tracker<Self>,
-    have_initial_commit: Cell<bool>,
+    initial_commit_state: Cell<InitialCommitState>,
     configure_scheduled: Cell<bool>,
     destroyed: Cell<bool>,
 }
 
+#[derive(Copy, Clone, Debug, Eq, PartialEq, Default)]
+enum InitialCommitState {
+    #[default]
+    Unmapped,
+    Sent,
+    Mapped,
+}
+
 struct Popup {
     parent: Rc<XdgSurface>,
     popup: Rc<XdgPopup>,
@@ -209,8 +218,8 @@ impl PendingXdgSurfaceData {
 }
 
 pub trait XdgSurfaceExt: Debug {
-    fn initial_configure(self: Rc<Self>) -> Result<(), XdgSurfaceError> {
-        Ok(())
+    fn initial_configure(self: Rc<Self>) {
+        // nothing
     }
 
     fn post_commit(self: Rc<Self>) {
@@ -250,7 +259,8 @@ impl XdgSurface {
             role: Cell::new(XdgSurfaceRole::None),
             surface: surface.clone(),
             requested_serial: NumCell::new(1),
-            acked_serial: Cell::new(None),
+            acked_serial: Cell::new(0),
+            applied_serial: Cell::new(0),
             geometry: Cell::new(None),
             extents: Cell::new(surface.extents.get()),
             effective_geometry: Default::default(),
@@ -261,7 +271,7 @@ impl XdgSurface {
             popups: Default::default(),
             workspace: Default::default(),
             tracker: Default::default(),
-            have_initial_commit: Default::default(),
+            initial_commit_state: Default::default(),
             configure_scheduled: Default::default(),
             destroyed: Default::default(),
         }
@@ -350,17 +360,34 @@ impl XdgSurface {
         if self.configure_scheduled.replace(true) {
             return;
         }
-        let serial = self.requested_serial.add_fetch(1);
         self.surface
             .client
             .state
             .xdg_surface_configure_events
             .push(XdgSurfaceConfigureEvent {
                 xdg: self.clone(),
-                serial,
+                serial: self.next_serial() as _,
             });
     }
 
+    fn next_serial(&self) -> u64 {
+        let mut serial = self.requested_serial.add_fetch(1);
+        if serial as u32 == 0 {
+            serial = self.requested_serial.add_fetch(1);
+        }
+        serial
+    }
+
+    fn map_serial(&self, serial: u32) -> u64 {
+        let max = self.requested_serial.get();
+        let mask = u32::MAX as u64;
+        let mut serial = max & !mask | (serial as u64);
+        if serial > max {
+            serial = serial.saturating_sub(mask + 1);
+        }
+        serial
+    }
+
     pub fn send_configure(&self, serial: u32) {
         self.surface.client.event(Configure {
             self_id: self.id,
@@ -510,9 +537,13 @@ impl XdgSurfaceRequestHandler for XdgSurface {
     }
 
     fn ack_configure(&self, req: AckConfigure, _slf: &Rc<Self>) -> Result<(), Self::Error> {
-        if self.requested_serial.get() == req.serial {
-            self.acked_serial.set(Some(req.serial));
+        let serial = self.map_serial(req.serial);
+        let last = self.acked_serial.get();
+        if serial <= last {
+            return Err(XdgSurfaceError::InvalidSerial(serial, last));
         }
+        self.acked_serial.set(serial);
+        self.surface.pending.borrow_mut().serial = Some(serial);
         Ok(())
     }
 }
@@ -616,26 +647,41 @@ impl SurfaceExt for XdgSurface {
         self: Rc<Self>,
         pending: &mut PendingState,
     ) -> Result<(), WlSurfaceError> {
-        if !self.have_initial_commit.get()
-            && let Some(ext) = self.ext.get()
-        {
-            ext.initial_configure()?;
-            self.schedule_configure();
-            self.have_initial_commit.set(true);
-        }
-        if let Some(pending) = &mut pending.xdg_surface
-            && let Some(geometry) = pending.geometry.take()
-        {
-            let prev = self.geometry.replace(Some(geometry));
-            if prev != Some(geometry) {
-                self.update_effective_geometry();
-                self.update_extents();
+        if let Some(pending) = &mut pending.xdg_surface {
+            if let Some(geometry) = pending.geometry.take() {
+                let prev = self.geometry.replace(Some(geometry));
+                if prev != Some(geometry) {
+                    self.update_effective_geometry();
+                    self.update_extents();
+                }
             }
         }
+        if let Some(serial) = pending.serial.take() {
+            self.applied_serial.set(serial);
+        }
         Ok(())
     }
 
     fn after_apply_commit(self: Rc<Self>) {
+        match self.initial_commit_state.get() {
+            InitialCommitState::Unmapped => {
+                if let Some(ext) = self.ext.get() {
+                    ext.initial_configure();
+                    self.schedule_configure();
+                    self.initial_commit_state.set(InitialCommitState::Sent);
+                }
+            }
+            InitialCommitState::Sent => {
+                if self.surface.buffer.is_some() {
+                    self.initial_commit_state.set(InitialCommitState::Mapped);
+                }
+            }
+            InitialCommitState::Mapped => {
+                if self.surface.buffer.is_none() {
+                    self.initial_commit_state.set(InitialCommitState::Unmapped);
+                }
+            }
+        }
         if let Some(ext) = self.ext.get() {
             ext.post_commit();
         }
@@ -682,6 +728,8 @@ pub enum XdgSurfaceError {
     AlreadyConstructed,
     #[error(transparent)]
     WlSurfaceError(Box<WlSurfaceError>),
+    #[error("The serial {0} is not larger than the previously acked serial {1}")]
+    InvalidSerial(u64, u64),
 }
 efrom!(XdgSurfaceError, WlSurfaceError);
 efrom!(XdgSurfaceError, ClientError);
diff --git a/src/ifs/wl_surface/xdg_surface/xdg_popup.rs b/src/ifs/wl_surface/xdg_surface/xdg_popup.rs
@@ -7,7 +7,7 @@ use {
             wl_seat::{NodeSeatState, WlSeatGlobal, tablet::TabletTool},
             wl_surface::{
                 tray::TrayItemId,
-                xdg_surface::{XdgSurface, XdgSurfaceError, XdgSurfaceExt},
+                xdg_surface::{XdgSurface, XdgSurfaceExt},
             },
             xdg_positioner::{
                 CA_FLIP_X, CA_FLIP_Y, CA_RESIZE_X, CA_RESIZE_Y, CA_SLIDE_X, CA_SLIDE_Y,
@@ -417,13 +417,12 @@ impl StackedNode for XdgPopup {
 }
 
 impl XdgSurfaceExt for XdgPopup {
-    fn initial_configure(self: Rc<Self>) -> Result<(), XdgSurfaceError> {
+    fn initial_configure(self: Rc<Self>) {
         if let Some(parent) = self.parent.get() {
             self.update_position(&*parent);
             let rel = self.relative_position.get();
             self.send_configure(rel.x1(), rel.y1(), rel.width(), rel.height());
         }
-        Ok(())
     }
 
     fn post_commit(self: Rc<Self>) {
diff --git a/src/ifs/wl_surface/xdg_surface/xdg_toplevel.rs b/src/ifs/wl_surface/xdg_surface/xdg_toplevel.rs
@@ -13,8 +13,7 @@ use {
             wl_surface::{
                 WlSurface,
                 xdg_surface::{
-                    XdgSurface, XdgSurfaceError, XdgSurfaceExt,
-                    xdg_toplevel::xdg_dialog_v1::XdgDialogV1,
+                    XdgSurface, XdgSurfaceExt, xdg_toplevel::xdg_dialog_v1::XdgDialogV1,
                 },
             },
             xdg_toplevel_drag_v1::XdgToplevelDragV1,
@@ -753,14 +752,13 @@ impl ToplevelNodeBase for XdgToplevel {
 }
 
 impl XdgSurfaceExt for XdgToplevel {
-    fn initial_configure(self: Rc<Self>) -> Result<(), XdgSurfaceError> {
+    fn initial_configure(self: Rc<Self>) {
         let rect = self.xdg.absolute_desired_extents.get();
         if rect.is_empty() {
             self.send_configure(0, 0);
         } else {
             self.send_configure_checked(rect.width(), rect.height());
         }
-        Ok(())
     }
 
     fn post_commit(self: Rc<Self>) {
diff --git a/src/it/test_ifs/test_xdg_base.rs b/src/it/test_ifs/test_xdg_base.rs
@@ -50,7 +50,7 @@ impl TestXdgWmBase {
             tran: self.tran.clone(),
             _server: server,
             destroyed: Cell::new(false),
-            last_serial: Cell::new(0),
+            last_serial: Default::default(),
         });
         self.tran.add_obj(xdg.clone())?;
         Ok(xdg)
diff --git a/src/it/test_ifs/test_xdg_surface.rs b/src/it/test_ifs/test_xdg_surface.rs
@@ -19,7 +19,7 @@ pub struct TestXdgSurface {
     pub tran: Rc<TestTransport>,
     pub _server: Rc<XdgSurface>,
     pub destroyed: Cell<bool>,
-    pub last_serial: Cell<u32>,
+    pub last_serial: Cell<Option<u32>>,
 }
 
 impl TestXdgSurface {
@@ -63,7 +63,7 @@ impl TestXdgSurface {
 
     fn handle_configure(&self, parser: MsgParser<'_, '_>) -> Result<(), TestError> {
         let ev = Configure::parse_full(parser)?;
-        self.last_serial.set(ev.serial);
+        self.last_serial.set(Some(ev.serial));
         Ok(())
     }
 }
diff --git a/src/it/test_utils/test_window.rs b/src/it/test_utils/test_window.rs
@@ -15,7 +15,9 @@ pub struct TestWindow {
 
 impl TestWindow {
     pub async fn map(&self) -> Result<(), TestError> {
-        self.xdg.ack_configure(self.xdg.last_serial.get())?;
+        if let Some(serial) = self.xdg.last_serial.take() {
+            self.xdg.ack_configure(serial)?;
+        }
         self.surface
             .map(self.tl.core.width.get(), self.tl.core.height.get())
             .await?;

Original file line number	Diff line number	Diff line change
`@@ -486,6 +486,7 @@ struct PendingState {`
`486`	`486`	`commit_time: Option<u64>,`
`487`	`487`	`tray_item_ack_serial: Option<u32>,`
`488`	`488`	`color_description: Option<Option<Rc<ColorDescription>>>,`
	`489`	`+ serial: Option<u64>,`
`489`	`490`	`}`
`490`	`491`
`491`	`492`	`struct AttachedSubsurfaceState {`
`@@ -539,6 +540,7 @@ impl PendingState {`
`539`	`540`	`opt!(commit_time);`
`540`	`541`	`opt!(tray_item_ack_serial);`
`541`	`542`	`opt!(color_description);`
	`543`	`+ opt!(serial);`
`542`	`544`	`{`
`543`	`545`	`let (dx1, dy1) = self.offset;`
`544`	`546`	`let (dx2, dy2) = mem::take(&mut next.offset);`
Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@ pub struct TestXdgSurface {`
`19`	`19`	`pub tran: Rc<TestTransport>,`
`20`	`20`	`pub _server: Rc<XdgSurface>,`
`21`	`21`	`pub destroyed: Cell<bool>,`
`22`		`- pub last_serial: Cell<u32>,`
	`22`	`+ pub last_serial: Cell<Option<u32>>,`
`23`	`23`	`}`
`24`	`24`
`25`	`25`	`impl TestXdgSurface {`
`@@ -63,7 +63,7 @@ impl TestXdgSurface {`
`63`	`63`
`64`	`64`	`fn handle_configure(&self, parser: MsgParser<'_, '_>) -> Result<(), TestError> {`
`65`	`65`	`let ev = Configure::parse_full(parser)?;`
`66`		`- self.last_serial.set(ev.serial);`
	`66`	`+ self.last_serial.set(Some(ev.serial));`
`67`	`67`	`Ok(())`
`68`	`68`	`}`
`69`	`69`	`}`