diff --git a/crates/ely_app/src/shell/gpui_harness_tests.rs b/crates/ely_app/src/shell/gpui_harness_tests.rs index d60fee6..a2c6ace 100644 --- a/crates/ely_app/src/shell/gpui_harness_tests.rs +++ b/crates/ely_app/src/shell/gpui_harness_tests.rs @@ -1033,6 +1033,7 @@ fn identical_live_frames_share_render_image_arc() -> Result<(), String> { Arc::as_ptr(first_image), Arc::as_ptr(second_image), ); + assert!(first.has_same_software_render_as(&second)); Ok(()) } diff --git a/crates/ely_app/src/shell/web_surface.rs b/crates/ely_app/src/shell/web_surface.rs index cbab5ab..3041a7e 100644 --- a/crates/ely_app/src/shell/web_surface.rs +++ b/crates/ely_app/src/shell/web_surface.rs @@ -6,17 +6,17 @@ use gpui::{Bounds, Pixels, Point}; use crate::services::ProfileDataMode; -use super::web_surface_metadata::WebSurfacePageMetadata; use super::{ web_surface_cadence::IDLE_POLL_INTERVAL, web_surface_frame::WebSurfaceFrame, web_surface_geometry::{WebSurfaceClickPoint, WebSurfaceScrollDelta, WebSurfaceSize}, + web_surface_metadata::WebSurfacePageMetadata, web_surface_permissions::WebSurfaceSitePermission, - web_surface_runtime::{WebSurfaceRuntime, WebSurfaceRuntimeFrame, WebSurfaceUrlChange}, + web_surface_runtime::{WebSurfaceRuntime, WebSurfaceRuntimeFrame}, web_surface_state::{ PerTabSurface, WebSurfaceClickState, WebSurfaceEnsureKey, WebSurfaceInputOutcome, WebSurfaceKeyboardFocusState, WebSurfacePendingInput, WebSurfaceScrollState, - WebSurfaceState, WebSurfaceTextInputState, + WebSurfaceState, WebSurfaceTextInputState, WebSurfaceTickResult, }, }; @@ -118,14 +118,18 @@ impl WebSurfaceStore { } continue; } - if let Some(metadata) = WebSurfacePageMetadata::from_frame(&tab_id, &frame) { - result.page_metadata.push(metadata); - } - self.surface_mut(&tab_id).state = Some(WebSurfaceState::Ready(*frame)); - result.changed = true; - if let Some(url_change) = url_change { - result.url_changes.push(url_change); + result + .page_metadata + .extend(WebSurfacePageMetadata::from_frame(&tab_id, &frame)); + if self + .surfaces + .get(&tab_id) + .is_none_or(|surface| !surface.matches_ready(&frame)) + { + self.surface_mut(&tab_id).state = Some(WebSurfaceState::Ready(*frame)); + result.changed = true; } + result.url_changes.extend(url_change); } WebSurfaceRuntimeFrame::Failed { tab_id, message } => { let had_ready = matches!( @@ -483,13 +487,6 @@ pub(super) fn is_external_web_url(url: &str) -> bool { url.starts_with("https://") || url.starts_with("http://") } -#[derive(Default)] -pub(super) struct WebSurfaceTickResult { - pub(super) changed: bool, - pub(super) url_changes: Vec, - pub(super) page_metadata: Vec, -} - #[cfg(test)] #[path = "web_surface_tests.rs"] mod tests; diff --git a/crates/ely_app/src/shell/web_surface_frame.rs b/crates/ely_app/src/shell/web_surface_frame.rs index 8d45c14..ebe8d11 100644 --- a/crates/ely_app/src/shell/web_surface_frame.rs +++ b/crates/ely_app/src/shell/web_surface_frame.rs @@ -14,24 +14,9 @@ use thiserror::Error; use crate::services::servo_live::ServoLiveFrame; thread_local! { - /// Single-slot cache of the most-recently-uploaded RGBA payload. - /// At 60 fps on a 1080p canvas the previous `from_parts` was - /// unconditionally building a fresh `Arc` for every - /// tick — even when the bytes were bit-identical to the last - /// frame. The cache keys on a 64-bit hash of the raw bytes and - /// reuses the existing `Arc` whenever the hash - /// matches, so steady-state idle pages no longer churn the GPUI - /// texture pool. - /// - /// Uses `AHasher` instead of std's `DefaultHasher`. SipHash13 - /// (default) tops out around ~1.5 GB/s; an 8 MB 1080p frame - /// hashes in ~5 ms on a modern CPU, which eats roughly 30 % of - /// the 16 ms scroll budget on every cache-miss frame. AHash - /// runs ~10 GB/s on the same hardware, dropping the per-frame - /// hash cost to ~0.8 ms and giving the scroll path back most of - /// that budget. Hash collisions remain ~1 in 2^64; if they ever - /// matter we'll trade in length + first/last 32 bytes as a - /// disambiguator before paying the full memcmp. + /// Single-slot cache for byte-identical software frames. + /// AHash keeps the 1080p hash pass below the frame budget while + /// avoiding repeated GPUI texture allocation for idle pages. static LAST_FRAME_IMAGE: RefCell)>> = const { RefCell::new(None) }; } @@ -62,12 +47,9 @@ pub(super) struct WebSurfaceFrame { content_pixel_count: u64, #[cfg(all(test, feature = "live-site-smoke"))] sample_hash: u64, - /// Software-path image built from RGBA bytes when the sidecar runs - /// without hardware surface publication. + /// Software-path image built from RGBA bytes. pub(super) image: Option>, - /// Hardware-path surface imported from the sidecar's IOSurface. - /// GPUI is patched locally to present BGRA CVPixelBuffers through - /// `surface(...)`, so hardware frames can skip the RGBA pipe. + /// Hardware-path IOSurface imported from the sidecar. #[cfg(target_os = "macos")] pub(super) pixel_buffer: Option, } @@ -231,10 +213,33 @@ impl WebSurfaceFrame { self.title.as_deref() } + pub(super) fn has_same_software_render_as(&self, other: &Self) -> bool { + #[cfg(target_os = "macos")] + if self.pixel_buffer.is_some() || other.pixel_buffer.is_some() { + return false; + } + let (Some(image), Some(other_image)) = (self.image.as_ref(), other.image.as_ref()) else { + return false; + }; + + Arc::ptr_eq(image, other_image) + && self.requested_url == other.requested_url + && self.loaded_url == other.loaded_url + && self.title == other.title + && self.render_state == other.render_state + && self.width == other.width + && self.height == other.height + && self.device_pixel_ratio == other.device_pixel_ratio + && self.css_viewport_width == other.css_viewport_width + && self.css_viewport_height == other.css_viewport_height + && self.scroll_offset == other.scroll_offset + && self.zoom_percent == other.zoom_percent + && self.click_point == other.click_point + && self.typed_text == other.typed_text + } + pub(super) fn has_visible_content_for_initial_display(&self) -> Result { - // The sidecar suppresses blank initial hardware frames with readback - // sampling before sending them. Keep this app-side gate metadata-only - // so GPUI's update path never locks or scans IOSurface memory. + // Sidecar readback suppresses blank initial frames before publication. #[cfg(all(test, feature = "live-site-smoke"))] { Ok(self.non_white_pixel_count > 0 && self.content_pixel_count > 0) 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 bf00811..82150b4 100644 --- a/crates/ely_app/src/shell/web_surface_runtime_tests.rs +++ b/crates/ely_app/src/shell/web_surface_runtime_tests.rs @@ -26,6 +26,7 @@ static FAKE_ENSURE_COUNT: AtomicUsize = AtomicUsize::new(0); static IDLE_SKIP_ENSURE_COUNT: AtomicUsize = AtomicUsize::new(0); static RECOVERY_FACTORY_COUNT: AtomicUsize = AtomicUsize::new(0); static FAILING_ENSURE_COUNT: AtomicUsize = AtomicUsize::new(0); +static REPEATED_FRAME_ENSURE_COUNT: AtomicUsize = AtomicUsize::new(0); #[test] fn runtime_keeps_independent_clients_for_profile_scopes() -> Result<(), String> { @@ -137,6 +138,48 @@ fn store_tick_delay_tracks_runtime_cadence() -> Result<(), String> { Ok(()) } +#[test] +fn identical_ready_software_frame_keeps_tick_unchanged() -> Result<(), String> { + REPEATED_FRAME_ENSURE_COUNT.store(0, Ordering::SeqCst); + let mut store = WebSurfaceStore::new_with_runtime(WebSurfaceRuntime::new_with_client_factory( + repeated_frame_client_factory, + )); + let tab = web_tab(TabId::new(), ProfileId::new(), "https://example.com/repeated")?; + let visible_tabs = vec![tab.id().clone()]; + + assert_eq!( + store.record_viewport_size(tab.id(), viewport_bounds(), 1.0), + WebSurfaceInputOutcome::Applied, + ); + assert!(store.ensure_surface(&tab, ProfileDataMode::Transient, &[])); + store.flush_runtime_for_test(); + let first_tick = store.tick(&visible_tabs); + + assert!(first_tick.changed); + assert_eq!(first_tick.page_metadata.len(), 1); + assert_eq!(first_tick.url_changes.len(), 1); + assert_eq!(REPEATED_FRAME_ENSURE_COUNT.load(Ordering::SeqCst), 1); + + assert_eq!( + store.record_click_point( + tab.id(), + tab.url().as_str(), + gpui::point(gpui::px(10.0), gpui::px(10.0)), + 1.0, + ), + WebSurfaceInputOutcome::Applied, + ); + let _ = store.ensure_surface(&tab, ProfileDataMode::Transient, &[]); + store.flush_runtime_for_test(); + let second_tick = store.tick(&visible_tabs); + + assert!(!second_tick.changed); + assert_eq!(second_tick.page_metadata.len(), 1); + assert_eq!(second_tick.url_changes.len(), 1); + assert_eq!(REPEATED_FRAME_ENSURE_COUNT.load(Ordering::SeqCst), 2); + Ok(()) +} + #[test] fn sidecar_exit_removes_dead_runtime_client() -> Result<(), String> { RECOVERY_FACTORY_COUNT.store(0, Ordering::SeqCst); @@ -238,6 +281,7 @@ struct FakeLiveRuntimeClient; struct IdleSkipLiveRuntimeClient; struct SidecarExitLiveRuntimeClient; struct FailingLiveRuntimeClient; +struct RepeatedFrameLiveRuntimeClient; impl LiveRuntimeClient for FakeLiveRuntimeClient { fn ensure( @@ -311,6 +355,24 @@ impl LiveRuntimeClient for FailingLiveRuntimeClient { } } +impl LiveRuntimeClient for RepeatedFrameLiveRuntimeClient { + fn ensure( + &mut self, + _request: ServoLiveEnsureRequest, + ) -> Result, LiveRuntimeClientError> { + REPEATED_FRAME_ENSURE_COUNT.fetch_add(1, Ordering::SeqCst); + Ok(Some(repeated_live_frame())) + } + + fn poll(&mut self, _tab_id: String) -> Result, LiveRuntimeClientError> { + Ok(None) + } + + fn close(&mut self, _tab_id: String) -> Result<(), LiveRuntimeClientError> { + Ok(()) + } +} + fn fake_client_factory( _config_dir: std::path::PathBuf, ) -> Result, String> { @@ -339,6 +401,16 @@ fn failing_client_factory( Ok(Box::new(FailingLiveRuntimeClient)) } +fn repeated_frame_client_factory( + _config_dir: std::path::PathBuf, +) -> Result, String> { + Ok(Box::new(RepeatedFrameLiveRuntimeClient)) +} + +fn repeated_live_frame() -> ServoLiveFrame { + ServoLiveFrame::for_test(1, 1, vec![16, 32, 64, 255]) +} + fn web_tab(tab_id: TabId, profile_id: ProfileId, url: &str) -> Result { let url = UrlText::parse(url).map_err(|error| error.to_string())?; Ok(BrowserTab::new(tab_id, SpaceId::new(), profile_id, "Web", url)) diff --git a/crates/ely_app/src/shell/web_surface_state.rs b/crates/ely_app/src/shell/web_surface_state.rs index 5d0e870..3bf460c 100644 --- a/crates/ely_app/src/shell/web_surface_state.rs +++ b/crates/ely_app/src/shell/web_surface_state.rs @@ -9,7 +9,9 @@ use super::{ web_surface_geometry::{ WebSurfaceClickPoint, WebSurfaceScrollDelta, WebSurfaceScrollOffset, WebSurfaceSize, }, + web_surface_metadata::WebSurfacePageMetadata, web_surface_permissions::WebSurfaceSitePermission, + web_surface_runtime::WebSurfaceUrlChange, }; pub(super) struct WebSurfaceScrollState { @@ -101,6 +103,13 @@ pub(super) enum WebSurfaceState { Failed { message: String }, } +#[derive(Default)] +pub(super) struct WebSurfaceTickResult { + pub(super) changed: bool, + pub(super) url_changes: Vec, + pub(super) page_metadata: Vec, +} + /// All per-tab surface invariants in one owner. /// /// Replaces the previous 11 parallel `BTreeMap` fields on @@ -179,6 +188,13 @@ impl PerTabSurface { self.last_ensure_key = Some(key); } + pub(super) fn matches_ready(&self, frame: &WebSurfaceFrame) -> bool { + match self.state.as_ref() { + Some(WebSurfaceState::Ready(current)) => current.has_same_software_render_as(frame), + _ => false, + } + } + fn has_pending_input(&self) -> bool { self.hover_point.is_some() || self.click_point.is_some()