From 8aa9ddaeb2a428d795e2d33f67c0e5e2aa56cd13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Mon, 18 May 2026 14:53:56 -0400 Subject: [PATCH] fix(shell): debounce viewport resize so animations stop flashing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sidebar / window-edge animations emit a new viewport bounds on every GPUI paint. The previous flow fired `runtime.ensure_tab` synchronously on every change, which reached down into Servo's surfman backend and destroyed + reallocated the rendering framebuffer 50+ times per second — each cycle leaving the NSView with an empty surface until Servo's next paint completed. The user saw the page strobe blank-then-content on every gesture, plus a one-time Metal driver warning about an unloadable texture (the previous framebuffer caught mid-recreate). Track `viewport_size_changed_at` on `PerTabSurface` and propagate the debounce on two seams: - `record_viewport_size` returns `Buffered` for a transition that arrives inside the 80 ms window of the previous one, so the synchronous `flush_external_web_surface_tick` from the GPUI paint callback skips the resize altogether. The very first measurement and the first transition after a quiet period still return `Applied` so a single drag step or page-load is not delayed. - `ensure_surface` skips while the viewport is settling, but only *after* the initial ensure has installed `last_ensure_key`. The first ensure must fire even mid-gesture or the page never loads. - `next_tick_delay` clamps the poll cadence to `ACTIVE_POLL_INTERVAL` while any visible tab is settling, so the trailing-edge resize fires within a frame of the gesture stopping instead of waiting for an 80 ms idle tick. The synchronous `record + ensure` pattern in `failed_surface_ensure_waits_for_a_new_key_before_retrying` can't advance an `Instant`, so it now calls a `#[cfg(test)]` `clear_viewport_resize_debounce_for_test` to simulate the trailing edge and exercise the retry-on-new-key business rule in isolation. Adds `rapid_viewport_changes_buffer_until_gesture_settles` to lock the new behaviour. --- crates/ely_app/src/shell/web_surface.rs | 42 ++++++++++++++++- crates/ely_app/src/shell/web_surface_input.rs | 23 +++++++++- .../src/shell/web_surface_runtime_tests.rs | 6 +++ crates/ely_app/src/shell/web_surface_state.rs | 45 +++++++++++++++++++ crates/ely_app/src/shell/web_surface_tests.rs | 30 +++++++++++++ 5 files changed, 143 insertions(+), 3 deletions(-) diff --git a/crates/ely_app/src/shell/web_surface.rs b/crates/ely_app/src/shell/web_surface.rs index 3dd47b5..5ec5ec9 100644 --- a/crates/ely_app/src/shell/web_surface.rs +++ b/crates/ely_app/src/shell/web_surface.rs @@ -6,7 +6,7 @@ use ely_domain::{BrowserTab, TabId}; use crate::services::ProfileDataMode; use super::{ - web_surface_cadence::IDLE_POLL_INTERVAL, + web_surface_cadence::{ACTIVE_POLL_INTERVAL, IDLE_POLL_INTERVAL}, web_surface_frame::WebSurfaceFrame, web_surface_permissions::WebSurfaceSitePermission, web_surface_runtime::{WebSurfaceRuntime, WebSurfaceRuntimeFrame}, @@ -72,6 +72,25 @@ impl WebSurfaceStore { if self.surfaces.get(tab.id()).is_some_and(|surface| !surface.should_ensure(&ensure_key)) { return false; } + // Once the page has ensured at least once, defer further ensures + // while the viewport is still being animated. Without this, a + // sidebar slide or window-edge drag fires `ensure_surface` per + // GPUI paint, each call hitting Servo's `rendering_context.resize` + // which destroys and reallocates the framebuffer — the source of + // the per-frame blank flash. The first ensure (when no prior + // `last_ensure_key` is set) always fires so the page can load. + let already_ensured = self + .surfaces + .get(tab.id()) + .is_some_and(|surface| surface.last_ensure_key.is_some()); + if already_ensured + && self + .surfaces + .get(tab.id()) + .is_some_and(|surface| surface.viewport_size_is_settling(Instant::now())) + { + return false; + } let input = self.take_pending_input(tab.id(), requested_url.as_str()); let previous_frame = self.previous_ready_frame(tab.id(), requested_url.as_str(), tab.zoom_percent()); @@ -186,8 +205,20 @@ impl WebSurfaceStore { } pub(super) fn next_tick_delay(&self, visible_tab_ids: &[TabId]) -> Duration { + let now = Instant::now(); + // While any visible tab's viewport is mid-animation, poll at the + // active 120 Hz cadence so the trailing-edge resize fires within + // one frame of the gesture settling. Without this boost the + // idle 80 ms polling adds a noticeable lag between letting go + // of a resize and the page re-laying-out at the final size. + let any_settling = visible_tab_ids + .iter() + .any(|tab_id| self.surfaces.get(tab_id).is_some_and(|s| s.viewport_size_is_settling(now))); + if any_settling { + return ACTIVE_POLL_INTERVAL; + } self.runtime - .next_poll_delay(visible_tab_ids, Instant::now()) + .next_poll_delay(visible_tab_ids, now) .unwrap_or(IDLE_POLL_INTERVAL) .min(IDLE_POLL_INTERVAL) } @@ -319,6 +350,13 @@ impl WebSurfaceStore { self.surfaces.get(tab_id) } + #[cfg(test)] + pub(super) fn clear_viewport_resize_debounce_for_test(&mut self, tab_id: &TabId) { + if let Some(surface) = self.surfaces.get_mut(tab_id) { + surface.clear_viewport_resize_debounce_for_test(); + } + } + #[cfg(test)] pub(super) fn flush_runtime_for_test(&self) { self.runtime.flush_for_test(); diff --git a/crates/ely_app/src/shell/web_surface_input.rs b/crates/ely_app/src/shell/web_surface_input.rs index 29e585e..7ff8f39 100644 --- a/crates/ely_app/src/shell/web_surface_input.rs +++ b/crates/ely_app/src/shell/web_surface_input.rs @@ -67,10 +67,14 @@ impl WebSurfaceStore { let Some(size) = WebSurfaceSize::from_bounds(bounds, scale_factor) else { return WebSurfaceInputOutcome::DroppedInvalidBounds; }; + let now = Instant::now(); let surface = self.surface_mut(tab_id); surface.viewport_bounds = Some(bounds); let Some(current_size) = surface.viewport_size else { + // First measurement: hand it to the runtime immediately so the + // page can start loading. Don't mark a transition timestamp — + // there's nothing to debounce yet. surface.viewport_size = Some(size); return WebSurfaceInputOutcome::Applied; }; @@ -79,8 +83,25 @@ impl WebSurfaceStore { return WebSurfaceInputOutcome::NoChange; } + // Genuine size transition. Stamp the moment regardless of + // outcome so `viewport_size_is_settling` keeps stretching the + // debounce as the animation emits more frames; the latest size + // always wins in the cadence-driven retry. + let was_settling = surface.viewport_size_is_settling(now); surface.viewport_size = Some(size); - WebSurfaceInputOutcome::Applied + surface.mark_viewport_size_changed(now); + + if was_settling { + // Inside the debounce window — drop the synchronous flush + // so the GPUI paint that emitted this bounds doesn't pay a + // Servo resize. The poll cadence already scheduled by the + // first frame of this animation re-runs `ensure_surface`, + // which will pick up the trailing size once the gesture + // settles past [`VIEWPORT_RESIZE_DEBOUNCE`]. + WebSurfaceInputOutcome::Buffered + } else { + WebSurfaceInputOutcome::Applied + } } pub(super) fn record_native_surface( diff --git a/crates/ely_app/src/shell/web_surface_runtime_tests.rs b/crates/ely_app/src/shell/web_surface_runtime_tests.rs index b42b981..8b5bf06 100644 --- a/crates/ely_app/src/shell/web_surface_runtime_tests.rs +++ b/crates/ely_app/src/shell/web_surface_runtime_tests.rs @@ -248,6 +248,12 @@ fn failed_surface_ensure_waits_for_a_new_key_before_retrying() -> Result<(), Str store.record_viewport_size(tab.id(), resized_viewport_bounds(), 1.0), WebSurfaceInputOutcome::Applied, ); + // `record_viewport_size` stamps the viewport-resize debounce on a + // genuine size transition; in production the poll cadence retries + // `ensure_surface` once the gesture settles, but a synchronous + // unit test can't advance the clock — clear the timestamp so we + // exercise the retry-on-new-key business rule in isolation. + store.clear_viewport_resize_debounce_for_test(tab.id()); assert!(store.ensure_surface(&tab, ProfileDataMode::Transient, &[])); store.flush_runtime_for_test(); let _ = store.tick(&[tab.id().clone()]); diff --git a/crates/ely_app/src/shell/web_surface_state.rs b/crates/ely_app/src/shell/web_surface_state.rs index f3fc3ed..0874059 100644 --- a/crates/ely_app/src/shell/web_surface_state.rs +++ b/crates/ely_app/src/shell/web_surface_state.rs @@ -126,6 +126,13 @@ pub(super) struct WebSurfaceTickResult { pub(super) struct PerTabSurface { pub(super) viewport_bounds: Option>, pub(super) viewport_size: Option, + /// When [`viewport_size`] last *transitioned* to a new value. The + /// initial measurement does not update this; only subsequent + /// genuine size changes do. Drives [`viewport_size_is_settling`] + /// so a sidebar / window animation that emits a viewport bounds on + /// every GPUI paint does not translate into a Servo framebuffer + /// destroy/recreate every frame (the "page flashing" symptom). + viewport_size_changed_at: Option, pub(super) native_surface: Option, pub(super) last_ensure_key: Option, pub(super) hover_point: Option, @@ -146,6 +153,7 @@ impl PerTabSurface { Self { viewport_bounds: None, viewport_size: None, + viewport_size_changed_at: None, native_surface: None, last_ensure_key: None, hover_point: None, @@ -188,6 +196,37 @@ impl PerTabSurface { self.last_ensure_key.as_ref() != Some(key) || self.has_pending_input() } + /// True when the viewport bounds have changed within the last + /// [`VIEWPORT_RESIZE_DEBOUNCE`] window. The renderer treats this + /// as "the user is mid-animation" and holds off telling Servo to + /// resize its rendering context until the gesture settles — + /// Servo's surfman backend recreates the framebuffer on every + /// `rendering_context.resize`, which would otherwise flash a + /// blank surface for every animation frame. + pub(super) fn viewport_size_is_settling(&self, now: Instant) -> bool { + self.viewport_size_changed_at + .is_some_and(|last| now.duration_since(last) < VIEWPORT_RESIZE_DEBOUNCE) + } + + /// Stamp the moment a genuine viewport size transition happened. + /// Only called when [`viewport_size`] is actually moving to a new + /// value — the very first measurement does *not* invoke this so + /// `viewport_size_is_settling` stays false at page-load time and + /// the initial `ensure_surface` is allowed to fire immediately. + pub(super) fn mark_viewport_size_changed(&mut self, now: Instant) { + self.viewport_size_changed_at = Some(now); + } + + /// Test-only escape hatch: simulate the trailing edge of a resize + /// animation by clearing the debounce timestamp, so a synchronous + /// `record + ensure` sequence in tests behaves as if the gesture + /// has fully settled. Production code drives this via the poll + /// cadence and time elapsing — tests can't advance an `Instant`. + #[cfg(test)] + pub(super) fn clear_viewport_resize_debounce_for_test(&mut self) { + self.viewport_size_changed_at = None; + } + pub(super) fn mark_ensured(&mut self, key: WebSurfaceEnsureKey) { self.last_ensure_key = Some(key); } @@ -226,6 +265,12 @@ impl PerTabSurface { const HOVER_INPUT_MIN_INTERVAL: Duration = Duration::from_millis(32); +/// How long the viewport must hold its new size before we propagate +/// the resize down to Servo. 80 ms is short enough that a user-driven +/// drag still feels live, and long enough to collapse a 60 Hz sidebar +/// animation (~17 ms per frame) into a single trailing-edge resize. +const VIEWPORT_RESIZE_DEBOUNCE: Duration = Duration::from_millis(80); + #[derive(Clone, Debug, Eq, PartialEq)] pub(super) struct WebSurfaceEnsureKey { requested_url: String, diff --git a/crates/ely_app/src/shell/web_surface_tests.rs b/crates/ely_app/src/shell/web_surface_tests.rs index f6b7ebf..a9e4ad5 100644 --- a/crates/ely_app/src/shell/web_surface_tests.rs +++ b/crates/ely_app/src/shell/web_surface_tests.rs @@ -71,6 +71,36 @@ fn viewport_size_changes_on_first_clean_measurement() -> Result<(), Box Result<(), Box> { + let mut store = WebSurfaceStore::new(); + let tab = web_tab("https://example.com/animation")?; + + assert_eq!( + store.record_viewport_size(tab.id(), web_bounds(), 1.0), + WebSurfaceInputOutcome::Applied, + "first measurement always fires immediately so the page can load", + ); + assert_eq!( + store.record_viewport_size(tab.id(), resized_once_bounds(), 1.0), + WebSurfaceInputOutcome::Applied, + "the first transition after an untimed measurement still fires once", + ); + assert_eq!( + store.record_viewport_size(tab.id(), web_bounds(), 1.0), + WebSurfaceInputOutcome::Buffered, + "a rapid second transition collapses into the trailing-edge resize", + ); + Ok(()) +} + /// Regression — a wheel scroll between focusing a field and typing /// must not erase keyboard focus or buffered keystrokes. ///