Land a red TDD guard for live-frame texture re-upload (T10)
Every `WebSurfaceFrame::from_live_frame` today calls
`Arc::new(RenderImage::new([image::Frame::new(image_buffer)]))`
unconditionally — even when the underlying RGBA bytes are
byte-for-byte identical to the previous frame. At 60 fps on a 1080p
canvas that is roughly 960 MB/s of host-side cloning + a fresh
GPUI texture upload, and the roundtable agreed it is the next
material bottleneck after the file-system pipe (a80d039).
The contract this test pins is the cheapest invariant we can hold
against today's `SoftwareRenderingContext`: two `ServoLiveFrame`
inputs whose `rgba_bytes` are bit-identical must produce the same
underlying `Arc<RenderImage>` instance. Today they do not; the
ignored run confirms two distinct pointer values for back-to-back
identical inputs.
The fix has two recognised shapes. The interim shape lives entirely
in `WebSurfaceFrame::from_parts`: remember the previous frame's
bytes (hash or pointer-eq) and reuse the existing `Arc<RenderImage>`
when they match. The endgame shape removes the host-side image step
entirely — `OffscreenRenderingContext` + IOSurface — at which point
the assertion becomes meaningless and is replaced by a frame-time
budget. Whichever lands first, the fix commit MUST delete the
`#[ignore]` attribute outright; toggling its reason is a broken
contract.
To call `WebSurfaceFrame::from_live_frame` from a unit test without
spawning a real sidecar process, `ServoLiveFrame` gains a
`#[cfg(test)] pub(crate) fn for_test(...)` constructor that wraps
the existing private `from_parts` with realistic defaults. No
production path uses it.
cargo test --bin ely_app: 112 passed, 0 failed, 2 ignored.
cargo test --bin ely_app -- --ignored: 2 failed (expected RED:
T7 click pipeline + T10 texture re-upload).
This commit is contained in:
@@ -236,6 +236,24 @@ impl ServoLiveFrame {
|
||||
pub fn into_rgba_bytes(self) -> Vec<u8> {
|
||||
self.rgba_bytes
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn for_test(width: u32, height: u32, rgba_bytes: Vec<u8>) -> Self {
|
||||
Self {
|
||||
loaded_url: Some("https://example.com/".to_string()),
|
||||
title: Some("Example".to_string()),
|
||||
render_state: "complete".to_string(),
|
||||
width,
|
||||
height,
|
||||
#[cfg(all(test, feature = "live-site-smoke"))]
|
||||
non_white_pixel_count: 0,
|
||||
#[cfg(all(test, feature = "live-site-smoke"))]
|
||||
content_pixel_count: 0,
|
||||
#[cfg(all(test, feature = "live-site-smoke"))]
|
||||
sample_hash: 0,
|
||||
rgba_bytes,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Error)]
|
||||
|
||||
@@ -25,6 +25,7 @@
|
||||
|
||||
use std::cell::RefCell;
|
||||
use std::rc::Rc;
|
||||
use std::sync::Arc;
|
||||
|
||||
use ely_domain::{TabId, UrlText};
|
||||
use gpui::{
|
||||
@@ -35,6 +36,9 @@ use gpui::InteractiveElement;
|
||||
|
||||
use super::ShellState;
|
||||
use super::web_surface::WebSurfaceStore;
|
||||
use super::web_surface_frame::WebSurfaceFrame;
|
||||
use super::web_surface_geometry::WebSurfaceScrollOffset;
|
||||
use crate::services::servo_live::ServoLiveFrame;
|
||||
|
||||
#[cfg(test)]
|
||||
impl super::ElyShell {
|
||||
@@ -252,6 +256,68 @@ async fn baseline_overlay_div_receives_simulated_click(cx: &mut TestAppContext)
|
||||
);
|
||||
}
|
||||
|
||||
/// TDD red guard for T10: today every `WebSurfaceFrame::from_live_frame`
|
||||
/// call allocates a fresh `Arc::new(RenderImage::new(...))` regardless
|
||||
/// of whether the underlying pixels changed. At 60 fps on a 1080p
|
||||
/// canvas that is `~8 MB / frame` of host-side RGBA cloning + a new
|
||||
/// GPUI texture upload, the cost Linus + Karpathy + Jony all flagged
|
||||
/// as the next material bottleneck after the file-system pipe.
|
||||
///
|
||||
/// The contract this test pins is the cheapest invariant we can hold
|
||||
/// against today's `SoftwareRenderingContext`: two frames carrying
|
||||
/// **byte-identical RGBA payloads must produce the same underlying
|
||||
/// `Arc<RenderImage>`**. Today they do not — every `from_live_frame`
|
||||
/// blindly reallocates. The fix path is either dedup the upload
|
||||
/// against the last bytes or switch to `OffscreenRenderingContext` +
|
||||
/// IOSurface so the GPU texture is the source of truth.
|
||||
///
|
||||
/// `#[ignore]` so default `cargo test` stays green; remove the
|
||||
/// attribute the moment T10 lands so the regression is permanent.
|
||||
#[test]
|
||||
#[ignore = "T10 red guard. Failure mode confirmed via `cargo test -- --ignored`: \
|
||||
two ServoLiveFrames carrying byte-identical RGBA payloads still \
|
||||
produce distinct Arc<RenderImage> instances, because \
|
||||
WebSurfaceFrame::from_parts unconditionally calls \
|
||||
Arc::new(RenderImage::new(...)). At 60 fps 1080p that is \
|
||||
~960 MB/s of host-side RGBA cloning + GPUI texture allocations \
|
||||
and is the next bottleneck after the file-system pipe. The \
|
||||
fix lives in WebSurfaceFrame (interim: dedup upload against \
|
||||
last frame bytes) or in the rendering pipeline (final: \
|
||||
OffscreenRenderingContext + IOSurface zero-copy). The fix \
|
||||
commit must remove this attribute outright — not toggle it."]
|
||||
fn identical_live_frames_share_render_image_arc() {
|
||||
let width = 16u32;
|
||||
let height = 8u32;
|
||||
let rgba_bytes = vec![0xAAu8; (width as usize) * (height as usize) * 4];
|
||||
|
||||
let first = WebSurfaceFrame::from_live_frame(
|
||||
"https://example.com/".to_string(),
|
||||
WebSurfaceScrollOffset::default(),
|
||||
100,
|
||||
ServoLiveFrame::for_test(width, height, rgba_bytes.clone()),
|
||||
)
|
||||
.expect("first frame builds from identical bytes");
|
||||
let second = WebSurfaceFrame::from_live_frame(
|
||||
"https://example.com/".to_string(),
|
||||
WebSurfaceScrollOffset::default(),
|
||||
100,
|
||||
ServoLiveFrame::for_test(width, height, rgba_bytes),
|
||||
)
|
||||
.expect("second frame builds from identical bytes");
|
||||
|
||||
assert!(
|
||||
Arc::ptr_eq(&first.image, &second.image),
|
||||
"TDD red: two ServoLiveFrames with byte-identical RGBA produced \
|
||||
distinct Arc<RenderImage> instances (first={:p}, second={:p}). \
|
||||
WebSurfaceFrame::from_parts must dedup the upload against the \
|
||||
previous frame's bytes, or the rendering pipeline must switch \
|
||||
to a GPU-side source of truth (IOSurface) so per-frame host \
|
||||
allocations stop entirely.",
|
||||
Arc::as_ptr(&first.image),
|
||||
Arc::as_ptr(&second.image),
|
||||
);
|
||||
}
|
||||
|
||||
fn active_tab_overlay_state(
|
||||
shell: &gpui::Entity<super::ElyShell>,
|
||||
cx: &mut gpui::VisualTestContext,
|
||||
|
||||
Reference in New Issue
Block a user