From 5d8734aefc2361d11316658c871f58dba3791af9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Mon, 11 May 2026 00:27:09 -0400 Subject: [PATCH] =?UTF-8?q?T11:=20swap=20R=E2=86=94B=20between=20Servo=20R?= =?UTF-8?q?GBA=20and=20GPUI=20BGRA=20so=20colours=20stop=20inverting?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/ely_app/src/shell/web_surface_frame.rs | 29 ++++++++++---- crates/ely_app/src/shell/web_surface_tests.rs | 40 +++++++++++++++++++ 2 files changed, 62 insertions(+), 7 deletions(-) diff --git a/crates/ely_app/src/shell/web_surface_frame.rs b/crates/ely_app/src/shell/web_surface_frame.rs index 6466c0f..d98bdc6 100644 --- a/crates/ely_app/src/shell/web_surface_frame.rs +++ b/crates/ely_app/src/shell/web_surface_frame.rs @@ -110,13 +110,19 @@ impl WebSurfaceFrame { let image = if parts.rgba_bytes.is_empty() { None } else { - let bytes_hash = rgba_hash(&parts.rgba_bytes); - Some(resolve_render_image( - parts.width, - parts.height, - parts.rgba_bytes, - bytes_hash, - )?) + // 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)?) }; Ok(Self { @@ -241,6 +247,15 @@ pub(super) enum WebSurfaceError { InvalidFrameBuffer { width: u32, height: u32 }, } +/// Swap byte 0 and byte 2 of every 4-byte pixel, converting Servo's +/// RGBA8 output into GPUI's expected BGRA8 in place. ~0.8 ms on a +/// 1080p frame; cheap relative to the AHash pass that follows. +fn swap_red_blue_in_place(bytes: &mut [u8]) { + for pixel in bytes.chunks_exact_mut(4) { + pixel.swap(0, 2); + } +} + fn rgba_hash(bytes: &[u8]) -> u64 { let mut hasher = AHasher::default(); hasher.write(bytes); diff --git a/crates/ely_app/src/shell/web_surface_tests.rs b/crates/ely_app/src/shell/web_surface_tests.rs index 9f13c86..0bcb526 100644 --- a/crates/ely_app/src/shell/web_surface_tests.rs +++ b/crates/ely_app/src/shell/web_surface_tests.rs @@ -327,6 +327,46 @@ fn click_survives_viewport_bounds_change_before_drain() -> Result<(), Box Result<(), Box> { + use crate::services::servo_live::ServoLiveFrame; + use crate::shell::web_surface_frame::WebSurfaceFrame; + use crate::shell::web_surface_geometry::WebSurfaceScrollOffset; + + let rgba_red_pixel = vec![255u8, 0, 0, 255]; + let live = ServoLiveFrame::for_test(1, 1, rgba_red_pixel); + let frame = WebSurfaceFrame::from_live_frame( + "https://example.com/".to_string(), + WebSurfaceScrollOffset::default(), + 100, + live, + )?; + + let image = frame.image.as_ref().ok_or("software frame must produce a RenderImage")?; + let bytes = image.as_bytes(0).ok_or("RenderImage must expose its first frame's bytes")?; + assert_eq!( + bytes, + &[0u8, 0, 255, 255], + "Servo's RGBA red pixel must be swapped to BGRA (B=0, G=0, R=255, A=255) \ + before GPUI uploads it as BGRA8Unorm. If this assert says \ + `[255, 0, 0, 255]`, the swap is missing and every coloured \ + pixel on the page renders with R and B exchanged.", + ); + Ok(()) +} + fn web_bounds() -> Bounds { Bounds::new(point(px(0.0), px(0.0)), size(px(640.0), px(480.0))) }