Hold hardware live mode until BGRA presentation

crates/ely_app/src/services/iosurface_metal.rs

@@ -1,13 +1,12 @@
 //! macOS-only import of cross-process IOSurface handles into
-//! `CVPixelBuffer`s suitable for GPUI's `Surface` element.
+//! `CVPixelBuffer`s that preserve the sidecar's IOSurface identity.
 //!
 //! `T10.4` originally imported the IOSurface into an `MTLTexture`
-//! directly, but GPUI 0.2.2 already speaks `CVPixelBuffer` end-to-end
-//! through `Window::paint_surface` / `elements::surface::Surface`. Its
-//! internal Blade Metal renderer takes care of building the Metal
-//! texture, so a parallel MTLTexture cache here would be wasted work.
-//! The cache now hands the renderer the CVPixelBuffer GPUI already
-//! knows how to render.
+//! directly. GPUI 0.2.2 exposes `Window::paint_surface` /
+//! `elements::surface::Surface` for `CVPixelBuffer`, and that public
+//! path is wired for NV12 video frames. Servo's hardware renderer
+//! publishes BGRA IOSurfaces, so this cache stays as verified
+//! cross-process plumbing until the presenter accepts BGRA surfaces.
 //!
 //! Lifetime contract:
 //!
@@ -211,7 +210,9 @@ mod tests {
         assert!(mach_port != 0, "IOSurfaceCreateMachPort must yield a real port");
         let surface_id: u64 = 0xDEAD_BEEFu64;
 
-        cache.import(mach_port, surface_id).expect("local IOSurface must round-trip into a CVPixelBuffer");
+        cache
+            .import(mach_port, surface_id)
+            .expect("local IOSurface must round-trip into a CVPixelBuffer");
 
         let pixel_buffer = cache
             .pixel_buffer_for(surface_id)

crates/ely_app/src/services/servo_live.rs

@@ -10,10 +10,11 @@ use std::{
 /// (default — bit-identical to pre-flag builds) and `hardware` (real
 /// GPU adapter via the vendored `HardwareOffscreenContext`; requires
 /// the sidecar binary to be compiled with the `hardware-render`
-/// feature). Anything else is silently dropped and the sidecar
-/// defaults to software so a typo'd value never blocks the browser
-/// from starting; the sidecar's own arg parser still errors loudly on
-/// an unrecognised value when set explicitly via the flag.
+/// feature and a GPUI BGRA surface presenter). Anything else is
+/// silently dropped and the sidecar defaults to software so a typo'd
+/// value never blocks the browser from starting; the sidecar's own
+/// arg parser still errors loudly on an unrecognised value when set
+/// explicitly via the flag.
 const RENDERING_CONTEXT_ENV: &str = "ELY_SERVO_RENDERING_CONTEXT";
 
 use ely_domain::SitePermissionDecision;
@@ -265,10 +266,10 @@ pub(crate) struct ServoLiveFrame {
     #[cfg(all(test, feature = "live-site-smoke"))]
     sample_hash: u64,
     rgba_bytes: Vec<u8>,
-    /// Hardware-path companion: when present, the renderer can hand
-    /// the underlying IOSurface straight to GPUI's Metal pipeline via
-    /// `gpui::surface(...)` and skip the RGBA upload entirely. Always
-    /// `None` on the software path and on non-macOS hosts.
+    /// Hardware-path companion: the imported IOSurface published by
+    /// the sidecar. GPUI 0.2.2 presents `surface(...)` through its
+    /// NV12 video path, so the current BGRA Servo surface stays as
+    /// observability plumbing until a BGRA presenter lands.
     #[cfg(target_os = "macos")]
     pixel_buffer: Option<CVPixelBuffer>,
 }
@@ -294,8 +295,9 @@ impl ServoLiveFrame {
     }
 
     /// Returns the imported `CVPixelBuffer` matching the frame's
-    /// current hardware surface, if any. The renderer hands this to
-    /// `gpui::surface(...)` to skip the RGBA→texture upload path.
+    /// current hardware surface, if any. The renderer keeps this as
+    /// wire-path evidence while GPUI's public `surface(...)` element
+    /// remains NV12-only.
     #[cfg(target_os = "macos")]
     #[must_use]
     pub fn pixel_buffer(&self) -> Option<&CVPixelBuffer> {
@@ -412,9 +414,52 @@ pub(crate) enum ServoLiveError {
 /// truth for what values are valid — we just gate which ones we
 /// forward.
 fn rendering_context_from_env() -> Option<&'static str> {
-    match env::var(RENDERING_CONTEXT_ENV).ok()?.to_lowercase().as_str() {
-        "software" => Some("software"),
-        "hardware" => Some("hardware"),
-        _ => None,
+    let raw = env::var(RENDERING_CONTEXT_ENV).ok()?;
+    match rendering_context_selection(raw.as_str()) {
+        RenderingContextSelection::Forward(value) => Some(value),
+        RenderingContextSelection::HoldHardware => {
+            tracing::warn!(
+                target: "ely::servo::iosurface",
+                "hardware rendering context requested; GPUI 0.2.2 surface presenter accepts NV12 CVPixelBuffers; Servo publishes BGRA IOSurfaces; using software rendering context",
+            );
+            None
+        }
+        RenderingContextSelection::Ignore => None,
+    }
+}
+
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+enum RenderingContextSelection {
+    Forward(&'static str),
+    HoldHardware,
+    Ignore,
+}
+
+fn rendering_context_selection(raw: &str) -> RenderingContextSelection {
+    match raw.to_lowercase().as_str() {
+        "software" => RenderingContextSelection::Forward("software"),
+        "hardware" => RenderingContextSelection::HoldHardware,
+        _ => RenderingContextSelection::Ignore,
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::{RenderingContextSelection, rendering_context_selection};
+
+    #[test]
+    fn hardware_env_is_held_until_gpui_can_present_bgra_surfaces() {
+        assert_eq!(
+            rendering_context_selection("hardware"),
+            RenderingContextSelection::HoldHardware
+        );
+    }
+
+    #[test]
+    fn software_env_still_forwards_to_the_sidecar() {
+        assert_eq!(
+            rendering_context_selection("software"),
+            RenderingContextSelection::Forward("software"),
+        );
     }
 }

crates/ely_app/src/shell/web_surface_frame.rs

@@ -57,15 +57,14 @@ pub(super) struct WebSurfaceFrame {
     content_pixel_count: u64,
     #[cfg(all(test, feature = "live-site-smoke"))]
     sample_hash: u64,
-    /// Software-path image. `None` whenever the sidecar took the
-    /// hardware shortcut and dropped the RGBA payload from the
-    /// wire — the IOSurface in `pixel_buffer` is the source of truth
-    /// for that frame.
+    /// Software-path image. Current GPUI builds require this for every
+    /// ready web frame because BGRA IOSurface presentation is still
+    /// held at the protocol boundary.
     pub(super) image: Option<Arc<RenderImage>>,
-    /// Hardware-path companion: when present, the view samples the
-    /// IOSurface through GPUI's Metal pipeline via `gpui::surface(...)`
-    /// instead of uploading the RGBA bytes again. Always `None` on
-    /// the software path; `image` is the source of truth there.
+    /// Hardware-path companion imported from the sidecar. GPUI 0.2.2's
+    /// public `surface(...)` presenter accepts NV12 video buffers, and
+    /// Servo publishes BGRA IOSurfaces; this remains observability
+    /// state until a BGRA presenter is available.
     #[cfg(target_os = "macos")]
     pub(super) pixel_buffer: Option<CVPixelBuffer>,
 }
@@ -103,27 +102,23 @@ impl WebSurfaceFrame {
     }
 
     fn from_parts(parts: WebSurfaceFrameParts) -> Result<Self, WebSurfaceError> {
-        // Hardware path frames arrive with `rgba_bytes` empty — the
-        // sidecar dropped the 8 MB payload from the wire and the
-        // receiver samples the IOSurface directly. In that case the
-        // RGBA hash + LAST_FRAME_IMAGE dedup are skipped entirely.
-        let image = if parts.rgba_bytes.is_empty() {
-            None
-        } else {
-            // Servo's `read_pixels(gl::RGBA, gl::UNSIGNED_BYTE)`
-            // writes R-G-B-A in memory order. GPUI's `RenderImage` is
-            // documented as "in BGRA format" and uploads via
-            // `MTLPixelFormat::BGRA8Unorm`, which reads B-G-R-A. Hand
-            // the bytes across unchanged and the Metal sampler treats
-            // R as B (and vice versa) — every coloured pixel renders
-            // with R and B swapped. Swap once here so the rest of the
-            // pipeline (dedup hash, image buffer, GPU upload) all
-            // operate on the same BGRA representation.
+        if parts.rgba_bytes.is_empty() {
+            return Err(WebSurfaceError::MissingRenderablePayload);
+        }
+
+        // Servo's `read_pixels(gl::RGBA, gl::UNSIGNED_BYTE)` writes
+        // R-G-B-A in memory order. GPUI's `RenderImage` is documented
+        // as "in BGRA format" and uploads via
+        // `MTLPixelFormat::BGRA8Unorm`, which reads B-G-R-A. Hand the bytes across
+        // unchanged and the Metal sampler treats R as B (and vice
+        // versa) — every coloured pixel renders with R and B swapped.
+        // Swap once here so the rest of the pipeline (dedup hash,
+        // image buffer, GPU upload) all operate on the same BGRA
+        // representation.
         let mut bytes = parts.rgba_bytes;
         swap_red_blue_in_place(&mut bytes);
         let bytes_hash = rgba_hash(&bytes);
-            Some(resolve_render_image(parts.width, parts.height, bytes, bytes_hash)?)
-        };
+        let image = Some(resolve_render_image(parts.width, parts.height, bytes, bytes_hash)?);
 
         Ok(Self {
             requested_url: parts.requested_url,
@@ -245,6 +240,10 @@ struct WebSurfaceFrameParts {
 pub(super) enum WebSurfaceError {
     #[error("invalid servo frame buffer for {width}x{height}")]
     InvalidFrameBuffer { width: u32, height: u32 },
+    #[error(
+        "servo live frame did not include renderable pixels; BGRA IOSurface presentation is unavailable in GPUI 0.2.2"
+    )]
+    MissingRenderablePayload,
 }
 
 /// Swap byte 0 and byte 2 of every 4-byte pixel, converting Servo's
@@ -268,8 +267,7 @@ fn resolve_render_image(
     rgba_bytes: Vec<u8>,
     bytes_hash: u64,
 ) -> Result<Arc<RenderImage>, WebSurfaceError> {
-    LAST_FRAME_IMAGE.with(
-        |cache| -> Result<Arc<RenderImage>, WebSurfaceError> {
+    LAST_FRAME_IMAGE.with(|cache| -> Result<Arc<RenderImage>, WebSurfaceError> {
         let mut cache = cache.borrow_mut();
         if let Some((cached_hash, cached_image)) = cache.as_ref() {
             if *cached_hash == bytes_hash {
@@ -282,6 +280,5 @@ fn resolve_render_image(
         let new_image = Arc::new(RenderImage::new([image::Frame::new(image_buffer)]));
         *cache = Some((bytes_hash, new_image.clone()));
         Ok(new_image)
-        },
-    )
+    })
 }

crates/ely_app/src/shell/web_surface_tests.rs

@@ -383,6 +383,28 @@ fn live_frame_swaps_red_and_blue_bytes_for_gpui_bgra() -> Result<(), Box<dyn Err
     Ok(())
 }
 
+#[test]
+fn empty_live_frame_payload_is_rejected() {
+    use crate::services::servo_live::ServoLiveFrame;
+    use crate::shell::web_surface_frame::WebSurfaceFrame;
+    use crate::shell::web_surface_geometry::WebSurfaceScrollOffset;
+
+    let result = WebSurfaceFrame::from_live_frame(
+        "https://example.com/".to_string(),
+        WebSurfaceScrollOffset::default(),
+        100,
+        ServoLiveFrame::for_test(1, 1, Vec::new()),
+    );
+
+    let Err(error) = result else {
+        panic!("empty Servo frame payload must be rejected before it reaches Ready state");
+    };
+    assert_eq!(
+        error.to_string(),
+        "servo live frame did not include renderable pixels; BGRA IOSurface presentation is unavailable in GPUI 0.2.2",
+    );
+}
+
 fn web_bounds() -> Bounds<gpui::Pixels> {
     Bounds::new(point(px(0.0), px(0.0)), size(px(640.0), px(480.0)))
 }

crates/ely_app/src/shell/web_surface_view.rs

@@ -12,7 +12,7 @@ pub(super) fn render_ready_web_surface(
     tab: &BrowserTab,
     state_entity: Entity<ElyShell>,
 ) -> AnyElement {
-    // T14: the `gpui::surface(...)` hardware path is disabled.
+    // T14: the `gpui::surface(...)` hardware path is held.
     //
     // GPUI 0.2.2's Blade Metal renderer hard-asserts that any
     // CVPixelBuffer handed to `surface(...)` is NV12 YUV
@@ -35,10 +35,11 @@ pub(super) fn render_ready_web_surface(
             img(ImageSource::Render(image.clone())).size_full().object_fit(ObjectFit::Fill),
         );
     }
-    // Both image variants empty: the sidecar should always publish
-    // RGBA while the hardware path is disabled, but a blank canvas is
-    // the honest user-facing fallback if it ever does not.
-    render_web_surface(tab, state_entity, div().size_full())
+    render_web_surface(
+        tab,
+        state_entity,
+        error_page("Web surface frame did not include renderable pixels."),
+    )
 }
 
 pub(super) fn render_loading_web_surface(