Merge pull request #361 from cmeissl/feature/xdg_v3

support for xdg-shell v3

CHANGELOG.md

@@ -2,6 +2,13 @@
 
 ## Unreleased
 
+### Breaking Changes
+
+#### Clients & Protocols
+
+- `XdgPositionerState` moved to `XdgPopupState` and added to `XdgRequest::NewPopup`
+- `PopupSurface::send_configure` now checks the protocol version and returns an `Result`
+
 ### Additions
 
 #### Clients & Protocols
@@ -13,6 +20,7 @@
 - Whether a surface is toplevel equivalent can be determined with the new function `shell::is_toplevel_equivalent`.
 - Setting the parent of a toplevel surface is now possible with the `xdg::ToplevelSurface::set_parent` function.
 - Add support for the zxdg-foreign-v2 protocol.
+- Support for `xdg_wm_base` protocol version 3
 
 ### Bugfixes
 

anvil/src/shell.rs

@@ -374,12 +374,42 @@ pub fn init_shell<BackendData: 'static>(display: Rc<RefCell<Display>>, log: ::sl
                     .borrow_mut()
                     .insert(SurfaceKind::Xdg(surface), (x, y).into());
             }
-            XdgRequest::NewPopup { surface } => {
+            XdgRequest::NewPopup { surface, positioner } => {
                 // Do not send a configure here, the initial configure
                 // of a xdg_surface has to be sent during the commit if
                 // the surface is not already configured
+
+                // TODO: properly recompute the geometry with the whole of positioner state
+                surface
+                    .with_pending_state(|state| {
+                        // NOTE: This is not really necessary as the default geometry
+                        // is already set the same way, but for demonstrating how
+                        // to set the initial popup geometry this code is left as
+                        // an example
+                        state.geometry = positioner.get_geometry();
+                    })
+                    .unwrap();
                 xdg_window_map.borrow_mut().insert_popup(PopupKind::Xdg(surface));
             }
+            XdgRequest::RePosition {
+                surface,
+                positioner,
+                token,
+            } => {
+                let result = surface.with_pending_state(|state| {
+                    // NOTE: This is again a simplification, a proper compositor would
+                    // calculate the geometry of the popup here. For simplicity we just
+                    // use the default implementation here that does not take the
+                    // window position and output constraints into account.
+                    let geometry = positioner.get_geometry();
+                    state.geometry = geometry;
+                    state.positioner = positioner;
+                });
+
+                if result.is_ok() {
+                    surface.send_repositioned(token);
+                }
+            }
             XdgRequest::Move {
                 surface,
                 seat,
@@ -1089,8 +1119,9 @@ fn surface_commit(
         })
         .unwrap();
         if !initial_configure_sent {
-            // TODO: properly recompute the geometry with the whole of positioner state
+            // NOTE: This should never fail as the initial configure is always
-            popup.send_configure();
+            // allowed.
+            popup.send_configure().expect("initial configure failed");
         }
     }
 

src/wayland/shell/xdg/mod.rs

@@ -125,6 +125,9 @@ pub const ZXDG_TOPLEVEL_ROLE: &str = "zxdg_toplevel";
 /// [xdg_popup]: self::XDG_POPUP_ROLE
 pub const ZXDG_POPUP_ROLE: &str = "zxdg_popup";
 
