Fix layer surfaces spuriously hiding/re-showing on their own realization

sync_layer_visibility could not tell a real hide (null-buffer commit on
an already-visible surface) apart from a layer-shell client's ordinary
realization sequence (commit with no buffer -> configure -> ack-commit
with no buffer again -> attach real content): both look like "committed,
no buffer" from has_buffer alone. Every layer surface's first realization
was spuriously unmapped and immediately remapped, doubling LayerMap
arrange() passes on every single popup open.

Live-reproduced via an AGS peer session: a full-monitor click-outside-to-
close popup surface came back from a hit-test with geometry wider than
the real output after several open/close cycles on a wl_surface GTK had
reused across role destroy/recreate, and sat in the Top layer above every
real window with no input region set - silently swallowing clicks meant
for windows, dropdowns, and CSD title bars alike.

layer_surfaces_shown_once now gates the hide path on a surface having
actually shown a buffer at least once, and is cleared in layer_destroyed
so a reused wl_surface's next role starts clean rather than inheriting
the previous role's flag.

6 files changed, 53 insertions, 27 deletions

diff --git a/crates/wayland/src/input.rs b/crates/wayland/src/input.rs
index be5b15f..2463d02 100644
--- a/crates/wayland/src/input.rs
+++ b/crates/wayland/src/input.rs
@@ -17,6 +17,7 @@ use smithay::input::keyboard::FilterResult;
 use smithay::input::pointer::{ButtonEvent, MotionEvent};
 use smithay::output::Output;
 use smithay::reexports::wayland_server::protocol::wl_surface::WlSurface;
+use smithay::reexports::wayland_server::Resource as _;
 use smithay::utils::{Logical, Point, SERIAL_COUNTER};
 use smithay::wayland::compositor::with_states;
 use smithay::wayland::shell::wlr_layer::{Anchor, ExclusiveZone, KeyboardInteractivity, Layer, LayerSurfaceCachedState};
