diff --git a/.gitignore b/.gitignore index 0bf1ddb2ec3b..84f58efc62ff 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ Cargo.lock *.bk .vscode .vagga +/wlcs diff --git a/CHANGELOG.md b/CHANGELOG.md index da4d3a59eb4b..194b7985b61b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,133 @@ enables accepting dmabufs originating from any node. All other methods have moved to this new type, allowing compositors to take the same lock across multiple operations. +xdg_shell, layer_shell, and session_lock surfaces now track `last_acked` state for each commit. +You can find it in the `last_acked` field of the corresponding `...CachedState` structs. + +The "current state" and "current serial" attribute fields were removed; use `last_acked` from the new `with_committed_state()` accessors instead (and verify that you didn't actually want `with_pending_state()` instead where you used the "current state"). + +```diff +-ToplevelSurface::current_state(); ++ToplevelSurface::with_committed_state(|state| { ++ // ... ++}); + +-PopupSurface::current_state(); ++PopupSurface::with_committed_state(|state| { ++ // ... ++}); + +-LayerSurface::current_state(); ++LayerSurface::with_committed_state(|state| { ++ // ... ++}); + +-LockSurface::current_state(); ++LockSurface::with_committed_state(|state| { ++ // ... ++}); + +struct XdgToplevelSurfaceRoleAttributes { +- configured: bool, +- configure_serial: Option, +- current: ToplevelState, +- current_serial: Option, +- last_acked: Option, ++ last_acked: Option, + // ... +} + +struct XdgPopupSurfaceRoleAttributes { +- configured: bool, +- configure_serial: Option, +- current: PopupState, +- current_serial: Option, +- committed: bool, +- last_acked: Option, ++ last_acked: Option, + // ... +} + +struct LayerSurfaceAttributes { +- configured: bool, +- configure_serial: Option, +- current: LayerSurfaceState, +- last_acked: Option, ++ last_acked: Option, + // ... +} + +-impl Copy for LayerSurfaceCachedState; +``` + +The following methods are no longer needed as Smithay does them automatically now: +```diff +-ToplevelSurface::reset_initial_configure_sent(); +-PopupSurface::reset_initial_configure_sent(); +``` + +You also no longer need to manually set `LayerSurfaceAttributes::initial_configure_sent`, Smithay handles it automatically. + +### Additions + +xdg_shell and layer_shell now enforce the client acking a configure before committing a buffer, as required by the protocols. + +```rs +struct ToplevelCachedState { + /// Configure last acknowledged by the client at the time of the commit. + last_acked: Option, +} + +/// Provides access to the current committed cached state. +fn ToplevelSurface::with_cached_state(&self, f: impl FnOnce(&ToplevelCachedState) -> T) -> T; +/// Provides access to the current committed state. +fn ToplevelSurface::with_committed_state(&self, f: impl FnOnce(Option<&ToplevelState>) -> T) -> T; + +struct PopupCachedState { + /// Configure last acknowledged by the client at the time of the commit. + last_acked: Option, +} + +/// Provides access to the current committed cached state. +fn PopupSurface::with_cached_state(&self, f: impl FnOnce(&PopupCachedState) -> T) -> T; +/// Provides access to the current committed state. +fn PopupSurface::with_committed_state(&self, f: impl FnOnce(Option<&PopupState>) -> T) -> T; + +struct LayerSurfaceCachedState { + /// Configure last acknowledged by the client at the time of the commit. + last_acked: Option, + // ... +} + +/// Provides access to the current committed cached state. +fn LayerSurface::with_cached_state(&self, f: impl FnOnce(&LayerSurfaceCachedState) -> T) -> T; +/// Provides access to the current committed state. +fn LayerSurface::with_committed_state(&self, f: impl FnOnce(Option<&LayerSurfaceState>) -> T) -> T; + +struct LockSurfaceCachedState { + /// Configure last acknowledged by the client at the time of the commit. + last_acked: Option, +} + +/// Provides access to the current committed cached state. +fn LockSurface::with_cached_state(&self, f: impl FnOnce(&LockSurfaceCachedState) -> T) -> T; +/// Provides access to the current committed state. +fn LockSurface::with_committed_state(&self, f: impl FnOnce(Option<&LockSurfaceState>) -> T) -> T; + +struct LockSurfaceAttributes { + server_pending: Option, + pending_configures: Vec, + last_acked: Option, +} + +type LockSurfaceData = Mutex; + +struct LockSurfaceConfigure { + state: LockSurfaceState, + serial: Serial, +} +``` + ## 0.7.0 ### Breaking changes diff --git a/anvil/src/input_handler.rs b/anvil/src/input_handler.rs index 14a113fbc4a9..1681f75f8685 100644 --- a/anvil/src/input_handler.rs +++ b/anvil/src/input_handler.rs @@ -24,10 +24,9 @@ use smithay::{ }, utils::{Logical, Point, Serial, Transform, SERIAL_COUNTER as SCOUNTER}, wayland::{ - compositor::with_states, input_method::InputMethodSeat, keyboard_shortcuts_inhibit::KeyboardShortcutsInhibitorSeat, - shell::wlr_layer::{KeyboardInteractivity, Layer as WlrLayer, LayerSurfaceCachedState}, + shell::wlr_layer::{KeyboardInteractivity, Layer as WlrLayer}, }, }; @@ -146,12 +145,11 @@ impl AnvilState { let keyboard = self.seat.get_keyboard().unwrap(); for layer in self.layer_shell_state.layer_surfaces().rev() { - let data = with_states(layer.wl_surface(), |states| { - *states.cached_state.get::().current() + let exclusive = layer.with_cached_state(|data| { + data.keyboard_interactivity == KeyboardInteractivity::Exclusive + && (data.layer == WlrLayer::Top || data.layer == WlrLayer::Overlay) }); - if data.keyboard_interactivity == KeyboardInteractivity::Exclusive - && (data.layer == WlrLayer::Top || data.layer == WlrLayer::Overlay) - { + if exclusive { let surface = self.space.outputs().find_map(|o| { let map = layer_map_for_output(o); let cloned = map.layers().find(|l| l.layer_surface() == &layer).cloned(); diff --git a/anvil/src/shell/xdg.rs b/anvil/src/shell/xdg.rs index 3b53483b2fe0..7b31b5af8b83 100644 --- a/anvil/src/shell/xdg.rs +++ b/anvil/src/shell/xdg.rs @@ -20,8 +20,8 @@ use smithay::{ compositor::{self, with_states}, seat::WaylandFocus, shell::xdg::{ - Configure, PopupSurface, PositionerState, ToplevelSurface, XdgShellHandler, XdgShellState, - XdgToplevelSurfaceData, + Configure, PopupSurface, PositionerState, ToplevelCachedState, ToplevelSurface, XdgShellHandler, + XdgShellState, }, }, }; @@ -221,14 +221,12 @@ impl XdgShellHandler for AnvilState { // will no longer have the resize state set let is_resizing = with_states(&surface, |states| { states - .data_map - .get::() - .unwrap() - .lock() - .unwrap() - .current - .states - .contains(xdg_toplevel::State::Resizing) + .cached_state + .get::() + .current() + .last_acked + .as_ref() + .is_some_and(|c| c.state.states.contains(xdg_toplevel::State::Resizing)) }); if configure.serial >= serial && is_resizing { @@ -264,49 +262,43 @@ impl XdgShellHandler for AnvilState { } fn fullscreen_request(&mut self, surface: ToplevelSurface, mut wl_output: Option) { - if surface - .current_state() - .capabilities - .contains(xdg_toplevel::WmCapabilities::Fullscreen) - { - // NOTE: This is only one part of the solution. We can set the - // location and configure size here, but the surface should be rendered fullscreen - // independently from its buffer size - let wl_surface = surface.wl_surface(); - - let output_geometry = fullscreen_output_geometry(wl_surface, wl_output.as_ref(), &mut self.space); - - if let Some(geometry) = output_geometry { - let output = wl_output - .as_ref() - .and_then(Output::from_resource) - .unwrap_or_else(|| self.space.outputs().next().unwrap().clone()); - let client = match self.display_handle.get_client(wl_surface.id()) { - Ok(client) => client, - Err(_) => return, - }; - for output in output.client_outputs(&client) { - wl_output = Some(output); - } - let window = self - .space - .elements() - .find(|window| window.wl_surface().map(|s| &*s == wl_surface).unwrap_or(false)) - .unwrap(); - - surface.with_pending_state(|state| { - state.states.set(xdg_toplevel::State::Fullscreen); - state.size = Some(geometry.size); - state.fullscreen_output = wl_output; - }); - output.user_data().insert_if_missing(FullscreenSurface::default); - output - .user_data() - .get::() - .unwrap() - .set(window.clone()); - trace!("Fullscreening: {:?}", window); + // NOTE: This is only one part of the solution. We can set the + // location and configure size here, but the surface should be rendered fullscreen + // independently from its buffer size + let wl_surface = surface.wl_surface(); + + let output_geometry = fullscreen_output_geometry(wl_surface, wl_output.as_ref(), &mut self.space); + + if let Some(geometry) = output_geometry { + let output = wl_output + .as_ref() + .and_then(Output::from_resource) + .unwrap_or_else(|| self.space.outputs().next().unwrap().clone()); + let client = match self.display_handle.get_client(wl_surface.id()) { + Ok(client) => client, + Err(_) => return, + }; + for output in output.client_outputs(&client) { + wl_output = Some(output); } + let window = self + .space + .elements() + .find(|window| window.wl_surface().map(|s| &*s == wl_surface).unwrap_or(false)) + .unwrap(); + + surface.with_pending_state(|state| { + state.states.set(xdg_toplevel::State::Fullscreen); + state.size = Some(geometry.size); + state.fullscreen_output = wl_output; + }); + output.user_data().insert_if_missing(FullscreenSurface::default); + output + .user_data() + .get::() + .unwrap() + .set(window.clone()); + trace!("Fullscreening: {:?}", window); } // The protocol demands us to always reply with a configure, @@ -319,14 +311,6 @@ impl XdgShellHandler for AnvilState { } fn unfullscreen_request(&mut self, surface: ToplevelSurface) { - if !surface - .current_state() - .states - .contains(xdg_toplevel::State::Fullscreen) - { - return; - } - let ret = surface.with_pending_state(|state| { state.states.unset(xdg_toplevel::State::Fullscreen); state.size = None; @@ -341,33 +325,33 @@ impl XdgShellHandler for AnvilState { } } - surface.send_pending_configure(); + // The protocol demands us to always reply with a configure, + // regardless of we fulfilled the request or not + if surface.is_initial_configure_sent() { + surface.send_configure(); + } else { + // Will be sent during initial configure + } } fn maximize_request(&mut self, surface: ToplevelSurface) { // NOTE: This should use layer-shell when it is implemented to // get the correct maximum size - if surface - .current_state() - .capabilities - .contains(xdg_toplevel::WmCapabilities::Maximize) - { - let window = self.window_for_surface(surface.wl_surface()).unwrap(); - let outputs_for_window = self.space.outputs_for_element(&window); - let output = outputs_for_window - .first() - // The window hasn't been mapped yet, use the primary output instead - .or_else(|| self.space.outputs().next()) - // Assumes that at least one output exists - .expect("No outputs found"); - let geometry = self.space.output_geometry(output).unwrap(); + let window = self.window_for_surface(surface.wl_surface()).unwrap(); + let outputs_for_window = self.space.outputs_for_element(&window); + let output = outputs_for_window + .first() + // The window hasn't been mapped yet, use the primary output instead + .or_else(|| self.space.outputs().next()) + // Assumes that at least one output exists + .expect("No outputs found"); + let geometry = self.space.output_geometry(output).unwrap(); - surface.with_pending_state(|state| { - state.states.set(xdg_toplevel::State::Maximized); - state.size = Some(geometry.size); - }); - self.space.map_element(window, geometry.loc, true); - } + surface.with_pending_state(|state| { + state.states.set(xdg_toplevel::State::Maximized); + state.size = Some(geometry.size); + }); + self.space.map_element(window, geometry.loc, true); // The protocol demands us to always reply with a configure, // regardless of we fulfilled the request or not @@ -379,19 +363,18 @@ impl XdgShellHandler for AnvilState { } fn unmaximize_request(&mut self, surface: ToplevelSurface) { - if !surface - .current_state() - .states - .contains(xdg_toplevel::State::Maximized) - { - return; - } - surface.with_pending_state(|state| { state.states.unset(xdg_toplevel::State::Maximized); state.size = None; }); - surface.send_pending_configure(); + + // The protocol demands us to always reply with a configure, + // regardless of we fulfilled the request or not + if surface.is_initial_configure_sent() { + surface.send_configure(); + } else { + // Will be sent during initial configure + } } fn grab(&mut self, surface: PopupSurface, seat: wl_seat::WlSeat, serial: Serial) { @@ -469,13 +452,15 @@ impl AnvilState { let mut initial_window_location = self.space.element_location(&window).unwrap(); // If surface is maximized then unmaximize it - let current_state = surface.current_state(); - if current_state.states.contains(xdg_toplevel::State::Maximized) { - surface.with_pending_state(|state| { - state.states.unset(xdg_toplevel::State::Maximized); + let changed = surface.with_pending_state(|state| { + if state.states.unset(xdg_toplevel::State::Maximized) { state.size = None; - }); - + true + } else { + false + } + }); + if changed { surface.send_configure(); // NOTE: In real compositor mouse location should be mapped to a new window size @@ -533,13 +518,15 @@ impl AnvilState { let mut initial_window_location = self.space.element_location(&window).unwrap(); // If surface is maximized then unmaximize it - let current_state = surface.current_state(); - if current_state.states.contains(xdg_toplevel::State::Maximized) { - surface.with_pending_state(|state| { - state.states.unset(xdg_toplevel::State::Maximized); + let changed = surface.with_pending_state(|state| { + if state.states.unset(xdg_toplevel::State::Maximized) { state.size = None; - }); - + true + } else { + false + } + }); + if changed { surface.send_configure(); // NOTE: In real compositor mouse location should be mapped to a new window size diff --git a/compile_wlcs.sh b/compile_wlcs.sh index b6d9ae3a8105..81760d871a5c 100755 --- a/compile_wlcs.sh +++ b/compile_wlcs.sh @@ -1,6 +1,6 @@ #!/bin/sh -WLCS_SHA=12234affdc0a4cc104fbaf8a502efc5f822b973b +WLCS_SHA=2b385328366de5db19521b9b3eef81861430d984 if [ -f "./wlcs/wlcs" ] && [ "$(cd wlcs; git rev-parse HEAD)" = "${WLCS_SHA}" ] ; then echo "Using cached WLCS." diff --git a/src/desktop/wayland/layer.rs b/src/desktop/wayland/layer.rs index 7031f4c603e1..497e80b0654b 100644 --- a/src/desktop/wayland/layer.rs +++ b/src/desktop/wayland/layer.rs @@ -297,9 +297,7 @@ impl LayerMap { ) } - let data = with_states(surface, |states| { - *states.cached_state.get::().current() - }); + let data = layer.cached_state(); let mut source = match data.exclusive_zone { ExclusiveZone::Exclusive(_) | ExclusiveZone::Neutral => zone, @@ -551,37 +549,24 @@ impl LayerSurface { self.0.surface.wl_surface() } - /// Returns the cached protocol state + /// Returns a clone of the cached protocol state pub fn cached_state(&self) -> LayerSurfaceCachedState { - with_states(self.0.surface.wl_surface(), |states| { - *states.cached_state.get::().current() - }) + self.0.surface.with_cached_state(Clone::clone) } /// Returns true, if the surface has indicated, that it is able to process keyboard events. pub fn can_receive_keyboard_focus(&self) -> bool { - with_states(self.0.surface.wl_surface(), |states| { - match states - .cached_state - .get::() - .current() - .keyboard_interactivity - { + self.0 + .surface + .with_cached_state(|state| match state.keyboard_interactivity { KeyboardInteractivity::Exclusive | KeyboardInteractivity::OnDemand => true, KeyboardInteractivity::None => false, - } - }) + }) } /// Returns the layer this surface resides on, if any yet. pub fn layer(&self) -> WlrLayer { - with_states(self.0.surface.wl_surface(), |states| { - states - .cached_state - .get::() - .current() - .layer - }) + self.0.surface.with_cached_state(|state| state.layer) } /// Returns the namespace of this surface diff --git a/src/desktop/wayland/popup/manager.rs b/src/desktop/wayland/popup/manager.rs index 8a2a0250ab6e..56fe5fae99f2 100644 --- a/src/desktop/wayland/popup/manager.rs +++ b/src/desktop/wayland/popup/manager.rs @@ -4,12 +4,12 @@ use crate::{ wayland::{ compositor::{get_role, with_states}, seat::WaylandFocus, - shell::xdg::{XdgPopupSurfaceData, XdgPopupSurfaceRoleAttributes, XDG_POPUP_ROLE}, + shell::xdg::{PopupCachedState, XdgPopupSurfaceData, XdgPopupSurfaceRoleAttributes, XDG_POPUP_ROLE}, }, }; use std::sync::{Arc, Mutex}; use tracing::trace; -use wayland_protocols::xdg::shell::server::{xdg_popup, xdg_wm_base}; +use wayland_protocols::xdg::shell::server::xdg_wm_base; use wayland_server::{protocol::wl_surface::WlSurface, Resource}; use super::{PopupGrab, PopupGrabError, PopupGrabInner, PopupKind}; @@ -74,23 +74,7 @@ impl PopupManager { ); match popup { - PopupKind::Xdg(ref xdg) => { - let surface = xdg.wl_surface(); - let committed = with_states(surface, |states| { - states - .data_map - .get::() - .unwrap() - .lock() - .unwrap() - .committed - }); - - if committed { - surface.post_error(xdg_popup::Error::InvalidGrab, "xdg_popup already is mapped"); - return Err(PopupGrabError::InvalidGrab); - } - } + PopupKind::Xdg(ref _xdg) => (), PopupKind::InputMethod(ref _input_method) => { return Err(PopupGrabError::InvalidGrab); } @@ -249,14 +233,12 @@ pub fn get_popup_toplevel_coords(popup: &PopupKind) -> Point { while get_role(&parent) == Some(XDG_POPUP_ROLE) { offset += with_states(&parent, |states| { states - .data_map - .get::>() - .unwrap() - .lock() - .unwrap() - .current - .geometry - .loc + .cached_state + .get::() + .current() + .last_acked + .map(|c| c.state.geometry.loc) + .unwrap_or_default() }); parent = with_states(&parent, |states| { states diff --git a/src/desktop/wayland/popup/mod.rs b/src/desktop/wayland/popup/mod.rs index 6ea9a3957a39..666702aab786 100644 --- a/src/desktop/wayland/popup/mod.rs +++ b/src/desktop/wayland/popup/mod.rs @@ -10,7 +10,7 @@ use crate::{ wayland::{ compositor::with_states, input_method, - shell::xdg::{self, SurfaceCachedState, XdgPopupSurfaceData}, + shell::xdg::{self, SurfaceCachedState}, }, }; @@ -81,21 +81,9 @@ impl PopupKind { } fn location(&self) -> Point { - let wl_surface = self.wl_surface(); - match *self { - PopupKind::Xdg(_) => { - with_states(wl_surface, |states| { - states - .data_map - .get::() - .unwrap() - .lock() - .unwrap() - .current - .geometry - }) - .loc + PopupKind::Xdg(ref t) => { + t.with_committed_state(|current| current.map(|state| state.geometry.loc).unwrap_or_default()) } PopupKind::InputMethod(ref t) => t.location(), } diff --git a/src/wayland/session_lock/lock.rs b/src/wayland/session_lock/lock.rs index 7867e17e511b..9f4492a7e00b 100644 --- a/src/wayland/session_lock/lock.rs +++ b/src/wayland/session_lock/lock.rs @@ -3,14 +3,11 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; -use crate::backend::renderer::buffer_dimensions; -use crate::utils::Size; use crate::wayland::compositor::SurfaceAttributes; use crate::wayland::compositor::{self, BufferAssignment}; -use crate::wayland::viewporter::{ViewportCachedState, ViewporterSurfaceState}; use _session_lock::ext_session_lock_surface_v1::ExtSessionLockSurfaceV1; use _session_lock::ext_session_lock_v1::{Error, ExtSessionLockV1, Request}; -use wayland_protocols::ext::session_lock::v1::server::{self as _session_lock, ext_session_lock_surface_v1}; +use wayland_protocols::ext::session_lock::v1::server::{self as _session_lock}; use wayland_server::{Client, DataInit, Dispatch, DisplayHandle, Resource}; use crate::wayland::session_lock::surface::{ExtLockSurfaceUserData, LockSurface, LockSurfaceAttributes}; @@ -101,75 +98,7 @@ where }); // Add pre-commit hook for updating surface state. - compositor::add_pre_commit_hook::(&surface, |_state, _dh, surface| { - compositor::with_states(surface, |states| { - let attributes = states.data_map.get::>(); - let attributes = attributes.unwrap().lock().unwrap(); - - let Some(state) = attributes.last_acked else { - attributes.surface.post_error( - ext_session_lock_surface_v1::Error::CommitBeforeFirstAck, - "Committed before the first ack_configure.", - ); - return; - }; - - // Verify the attached buffer: ext-session-lock requires no NULL buffers - // and an exact dimentions match. - let mut guard = states.cached_state.get::(); - let surface_attrs = guard.pending(); - if let Some(assignment) = surface_attrs.buffer.as_ref() { - match assignment { - BufferAssignment::Removed => { - attributes.surface.post_error( - ext_session_lock_surface_v1::Error::NullBuffer, - "Surface attached a NULL buffer.", - ); - } - BufferAssignment::NewBuffer(buffer) => { - if let Some(buf_size) = buffer_dimensions(buffer) { - let viewport = states - .data_map - .get::() - .map(|v| v.lock().unwrap()); - let surface_size = if let Some(dest) = - viewport.as_ref().and_then(|_| { - let mut guard = - states.cached_state.get::(); - let viewport_state = guard.pending(); - viewport_state.dst - }) { - Size::from((dest.w as u32, dest.h as u32)) - } else { - let scale = surface_attrs.buffer_scale; - let transform = surface_attrs.buffer_transform.into(); - let surface_size = buf_size.to_logical(scale, transform); - - Size::from((surface_size.w as u32, surface_size.h as u32)) - }; - - if Some(surface_size) != state.size { - attributes.surface.post_error( - ext_session_lock_surface_v1::Error::DimensionsMismatch, - "Surface dimensions do not match acked configure.", - ); - } - } - } - } - } - }); - }); - compositor::add_post_commit_hook::(&surface, |_state, _dh, surface| { - compositor::with_states(surface, |states| { - let attributes = states.data_map.get::>(); - let mut attributes = attributes.unwrap().lock().unwrap(); - - if let Some(state) = attributes.last_acked { - attributes.current = state; - } - }); - }); + compositor::add_pre_commit_hook::(&surface, LockSurface::pre_commit_hook); // Call compositor handler. let lock_surface = LockSurface::new(surface, lock_surface); diff --git a/src/wayland/session_lock/mod.rs b/src/wayland/session_lock/mod.rs index 4981dd080c95..bdac9a125d33 100644 --- a/src/wayland/session_lock/mod.rs +++ b/src/wayland/session_lock/mod.rs @@ -58,13 +58,14 @@ use wayland_server::protocol::wl_output::WlOutput; use wayland_server::protocol::wl_surface::WlSurface; use wayland_server::{Client, DataInit, Dispatch, DisplayHandle, GlobalDispatch, New}; -use crate::wayland::session_lock::surface::LockSurfaceConfigure; - mod lock; mod surface; pub use lock::SessionLockState; -pub use surface::{ExtLockSurfaceUserData, LockSurface, LockSurfaceState}; +pub use surface::{ + ExtLockSurfaceUserData, LockSurface, LockSurfaceAttributes, LockSurfaceCachedState, LockSurfaceConfigure, + LockSurfaceData, LockSurfaceState, +}; const MANAGER_VERSION: u32 = 1; diff --git a/src/wayland/session_lock/surface.rs b/src/wayland/session_lock/surface.rs index 8865a88df800..92e2f10b0731 100644 --- a/src/wayland/session_lock/surface.rs +++ b/src/wayland/session_lock/surface.rs @@ -2,9 +2,12 @@ use std::sync::Mutex; +use crate::backend::renderer::buffer_dimensions; use crate::utils::{IsAlive, Logical, Serial, Size, SERIAL_COUNTER}; -use crate::wayland::compositor; +use crate::wayland::compositor::{self, BufferAssignment, Cacheable, SurfaceAttributes}; +use crate::wayland::viewporter::{ViewportCachedState, ViewporterSurfaceState}; use _session_lock::ext_session_lock_surface_v1::{Error, ExtSessionLockSurfaceV1, Request}; +use tracing::trace_span; use wayland_protocols::ext::session_lock::v1::server::{self as _session_lock, ext_session_lock_surface_v1}; use wayland_server::protocol::wl_surface::WlSurface; use wayland_server::{Client, DataInit, Dispatch, DisplayHandle, Resource, Weak}; @@ -60,16 +63,49 @@ where _ => unreachable!(), } } + + fn destroyed( + _state: &mut D, + _client: wayland_server::backend::ClientId, + _resource: &ExtSessionLockSurfaceV1, + data: &ExtLockSurfaceUserData, + ) { + if let Ok(surface) = data.surface.upgrade() { + compositor::with_states(&surface, |states| { + let mut attributes = states + .data_map + .get::>() + .unwrap() + .lock() + .unwrap(); + attributes.reset(); + + let mut guard = states.cached_state.get::(); + *guard.pending() = Default::default(); + *guard.current() = Default::default(); + }); + } + } } +/// Data associated with session lock surface +/// +/// ```no_run +/// use smithay::wayland::compositor; +/// use smithay::wayland::session_lock::LockSurfaceData; +/// +/// # let wl_surface = todo!(); +/// compositor::with_states(&wl_surface, |states| { +/// states.data_map.get::(); +/// }); +/// ``` +pub type LockSurfaceData = Mutex; + /// Attributes for ext-session-lock surfaces. #[derive(Debug)] pub struct LockSurfaceAttributes { pub(crate) surface: ext_session_lock_surface_v1::ExtSessionLockSurfaceV1, - /// The serial of the last acked configure - pub configure_serial: Option, - /// Holds the pending state as set by the server. pub server_pending: Option, @@ -79,23 +115,19 @@ pub struct LockSurfaceAttributes { /// layer_surface.ack_configure. pub pending_configures: Vec, - /// Holds the last server_pending state that has been acknowledged by the - /// client. This state should be cloned to the current during a commit. - pub last_acked: Option, - - /// Holds the current state of the layer after a successful commit. - pub current: LockSurfaceState, + /// Holds the last configure that has been acknowledged by the client. This state should be + /// cloned to the current during a commit. Note that this state can be newer than the last + /// acked state at the time of the last commit. + pub last_acked: Option, } impl LockSurfaceAttributes { pub(crate) fn new(surface: ext_session_lock_surface_v1::ExtSessionLockSurfaceV1) -> Self { Self { surface, - configure_serial: None, server_pending: None, pending_configures: vec![], last_acked: None, - current: Default::default(), } } @@ -108,11 +140,23 @@ impl LockSurfaceAttributes { self.pending_configures .retain(|configure| configure.serial > serial); - self.last_acked = Some(configure.state); - self.configure_serial = Some(serial); + self.last_acked = Some(configure); Some(configure) } + + fn reset(&mut self) { + self.server_pending = None; + self.pending_configures = Vec::new(); + self.last_acked = None; + } + + fn current_server_state(&self) -> Option<&LockSurfaceState> { + self.pending_configures + .last() + .map(|c| &c.state) + .or(self.last_acked.as_ref().map(|c| &c.state)) + } } /// Handle for a ext-session-lock surface. @@ -147,12 +191,8 @@ impl LockSurface { pub fn get_pending_state(&self, attributes: &mut LockSurfaceAttributes) -> Option { let server_pending = attributes.server_pending.take()?; - // Get the last pending state. - let pending = attributes.pending_configures.last(); - let last_state = pending.map(|c| &c.state).or(attributes.last_acked.as_ref()); - // Check if last state matches pending state. - match last_state { + match attributes.current_server_state() { Some(state) if state == &server_pending => None, _ => Some(server_pending), } @@ -203,26 +243,119 @@ impl LockSurface { let mut attributes = attributes.unwrap().lock().unwrap(); // Ensure pending state is initialized. - let current = attributes.current; - let server_pending = attributes.server_pending.get_or_insert(current); + if attributes.server_pending.is_none() { + attributes.server_pending = + Some(attributes.current_server_state().cloned().unwrap_or_default()); + } + let server_pending = attributes.server_pending.as_mut().unwrap(); f(server_pending) }) } - /// Get the current pending state. - #[allow(unused)] - pub fn current_state(&self) -> LockSurfaceState { + /// Provides access to the current committed cached state. + pub fn with_cached_state(&self, f: F) -> T + where + F: FnOnce(&LockSurfaceCachedState) -> T, + { compositor::with_states(&self.surface, |states| { - states - .data_map - .get::>() - .unwrap() - .lock() - .unwrap() - .current + let mut guard = states.cached_state.get::(); + f(guard.current()) }) } + + /// Provides access to the current committed state. + /// + /// This is the state that the client last acked before making the current commit. + pub fn with_committed_state(&self, f: F) -> T + where + F: FnOnce(Option<&LockSurfaceState>) -> T, + { + self.with_cached_state(move |state| f(state.last_acked.as_ref().map(|c| &c.state))) + } + + /// Handles the role specific commit error checking + /// + /// This should be called when the underlying WlSurface + /// handles a wl_surface.commit request. + pub(crate) fn pre_commit_hook(_state: &mut D, _dh: &DisplayHandle, surface: &WlSurface) { + let _span = trace_span!("session-lock-surface pre-commit", surface = %surface.id()).entered(); + + compositor::with_states(surface, |states| { + let role = states.data_map.get::().unwrap().lock().unwrap(); + + let Some(last_acked) = role.last_acked else { + role.surface.post_error( + ext_session_lock_surface_v1::Error::CommitBeforeFirstAck, + "Committed before the first ack_configure.", + ); + return; + }; + let LockSurfaceConfigure { state, serial: _ } = &last_acked; + + let mut guard_layer = states.cached_state.get::(); + let pending = guard_layer.pending(); + + // The presence of last_acked always follows the buffer assignment because the + // surface is not allowed to attach a buffer without acking the initial configure. + let had_buffer_before = pending.last_acked.is_some(); + + let mut guard_surface = states.cached_state.get::(); + let surface_attrs = guard_surface.pending(); + let has_buffer = match &surface_attrs.buffer { + Some(BufferAssignment::NewBuffer(buffer)) => { + // Verify buffer size. + if let Some(buf_size) = buffer_dimensions(buffer) { + let viewport = states + .data_map + .get::() + .map(|v| v.lock().unwrap()); + let surface_size = if let Some(dest) = viewport.as_ref().and_then(|_| { + let mut guard = states.cached_state.get::(); + let viewport_state = guard.pending(); + viewport_state.dst + }) { + Size::from((dest.w as u32, dest.h as u32)) + } else { + let scale = surface_attrs.buffer_scale; + let transform = surface_attrs.buffer_transform.into(); + let surface_size = buf_size.to_logical(scale, transform); + + Size::from((surface_size.w as u32, surface_size.h as u32)) + }; + + if Some(surface_size) != state.size { + role.surface.post_error( + ext_session_lock_surface_v1::Error::DimensionsMismatch, + "Surface dimensions do not match acked configure.", + ); + return; + } + } + + true + } + Some(BufferAssignment::Removed) => { + role.surface.post_error( + ext_session_lock_surface_v1::Error::NullBuffer, + "Surface attached a NULL buffer.", + ); + return; + } + None => had_buffer_before, + }; + + if has_buffer { + // The surface remains, or became mapped, track the last acked state. + pending.last_acked = Some(last_acked); + } else { + // The surface remains unmapped, meaning that it's in the initial configure stage. + pending.last_acked = None; + } + + // Lock surfaces aren't allowed to attach a null buffer, and therefore to unmap. + }); + } } /// State of an ext-session-lock surface. @@ -250,3 +383,21 @@ impl LockSurfaceConfigure { } } } + +/// Represents the client pending state +#[derive(Debug, Default, Copy, Clone)] +pub struct LockSurfaceCachedState { + /// Configure last acknowledged by the client at the time of the commit. + /// + /// Reset to `None` when the surface unmaps. + pub last_acked: Option, +} + +impl Cacheable for LockSurfaceCachedState { + fn commit(&mut self, _dh: &DisplayHandle) -> Self { + *self + } + fn merge_into(self, into: &mut Self, _dh: &DisplayHandle) { + *into = self; + } +} diff --git a/src/wayland/shell/wlr_layer/handlers.rs b/src/wayland/shell/wlr_layer/handlers.rs index d3de45852850..5eb3bc9c7291 100644 --- a/src/wayland/shell/wlr_layer/handlers.rs +++ b/src/wayland/shell/wlr_layer/handlers.rs @@ -122,49 +122,10 @@ where }); if initial { - compositor::add_pre_commit_hook::(&wl_surface, |_state, _dh, surface| { - compositor::with_states(surface, |states| { - let guard = states - .data_map - .get::>() - .unwrap() - .lock() - .unwrap(); - - let mut cached_guard = states.cached_state.get::(); - let pending = cached_guard.pending(); - - if pending.size.w == 0 && !pending.anchor.anchored_horizontally() { - guard.surface.post_error( - zwlr_layer_surface_v1::Error::InvalidSize, - "width 0 requested without setting left and right anchors", - ); - return; - } - - if pending.size.h == 0 && !pending.anchor.anchored_vertically() { - guard.surface.post_error( - zwlr_layer_surface_v1::Error::InvalidSize, - "height 0 requested without setting top and bottom anchors", - ); - } - }); - }); - - compositor::add_post_commit_hook::(&wl_surface, |_state, _dh, surface| { - compositor::with_states(surface, |states| { - let mut guard = states - .data_map - .get::>() - .unwrap() - .lock() - .unwrap(); - - if let Some(state) = guard.last_acked.clone() { - guard.current = state; - } - }); - }); + compositor::add_pre_commit_hook::( + &wl_surface, + super::LayerSurface::pre_commit_hook, + ); } let handle = super::LayerSurface { diff --git a/src/wayland/shell/wlr_layer/mod.rs b/src/wayland/shell/wlr_layer/mod.rs index 4dd819978a95..ba2aed561f4e 100644 --- a/src/wayland/shell/wlr_layer/mod.rs +++ b/src/wayland/shell/wlr_layer/mod.rs @@ -47,20 +47,20 @@ use std::sync::{Arc, Mutex}; +use tracing::{trace, trace_span}; use wayland_protocols_wlr::layer_shell::v1::server::{ - zwlr_layer_shell_v1::{self, ZwlrLayerShellV1}, - zwlr_layer_surface_v1, + zwlr_layer_shell_v1::ZwlrLayerShellV1, zwlr_layer_surface_v1, }; use wayland_server::{ backend::GlobalId, protocol::{wl_output::WlOutput, wl_surface}, - Client, DisplayHandle, GlobalDispatch, Resource, + Client, DisplayHandle, GlobalDispatch, Resource as _, }; use crate::{ utils::{alive_tracker::IsAlive, Logical, Serial, Size, SERIAL_COUNTER}, wayland::{ - compositor::{self, Cacheable}, + compositor::{self, BufferAssignment, Cacheable, SurfaceAttributes}, shell::xdg, }, }; @@ -74,7 +74,7 @@ pub use types::{Anchor, ExclusiveZone, KeyboardInteractivity, Layer, Margins}; /// The role of a wlr_layer_shell_surface pub const LAYER_SURFACE_ROLE: &str = "zwlr_layer_surface_v1"; -/// Data associated with XDG popup surface +/// Data associated with layer surface /// /// ```no_run /// use smithay::wayland::compositor; @@ -91,11 +91,6 @@ pub type LayerSurfaceData = Mutex; #[derive(Debug)] pub struct LayerSurfaceAttributes { surface: zwlr_layer_surface_v1::ZwlrLayerSurfaceV1, - /// Defines if the surface has received at least one - /// layer_surface.ack_configure from the client - pub configured: bool, - /// The serial of the last acked configure - pub configure_serial: Option, /// Holds the state if the surface has sent the initial /// configure event to the client. It is expected that /// during the first commit a initial @@ -109,26 +104,20 @@ pub struct LayerSurfaceAttributes { pending_configures: Vec, /// Holds the pending state as set by the server. pub server_pending: Option, - /// Holds the last server_pending state that has been acknowledged - /// by the client. This state should be cloned to the current - /// during a commit. - pub last_acked: Option, - /// Holds the current state of the layer after a successful - /// commit. - pub current: LayerSurfaceState, + /// Holds the last configure that has been acknowledged by the client. This state should be + /// cloned to the current during a commit. Note that this state can be newer than the last + /// acked state at the time of the last commit. + pub last_acked: Option, } impl LayerSurfaceAttributes { fn new(surface: zwlr_layer_surface_v1::ZwlrLayerSurfaceV1) -> Self { Self { surface, - configured: false, - configure_serial: None, initial_configure_sent: false, pending_configures: Vec::new(), server_pending: None, last_acked: None, - current: Default::default(), } } @@ -139,36 +128,32 @@ impl LayerSurfaceAttributes { .find(|configure| configure.serial == serial) .cloned()?; - self.last_acked = Some(configure.state.clone()); + self.last_acked = Some(configure.clone()); - self.configured = true; - self.configure_serial = Some(serial); self.pending_configures.retain(|c| c.serial > serial); Some(configure) } fn reset(&mut self) { - self.configured = false; - self.configure_serial = None; self.initial_configure_sent = false; self.pending_configures = Vec::new(); self.server_pending = None; self.last_acked = None; - self.current = Default::default(); } - fn current_server_state(&self) -> &LayerSurfaceState { + fn current_server_state(&self) -> LayerSurfaceState { self.pending_configures .last() .map(|c| &c.state) - .or(self.last_acked.as_ref()) - .unwrap_or(&self.current) + .or(self.last_acked.as_ref().map(|c| &c.state)) + .cloned() + .unwrap_or_default() } fn has_pending_changes(&self) -> bool { self.server_pending .as_ref() - .map(|s| s != self.current_server_state()) + .map(|s| *s != self.current_server_state()) .unwrap_or(false) } } @@ -181,7 +166,7 @@ pub struct LayerSurfaceState { } /// Represents the client pending state -#[derive(Debug, Default, Clone, Copy)] +#[derive(Debug, Default, Clone)] pub struct LayerSurfaceCachedState { /// The size requested by the client pub size: Size, @@ -195,11 +180,15 @@ pub struct LayerSurfaceCachedState { pub keyboard_interactivity: KeyboardInteractivity, /// The layer that the surface is rendered on pub layer: Layer, + /// Configure last acknowledged by the client at the time of the commit. + /// + /// Reset to `None` when the surface unmaps. + pub last_acked: Option, } impl Cacheable for LayerSurfaceCachedState { fn commit(&mut self, _dh: &DisplayHandle) -> Self { - *self + self.clone() } fn merge_into(self, into: &mut Self, _dh: &DisplayHandle) { *into = self; @@ -398,30 +387,6 @@ impl LayerSurface { serial } - /// Make sure this surface was configured - /// - /// Returns `true` if it was, if not, returns `false` and raise - /// a protocol error to the associated layer surface. Also returns `false` - /// if the surface is already destroyed. - pub fn ensure_configured(&self) -> bool { - let configured = compositor::with_states(&self.wl_surface, |states| { - states - .data_map - .get::>() - .unwrap() - .lock() - .unwrap() - .configured - }); - if !configured { - self.shell_surface.post_error( - zwlr_layer_shell_v1::Error::AlreadyConstructed, - "layer_surface has never been configured", - ); - } - configured - } - /// Send a "close" event to the client pub fn send_close(&self) { self.shell_surface.closed() @@ -479,23 +444,109 @@ impl LayerSurface { }) } - /// Gets a copy of the current state of this layer - /// - /// Returns `None` if the underlying surface has been - /// destroyed - pub fn current_state(&self) -> LayerSurfaceState { + /// Provides access to the current committed cached state. + pub fn with_cached_state(&self, f: F) -> T + where + F: FnOnce(&LayerSurfaceCachedState) -> T, + { compositor::with_states(&self.wl_surface, |states| { - let attributes = states - .data_map - .get::>() - .unwrap() - .lock() - .unwrap(); - - attributes.current.clone() + let mut guard = states.cached_state.get::(); + f(guard.current()) }) } + /// Provides access to the current committed state. + /// + /// This is the state that the client last acked before making the current commit. + pub fn with_committed_state(&self, f: F) -> T + where + F: FnOnce(Option<&LayerSurfaceState>) -> T, + { + self.with_cached_state(move |state| f(state.last_acked.as_ref().map(|c| &c.state))) + } + + /// Handles the role specific commit error checking + /// + /// This should be called when the underlying WlSurface + /// handles a wl_surface.commit request. + pub(crate) fn pre_commit_hook( + _state: &mut D, + _dh: &DisplayHandle, + surface: &wl_surface::WlSurface, + ) { + let _span = trace_span!("layer-surface pre-commit", surface = %surface.id()).entered(); + + compositor::with_states(surface, |states| { + let mut role = states.data_map.get::().unwrap().lock().unwrap(); + + let mut guard_layer = states.cached_state.get::(); + let pending = guard_layer.pending(); + + if pending.size.w == 0 && !pending.anchor.anchored_horizontally() { + role.surface.post_error( + zwlr_layer_surface_v1::Error::InvalidSize, + "width 0 requested without setting left and right anchors", + ); + return; + } + + if pending.size.h == 0 && !pending.anchor.anchored_vertically() { + role.surface.post_error( + zwlr_layer_surface_v1::Error::InvalidSize, + "height 0 requested without setting top and bottom anchors", + ); + return; + } + + // The presence of last_acked always follows the buffer assignment because the + // surface is not allowed to attach a buffer without acking the initial configure. + let had_buffer_before = pending.last_acked.is_some(); + + let mut guard_surface = states.cached_state.get::(); + let has_buffer = match &guard_surface.pending().buffer { + Some(BufferAssignment::NewBuffer(_)) => true, + Some(BufferAssignment::Removed) => false, + None => had_buffer_before, + }; + // Need to check had_buffer_before in case the client attaches a null buffer for the + // initial commit---we don't want to consider that as "got unmapped" and reset role. + // Reproducer: waybar. + let got_unmapped = had_buffer_before && !has_buffer; + + if has_buffer { + let Some(last_acked) = role.last_acked.clone() else { + role.surface.post_error( + zwlr_layer_surface_v1::Error::InvalidSurfaceState, + "must ack the initial configure before attaching buffer", + ); + return; + }; + + // The surface remains, or became mapped, track the last acked state. + pending.last_acked = Some(last_acked); + } else { + // The surface remains, or became, unmapped, meaning that it's in the initial + // configure stage. + pending.last_acked = None; + } + + if got_unmapped { + trace!( + "got unmapped; resetting role and cached state; have {} pending configures", + role.pending_configures.len() + ); + + // All attributes are discarded when an xdg_surface is unmapped. Though, we keep + // the list of pending configures because there's no way for a surface to tell + // an in-flight configure apart from our next initial configure after unmapping. + let pending_configures = std::mem::take(&mut role.pending_configures); + role.reset(); + role.pending_configures = pending_configures; + *guard_layer.pending() = Default::default(); + } + }); + } + /// Access the underlying `zwlr_layer_surface_v1` of this layer surface /// pub fn shell_surface(&self) -> &zwlr_layer_surface_v1::ZwlrLayerSurfaceV1 { diff --git a/src/wayland/shell/xdg/handlers/surface.rs b/src/wayland/shell/xdg/handlers/surface.rs index 3ae3a79e954b..61ffe1119043 100644 --- a/src/wayland/shell/xdg/handlers/surface.rs +++ b/src/wayland/shell/xdg/handlers/surface.rs @@ -107,23 +107,26 @@ where // Initialize the toplevel capabilities from the default capabilities let default_capabilities = &state.xdg_shell_state().default_capabilities; - let current_capabilties = &mut states + let mut attributes = states .data_map .get::>() .unwrap() .lock() - .unwrap() - .current - .capabilities; + .unwrap(); + if attributes.server_pending.is_none() { + let current = attributes.current_server_state(); + attributes.server_pending = Some(current); + } + let current_capabilties = &mut attributes.server_pending.as_mut().unwrap().capabilities; current_capabilties.replace(default_capabilities.capabilities.iter().copied()); initial }); if initial { - compositor::add_post_commit_hook::( + compositor::add_pre_commit_hook::( surface, - super::super::ToplevelSurface::commit_hook, + super::super::ToplevelSurface::pre_commit_hook, ); } @@ -203,10 +206,6 @@ where surface, super::super::PopupSurface::pre_commit_hook, ); - compositor::add_post_commit_hook::( - surface, - super::super::PopupSurface::post_commit_hook, - ); } let popup = data_init.init( diff --git a/src/wayland/shell/xdg/handlers/surface/popup.rs b/src/wayland/shell/xdg/handlers/surface/popup.rs index 227744c909f6..08b50c4e72b2 100644 --- a/src/wayland/shell/xdg/handlers/surface/popup.rs +++ b/src/wayland/shell/xdg/handlers/surface/popup.rs @@ -4,8 +4,8 @@ use crate::{ input::SeatHandler, utils::Serial, wayland::{ - compositor, - shell::xdg::{SurfaceCachedState, XdgPopupSurfaceData, XdgPositionerUserData}, + compositor::{self, with_states}, + shell::xdg::{PopupCachedState, SurfaceCachedState, XdgPopupSurfaceData, XdgPositionerUserData}, }, }; @@ -45,7 +45,15 @@ where let serial = Serial::from(serial); - XdgShellHandler::grab(state, handle, seat, serial); + with_states(handle.wl_surface(), |states| { + states + .data_map + .get::() + .unwrap() + .lock() + .unwrap() + .requested_grab = Some((seat, serial)); + }); } xdg_popup::Request::Reposition { positioner, token } => { let handle = crate::wayland::shell::xdg::PopupSurface { @@ -90,6 +98,10 @@ where let mut guard = states.cached_state.get::(); *guard.pending() = Default::default(); *guard.current() = Default::default(); + + let mut guard = states.cached_state.get::(); + *guard.pending() = Default::default(); + *guard.current() = Default::default(); }) } } diff --git a/src/wayland/shell/xdg/handlers/surface/toplevel.rs b/src/wayland/shell/xdg/handlers/surface/toplevel.rs index 17bd3b9b4d4d..c91f4991b1fb 100644 --- a/src/wayland/shell/xdg/handlers/surface/toplevel.rs +++ b/src/wayland/shell/xdg/handlers/surface/toplevel.rs @@ -4,7 +4,10 @@ use crate::{ utils::Serial, wayland::{ compositor, - shell::{is_valid_parent, xdg::XdgToplevelSurfaceData}, + shell::{ + is_valid_parent, + xdg::{ToplevelCachedState, XdgToplevelSurfaceData}, + }, }, }; @@ -182,6 +185,10 @@ where let mut guard = states.cached_state.get::(); *guard.pending() = Default::default(); *guard.current() = Default::default(); + + let mut guard = states.cached_state.get::(); + *guard.pending() = Default::default(); + *guard.current() = Default::default(); }) } } diff --git a/src/wayland/shell/xdg/mod.rs b/src/wayland/shell/xdg/mod.rs index d427d3060204..0341ad317498 100644 --- a/src/wayland/shell/xdg/mod.rs +++ b/src/wayland/shell/xdg/mod.rs @@ -121,11 +121,12 @@ use crate::utils::alive_tracker::IsAlive; use crate::utils::{user_data::UserDataMap, Logical, Point, Rectangle, Size}; use crate::utils::{Serial, SERIAL_COUNTER}; -use crate::wayland::compositor; use crate::wayland::compositor::Cacheable; +use crate::wayland::compositor::{self, BufferAssignment, SurfaceAttributes}; use std::cmp::min; use std::{collections::HashSet, fmt::Debug, sync::Mutex}; +use tracing::{error, trace, trace_span}; use wayland_protocols::xdg::decoration::zv1::server::zxdg_toplevel_decoration_v1; use wayland_protocols::xdg::shell::server::xdg_positioner::{Anchor, ConstraintAdjustment, Gravity}; use wayland_protocols::xdg::shell::server::xdg_surface; @@ -159,7 +160,8 @@ const XDG_TOPLEVEL_STATE_SUSPENDED_SINCE: u32 = 6; macro_rules! xdg_role { ($state:ty, $(#[$configure_meta:meta])* $configure_name:ident $({$($(#[$configure_field_meta:meta])* $configure_field_vis:vis$configure_field_name:ident:$configure_field_type:ty),*}),*, - $(#[$attributes_meta:meta])* $attributes_name:ident {$($(#[$attributes_field_meta:meta])* $attributes_field_vis:vis$attributes_field_name:ident:$attributes_field_type:ty),*}) => { + $(#[$attributes_meta:meta])* $attributes_name:ident {$($(#[$attributes_field_meta:meta])* $attributes_field_vis:vis$attributes_field_name:ident:$attributes_field_type:ty),*}, + $(#[$cached_state_meta:meta])* $cached_state_name:ident) => { $(#[$configure_meta])* pub struct $configure_name { @@ -180,11 +182,6 @@ macro_rules! xdg_role { $(#[$attributes_meta])* pub struct $attributes_name { - /// Defines if the surface has received at least one - /// xdg_surface.ack_configure from the client - pub configured: bool, - /// The serial of the last acked configure - pub configure_serial: Option, /// Holds the state if the surface has sent the initial /// configure event to the client. It is expected that /// during the first commit a initial @@ -198,17 +195,10 @@ macro_rules! xdg_role { pending_configures: Vec<$configure_name>, /// Holds the pending state as set by the server. pub server_pending: Option<$state>, - /// Holds the last server_pending state that has been acknowledged - /// by the client. This state should be cloned to the current - /// during a commit. - pub last_acked: Option<$state>, - /// Holds the current state after a successful commit. - pub current: $state, - /// Holds the last acked configure serial at the time of the last successful - /// commit. This serial corresponds to the current state. - pub current_serial: Option, - /// Does the surface have a buffer (updated on every commit) - has_buffer: bool, + /// Holds the last configure that has been acknowledged by the client. This state + /// should be cloned to the current during a commit. Note that this state can be + /// newer than the last acked state at the time of the last commit. + pub last_acked: Option<$configure_name>, $( $(#[$attributes_field_meta])* @@ -229,14 +219,8 @@ macro_rules! xdg_role { } }; - // Save the state as the last acked state - self.last_acked = Some(configure.state.clone()); - - // Set the xdg_surface to configured - self.configured = true; - - // Save the last configure serial as a reference - self.configure_serial = Some(Serial::from(serial)); + // Save the configure as the last acked configure + self.last_acked = Some(configure.clone()); // Clean old configures self.pending_configures.retain(|c| c.serial > serial); @@ -255,7 +239,7 @@ macro_rules! xdg_role { /// This can be used for example to check if the /// [`server_pending`](#structfield.server_pending) state /// is different from the last configured. - pub fn current_server_state(&self) -> &$state { + pub fn current_server_state(&self) -> $state { // We check if there is already a non-acked pending // configure and use its state or otherwise we could // loose some state that was previously configured @@ -263,17 +247,15 @@ macro_rules! xdg_role { // again. If there is no pending state we try to use the // last acked state which could contain state changes // already acked but not committed to the current state. - // In case no last acked state is available, which is - // the case on the first configure we fallback to the - // current state. // In both cases the state already contains all previous // sent states. This way all pending state is accumulated // into the current state. self.pending_configures .last() .map(|c| &c.state) - .or_else(|| self.last_acked.as_ref()) - .unwrap_or(&self.current) + .or_else(|| self.last_acked.as_ref().map(|c| &c.state)) + .cloned() + .unwrap_or_default() } /// Check if the state has pending changes that have @@ -283,7 +265,7 @@ macro_rules! xdg_role { /// state is [`Some`] in that it also checks if a current pending /// state is different from the [`current_server_state`](#method.current_server_state). pub fn has_pending_changes(&self) -> bool { - self.server_pending.as_ref().map(|s| s != self.current_server_state()).unwrap_or(false) + self.server_pending.as_ref().map(|s| *s != self.current_server_state()).unwrap_or(false) } /// Returns a list of configures sent to, but not yet acknowledged by the client. @@ -298,15 +280,10 @@ macro_rules! xdg_role { impl Default for $attributes_name { fn default() -> Self { Self { - configured: false, - configure_serial: None, pending_configures: Vec::new(), initial_configure_sent: false, server_pending: None, last_acked: None, - current: Default::default(), - current_serial: None, - has_buffer: false, $( $attributes_field_name: Default::default(), @@ -314,6 +291,23 @@ macro_rules! xdg_role { } } } + + $(#[$cached_state_meta])* + pub struct $cached_state_name { + /// Configure last acknowledged by the client at the time of the commit. + /// + /// Reset to `None` when the surface unmaps. + pub last_acked: Option<$configure_name>, + } + + impl Cacheable for $cached_state_name { + fn commit(&mut self, _dh: &DisplayHandle) -> Self { + self.clone() + } + fn merge_into(self, into: &mut Self, _dh: &DisplayHandle) { + *into = self; + } + } }; } @@ -375,7 +369,10 @@ xdg_role!( /// An `zxdg_toplevel_decoration_v1::configure` event has been sent /// to the client. pub initial_decoration_configure_sent: bool - } + }, + /// Represents the xdg_toplevel pending state + #[derive(Debug, Default, Clone)] + ToplevelCachedState ); /// Data associated with XDG toplevel surface @@ -441,15 +438,12 @@ xdg_role!( /// the xdg_popup role when no parent is set. pub parent: Option, - /// Defines if the surface has received at least one commit - /// - /// This can be used to check for protocol errors, like - /// checking if a popup requested a grab after it has been - /// mapped. - pub committed: bool, - - popup_handle: Option - } + /// If the popup requested a grab, this is the seat and serial. + requested_grab: Option<(wl_seat::WlSeat, Serial)> + }, + /// Represents the xdg_popup pending state + #[derive(Debug, Default, Clone)] + PopupCachedState ); /// Data associated with XDG popup surface @@ -1449,7 +1443,7 @@ impl ToplevelSurface { attributes .server_pending .take() - .unwrap_or_else(|| attributes.current_server_state().clone()), + .unwrap_or_else(|| attributes.current_server_state()), ); } @@ -1503,7 +1497,7 @@ impl ToplevelSurface { let pending = self .get_pending_state(&mut attributes) - .unwrap_or_else(|| attributes.current_server_state().clone()); + .unwrap_or_else(|| attributes.current_server_state()); // retrieve the current state before adding it to the // pending state so that we can compare what has changed let current = attributes.current_server_state(); @@ -1580,96 +1574,98 @@ impl ToplevelSurface { }) } - /// A newly-unmapped toplevel surface has to perform the initial commit-configure sequence as if it was - /// a new toplevel. - /// - /// This method is used to mark a surface for reinitialization. - /// - /// NOTE: If you are using smithay's rendering abstractions you don't have to call this manually - /// - /// Calls [`compositor::with_states`] internally. - pub fn reset_initial_configure_sent(&self) { - compositor::with_states(&self.wl_surface, |states| { - let mut data = states - .data_map - .get::() - .unwrap() - .lock() - .unwrap(); - data.initial_configure_sent = false; - data.initial_decoration_configure_sent = false; - }); - } - - /// Handles the role specific commit logic + /// Handles the role specific commit error checking /// /// This should be called when the underlying WlSurface /// handles a wl_surface.commit request. - pub(crate) fn commit_hook( - _state: &mut D, + pub(crate) fn pre_commit_hook( + state: &mut D, _dh: &DisplayHandle, surface: &wl_surface::WlSurface, ) { - let has_buffer = crate::backend::renderer::utils::with_renderer_surface_state(surface, |state| { - state.buffer().is_some() - }); + let _span = trace_span!("xdg-toplevel pre-commit", surface = %surface.id()).entered(); - compositor::with_states(surface, |states| { - let mut guard = states + let error = compositor::with_states(surface, |states| { + let mut role = states .data_map .get::() .unwrap() .lock() .unwrap(); - // This can be None if rendering utils are not used by the user - if let Some(has_buffer) = has_buffer { - // The surface was mapped in the past, and now got unmapped - if guard.has_buffer && !has_buffer { - // After xdg surface unmaps it has to perform the initial commit-configure sequence again - guard.initial_configure_sent = false; - guard.initial_decoration_configure_sent = false; - } + let mut guard_toplevel = states.cached_state.get::(); + let pending = guard_toplevel.pending(); - guard.has_buffer = has_buffer; + // The presence of last_acked always follows the buffer assignment because the + // surface is not allowed to attach a buffer without acking the initial configure. + let had_buffer_before = pending.last_acked.is_some(); + + let mut guard_surface = states.cached_state.get::(); + let has_buffer = match &guard_surface.pending().buffer { + Some(BufferAssignment::NewBuffer(_)) => true, + Some(BufferAssignment::Removed) => false, + None => had_buffer_before, + }; + // Need to check had_buffer_before in case the client attaches a null buffer for the + // initial commit---we don't want to consider that as "got unmapped" and reset role. + let got_unmapped = had_buffer_before && !has_buffer; + + if has_buffer { + let Some(last_acked) = role.last_acked.clone() else { + return Some(( + xdg_surface::Error::UnconfiguredBuffer, + "must ack the initial configure before attaching buffer", + )); + }; + + // The surface remains, or became mapped, track the last acked state. + pending.last_acked = Some(last_acked); + } else { + // The surface remains, or became, unmapped, meaning that it's in the initial + // configure stage. + pending.last_acked = None; } - if let Some(state) = guard.last_acked.clone() { - guard.current = state; - guard.current_serial = guard.configure_serial; + if got_unmapped { + trace!( + "got unmapped; resetting role and cached state; have {} pending configures", + role.pending_configures.len() + ); + + // All attributes are discarded when an xdg_surface is unmapped. Though, we keep + // the list of pending configures because there's no way for a surface to tell + // an in-flight configure apart from our next initial configure after unmapping. + // + // This can be reproduced with wleird-unmap if, say, and Activated configure is + // timed right. + let pending_configures = std::mem::take(&mut role.pending_configures); + *role = Default::default(); + role.pending_configures = pending_configures; + *guard_toplevel.pending() = Default::default(); + + // Don't forget to prepare the default capabilities though. + let default_capabilities = &state.xdg_shell_state().default_capabilities; + role.server_pending = Some(Default::default()); + let current_capabilties = &mut role.server_pending.as_mut().unwrap().capabilities; + current_capabilties.replace(default_capabilities.capabilities.iter().copied()); } - }); - } - /// Make sure this surface was configured - /// - /// Returns `true` if it was, if not, returns `false` and raise - /// a protocol error to the associated client. Also returns `false` - /// if the surface is already destroyed. - /// - /// `xdg_shell` mandates that a client acks a configure before committing - /// anything. - pub fn ensure_configured(&self) -> bool { - let configured = compositor::with_states(&self.wl_surface, |states| { - states - .data_map - .get::() - .unwrap() - .lock() - .unwrap() - .configured + None }); - if !configured { - let data = self - .shell_surface - .data::() - .unwrap(); - data.xdg_surface.post_error( - xdg_surface::Error::NotConstructed, - "Surface has not been configured yet.", - ); + + if let Some((error, msg)) = error { + // The surface attached a buffer without acking the initial configure. + let toplevels = &state.xdg_shell_state().known_toplevels; + if let Some(toplevel) = toplevels.iter().find(|handle| handle.wl_surface == *surface) { + let data = toplevel + .shell_surface + .data::() + .unwrap(); + data.xdg_surface.post_error(error, msg); + } else { + error!("surface missing from known toplevels"); + } } - configured } /// Send a "close" event to the client @@ -1707,7 +1703,7 @@ impl ToplevelSurface { .lock() .unwrap(); if attributes.server_pending.is_none() { - attributes.server_pending = Some(attributes.current_server_state().clone()); + attributes.server_pending = Some(attributes.current_server_state()); } let server_pending = attributes.server_pending.as_mut().unwrap(); @@ -1732,20 +1728,27 @@ impl ToplevelSurface { }) } - /// Gets a copy of the current state of this toplevel - pub fn current_state(&self) -> ToplevelState { + /// Provides access to the current committed cached state. + pub fn with_cached_state(&self, f: F) -> T + where + F: FnOnce(&ToplevelCachedState) -> T, + { compositor::with_states(&self.wl_surface, |states| { - let attributes = states - .data_map - .get::() - .unwrap() - .lock() - .unwrap(); - - attributes.current.clone() + let mut guard = states.cached_state.get::(); + f(guard.current()) }) } + /// Provides access to the current committed state. + /// + /// This is the state that the client last acked before making the current commit. + pub fn with_committed_state(&self, f: F) -> T + where + F: FnOnce(Option<&ToplevelState>) -> T, + { + self.with_cached_state(move |state| f(state.last_acked.as_ref().map(|c| &c.state))) + } + /// Returns the parent of this toplevel surface. pub fn parent(&self) -> Option { handlers::get_parent(&self.shell_surface) @@ -1839,7 +1842,7 @@ impl PopupSurface { let pending = attributes .server_pending .take() - .unwrap_or_else(|| *attributes.current_server_state()); + .unwrap_or_else(|| attributes.current_server_state()); let configure = PopupConfigure { state: pending, @@ -1902,7 +1905,12 @@ impl PopupSurface { return Err(PopupConfigureError::AlreadyConfigured); } - let is_reactive = attributes.current.positioner.reactive; + let mut cached = states.cached_state.get::(); + let is_reactive = cached + .current() + .last_acked + .as_ref() + .is_some_and(|c| c.state.positioner.reactive); if attributes.initial_configure_sent && !is_reactive { // Return error, the positioner does not allow re-configure @@ -1933,26 +1941,6 @@ impl PopupSurface { }) } - /// A newly-unmapped popup surface has to perform the initial commit-configure sequence as if it was - /// a new popup. - /// - /// This method is used to mark a surface for reinitialization. - /// - /// NOTE: If you are using smithay's rendering abstractions you don't have to call this manually - /// - /// Calls [`compositor::with_states`] internally. - pub fn reset_initial_configure_sent(&self) { - compositor::with_states(&self.wl_surface, |states| { - let mut data = states - .data_map - .get::() - .unwrap() - .lock() - .unwrap(); - data.initial_configure_sent = false; - }); - } - /// Send a configure event, including the `repositioned` event to the client /// in response to a `reposition` request. /// @@ -1965,104 +1953,115 @@ impl PopupSurface { /// /// This should be called when the underlying WlSurface /// handles a wl_surface.commit request. - pub(crate) fn pre_commit_hook( - _state: &mut D, + pub(crate) fn pre_commit_hook( + state: &mut D, _dh: &DisplayHandle, surface: &wl_surface::WlSurface, ) { - let send_error_to = compositor::with_states(surface, |states| { - let attributes = states + let _span = trace_span!("xdg-popup pre-commit", surface = %surface.id()).entered(); + + let res = compositor::with_states(surface, |states| { + let mut role = states .data_map .get::() .unwrap() .lock() .unwrap(); - if attributes.parent.is_none() { - attributes.popup_handle.clone() - } else { - None + if role.parent.is_none() { + return Err(( + xdg_surface::Error::NotConstructed, + "xdg_popup must have parent before mapping", + )); } - }); - if let Some(handle) = send_error_to { - let data = handle.data::().unwrap(); - data.xdg_surface.post_error( - xdg_surface::Error::NotConstructed, - "Surface has not been configured yet.", - ); - } - } - /// Handles the role specific commit state application - /// - /// This should be called when the underlying WlSurface - /// applies a wl_surface.commit state. - pub(crate) fn post_commit_hook( - _state: &mut D, - _dh: &DisplayHandle, - surface: &wl_surface::WlSurface, - ) { - let has_buffer = crate::backend::renderer::utils::with_renderer_surface_state(surface, |state| { - state.buffer().is_some() - }); + let mut guard_popup = states.cached_state.get::(); + let pending = guard_popup.pending(); - compositor::with_states(surface, |states| { - let mut attributes = states - .data_map - .get::() - .unwrap() - .lock() - .unwrap(); - attributes.committed = true; - - // This can be None if rendering utils are not used by the user - if let Some(has_buffer) = has_buffer { - // The surface was mapped in the past, and now got unmapped - if attributes.has_buffer && !has_buffer { - // After xdg surface unmaps it has to perform the initial commit-configure sequence again - attributes.initial_configure_sent = false; + // The presence of last_acked always follows the buffer assignment because the + // surface is not allowed to attach a buffer without acking the initial configure. + let had_buffer_before = pending.last_acked.is_some(); + + let grab = role.requested_grab.take(); + if grab.is_some() && had_buffer_before { + let popups = &state.xdg_shell_state().known_popups; + if let Some(popup) = popups.iter().find(|handle| handle.wl_surface == *surface) { + popup + .shell_surface + .post_error(xdg_popup::Error::InvalidGrab, "tried to grab after being mapped"); + } else { + error!("surface missing from known popups"); } + return Ok(None); + } - attributes.has_buffer = has_buffer; + let mut guard_surface = states.cached_state.get::(); + let has_buffer = match &guard_surface.pending().buffer { + Some(BufferAssignment::NewBuffer(_)) => true, + Some(BufferAssignment::Removed) => false, + None => had_buffer_before, + }; + // Need to check had_buffer_before in case the client attaches a null buffer for the + // initial commit---we don't want to consider that as "got unmapped" and reset role. + let got_unmapped = had_buffer_before && !has_buffer; + + if has_buffer { + let Some(last_acked) = role.last_acked else { + return Err(( + xdg_surface::Error::UnconfiguredBuffer, + "must ack the initial configure before attaching buffer", + )); + }; + + // The surface remains, or became mapped, track the last acked state. + pending.last_acked = Some(last_acked); + } else { + // The surface remains, or became, unmapped, meaning that it's in the initial + // configure stage. + pending.last_acked = None; } - if attributes.initial_configure_sent { - if let Some(state) = attributes.last_acked { - attributes.current = state; - attributes.current_serial = attributes.configure_serial; - } + if got_unmapped { + trace!( + "got unmapped; resetting role and cached state; have {} pending configures", + role.pending_configures.len() + ); + + // All attributes are discarded when an xdg_surface is unmapped. Though, we keep + // the list of pending configures because there's no way for a surface to tell + // an in-flight configure apart from our next initial configure after unmapping. + let pending_configures = std::mem::take(&mut role.pending_configures); + *role = Default::default(); + role.pending_configures = pending_configures; + *guard_popup.pending() = Default::default(); } - }); - } - /// Make sure this surface was configured - /// - /// Returns `true` if it was, if not, returns `false` and raise - /// a protocol error to the associated client. Also returns `false` - /// if the surface is already destroyed. - /// - /// xdg_shell mandates that a client acks a configure before committing - /// anything. - pub fn ensure_configured(&self) -> bool { - let configured = compositor::with_states(&self.wl_surface, |states| { - states - .data_map - .get::() - .unwrap() - .lock() - .unwrap() - .configured + Ok(grab) }); - if !configured { - let data = self - .shell_surface - .data::() - .unwrap(); - data.xdg_surface.post_error( - xdg_surface::Error::NotConstructed, - "Surface has not been configured yet.", - ); + + match res { + Ok(Some((seat, serial))) => { + let popups = &state.xdg_shell_state().known_popups; + if let Some(popup) = popups.iter().find(|handle| handle.wl_surface == *surface) { + let popup = popup.clone(); + XdgShellHandler::grab(state, popup, seat, serial); + } else { + error!("surface missing from known popups"); + } + } + Ok(None) => (), + Err((error, msg)) => { + let popups = &state.xdg_shell_state().known_popups; + if let Some(popup) = popups.iter().find(|handle| handle.wl_surface == *surface) { + let data = popup + .shell_surface + .data::() + .unwrap(); + data.xdg_surface.post_error(error, msg); + } else { + error!("surface missing from known popups"); + } + } } - configured } /// Send a `popup_done` event to the popup surface @@ -2103,7 +2102,7 @@ impl PopupSurface { .lock() .unwrap(); if attributes.server_pending.is_none() { - attributes.server_pending = Some(*attributes.current_server_state()); + attributes.server_pending = Some(attributes.current_server_state()); } let server_pending = attributes.server_pending.as_mut().unwrap(); @@ -2127,6 +2126,27 @@ impl PopupSurface { !attributes.initial_configure_sent || attributes.has_pending_changes() }) } + + /// Provides access to the current committed cached state. + pub fn with_cached_state(&self, f: F) -> T + where + F: FnOnce(&PopupCachedState) -> T, + { + compositor::with_states(&self.wl_surface, |states| { + let mut guard = states.cached_state.get::(); + f(guard.current()) + }) + } + + /// Provides access to the current committed state. + /// + /// This is the state that the client last acked before making the current commit. + pub fn with_committed_state(&self, f: F) -> T + where + F: FnOnce(Option<&PopupState>) -> T, + { + self.with_cached_state(move |state| f(state.last_acked.as_ref().map(|c| &c.state))) + } } /// Defines the possible configure variants