wayland: allow to always send a configure event

this changes send_configure for ToplevelSurface,
PopupSurface and LayerSurface to always generate
a configure event. The old behavior can be found
in send_pending_configure. This is needed as
set_fullscreen and set_maximized always expect
a configure event regardless if the request was
fulfilled or not.

CHANGELOG.md

@@ -21,6 +21,8 @@
 - The `slot` method on touch events no longer returns an `Option` and multi-touch capability is thus opaque to the compositor
 - `wayland::output::Output` now is created separately from it's `Global` as reflected by [`Output::new`] and the new [`Output::create_global] method.
 - `PointerHandle` no longer sends an implicit motion event when a grab is set, `time` has been replaced by an explicit `focus` parameter in [`PointerHandle::set_grab`]
+- `ToplevelSurface::send_configure`/`PopupSurface::send_configure`/`LayerSurface::send_configure` now always send a configure event regardless of changes and return
+  the serial of the configure event. `send_pending_configure` can be used to only send a configure event on pending changes.
 
 #### Backends
 

anvil/src/input_handler.rs

@@ -127,7 +127,7 @@ impl<BackendData: Backend> AnvilState<BackendData> {
                                 .initial_configure_sent
                         });
                         if mode_changed && initial_configure_sent {
-                            toplevel.send_configure();
+                            toplevel.send_pending_configure();
                         }
                     }
                 }

anvil/src/shell/grabs.rs

@@ -238,7 +238,7 @@ impl<BackendData: Backend> PointerGrab<AnvilState<BackendData>> for ResizeSurfac
                     state.states.set(xdg_toplevel::State::Resizing);
                     state.size = Some(self.last_window_size);
                 });
-                xdg.send_configure();
+                xdg.send_pending_configure();
             }
             #[cfg(feature = "xwayland")]
             WindowElement::X11(x11) => {
@@ -282,7 +282,7 @@ impl<BackendData: Backend> PointerGrab<AnvilState<BackendData>> for ResizeSurfac
                         state.states.unset(xdg_toplevel::State::Resizing);
                         state.size = Some(self.last_window_size);
                     });
-                    xdg.send_configure();
+                    xdg.send_pending_configure();
                     if self.edges.intersects(ResizeEdge::TOP_LEFT) {
                         let geometry = self.window.geometry();
                         let mut location = data.space.element_location(&self.window).unwrap();

anvil/src/shell/xdg.rs

@@ -240,7 +240,6 @@ impl<BackendData: Backend> XdgShellHandler for AnvilState<BackendData> {
                 state.size = Some(geometry.size);
                 state.fullscreen_output = wl_output;
             });
-            surface.send_configure();
             output.user_data().insert_if_missing(FullscreenSurface::default);
             output
                 .user_data()
@@ -249,9 +248,21 @@ impl<BackendData: Backend> XdgShellHandler for AnvilState<BackendData> {
                 .set(window.clone());
             trace!("Fullscreening: {:?}", window);
         }
+
+        // The protocol demands us to always reply with a configure,
+        // regardless of we fulfilled the request or not
+        surface.send_configure();
     }
 
     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;
@@ -264,9 +275,9 @@ impl<BackendData: Backend> XdgShellHandler for AnvilState<BackendData> {
                 fullscreen.clear();
                 self.backend_data.reset_buffers(&output);
             }
-
-            surface.send_configure();
         }
+
+        surface.send_pending_configure();
     }
 
     fn maximize_request(&mut self, surface: ToplevelSurface) {
@@ -286,16 +297,27 @@ impl<BackendData: Backend> XdgShellHandler for AnvilState<BackendData> {
             state.states.set(xdg_toplevel::State::Maximized);
             state.size = Some(geometry.size);
         });
-        surface.send_configure();
         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
+        surface.send_configure();
     }
 
     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_configure();
