From 90c029eddbcdc9ecfb8df1d6871d967a650356ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Fri, 15 May 2026 16:41:40 -0400 Subject: [PATCH] Move Servo IPC off UI thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of the post-tab lag: the GPUI 16 ms timer was calling `WebSurfaceRuntime::ensure_tab` and `tick` on the UI thread, and each call did a synchronous `serde_json` write plus `read_line` against the Servo sidecar over stdin/stdout. With even one visible tab, every frame stalled on cross-process IPC. Introduce `web_surface_worker.rs` — a per-profile worker thread that owns the `ServoLiveClient`, drains a coalescing request queue (latest Ensure/Poll per tab wins, no unbounded growth), and ships results back through a `std::sync::mpsc` channel. `WebSurfaceRuntime` now submits work non-blockingly and drains responses in `tick`; the UI thread never blocks on the sidecar. Adjacent in-flight cleanup riding along: hardware IOSurface rendering-context completion (sidecar `live_protocol`, `hardware_rendering_context`, GPUI BGRA surface shader), CSS viewport size + device pixel ratio plumbing into `ServoLiveFrame`, and the Send opt-ins for `CVPixelBuffer`-bearing types so frames can cross the thread boundary. --- crates/ely_app/src/main.rs | 14 +- .../ely_app/src/services/iosurface_metal.rs | 9 + crates/ely_app/src/services/servo_live.rs | 66 ++++ .../ely_app/src/services/servo_live_wire.rs | 10 + .../src/services/servo_sidecar_command.rs | 28 +- crates/ely_app/src/shell/mod.rs | 14 +- crates/ely_app/src/shell/web_surface.rs | 127 +++++-- .../src/shell/web_surface_controller.rs | 130 +++++-- crates/ely_app/src/shell/web_surface_frame.rs | 166 +++++++- .../src/shell/web_surface_live_site_tests.rs | 169 ++++++++- .../ely_app/src/shell/web_surface_runtime.rs | 354 +++++++++--------- .../src/shell/web_surface_runtime_tests.rs | 213 ++++++++++- crates/ely_app/src/shell/web_surface_state.rs | 101 ++++- crates/ely_app/src/shell/web_surface_tests.rs | 61 ++- crates/ely_app/src/shell/web_surface_view.rs | 20 +- .../ely_app/src/shell/web_surface_worker.rs | 336 +++++++++++++++++ .../src/bin/ely_servo_sidecar/live.rs | 140 ++++--- .../src/bin/ely_servo_sidecar/live_output.rs | 52 ++- .../bin/ely_servo_sidecar/live_protocol.rs | 34 +- .../src/hardware_rendering_context.rs | 118 +++--- crates/ely_servo_host/src/lib.rs | 10 +- crates/ely_servo_host/src/runtime.rs | 13 +- .../ely_servo_host/tests/live_perf_bench.rs | 181 +++++---- crates/ely_servo_host/tests/software_host.rs | 161 +++++++- third_party/gpui/src/elements/surface.rs | 21 +- .../gpui/src/platform/mac/metal_renderer.rs | 13 +- .../gpui/src/platform/mac/shaders.metal | 38 +- third_party/gpui/src/scene.rs | 1 + third_party/gpui/src/window.rs | 9 +- 29 files changed, 2113 insertions(+), 496 deletions(-) create mode 100644 crates/ely_app/src/shell/web_surface_worker.rs diff --git a/crates/ely_app/src/main.rs b/crates/ely_app/src/main.rs index ca06df5..018e59a 100644 --- a/crates/ely_app/src/main.rs +++ b/crates/ely_app/src/main.rs @@ -11,6 +11,7 @@ use std::{ time::Duration, }; +use ely_design_system::spacing; use ely_domain::UrlText; use gpui::{ AnyWindowHandle, App, AppContext, Application, Bounds, Entity, Focusable, Menu, MenuItem, @@ -51,6 +52,11 @@ actions!( ] ); +// Measured from the window's top-left to the close button origin. +// Places the macOS traffic lights inside the calm part of the corner curve. +const TRAFFIC_LIGHT_ORIGIN_X: f32 = spacing::SHELL_INSET + 34.0; +const TRAFFIC_LIGHT_ORIGIN_Y: f32 = spacing::SHELL_INSET + 22.0; + fn main() { init_tracing(); let pending_deep_links = PendingDeepLinks::default(); @@ -73,8 +79,7 @@ fn main() { // exactly the "fonts still wrong" the screenshot showed. // Override the theme so every gpui-component sub-element uses // Geist too. - gpui_component::Theme::global_mut(cx).font_family = - shell::chrome::SANS_FAMILY.into(); + gpui_component::Theme::global_mut(cx).font_family = shell::chrome::SANS_FAMILY.into(); cx.on_action(quit); bind_shortcuts(cx); cx.on_action(open_private_window); @@ -185,7 +190,10 @@ fn open_browser_window(cx: &mut App, mode: BrowserWindowMode) -> Option, } +// SAFETY: CVPixelBuffer wraps CVPixelBufferRef, a CoreFoundation type +// Apple documents as safe to share across threads. The cache is owned +// by ServoLiveClient which now lives on the LiveRuntimeWorker thread, +// so the auto-Send check (rightly) rejects the raw pointer inside the +// crate's `CVPixelBuffer`. The pointer is atomically refcounted CFTypeRef +// and only mutated via Mach IPC, which is itself thread-safe. +#[expect(unsafe_code)] +unsafe impl Send for IOSurfaceCache {} + struct CachedPixelBuffer { pixel_buffer: CVPixelBuffer, width: u32, diff --git a/crates/ely_app/src/services/servo_live.rs b/crates/ely_app/src/services/servo_live.rs index eaf5c77..b552f23 100644 --- a/crates/ely_app/src/services/servo_live.rs +++ b/crates/ely_app/src/services/servo_live.rs @@ -289,6 +289,9 @@ pub(crate) struct ServoLiveFrame { render_state: String, width: u32, height: u32, + device_pixel_ratio: f32, + css_viewport_width: u32, + css_viewport_height: u32, #[cfg(all(test, feature = "live-site-smoke"))] non_white_pixel_count: u64, #[cfg(all(test, feature = "live-site-smoke"))] @@ -302,14 +305,26 @@ pub(crate) struct ServoLiveFrame { pixel_buffer: Option, } +// SAFETY: CVPixelBuffer wraps CVPixelBufferRef, a CoreFoundation type +// Apple documents as safe to share across threads. The Rust core-video +// crate does not mark it Send, so the worker thread needs this opt-in +// to ship hardware frames back to the UI thread via mpsc::Sender. +#[cfg(target_os = "macos")] +#[expect(unsafe_code)] +unsafe impl Send for ServoLiveFrame {} + impl ServoLiveFrame { fn from_parts(report: LiveFrameReport, rgba_bytes: Vec) -> Self { + let (css_viewport_width, css_viewport_height) = css_viewport_size_from_report(&report); Self { loaded_url: report.loaded_url, title: report.title, render_state: report.state, width: report.width, height: report.height, + device_pixel_ratio: report.device_pixel_ratio, + css_viewport_width, + css_viewport_height, #[cfg(all(test, feature = "live-site-smoke"))] non_white_pixel_count: report.non_white_pixel_count, #[cfg(all(test, feature = "live-site-smoke"))] @@ -355,6 +370,21 @@ impl ServoLiveFrame { self.height } + #[must_use] + pub fn device_pixel_ratio(&self) -> f32 { + self.device_pixel_ratio + } + + #[must_use] + pub fn css_viewport_width(&self) -> u32 { + self.css_viewport_width + } + + #[must_use] + pub fn css_viewport_height(&self) -> u32 { + self.css_viewport_height + } + #[cfg(all(test, feature = "live-site-smoke"))] #[must_use] pub fn non_white_pixel_count(&self) -> u64 { @@ -386,6 +416,9 @@ impl ServoLiveFrame { render_state: "complete".to_string(), width, height, + device_pixel_ratio: 1.0, + css_viewport_width: width, + css_viewport_height: height, #[cfg(all(test, feature = "live-site-smoke"))] non_white_pixel_count: 0, #[cfg(all(test, feature = "live-site-smoke"))] @@ -410,6 +443,9 @@ impl ServoLiveFrame { render_state: "complete".to_string(), width, height, + device_pixel_ratio: 1.0, + css_viewport_width: width, + css_viewport_height: height, #[cfg(all(test, feature = "live-site-smoke"))] non_white_pixel_count: 0, #[cfg(all(test, feature = "live-site-smoke"))] @@ -422,6 +458,20 @@ impl ServoLiveFrame { } } +fn css_viewport_size_from_report(report: &LiveFrameReport) -> (u32, u32) { + let dpr = if report.device_pixel_ratio.is_finite() && report.device_pixel_ratio > 0.0 { + report.device_pixel_ratio + } else { + 1.0 + }; + let fallback_width = ((report.width as f32) / dpr).round().max(1.0) as u32; + let fallback_height = ((report.height as f32) / dpr).round().max(1.0) as u32; + ( + if report.css_viewport_width > 0 { report.css_viewport_width } else { fallback_width }, + if report.css_viewport_height > 0 { report.css_viewport_height } else { fallback_height }, + ) +} + #[derive(Debug, Error)] pub(crate) enum ServoLiveError { #[error("servo sidecar binary is unavailable at {path}")] @@ -469,3 +519,19 @@ pub(crate) enum ServoLiveError { #[error(transparent)] SidecarCommand(#[from] SidecarCommandError), } + +impl ServoLiveError { + pub(crate) fn is_sidecar_process_unusable(&self) -> bool { + match self { + Self::SidecarExited => true, + Self::Command(error) | Self::FrameRead(error) => matches!( + error.kind(), + io::ErrorKind::BrokenPipe + | io::ErrorKind::ConnectionAborted + | io::ErrorKind::ConnectionReset + | io::ErrorKind::UnexpectedEof + ), + _ => false, + } + } +} diff --git a/crates/ely_app/src/services/servo_live_wire.rs b/crates/ely_app/src/services/servo_live_wire.rs index 1cfbee5..27d32f2 100644 --- a/crates/ely_app/src/services/servo_live_wire.rs +++ b/crates/ely_app/src/services/servo_live_wire.rs @@ -96,6 +96,12 @@ pub(super) struct LiveFrameReport { pub(super) state: String, pub(super) width: u32, pub(super) height: u32, + #[serde(default = "default_device_pixel_ratio")] + pub(super) device_pixel_ratio: f32, + #[serde(default)] + pub(super) css_viewport_width: u32, + #[serde(default)] + pub(super) css_viewport_height: u32, pub(super) rgba_byte_count: usize, #[cfg(all(test, feature = "live-site-smoke"))] pub(super) non_white_pixel_count: u64, @@ -105,6 +111,10 @@ pub(super) struct LiveFrameReport { pub(super) sample_hash: u64, } +fn default_device_pixel_ratio() -> f32 { + 1.0 +} + /// Per-frame tag that tells the renderer which already-imported /// `MTLTexture` to sample. Emitted at `trace` instead of `info` because /// it fires every frame on the hardware path; the import event above diff --git a/crates/ely_app/src/services/servo_sidecar_command.rs b/crates/ely_app/src/services/servo_sidecar_command.rs index 57f04a5..4005335 100644 --- a/crates/ely_app/src/services/servo_sidecar_command.rs +++ b/crates/ely_app/src/services/servo_sidecar_command.rs @@ -89,14 +89,19 @@ pub(super) fn default_sidecar_command() -> Result String { format!("ely_servo_sidecar{}", env::consts::EXE_SUFFIX) } +fn is_macos_app_bundle_exe_dir(path: &Path) -> bool { + path.file_name().is_some_and(|name| name == "MacOS") + && path + .parent() + .is_some_and(|contents| contents.file_name().is_some_and(|name| name == "Contents")) + && path + .parent() + .and_then(Path::parent) + .is_some_and(|bundle| bundle.extension().is_some_and(|extension| extension == "app")) +} + #[cfg(test)] mod tests { use super::{ HARDWARE_SIDECAR_FEATURES, SOFTWARE_SIDECAR_FEATURES, SidecarRenderingContext, - rendering_context_selection, + is_macos_app_bundle_exe_dir, rendering_context_selection, }; #[test] @@ -180,6 +196,14 @@ mod tests { assert_eq!(context.sidecar_features(), SOFTWARE_SIDECAR_FEATURES); } + #[test] + fn recognizes_macos_app_bundle_executable_directory() { + assert!(is_macos_app_bundle_exe_dir(std::path::Path::new( + "/tmp/ELY Browser.app/Contents/MacOS" + ))); + assert!(!is_macos_app_bundle_exe_dir(std::path::Path::new("/tmp/target/debug"))); + } + #[cfg(target_os = "macos")] #[test] fn defaults_to_hardware_rendering_context_on_macos() { diff --git a/crates/ely_app/src/shell/mod.rs b/crates/ely_app/src/shell/mod.rs index 363a435..0c05d1c 100644 --- a/crates/ely_app/src/shell/mod.rs +++ b/crates/ely_app/src/shell/mod.rs @@ -32,6 +32,7 @@ mod web_surface_permissions; mod web_surface_runtime; mod web_surface_state; mod web_surface_view; +mod web_surface_worker; #[cfg(test)] mod gpui_harness_tests; @@ -116,9 +117,8 @@ impl ElyShell { window: &mut Window, cx: &mut Context, ) -> Self { - let command_input = cx.new(|cx| { - InputState::new(window, cx).placeholder("Search ELY or type a command…") - }); + let command_input = + cx.new(|cx| InputState::new(window, cx).placeholder("Search ELY or type a command…")); let plugin_search_input = cx.new(|cx| InputState::new(window, cx).placeholder("Search plugins…")); let translucency_slider = cx.new(|_cx| { @@ -129,14 +129,16 @@ impl ElyShell { .default_value(f32::from(DEFAULT_TRANSLUCENCY_PCT)) }); - let translucency_subscription = - cx.subscribe(&translucency_slider, |shell: &mut Self, _state, event: &SliderEvent, cx| { + let translucency_subscription = cx.subscribe( + &translucency_slider, + |shell: &mut Self, _state, event: &SliderEvent, cx| { let SliderEvent::Change(SliderValue::Single(value)) = event else { return; }; let pct = value.clamp(0.0, 100.0).round() as u8; shell.set_translucency_pct(pct, cx); - }); + }, + ); let command_subscription = cx.subscribe_in( &command_input, diff --git a/crates/ely_app/src/shell/web_surface.rs b/crates/ely_app/src/shell/web_surface.rs index 009bba8..44a0ada 100644 --- a/crates/ely_app/src/shell/web_surface.rs +++ b/crates/ely_app/src/shell/web_surface.rs @@ -11,8 +11,9 @@ use super::{ web_surface_permissions::WebSurfaceSitePermission, web_surface_runtime::{WebSurfaceRuntime, WebSurfaceRuntimeFrame, WebSurfaceUrlChange}, web_surface_state::{ - PerTabSurface, WebSurfaceClickState, WebSurfaceInputOutcome, WebSurfaceKeyboardFocusState, - WebSurfacePendingInput, WebSurfaceScrollState, WebSurfaceState, WebSurfaceTextInputState, + PerTabSurface, WebSurfaceClickState, WebSurfaceEnsureKey, WebSurfaceInputOutcome, + WebSurfaceKeyboardFocusState, WebSurfacePendingInput, WebSurfaceScrollState, + WebSurfaceState, WebSurfaceTextInputState, }, }; @@ -30,6 +31,11 @@ impl WebSurfaceStore { Self { runtime: WebSurfaceRuntime::new(), surfaces: BTreeMap::new(), keyboard_focus: None } } + #[cfg(test)] + pub(super) fn new_with_runtime(runtime: WebSurfaceRuntime) -> Self { + Self { runtime, surfaces: BTreeMap::new(), keyboard_focus: None } + } + pub(super) fn state(&self, tab_id: &TabId) -> Option<&WebSurfaceState> { self.surfaces.get(tab_id).and_then(|surface| surface.state.as_ref()) } @@ -39,37 +45,41 @@ impl WebSurfaceStore { tab: &BrowserTab, profile_data_mode: ProfileDataMode, permissions: &[WebSurfaceSitePermission], - ) -> Option { + ) -> WebSurfaceEnsureOutcome { if !is_external_web_url(tab.url().as_str()) { - return None; + return WebSurfaceEnsureOutcome::default(); } let requested_url = tab.url().as_str().to_string(); - let size = self.surfaces.get(tab.id()).and_then(|surface| surface.viewport_size)?; + let Some(size) = self.surfaces.get(tab.id()).and_then(|surface| surface.viewport_size) + else { + return WebSurfaceEnsureOutcome::default(); + }; + let ensure_key = + WebSurfaceEnsureKey::new(requested_url.clone(), size, tab.zoom_percent(), permissions); + if self.surfaces.get(tab.id()).is_some_and(|surface| !surface.should_ensure(&ensure_key)) { + return WebSurfaceEnsureOutcome::default(); + } 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()); match self.runtime.ensure_tab(tab, size, profile_data_mode, permissions, input) { - Ok(result) if result.frame.is_some() => { - let url_change = result.url_change; - let Some(frame) = result.frame else { - return url_change; - }; - self.surface_mut(tab.id()).state = Some(WebSurfaceState::Ready(frame)); - url_change + Ok(result) => { + self.surface_mut(tab.id()).mark_ensured(ensure_key); + if result.started_loading { + self.surface_mut(tab.id()).state = Some(WebSurfaceState::Loading { + requested_url: result.requested_url, + previous_frame, + }); + return WebSurfaceEnsureOutcome { changed: true, url_change: None }; + } + WebSurfaceEnsureOutcome::default() } - Ok(result) if result.started_loading => { - self.surface_mut(tab.id()).state = Some(WebSurfaceState::Loading { - requested_url: result.requested_url, - previous_frame, - }); - result.url_change - } - Ok(result) => result.url_change, Err(message) => { + self.surface_mut(tab.id()).mark_ensured(ensure_key); self.surface_mut(tab.id()).state = Some(WebSurfaceState::Failed { message }); - None + WebSurfaceEnsureOutcome { changed: true, url_change: None } } } } @@ -81,6 +91,23 @@ impl WebSurfaceStore { for frame in frames { match frame { WebSurfaceRuntimeFrame::Ready { tab_id, frame, url_change } => { + match self.initial_display_gate_message(&tab_id, &frame, false) { + Ok(()) => {} + Err(message) => { + self.surface_mut(&tab_id).state = + Some(WebSurfaceState::Failed { message }); + result.changed = true; + continue; + } + } + if self.should_hold_initial_frame(&tab_id, &frame, false) { + self.surface_mut(&tab_id).state = Some(WebSurfaceState::Loading { + requested_url: frame.requested_url.clone(), + previous_frame: None, + }); + result.changed = true; + continue; + } self.surface_mut(&tab_id).state = Some(WebSurfaceState::Ready(*frame)); result.changed = true; if let Some(url_change) = url_change { @@ -148,6 +175,7 @@ impl WebSurfaceStore { None => delta, }); surface.pending_scroll_point = Some(point); + surface.mark_pending_input_started(); // Drop any buffered click — its viewport coordinates were // captured against the pre-scroll page, so applying it after // the scroll would land on the wrong DOM element. Keep @@ -172,21 +200,13 @@ impl WebSurfaceStore { let Some(current_size) = surface.viewport_size else { surface.viewport_size = Some(size); - surface.pending_viewport_size = None; return WebSurfaceInputOutcome::Applied; }; if current_size == size { - surface.pending_viewport_size = None; return WebSurfaceInputOutcome::NoChange; } - if surface.pending_viewport_size != Some(size) { - surface.pending_viewport_size = Some(size); - return WebSurfaceInputOutcome::Buffered; - } - - surface.pending_viewport_size = None; surface.viewport_size = Some(size); WebSurfaceInputOutcome::Applied } @@ -246,6 +266,7 @@ impl WebSurfaceStore { let surface = self.surface_mut(tab_id); surface.typed_text = None; surface.click_point = Some(state); + surface.mark_pending_input_started(); WebSurfaceInputOutcome::Applied } @@ -288,6 +309,7 @@ impl WebSurfaceStore { } entry.text.push_str(text); + surface.mark_pending_input_started(); WebSurfaceInputOutcome::Applied } @@ -313,8 +335,10 @@ impl WebSurfaceStore { .filter(|state| state.requested_url == requested_url) .map(|state| state.text); let hover_point = surface.hover_point.take(); + let enqueued_at = surface.pending_input_started_at.take(); WebSurfacePendingInput { + enqueued_at, scroll_offset, scroll_delta, scroll_point, @@ -346,6 +370,40 @@ impl WebSurfaceStore { } } + fn should_hold_initial_frame( + &self, + tab_id: &TabId, + frame: &WebSurfaceFrame, + has_previous_frame: bool, + ) -> bool { + !has_previous_frame + && self + .previous_ready_frame(tab_id, frame.requested_url.as_str(), frame.zoom_percent()) + .is_none() + && matches!(frame.has_visible_content_for_initial_display(), Ok(false)) + } + + fn initial_display_gate_message( + &self, + tab_id: &TabId, + frame: &WebSurfaceFrame, + has_previous_frame: bool, + ) -> Result<(), String> { + if has_previous_frame + || self + .previous_ready_frame(tab_id, frame.requested_url.as_str(), frame.zoom_percent()) + .is_some() + { + return Ok(()); + } + frame.has_visible_content_for_initial_display().map(|_| ()).map_err(|error| { + format!( + "Servo hardware surface initial content check failed for {}: {error}", + frame.requested_url + ) + }) + } + fn surface_mut(&mut self, tab_id: &TabId) -> &mut PerTabSurface { self.surfaces.entry(tab_id.clone()).or_insert_with(PerTabSurface::new) } @@ -362,12 +420,23 @@ impl WebSurfaceStore { pub(super) fn surface_for_test(&self, tab_id: &TabId) -> Option<&PerTabSurface> { self.surfaces.get(tab_id) } + + #[cfg(test)] + pub(super) fn flush_runtime_for_test(&self) { + self.runtime.flush_for_test(); + } } pub(super) fn is_external_web_url(url: &str) -> bool { url.starts_with("https://") || url.starts_with("http://") } +#[derive(Default)] +pub(super) struct WebSurfaceEnsureOutcome { + pub(super) changed: bool, + pub(super) url_change: Option, +} + #[derive(Default)] pub(super) struct WebSurfaceTickResult { pub(super) changed: bool, diff --git a/crates/ely_app/src/shell/web_surface_controller.rs b/crates/ely_app/src/shell/web_surface_controller.rs index 4554e01..64e94a2 100644 --- a/crates/ely_app/src/shell/web_surface_controller.rs +++ b/crates/ely_app/src/shell/web_surface_controller.rs @@ -22,16 +22,8 @@ impl ElyShell { cx: &mut Context, ) -> AnyElement { let state_entity = cx.entity().clone(); - let Some(profile_data_mode) = profile_data_mode_for(tab, snapshot) else { + if profile_data_mode_for(tab, snapshot).is_none() { return render_failed_web_surface(tab, "Profile context is unavailable.", state_entity); - }; - - let permissions = web_surface_site_permissions_for_tab(tab, snapshot); - if let Some(url_change) = - self.web_surfaces.ensure_surface(tab, profile_data_mode, &permissions) - && self.apply_web_surface_url_change(url_change) - { - cx.notify(); } match self.web_surfaces.state(tab.id()) { @@ -51,15 +43,20 @@ impl ElyShell { } pub(super) fn tick_external_web_surfaces(&mut self) -> bool { - let (visible_tab_ids, open_tab_ids) = match &self.state { - super::ShellState::Ready(core) => { - (core.visible_content_tab_ids().unwrap_or_else(|_| Vec::new()), core.open_tab_ids()) - } - super::ShellState::StartupError(_) => (Vec::new(), Vec::new()), + let (visible_tab_ids, open_tab_ids, snapshot) = match &self.state { + super::ShellState::Ready(core) => ( + core.visible_content_tab_ids().unwrap_or_else(|_| Vec::new()), + core.open_tab_ids(), + core.snapshot().ok(), + ), + super::ShellState::StartupError(_) => (Vec::new(), Vec::new(), None), }; self.web_surfaces.retain_tabs(&open_tab_ids); + let mut url_changed = snapshot + .as_ref() + .map(|snapshot| self.ensure_visible_web_surfaces(snapshot, &visible_tab_ids)) + .unwrap_or(false); let result = self.web_surfaces.tick(&visible_tab_ids); - let mut url_changed = false; for url_change in result.url_changes { url_changed |= self.apply_web_surface_url_change(url_change); } @@ -71,13 +68,9 @@ impl ElyShell { tab_id: TabId, bounds: Bounds, scale_factor: f32, - cx: &mut Context, + _cx: &mut Context, ) { - if self.web_surfaces.record_viewport_size(&tab_id, bounds, scale_factor) - == WebSurfaceInputOutcome::Applied - { - cx.notify(); - } + let _ = self.web_surfaces.record_viewport_size(&tab_id, bounds, scale_factor); } pub(super) fn scroll_external_web_viewport( @@ -87,18 +80,15 @@ impl ElyShell { delta: Point, position: Point, scale_factor: f32, - cx: &mut Context, + _cx: &mut Context, ) { - if self.web_surfaces.record_scroll_delta( + let _ = self.web_surfaces.record_scroll_delta( &tab_id, requested_url.as_str(), delta, position, scale_factor, - ) == WebSurfaceInputOutcome::Applied - { - cx.notify(); - } + ); } pub(super) fn hover_external_web_viewport( @@ -106,13 +96,9 @@ impl ElyShell { tab_id: TabId, position: Point, scale_factor: f32, - cx: &mut Context, + _cx: &mut Context, ) { - if self.web_surfaces.record_hover_point(&tab_id, position, scale_factor) - == WebSurfaceInputOutcome::Applied - { - cx.notify(); - } + let _ = self.web_surfaces.record_hover_point(&tab_id, position, scale_factor); } pub(super) fn click_external_web_viewport( @@ -121,19 +107,16 @@ impl ElyShell { requested_url: String, position: Point, window: &mut gpui::Window, - cx: &mut Context, + _cx: &mut Context, ) { self.focus_handle.focus(window); let scale_factor = window.scale_factor(); - if self.web_surfaces.record_click_point( + let _ = self.web_surfaces.record_click_point( &tab_id, requested_url.as_str(), position, scale_factor, - ) == WebSurfaceInputOutcome::Applied - { - cx.notify(); - } + ); } /// Hand focus to the shell's root focus handle so subsequent @@ -149,12 +132,11 @@ impl ElyShell { tab_id: TabId, requested_url: String, text: &str, - cx: &mut Context, + _cx: &mut Context, ) -> bool { if self.web_surfaces.record_typed_text(&tab_id, requested_url.as_str(), text) == WebSurfaceInputOutcome::Applied { - cx.notify(); return true; } @@ -163,6 +145,28 @@ impl ElyShell { } impl ElyShell { + fn ensure_visible_web_surfaces( + &mut self, + snapshot: &BrowserSnapshot, + visible_tab_ids: &[TabId], + ) -> bool { + let mut url_changed = false; + let mut changed = false; + let visible_tabs = visible_external_web_tabs(&snapshot.tabs, visible_tab_ids); + for tab in visible_tabs { + let Some(profile_data_mode) = profile_data_mode_for(tab, snapshot) else { + continue; + }; + let permissions = web_surface_site_permissions_for_tab(tab, snapshot); + let outcome = self.web_surfaces.ensure_surface(tab, profile_data_mode, &permissions); + changed |= outcome.changed; + if let Some(url_change) = outcome.url_change { + url_changed |= self.apply_web_surface_url_change(url_change); + } + } + changed || url_changed + } + fn apply_web_surface_url_change(&mut self, change: WebSurfaceUrlChange) -> bool { let Ok(url) = UrlText::parse(change.loaded_url) else { return false; @@ -182,6 +186,17 @@ impl ElyShell { } } +fn visible_external_web_tabs<'a>( + tabs: &'a [BrowserTab], + visible_tab_ids: &[TabId], +) -> Vec<&'a BrowserTab> { + visible_tab_ids + .iter() + .filter_map(|tab_id| tabs.iter().find(|tab| tab.id() == tab_id)) + .filter(|tab| super::web_surface::is_external_web_url(tab.url().as_str())) + .collect() +} + fn profile_data_mode_for(tab: &BrowserTab, snapshot: &BrowserSnapshot) -> Option { snapshot.profiles.iter().find(|profile| profile.id() == tab.profile_id()).map(|profile| { match profile.kind() { @@ -190,3 +205,36 @@ fn profile_data_mode_for(tab: &BrowserTab, snapshot: &BrowserSnapshot) -> Option } }) } + +#[cfg(test)] +mod tests { + use ely_domain::{BrowserTab, ProfileId, SpaceId, TabId, UrlText}; + + use super::visible_external_web_tabs; + + #[test] + fn visible_external_web_tabs_follow_visible_order() -> Result<(), String> { + let first_id = TabId::new(); + let second_id = TabId::new(); + let internal_id = TabId::new(); + let tabs = vec![ + web_tab(first_id.clone(), "https://example.com/first")?, + web_tab(internal_id.clone(), "ely://settings")?, + web_tab(second_id.clone(), "http://example.com/second")?, + ]; + + let visible = + visible_external_web_tabs(&tabs, &[internal_id, second_id.clone(), first_id.clone()]) + .into_iter() + .map(|tab| tab.id().clone()) + .collect::>(); + + assert_eq!(visible, vec![second_id, first_id]); + Ok(()) + } + + fn web_tab(tab_id: TabId, url: &str) -> Result { + let url = UrlText::parse(url).map_err(|error| error.to_string())?; + Ok(BrowserTab::new(tab_id, SpaceId::new(), ProfileId::new(), "Web", url)) + } +} diff --git a/crates/ely_app/src/shell/web_surface_frame.rs b/crates/ely_app/src/shell/web_surface_frame.rs index 2b58adf..a084442 100644 --- a/crates/ely_app/src/shell/web_surface_frame.rs +++ b/crates/ely_app/src/shell/web_surface_frame.rs @@ -4,7 +4,10 @@ use std::sync::Arc; use ahash::AHasher; #[cfg(target_os = "macos")] -use core_video::pixel_buffer::{CVPixelBuffer, kCVPixelFormatType_32BGRA}; +use core_video::{ + pixel_buffer::{CVPixelBuffer, kCVPixelBufferLock_ReadOnly, kCVPixelFormatType_32BGRA}, + r#return::kCVReturnSuccess, +}; use gpui::RenderImage; use image::{ImageBuffer, Rgba}; use thiserror::Error; @@ -47,6 +50,9 @@ pub(super) struct WebSurfaceFrame { render_state: String, width: u32, height: u32, + device_pixel_ratio: f32, + css_viewport_width: u32, + css_viewport_height: u32, scroll_offset: WebSurfaceScrollOffset, zoom_percent: u16, click_point: Option, @@ -83,6 +89,9 @@ impl WebSurfaceFrame { render_state: frame.render_state().to_string(), width: frame.width(), height: frame.height(), + device_pixel_ratio: frame.device_pixel_ratio(), + css_viewport_width: frame.css_viewport_width(), + css_viewport_height: frame.css_viewport_height(), scroll_offset, zoom_percent, click_point: None, @@ -114,6 +123,9 @@ impl WebSurfaceFrame { validate_hardware_pixel_buffer(pixel_buffer, parts.width, parts.height)?; } + #[cfg(all(test, feature = "live-site-smoke"))] + let pixel_sample = pixel_sample_for_parts(&parts)?; + let image = if parts.rgba_bytes.is_empty() { None } else { @@ -139,16 +151,19 @@ impl WebSurfaceFrame { render_state: parts.render_state, width: parts.width, height: parts.height, + device_pixel_ratio: parts.device_pixel_ratio, + css_viewport_width: parts.css_viewport_width, + css_viewport_height: parts.css_viewport_height, scroll_offset: parts.scroll_offset, zoom_percent: parts.zoom_percent, click_point: parts.click_point, typed_text: parts.typed_text, #[cfg(all(test, feature = "live-site-smoke"))] - non_white_pixel_count: parts.non_white_pixel_count, + non_white_pixel_count: pixel_sample.non_white_pixel_count, #[cfg(all(test, feature = "live-site-smoke"))] - content_pixel_count: parts.content_pixel_count, + content_pixel_count: pixel_sample.content_pixel_count, #[cfg(all(test, feature = "live-site-smoke"))] - sample_hash: parts.sample_hash, + sample_hash: pixel_sample.sample_hash, image, #[cfg(target_os = "macos")] pixel_buffer: parts.pixel_buffer, @@ -183,7 +198,16 @@ impl WebSurfaceFrame { #[cfg(all(test, feature = "live-site-smoke"))] pub(super) fn size(&self) -> WebSurfaceSize { - WebSurfaceSize { width: self.width, height: self.height, device_pixel_ratio_percent: 100 } + WebSurfaceSize { + width: self.width, + height: self.height, + device_pixel_ratio_percent: (self.device_pixel_ratio * 100.0).round() as u16, + } + } + + #[cfg(all(test, feature = "live-site-smoke"))] + pub(super) fn css_viewport_size(&self) -> (u32, u32) { + (self.css_viewport_width, self.css_viewport_height) } #[cfg(all(test, feature = "live-site-smoke"))] @@ -204,6 +228,16 @@ impl WebSurfaceFrame { self.loaded_url.as_deref() } + pub(super) fn has_visible_content_for_initial_display(&self) -> Result { + #[cfg(target_os = "macos")] + if let Some(pixel_buffer) = self.pixel_buffer.as_ref() { + return sample_hardware_pixel_buffer(pixel_buffer) + .map(|sample| sample.has_visible_content()); + } + + Ok(true) + } + #[cfg(all(test, feature = "live-site-smoke"))] pub(super) fn non_white_pixel_count(&self) -> u64 { self.non_white_pixel_count @@ -239,6 +273,9 @@ struct WebSurfaceFrameParts { render_state: String, width: u32, height: u32, + device_pixel_ratio: f32, + css_viewport_width: u32, + css_viewport_height: u32, scroll_offset: WebSurfaceScrollOffset, zoom_percent: u16, click_point: Option, @@ -273,6 +310,18 @@ pub(super) enum WebSurfaceError { #[cfg(target_os = "macos")] #[error("servo hardware surface pixel format 0x{actual:x} is unsupported; expected 32BGRA")] UnsupportedHardwareSurfaceFormat { actual: u32 }, + #[cfg(target_os = "macos")] + #[error("servo hardware surface lock failed with status {status}")] + HardwareSurfaceLockFailed { status: i32 }, + #[cfg(target_os = "macos")] + #[error("servo hardware surface unlock failed with status {status}")] + HardwareSurfaceUnlockFailed { status: i32 }, + #[cfg(target_os = "macos")] + #[error("servo hardware surface base address is unavailable")] + HardwareSurfaceBaseAddressUnavailable, + #[cfg(target_os = "macos")] + #[error("servo hardware surface row stride {bytes_per_row} is too small for width {width}")] + HardwareSurfaceRowStrideTooSmall { width: usize, bytes_per_row: usize }, } #[cfg(target_os = "macos")] @@ -315,6 +364,113 @@ fn rgba_hash(bytes: &[u8]) -> u64 { hasher.finish() } +#[cfg(any(all(test, feature = "live-site-smoke"), target_os = "macos"))] +struct WebSurfacePixelSample { + non_white_pixel_count: u64, + content_pixel_count: u64, + #[cfg_attr(not(all(test, feature = "live-site-smoke")), allow(dead_code))] + sample_hash: u64, +} + +#[cfg(any(all(test, feature = "live-site-smoke"), target_os = "macos"))] +impl WebSurfacePixelSample { + fn has_visible_content(&self) -> bool { + self.non_white_pixel_count > 0 && self.content_pixel_count > 0 + } +} + +#[cfg(all(test, feature = "live-site-smoke"))] +fn pixel_sample_for_parts( + parts: &WebSurfaceFrameParts, +) -> Result { + #[cfg(target_os = "macos")] + if let Some(pixel_buffer) = parts.pixel_buffer.as_ref() { + return sample_hardware_pixel_buffer(pixel_buffer); + } + + Ok(WebSurfacePixelSample { + non_white_pixel_count: parts.non_white_pixel_count, + content_pixel_count: parts.content_pixel_count, + sample_hash: parts.sample_hash, + }) +} + +#[cfg(target_os = "macos")] +fn sample_hardware_pixel_buffer( + pixel_buffer: &CVPixelBuffer, +) -> Result { + let lock_status = pixel_buffer.lock_base_address(kCVPixelBufferLock_ReadOnly); + if lock_status != kCVReturnSuccess { + return Err(WebSurfaceError::HardwareSurfaceLockFailed { status: lock_status }); + } + + let sample = sample_locked_hardware_pixel_buffer(pixel_buffer); + let unlock_status = pixel_buffer.unlock_base_address(kCVPixelBufferLock_ReadOnly); + if unlock_status != kCVReturnSuccess { + return Err(WebSurfaceError::HardwareSurfaceUnlockFailed { status: unlock_status }); + } + + sample +} + +#[cfg(target_os = "macos")] +fn sample_locked_hardware_pixel_buffer( + pixel_buffer: &CVPixelBuffer, +) -> Result { + let width = pixel_buffer.get_width(); + let height = pixel_buffer.get_height(); + let bytes_per_row = pixel_buffer.get_bytes_per_row(); + let row_width = width.saturating_mul(4); + if bytes_per_row < row_width { + return Err(WebSurfaceError::HardwareSurfaceRowStrideTooSmall { width, bytes_per_row }); + } + + #[expect(unsafe_code)] + let base_address = unsafe { pixel_buffer.get_base_address() }; + if base_address.is_null() { + return Err(WebSurfaceError::HardwareSurfaceBaseAddressUnavailable); + } + + let byte_len = bytes_per_row.saturating_mul(height); + #[expect(unsafe_code)] + let bytes = unsafe { std::slice::from_raw_parts(base_address.cast::(), byte_len) }; + Ok(sample_bgra_rows(bytes, width, height, bytes_per_row)) +} + +#[cfg(target_os = "macos")] +fn sample_bgra_rows( + bytes: &[u8], + width: usize, + height: usize, + bytes_per_row: usize, +) -> WebSurfacePixelSample { + let mut non_white_pixel_count = 0; + let mut content_pixel_count = 0; + let mut sample_hash = 0xcbf29ce484222325_u64; + + for y in 0..height { + let row_start = y * bytes_per_row; + let row = &bytes[row_start..row_start + width * 4]; + for (x, pixel) in row.chunks_exact(4).enumerate() { + let [blue, green, red, alpha] = [pixel[0], pixel[1], pixel[2], pixel[3]]; + if alpha > 0 && (red < 245 || green < 245 || blue < 245) { + non_white_pixel_count += 1; + } + if alpha > 0 && (red < 220 || green < 220 || blue < 220) { + content_pixel_count += 1; + } + if (y * width + x).is_multiple_of(97) { + for byte in [red, green, blue, alpha] { + sample_hash ^= u64::from(byte); + sample_hash = sample_hash.wrapping_mul(0x100000001b3); + } + } + } + } + + WebSurfacePixelSample { non_white_pixel_count, content_pixel_count, sample_hash } +} + fn resolve_render_image( width: u32, height: u32, diff --git a/crates/ely_app/src/shell/web_surface_live_site_tests.rs b/crates/ely_app/src/shell/web_surface_live_site_tests.rs index 693c23e..72a6ef0 100644 --- a/crates/ely_app/src/shell/web_surface_live_site_tests.rs +++ b/crates/ely_app/src/shell/web_surface_live_site_tests.rs @@ -39,6 +39,7 @@ const LIVE_SITE_SCROLL_DOWN_Y: i32 = 360; const LIVE_SITE_SCROLL_UP_Y: i32 = -240; const LIVE_SITE_SCROLL_POINT_X: f32 = 320.0; const LIVE_SITE_SCROLL_POINT_Y: f32 = 320.0; +const RETINA_SCALE_FACTOR: f32 = 2.0; #[test] fn web_surface_cases_cover_prd_reference_urls() -> Result<(), Box> { @@ -73,6 +74,13 @@ fn web_surface_resizes_prd_site_without_failed_state() -> Result<(), Box Result<(), Box> { + run_isolated_live_site_test("web_surface_reports_css_viewport_at_retina_scale", || { + assert_web_surface_reports_retina_css_viewport() + }) +} + fn run_isolated_live_site_test( test_name: &str, test: impl FnOnce() -> Result<(), Box>, @@ -199,12 +207,6 @@ fn assert_web_surface_resizes_prd_site() -> Result<(), Box> { LIVE_SURFACE_HEIGHT, )?; - assert_eq!( - store.record_viewport_size(tab.id(), resized_live_surface_bounds(), 1.0), - WebSurfaceInputOutcome::Buffered, - "{}", - case.url, - ); assert_eq!( store.record_viewport_size(tab.id(), resized_live_surface_bounds(), 1.0), WebSurfaceInputOutcome::Applied, @@ -223,6 +225,38 @@ fn assert_web_surface_resizes_prd_site() -> Result<(), Box> { Ok(()) } +fn assert_web_surface_reports_retina_css_viewport() -> Result<(), Box> { + let mut store = WebSurfaceStore::new(); + let profile_id = ProfileId::new(); + let case = PRD_TOP_SITE_CASES + .iter() + .find(|case| case.url == "https://servo.org/") + .ok_or("missing servo.org live-site case")?; + let tab = web_tab(profile_id, case.url)?; + + assert_eq!( + store.record_viewport_size(tab.id(), live_surface_bounds(), RETINA_SCALE_FACTOR), + WebSurfaceInputOutcome::Applied, + "{}", + case.url, + ); + store.ensure_surface(&tab, ProfileDataMode::Transient, &[]); + wait_for_ready_frame_at_css_size( + &mut store, + tab.id(), + case, + ExpectedCssViewport { + physical_width: LIVE_SURFACE_WIDTH * RETINA_SCALE_FACTOR as u32, + physical_height: LIVE_SURFACE_HEIGHT * RETINA_SCALE_FACTOR as u32, + css_width: LIVE_SURFACE_WIDTH, + css_height: LIVE_SURFACE_HEIGHT, + dpr_percent: (RETINA_SCALE_FACTOR * 100.0).round() as u16, + }, + )?; + store.close_surface(tab.id()); + Ok(()) +} + fn render_web_surface_frame( store: &mut WebSurfaceStore, profile_id: &ProfileId, @@ -383,6 +417,62 @@ fn wait_for_ready_frame_at_size( } } +fn wait_for_ready_frame_at_css_size( + store: &mut WebSurfaceStore, + tab_id: &TabId, + case: &LiveSiteCase, + expected: ExpectedCssViewport, +) -> Result { + let started_at = Instant::now(); + let mut last_error = None; + + loop { + if started_at.elapsed() >= LIVE_SITE_WAIT_TIMEOUT { + return Err(last_error.unwrap_or_else(|| { + format!( + "timed out rendering {} at {}x{} css {}x{}", + case.url, + expected.physical_width, + expected.physical_height, + expected.css_width, + expected.css_height, + ) + })); + } + + store.tick(std::slice::from_ref(tab_id)); + match store.state(tab_id) { + Some(WebSurfaceState::Ready(frame)) + if frame.size().width == expected.physical_width + && frame.size().height == expected.physical_height => + { + if let Err(error) = validate_prd_frame_at_css_size(frame, case, expected) { + last_error = Some(error); + thread::sleep(LIVE_SITE_WAIT_INTERVAL); + continue; + } + return Ok(frame.clone()); + } + Some(WebSurfaceState::Ready(_)) => {} + Some(WebSurfaceState::Failed { message, .. }) => { + return Err(format!("{} failed: {message}", case.url)); + } + Some(WebSurfaceState::Loading { .. }) | None => {} + } + + thread::sleep(LIVE_SITE_WAIT_INTERVAL); + } +} + +#[derive(Clone, Copy)] +struct ExpectedCssViewport { + physical_width: u32, + physical_height: u32, + css_width: u32, + css_height: u32, + dpr_percent: u16, +} + fn validate_prd_frame( frame: &WebSurfaceFrame, case: &LiveSiteCase, @@ -401,6 +491,10 @@ fn validate_prd_frame( frame.scroll_offset().y() == expected_scroll_y, format!("{} scroll: {:?}", case.url, frame.scroll_offset()), )?; + require( + frame.css_viewport_size() == (LIVE_SURFACE_WIDTH, LIVE_SURFACE_HEIGHT), + format!("{} CSS viewport: {:?}", case.url, frame.css_viewport_size()), + )?; require_render_state_is_open(frame.render_state(), case.url)?; require( frame.url_label().contains(normalized_url(case.url)), @@ -411,23 +505,28 @@ fn validate_prd_frame( format!("title: {}", frame.title_label()), )?; let expected_detail = if expected_scroll_y == 0 { - format!("{} 934x657", frame.render_state()) + format!("{} {}x{}", frame.render_state(), LIVE_SURFACE_WIDTH, LIVE_SURFACE_HEIGHT) } else { - format!("{} 934x657 y={expected_scroll_y}", frame.render_state()) + format!( + "{} {}x{} y={expected_scroll_y}", + frame.render_state(), + LIVE_SURFACE_WIDTH, + LIVE_SURFACE_HEIGHT, + ) }; require( frame.detail_label() == expected_detail, format!("{} detail: {}", case.url, frame.detail_label()), )?; - if frame.has_hardware_surface() { - return Ok(()); - } - require(frame.non_white_pixel_count() > 0, case.url.to_string())?; + require( + frame.non_white_pixel_count() > 0, + format!("{} non-white pixels: {}", case.url, frame.non_white_pixel_count()), + )?; require( frame.content_pixel_count() >= MINIMUM_CONTENT_PIXELS, format!("{} content pixels: {}", case.url, frame.content_pixel_count()), )?; - require(frame.sample_hash() > 0, case.url.to_string()) + require(frame.sample_hash() > 0, format!("{} sample hash: {}", case.url, frame.sample_hash())) } fn validate_prd_frame_at_size( @@ -446,6 +545,10 @@ fn validate_prd_frame_at_size( format!("{} size: {:?}", case.url, frame.size()), )?; require_render_state_is_open(frame.render_state(), case.url)?; + require( + frame.css_viewport_size() == (expected_width, expected_height), + format!("{} CSS viewport: {:?}", case.url, frame.css_viewport_size()), + )?; require( frame.url_label().contains(normalized_url(case.url)), format!("url: {}", frame.url_label()), @@ -454,15 +557,45 @@ fn validate_prd_frame_at_size( frame.title_label().contains(case.title_fragment), format!("title: {}", frame.title_label()), )?; - if frame.has_hardware_surface() { - return Ok(()); - } - require(frame.non_white_pixel_count() > 0, case.url.to_string())?; + require( + frame.non_white_pixel_count() > 0, + format!("{} non-white pixels: {}", case.url, frame.non_white_pixel_count()), + )?; require( frame.content_pixel_count() >= MINIMUM_CONTENT_PIXELS, format!("{} content pixels: {}", case.url, frame.content_pixel_count()), )?; - require(frame.sample_hash() > 0, case.url.to_string()) + require(frame.sample_hash() > 0, format!("{} sample hash: {}", case.url, frame.sample_hash())) +} + +fn validate_prd_frame_at_css_size( + frame: &WebSurfaceFrame, + case: &LiveSiteCase, + expected: ExpectedCssViewport, +) -> Result<(), String> { + require( + frame.size() + == WebSurfaceSize { + width: expected.physical_width, + height: expected.physical_height, + device_pixel_ratio_percent: expected.dpr_percent, + }, + format!("{} size: {:?}", case.url, frame.size()), + )?; + require_render_state_is_open(frame.render_state(), case.url)?; + require( + frame.css_viewport_size() == (expected.css_width, expected.css_height), + format!("{} CSS viewport: {:?}", case.url, frame.css_viewport_size()), + )?; + require( + frame.url_label().contains(normalized_url(case.url)), + format!("url: {}", frame.url_label()), + )?; + require( + frame.title_label().contains(case.title_fragment), + format!("title: {}", frame.title_label()), + )?; + Ok(()) } fn log_prd_frame(label: &str, frame: &WebSurfaceFrame, case: &LiveSiteCase) { diff --git a/crates/ely_app/src/shell/web_surface_runtime.rs b/crates/ely_app/src/shell/web_surface_runtime.rs index 5f010cc..6ad789d 100644 --- a/crates/ely_app/src/shell/web_surface_runtime.rs +++ b/crates/ely_app/src/shell/web_surface_runtime.rs @@ -1,4 +1,4 @@ -use std::{collections::BTreeMap, fs, path::PathBuf}; +use std::{collections::BTreeMap, fs, path::PathBuf, time::Instant}; use ely_domain::{BrowserTab, ProfileId, TabId}; @@ -13,10 +13,11 @@ use super::{ web_surface_geometry::{WebSurfaceScrollOffset, WebSurfaceSize}, web_surface_permissions::WebSurfaceSitePermission, web_surface_state::WebSurfacePendingInput, + web_surface_worker::{LiveRuntimeClient, LiveRuntimeWorker, WorkerResponse}, }; pub(super) struct WebSurfaceRuntime { - clients: BTreeMap, + workers: BTreeMap, sessions: BTreeMap, client_factory: LiveRuntimeClientFactory, } @@ -24,15 +25,15 @@ pub(super) struct WebSurfaceRuntime { impl WebSurfaceRuntime { pub(super) fn new() -> Self { Self { - clients: BTreeMap::new(), + workers: BTreeMap::new(), sessions: BTreeMap::new(), client_factory: new_servo_live_client, } } #[cfg(test)] - fn new_with_client_factory(client_factory: LiveRuntimeClientFactory) -> Self { - Self { clients: BTreeMap::new(), sessions: BTreeMap::new(), client_factory } + pub(super) fn new_with_client_factory(client_factory: LiveRuntimeClientFactory) -> Self { + Self { workers: BTreeMap::new(), sessions: BTreeMap::new(), client_factory } } pub(super) fn ensure_tab( @@ -44,15 +45,17 @@ impl WebSurfaceRuntime { input: WebSurfacePendingInput, ) -> Result { let scope = WebSurfaceRuntimeScope::new(tab.profile_id().clone(), profile_data_mode); - self.ensure_runtime(scope.clone())?; + self.ensure_worker(scope.clone())?; let requested_url = tab.url().as_str().to_string(); let zoom_percent = tab.zoom_percent(); + let enqueued_at = input.enqueued_at; + let input_kind = pending_input_kind(&input); let (scroll_delta_x, scroll_delta_y, scroll_point_x, scroll_point_y) = scroll_wire_fields(input.scroll_delta, input.scroll_point)?; let user_navigation_input = input_requests_history_navigation(&input); - let next_scroll_offset = input.scroll_offset; + let started_loading = { let session = session_for_scope(&mut self.sessions, tab.id(), scope.clone()); let started_loading = session.started_loading(&requested_url, size, zoom_percent); @@ -62,89 +65,116 @@ impl WebSurfaceRuntime { if user_navigation_input { session.pending_user_navigation = true; } + session.requested_url = requested_url.clone(); + session.size = size; + session.zoom_percent = zoom_percent; + session.scroll_offset = next_scroll_offset; started_loading }; - let frame = self - .client_for_scope(&scope)? - .ensure(ServoLiveEnsureRequest { - tab_id: tab.id().as_str().to_string(), - profile_id: tab.profile_id().as_str().to_string(), - url: requested_url.clone(), - width: size.width, - height: size.height, - page_zoom_percent: zoom_percent, - device_pixel_ratio: size.device_pixel_ratio_f32(), - scroll_delta_x, - scroll_delta_y, - scroll_point_x, - scroll_point_y, - click_x: input.click_point.map(|point| point.x()), - click_y: input.click_point.map(|point| point.y()), - hover_x: input.hover_point.map(|point| point.x()), - hover_y: input.hover_point.map(|point| point.y()), - typed_text: input.typed_text, - site_permissions: permissions.iter().map(ServoLiveSitePermission::from).collect(), - })? - .map(|frame| { - WebSurfaceFrame::from_live_frame( - requested_url.clone(), - next_scroll_offset, - zoom_percent, - frame, - ) - .map_err(|error| error.to_string()) - }) - .transpose()?; - let url_change = frame.as_ref().and_then(|frame| { - self.sessions - .get_mut(tab.id()) - .and_then(|session| session.url_change_for(tab.id(), requested_url.as_str(), frame)) - }); + let request = ServoLiveEnsureRequest { + tab_id: tab.id().as_str().to_string(), + profile_id: tab.profile_id().as_str().to_string(), + url: requested_url.clone(), + width: size.width, + height: size.height, + page_zoom_percent: zoom_percent, + device_pixel_ratio: size.device_pixel_ratio_f32(), + scroll_delta_x, + scroll_delta_y, + scroll_point_x, + scroll_point_y, + click_x: input.click_point.map(|point| point.x()), + click_y: input.click_point.map(|point| point.y()), + hover_x: input.hover_point.map(|point| point.x()), + hover_y: input.hover_point.map(|point| point.y()), + typed_text: input.typed_text, + site_permissions: permissions.iter().map(ServoLiveSitePermission::from).collect(), + }; - let session = session_for_scope(&mut self.sessions, tab.id(), scope); - session.requested_url = requested_url.clone(); - session.size = size; - session.zoom_percent = zoom_percent; - session.scroll_offset = next_scroll_offset; + let Some(scoped) = self.workers.get(&scope) else { + return Err("Servo worker was created but is no longer registered".to_string()); + }; + scoped.worker.submit_ensure(request); + log_ensure_submitted(tab, size, input_kind, enqueued_at, started_loading); - Ok(WebSurfaceEnsureResult { requested_url, started_loading, frame, url_change }) + Ok(WebSurfaceEnsureResult { requested_url, started_loading }) } pub(super) fn tick(&mut self, visible_tab_ids: &[TabId]) -> Vec { - let mut frames = Vec::new(); - for job in visible_poll_jobs(&self.sessions, visible_tab_ids) { - let result = match self.clients.get_mut(&job.scope) { - Some(client) => client.client.poll(job.tab_id.as_str().to_string()), - None => Err(missing_runtime_message(&job.scope)), + // Submit a Poll for every visible tab whose session is live so + // animations / JS-driven content keep advancing without user + // input. The worker coalesces — a Poll never overrides a + // pending Ensure — so this stays cheap even at 120 Hz. + for tab_id in visible_tab_ids { + let Some(session) = self.sessions.get(tab_id) else { + continue; }; - match result { - Ok(Some(frame)) => match WebSurfaceFrame::from_live_frame( - job.requested_url.clone(), - job.scroll_offset, - job.zoom_percent, - frame, - ) { - Ok(frame) => { - let url_change = self.sessions.get_mut(&job.tab_id).and_then(|session| { - session.url_change_for(&job.tab_id, job.requested_url.as_str(), &frame) - }); - frames.push(WebSurfaceRuntimeFrame::Ready { - tab_id: job.tab_id, - frame: Box::new(frame), - url_change, - }) + let Some(scoped) = self.workers.get(&session.scope) else { + continue; + }; + scoped.worker.submit_poll(tab_id.as_str().to_string()); + } + + let mut frames = Vec::new(); + let mut dead_scopes = Vec::new(); + let scopes: Vec = self.workers.keys().cloned().collect(); + for scope in scopes { + let responses = self + .workers + .get(&scope) + .map(|scoped| scoped.worker.drain_responses()) + .unwrap_or_default(); + for response in responses { + match response { + WorkerResponse::Frame { tab_id, frame } => { + let Some(tab_id_obj) = self.lookup_session_tab_id(&tab_id) else { + continue; + }; + let session = match self.sessions.get_mut(&tab_id_obj) { + Some(session) => session, + None => continue, + }; + let requested_url = session.requested_url.clone(); + let scroll_offset = session.scroll_offset; + let zoom_percent = session.zoom_percent; + match WebSurfaceFrame::from_live_frame( + requested_url.clone(), + scroll_offset, + zoom_percent, + frame, + ) { + Ok(frame) => { + let url_change = session.url_change_for( + &tab_id_obj, + requested_url.as_str(), + &frame, + ); + frames.push(WebSurfaceRuntimeFrame::Ready { + tab_id: tab_id_obj, + frame: Box::new(frame), + url_change, + }); + } + Err(error) => frames.push(WebSurfaceRuntimeFrame::Failed { + tab_id: tab_id_obj, + message: error.to_string(), + }), + } } - Err(error) => frames.push(WebSurfaceRuntimeFrame::Failed { - tab_id: job.tab_id, - message: error.to_string(), - }), - }, - Ok(None) => {} - Err(error) => frames - .push(WebSurfaceRuntimeFrame::Failed { tab_id: job.tab_id, message: error }), + WorkerResponse::Failed { tab_id, message } => { + let Some(tab_id_obj) = self.lookup_session_tab_id(&tab_id) else { + continue; + }; + frames.push(WebSurfaceRuntimeFrame::Failed { tab_id: tab_id_obj, message }); + } + WorkerResponse::SidecarExited => dead_scopes.push(scope.clone()), + } } } + for scope in dead_scopes { + self.workers.remove(&scope); + } frames } @@ -153,81 +183,60 @@ impl WebSurfaceRuntime { let Some(session) = self.sessions.remove(tab_id) else { return; }; - if let Some(client) = self.clients.get_mut(&session.scope) { - let _ = client.client.close(tab_id.as_str().to_string()); + if let Some(scoped) = self.workers.get(&session.scope) { + scoped.worker.submit_close(tab_id.as_str().to_string()); } } - fn ensure_runtime(&mut self, scope: WebSurfaceRuntimeScope) -> Result<(), String> { - if self.clients.contains_key(&scope) { + fn ensure_worker(&mut self, scope: WebSurfaceRuntimeScope) -> Result<(), String> { + if self.workers.contains_key(&scope) { return Ok(()); } let (config_dir, transient_profile_data_dir) = config_dir_for_scope(&scope)?; let client = (self.client_factory)(config_dir)?; - self.clients.insert(scope, ScopedRuntimeClient { client, transient_profile_data_dir }); + let worker = LiveRuntimeWorker::new(client)?; + self.workers.insert(scope, ScopedWorker { worker, transient_profile_data_dir }); Ok(()) } - fn client_for_scope( - &mut self, - scope: &WebSurfaceRuntimeScope, - ) -> Result<&mut dyn LiveRuntimeClient, String> { - match self.clients.get_mut(scope) { - Some(client) => Ok(client.client.as_mut()), - None => Err(missing_runtime_message(scope)), - } + fn lookup_session_tab_id(&self, tab_id: &str) -> Option { + self.sessions.keys().find(|key| key.as_str() == tab_id).cloned() } #[cfg(test)] - fn client_count_for_test(&self) -> usize { - self.clients.len() + pub(super) fn client_count_for_test(&self) -> usize { + self.workers.len() } #[cfg(test)] - fn session_scope_for_test(&self, tab_id: &TabId) -> Option<&WebSurfaceRuntimeScope> { + pub(super) fn session_scope_for_test(&self, tab_id: &TabId) -> Option<&WebSurfaceRuntimeScope> { self.sessions.get(tab_id).map(|session| &session.scope) } + + #[cfg(test)] + pub(super) fn flush_for_test(&self) { + for scoped in self.workers.values() { + scoped.worker.wait_until_idle(); + } + } } impl Drop for WebSurfaceRuntime { fn drop(&mut self) { let transient_profile_data_dirs = self - .clients + .workers .values() - .filter_map(|client| client.transient_profile_data_dir.clone()) + .filter_map(|scoped| scoped.transient_profile_data_dir.clone()) .collect::>(); - self.clients.clear(); + self.workers.clear(); for path in transient_profile_data_dirs { let _ = fs::remove_dir_all(path); } } } -type LiveRuntimeClientFactory = fn(PathBuf) -> Result, String>; - -trait LiveRuntimeClient { - fn ensure(&mut self, request: ServoLiveEnsureRequest) -> Result, String>; - - fn poll(&mut self, tab_id: String) -> Result, String>; - - fn close(&mut self, tab_id: String) -> Result<(), String>; -} - -type WebLiveFrame = crate::services::servo_live::ServoLiveFrame; - -impl LiveRuntimeClient for ServoLiveClient { - fn ensure(&mut self, request: ServoLiveEnsureRequest) -> Result, String> { - ServoLiveClient::ensure(self, request).map_err(|error| error.to_string()) - } - - fn poll(&mut self, tab_id: String) -> Result, String> { - ServoLiveClient::poll(self, tab_id).map_err(|error| error.to_string()) - } - - fn close(&mut self, tab_id: String) -> Result<(), String> { - ServoLiveClient::close(self, tab_id).map_err(|error| error.to_string()) - } -} +pub(super) type LiveRuntimeClientFactory = + fn(PathBuf) -> Result, String>; fn new_servo_live_client(config_dir: PathBuf) -> Result, String> { ServoLiveClient::new(config_dir) @@ -235,31 +244,31 @@ fn new_servo_live_client(config_dir: PathBuf) -> Result, +struct ScopedWorker { + worker: LiveRuntimeWorker, transient_profile_data_dir: Option, } #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] -struct WebSurfaceRuntimeScope { +pub(super) struct WebSurfaceRuntimeScope { profile_id: ProfileId, profile_data_mode: ProfileDataMode, } impl WebSurfaceRuntimeScope { - fn new(profile_id: ProfileId, profile_data_mode: ProfileDataMode) -> Self { + pub(super) fn new(profile_id: ProfileId, profile_data_mode: ProfileDataMode) -> Self { Self { profile_id, profile_data_mode } } } #[derive(Clone)] -struct WebSurfaceSession { - scope: WebSurfaceRuntimeScope, - requested_url: String, - size: WebSurfaceSize, - zoom_percent: u16, - scroll_offset: WebSurfaceScrollOffset, - pending_user_navigation: bool, +pub(super) struct WebSurfaceSession { + pub(super) scope: WebSurfaceRuntimeScope, + pub(super) requested_url: String, + pub(super) size: WebSurfaceSize, + pub(super) zoom_percent: u16, + pub(super) scroll_offset: WebSurfaceScrollOffset, + pub(super) pending_user_navigation: bool, } impl WebSurfaceSession { @@ -310,20 +319,9 @@ impl WebSurfaceSession { } } -#[derive(Clone)] -struct WebSurfacePollJob { - tab_id: TabId, - scope: WebSurfaceRuntimeScope, - requested_url: String, - scroll_offset: WebSurfaceScrollOffset, - zoom_percent: u16, -} - pub(super) struct WebSurfaceEnsureResult { pub(super) requested_url: String, pub(super) started_loading: bool, - pub(super) frame: Option, - pub(super) url_change: Option, } pub(super) enum WebSurfaceRuntimeFrame { @@ -364,7 +362,7 @@ fn config_dir_for_scope( } } -fn session_for_scope<'a>( +pub(super) fn session_for_scope<'a>( sessions: &'a mut BTreeMap, tab_id: &TabId, scope: WebSurfaceRuntimeScope, @@ -377,33 +375,6 @@ fn session_for_scope<'a>( session } -fn visible_poll_jobs( - sessions: &BTreeMap, - visible_tab_ids: &[TabId], -) -> Vec { - sessions - .iter() - .filter(|(tab_id, _)| { - visible_tab_ids.iter().any(|visible_tab_id| visible_tab_id == *tab_id) - }) - .map(|(tab_id, session)| WebSurfacePollJob { - tab_id: tab_id.clone(), - scope: session.scope.clone(), - requested_url: session.requested_url.clone(), - scroll_offset: session.scroll_offset, - zoom_percent: session.zoom_percent, - }) - .collect() -} - -fn missing_runtime_message(scope: &WebSurfaceRuntimeScope) -> String { - format!( - "Servo live runtime is unavailable for profile {} ({:?})", - scope.profile_id.as_str(), - scope.profile_data_mode - ) -} - fn scroll_wire_fields( delta: Option, point: Option, @@ -423,6 +394,45 @@ fn input_requests_history_navigation(input: &WebSurfacePendingInput) -> bool { || input.typed_text.as_deref().is_some_and(|text| text.contains('\n')) } +fn pending_input_kind(input: &WebSurfacePendingInput) -> &'static str { + if input.scroll_delta.is_some() { + "scroll" + } else if input.click_point.is_some() { + "click" + } else if input.typed_text.is_some() { + "text" + } else if input.hover_point.is_some() { + "hover" + } else { + "idle" + } +} + +fn log_ensure_submitted( + tab: &BrowserTab, + size: WebSurfaceSize, + input_kind: &'static str, + enqueued_at: Option, + started_loading: bool, +) { + if input_kind == "idle" && !started_loading { + return; + } + let queued_us = enqueued_at.map(|started_at| started_at.elapsed().as_micros()); + tracing::info!( + target: "ely::web_surface::latency", + tab_id = %tab.id().as_str(), + url = %tab.url().as_str(), + input_kind, + queued_us, + started_loading, + width = size.width, + height = size.height, + device_pixel_ratio = size.device_pixel_ratio_f32(), + "web_surface_ensure_submitted", + ); +} + impl From<&WebSurfaceSitePermission> for ServoLiveSitePermission { fn from(permission: &WebSurfaceSitePermission) -> Self { Self::new( 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 45eb547..cec60ee 100644 --- a/crates/ely_app/src/shell/web_surface_runtime_tests.rs +++ b/crates/ely_app/src/shell/web_surface_runtime_tests.rs @@ -8,14 +8,22 @@ use ely_domain::{BrowserTab, ProfileId, SpaceId, TabId, UrlText}; use crate::{ services::ProfileDataMode, shell::{ + WebSurfaceStore, web_surface_geometry::{WebSurfaceScrollOffset, WebSurfaceSize}, - web_surface_state::WebSurfacePendingInput, + web_surface_state::{WebSurfaceInputOutcome, WebSurfacePendingInput}, + web_surface_worker::{LiveRuntimeClient, LiveRuntimeClientError}, }, }; use super::*; +use crate::services::servo_live::{ServoLiveEnsureRequest, ServoLiveFrame}; + static FAKE_CLOSE_COUNT: AtomicUsize = AtomicUsize::new(0); +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); #[test] fn runtime_keeps_independent_clients_for_profile_scopes() -> Result<(), String> { @@ -47,6 +55,8 @@ fn runtime_keeps_independent_clients_for_profile_scopes() -> Result<(), String> pending_input(), )?; + runtime.flush_for_test(); + assert_eq!(runtime.client_count_for_test(), 2); assert_eq!( runtime.session_scope_for_test(first_tab.id()), @@ -66,16 +76,114 @@ fn close_tab_removes_session_and_closes_client() -> Result<(), String> { let tab = web_tab(TabId::new(), ProfileId::new(), "https://example.com/close")?; runtime.ensure_tab(&tab, surface_size(), ProfileDataMode::Transient, &[], pending_input())?; + runtime.flush_for_test(); runtime.close_tab(tab.id()); + runtime.flush_for_test(); assert_eq!(runtime.session_scope_for_test(tab.id()), None); assert_eq!(FAKE_CLOSE_COUNT.load(Ordering::SeqCst), before + 1); runtime.close_tab(tab.id()); + runtime.flush_for_test(); assert_eq!(FAKE_CLOSE_COUNT.load(Ordering::SeqCst), before + 1); Ok(()) } +#[test] +fn unchanged_surface_without_input_skips_runtime_ensure() -> Result<(), String> { + IDLE_SKIP_ENSURE_COUNT.store(0, Ordering::SeqCst); + let mut store = WebSurfaceStore::new_with_runtime(WebSurfaceRuntime::new_with_client_factory( + idle_skip_client_factory, + )); + let tab = web_tab(TabId::new(), ProfileId::new(), "https://example.com/idle")?; + + assert_eq!( + store.record_viewport_size(tab.id(), viewport_bounds(), 1.0), + WebSurfaceInputOutcome::Applied, + ); + assert!(store.ensure_surface(&tab, ProfileDataMode::Transient, &[]).changed); + store.flush_runtime_for_test(); + assert_eq!(IDLE_SKIP_ENSURE_COUNT.load(Ordering::SeqCst), 1); + + assert!(!store.ensure_surface(&tab, ProfileDataMode::Transient, &[]).changed); + store.flush_runtime_for_test(); + assert_eq!(IDLE_SKIP_ENSURE_COUNT.load(Ordering::SeqCst), 1); + Ok(()) +} + +#[test] +fn sidecar_exit_removes_dead_runtime_client() -> Result<(), String> { + RECOVERY_FACTORY_COUNT.store(0, Ordering::SeqCst); + let mut runtime = WebSurfaceRuntime::new_with_client_factory(recovery_client_factory); + let profile = ProfileId::new(); + let crashed_tab = web_tab(TabId::new(), profile.clone(), "https://example.com/crash")?; + + runtime.ensure_tab( + &crashed_tab, + surface_size(), + ProfileDataMode::Transient, + &[], + pending_input(), + )?; + runtime.flush_for_test(); + let frames = runtime.tick(&[crashed_tab.id().clone()]); + assert!( + frames.iter().any( + |frame| matches!(frame, WebSurfaceRuntimeFrame::Failed { tab_id, .. } if tab_id == crashed_tab.id()) + ), + "the crashed tab must surface as a Failed frame", + ); + assert_eq!(runtime.client_count_for_test(), 0); + + let next_tab = web_tab(TabId::new(), profile, "https://example.com/next")?; + runtime.ensure_tab( + &next_tab, + surface_size(), + ProfileDataMode::Transient, + &[], + pending_input(), + )?; + runtime.flush_for_test(); + + assert_eq!(RECOVERY_FACTORY_COUNT.load(Ordering::SeqCst), 2); + assert_eq!(runtime.client_count_for_test(), 1); + Ok(()) +} + +#[test] +fn failed_surface_ensure_waits_for_a_new_key_before_retrying() -> Result<(), String> { + FAILING_ENSURE_COUNT.store(0, Ordering::SeqCst); + let mut store = WebSurfaceStore::new_with_runtime(WebSurfaceRuntime::new_with_client_factory( + failing_client_factory, + )); + let tab = web_tab(TabId::new(), ProfileId::new(), "https://example.com/crash")?; + + assert_eq!( + store.record_viewport_size(tab.id(), viewport_bounds(), 1.0), + WebSurfaceInputOutcome::Applied, + ); + assert!(store.ensure_surface(&tab, ProfileDataMode::Transient, &[]).changed); + store.flush_runtime_for_test(); + let tick = store.tick(&[tab.id().clone()]); + assert!(tick.changed, "the failing client must surface a state change via tick"); + assert_eq!(FAILING_ENSURE_COUNT.load(Ordering::SeqCst), 1); + + assert!(!store.ensure_surface(&tab, ProfileDataMode::Transient, &[]).changed); + store.flush_runtime_for_test(); + let _ = store.tick(&[tab.id().clone()]); + assert_eq!(FAILING_ENSURE_COUNT.load(Ordering::SeqCst), 1); + + assert_eq!( + store.record_viewport_size(tab.id(), resized_viewport_bounds(), 1.0), + WebSurfaceInputOutcome::Applied, + ); + assert!(store.ensure_surface(&tab, ProfileDataMode::Transient, &[]).changed); + store.flush_runtime_for_test(); + let _ = store.tick(&[tab.id().clone()]); + assert_eq!(FAILING_ENSURE_COUNT.load(Ordering::SeqCst), 2); + Ok(()) +} + #[test] fn session_scope_change_resets_tab_state() { let tab_id = TabId::new(); @@ -101,28 +209,110 @@ fn session_scope_change_resets_tab_state() { } struct FakeLiveRuntimeClient; +struct IdleSkipLiveRuntimeClient; +struct SidecarExitLiveRuntimeClient; +struct FailingLiveRuntimeClient; impl LiveRuntimeClient for FakeLiveRuntimeClient { - fn ensure(&mut self, _request: ServoLiveEnsureRequest) -> Result, String> { + fn ensure( + &mut self, + _request: ServoLiveEnsureRequest, + ) -> Result, LiveRuntimeClientError> { + FAKE_ENSURE_COUNT.fetch_add(1, Ordering::SeqCst); Ok(None) } - fn poll(&mut self, _tab_id: String) -> Result, String> { + fn poll(&mut self, _tab_id: String) -> Result, LiveRuntimeClientError> { Ok(None) } - fn close(&mut self, _tab_id: String) -> Result<(), String> { + fn close(&mut self, _tab_id: String) -> Result<(), LiveRuntimeClientError> { FAKE_CLOSE_COUNT.fetch_add(1, Ordering::SeqCst); Ok(()) } } +impl LiveRuntimeClient for IdleSkipLiveRuntimeClient { + fn ensure( + &mut self, + _request: ServoLiveEnsureRequest, + ) -> Result, LiveRuntimeClientError> { + IDLE_SKIP_ENSURE_COUNT.fetch_add(1, Ordering::SeqCst); + Ok(None) + } + + fn poll(&mut self, _tab_id: String) -> Result, LiveRuntimeClientError> { + Ok(None) + } + + fn close(&mut self, _tab_id: String) -> Result<(), LiveRuntimeClientError> { + Ok(()) + } +} + +impl LiveRuntimeClient for SidecarExitLiveRuntimeClient { + fn ensure( + &mut self, + _request: ServoLiveEnsureRequest, + ) -> Result, LiveRuntimeClientError> { + Err(LiveRuntimeClientError::SidecarExited) + } + + fn poll(&mut self, _tab_id: String) -> Result, LiveRuntimeClientError> { + Err(LiveRuntimeClientError::SidecarExited) + } + + fn close(&mut self, _tab_id: String) -> Result<(), LiveRuntimeClientError> { + Err(LiveRuntimeClientError::SidecarExited) + } +} + +impl LiveRuntimeClient for FailingLiveRuntimeClient { + fn ensure( + &mut self, + _request: ServoLiveEnsureRequest, + ) -> Result, LiveRuntimeClientError> { + FAILING_ENSURE_COUNT.fetch_add(1, Ordering::SeqCst); + Err(LiveRuntimeClientError::SidecarExited) + } + + 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> { Ok(Box::new(FakeLiveRuntimeClient)) } +fn idle_skip_client_factory( + _config_dir: std::path::PathBuf, +) -> Result, String> { + Ok(Box::new(IdleSkipLiveRuntimeClient)) +} + +fn recovery_client_factory( + _config_dir: std::path::PathBuf, +) -> Result, String> { + let factory_call = RECOVERY_FACTORY_COUNT.fetch_add(1, Ordering::SeqCst); + if factory_call == 0 { + return Ok(Box::new(SidecarExitLiveRuntimeClient)); + } + Ok(Box::new(FakeLiveRuntimeClient)) +} + +fn failing_client_factory( + _config_dir: std::path::PathBuf, +) -> Result, String> { + Ok(Box::new(FailingLiveRuntimeClient)) +} + 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)) @@ -132,8 +322,23 @@ fn surface_size() -> WebSurfaceSize { WebSurfaceSize { width: 640, height: 480, device_pixel_ratio_percent: 100 } } +fn viewport_bounds() -> gpui::Bounds { + gpui::Bounds::new( + gpui::point(gpui::px(0.0), gpui::px(0.0)), + gpui::size(gpui::px(640.0), gpui::px(480.0)), + ) +} + +fn resized_viewport_bounds() -> gpui::Bounds { + gpui::Bounds::new( + gpui::point(gpui::px(0.0), gpui::px(0.0)), + gpui::size(gpui::px(720.0), gpui::px(480.0)), + ) +} + fn pending_input() -> WebSurfacePendingInput { WebSurfacePendingInput { + enqueued_at: None, scroll_offset: WebSurfaceScrollOffset::default(), scroll_delta: None, scroll_point: None, diff --git a/crates/ely_app/src/shell/web_surface_state.rs b/crates/ely_app/src/shell/web_surface_state.rs index 3e456f9..60ad4fc 100644 --- a/crates/ely_app/src/shell/web_surface_state.rs +++ b/crates/ely_app/src/shell/web_surface_state.rs @@ -1,3 +1,5 @@ +use std::time::Instant; + use ely_domain::TabId; use gpui::{Bounds, Pixels}; @@ -6,6 +8,7 @@ use super::{ web_surface_geometry::{ WebSurfaceClickPoint, WebSurfaceScrollDelta, WebSurfaceScrollOffset, WebSurfaceSize, }, + web_surface_permissions::WebSurfaceSitePermission, }; pub(super) struct WebSurfaceScrollState { @@ -43,6 +46,7 @@ pub(super) struct WebSurfaceTextInputState { #[derive(Clone, Debug, Eq, PartialEq)] pub(super) struct WebSurfacePendingInput { + pub(super) enqueued_at: Option, pub(super) scroll_offset: WebSurfaceScrollOffset, pub(super) scroll_delta: Option, pub(super) scroll_point: Option, @@ -68,9 +72,6 @@ pub(super) enum WebSurfaceInputOutcome { Applied, /// Same value as currently recorded — nothing to flush downstream. NoChange, - /// First sighting of a new value; held back until a second - /// matching measurement confirms it (viewport-resize debounce). - Buffered, /// Geometry constructor rejected the input (zero/NaN/negative /// bounds). The viewport never measured cleanly. DroppedInvalidBounds, @@ -113,11 +114,12 @@ pub(super) enum WebSurfaceState { pub(super) struct PerTabSurface { pub(super) viewport_bounds: Option>, pub(super) viewport_size: Option, - pub(super) pending_viewport_size: Option, + pub(super) last_ensure_key: Option, pub(super) hover_point: Option, pub(super) click_point: Option, pub(super) pending_scroll_delta: Option, pub(super) pending_scroll_point: Option, + pub(super) pending_input_started_at: Option, pub(super) scroll_offset: Option, pub(super) typed_text: Option, pub(super) state: Option, @@ -128,17 +130,38 @@ impl PerTabSurface { Self { viewport_bounds: None, viewport_size: None, - pending_viewport_size: None, + last_ensure_key: None, hover_point: None, click_point: None, pending_scroll_delta: None, pending_scroll_point: None, + pending_input_started_at: None, scroll_offset: None, typed_text: None, state: None, } } + pub(super) fn mark_pending_input_started(&mut self) { + self.pending_input_started_at.get_or_insert_with(Instant::now); + } + + pub(super) fn should_ensure(&self, key: &WebSurfaceEnsureKey) -> bool { + self.last_ensure_key.as_ref() != Some(key) || self.has_pending_input() + } + + pub(super) fn mark_ensured(&mut self, key: WebSurfaceEnsureKey) { + self.last_ensure_key = Some(key); + } + + fn has_pending_input(&self) -> bool { + self.hover_point.is_some() + || self.click_point.is_some() + || self.pending_scroll_delta.is_some() + || self.pending_scroll_point.is_some() + || self.typed_text.is_some() + } + pub(super) fn scroll_offset_for(&self, requested_url: &str) -> WebSurfaceScrollOffset { self.scroll_offset .as_ref() @@ -147,3 +170,71 @@ impl PerTabSurface { .unwrap_or_default() } } + +#[derive(Clone, Debug, Eq, PartialEq)] +pub(super) struct WebSurfaceEnsureKey { + requested_url: String, + size: WebSurfaceSize, + zoom_percent: u16, + permissions: Vec, +} + +impl WebSurfaceEnsureKey { + pub(super) fn new( + requested_url: String, + size: WebSurfaceSize, + zoom_percent: u16, + permissions: &[WebSurfaceSitePermission], + ) -> Self { + Self { requested_url, size, zoom_percent, permissions: permissions.to_vec() } + } +} + +#[cfg(test)] +mod tests { + use gpui::{point, px}; + + use super::*; + + #[test] + fn unchanged_surface_without_input_skips_ensure() { + let key = ensure_key("https://example.com/", 800, 600); + let mut surface = PerTabSurface::new(); + + assert!(surface.should_ensure(&key)); + + surface.mark_ensured(key.clone()); + + assert!(!surface.should_ensure(&key)); + } + + #[test] + fn pending_input_forces_ensure_even_when_key_matches() { + let key = ensure_key("https://example.com/", 800, 600); + let mut surface = PerTabSurface::new(); + surface.mark_ensured(key.clone()); + surface.pending_scroll_delta = + WebSurfaceScrollDelta::from_point(point(px(0.0), px(120.0)), 1.0); + + assert!(surface.should_ensure(&key)); + } + + #[test] + fn viewport_change_forces_ensure() { + let old_key = ensure_key("https://example.com/", 800, 600); + let new_key = ensure_key("https://example.com/", 1024, 768); + let mut surface = PerTabSurface::new(); + surface.mark_ensured(old_key); + + assert!(surface.should_ensure(&new_key)); + } + + fn ensure_key(url: &str, width: u32, height: u32) -> WebSurfaceEnsureKey { + WebSurfaceEnsureKey::new( + url.to_string(), + WebSurfaceSize { width, height, device_pixel_ratio_percent: 100 }, + 100, + &[], + ) + } +} diff --git a/crates/ely_app/src/shell/web_surface_tests.rs b/crates/ely_app/src/shell/web_surface_tests.rs index b11b66a..dcbe829 100644 --- a/crates/ely_app/src/shell/web_surface_tests.rs +++ b/crates/ely_app/src/shell/web_surface_tests.rs @@ -62,16 +62,11 @@ fn scroll_delta_enters_pending_input_after_wheel() -> Result<(), Box> } #[test] -fn viewport_size_changes_after_stable_second_measurement() -> Result<(), Box> { +fn viewport_size_changes_on_first_clean_measurement() -> Result<(), Box> { let mut store = WebSurfaceStore::new(); let tab = web_tab("https://example.com/resize")?; assert_applied(store.record_viewport_size(tab.id(), web_bounds(), 1.0)); - assert_eq!( - store.record_viewport_size(tab.id(), resized_once_bounds(), 1.0), - WebSurfaceInputOutcome::Buffered, - "first sighting of a new size must wait for a confirming second measurement", - ); assert_applied(store.record_viewport_size(tab.id(), resized_once_bounds(), 1.0)); Ok(()) } @@ -327,11 +322,7 @@ fn click_survives_viewport_bounds_change_before_drain() -> Result<(), Box Result<(), String Ok(()) } +#[cfg(all(target_os = "macos", feature = "live-site-smoke"))] +#[test] +fn hardware_live_frame_samples_bgra_surface_pixels() -> Result<(), String> { + use core_video::{ + pixel_buffer::{CVPixelBuffer, kCVPixelFormatType_32BGRA}, + r#return::kCVReturnSuccess, + }; + + use crate::services::servo_live::ServoLiveFrame; + use crate::shell::web_surface_frame::WebSurfaceFrame; + use crate::shell::web_surface_geometry::WebSurfaceScrollOffset; + + let pixel_buffer = CVPixelBuffer::new(kCVPixelFormatType_32BGRA, 2, 1, None) + .map_err(|status| format!("CVPixelBufferCreate returned status {status}"))?; + let lock_status = pixel_buffer.lock_base_address(0); + if lock_status != kCVReturnSuccess { + return Err(format!("CVPixelBufferLockBaseAddress returned status {lock_status}")); + } + let bytes_per_row = pixel_buffer.get_bytes_per_row(); + #[expect(unsafe_code)] + unsafe { + let base_address = pixel_buffer.get_base_address().cast::(); + let bytes = std::slice::from_raw_parts_mut(base_address, bytes_per_row); + bytes[0..8].copy_from_slice(&[ + 0, 0, 255, 255, // red in BGRA memory order + 255, 255, 255, 255, + ]); + } + let unlock_status = pixel_buffer.unlock_base_address(0); + if unlock_status != kCVReturnSuccess { + return Err(format!("CVPixelBufferUnlockBaseAddress returned status {unlock_status}")); + } + + let live = ServoLiveFrame::for_test_with_pixel_buffer(2, 1, pixel_buffer); + let frame = WebSurfaceFrame::from_live_frame( + "https://example.com/".to_string(), + WebSurfaceScrollOffset::default(), + 100, + live, + ) + .map_err(|error| error.to_string())?; + + assert_eq!(frame.non_white_pixel_count(), 1); + assert_eq!(frame.content_pixel_count(), 1); + assert_ne!(frame.sample_hash(), 0); + Ok(()) +} + fn web_bounds() -> Bounds { Bounds::new(point(px(0.0), px(0.0)), size(px(640.0), px(480.0))) } diff --git a/crates/ely_app/src/shell/web_surface_view.rs b/crates/ely_app/src/shell/web_surface_view.rs index 60e87b9..3efd103 100644 --- a/crates/ely_app/src/shell/web_surface_view.rs +++ b/crates/ely_app/src/shell/web_surface_view.rs @@ -1,14 +1,14 @@ use ely_domain::{BrowserTab, TabId}; use gpui::{ - AnyElement, App, Entity, ImageSource, InteractiveElement, IntoElement, MouseButton, ObjectFit, - ParentElement, Styled, StyledImage, Window, canvas, div, img, px, rgb, surface, + AnyElement, App, Corners, Entity, ImageSource, InteractiveElement, IntoElement, MouseButton, + ObjectFit, ParentElement, Styled, StyledImage, Window, canvas, div, img, px, rgb, surface, }; use super::{ ElyShell, web_surface_frame::WebSurfaceFrame, web_surface_geometry::servo_scroll_delta_from_wheel_delta, }; -use ely_design_system::colors; +use ely_design_system::{colors, spacing}; pub(super) fn render_ready_web_surface( frame: &WebSurfaceFrame, @@ -20,7 +20,10 @@ pub(super) fn render_ready_web_surface( return render_web_surface( tab, state_entity, - surface(pixel_buffer.clone()).size_full().object_fit(ObjectFit::Fill), + surface(pixel_buffer.clone()) + .size_full() + .corner_radii(web_surface_corner_radii()) + .object_fit(ObjectFit::Fill), ); } @@ -53,6 +56,15 @@ pub(super) fn render_failed_web_surface( render_web_surface(tab, state_entity, error_page(message)) } +fn web_surface_corner_radii() -> Corners { + Corners { + top_left: px(0.0), + top_right: px(0.0), + bottom_right: px(spacing::RADIUS_CARD), + bottom_left: px(spacing::RADIUS_CARD), + } +} + fn error_page(message: &str) -> impl IntoElement { div() .size_full() diff --git a/crates/ely_app/src/shell/web_surface_worker.rs b/crates/ely_app/src/shell/web_surface_worker.rs new file mode 100644 index 0000000..c98996f --- /dev/null +++ b/crates/ely_app/src/shell/web_surface_worker.rs @@ -0,0 +1,336 @@ +use std::{ + collections::BTreeMap, + io, + sync::{Arc, Condvar, Mutex, mpsc}, + thread::JoinHandle, +}; + +use crate::services::servo_live::{ + ServoLiveClient, ServoLiveEnsureRequest, ServoLiveError, ServoLiveFrame, +}; + +/// IPC surface for the per-profile Servo sidecar. +/// +/// Production wraps [`ServoLiveClient`] directly; tests substitute a +/// fake. The contract: every call is blocking and may run for tens of +/// milliseconds. Implementations live on the worker thread, never the +/// UI thread. +pub(super) trait LiveRuntimeClient: Send { + fn ensure( + &mut self, + request: ServoLiveEnsureRequest, + ) -> Result, LiveRuntimeClientError>; + + fn poll(&mut self, tab_id: String) -> Result, LiveRuntimeClientError>; + + fn close(&mut self, tab_id: String) -> Result<(), LiveRuntimeClientError>; +} + +impl LiveRuntimeClient for ServoLiveClient { + fn ensure( + &mut self, + request: ServoLiveEnsureRequest, + ) -> Result, LiveRuntimeClientError> { + ServoLiveClient::ensure(self, request).map_err(LiveRuntimeClientError::from) + } + + fn poll(&mut self, tab_id: String) -> Result, LiveRuntimeClientError> { + ServoLiveClient::poll(self, tab_id).map_err(LiveRuntimeClientError::from) + } + + fn close(&mut self, tab_id: String) -> Result<(), LiveRuntimeClientError> { + ServoLiveClient::close(self, tab_id).map_err(LiveRuntimeClientError::from) + } +} + +#[derive(Debug)] +pub(super) enum LiveRuntimeClientError { + SidecarExited, + Message(String), +} + +impl LiveRuntimeClientError { + pub(super) fn is_sidecar_exited(&self) -> bool { + matches!(self, Self::SidecarExited) + } +} + +impl std::fmt::Display for LiveRuntimeClientError { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::SidecarExited => formatter.write_str("servo live sidecar exited"), + Self::Message(message) => formatter.write_str(message), + } + } +} + +impl From for LiveRuntimeClientError { + fn from(message: String) -> Self { + Self::Message(message) + } +} + +impl From for LiveRuntimeClientError { + fn from(error: ServoLiveError) -> Self { + if error.is_sidecar_process_unusable() { + return Self::SidecarExited; + } + Self::Message(error.to_string()) + } +} + +impl From for LiveRuntimeClientError { + fn from(error: io::Error) -> Self { + Self::Message(error.to_string()) + } +} + +/// Output of a worker request. +pub(super) enum WorkerResponse { + Frame { tab_id: String, frame: ServoLiveFrame }, + Failed { tab_id: String, message: String }, + SidecarExited, +} + +enum WorkerRequest { + Ensure(ServoLiveEnsureRequest), + Poll { tab_id: String }, +} + +struct WorkerQueue { + /// Latest request per tab. A new submission for a tab replaces any + /// earlier in-flight-but-not-yet-started request, so a flurry of + /// scrolls never piles up — the worker always processes the most + /// recent frame's worth of inputs. + pending: BTreeMap, + /// Close orders. Sent after pending is cleared for that tab so the + /// worker never closes a tab that still has live frames in flight. + closes: Vec, + /// True while the worker is processing a request. `wait_until_idle` + /// uses this alongside the queue emptiness to know when all + /// previously-submitted work has actually run. + in_flight: bool, + shutdown: bool, +} + +/// Owns a [`LiveRuntimeClient`] on a dedicated OS thread and exposes +/// a non-blocking API: submit ensure/poll/close, then drain responses. +/// +/// The UI thread never blocks on Servo IPC. Submissions push into a +/// coalescing queue (latest request per tab wins). The worker thread +/// drains the queue, runs the blocking IPC, and emits responses on a +/// `std::sync::mpsc` channel that the UI thread reads with `try_recv`. +pub(super) struct LiveRuntimeWorker { + queue: Arc<(Mutex, Condvar)>, + response_rx: mpsc::Receiver, + thread: Option>, +} + +impl LiveRuntimeWorker { + pub(super) fn new(client: Box) -> Result { + let queue = Arc::new(( + Mutex::new(WorkerQueue { + pending: BTreeMap::new(), + closes: Vec::new(), + in_flight: false, + shutdown: false, + }), + Condvar::new(), + )); + let (response_tx, response_rx) = mpsc::channel(); + let queue_for_thread = queue.clone(); + let thread = std::thread::Builder::new() + .name("ely-servo-live".to_string()) + .spawn(move || { + run_worker(client, queue_for_thread, response_tx); + }) + .map_err(|error| format!("failed to spawn servo live worker thread: {error}"))?; + Ok(Self { queue, response_rx, thread: Some(thread) }) + } + + pub(super) fn submit_ensure(&self, request: ServoLiveEnsureRequest) { + let tab_id = request.tab_id.clone(); + let (lock, cvar) = &*self.queue; + let mut q = match lock.lock() { + Ok(guard) => guard, + Err(poisoned) => poisoned.into_inner(), + }; + if q.shutdown { + return; + } + q.pending.insert(tab_id, WorkerRequest::Ensure(request)); + cvar.notify_one(); + } + + pub(super) fn submit_poll(&self, tab_id: String) { + let (lock, cvar) = &*self.queue; + let mut q = match lock.lock() { + Ok(guard) => guard, + Err(poisoned) => poisoned.into_inner(), + }; + if q.shutdown { + return; + } + // A pending Ensure already produces the latest frame after its + // run; don't downgrade it to a Poll. Only insert if nothing is + // queued. + q.pending.entry(tab_id.clone()).or_insert(WorkerRequest::Poll { tab_id }); + cvar.notify_one(); + } + + pub(super) fn submit_close(&self, tab_id: String) { + let (lock, cvar) = &*self.queue; + let mut q = match lock.lock() { + Ok(guard) => guard, + Err(poisoned) => poisoned.into_inner(), + }; + if q.shutdown { + return; + } + q.pending.remove(&tab_id); + q.closes.push(tab_id); + cvar.notify_one(); + } + + pub(super) fn drain_responses(&self) -> Vec { + let mut out = Vec::new(); + while let Ok(response) = self.response_rx.try_recv() { + out.push(response); + } + out + } + + /// Test-only barrier. Blocks the caller until the worker has + /// drained everything currently submitted. Production code never + /// waits — the whole point of the worker is that the UI thread + /// progresses without IPC latency. + #[cfg(test)] + pub(super) fn wait_until_idle(&self) { + let (lock, cvar) = &*self.queue; + let mut q = match lock.lock() { + Ok(guard) => guard, + Err(poisoned) => poisoned.into_inner(), + }; + while !q.pending.is_empty() || !q.closes.is_empty() || q.in_flight { + q = match cvar.wait(q) { + Ok(guard) => guard, + Err(poisoned) => poisoned.into_inner(), + }; + } + } +} + +impl Drop for LiveRuntimeWorker { + fn drop(&mut self) { + { + let (lock, cvar) = &*self.queue; + if let Ok(mut q) = lock.lock() { + q.shutdown = true; + cvar.notify_all(); + } + } + if let Some(handle) = self.thread.take() { + let _ = handle.join(); + } + } +} + +fn run_worker( + mut client: Box, + queue: Arc<(Mutex, Condvar)>, + response_tx: mpsc::Sender, +) { + let (lock, cvar) = &*queue; + loop { + let work = { + let mut q = match lock.lock() { + Ok(guard) => guard, + Err(poisoned) => poisoned.into_inner(), + }; + q.in_flight = false; + cvar.notify_all(); + while q.pending.is_empty() && q.closes.is_empty() && !q.shutdown { + q = match cvar.wait(q) { + Ok(guard) => guard, + Err(poisoned) => poisoned.into_inner(), + }; + } + if q.shutdown { + return; + } + let next = if let Some(close_id) = q.closes.pop() { + Work::Close(close_id) + } else { + let key = match q.pending.keys().next().cloned() { + Some(key) => key, + None => continue, + }; + let Some(request) = q.pending.remove(&key) else { + continue; + }; + Work::Request(request) + }; + q.in_flight = true; + next + }; + + let exit_after_dispatch = match work { + Work::Close(tab_id) => { + let _ = client.close(tab_id); + false + } + Work::Request(WorkerRequest::Ensure(request)) => { + let tab_id = request.tab_id.clone(); + dispatch_result(&response_tx, tab_id, client.ensure(request)) + } + Work::Request(WorkerRequest::Poll { tab_id }) => { + let request_tab_id = tab_id.clone(); + dispatch_result(&response_tx, request_tab_id, client.poll(tab_id)) + } + }; + + if exit_after_dispatch { + // Release the in-flight flag and wake any flush waiter + // before exiting so wait_until_idle doesn't block forever + // on a thread that has already returned. + let mut q = match lock.lock() { + Ok(guard) => guard, + Err(poisoned) => poisoned.into_inner(), + }; + q.in_flight = false; + cvar.notify_all(); + return; + } + } +} + +enum Work { + Close(String), + Request(WorkerRequest), +} + +/// Forward a single client result to the response channel. Returns +/// `true` when the worker should exit (sidecar process died). +fn dispatch_result( + response_tx: &mpsc::Sender, + tab_id: String, + result: Result, LiveRuntimeClientError>, +) -> bool { + match result { + Ok(Some(frame)) => { + let _ = response_tx.send(WorkerResponse::Frame { tab_id, frame }); + false + } + Ok(None) => false, + Err(error) => { + let exited = error.is_sidecar_exited(); + let message = error.to_string(); + let _ = response_tx.send(WorkerResponse::Failed { tab_id, message }); + if exited { + let _ = response_tx.send(WorkerResponse::SidecarExited); + return true; + } + false + } + } +} diff --git a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live.rs b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live.rs index ae21911..5408467 100644 --- a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live.rs +++ b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live.rs @@ -2,8 +2,7 @@ use std::{ collections::{HashMap, HashSet}, fs, io::{self, BufRead}, - thread, - time::{Duration, Instant}, + time::Instant, }; use ely_domain::{DEFAULT_ZOOM_PERCENT, ProfileId, TabId, UrlText}; @@ -11,7 +10,7 @@ use ely_servo_host::{ IOSurfaceIdentity, KeyboardTextRequest, MouseClickRequest, MouseHoverRequest, NavigationRequest, PageZoomRequest, PermissionDecision, PermissionRequest, RenderingContextKind, ResizeRequest, ScrollRequest, ServoHost, ServoSurfaceSize, - SoftwareServoHost, + SoftwareServoHost, WebViewState, }; use super::args::LiveArgs; @@ -24,12 +23,6 @@ use super::live_protocol::{ }; use super::perf::{FramePerfAggregator, FramePerfSummary, elapsed_ns}; -/// Per-`Ensure` budget for Servo to paint after input dispatch. -/// 250 ms catches the common click + paint round trip within the -/// same `Ensure` instead of waiting for the next 16 ms `Poll`. -const LIVE_FRAME_WAIT_TIMEOUT: Duration = Duration::from_millis(250); -const LIVE_FRAME_WAIT_INTERVAL: Duration = Duration::from_millis(2); - pub(super) fn run_live(args: LiveArgs) -> Result<(), LiveSidecarError> { let LiveArgs { profile_data_dir, iosurface_mach_service, rendering_context_kind } = args; fs::create_dir_all(&profile_data_dir)?; @@ -64,7 +57,7 @@ pub(super) fn run_live(args: LiveArgs) -> Result<(), LiveSidecarError> { // matching stop is the `stdout.flush()` inside // `write_outcome`. let frame_started_at = Instant::now(); - let mut outcome = match serde_json::from_str::(&line) { + let outcome = match serde_json::from_str::(&line) { Ok(request) => handle_request( &mut host, &mut sessions, @@ -75,7 +68,11 @@ pub(super) fn run_live(args: LiveArgs) -> Result<(), LiveSidecarError> { Err(error) => Err(LiveSidecarError::Json(error)), }; #[cfg(all(feature = "hardware-render", target_os = "macos"))] - send_surface_port_if_needed(iosurface_mach_sender.as_mut(), &mut outcome); + let outcome = { + let mut outcome = outcome; + send_surface_port_if_needed(iosurface_mach_sender.as_mut(), &mut outcome); + outcome + }; write_outcome(&mut stdout, &mut perf, &mut pending_summary, outcome, frame_started_at)?; } @@ -153,12 +150,11 @@ fn handle_request( typed_text, }; if apply_input(host, session, input)? { - // Tell poll_frame to actually wait for Servo to paint - // a response to this input. The visible-content gate - // is bypassed on the hardware path inside poll_frame, - // so we return on the first `has_pending_frame=true` - // (~3 ms in practice) rather than burning the full - // LIVE_FRAME_WAIT_TIMEOUT. + // The app tick calls this sidecar synchronously from + // GPUI's update path. Mark that a fresh frame is + // desired, then let poll_frame take one event-loop + // step; a later 16 ms app tick will poll again if + // Servo has not painted yet. session.awaiting_visible_frame = true; } let webview_id = session.webview_id.clone(); @@ -363,33 +359,24 @@ fn poll_frame( session: &mut LiveSession, rendering_context_kind: RenderingContextKind, ) -> Result { - let started_at = Instant::now(); - - loop { - host.tick(); - let snapshot = host.snapshot(&session.webview_id)?; - if snapshot.has_pending_frame() { - let (outcome, has_visible_content) = - paint_pending_frame(host, session, rendering_context_kind)?; - if has_visible_content { - session.awaiting_visible_frame = false; - session.ever_visible_frame = true; - return Ok(outcome); - } - if !session.awaiting_visible_frame { - return Ok(outcome); - } - } - - if !session.awaiting_visible_frame { - return Ok(LiveOutcome::empty()); - } - if started_at.elapsed() >= LIVE_FRAME_WAIT_TIMEOUT { - return Ok(LiveOutcome::empty()); - } - - thread::sleep(LIVE_FRAME_WAIT_INTERVAL); + host.tick(); + let snapshot = host.snapshot(&session.webview_id)?; + if !snapshot.has_pending_frame() { + return Ok(LiveOutcome::empty()); } + + let (outcome, has_visible_content) = + paint_pending_frame(host, session, rendering_context_kind)?; + if has_visible_content { + session.awaiting_visible_frame = false; + session.ever_visible_frame = true; + return Ok(outcome); + } + if !session.awaiting_visible_frame { + return Ok(outcome); + } + + Ok(LiveOutcome::empty()) } fn paint_pending_frame( @@ -418,7 +405,7 @@ fn paint_readback_frame( let encode_started_at = Instant::now(); let has_visible_content = session.ever_visible_frame || (frame.non_white_pixel_count() > 0 && frame.content_pixel_count() > 0); - let report = LiveFrameReport::new(&snapshot, &frame); + let report = LiveFrameReport::new(&snapshot, &frame, session.device_pixel_ratio()); let encode_ns = elapsed_ns(encode_started_at); let timings = PartialFrameTimings { paint_ns, encode_ns }; Ok((LiveOutcome::from_frame(report, frame, timings), has_visible_content)) @@ -429,17 +416,51 @@ fn paint_hardware_surface_frame( host: &mut SoftwareServoHost, session: &LiveSession, ) -> Result<(LiveOutcome, bool), LiveSidecarError> { + if !session.ever_visible_frame { + return paint_initial_hardware_surface_frame(host, session); + } + let paint_started_at = Instant::now(); host.paint_without_readback(&session.webview_id)?; let snapshot = host.snapshot(&session.webview_id)?; let paint_ns = elapsed_ns(paint_started_at); let encode_started_at = Instant::now(); - let report = LiveFrameReport::new_hardware_surface(&snapshot, session.width, session.height); + let report = LiveFrameReport::new_hardware_surface( + &snapshot, + session.width, + session.height, + session.device_pixel_ratio(), + ); let encode_ns = elapsed_ns(encode_started_at); let timings = PartialFrameTimings { paint_ns, encode_ns }; Ok((LiveOutcome::from_report(report, timings), true)) } +#[cfg(all(feature = "hardware-render", target_os = "macos"))] +fn paint_initial_hardware_surface_frame( + host: &mut SoftwareServoHost, + session: &LiveSession, +) -> Result<(LiveOutcome, bool), LiveSidecarError> { + let paint_started_at = Instant::now(); + host.paint(&session.webview_id)?; + let snapshot = host.snapshot(&session.webview_id)?; + let frame = host.last_rendered_frame()?; + let paint_ns = elapsed_ns(paint_started_at); + let encode_started_at = Instant::now(); + let report = LiveFrameReport::new(&snapshot, &frame, session.device_pixel_ratio()); + let has_visible_content = frame.non_white_pixel_count() > 0 + && frame.content_pixel_count() > 0 + && hardware_snapshot_has_visible_document(&snapshot); + let encode_ns = elapsed_ns(encode_started_at); + let timings = PartialFrameTimings { paint_ns, encode_ns }; + Ok((LiveOutcome::from_frame(report, frame, timings), has_visible_content)) +} + +#[cfg(all(feature = "hardware-render", target_os = "macos"))] +fn hardware_snapshot_has_visible_document(snapshot: &ely_servo_host::WebViewSnapshot) -> bool { + snapshot.title().is_some() || matches!(snapshot.state(), WebViewState::Complete) +} + #[derive(Clone)] struct LiveSession { webview_id: ely_domain::WebViewId, @@ -468,12 +489,12 @@ struct LiveSession { } impl LiveSession { - fn new(webview_id: ely_domain::WebViewId, width: u32, height: u32) -> Self { + fn new(webview_id: ely_domain::WebViewId, _width: u32, _height: u32) -> Self { Self { webview_id, requested_url: String::new(), - width: width.max(1), - height: height.max(1), + width: 0, + height: 0, page_zoom_percent: DEFAULT_ZOOM_PERCENT, hidpi_scale_milli: 0, scroll_x: 0, @@ -482,9 +503,32 @@ impl LiveSession { ever_visible_frame: false, } } + + fn device_pixel_ratio(&self) -> f32 { + hidpi_scale_milli_to_f32(self.hidpi_scale_milli) + } } fn positive_scroll_component(current: i32, delta: i32) -> i32 { let value = i64::from(current) + i64::from(delta); value.clamp(0, i64::from(i32::MAX)) as i32 } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn new_live_session_forces_first_resize_after_hidpi() { + let session = LiveSession::new(ely_domain::WebViewId::new(), 1280, 720); + + assert_ne!( + session.width, 1280, + "first apply_layout must resize after hidpi has been pushed", + ); + assert_ne!( + session.height, 720, + "first apply_layout must resize after hidpi has been pushed", + ); + } +} diff --git a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_output.rs b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_output.rs index ce1d559..4b48068 100644 --- a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_output.rs +++ b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_output.rs @@ -4,9 +4,9 @@ use std::{ time::{Duration, Instant}, }; -use ely_servo_host::SoftwareServoHost; #[cfg(any(test, all(feature = "hardware-render", target_os = "macos")))] -use ely_servo_host::{IOSurfaceHandle, IOSurfaceIdentity}; +use ely_servo_host::IOSurfaceHandle; +use ely_servo_host::{IOSurfaceIdentity, SoftwareServoHost}; use super::live_protocol::{LiveOutcome, LiveSidecarError, PartialFrameTimings}; use super::perf::{FramePerfAggregator, FramePerfSummary, FrameStageTimings, elapsed_ns}; @@ -35,12 +35,18 @@ pub(super) fn populate_surface_fields( if outcome.response.frame.is_none() { return; } + if outcome.frame.is_some() { + return; + } #[cfg(all(feature = "hardware-render", target_os = "macos"))] { let Ok(Some(identity)) = host.peek_iosurface_identity(webview_id) else { return; }; - align_report_to_surface_identity(outcome, identity); + if let Err(message) = require_report_matches_surface_identity(outcome, identity) { + *outcome = LiveOutcome::error(message); + return; + } let handle = if surface_has_been_published(published_surface_ids, tab_id, identity) { None } else { @@ -105,11 +111,21 @@ fn handle_matches_identity(handle: IOSurfaceHandle, identity: IOSurfaceIdentity) } #[cfg(any(test, all(feature = "hardware-render", target_os = "macos")))] -fn align_report_to_surface_identity(outcome: &mut LiveOutcome, identity: IOSurfaceIdentity) { - if let Some(frame) = outcome.response.frame.as_mut() { - frame.width = identity.width; - frame.height = identity.height; +fn require_report_matches_surface_identity( + outcome: &LiveOutcome, + identity: IOSurfaceIdentity, +) -> Result<(), String> { + let Some(frame) = outcome.response.frame.as_ref() else { + return Ok(()); + }; + if frame.width == identity.width && frame.height == identity.height { + return Ok(()); } + + Err(format!( + "servo hardware surface size {}x{} did not match frame report {}x{}", + identity.width, identity.height, frame.width, frame.height, + )) } /// Serialise the response then stream the optional raw RGBA frame on @@ -181,7 +197,7 @@ mod tests { live_protocol::{LiveFrameReport, LiveOutcome, PartialFrameTimings}, perf::FramePerfAggregator, }; - use super::{align_report_to_surface_identity, surface_publication_for, write_outcome}; + use super::{require_report_matches_surface_identity, surface_publication_for, write_outcome}; #[test] fn unpublished_surface_without_handle_leaves_selector_empty() { @@ -235,17 +251,22 @@ mod tests { } #[test] - fn hardware_report_uses_surface_identity_dimensions() -> Result<(), Box> { - let mut outcome = LiveOutcome::from_report( + fn hardware_report_mismatch_is_reported() -> Result<(), Box> { + let outcome = LiveOutcome::from_report( report_with_size(2180, 1586), PartialFrameTimings { paint_ns: 1_000, encode_ns: 2_000 }, ); - align_report_to_surface_identity(&mut outcome, identity(7, 2168, 1566)); + let error = match require_report_matches_surface_identity(&outcome, identity(7, 2168, 1566)) + { + Ok(()) => return Err("mismatched IOSurface dimensions must be reported".into()), + Err(error) => error, + }; - let report = outcome.response.frame.ok_or("report must remain present")?; - assert_eq!(report.width, 2168); - assert_eq!(report.height, 1566); + assert_eq!( + error, + "servo hardware surface size 2168x1566 did not match frame report 2180x1586", + ); Ok(()) } @@ -314,6 +335,9 @@ mod tests { state: "complete", width, height, + device_pixel_ratio: 1.0, + css_viewport_width: width, + css_viewport_height: height, rgba_byte_count: 0, non_white_pixel_count: 0, content_pixel_count: 0, diff --git a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_protocol.rs b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_protocol.rs index 785e807..ed8d44b 100644 --- a/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_protocol.rs +++ b/crates/ely_servo_host/src/bin/ely_servo_sidecar/live_protocol.rs @@ -176,6 +176,9 @@ pub(super) struct LiveFrameReport { pub state: &'static str, pub width: u32, pub height: u32, + pub device_pixel_ratio: f32, + pub css_viewport_width: u32, + pub css_viewport_height: u32, pub rgba_byte_count: usize, pub non_white_pixel_count: u64, pub content_pixel_count: u64, @@ -183,13 +186,18 @@ pub(super) struct LiveFrameReport { } impl LiveFrameReport { - pub fn new(snapshot: &WebViewSnapshot, frame: &RenderedFrame) -> Self { + pub fn new(snapshot: &WebViewSnapshot, frame: &RenderedFrame, device_pixel_ratio: f32) -> Self { + let (css_viewport_width, css_viewport_height) = + css_viewport_size(frame.width(), frame.height(), device_pixel_ratio); Self { loaded_url: snapshot.url().map(str::to_string), title: snapshot.title().map(str::to_string), state: state_label(snapshot.state()), width: frame.width(), height: frame.height(), + device_pixel_ratio, + css_viewport_width, + css_viewport_height, rgba_byte_count: frame.rgba_bytes().len(), non_white_pixel_count: frame.non_white_pixel_count(), content_pixel_count: frame.content_pixel_count(), @@ -200,13 +208,23 @@ impl LiveFrameReport { /// Build a report for the hardware IOSurface path. Pixel metrics /// are unavailable because the path skips framebuffer readback. #[cfg(all(feature = "hardware-render", target_os = "macos"))] - pub fn new_hardware_surface(snapshot: &WebViewSnapshot, width: u32, height: u32) -> Self { + pub fn new_hardware_surface( + snapshot: &WebViewSnapshot, + width: u32, + height: u32, + device_pixel_ratio: f32, + ) -> Self { + let (css_viewport_width, css_viewport_height) = + css_viewport_size(width, height, device_pixel_ratio); Self { loaded_url: snapshot.url().map(str::to_string), title: snapshot.title().map(str::to_string), state: state_label(snapshot.state()), width, height, + device_pixel_ratio, + css_viewport_width, + css_viewport_height, rgba_byte_count: 0, non_white_pixel_count: 0, content_pixel_count: 0, @@ -215,6 +233,18 @@ impl LiveFrameReport { } } +fn css_viewport_size(width: u32, height: u32, device_pixel_ratio: f32) -> (u32, u32) { + let dpr = if device_pixel_ratio.is_finite() && device_pixel_ratio > 0.0 { + device_pixel_ratio + } else { + 1.0 + }; + ( + ((width as f32) / dpr).round().max(1.0) as u32, + ((height as f32) / dpr).round().max(1.0) as u32, + ) +} + fn state_label(state: &WebViewState) -> &'static str { match state { WebViewState::Created => "created", diff --git a/crates/ely_servo_host/src/hardware_rendering_context.rs b/crates/ely_servo_host/src/hardware_rendering_context.rs index ede1263..80952a9 100644 --- a/crates/ely_servo_host/src/hardware_rendering_context.rs +++ b/crates/ely_servo_host/src/hardware_rendering_context.rs @@ -50,7 +50,9 @@ use euclid::Size2D; use gleam::gl::{self, Gl}; use image::RgbaImage; use servo::{DeviceIntRect, RenderingContext}; -use surfman::chains::{PreserveBuffer, SwapChain}; +use surfman::chains::{PreserveBuffer, SwapChain, SwapChainAPI}; +#[cfg(target_os = "macos")] +use surfman::platform::macos::cgl::surface::NativeSurface; use surfman::{ Connection, Context, ContextAttributeFlags, ContextAttributes, Device, Error as SurfmanError, GLApi, NativeWidget, Surface, SurfaceAccess, SurfaceType, @@ -64,6 +66,10 @@ pub struct HardwareOffscreenContext { size: Cell>, inner: SurfmanInner, swap_chain: SwapChain, + #[cfg(target_os = "macos")] + held_presented_surface: RefCell>, + #[cfg(target_os = "macos")] + last_presented_iosurface: RefCell>, } impl HardwareOffscreenContext { @@ -86,7 +92,15 @@ impl HardwareOffscreenContext { inner.bind_surface(surface)?; inner.make_current()?; let swap_chain = inner.create_attached_swap_chain()?; - Ok(Self { size: Cell::new(size), inner, swap_chain }) + Ok(Self { + size: Cell::new(size), + inner, + swap_chain, + #[cfg(target_os = "macos")] + held_presented_surface: RefCell::new(None), + #[cfg(target_os = "macos")] + last_presented_iosurface: RefCell::new(None), + }) } } @@ -94,6 +108,8 @@ impl Drop for HardwareOffscreenContext { fn drop(&mut self) { let device = &mut self.inner.device.borrow_mut(); let context = &mut self.inner.context.borrow_mut(); + #[cfg(target_os = "macos")] + self.destroy_held_presented_surface(device, context); let _ = self.swap_chain.destroy(device, context); } } @@ -120,6 +136,8 @@ impl RenderingContext for HardwareOffscreenContext { let device = &mut self.inner.device.borrow_mut(); let context = &mut self.inner.context.borrow_mut(); + #[cfg(target_os = "macos")] + self.destroy_held_presented_surface(device, context); let size = Size2D::new(size.width as i32, size.height as i32); let _ = self.swap_chain.resize(device, context, size); } @@ -127,7 +145,11 @@ impl RenderingContext for HardwareOffscreenContext { fn present(&self) { let device = &mut self.inner.device.borrow_mut(); let context = &mut self.inner.context.borrow_mut(); + #[cfg(target_os = "macos")] + self.recycle_held_presented_surface(); let _ = self.swap_chain.swap_buffers(device, context, PreserveBuffer::No); + #[cfg(target_os = "macos")] + self.capture_presented_iosurface(device); } fn make_current(&self) -> Result<(), SurfmanError> { @@ -152,54 +174,64 @@ use crate::iosurface_handle::{IOSurfaceHandle, IOSurfaceIdentity}; #[cfg(target_os = "macos")] impl HardwareOffscreenContext { - /// Cheap, non-mutating identity probe of the currently bound - /// surface. Reads `Device::context_surface_info` (no unbind, no - /// mach port creation) so callers can dedup before paying the - /// price of `current_iosurface_mach_port`. - pub fn peek_iosurface_identity(&self) -> Result { - let device = self.inner.device.borrow(); - let context = self.inner.context.borrow(); - let info = device.context_surface_info(&context)?.ok_or(SurfmanError::Failed)?; - Ok(IOSurfaceIdentity { - surface_id: info.id.0 as u64, - width: u32::try_from(info.size.width).unwrap_or(0), - height: u32::try_from(info.size.height).unwrap_or(0), - }) + /// Cheap, non-mutating identity probe of the IOSurface that was + /// just presented. Used by the sidecar to dedup mach port creation. + pub fn peek_iosurface_identity(&self) -> Result, SurfmanError> { + Ok(self.last_presented_iosurface.borrow().as_ref().map(|surface| surface.identity)) } - /// Snapshot the IOSurface currently bound to the context and - /// return its mach port name plus dimensions and stable surface - /// id. Increments the IOSurface's mach-port use count; the + /// Snapshot the just-presented IOSurface and return its mach port + /// name plus dimensions and stable surface id. Increments the + /// IOSurface's mach-port use count; the /// receiving process holds it via `IOSurfaceLookupFromMachPort` and /// is responsible for `mach_port_deallocate` once the import is /// finished. - /// - /// Implementation note: surfman's CGL backend keeps the bound - /// surface inside the GL context. To inspect it we temporarily - /// `unbind_surface_from_context`, call `device.native_surface()` - /// (which retains the `IOSurfaceRef`), then `bind_surface_to_context` - /// again. The unbind path calls `glFlush` so the IOSurface contents - /// are consistent for any reader importing it after this returns. pub fn current_iosurface_mach_port(&self) -> Result { - let device = &mut self.inner.device.borrow_mut(); - let context = &mut self.inner.context.borrow_mut(); - // `new` always binds a surface and `current_iosurface_mach_port` - // is the only method that unbinds; the `None` branch only fires - // if the invariant has been broken from outside. - let surface = - device.unbind_surface_from_context(context)?.ok_or(SurfmanError::Failed)?; - let native = device.native_surface(&surface); - let mach_port = native.0.create_mach_port(); - let info = device.surface_info(&surface); - let handle = IOSurfaceHandle { + let presented = self.last_presented_iosurface.borrow(); + let presented = presented.as_ref().ok_or(SurfmanError::Failed)?; + let mach_port = presented.native.0.create_mach_port(); + Ok(IOSurfaceHandle { mach_port_name: mach_port, + surface_id: presented.identity.surface_id, + width: presented.identity.width, + height: presented.identity.height, + }) + } + + fn capture_presented_iosurface(&self, device: &mut Device) { + let Some(surface) = self.swap_chain.take_pending_surface() else { + self.last_presented_iosurface.borrow_mut().take(); + return; + }; + let info = device.surface_info(&surface); + let native = device.native_surface(&surface); + let identity = IOSurfaceIdentity { surface_id: info.id.0 as u64, width: u32::try_from(info.size.width).unwrap_or(0), height: u32::try_from(info.size.height).unwrap_or(0), }; - device.bind_surface_to_context(context, surface).map_err(|(error, _)| error)?; - Ok(handle) + self.held_presented_surface.replace(Some(surface)); + self.last_presented_iosurface.replace(Some(PresentedIOSurface { identity, native })); } + + fn recycle_held_presented_surface(&self) { + if let Some(surface) = self.held_presented_surface.borrow_mut().take() { + self.swap_chain.recycle_surface(surface); + } + } + + fn destroy_held_presented_surface(&self, device: &mut Device, context: &mut Context) { + self.last_presented_iosurface.borrow_mut().take(); + if let Some(mut surface) = self.held_presented_surface.borrow_mut().take() { + let _ = device.destroy_surface(context, &mut surface); + } + } +} + +#[cfg(target_os = "macos")] +struct PresentedIOSurface { + identity: IOSurfaceIdentity, + native: NativeSurface, } /// Trimmed mirror of `paint_api::rendering_context::SurfmanRenderingContext`. @@ -281,12 +313,10 @@ impl SurfmanInner { fn bind_surface(&self, surface: Surface) -> Result<(), SurfmanError> { let device = &self.device.borrow(); let context = &mut self.context.borrow_mut(); - device - .bind_surface_to_context(context, surface) - .map_err(|(err, mut surface)| { - let _ = device.destroy_surface(context, &mut surface); - err - })?; + device.bind_surface_to_context(context, surface).map_err(|(err, mut surface)| { + let _ = device.destroy_surface(context, &mut surface); + err + })?; Ok(()) } diff --git a/crates/ely_servo_host/src/lib.rs b/crates/ely_servo_host/src/lib.rs index 06b57be..5bec4e1 100644 --- a/crates/ely_servo_host/src/lib.rs +++ b/crates/ely_servo_host/src/lib.rs @@ -19,12 +19,12 @@ mod runtime_webview; pub use error::ServoHostError; #[cfg(feature = "hardware-render")] pub use hardware_rendering_context::HardwareOffscreenContext; -pub use iosurface_handle::{IOSurfaceHandle, IOSurfaceIdentity}; pub use host::{ - HidpiScaleRequest, KeyboardTextRequest, MouseClickRequest, MouseDragRequest, - MouseHoverRequest, NavigationRequest, PageZoomRequest, PermissionDecision, PermissionRequest, - RenderedFrame, RenderedFrameSummary, ResizeRequest, ScreenshotRequest, ScrollRequest, - ServoHost, TouchTapRequest, WebViewSnapshot, WebViewState, + HidpiScaleRequest, KeyboardTextRequest, MouseClickRequest, MouseDragRequest, MouseHoverRequest, + NavigationRequest, PageZoomRequest, PermissionDecision, PermissionRequest, RenderedFrame, + RenderedFrameSummary, ResizeRequest, ScreenshotRequest, ScrollRequest, ServoHost, + TouchTapRequest, WebViewSnapshot, WebViewState, }; +pub use iosurface_handle::{IOSurfaceHandle, IOSurfaceIdentity}; #[cfg(feature = "servo-engine")] pub use runtime::{RenderingContextKind, ServoSurfaceSize, SoftwareServoHost}; diff --git a/crates/ely_servo_host/src/runtime.rs b/crates/ely_servo_host/src/runtime.rs index 84cb232..8c6669f 100644 --- a/crates/ely_servo_host/src/runtime.rs +++ b/crates/ely_servo_host/src/runtime.rs @@ -108,8 +108,8 @@ impl SoftwareServoHost { /// Paint and present the webview's current surface while leaving /// framebuffer readback to callers that explicitly need RGBA - /// bytes. The live hardware path uses this before publishing the - /// IOSurface handle to the renderer process. + /// bytes. The live hardware path exports the just-presented + /// IOSurface from the rendering context. pub fn paint_without_readback(&mut self, webview_id: &WebViewId) -> Result<(), ServoHostError> { self.paint_webview(webview_id, false).map(|_| ()) } @@ -204,9 +204,13 @@ impl ServoHost for SoftwareServoHost { webview.delegate.set_state(WebViewState::Loading); if should_create_initial_document { + let hidpi_scale_factor = webview.webview.hidpi_scale_factor(); webview.webview = WebViewBuilder::new(&servo, webview.rendering_context.clone()) .delegate(webview.delegate.clone()) .url(url) + // The live path pushes DPR before first navigation. Preserve that scale when + // replacing the about:blank WebView so CSS viewport = physical surface / DPR. + .hidpi_scale_factor(hidpi_scale_factor) .build(); // Cosmetic: makes the freshly built WebView paint its // first frame. The input-accepting invariant lives in @@ -444,10 +448,7 @@ impl SoftwareServoHost { let Some(hardware) = webview.hardware_context.as_ref() else { return Ok(None); }; - hardware - .peek_iosurface_identity() - .map(Some) - .map_err(|_| ServoHostError::RenderingContextUnavailable) + hardware.peek_iosurface_identity().map_err(|_| ServoHostError::RenderingContextUnavailable) } /// Mint a fresh mach port for the IOSurface bound to this diff --git a/crates/ely_servo_host/tests/live_perf_bench.rs b/crates/ely_servo_host/tests/live_perf_bench.rs index 3ab2b05..8ee956e 100644 --- a/crates/ely_servo_host/tests/live_perf_bench.rs +++ b/crates/ely_servo_host/tests/live_perf_bench.rs @@ -82,6 +82,12 @@ struct LiveFrameReport { width: u32, #[serde(default)] height: u32, + #[serde(default)] + device_pixel_ratio: f32, + #[serde(default)] + css_viewport_width: u32, + #[serde(default)] + css_viewport_height: u32, } #[derive(Deserialize, Debug, Clone)] @@ -127,16 +133,16 @@ fn run_live_bench() -> Result<(), Box> { let stdout = child.stdout.take().ok_or("sidecar stdout missing")?; let mut reader = BufReader::new(stdout); - let outcome = - match drive_bench(&mut stdin, &mut reader, &kind, &tab, &profile_id, &url, frames) { - Ok(outcome) => outcome, - Err(error) => { - drop(stdin); - let _ = child.kill(); - cleanup(&profile_data_dir)?; - return Err(error); - } - }; + let outcome = match drive_bench(&mut stdin, &mut reader, &kind, &tab, &profile_id, &url, frames) + { + Ok(outcome) => outcome, + Err(error) => { + drop(stdin); + let _ = child.kill(); + cleanup(&profile_data_dir)?; + return Err(error); + } + }; drop(stdin); let _ = child.wait(); @@ -146,8 +152,8 @@ fn run_live_bench() -> Result<(), Box> { print_surface_handles(&kind, &outcome.surface_handles); print_current_surface_summary(&kind, &outcome.current_surface_ids); eprintln!( - "\n=== ELY_PERF_KIND={kind} rgba_bytes_received={} ===", - outcome.rgba_bytes_received + "\n=== ELY_PERF_KIND={kind} bootstrap_rgba_bytes={} steady_state_rgba_bytes={} ===", + outcome.bootstrap_rgba_bytes, outcome.steady_state_rgba_bytes, ); assert!( !outcome.summaries.is_empty(), @@ -174,17 +180,10 @@ fn run_live_bench() -> Result<(), Box> { !outcome.current_surface_ids.is_empty(), "hardware path must report current_surface_id on every frame" ); - // T10.6: once the receiver samples the IOSurface directly, - // the sidecar drops the RGBA payload. The initial navigate - // response may still carry bytes (no current_surface_id yet - // because surfman hasn't bound the painted surface), but the - // steady-state per-frame cost must be zero. - assert!( - outcome.rgba_bytes_received < (frames as u64) * 1_024, - "hardware path leaked {} RGBA bytes across {} frames \ - (expected ~0 — the wire-drop optimisation regressed)", - outcome.rgba_bytes_received, - frames + 1, + assert_eq!( + outcome.steady_state_rgba_bytes, 0, + "hardware path leaked {} RGBA bytes while scrolling", + outcome.steady_state_rgba_bytes, ); } else { assert!( @@ -198,10 +197,11 @@ fn run_live_bench() -> Result<(), Box> { // Software path keeps streaming pixels — every frame must // carry a full RGBA payload. let viewport_bytes = (1024u64) * (768u64) * 4; + let total_rgba_bytes = outcome.bootstrap_rgba_bytes + outcome.steady_state_rgba_bytes; assert!( - outcome.rgba_bytes_received >= viewport_bytes, + total_rgba_bytes >= viewport_bytes, "software path delivered only {} bytes — expected at least one full frame ({})", - outcome.rgba_bytes_received, + total_rgba_bytes, viewport_bytes, ); } @@ -212,7 +212,8 @@ struct BenchOutcome { summaries: Vec, surface_handles: Vec, current_surface_ids: Vec, - rgba_bytes_received: u64, + bootstrap_rgba_bytes: u64, + steady_state_rgba_bytes: u64, } fn spawn_sidecar(kind: &str, profile_data_dir: &PathBuf) -> Result> { @@ -241,23 +242,22 @@ fn drive_bench( let mut summaries = Vec::new(); let mut surface_handles = Vec::new(); let mut current_surface_ids = Vec::new(); - let mut rgba_bytes_received: u64 = 0; + let mut bootstrap_rgba_bytes: u64 = 0; + let mut steady_state_rgba_bytes: u64 = 0; let navigate = build_ensure(tab, profile_id, url, 0, 0, false); write_request(stdin, &navigate)?; let response = read_response(reader, RESPONSE_TIMEOUT)?; + assert_frame_viewport_report(&response); record_summary(&response, kind, &mut summaries); record_surface_handle(&response, kind, &mut surface_handles); record_current_surface_id(&response, &mut current_surface_ids); - rgba_bytes_received += response.frame.as_ref().map_or(0, |f| f.rgba_byte_count as u64); + record_rgba_bytes(&response, &mut bootstrap_rgba_bytes, &mut steady_state_rgba_bytes); let mut accumulated_scroll = 0; for frame_index in 0..frames { - let scroll_delta_y = if frame_index % 80 == 79 { - -SCROLL_STEP_PX * 60 - } else { - SCROLL_STEP_PX - }; + let scroll_delta_y = + if frame_index % 80 == 79 { -SCROLL_STEP_PX * 60 } else { SCROLL_STEP_PX }; accumulated_scroll += scroll_delta_y; let request = build_ensure(tab, profile_id, url, 0, scroll_delta_y, true); write_request(stdin, &request)?; @@ -265,14 +265,56 @@ fn drive_bench( if let Some(error) = response.error.as_ref() { return Err(format!("sidecar error at frame {frame_index}: {error}").into()); } + assert_frame_viewport_report(&response); record_summary(&response, kind, &mut summaries); record_surface_handle(&response, kind, &mut surface_handles); record_current_surface_id(&response, &mut current_surface_ids); - rgba_bytes_received += response.frame.as_ref().map_or(0, |f| f.rgba_byte_count as u64); + record_rgba_bytes(&response, &mut bootstrap_rgba_bytes, &mut steady_state_rgba_bytes); } let _ = accumulated_scroll; - Ok(BenchOutcome { summaries, surface_handles, current_surface_ids, rgba_bytes_received }) + Ok(BenchOutcome { + summaries, + surface_handles, + current_surface_ids, + bootstrap_rgba_bytes, + steady_state_rgba_bytes, + }) +} + +fn record_rgba_bytes( + response: &LiveResponse, + bootstrap_rgba_bytes: &mut u64, + steady_state_rgba_bytes: &mut u64, +) { + let rgba_byte_count = response.frame.as_ref().map_or(0, |frame| frame.rgba_byte_count as u64); + if response.current_surface_id.is_some() { + *steady_state_rgba_bytes += rgba_byte_count; + } else { + *bootstrap_rgba_bytes += rgba_byte_count; + } +} + +fn assert_frame_viewport_report(response: &LiveResponse) { + let Some(frame) = response.frame.as_ref() else { + return; + }; + let dpr = if frame.device_pixel_ratio.is_finite() && frame.device_pixel_ratio > 0.0 { + frame.device_pixel_ratio + } else { + 1.0 + }; + let expected_width = ((frame.width as f32) / dpr).round().max(1.0) as u32; + let expected_height = ((frame.height as f32) / dpr).round().max(1.0) as u32; + + assert_eq!( + frame.css_viewport_width, expected_width, + "CSS viewport width must match physical width divided by DPR", + ); + assert_eq!( + frame.css_viewport_height, expected_height, + "CSS viewport height must match physical height divided by DPR", + ); } fn record_surface_handle( @@ -314,9 +356,7 @@ fn print_current_surface_summary(kind: &str, current_surface_ids: &[u64]) { for id in current_surface_ids { *counts.entry(*id).or_default() += 1; } - eprintln!( - "\n=== ELY_PERF_KIND={kind} current_surface_id histogram (per-frame selector) ===", - ); + eprintln!("\n=== ELY_PERF_KIND={kind} current_surface_id histogram (per-frame selector) ===",); for (surface_id, count) in counts.iter() { eprintln!("surface_id=0x{:x} frames={}", surface_id, count); } @@ -332,11 +372,7 @@ fn build_ensure( ) -> String { let hover_x = if include_hover { Some(256u32) } else { None }; let hover_y = if include_hover { Some(256u32) } else { None }; - let scroll_point = if scroll_dx != 0 || scroll_dy != 0 { - Some((256u32, 256u32)) - } else { - None - }; + let scroll_point = if scroll_dx != 0 || scroll_dy != 0 { Some((256u32, 256u32)) } else { None }; let hover_x_json = match hover_x { Some(value) => format!("{value}"), None => "null".to_string(), @@ -417,17 +453,22 @@ fn read_response_with_bytes( fn record_summary(response: &LiveResponse, kind: &str, summaries: &mut Vec) { if let Some(perf) = response.perf.as_ref() { - assert_eq!( - perf.context, kind, - "sidecar context label must match requested kind" - ); + assert_eq!(perf.context, kind, "sidecar context label must match requested kind"); eprintln!( "[perf {kind}] window={} paint p50/p95/p99={}/{}/{} encode {}/{}/{} write {}/{}/{} total {}/{}/{} (µs)", perf.window, - perf.paint_p50_us, perf.paint_p95_us, perf.paint_p99_us, - perf.encode_p50_us, perf.encode_p95_us, perf.encode_p99_us, - perf.write_p50_us, perf.write_p95_us, perf.write_p99_us, - perf.total_p50_us, perf.total_p95_us, perf.total_p99_us, + perf.paint_p50_us, + perf.paint_p95_us, + perf.paint_p99_us, + perf.encode_p50_us, + perf.encode_p95_us, + perf.encode_p99_us, + perf.write_p50_us, + perf.write_p95_us, + perf.write_p99_us, + perf.total_p50_us, + perf.total_p95_us, + perf.total_p99_us, ); summaries.push(perf.clone()); } @@ -438,19 +479,35 @@ fn print_summaries(kind: &str, frames: u32, summaries: &[FramePerfSummary]) { eprintln!( "{:<8} {:>10} {:>10} {:>10} {:>10} {:>10} {:>10} {:>10} {:>10} {:>10} {:>10} {:>10} {:>10}", "win", - "paint50", "paint95", "paint99", - "enc50", "enc95", "enc99", - "wr50", "wr95", "wr99", - "tot50", "tot95", "tot99", + "paint50", + "paint95", + "paint99", + "enc50", + "enc95", + "enc99", + "wr50", + "wr95", + "wr99", + "tot50", + "tot95", + "tot99", ); for summary in summaries { eprintln!( "{:<8} {:>10} {:>10} {:>10} {:>10} {:>10} {:>10} {:>10} {:>10} {:>10} {:>10} {:>10} {:>10}", summary.window, - summary.paint_p50_us, summary.paint_p95_us, summary.paint_p99_us, - summary.encode_p50_us, summary.encode_p95_us, summary.encode_p99_us, - summary.write_p50_us, summary.write_p95_us, summary.write_p99_us, - summary.total_p50_us, summary.total_p95_us, summary.total_p99_us, + summary.paint_p50_us, + summary.paint_p95_us, + summary.paint_p99_us, + summary.encode_p50_us, + summary.encode_p95_us, + summary.encode_p99_us, + summary.write_p50_us, + summary.write_p95_us, + summary.write_p99_us, + summary.total_p50_us, + summary.total_p95_us, + summary.total_p99_us, ); } } @@ -586,11 +643,7 @@ fn drive_solid_color_render( let report = report.ok_or("never received a frame with bytes")?; let width = report.width as usize; let height = report.height as usize; - assert_eq!( - bytes.len(), - width * height * 4, - "rgba byte count must match width × height × 4", - ); + assert_eq!(bytes.len(), width * height * 4, "rgba byte count must match width × height × 4",); // Sample 9 evenly-spaced points in the inner quartile of the // viewport. Solid backgrounds should pass every sample; if Servo diff --git a/crates/ely_servo_host/tests/software_host.rs b/crates/ely_servo_host/tests/software_host.rs index b0d5049..0e61b18 100644 --- a/crates/ely_servo_host/tests/software_host.rs +++ b/crates/ely_servo_host/tests/software_host.rs @@ -10,9 +10,10 @@ use std::{ use ely_domain::{ProfileId, SiteOrigin, SitePermissionFeature, TabId, UrlText}; use ely_servo_host::{ - KeyboardTextRequest, MouseClickRequest, MouseDragRequest, NavigationRequest, PageZoomRequest, - PermissionDecision, PermissionRequest, ResizeRequest, ScreenshotRequest, ScrollRequest, - ServoHost, ServoHostError, ServoSurfaceSize, SoftwareServoHost, TouchTapRequest, WebViewState, + HidpiScaleRequest, KeyboardTextRequest, MouseClickRequest, MouseDragRequest, NavigationRequest, + PageZoomRequest, PermissionDecision, PermissionRequest, ResizeRequest, ScreenshotRequest, + ScrollRequest, ServoHost, ServoHostError, ServoSurfaceSize, SoftwareServoHost, TouchTapRequest, + WebViewState, }; const MINIMUM_CONTENT_PIXELS: u64 = 1_000; @@ -25,6 +26,7 @@ const PRD_SITE_COMPATIBILITY_CASES: &[PrdSiteCompatibilityCase] = &[ PrdSiteCompatibilityCase { url: "https://servo.org/", title_fragment: "Servo" }, ]; const SOFTWARE_HOST_CHILD_ENV: &str = "ELY_SERVO_SOFTWARE_HOST_CHILD"; +const DPR_VIEWPORT_CHILD_ENV: &str = "ELY_SERVO_DPR_VIEWPORT_CHILD"; const CLICK_PROBE_URL: &str = "data:text/html,%3C!doctype%20html%3E%3Ctitle%3EClick%20Probe%3C%2Ftitle%3E%3Cstyle%3Ebody%7Bmargin%3A0%3Bbackground%3A%23f7f7f7%3B%7Dbutton%7Bposition%3Aabsolute%3Bleft%3A80px%3Btop%3A80px%3Bwidth%3A220px%3Bheight%3A90px%3Bfont%3A28px%20sans-serif%3Bbackground%3A%23ffffff%3Bcolor%3A%23111111%3B%7D%3C%2Fstyle%3E%3Cbutton%20onclick%3D%22document.body.style.background%3D%27%230039ff%27%3Bdocument.title%3D%27Clicked%27%3Bthis.textContent%3D%27Clicked%27%3B%22%3ETap%3C%2Fbutton%3E"; const DRAG_PROBE_URL: &str = "data:text/html,%3C%21doctype%20html%3E%3Ctitle%3EDrag%20Probe%3C%2Ftitle%3E%3Cstyle%3Ebody%7Bmargin%3A0%3Bbackground%3A%23f7f7f7%3B%7Dbutton%7Bposition%3Aabsolute%3Bleft%3A80px%3Btop%3A80px%3Bwidth%3A220px%3Bheight%3A90px%3Bfont%3A28px%20sans-serif%3Bbackground%3A%23ffffff%3Bcolor%3A%23111111%3B%7D%3C%2Fstyle%3E%3Cbutton%20id%3Dbox%3EDrag%3C%2Fbutton%3E%3Cscript%3Elet%20dragging%3Dfalse%3Bconst%20box%3Ddocument.getElementById%28%27box%27%29%3BaddEventListener%28%27mousedown%27%2Cevent%3D%3E%7Bif%28event.target%3D%3D%3Dbox%29%7Bdragging%3Dtrue%3B%7D%7D%29%3BaddEventListener%28%27mousemove%27%2Cevent%3D%3E%7Bif%28dragging%26%26event.clientX%3E280%29%7Bdocument.body.style.background%3D%27%230039ff%27%3Bdocument.title%3D%27Dragged%27%3Bbox.textContent%3D%27Dragged%27%3B%7D%7D%29%3BaddEventListener%28%27mouseup%27%2C%28%29%3D%3E%7Bdragging%3Dfalse%3B%7D%29%3B%3C%2Fscript%3E"; const TOUCH_PROBE_URL: &str = "data:text/html,%3C%21doctype%20html%3E%3Ctitle%3ETouch%20Probe%3C%2Ftitle%3E%3Cstyle%3Ebody%7Bmargin%3A0%3Bbackground%3A%23f7f7f7%3B%7Dbutton%7Bposition%3Aabsolute%3Bleft%3A80px%3Btop%3A80px%3Bwidth%3A220px%3Bheight%3A90px%3Bfont%3A28px%20sans-serif%3Bbackground%3A%23ffffff%3Bcolor%3A%23111111%3Btouch-action%3Amanipulation%3B%7D%3C%2Fstyle%3E%3Cbutton%20ontouchstart%3D%22document.body.dataset.touch%3D%27start%27%3B%22%20onclick%3D%22document.body.style.background%3D%27%230039ff%27%3Bdocument.title%3D%27Touched%27%3Bthis.textContent%3D%27Touched%27%3B%22%3ETap%3C%2Fbutton%3E"; @@ -36,6 +38,13 @@ struct PrdSiteCompatibilityCase { title_fragment: &'static str, } +struct DprViewportCase { + physical_width: u32, + physical_height: u32, + dpr: f32, + expected_css_width: u32, +} + #[test] fn manages_real_servo_webview_lifecycle() -> Result<(), Box> { if env::var_os(SOFTWARE_HOST_CHILD_ENV).is_none() { @@ -67,6 +76,122 @@ fn run_isolated_software_host_lifecycle() -> Result<(), Box> { .into()) } +#[test] +fn first_navigation_preserves_dpr_for_css_viewport() -> Result<(), Box> { + if env::var_os(DPR_VIEWPORT_CHILD_ENV).is_none() { + return run_isolated_dpr_viewport_test(); + } + + exercise_dpr_viewport_cases() +} + +fn run_isolated_dpr_viewport_test() -> Result<(), Box> { + let output = Command::new(env::current_exe()?) + .arg("--exact") + .arg("first_navigation_preserves_dpr_for_css_viewport") + .env(DPR_VIEWPORT_CHILD_ENV, "1") + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .output()?; + + if output.status.success() { + return Ok(()); + } + + Err(format!( + "isolated DPR viewport test failed\nstatus: {}\nstdout: {}\nstderr: {}", + output.status, + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ) + .into()) +} + +fn exercise_dpr_viewport_cases() -> Result<(), Box> { + let mut host = SoftwareServoHost::new(ServoSurfaceSize::new(1, 1))?; + let profile_id = ProfileId::new(); + let cases = [ + DprViewportCase { + physical_width: 800, + physical_height: 600, + dpr: 1.0, + expected_css_width: 800, + }, + DprViewportCase { + physical_width: 960, + physical_height: 640, + dpr: 2.0, + expected_css_width: 480, + }, + DprViewportCase { + physical_width: 1250, + physical_height: 800, + dpr: 1.25, + expected_css_width: 1000, + }, + DprViewportCase { + physical_width: 1440, + physical_height: 900, + dpr: 1.5, + expected_css_width: 960, + }, + DprViewportCase { + physical_width: 1750, + physical_height: 1000, + dpr: 1.75, + expected_css_width: 1000, + }, + DprViewportCase { + physical_width: 1500, + physical_height: 900, + dpr: 2.5, + expected_css_width: 600, + }, + DprViewportCase { + physical_width: 2160, + physical_height: 1440, + dpr: 3.0, + expected_css_width: 720, + }, + ]; + + for case in cases { + let tab_id = TabId::new(); + let webview_id = host.create_webview_with_size( + tab_id.clone(), + profile_id.clone(), + ServoSurfaceSize::new(case.physical_width, case.physical_height), + )?; + + host.set_hidpi_scale(HidpiScaleRequest { + webview_id: webview_id.clone(), + scale_factor: case.dpr, + })?; + host.navigate(NavigationRequest { + webview_id: webview_id.clone(), + tab_id, + url: UrlText::parse(viewport_probe_url(case.expected_css_width + 1))?, + })?; + + let snapshot = wait_for_rendered_webview(&mut host, &webview_id, None)?; + assert_eq!(snapshot.state(), &WebViewState::Complete, "snapshot: {snapshot:?}"); + + let frame = host.last_rendered_frame()?; + assert_eq!(frame.width(), case.physical_width, "DPR case frame width"); + assert_eq!(frame.height(), case.physical_height, "DPR case frame height"); + assert_eq!( + center_pixel_rgb(&frame), + [238, 32, 77], + "CSS viewport must equal physical width divided by DPR: physical={} dpr={} expected_css={}", + case.physical_width, + case.dpr, + case.expected_css_width, + ); + host.close_webview(&webview_id); + } + Ok(()) +} + fn exercise_real_servo_webview_lifecycle() -> Result<(), Box> { let mut host = SoftwareServoHost::new(ServoSurfaceSize::new(INITIAL_WIDTH, INITIAL_HEIGHT))?; let tab_id = TabId::new(); @@ -340,3 +465,33 @@ fn assert_frame_has_dimensions_and_content( assert!(frame.content_pixel_count() >= minimum_content_pixels, "{label}: {frame:?}"); assert_ne!(frame.sample_hash(), 0, "{label}: {frame:?}"); } + +fn center_pixel_rgb(frame: &ely_servo_host::RenderedFrame) -> [u8; 3] { + let x = frame.width() / 2; + let y = frame.height() / 2; + let index = ((y * frame.width() + x) * 4) as usize; + let rgba = &frame.rgba_bytes()[index..index + 4]; + [rgba[0], rgba[1], rgba[2]] +} + +fn viewport_probe_url(min_width_threshold: u32) -> String { + let html = format!( + "DPR Probe", + ); + format!("data:text/html,{}", percent_encode_for_data_url(&html)) +} + +fn percent_encode_for_data_url(value: &str) -> String { + value + .bytes() + .map(|byte| match byte { + b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' | b'.' | b'~' => { + (byte as char).to_string() + } + _ => format!("%{byte:02X}"), + }) + .collect() +} diff --git a/third_party/gpui/src/elements/surface.rs b/third_party/gpui/src/elements/surface.rs index b4fced1..d6eb84c 100644 --- a/third_party/gpui/src/elements/surface.rs +++ b/third_party/gpui/src/elements/surface.rs @@ -1,6 +1,6 @@ use crate::{ - App, Bounds, Element, ElementId, GlobalElementId, InspectorElementId, IntoElement, LayoutId, - ObjectFit, Pixels, Style, StyleRefinement, Styled, Window, + App, Bounds, Corners, Element, ElementId, GlobalElementId, InspectorElementId, IntoElement, + LayoutId, ObjectFit, Pixels, Style, StyleRefinement, Styled, Window, }; #[cfg(target_os = "macos")] use core_video::pixel_buffer::CVPixelBuffer; @@ -25,6 +25,7 @@ impl From for SurfaceSource { pub struct Surface { source: SurfaceSource, object_fit: ObjectFit, + corner_radii: Option>, style: StyleRefinement, } @@ -33,6 +34,7 @@ pub fn surface(source: impl Into) -> Surface { Surface { source: source.into(), object_fit: ObjectFit::Contain, + corner_radii: None, style: Default::default(), } } @@ -43,6 +45,12 @@ impl Surface { self.object_fit = object_fit; self } + + /// Set rounded clipping for the rendered surface. + pub fn corner_radii(mut self, corner_radii: impl Into>) -> Self { + self.corner_radii = Some(corner_radii.into()); + self + } } impl Element for Surface { @@ -96,8 +104,13 @@ impl Element for Surface { SurfaceSource::Surface(surface) => { let size = crate::size(surface.get_width().into(), surface.get_height().into()); let new_bounds = self.object_fit.get_bounds(bounds, size); - // TODO: Add support for corner_radii - window.paint_surface(new_bounds, surface.clone()); + let mut style = Style::default(); + style.refine(&self.style); + let corner_radii = self + .corner_radii + .unwrap_or_else(|| style.corner_radii.to_pixels(window.rem_size())) + .clamp_radii_for_quad_size(new_bounds.size); + window.paint_surface(new_bounds, corner_radii, surface.clone()); } #[allow(unreachable_patterns)] _ => {} diff --git a/third_party/gpui/src/platform/mac/metal_renderer.rs b/third_party/gpui/src/platform/mac/metal_renderer.rs index edf822e..1f637b1 100644 --- a/third_party/gpui/src/platform/mac/metal_renderer.rs +++ b/third_party/gpui/src/platform/mac/metal_renderer.rs @@ -1,8 +1,8 @@ use super::metal_atlas::MetalAtlas; use crate::{ - AtlasTextureId, Background, Bounds, ContentMask, DevicePixels, MonochromeSprite, PaintSurface, - Path, Point, PolychromeSprite, PrimitiveBatch, Quad, ScaledPixels, Scene, Shadow, Size, - Surface, Underline, point, size, + AtlasTextureId, Background, Bounds, ContentMask, Corners, DevicePixels, MonochromeSprite, + PaintSurface, Path, Point, PolychromeSprite, PrimitiveBatch, Quad, ScaledPixels, Scene, Shadow, + Size, Surface, Underline, point, size, }; use anyhow::Result; use block::ConcreteBlock; @@ -1112,6 +1112,11 @@ impl MetalRenderer { Some(&instance_buffer.metal_buffer), *instance_offset as u64, ); + command_encoder.set_fragment_buffer( + SurfaceInputIndex::Surfaces as u64, + Some(&instance_buffer.metal_buffer), + *instance_offset as u64, + ); command_encoder.set_vertex_bytes( SurfaceInputIndex::TextureSize as u64, mem::size_of_val(&texture_size) as u64, @@ -1180,6 +1185,7 @@ impl MetalRenderer { SurfaceBounds { bounds: surface.bounds, content_mask: surface.content_mask.clone(), + corner_radii: surface.corner_radii, }, ); } @@ -1387,4 +1393,5 @@ pub struct PathSprite { pub struct SurfaceBounds { pub bounds: Bounds, pub content_mask: ContentMask, + pub corner_radii: Corners, } diff --git a/third_party/gpui/src/platform/mac/shaders.metal b/third_party/gpui/src/platform/mac/shaders.metal index 69c2fa1..2638da8 100644 --- a/third_party/gpui/src/platform/mac/shaders.metal +++ b/third_party/gpui/src/platform/mac/shaders.metal @@ -835,12 +835,14 @@ fragment float4 path_sprite_fragment( } struct SurfaceVertexOutput { + uint surface_id [[flat]]; float4 position [[position]]; float2 texture_position; float clip_distance [[clip_distance]][4]; }; struct SurfaceFragmentInput { + uint surface_id [[flat]]; float4 position [[position]]; float2 texture_position; }; @@ -863,12 +865,28 @@ vertex SurfaceVertexOutput surface_vertex( // to the current vertex of the unit triangle. float2 texture_position = unit_vertex; return SurfaceVertexOutput{ + surface_id, device_position, texture_position, {clip_distance.x, clip_distance.y, clip_distance.z, clip_distance.w}}; } +float surface_corner_alpha(float2 position, SurfaceBounds surface) { + bool unrounded = surface.corner_radii.top_left == 0.0 && + surface.corner_radii.bottom_left == 0.0 && + surface.corner_radii.top_right == 0.0 && + surface.corner_radii.bottom_right == 0.0; + if (unrounded) { + return 1.0; + } + + float distance = quad_sdf(position, surface.bounds, surface.corner_radii); + return 1.0 - smoothstep(-0.5, 0.5, distance); +} + fragment float4 surface_fragment(SurfaceFragmentInput input [[stage_in]], + constant SurfaceBounds *surfaces + [[buffer(SurfaceInputIndex_Surfaces)]], texture2d y_texture [[texture(SurfaceInputIndex_YTexture)]], texture2d cb_cr_texture @@ -883,10 +901,19 @@ fragment float4 surface_fragment(SurfaceFragmentInput input [[stage_in]], y_texture.sample(texture_sampler, input.texture_position).r, cb_cr_texture.sample(texture_sampler, input.texture_position).rg, 1.0); - return ycbcrToRGBTransform * ycbcr; + SurfaceBounds surface = surfaces[input.surface_id]; + float4 color = ycbcrToRGBTransform * ycbcr; + float alpha = surface_corner_alpha(input.position.xy, surface); + if (alpha <= 0.0) { + discard_fragment(); + } + color.a *= alpha; + return color; } fragment float4 surface_bgra_fragment(SurfaceFragmentInput input [[stage_in]], + constant SurfaceBounds *surfaces + [[buffer(SurfaceInputIndex_Surfaces)]], texture2d bgra_texture [[texture(SurfaceInputIndex_YTexture)]]) { constexpr sampler texture_sampler(mag_filter::linear, min_filter::linear); @@ -894,7 +921,14 @@ fragment float4 surface_bgra_fragment(SurfaceFragmentInput input [[stage_in]], // from GPUI's surface quad, matching the software readback path. float2 texture_position = float2(input.texture_position.x, 1.0 - input.texture_position.y); - return bgra_texture.sample(texture_sampler, texture_position); + SurfaceBounds surface = surfaces[input.surface_id]; + float4 color = bgra_texture.sample(texture_sampler, texture_position); + float alpha = surface_corner_alpha(input.position.xy, surface); + if (alpha <= 0.0) { + discard_fragment(); + } + color.a *= alpha; + return color; } float4 hsla_to_rgba(Hsla hsla) { diff --git a/third_party/gpui/src/scene.rs b/third_party/gpui/src/scene.rs index 758d06e..50334de 100644 --- a/third_party/gpui/src/scene.rs +++ b/third_party/gpui/src/scene.rs @@ -658,6 +658,7 @@ pub(crate) struct PaintSurface { pub order: DrawOrder, pub bounds: Bounds, pub content_mask: ContentMask, + pub corner_radii: Corners, #[cfg(target_os = "macos")] pub image_buffer: core_video::pixel_buffer::CVPixelBuffer, } diff --git a/third_party/gpui/src/window.rs b/third_party/gpui/src/window.rs index 6d74a0e..92835a1 100644 --- a/third_party/gpui/src/window.rs +++ b/third_party/gpui/src/window.rs @@ -3178,18 +3178,25 @@ impl Window { /// /// This method should only be called as part of the paint phase of element drawing. #[cfg(target_os = "macos")] - pub fn paint_surface(&mut self, bounds: Bounds, image_buffer: CVPixelBuffer) { + pub fn paint_surface( + &mut self, + bounds: Bounds, + corner_radii: Corners, + image_buffer: CVPixelBuffer, + ) { use crate::PaintSurface; self.invalidator.debug_assert_paint(); let scale_factor = self.scale_factor(); let bounds = bounds.scale(scale_factor); + let corner_radii = corner_radii.scale(scale_factor); let content_mask = self.content_mask().scale(scale_factor); self.next_frame.scene.insert_primitive(PaintSurface { order: 0, bounds, content_mask, + corner_radii, image_buffer, }); }