Fix Wayland overlay flicker by honoring SHM buffer lifetimes

src/about_window/render/mod.rs

@@ -74,7 +74,11 @@ impl AboutWindowState {
 
         let wl_surface = self.window.wl_surface();
         wl_surface.set_buffer_scale(self.scale);
-        wl_surface.attach(Some(buffer.wl_buffer()), 0, 0);
+        // Hold the slot until the compositor releases it; a raw attach frees it
+        // immediately and the next frame repaints the buffer still on screen.
+        buffer
+            .attach_to(wl_surface)
+            .map_err(|err| anyhow::anyhow!("failed to attach the about-window buffer: {err}"))?;
         wl_surface.damage_buffer(0, 0, phys_w as i32, phys_h as i32);
         wl_surface.commit();
 

src/backend/wayland/backend/event_loop/render.rs

@@ -2,7 +2,7 @@ use std::time::{Duration, Instant};
 
 use log::{debug, warn};
 
-use super::super::super::state::{PerfRenderSkipReason, WaylandState};
+use super::super::super::state::{PerfRenderSkipReason, RenderOutcome, WaylandState};
 
 const MAX_RENDER_FAILURES: u32 = 10;
 
@@ -92,7 +92,17 @@ pub(super) fn maybe_render(
             state.input_state.zoom_chip_hover,
         );
         match state.render(qh) {
-            Ok(keep_rendering) => {
+            Ok(RenderOutcome::BuffersInFlight) => {
+                // Nothing was painted or committed: the compositor still owns
+                // every slot. Leave `needs_redraw` set so the next pass retries,
+                // and pace that retry off `last_render_time` like any other
+                // attempt so the FPS cap throttles the wait instead of spinning.
+                // The frame counters stay untouched - this was not a frame.
+                *consecutive_render_failures = 0;
+                *last_render_time = Some(Instant::now());
+                debug!("Main loop: render deferred - all buffers still held by the compositor");
+            }
+            Ok(RenderOutcome::Committed { keep_rendering }) => {
                 let render_end = Instant::now();
                 let render_duration = render_end.saturating_duration_since(render_start);
                 if render_duration > Duration::from_millis(5) {

src/backend/wayland/state.rs

@@ -27,7 +27,7 @@ use std::{
 };
 use wayland_client::{
     Proxy, QueueHandle,
-    protocol::{wl_output, wl_pointer, wl_seat, wl_shm, wl_surface, wl_touch},
+    protocol::{wl_output, wl_pointer, wl_seat, wl_surface, wl_touch},
 };
 #[cfg(feature = "tablet-input")]
 use wayland_protocols::wp::tablet::zv2::client::{
@@ -131,6 +131,7 @@ pub(in crate::backend::wayland) use self::perf::{
     PerfDamageDiagnostics, PerfFrameDamageContext, PerfInputSource, PerfRenderBreakdown,
     PerfRenderProfileKind, PerfRenderSkipReason, damage_covers_logical_surface,
 };
+pub(in crate::backend::wayland) use self::render::RenderOutcome;
 pub(super) use helpers::{
     damage_summary, debug_damage_logging_enabled, debug_toolbar_drag_logging_enabled, drag_log,
     force_inline_toolbars_requested, scale_damage_regions, surface_id,

src/backend/wayland/state/perf.rs

@@ -71,6 +71,7 @@ pub(in crate::backend::wayland) enum PerfRenderSkipReason {
     FpsCap,
     SurfaceUnconfigured,
     NoRedraw,
+    BuffersInFlight,
 }
 
 impl fmt::Display for PerfRenderSkipReason {
@@ -80,6 +81,7 @@ impl fmt::Display for PerfRenderSkipReason {
             Self::FpsCap => f.write_str("fps_cap"),
             Self::SurfaceUnconfigured => f.write_str("surface_unconfigured"),
             Self::NoRedraw => f.write_str("no_redraw"),
+            Self::BuffersInFlight => f.write_str("buffers_in_flight"),
         }
     }
 }
@@ -218,6 +220,7 @@ struct PerfFramePacingSummary {
     skipped_fps_cap: u64,
     skipped_surface_unconfigured: u64,
     skipped_no_redraw: u64,
+    skipped_buffers_in_flight: u64,
 }
 
 #[derive(Debug)]
@@ -244,6 +247,7 @@ pub(super) struct PerfMetrics {
     skipped_fps_cap: u64,
     skipped_surface_unconfigured: u64,
     skipped_no_redraw: u64,
+    skipped_buffers_in_flight: u64,
     dropped_input_samples: u64,
     last_summary_at: Option<Instant>,
     last_frame_pacing_summary_at: Option<Instant>,
@@ -386,7 +390,7 @@ fn log_input_summary(summary: &PerfSummary, final_summary: bool) {
 
 fn log_frame_pacing_summary(summary: &PerfFramePacingSummary, final_summary: bool) {
     info!(
-        "perf.frame_pacing frames={} window_frames={} render_p50_ms={} render_p95_ms={} render_p99_ms={} render_max_ms={} render_over_8ms={} render_over_16ms={} render_over_33ms={} render_over_50ms={} full_damage_count={} full_damage_pct={} force_full_reason={} force_full_reasons={} skipped_frame_callback_pending={} skipped_fps_cap={} skipped_surface_unconfigured={} skipped_no_redraw={} final={}",
+        "perf.frame_pacing frames={} window_frames={} render_p50_ms={} render_p95_ms={} render_p99_ms={} render_max_ms={} render_over_8ms={} render_over_16ms={} render_over_33ms={} render_over_50ms={} full_damage_count={} full_damage_pct={} force_full_reason={} force_full_reasons={} skipped_frame_callback_pending={} skipped_fps_cap={} skipped_surface_unconfigured={} skipped_no_redraw={} skipped_buffers_in_flight={} final={}",
         summary.frames,
         summary.window_frames,
         summary.render_p50_ms,
@@ -405,6 +409,7 @@ fn log_frame_pacing_summary(summary: &PerfFramePacingSummary, final_summary: boo
         summary.skipped_fps_cap,
         summary.skipped_surface_unconfigured,
         summary.skipped_no_redraw,
+        summary.skipped_buffers_in_flight,
         final_summary
     );
 }

src/backend/wayland/state/perf_modules/metrics.rs

@@ -33,6 +33,7 @@ impl PerfMetrics {
             skipped_fps_cap: 0,
             skipped_surface_unconfigured: 0,
             skipped_no_redraw: 0,
+            skipped_buffers_in_flight: 0,
             dropped_input_samples: 0,
             last_summary_at: None,
             last_frame_pacing_summary_at: None,
@@ -75,6 +76,9 @@ impl PerfMetrics {
             PerfRenderSkipReason::NoRedraw => {
                 self.skipped_no_redraw += 1;
             }
+            PerfRenderSkipReason::BuffersInFlight => {
+                self.skipped_buffers_in_flight += 1;
+            }
         }
     }
 
@@ -424,6 +428,7 @@ impl PerfMetrics {
             skipped_fps_cap: self.skipped_fps_cap,
             skipped_surface_unconfigured: self.skipped_surface_unconfigured,
             skipped_no_redraw: self.skipped_no_redraw,
+            skipped_buffers_in_flight: self.skipped_buffers_in_flight,
         }
     }
 
@@ -445,6 +450,7 @@ impl PerfMetrics {
         self.skipped_fps_cap = 0;
         self.skipped_surface_unconfigured = 0;
         self.skipped_no_redraw = 0;
+        self.skipped_buffers_in_flight = 0;
         self.last_frame_pacing_summary_at = Some(now);
     }
 

src/backend/wayland/state/perf_modules/metrics/tests.rs

@@ -249,6 +249,7 @@ fn frame_pacing_summary_reports_render_percentiles_and_skips() {
     metrics.record_render_skip(PerfRenderSkipReason::FpsCap);
     metrics.record_render_skip(PerfRenderSkipReason::SurfaceUnconfigured);
     metrics.record_render_skip(PerfRenderSkipReason::NoRedraw);
+    metrics.record_render_skip(PerfRenderSkipReason::BuffersInFlight);
 
     for frame in 0..SUMMARY_FRAME_INTERVAL {
         let started_at = base + Duration::from_millis(frame * 2);
@@ -282,6 +283,7 @@ fn frame_pacing_summary_reports_render_percentiles_and_skips() {
             assert_eq!(summary.skipped_fps_cap, 1);
             assert_eq!(summary.skipped_surface_unconfigured, 1);
             assert_eq!(summary.skipped_no_redraw, 1);
+            assert_eq!(summary.skipped_buffers_in_flight, 1);
             assert_eq!(summary.render_over_50ms, 70);
             assert_eq!(summary.full_damage_count, 3);
             assert_eq!(summary.full_damage_pct, "2.50");
@@ -426,3 +428,32 @@ fn damage_percentage_clamps_to_surface_bounds() {
         100
     ));
 }
+
+/// Skip counters are per-window: a deferral counted in one summary must not
+/// be carried into the next, or a running total hides whether the swapchain
+/// bound is still engaging.
+#[test]
+fn frame_pacing_summary_resets_skip_counters_between_windows() {
+    let base = Instant::now();
+    let mut metrics = PerfMetrics::new(true);
+    metrics.record_render_skip(PerfRenderSkipReason::BuffersInFlight);
+
+    let mut summaries = Vec::new();
+    for frame in 0..(SUMMARY_FRAME_INTERVAL * 2) {
+        let started_at = base + Duration::from_millis(frame * 2);
+        let report = metrics.record_render_complete(
+            started_at,
+            started_at + Duration::from_millis(1),
+            true,
+            120,
+            false,
+        );
+        if let Some(summary) = report.and_then(|report| report.summary) {
+            summaries.push(summary);
+        }
+    }
+
+    assert_eq!(summaries.len(), 2);
+    assert_eq!(summaries[0].skipped_buffers_in_flight, 1);
+    assert_eq!(summaries[1].skipped_buffers_in_flight, 0);
+}

src/backend/wayland/state/render/mod.rs

@@ -5,8 +5,24 @@ mod tool_preview;
 mod ui;
 mod ui_effect_damage;
 
+/// What a render pass actually did.
+///
+/// `BuffersInFlight` is not a frame: nothing was painted or committed, so the
+/// caller must keep the redraw pending and leave the frame counters alone.
+/// Discarding this silently would let a caller treat an uncommitted frame as
+/// on-screen, so it is `#[must_use]`.
+#[must_use]
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub(in crate::backend::wayland) enum RenderOutcome {
+    Committed { keep_rendering: bool },
+    BuffersInFlight,
+}
+
 impl WaylandState {
-    pub(in crate::backend::wayland) fn render(&mut self, qh: &QueueHandle<Self>) -> Result<bool> {
+    pub(in crate::backend::wayland) fn render(
+        &mut self,
+        qh: &QueueHandle<Self>,
+    ) -> Result<RenderOutcome> {
         debug!("=== RENDER START ===");
         let board_is_transparent = self.input_state.board_is_transparent();
         let suppression = self
@@ -44,6 +60,40 @@ impl WaylandState {
             }};
         }
 
+        // Acquire the buffer before anything mutates render state. Animation
+        // ticks and `collect_ui_effect_damage` both advance clocks and record
+        // previous bounds exactly once per rendered frame, so running them for
+        // a frame that is then deferred would let the next real commit miss an
+        // effect's on-screen footprint.
+        let acquired = record_stage!(buffer_acquire, {
+            self.surface.acquire_buffer(
+                &self.shm,
+                buffer_count,
+                phys_width as i32,
+                phys_height as i32,
+                (phys_width * 4) as i32,
+            )?
+        });
+        let Some(acquired) = acquired else {
+            // Every slot is still owned by the compositor. Keep the redraw
+            // pending and retry on the next pass rather than painting over a
+            // buffer that is still on screen.
+            debug!("All {buffer_count} buffers in flight - deferring this frame");
+            self.record_perf_render_skip(PerfRenderSkipReason::BuffersInFlight);
+            return Ok(RenderOutcome::BuffersInFlight);
+        };
+        // The canvas pointer doubles as the slot identifier for damage
+        // tracking: a slot keeps its memory for the pool's lifetime, so the
+        // same pointer means the same slot.
+        let super::super::surface::AcquiredBuffer {
+            buffer,
+            canvas_ptr,
+            pool_generation: pool_gen,
+            pool_size,
+        } = acquired;
+        debug!("Buffer acquired from pool (slot ptr: 0x{:x})", canvas_ptr);
+        self.surface.update_pool_size(pool_size);
+
         let now = Instant::now();
         let (
             highlight_active,
@@ -71,9 +121,10 @@ impl WaylandState {
         self.update_ui_animation_tick(now, ui_animation_active);
         let keep_rendering = ui_animation_active && self.ui_animation_interval.is_none();
 
-        // Add new dirty regions from input state to the per-buffer damage tracker.
-        // We do this BEFORE acquiring the buffer/damage so the current frame's changes
-        // are included in the damage for the current buffer.
+        // Add new dirty regions from input state to the per-buffer damage
+        // tracker. This runs after the buffer is acquired but before its damage
+        // is drained below, so the current frame's changes are included in the
+        // damage reported for this slot.
         let logical_width = width.min(i32::MAX as u32) as i32;
         let logical_height = height.min(i32::MAX as u32) as i32;
         let mut damage_diagnostics = PerfDamageDiagnostics::default();
@@ -121,36 +172,6 @@ impl WaylandState {
             }
         });
 
-        // Get a buffer from the pool for rendering
-        let (buffer, canvas_ptr, pool_gen, pool_size) = record_stage!(buffer_acquire, {
-            let (pool, generation) = self.surface.ensure_pool(&self.shm, buffer_count)?;
-            debug!(
-                "Requesting buffer from pool (gen {}, size {})",
-                generation,
-                pool.len()
-            );
-            let (buf, cvs) = pool
-                .create_buffer(
-                    phys_width as i32,
-                    phys_height as i32,
-                    (phys_width * 4) as i32,
-                    wl_shm::Format::Argb8888,
-                )
-                .context("Failed to create buffer")?;
-            // Capture canvas pointer as stable slot identifier for damage tracking.
-            // SlotPool reuses the same memory regions, so this pointer identifies the slot.
-            let ptr = cvs.as_mut_ptr();
-            let key = ptr as usize;
-            // Drop the slice borrow so we can query pool metadata; keep raw pointer for Cairo.
-            let _ = cvs;
-            let pool_size = pool.len();
-            debug!("Buffer acquired from pool (slot ptr: 0x{:x})", key);
-            (buf, key, generation, pool_size)
-        });
-
-        // Record pool size after create_buffer to detect growth.
-        self.surface.update_pool_size(pool_size);
-
         // Take damage for this buffer slot (identified by canvas memory address).
         // Pool identity (generation + size) is passed to detect pool recreation/growth.
         // SlotPool reuses the same memory regions for released buffers, so the
@@ -362,7 +383,15 @@ impl WaylandState {
                 .cloned()
                 .context("Surface not created")?;
             wl_surface.set_buffer_scale(scale);
-            wl_surface.attach(Some(buffer.wl_buffer()), 0, 0);
+            // `attach_to` marks the slot active until the compositor releases the
+            // buffer. Attaching the raw `wl_buffer()` instead leaves the slot free,
+            // so the pool hands the same memory back on the next frame and the next
+            // paint lands in the buffer the compositor is still reading - the whole
+            // swapchain collapses to one slot and partial damage resurfaces stale or
+            // half-drawn pixels.
+            buffer
+                .attach_to(&wl_surface)
+                .map_err(|err| anyhow::anyhow!("failed to attach the overlay buffer: {err}"))?;
 
             // Damage logic moved to top of function (add_regions and take_buffer_damage).
             // We now use the computed screen-space damage for clipping and compositor hints.
@@ -428,7 +457,7 @@ impl WaylandState {
         if self.capture_suppressed() {
             self.capture.mark_preflight_rendered();
         }
-        Ok(keep_rendering)
+        Ok(RenderOutcome::Committed { keep_rendering })
     }
 
     fn render_force_full_damage_reason(&self) -> Option<FullDamageReason> {