+        surface.send_pending_configure();
     }
 
     fn grab(&mut self, surface: PopupSurface, seat: wl_seat::WlSeat, serial: Serial) {

anvil/src/state.rs

@@ -301,7 +301,7 @@ impl<BackendData: Backend> XdgDecorationHandler for AnvilState<BackendData> {
                 .initial_configure_sent
         });
         if initial_configure_sent {
-            toplevel.send_configure();
+            toplevel.send_pending_configure();
         }
     }
     fn unset_mode(&mut self, toplevel: ToplevelSurface) {
@@ -319,7 +319,7 @@ impl<BackendData: Backend> XdgDecorationHandler for AnvilState<BackendData> {
                 .initial_configure_sent
         });
         if initial_configure_sent {
-            toplevel.send_configure();
+            toplevel.send_pending_configure();
         }
     }
 }

smallvil/src/grabs/resize_grab.rs

@@ -122,7 +122,7 @@ impl PointerGrab<Smallvil> for ResizeSurfaceGrab {
             state.size = Some(self.last_window_size);
         });
 
-        xdg.send_configure();
+        xdg.send_pending_configure();
     }
 
     fn relative_motion(
@@ -157,7 +157,7 @@ impl PointerGrab<Smallvil> for ResizeSurfaceGrab {
                 state.size = Some(self.last_window_size);
             });
 
