From 17ac4ca0e12a0758ef40c2a8bf3b8c7d96db838a Mon Sep 17 00:00:00 2001 From: Julian Orth Date: Fri, 19 Sep 2025 12:27:42 +0200 Subject: [PATCH] xdg-shell: make acked serial part of the pending state --- src/ifs/wl_surface.rs | 2 + src/ifs/wl_surface/xdg_surface.rs | 98 ++++++++++++++----- src/ifs/wl_surface/xdg_surface/xdg_popup.rs | 5 +- .../wl_surface/xdg_surface/xdg_toplevel.rs | 6 +- src/it/test_ifs/test_xdg_base.rs | 2 +- src/it/test_ifs/test_xdg_surface.rs | 4 +- src/it/test_utils/test_window.rs | 4 +- 7 files changed, 85 insertions(+), 36 deletions(-) diff --git a/src/ifs/wl_surface.rs b/src/ifs/wl_surface.rs index 99c6c27b..54ed4b4d 100644 --- a/src/ifs/wl_surface.rs +++ b/src/ifs/wl_surface.rs @@ -486,6 +486,7 @@ struct PendingState { commit_time: Option, tray_item_ack_serial: Option, color_description: Option>>, + serial: Option, } 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 index 2c81c72c..69a5f085 100644 --- 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, role: Cell, pub surface: Rc, - requested_serial: NumCell, - acked_serial: Cell>, + requested_serial: NumCell, + acked_serial: Cell, + applied_serial: Cell, geometry: Cell>, extents: Cell, effective_geometry: Cell, @@ -99,11 +100,19 @@ pub struct XdgSurface { popups: CopyHashMap>, pub workspace: CloneCell>>, pub tracker: Tracker, - have_initial_commit: Cell, + initial_commit_state: Cell, configure_scheduled: Cell, destroyed: Cell, } +#[derive(Copy, Clone, Debug, Eq, PartialEq, Default)] +enum InitialCommitState { + #[default] + Unmapped, + Sent, + Mapped, +} + struct Popup { parent: Rc, popup: Rc, @@ -209,8 +218,8 @@ impl PendingXdgSurfaceData { } pub trait XdgSurfaceExt: Debug { - fn initial_configure(self: Rc) -> Result<(), XdgSurfaceError> { - Ok(()) + fn initial_configure(self: Rc) { + // nothing } fn post_commit(self: Rc) { @@ -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) -> 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, 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) { + 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), + #[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 index 64808810..f4c98bde 100644 --- 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) -> Result<(), XdgSurfaceError> { + fn initial_configure(self: Rc) { 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) { diff --git a/src/ifs/wl_surface/xdg_surface/xdg_toplevel.rs b/src/ifs/wl_surface/xdg_surface/xdg_toplevel.rs index 424449d5..2ceaaa35 100644 --- 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) -> Result<(), XdgSurfaceError> { + fn initial_configure(self: Rc) { 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) { diff --git a/src/it/test_ifs/test_xdg_base.rs b/src/it/test_ifs/test_xdg_base.rs index 781697a9..a9a13cf4 100644 --- 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 index a9d3269b..9d838087 100644 --- 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, pub _server: Rc, pub destroyed: Cell, - pub last_serial: Cell, + pub last_serial: Cell>, } 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 index 70dc7644..7e02bf65 100644 --- 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?;