@@ -89,25 +90,27 @@ pub(crate) fn layer_surface_under_layers(state: &CompState, pos: Point<f64, Logi
             if !geo.to_f64().contains(local) {
                 continue;
             }
-            // Temporary: tracing a live report that a bottom-anchored
-            // layer surface (a dock) receives no pointer input at all,
-            // despite its own committed input region - as measured from
-            // the AGS side - covering the point being tested. Logs
-            // exactly what this compositor's own view of that surface is
-            // at the moment of the hit-test, so the two sides' numbers can
-            // be compared directly instead of guessed at. Remove once
-            // that's settled.
+            // Temporary: verifying the `layer_surfaces_shown_once` fix
+            // (state/layers.rs) actually stops a reused `wl_surface`'s
+            // stale layer-shell entry from outliving its role destroy --
+            // live-reproduced this session as a full-monitor click-catcher
+            // popup whose hit-tested geometry came back wider than the
+            // real output after several open/close cycles. Remove once a
+            // restart confirms the geometry stays sane across repeated
+            // popup toggles.
             let local_in_surface = local - geo.loc.to_f64();
             // `None` here means "no region ever committed" - per-protocol
             // that means the *whole* surface is input-sensitive, not that
-            // nothing is, so it is its own distinct, meaningful answer.
+            // nothing is, so it is its own distinct, meaningful answer from
+            // `Some([])` (a region was committed and it is empty).
             let region_dump = with_states(layer.wl_surface(), |states| {
                 states.cached_state.get::<smithay::wayland::compositor::SurfaceAttributes>().current().input_region.as_ref().map(|r| r.rects.clone())
             });
             log::info!(
-                "layer_hit_test: layer={:?} namespace={:?} geo={:?} local_in_surface={:?} input_region={:?}",
+                "layer_hit_test: layer={:?} namespace={:?} surface={:?} geo={:?} local_in_surface={:?} input_region={:?}",
                 layer_kind,
                 layer.namespace(),
+                layer.wl_surface().id(),
                 geo,
                 local_in_surface,
                 region_dump
diff --git a/crates/wayland/src/protocols.rs b/crates/wayland/src/protocols.rs
index 94075c2..0b0eea2 100644
--- a/crates/wayland/src/protocols.rs
+++ b/crates/wayland/src/protocols.rs
@@ -661,6 +661,15 @@ impl WlrLayerShellHandler for CompState {
         // `new_surface` - see that function's doc comment for the bug
         // this exists to route around.
         self.dead_layer_surfaces.insert(surface.wl_surface().clone());
+        // GTK (confirmed live via an AGS peer session's WAYLAND_DEBUG trace)
+        // reuses the same `wl_surface` for the next `get_layer_surface` role
+        // rather than creating a fresh one - so without this, a "shown at
+        // least once" flag from *this* role would leak onto the next one
+        // and make `sync_layer_visibility` treat that new role's own
+        // ack-configure commit as eligible to hide again, the same bug
+        // `layer_surfaces_shown_once` exists to prevent, just reintroduced
+        // for exactly the reused-surface case that matters here.
+        self.layer_surfaces_shown_once.remove(surface.wl_surface());
         // The surface belongs to exactly one output's map, but which one is
         // the client's choice, so unmap from whichever holds it.
         for output in self.outputs().cloned().collect::<Vec<_>>() {
diff --git a/crates/wayland/src/state/layers.rs b/crates/wayland/src/state/layers.rs
index 34db3f1..ee4c022 100644
--- a/crates/wayland/src/state/layers.rs
+++ b/crates/wayland/src/state/layers.rs
@@ -61,29 +61,13 @@ impl CompState {
         }
         let has_buffer = with_renderer_surface_state(surface, |state: &mut RendererSurfaceState| state.buffer().is_some()).unwrap_or(false);
 
-        // TEMPORARY diagnostic for the "AGS dock reserves its exclusive
-        // zone but never paints" investigation, relayed from the AGS peer
-        // session - this function's own `has_buffer`-based hide/show logic
-        // (see the doc comment above) is the leading suspect: if a client
-        // ever legitimately commits a null buffer for a reason other than
-        // an intentional hide (an internal `Gtk.Revealer` transition
-        // artifact, a resize-in-progress commit), this treats it as a hide,
-        // `unmap_layer`s it, and requires a *later* has-buffer commit to
-        // ever come back - which would look exactly like this symptom if
-        // the client's own state machine doesn't expect the compositor to
-        // have done that and never re-triggers one. Logs every call for
-        // every layer surface, not just suspected ones, since which
-        // surface is actually affected isn't confirmed yet. Remove once
-        // resolved.
-        log::warn!("LAYER-VIS-DIAG surface={:?} has_buffer={has_buffer} already_hidden={}", surface.id(), self.hidden_layer_surfaces.contains_key(surface));
-
         if has_buffer {
+            self.layer_surfaces_shown_once.insert(surface.clone());
             let Some((output, layer)) = self.hidden_layer_surfaces.remove(surface) else { return };
             let mut map = layer_map_for_output(&output);
             let zone_before = map.non_exclusive_zone();
             let _ = map.map_layer(&layer);
             let zone_after = map.non_exclusive_zone();
-            log::warn!("LAYER-VIS-DIAG re-mapped namespace={:?} zone_before={zone_before:?} zone_after={zone_after:?}", layer.namespace());
             if zone_after != zone_before {
                 self.pending.borrow_mut().push(CoreEvent::MonitorAdded(srdwm_core::Monitor::new(0, "", srdwm_core::Rect::new(0, 0, 0, 0))));
             }
@@ -93,6 +77,13 @@ impl CompState {
         if self.hidden_layer_surfaces.contains_key(surface) {
             return;
         }
+        // A commit with no buffer on a surface that has never shown one
+        // yet is the ack-configure step of realization, not a hide - see
+        // `layer_surfaces_shown_once`'s own doc comment. Only a surface
+        // that has genuinely been visible at least once can be hidden.
+        if !self.layer_surfaces_shown_once.contains(surface) {
+            return;
+        }
         for output in self.outputs().cloned().collect::<Vec<_>>() {
             let mut map = layer_map_for_output(&output);
             let Some(layer) = map.layers().find(|l| l.wl_surface() == surface).cloned() else { continue };
diff --git a/crates/wayland/src/state/mod.rs b/crates/wayland/src/state/mod.rs
index e5b4d6a..5643ebc 100644
--- a/crates/wayland/src/state/mod.rs
+++ b/crates/wayland/src/state/mod.rs
@@ -303,6 +303,27 @@ pub(crate) struct CompState {
     /// again on its own - this is the only way `sync_layer_visibility`
     /// can re-map it once the client commits real content again.
     pub(crate) hidden_layer_surfaces: HashMap<WlSurface, (smithay::output::Output, smithay::desktop::LayerSurface)>,
+    /// Layer surfaces `sync_layer_visibility` has seen commit an actual
+    /// buffer at least once. A layer-shell client's realization sequence is
+    /// (commit with no buffer -> receive configure -> commit with no buffer
+    /// again to ack it -> *then* attach and commit real content), and that
+    /// middle ack-commit is indistinguishable from a real "hide" (a null-
+    /// buffer commit on an already-visible surface) by buffer-presence
+    /// alone - both are "committed, no buffer". Without this, every
+    /// layer-shell surface's very first realization spuriously unmapped and
+    /// immediately remapped itself through `sync_layer_visibility`, doubling
+    /// the number of `LayerMap::arrange()` passes on every single popup
+    /// open (confirmed live: an AGS popup toggle logged unmapped/re-mapped
+    /// within the same ~300ms window every time) and giving a second,
+    /// needless remap for `arrange()`'s zone/size math to disagree with
+    /// itself across - the leading suspect for a live-reproduced bug where
+    /// a full-monitor click-catcher popup's hit-tested geometry came back
+    /// wider than the real output after several open/close cycles. Real
+    /// hides (a role kept alive, buffer later reattached) still work:
+    /// `sync_layer_visibility`'s own `has_buffer` branch inserts here before
+    /// this set is ever consulted, so a surface only reaches the unmap path
+    /// once it has legitimately shown something.
+    pub(crate) layer_surfaces_shown_once: HashSet<WlSurface>,
     pub(crate) decorations: HashMap<WindowId, MemoryRenderBuffer>,
     /// The top border strip's rounded-corner bitmap, cached the same way
     /// and at the same trigger points as `decorations` (built in
diff --git a/crates/wayland/src/udev/platform.rs b/crates/wayland/src/udev/platform.rs
index 8676a5a..14dd677 100644
--- a/crates/wayland/src/udev/platform.rs
+++ b/crates/wayland/src/udev/platform.rs
@@ -182,6 +182,7 @@ impl UdevPlatform {
             id_to_window: HashMap::new(),
             dead_layer_surfaces: HashSet::new(),
             hidden_layer_surfaces: HashMap::new(),
+            layer_surfaces_shown_once: HashSet::new(),
             decorations: HashMap::new(),
             border_top_decorations: HashMap::new(),
             border_bottom_decorations: HashMap::new(),
diff --git a/crates/wayland/src/winit/connect.rs b/crates/wayland/src/winit/connect.rs
index 0f3a6d3..41e9b2a 100644
--- a/crates/wayland/src/winit/connect.rs
+++ b/crates/wayland/src/winit/connect.rs
@@ -152,6 +152,7 @@ impl WaylandPlatform {
             id_to_window: HashMap::new(),
             dead_layer_surfaces: HashSet::new(),
             hidden_layer_surfaces: HashMap::new(),
+            layer_surfaces_shown_once: HashSet::new(),
             decorations: HashMap::new(),
             border_top_decorations: HashMap::new(),
             border_bottom_decorations: HashMap::new(),