-            xdg.send_configure();
+            xdg.send_pending_configure();
 
             ResizeSurfaceState::with(xdg.wl_surface(), |state| {
                 *state = ResizeSurfaceState::WaitingForLastCommit {

smallvil/src/handlers/xdg_shell.rs

@@ -94,7 +94,7 @@ impl XdgShellHandler for Smallvil {
                 state.states.set(xdg_toplevel::State::Resizing);
             });
 
-            surface.send_configure();
+            surface.send_pending_configure();
 
             let grab = ResizeSurfaceGrab::start(
                 start_data,

smallvil/src/input.rs

@@ -72,12 +72,12 @@ impl Smallvil {
                         self.space.raise_element(&window, true);
                         keyboard.set_focus(self, Some(window.toplevel().wl_surface().clone()), serial);
                         self.space.elements().for_each(|window| {
-                            window.toplevel().send_configure();
+                            window.toplevel().send_pending_configure();
                         });
                     } else {
                         self.space.elements().for_each(|window| {
                             window.set_activated(false);
-                            window.toplevel().send_configure();
+                            window.toplevel().send_pending_configure();
                         });
                         keyboard.set_focus(self, Option::<WlSurface>::None, serial);
                     }

src/desktop/wayland/layer.rs

@@ -422,7 +422,7 @@ impl LayerMap {
                 // we would send a wrong size to the client and also violate
                 // the spec by sending a configure event before a prior commit.
                 if size_changed && initial_configure_sent {
-                    layer.0.surface.send_configure();
+                    layer.0.surface.send_pending_configure();
                 }
 
                 layer_state(layer).location = location;

src/wayland/shell/wlr_layer/mod.rs

@@ -156,6 +156,21 @@ impl LayerSurfaceAttributes {
         self.last_acked = None;
         self.current = Default::default();
     }
+
+    fn current_server_state(&self) -> &LayerSurfaceState {
+        self.pending_configures
+            .last()
+            .map(|c| &c.state)
+            .or(self.last_acked.as_ref())
+            .unwrap_or(&self.current)
+    }
+
+    fn has_pending_changes(&self) -> bool {
+        self.server_pending
+            .as_ref()
+            .map(|s| s != self.current_server_state())
+            .unwrap_or(false)
+    }
 }
 
 /// State of a layer surface
@@ -285,29 +300,38 @@ impl LayerSurface {
     /// to the `last_acked`
     fn get_pending_state(&self, attributes: &mut LayerSurfaceAttributes) -> Option<LayerSurfaceState> {
         if !attributes.initial_configure_sent {
-            return Some(attributes.server_pending.take().unwrap_or_default());
+            return Some(
+                attributes
+                    .server_pending
+                    .take()
+                    .unwrap_or_else(|| attributes.current_server_state().clone()),
+            );
         }
 
-        let server_pending = match attributes.server_pending.take() {
-            Some(state) => state,
-            None => {
-                return None;
-            }
-        };
-
-        let last_state = attributes
-            .pending_configures
-            .last()
-            .map(|c| &c.state)
-            .or(attributes.last_acked.as_ref());
-
-        if let Some(state) = last_state {
-            if state == &server_pending {
-                return None;
-            }
+        // Check if the state really changed, it is possible
+        // that with_pending_state has been called without
+        // modifying the state.
+        if !attributes.has_pending_changes() {
+            return None;
         }
 
-        Some(server_pending)
+        attributes.server_pending.take()
+    }
+
+    /// Send a pending configure event to this layer surface to suggest it a new configuration
+    ///
+    /// If changes have occurred a configure event will be send to the clients and the serial will be returned
+    /// (for tracking the configure in [`LayerShellHandler::ack_configure`] if desired).
+    /// If no changes occurred no event will be send and `None` will be returned.
+    ///
+    /// See [`send_configure`](LayerSurface::send_configure) and [`has_pending_changes`](LayerSurface::has_pending_changes)
+    /// for more information.
+    pub fn send_pending_configure(&self) -> Option<Serial> {
+        if self.has_pending_changes() {
+            Some(self.send_configure())
+        } else {
+            None
+        }
     }
 
     /// Send a configure event to this layer surface to suggest it a new configuration
@@ -317,10 +341,9 @@ impl LayerSurface {
     /// You can manipulate the state that will be sent to the client with the [`with_pending_state`](#method.with_pending_state)
     /// method.
     ///
-    /// If changes have occured a configure event will be send to the clients and the serial will be returned
-    /// (for tracking the configure in [`LayerShellHandler::ack_configure`] if desired).
-    /// If no changes occured no event will be send and `None` will be returned.
-    pub fn send_configure(&self) -> Option<Serial> {
+    /// Note: This will always send a configure event, if you intend to only send a configure event on changes take a look at
+    /// [`send_pending_configure`](LayerSurface::send_pending_configure)
+    pub fn send_configure(&self) -> Serial {
         let configure = compositor::with_states(&self.wl_surface, |states| {
             let mut attributes = states
                 .data_map
@@ -328,31 +351,28 @@ impl LayerSurface {
                 .unwrap()
                 .lock()
                 .unwrap();
-            if let Some(pending) = self.get_pending_state(&mut attributes) {
-                let configure = LayerSurfaceConfigure {
-                    serial: SERIAL_COUNTER.next_serial(),
-                    state: pending,
-                };
 
-                attributes.pending_configures.push(configure.clone());
-                attributes.initial_configure_sent = true;
+            let state = self
+                .get_pending_state(&mut attributes)
+                .unwrap_or_else(|| attributes.current_server_state().clone());
 
-                Some(configure)
-            } else {
-                None
-            }
+            let configure = LayerSurfaceConfigure {
+                serial: SERIAL_COUNTER.next_serial(),
+                state,
+            };
+
+            attributes.pending_configures.push(configure.clone());
+            attributes.initial_configure_sent = true;
+
+            configure
         });
 
         // send surface configure
-        if let Some(configure) = configure {
-            let (width, height) = configure.state.size.unwrap_or_default().into();
-            let serial = configure.serial;
-            self.shell_surface
-                .configure(serial.into(), width as u32, height as u32);
-            Some(configure.serial)
-        } else {
-            None
-        }
+        let (width, height) = configure.state.size.unwrap_or_default().into();
+        let serial = configure.serial;
+        self.shell_surface
+            .configure(serial.into(), width as u32, height as u32);
+        serial
     }
 
     /// Make sure this surface was configured
@@ -410,7 +430,7 @@ impl LayerSurface {
                 .lock()
                 .unwrap();
             if attributes.server_pending.is_none() {
-                attributes.server_pending = Some(attributes.current.clone());
+                attributes.server_pending = Some(attributes.current_server_state().clone());
             }
 
             let server_pending = attributes.server_pending.as_mut().unwrap();
@@ -418,6 +438,23 @@ impl LayerSurface {
         })
     }
 
+    /// Tests this [`LayerSurface`] for pending changes
+    ///
+    /// Returns `true` if [`with_pending_state`](LayerSurface::with_pending_state) was used to manipulate the state
+    /// and resulted in a different state or if the initial configure is still pending.
+    pub fn has_pending_changes(&self) -> bool {
+        compositor::with_states(&self.wl_surface, |states| {
+            let attributes = states
+                .data_map
+                .get::<Mutex<LayerSurfaceAttributes>>()
+                .unwrap()
+                .lock()
+                .unwrap();
+
+            !attributes.initial_configure_sent || attributes.has_pending_changes()
+        })
+    }
+
     /// Gets a copy of the current state of this layer
     ///
     /// Returns `None` if the underlying surface has been

src/wayland/shell/xdg/mod.rs

@@ -835,13 +835,17 @@ pub trait XdgShellHandler {
     fn grab(&mut self, surface: PopupSurface, seat: wl_seat::WlSeat, serial: Serial);
 
     /// A toplevel surface requested to be maximized
-    fn maximize_request(&mut self, surface: ToplevelSurface) {}
+    fn maximize_request(&mut self, surface: ToplevelSurface) {
+        surface.send_configure();
+    }
 
     /// A toplevel surface requested to stop being maximized
     fn unmaximize_request(&mut self, surface: ToplevelSurface) {}
 
     /// A toplevel surface requested to be set fullscreen
-    fn fullscreen_request(&mut self, surface: ToplevelSurface, output: Option<wl_output::WlOutput>) {}
+    fn fullscreen_request(&mut self, surface: ToplevelSurface, output: Option<wl_output::WlOutput>) {
+        surface.send_configure();
+    }
 
     /// A toplevel surface request to stop being fullscreen
     fn unfullscreen_request(&mut self, surface: ToplevelSurface) {}
@@ -1085,6 +1089,22 @@ impl ToplevelSurface {
         attributes.server_pending.take()
     }
 
+    /// Send a pending configure event to this toplevel surface to suggest it a new configuration
+    ///
+    /// If changes have occurred a configure event will be send to the clients and the serial will be returned
+    /// (for tracking the configure in [`XdgShellHandler::ack_configure`] if desired).
+    /// If no changes occurred no event will be send and `None` will be returned.
+    ///
+    /// See [`send_configure`](ToplevelSurface::send_configure) and [`has_pending_changes`](ToplevelSurface::has_pending_changes)
+    /// for more information.
+    pub fn send_pending_configure(&self) -> Option<Serial> {
+        if self.has_pending_changes() {
+            Some(self.send_configure())
+        } else {
+            None
+        }
+    }
+
     /// Send a configure event to this toplevel surface to suggest it a new configuration
     ///
     /// The serial of this configure will be tracked waiting for the client to ACK it.
@@ -1092,21 +1112,24 @@ impl ToplevelSurface {
     /// You can manipulate the state that will be sent to the client with the [`with_pending_state`](#method.with_pending_state)
     /// method.
     ///
-    /// If changes have occured a configure event will be send to the clients and the serial will be returned
-    /// (for tracking the configure in [`XdghellHandler::ack_configure`] if desired).
-    /// If no changes occured no event will be send and `None` will be returned.
-    pub fn send_configure(&self) -> Option<Serial> {
+    /// Note: This will always send a configure event, if you intend to only send a configure event on changes take a look at
+    /// [`send_pending_configure`](ToplevelSurface::send_pending_configure)
+    pub fn send_configure(&self) -> Serial {
         let shell_surface_data = self.shell_surface.data::<XdgShellSurfaceUserData>();
         let decoration =
             shell_surface_data.and_then(|data| data.decoration.lock().unwrap().as_ref().cloned());
-        let configure = compositor::with_states(&self.wl_surface, |states| {
-            let mut attributes = states
-                .data_map
-                .get::<XdgToplevelSurfaceData>()
-                .unwrap()
-                .lock()
-                .unwrap();
-            if let Some(pending) = self.get_pending_state(&mut attributes) {
+        let (configure, decoration_mode_changed, bounds_changed) =
+            compositor::with_states(&self.wl_surface, |states| {
+                let mut attributes = states
+                    .data_map
+                    .get::<XdgToplevelSurfaceData>()
+                    .unwrap()
+                    .lock()
+                    .unwrap();
+
+                let pending = self
+                    .get_pending_state(&mut attributes)
+                    .unwrap_or_else(|| attributes.current_server_state().clone());
                 // 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();
@@ -1131,31 +1154,25 @@ impl ToplevelSurface {
                     attributes.initial_decoration_configure_sent = true;
                 }
 
-                Some((configure, decoration_mode_changed, bounds_changed))
-            } else {
-                None
-            }
-        });
-        if let Some((configure, decoration_mode_changed, bounds_changed)) = configure {
-            if decoration_mode_changed {
-                if let Some(decoration) = &decoration {
-                    self::decoration::send_decoration_configure(
-                        decoration,
-                        configure
-                            .state
-                            .decoration_mode
-                            .unwrap_or(zxdg_toplevel_decoration_v1::Mode::ClientSide),
-                    );
-                }
-            }
+                (configure, decoration_mode_changed, bounds_changed)
+            });
 
-            let serial = configure.serial;
-            self::handlers::send_toplevel_configure(&self.shell_surface, configure, bounds_changed);
-
-            Some(serial)
-        } else {
-            None
+        if decoration_mode_changed {
+            if let Some(decoration) = &decoration {
+                self::decoration::send_decoration_configure(
+                    decoration,
+                    configure
+                        .state
+                        .decoration_mode
+                        .unwrap_or(zxdg_toplevel_decoration_v1::Mode::ClientSide),
+                );
+            }
         }
+
+        let serial = configure.serial;
+        self::handlers::send_toplevel_configure(&self.shell_surface, configure, bounds_changed);
+
+        serial
     }
 
     /// Handles the role specific commit logic
@@ -1249,6 +1266,23 @@ impl ToplevelSurface {
         })
     }
 
+    /// Tests this [`ToplevelSurface`] for pending changes
+    ///
+    /// Returns `true` if [`with_pending_state`](ToplevelSurface::with_pending_state) was used to manipulate the state
+    /// and resulted in a different state or if the initial configure is still pending.
+    pub fn has_pending_changes(&self) -> bool {
+        compositor::with_states(&self.wl_surface, |states| {
+            let attributes = states
+                .data_map
+                .get::<XdgToplevelSurfaceData>()
+                .unwrap()
+                .lock()
+                .unwrap();
+
+            !attributes.initial_configure_sent || attributes.has_pending_changes()
+        })
+    }
+
     /// Gets a copy of the current state of this toplevel
     pub fn current_state(&self) -> ToplevelState {
         compositor::with_states(&self.wl_surface, |states| {
@@ -1360,8 +1394,8 @@ impl PopupSurface {
 
     /// 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>) -> Option<Serial> {
-        let next_configure = compositor::with_states(&self.wl_surface, |states| {
+    fn send_configure_internal(&self, reposition_token: Option<u32>) -> Serial {
+        let configure = compositor::with_states(&self.wl_surface, |states| {
             let mut attributes = states
                 .data_map
                 .get::<XdgPopupSurfaceData>()
@@ -1369,35 +1403,40 @@ impl PopupSurface {
                 .lock()
                 .unwrap();
 
-            if !attributes.initial_configure_sent
-                || attributes.has_pending_changes()
-                || reposition_token.is_some()
-            {
-                let pending = attributes
-                    .server_pending
-                    .take()
-                    .unwrap_or_else(|| *attributes.current_server_state());
+            let pending = attributes
+                .server_pending
+                .take()
+                .unwrap_or_else(|| *attributes.current_server_state());
 
-                let configure = PopupConfigure {
-                    state: pending,
-                    serial: SERIAL_COUNTER.next_serial(),
-                    reposition_token,
-                };
+            let configure = PopupConfigure {
+                state: pending,
+                serial: SERIAL_COUNTER.next_serial(),
+                reposition_token,
+            };
 
-                attributes.pending_configures.push(configure);
-                attributes.initial_configure_sent = true;
+            attributes.pending_configures.push(configure);
+            attributes.initial_configure_sent = true;
 
-                Some(configure)
-            } else {
-                None
-            }
+            configure
         });
-        if let Some(configure) = next_configure {
-            let serial = configure.serial;
-            self::handlers::send_popup_configure(&self.shell_surface, configure);
-            Some(serial)
+        let serial = configure.serial;
+        self::handlers::send_popup_configure(&self.shell_surface, configure);
+        serial
+    }
+
+    /// Send a pending configure event to this popup surface to suggest it a new configuration
+    ///
+    /// If changes have occurred a configure event will be send to the clients and the serial will be returned
+    /// (for tracking the configure in [`XdgShellHandler::ack_configure`] if desired).
+    /// If no changes occurred no event will be send and `Ok(None)` will be returned.
+    ///
+    /// See [`send_configure`](PopupSurface::send_configure) and [`has_pending_changes`](PopupSurface::has_pending_changes)
+    /// for more information.
+    pub fn send_pending_configure(&self) -> Result<Option<Serial>, PopupConfigureError> {
+        if self.has_pending_changes() {
+            self.send_configure().map(Some)
         } else {
-            None
+            Ok(None)
         }
     }
 
@@ -1412,10 +1451,9 @@ impl PopupSurface {
     /// the client protocol version disallows a re-configure or the current [`PositionerState`]
     /// is not reactive.
     ///
-    /// If changes have occured a configure event will be send to the clients and the serial will be returned
-    /// (for tracking the configure in [`XdgShellHandler::ack_configure`] if desired).
-    /// If no changes occured no event will be send and `Ok(None)` will be returned.
-    pub fn send_configure(&self) -> Result<Option<Serial>, PopupConfigureError> {
+    /// Note: This will always send a configure event, if you intend to only send a configure event on changes take a look at
+    /// [`send_pending_configure`](PopupSurface::send_pending_configure)
+    pub fn send_configure(&self) -> Result<Serial, PopupConfigureError> {
         // Check if we are allowed to send a configure
         compositor::with_states(&self.wl_surface, |states| {
             let attributes = states
@@ -1450,7 +1488,7 @@ impl PopupSurface {
     /// in response to a `reposition` request.
     ///
     /// For further information see [`send_configure`](#method.send_configure)
-    pub fn send_repositioned(&self, token: u32) -> Option<Serial> {
+    pub fn send_repositioned(&self, token: u32) -> Serial {
         self.send_configure_internal(Some(token))
     }
 
@@ -1575,6 +1613,23 @@ impl PopupSurface {
             f(server_pending)
         })
     }
+
+    /// Tests this [`PopupSurface`] for pending changes
+    ///
+    /// Returns `true` if [`with_pending_state`](PopupSurface::with_pending_state) was used to manipulate the state
+    /// and resulted in a different state or if the initial configure is still pending.
+    pub fn has_pending_changes(&self) -> bool {
+        compositor::with_states(&self.wl_surface, |states| {
+            let attributes = states
+                .data_map
+                .get::<XdgPopupSurfaceData>()
+                .unwrap()
+                .lock()
+                .unwrap();
+
+            !attributes.initial_configure_sent || attributes.has_pending_changes()
+        })
+    }
 }
 
 /// Defines the possible configure variants