+/// Constant for toplevel state version checking
+const XDG_TOPLEVEL_STATE_TILED_SINCE: u32 = 2;
+
 macro_rules! xdg_role {
     ($configure:ty,
      $(#[$attr:meta])* $element:ident {$($(#[$field_attr:meta])* $vis:vis$field:ident:$type:ty),*},
@@ -315,12 +318,6 @@ xdg_role!(
         /// It is a protocol error to call commit on a wl_surface with
         /// the xdg_popup role when no parent is set.
         pub parent: Option<wl_surface::WlSurface>,
-        /// The positioner state can be used by the compositor
-        /// to calculate the best placement for the popup.
-        ///
-        /// For example the compositor should prevent that a popup
-        /// is placed outside the visible rectangle of a output.
-        pub positioner: PositionerState,
         /// Holds the last server_pending state that has been acknowledged
         /// by the client. This state should be cloned to the current
         /// during a commit.
@@ -340,6 +337,12 @@ xdg_role!(
 /// Represents the state of the popup
 #[derive(Debug, Clone, Copy, PartialEq)]
 pub struct PopupState {
+    /// The positioner state can be used by the compositor
+    /// to calculate the best placement for the popup.
+    ///
+    /// For example the compositor should prevent that a popup
+    /// is placed outside the visible rectangle of a output.
+    pub positioner: PositionerState,
     /// Holds the geometry of the popup as defined by the positioner.
     ///
     /// `Rectangle::width` and `Rectangle::height` holds the size of the
@@ -356,11 +359,12 @@ impl Default for PopupState {
     fn default() -> Self {
         Self {
             geometry: Default::default(),
+            positioner: Default::default(),
         }
     }
 }
 
-#[derive(Clone, Debug)]
+#[derive(Clone, Copy, Debug, PartialEq)]
 /// The state of a positioner, as set by the client
 pub struct PositionerState {
     /// Size of the rectangle that needs to be positioned
@@ -378,6 +382,26 @@ pub struct PositionerState {
     pub constraint_adjustment: xdg_positioner::ConstraintAdjustment,
     /// Offset placement relative to the anchor point
     pub offset: Point<i32, Logical>,
+    /// When set reactive, the surface is reconstrained if the conditions
+    /// used for constraining changed, e.g. the parent window moved.
+    ///
+    /// If the conditions changed and the popup was reconstrained,
+    /// an xdg_popup.configure event is sent with updated geometry,
+    /// followed by an xdg_surface.configure event.
+    pub reactive: bool,
+    /// The parent window geometry the compositor should use when
+    /// positioning the popup. The compositor may use this information
+    /// to determine the future state the popup should be constrained using.
+    /// If this doesn't match the dimension of the parent the popup is
+    /// eventually positioned against, the behavior is undefined.
+    ///
+    /// The arguments are given in the surface-local coordinate space.
+    pub parent_size: Option<Size<i32, Logical>>,
+    /// The serial of an xdg_surface.configure event this positioner will
+    /// be used in response to. The compositor may use this information
+    /// together with set_parent_size to determine what future state the
+    /// popup should be constrained using.
+    pub parent_configure: Option<Serial>,
 }
 
 impl Default for PositionerState {
@@ -389,6 +413,9 @@ impl Default for PositionerState {
             gravity: xdg_positioner::Gravity::None,
             offset: Default::default(),
             rect_size: Default::default(),
+            reactive: false,
+            parent_size: None,
+            parent_configure: None,
         }
     }
 }
@@ -458,7 +485,7 @@ impl PositionerState {
     /// The `constraint_adjustment` will not be considered by this
     /// implementation and the position and size should be re-calculated
     /// in the compositor if the compositor implements `constraint_adjustment`
-    pub(crate) fn get_geometry(&self) -> Rectangle<i32, Logical> {
+    pub fn get_geometry(&self) -> Rectangle<i32, Logical> {
         // From the `xdg_shell` prococol specification:
         //
         // set_offset:
@@ -591,6 +618,39 @@ impl ToplevelStateSet {
             true
         }
     }
+
+    /// Filter the states according to the provided version
+    /// of the [`XdgToplevel`]
+    pub(crate) fn into_filtered_states(self, version: u32) -> Vec<xdg_toplevel::State> {
+        // If the client version supports the tiled states
+        // we can directly return the states which will save
+        // us from allocating another vector
+        if version >= XDG_TOPLEVEL_STATE_TILED_SINCE {
+            return self.states;
+        }
+
+        let is_tiled = |state: &xdg_toplevel::State| {
+            matches!(
+                state,
+                xdg_toplevel::State::TiledTop
+                    | xdg_toplevel::State::TiledBottom
+                    | xdg_toplevel::State::TiledLeft
+                    | xdg_toplevel::State::TiledRight
+            )
+        };
+
+        let contains_tiled = self.states.iter().any(|state| is_tiled(state));
+
+        // If the states do not contain a tiled state
+        // we can directly return the states which will save
+        // us from allocating another vector
+        if !contains_tiled {
+            return self.states;
+        }
+
+        // We need to filter out the unsupported states
+        self.states.into_iter().filter(|state| !is_tiled(state)).collect()
+    }
 }
 
 impl Default for ToplevelStateSet {
@@ -700,7 +760,7 @@ where
     let shell_data_z = shell_data.clone();
 
     let xdg_shell_global = display.create_global(
-        1,
+        3,
         Filter::new(move |(shell, _version), _, dispatch_data| {
             self::xdg_handlers::implement_wm_base(shell, &shell_data, dispatch_data);
         }),
@@ -1187,6 +1247,21 @@ impl ToplevelSurface {
     }
 }
 
+/// Represents the possible errors that
+/// can be returned from [`PopupSurface::send_configure`]
+#[derive(Debug, thiserror::Error)]
+pub enum PopupConfigureError {
+    /// The popup has already been configured and the
+    /// protocol version forbids the popup to
+    /// be re-configured
+    #[error("The popup has already been configured")]
+    AlreadyConfigured,
+    /// The popup is not allowed to be re-configured,
+    /// the positioner is not reactive
+    #[error("The popup positioner is not reactive")]
+    NotReactive,
+}
+
 #[derive(Debug, Clone)]
 pub(crate) enum PopupKind {
     Xdg(xdg_popup::XdgPopup),
@@ -1269,13 +1344,17 @@ impl PopupSurface {
         Some(ShellClient { kind: shell })
     }
 
-    /// Send a configure event to this popup surface to suggest it a new configuration
+    /// Get the version of the popup resource
-    ///
+    fn version(&self) -> u32 {
-    /// The serial of this configure will be tracked waiting for the client to ACK it.
+        match self.shell_surface {
-    ///
+            PopupKind::Xdg(ref xdg) => xdg.as_ref().version(),
-    /// You can manipulate the state that will be sent to the client with the [`with_pending_state`](#method.with_pending_state)
+            PopupKind::ZxdgV6(ref zxdg) => zxdg.as_ref().version(),
-    /// method.
+        }
-    pub fn send_configure(&self) {
+    }
+
+    /// Internal configure function to re-use the configure
+    /// logic for both [`XdgRequest::send_configure`] and [`XdgRequest::send_repositioned`]
+    fn send_configure_internal(&self, reposition_token: Option<u32>) {
         if let Some(surface) = self.get_surface() {
             let next_configure = compositor::with_states(surface, |states| {
                 let mut attributes = states
@@ -1284,12 +1363,17 @@ impl PopupSurface {
                     .unwrap()
                     .lock()
                     .unwrap();
-                if !attributes.initial_configure_sent || attributes.server_pending.is_some() {
+
+                if !attributes.initial_configure_sent
+                    || attributes.server_pending.is_some()
+                    || reposition_token.is_some()
+                {
                     let pending = attributes.server_pending.take().unwrap_or(attributes.current);
 
                     let configure = PopupConfigure {
                         state: pending,
                         serial: SERIAL_COUNTER.next_serial(),
+                        reposition_token,
                     };
 
                     attributes.pending_configures.push(configure);
@@ -1314,6 +1398,58 @@ impl PopupSurface {
         }
     }
 
+    /// Send a configure event to this popup surface to suggest it a new configuration
+    ///
+    /// The serial of this configure will be tracked waiting for the client to ACK it.
+    ///
+    /// You can manipulate the state that will be sent to the client with the [`with_pending_state`](#method.with_pending_state)
+    /// method.
+    ///
+    /// Returns [`Err(PopupConfigureError)`] if the initial configure has already been sent and
+    /// the client protocol version disallows a re-configure or the current [`PositionerState`]
+    /// is not reactive
+    pub fn send_configure(&self) -> Result<(), PopupConfigureError> {
+        if let Some(surface) = self.get_surface() {
+            // Check if we are allowed to send a configure
+            compositor::with_states(surface, |states| {
+                let attributes = states
+                    .data_map
+                    .get::<Mutex<XdgPopupSurfaceRoleAttributes>>()
+                    .unwrap()
+                    .lock()
+                    .unwrap();
+
+                if attributes.initial_configure_sent && self.version() < xdg_popup::EVT_REPOSITIONED_SINCE {
+                    // Return error, initial configure already sent and client
+                    // does not support re-configure
+                    return Err(PopupConfigureError::AlreadyConfigured);
+                }
+
+                let is_reactive = attributes.current.positioner.reactive;
+
+                if attributes.initial_configure_sent && !is_reactive {
+                    // Return error, the positioner does not allow re-configure
+                    return Err(PopupConfigureError::NotReactive);
+                }
+
+                Ok(())
+            })
+            .unwrap_or(Ok(()))?;
+
+            self.send_configure_internal(None);
+        }
+
+        Ok(())
+    }
+
+    /// Send a configure event, including the `repositioned` event to the client
+    /// in response to a `reposition` request.
+    ///
+    /// For further information see [`XdgPopup::send_configure`]
+    pub fn send_repositioned(&self, token: u32) {
+        self.send_configure_internal(Some(token))
+    }
+
     /// Handles the role specific commit logic
     ///
     /// This should be called when the underlying WlSurface
@@ -1436,6 +1572,10 @@ impl PopupSurface {
     /// It means that the use has dismissed the popup surface, or that
     /// the pointer has left the area of popup grab if there was a grab.
     pub fn send_popup_done(&self) {
+        if !self.alive() {
+            return;
+        }
+
         match self.shell_surface {
             PopupKind::Xdg(ref p) => p.popup_done(),
             PopupKind::ZxdgV6(ref p) => p.popup_done(),
@@ -1511,6 +1651,10 @@ pub struct PopupConfigure {
     /// from a client for a serial will validate all pending lower
     /// serials.
     pub serial: Serial,
+    /// The token the client provided in the `xdg_popup::reposition`
+    /// request. The token itself is opaque, and has no other special meaning.
+    /// The token is sent in the corresponding `xdg_popup::repositioned` event.
+    pub reposition_token: Option<u32>,
 }
 
 /// Defines the possible configure variants
@@ -1574,6 +1718,15 @@ pub enum XdgRequest {
     NewPopup {
         /// the surface
         surface: PopupSurface,
+        /// The state of the positioner at the time
+        /// the popup was requested.
+        ///
+        /// The positioner state can be used by the compositor
+        /// to calculate the best placement for the popup.
+        ///
+        /// For example the compositor should prevent that a popup
+        /// is placed outside the visible rectangle of a output.
+        positioner: PositionerState,
     },
     /// The client requested the start of an interactive move for this surface
     Move {
@@ -1655,4 +1808,24 @@ pub enum XdgRequest {
         /// The configure serial.
         configure: Configure,
     },
+    /// A client requested a reposition, providing a new
+    /// positioner, of a popup.
+    RePosition {
+        /// The popup for which a reposition has been requested
+        surface: PopupSurface,
+        /// The state of the positioner at the time
+        /// the reposition request was made.
+        ///
+        /// The positioner state can be used by the compositor
+        /// to calculate the best placement for the popup.
+        ///
+        /// For example the compositor should prevent that a popup
+        /// is placed outside the visible rectangle of a output.
+        positioner: PositionerState,
+        /// The passed token will be sent in the corresponding xdg_popup.repositioned event.
+        /// The new popup position will not take effect until the corresponding configure event
+        /// is acknowledged by the client. See xdg_popup.repositioned for details.
+        /// The token itself is opaque, and has no other special meaning.
+        token: u32,
+    },
 }

src/wayland/shell/xdg/xdg_handlers.rs

@@ -158,6 +158,18 @@ fn implement_positioner(positioner: Main<xdg_positioner::XdgPositioner>) -> xdg_
             xdg_positioner::Request::SetOffset { x, y } => {
                 state.offset = (x, y).into();
             }
+            xdg_positioner::Request::SetReactive => {
+                state.reactive = true;
+            }
+            xdg_positioner::Request::SetParentSize {
+                parent_width,
+                parent_height,
+            } => {
+                state.parent_size = Some((parent_width, parent_height).into());
+            }
+            xdg_positioner::Request::SetParentConfigure { serial } => {
+                state.parent_configure = Some(Serial::from(serial));
+            }
             _ => unreachable!(),
         }
     });
@@ -265,13 +277,12 @@ fn xdg_surface_implementation(
             parent,
             positioner,
         } => {
-            let positioner_data = positioner
+            let positioner_data = *positioner
                 .as_ref()
                 .user_data()
                 .get::<RefCell<PositionerState>>()
                 .unwrap()
-                .borrow()
+                .borrow();
-                .clone();
 
             let parent_surface = parent.map(|parent| {
                 let parent_data = parent.as_ref().user_data().get::<XdgSurfaceUserData>().unwrap();
@@ -287,6 +298,7 @@ fn xdg_surface_implementation(
                 server_pending: Some(PopupState {
                     // Set the positioner data as the popup geometry
                     geometry: positioner_data.get_geometry(),
+                    positioner: positioner_data,
                 }),
                 ..Default::default()
             };
@@ -333,7 +345,13 @@ fn xdg_surface_implementation(
 
             let handle = make_popup_handle(&id);
             let mut user_impl = data.shell_data.user_impl.borrow_mut();
-            (&mut *user_impl)(XdgRequest::NewPopup { surface: handle }, dispatch_data);
+            (&mut *user_impl)(
+                XdgRequest::NewPopup {
+                    surface: handle,
+                    positioner: positioner_data,
+                },
+                dispatch_data,
+            );
         }
         xdg_surface::Request::SetWindowGeometry { x, y, width, height } => {
             // Check the role of the surface, this can be either xdg_toplevel
@@ -501,7 +519,10 @@ pub fn send_toplevel_configure(resource: &xdg_toplevel::XdgToplevel, configure:
     let (width, height) = configure.state.size.unwrap_or_default().into();
     // convert the Vec<State> (which is really a Vec<u32>) into Vec<u8>
     let states = {
-        let mut states: Vec<xdg_toplevel::State> = configure.state.states.into();
+        let mut states: Vec<xdg_toplevel::State> = configure
+            .state
+            .states
+            .into_filtered_states(resource.as_ref().version());
         let ptr = states.as_mut_ptr();
         let len = states.len();
         let cap = states.capacity();
@@ -693,6 +714,11 @@ pub(crate) fn send_popup_configure(resource: &xdg_popup::XdgPopup, configure: Po
     let serial = configure.serial;
     let geometry = configure.state.geometry;
 
+    // Send repositioned if token is set
+    if let Some(token) = configure.reposition_token {
+        resource.repositioned(token);
+    }
+
     // Send the popup configure
     resource.configure(geometry.loc.x, geometry.loc.y, geometry.size.w, geometry.size.h);
 
@@ -736,6 +762,26 @@ fn xdg_popup_implementation(
                 dispatch_data,
             );
         }
+        xdg_popup::Request::Reposition { positioner, token } => {
+            let handle = make_popup_handle(&popup);
+            let mut user_impl = data.shell_data.user_impl.borrow_mut();
+
+            let positioner_data = *positioner
+                .as_ref()
+                .user_data()
+                .get::<RefCell<PositionerState>>()
+                .unwrap()
+                .borrow();
+
+            (&mut *user_impl)(
+                XdgRequest::RePosition {
+                    surface: handle,
+                    positioner: positioner_data,
+                    token,
+                },
+                dispatch_data,
+            );
+        }
         _ => unreachable!(),
     }
 }

src/wayland/shell/xdg/zxdgv6_handlers.rs

@@ -279,13 +279,12 @@ fn xdg_surface_implementation(
             parent,
             positioner,
         } => {
-            let positioner_data = positioner
+            let positioner_data = *positioner
                 .as_ref()
                 .user_data()
                 .get::<RefCell<PositionerState>>()
                 .unwrap()
-                .borrow()
+                .borrow();
-                .clone();
 
             let parent_surface = {
                 let parent_data = parent.as_ref().user_data().get::<XdgSurfaceUserData>().unwrap();
@@ -301,6 +300,7 @@ fn xdg_surface_implementation(
                 server_pending: Some(PopupState {
                     // Set the positioner data as the popup geometry
                     geometry: positioner_data.get_geometry(),
+                    positioner: positioner_data,
                 }),
                 ..Default::default()
             };
@@ -347,7 +347,13 @@ fn xdg_surface_implementation(
 
             let handle = make_popup_handle(&id);
             let mut user_impl = data.shell_data.user_impl.borrow_mut();
-            (&mut *user_impl)(XdgRequest::NewPopup { surface: handle }, dispatch_data);
+            (&mut *user_impl)(
+                XdgRequest::NewPopup {
+                    surface: handle,
+                    positioner: positioner_data,
+                },
+                dispatch_data,
+            );
         }
         zxdg_surface_v6::Request::SetWindowGeometry { x, y, width, height } => {
             // Check the role of the surface, this can be either xdg_toplevel
@@ -516,7 +522,10 @@ pub fn send_toplevel_configure(resource: &zxdg_toplevel_v6::ZxdgToplevelV6, conf
     let (width, height) = configure.state.size.unwrap_or_default().into();
     // convert the Vec<State> (which is really a Vec<u32>) into Vec<u8>
     let states = {
-        let mut states: Vec<xdg_toplevel::State> = configure.state.states.into();
+        let mut states: Vec<xdg_toplevel::State> = configure
+            .state
+            .states
+            .into_filtered_states(resource.as_ref().version());
         let ptr = states.as_mut_ptr();
         let len = states.len();
         let cap = states.capacity();

wlcs_anvil/src/lib.rs

@@ -30,7 +30,7 @@ static SUPPORTED_EXTENSIONS: &[WlcsExtensionDescriptor] = extension_list!(
     ("wl_shell", 1),
     ("wl_seat", 5),
     ("wl_output", 3),
-    ("xdg_wm_base", 1),
+    ("xdg_wm_base", 3),
 );
 
 static DESCRIPTOR: WlcsIntegrationDescriptor = WlcsIntegrationDescriptor {