From 6ce5df83a8fae60f7a2b8d9a324b4dd21881521b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Wed, 13 May 2026 01:41:58 -0400 Subject: [PATCH] Skip hardware live readback --- crates/ely_app/src/shell/web_surface_frame.rs | 24 ++--- .../src/shell/web_surface_live_site_tests.rs | 10 +- .../src/bin/ely_servo_sidecar/live.rs | 92 ++++++++++--------- .../src/bin/ely_servo_sidecar/live_output.rs | 59 +++++++++++- .../bin/ely_servo_sidecar/live_protocol.rs | 34 ++++++- crates/ely_servo_host/src/runtime.rs | 67 +++++++++----- 6 files changed, 202 insertions(+), 84 deletions(-) diff --git a/crates/ely_app/src/shell/web_surface_frame.rs b/crates/ely_app/src/shell/web_surface_frame.rs index 931168f..2b58adf 100644 --- a/crates/ely_app/src/shell/web_surface_frame.rs +++ b/crates/ely_app/src/shell/web_surface_frame.rs @@ -183,7 +183,7 @@ impl WebSurfaceFrame { #[cfg(all(test, feature = "live-site-smoke"))] pub(super) fn size(&self) -> WebSurfaceSize { - WebSurfaceSize { width: self.width, height: self.height } + WebSurfaceSize { width: self.width, height: self.height, device_pixel_ratio_percent: 100 } } #[cfg(all(test, feature = "live-site-smoke"))] @@ -204,16 +204,6 @@ impl WebSurfaceFrame { self.loaded_url.as_deref() } - #[cfg(all(test, feature = "live-site-smoke"))] - pub(super) fn click_point(&self) -> Option { - self.click_point - } - - #[cfg(all(test, feature = "live-site-smoke"))] - pub(super) fn typed_text(&self) -> Option<&str> { - self.typed_text.as_deref() - } - #[cfg(all(test, feature = "live-site-smoke"))] pub(super) fn non_white_pixel_count(&self) -> u64 { self.non_white_pixel_count @@ -228,6 +218,18 @@ impl WebSurfaceFrame { pub(super) fn sample_hash(&self) -> u64 { self.sample_hash } + + #[cfg(all(test, feature = "live-site-smoke"))] + pub(super) fn has_hardware_surface(&self) -> bool { + #[cfg(target_os = "macos")] + { + self.pixel_buffer.is_some() + } + #[cfg(not(target_os = "macos"))] + { + false + } + } } struct WebSurfaceFrameParts { 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 b8f699c..132bf5f 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 @@ -152,7 +152,12 @@ fn wait_for_ready_frame( fn validate_prd_frame(frame: &WebSurfaceFrame, case: &LiveSiteCase) -> Result<(), String> { require( - frame.size() == WebSurfaceSize { width: LIVE_SURFACE_WIDTH, height: LIVE_SURFACE_HEIGHT }, + frame.size() + == WebSurfaceSize { + width: LIVE_SURFACE_WIDTH, + height: LIVE_SURFACE_HEIGHT, + device_pixel_ratio_percent: 100, + }, format!("{} size: {:?}", case.url, frame.size()), )?; require( @@ -172,6 +177,9 @@ fn validate_prd_frame(frame: &WebSurfaceFrame, case: &LiveSiteCase) -> Result<() frame.detail_label() == format!("{} 934x657", frame.render_state()), 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.content_pixel_count() >= MINIMUM_CONTENT_PIXELS, 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 45270f5..10575ae 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 @@ -353,48 +353,8 @@ fn poll_frame( host.tick(); let snapshot = host.snapshot(&session.webview_id)?; if snapshot.has_pending_frame() { - 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(); - // `non_white`/`content_pixel_count` come from a CPU - // readback of the bound framebuffer. On the software - // path that's the source of truth: Servo's compositor - // returns a white framebuffer until layout completes, so - // the threshold check is how we skip blank loading - // frames. On the hardware path the readback goes through - // `glReadPixels` on the surfman swap-chain surface, which - // can return content that fails the threshold even when - // Servo painted real pixels (the IOSurface itself is - // valid for GPUI to sample directly). `has_pending_frame` - // already encodes "Servo finished painting" — trust it - // for the hardware path instead of double-checking via a - // readback we know to be unreliable. - // - // For the software path, the threshold check is only - // useful once per URL: after we've confirmed at least one - // paint had real content, every subsequent input deserves - // an immediate return on the next pending frame instead - // of another 250 ms wait. `session.ever_visible_frame` - // tracks that, reset on navigate. - let has_visible_content = match rendering_context_kind { - RenderingContextKind::Software => { - session.ever_visible_frame - || (frame.non_white_pixel_count() > 0 && frame.content_pixel_count() > 0) - } - #[cfg(feature = "hardware-render")] - RenderingContextKind::Hardware => true, - #[cfg(not(feature = "hardware-render"))] - RenderingContextKind::Hardware => { - frame.non_white_pixel_count() > 0 && frame.content_pixel_count() > 0 - } - }; - let report = LiveFrameReport::new(&snapshot, &frame); - let encode_ns = elapsed_ns(encode_started_at); - let timings = PartialFrameTimings { paint_ns, encode_ns }; - let outcome = LiveOutcome::from_frame(report, frame, timings); + 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; @@ -417,6 +377,54 @@ fn poll_frame( } } +fn paint_pending_frame( + host: &mut SoftwareServoHost, + session: &mut LiveSession, + rendering_context_kind: RenderingContextKind, +) -> Result<(LiveOutcome, bool), LiveSidecarError> { + match rendering_context_kind { + RenderingContextKind::Software => paint_readback_frame(host, session), + #[cfg(all(feature = "hardware-render", target_os = "macos"))] + RenderingContextKind::Hardware => paint_hardware_surface_frame(host, session), + #[cfg(not(all(feature = "hardware-render", target_os = "macos")))] + RenderingContextKind::Hardware => paint_readback_frame(host, session), + } +} + +fn paint_readback_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 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 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 paint_hardware_surface_frame( + host: &mut SoftwareServoHost, + session: &LiveSession, +) -> Result<(LiveOutcome, bool), LiveSidecarError> { + 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 encode_ns = elapsed_ns(encode_started_at); + let timings = PartialFrameTimings { paint_ns, encode_ns }; + Ok((LiveOutcome::from_report(report, timings), true)) +} + #[derive(Clone)] struct LiveSession { webview_id: ely_domain::WebViewId, 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 8996234..41004e9 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 @@ -32,7 +32,7 @@ pub(super) fn populate_surface_fields( published_surface_ids: &mut HashMap>, outcome: &mut LiveOutcome, ) { - if outcome.frame.is_none() { + if outcome.response.frame.is_none() { return; } #[cfg(all(feature = "hardware-render", target_os = "macos"))] @@ -124,7 +124,7 @@ pub(super) fn write_outcome( ) -> Result<(), LiveSidecarError> { let mut outcome = outcome.unwrap_or_else(|error| LiveOutcome::error(error.to_string())); let partial_timings = outcome.partial_timings.take(); - let frame_present = outcome.frame.is_some(); + let frame_present = outcome.response.frame.is_some(); if let Some(summary) = pending_summary.take() { outcome.response.perf = Some(summary); } @@ -163,11 +163,19 @@ pub(super) fn write_outcome( #[cfg(test)] mod tests { - use std::collections::{HashMap, HashSet}; + use std::{ + collections::{HashMap, HashSet}, + error::Error, + time::Instant, + }; use ely_servo_host::{IOSurfaceHandle, IOSurfaceIdentity}; - use super::surface_publication_for; + use super::super::{ + live_protocol::{LiveFrameReport, LiveOutcome, PartialFrameTimings}, + perf::FramePerfAggregator, + }; + use super::{surface_publication_for, write_outcome}; #[test] fn unpublished_surface_without_handle_leaves_selector_empty() { @@ -217,6 +225,35 @@ mod tests { assert!(published.is_empty()); } + #[test] + fn payloadless_surface_report_records_perf_and_writes_no_rgba() -> Result<(), Box> { + let mut outcome = LiveOutcome::from_report( + report_with_byte_count(16), + PartialFrameTimings { paint_ns: 1_000, encode_ns: 2_000 }, + ); + outcome.response.current_surface_id = Some(7); + let mut stdout = Vec::new(); + let mut perf = FramePerfAggregator::new("hardware", 1); + let mut pending_summary = None; + + write_outcome(&mut stdout, &mut perf, &mut pending_summary, Ok(outcome), Instant::now())?; + + let Some(newline_index) = stdout.iter().position(|byte| *byte == b'\n') else { + return Err("response newline missing".into()); + }; + let line = std::str::from_utf8(&stdout[..newline_index])?; + let response: serde_json::Value = serde_json::from_str(line)?; + let rgba_byte_count = response + .get("frame") + .and_then(|frame| frame.get("rgba_byte_count")) + .and_then(serde_json::Value::as_u64); + + assert_eq!(rgba_byte_count, Some(0)); + assert!(stdout[newline_index + 1..].is_empty()); + assert!(pending_summary.is_some()); + Ok(()) + } + fn identity(surface_id: u64, width: u32, height: u32) -> IOSurfaceIdentity { IOSurfaceIdentity { surface_id, width, height } } @@ -224,4 +261,18 @@ mod tests { fn handle(surface_id: u64, width: u32, height: u32) -> IOSurfaceHandle { IOSurfaceHandle { mach_port_name: 42, surface_id, width, height } } + + fn report_with_byte_count(rgba_byte_count: usize) -> LiveFrameReport { + LiveFrameReport { + loaded_url: Some("https://example.com/".to_string()), + title: Some("Example".to_string()), + state: "complete", + width: 2, + height: 2, + rgba_byte_count, + non_white_pixel_count: 0, + content_pixel_count: 0, + sample_hash: 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 29c80a0..eaef074 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 @@ -67,10 +67,10 @@ pub(super) struct PartialFrameTimings { pub encode_ns: u64, } -/// A handle plus an optional rendered frame and partial stage -/// timings. We hold the `RenderedFrame` so the write step can stream -/// its existing rgba slice straight onto the pipe — no extra -/// `to_vec()`. +/// A response plus an optional software RGBA payload and partial stage +/// timings. Software frames carry `RenderedFrame` so the write step +/// can stream its existing rgba slice straight onto the pipe; hardware +/// surface frames carry only a `LiveFrameReport`. pub(super) struct LiveOutcome { pub response: LiveResponse, pub frame: Option, @@ -97,6 +97,15 @@ impl LiveOutcome { partial_timings: Some(partial_timings), } } + + #[cfg(any(test, all(feature = "hardware-render", target_os = "macos")))] + pub fn from_report(report: LiveFrameReport, partial_timings: PartialFrameTimings) -> Self { + Self { + response: LiveResponse::frame(report), + frame: None, + partial_timings: Some(partial_timings), + } + } } #[derive(Serialize)] @@ -182,6 +191,23 @@ impl LiveFrameReport { sample_hash: frame.sample_hash(), } } + + /// 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 { + Self { + loaded_url: snapshot.url().map(str::to_string), + title: snapshot.title().map(str::to_string), + state: state_label(snapshot.state()), + width, + height, + rgba_byte_count: 0, + non_white_pixel_count: 0, + content_pixel_count: 0, + sample_hash: 0, + } + } } fn state_label(state: &WebViewState) -> &'static str { diff --git a/crates/ely_servo_host/src/runtime.rs b/crates/ely_servo_host/src/runtime.rs index d605baa..4d1a34f 100644 --- a/crates/ely_servo_host/src/runtime.rs +++ b/crates/ely_servo_host/src/runtime.rs @@ -106,6 +106,14 @@ impl SoftwareServoHost { self.create_webview_in_context(tab_id, profile_id, size) } + /// 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. + pub fn paint_without_readback(&mut self, webview_id: &WebViewId) -> Result<(), ServoHostError> { + self.paint_webview(webview_id, false).map(|_| ()) + } + fn new_started( size: ServoSurfaceSize, config_dir: Option, @@ -129,6 +137,40 @@ impl SoftwareServoHost { last_rendered_frame: None, }) } + + fn paint_webview( + &mut self, + webview_id: &WebViewId, + capture_frame: bool, + ) -> Result, ServoHostError> { + let rendering_context = self.webview(webview_id)?.rendering_context.clone(); + rendering_context.make_current().map_err(|_| ServoHostError::RenderingContextNotCurrent)?; + rendering_context.prepare_for_rendering(); + // `webview.paint()` dispatches a render command to Servo's paint + // thread — it does NOT block until the framebuffer is consistent. + // Without a barrier, `read_rendered_frame` below races the paint + // thread and reliably reads the cleared-white state on data: URLs. + // Clear the pending-frame flag first so we can detect the *next* + // `notify_new_frame_ready` (the one our `paint()` triggers), then + // pump the event loop until Servo reports the new frame is ready + // or `paint_barrier_budget()` elapses. On timeout we fall through + // and read anyway, preserving the pre-T15 fast path for callers + // that explicitly disable the barrier with `ELY_PAINT_BARRIER_MS=0`. + { + let webview = self.webview(webview_id)?; + webview.delegate.mark_frame_presented(); + webview.webview.paint(); + } + self.wait_for_paint_completion(webview_id); + let rendered_frame = if capture_frame { + Some(Self::read_rendered_frame(rendering_context.as_ref())?) + } else { + None + }; + rendering_context.present(); + self.webview(webview_id)?.delegate.mark_frame_presented(); + Ok(rendered_frame) + } } impl ServoHost for SoftwareServoHost { @@ -333,28 +375,9 @@ impl ServoHost for SoftwareServoHost { } fn paint(&mut self, webview_id: &WebViewId) -> Result<(), ServoHostError> { - let rendering_context = self.webview(webview_id)?.rendering_context.clone(); - rendering_context.make_current().map_err(|_| ServoHostError::RenderingContextNotCurrent)?; - rendering_context.prepare_for_rendering(); - // `webview.paint()` dispatches a render command to Servo's paint - // thread — it does NOT block until the framebuffer is consistent. - // Without a barrier, `read_rendered_frame` below races the paint - // thread and reliably reads the cleared-white state on data: URLs. - // Clear the pending-frame flag first so we can detect the *next* - // `notify_new_frame_ready` (the one our `paint()` triggers), then - // pump the event loop until Servo reports the new frame is ready - // or `paint_barrier_budget()` elapses. On timeout we fall through - // and read anyway, preserving the pre-T15 fast path for callers - // that explicitly disable the barrier with `ELY_PAINT_BARRIER_MS=0`. - { - let webview = self.webview(webview_id)?; - webview.delegate.mark_frame_presented(); - webview.webview.paint(); - } - self.wait_for_paint_completion(webview_id); - let rendered_frame = Self::read_rendered_frame(rendering_context.as_ref())?; - rendering_context.present(); - self.webview(webview_id)?.delegate.mark_frame_presented(); + let Some(rendered_frame) = self.paint_webview(webview_id, true)? else { + return Err(ServoHostError::RenderedFrameUnavailable); + }; self.last_rendered_frame = Some(rendered_frame); Ok(()) }