mapped: Add needs_configure flag

Allows to de-duplicate configures from requests that require one.

3 files changed, 85 insertions, 30 deletions

diff --git a/src/handlers/xdg_shell.rs b/src/handlers/xdg_shell.rs
index 5cac5d29..840dd599 100644
--- a/src/handlers/xdg_shell.rs
+++ b/src/handlers/xdg_shell.rs
@@ -433,8 +433,12 @@ impl XdgShellHandler for State {
         if let Some((mapped, current_output)) = self
             .niri
             .layout
-            .find_window_and_output(toplevel.wl_surface())
+            .find_window_and_output_mut(toplevel.wl_surface())
         {
+            // A configure is required in response to this event regardless if there are pending
+            // changes.
+            mapped.set_needs_configure();
+
             let window = mapped.window.clone();
 
             if let Some(requested_output) = requested_output {
@@ -446,10 +450,6 @@ impl XdgShellHandler for State {
             }
 
             self.niri.layout.set_fullscreen(&window, true);
-
-            // A configure is required in response to this event regardless if there are pending
-            // changes.
-            toplevel.send_configure();
         } else if let Some(unmapped) = self.niri.unmapped_windows.get_mut(toplevel.wl_surface()) {
             match &mut unmapped.state {
                 InitialConfigureState::NotConfigured { wants_fullscreen } => {
@@ -513,17 +513,14 @@ impl XdgShellHandler for State {
         if let Some((mapped, _)) = self
             .niri
             .layout
-            .find_window_and_output(toplevel.wl_surface())
+            .find_window_and_output_mut(toplevel.wl_surface())
         {
-            let window = mapped.window.clone();
-            self.niri.layout.set_fullscreen(&window, false);
-
             // A configure is required in response to this event regardless if there are pending
             // changes.
-            //
-            // FIXME: when unfullscreening to floating, this will send an extra configure with
-            // scrolling layout bounds. We should probably avoid it.
-            toplevel.send_configure();
+            mapped.set_needs_configure();
+
+            let window = mapped.window.clone();
+            self.niri.layout.set_fullscreen(&window, false);
         } else if let Some(unmapped) = self.niri.unmapped_windows.get_mut(toplevel.wl_surface()) {
             match &mut unmapped.state {
                 InitialConfigureState::NotConfigured { wants_fullscreen } => {
@@ -744,7 +741,13 @@ impl XdgDecorationHandler for State {
         // A configure is required in response to this event. However, if an initial configure
         // wasn't sent, then we will send this as part of the initial configure later.
         if toplevel.is_initial_configure_sent() {
-            toplevel.send_configure();
+            // If this is a mapped window, flag it as needs configure to avoid duplicate configures.
+            let surface = toplevel.wl_surface();
+            if let Some((mapped, _)) = self.niri.layout.find_window_and_output_mut(surface) {
+                mapped.set_needs_configure();
+            } else {
+                toplevel.send_configure();
+            }
         }
     }
 
@@ -757,7 +760,13 @@ impl XdgDecorationHandler for State {
         // A configure is required in response to this event. However, if an initial configure
         // wasn't sent, then we will send this as part of the initial configure later.
         if toplevel.is_initial_configure_sent() {
-            toplevel.send_configure();
+            // If this is a mapped window, flag it as needs configure to avoid duplicate configures.
+            let surface = toplevel.wl_surface();
+            if let Some((mapped, _)) = self.niri.layout.find_window_and_output_mut(surface) {
+                mapped.set_needs_configure();
+            } else {
+                toplevel.send_configure();
+            }
         }
     }
 }
diff --git a/src/tests/floating.rs b/src/tests/floating.rs
index 8b63d55d..81e507f1 100644
--- a/src/tests/floating.rs
+++ b/src/tests/floating.rs
@@ -854,3 +854,30 @@ window-rule {
         @"size: 300 × 100, bounds: 1920 × 1080, states: [Activated]"
     );
 }
+
+#[test]
+fn unfullscreen_to_floating_doesnt_send_extra_configure() {
+    let (mut f, id, surface) = set_up();
+
+    // Make it floating.
+    f.niri().layout.toggle_window_floating(None);
+    f.roundtrip(id);
+
+    // Fullscreen.
+    let window = f.client(id).window(&surface);
+    window.set_fullscreen(None);
+    f.double_roundtrip(id);
+
+    let _ = f.client(id).window(&surface).recent_configures();
+
+    // Unfullscreen via the window request which requires a configure response.
+    let window = f.client(id).window(&surface);
+    window.unset_fullscreen();
+    f.double_roundtrip(id);
+
+    // This should configure only once and not twice.
+    assert_snapshot!(
+        f.client(id).window(&surface).format_recent_configures(),
+        @"size: 936 × 1048, bounds: 1920 × 1080, states: [Activated]"
+    );
+}
diff --git a/src/window/mapped.rs b/src/window/mapped.rs
index 7b542574..559b4641 100644
--- a/src/window/mapped.rs
+++ b/src/window/mapped.rs
@@ -60,6 +60,11 @@ pub struct Mapped {
     /// immediately, rather than setting this flag.
     need_to_recompute_rules: bool,
 
+    /// Whether this window needs a configure this loop cycle.
+    ///
+    /// Certain Wayland requests require a configure in response, like un/fullscreen.
+    needs_configure: bool,
+
     /// Whether this window has the keyboard focus.
     is_focused: bool,
 
@@ -172,6 +177,7 @@ impl Mapped {
             pre_commit_hook: hook,
             rules,
             need_to_recompute_rules: false,
+            needs_configure: false,
             is_focused: false,
             is_active_in_column: true,
             is_floating: false,
@@ -223,6 +229,10 @@ impl Mapped {
         self.recompute_window_rules(rules, is_at_startup)
     }
 
+    pub fn set_needs_configure(&mut self) {
+        self.needs_configure = true;
+    }
+
     pub fn id(&self) -> MappedId {
         self.id
     }
@@ -733,6 +743,11 @@ impl LayoutElement for Mapped {
         let _span =
             trace_span!("configure_intent", surface = ?self.toplevel().wl_surface().id()).entered();
 
+        if self.needs_configure {
+            trace!("the window needs_configure");
+            return ConfigureIntent::ShouldSend;
+        }
+
         with_toplevel_role(self.toplevel(), |attributes| {
             if let Some(server_pending) = &attributes.server_pending {
                 let current_server = attributes.current_server_state();
@@ -783,24 +798,26 @@ impl LayoutElement for Mapped {
         let _span =
             trace_span!("send_pending_configure", surface = ?toplevel.wl_surface().id()).entered();
 
-        // Check for pending changes manually to account for RequestSizeOnce::UseWindowSize.
-        let has_pending_changes = with_toplevel_role(self.toplevel(), |role| {
-            if role.server_pending.is_none() {
-                return false;
-            }
+        // If the window needs a configure, send it regardless.
+        let has_pending_changes = self.needs_configure
+            || with_toplevel_role(self.toplevel(), |role| {
+                // Check for pending changes manually to account for RequestSizeOnce::UseWindowSize.
+                if role.server_pending.is_none() {
+                    return false;
+                }
 
-            let current_server_size = role.current_server_state().size;
-            let server_pending = role.server_pending.as_mut().unwrap();
+                let current_server_size = role.current_server_state().size;
+                let server_pending = role.server_pending.as_mut().unwrap();
 
-            // With UseWindowSize, we do not consider size-only changes, because we will
-            // request the current window size and do not expect it to actually change.
-            if let Some(RequestSizeOnce::UseWindowSize) = self.request_size_once {
-                server_pending.size = current_server_size;
-            }
+                // With UseWindowSize, we do not consider size-only changes, because we will
+                // request the current window size and do not expect it to actually change.
+                if let Some(RequestSizeOnce::UseWindowSize) = self.request_size_once {
+                    server_pending.size = current_server_size;
+                }
 
-            let server_pending = role.server_pending.as_ref().unwrap();
-            server_pending != role.current_server_state()
-        });
+                let server_pending = role.server_pending.as_ref().unwrap();
+                server_pending != role.current_server_state()
+            });
 
         if has_pending_changes {
             // If needed, replace the pending size with the current window size.
@@ -814,6 +831,8 @@ impl LayoutElement for Mapped {
             let serial = toplevel.send_configure();
             trace!(?serial, "sending configure");
 
+            self.needs_configure = false;
+
             if self.animate_next_configure {
                 self.animate_serials.push(serial);
             }