T11: swap R↔B between Servo RGBA and GPUI BGRA so colours stop inverting

This commit is contained in:
2026-05-11 00:27:09 -04:00
parent d570b84c9c
commit 5d8734aefc
2 changed files with 62 additions and 7 deletions
+22 -7
View File
@@ -110,13 +110,19 @@ impl WebSurfaceFrame {
let image = if parts.rgba_bytes.is_empty() { let image = if parts.rgba_bytes.is_empty() {
None None
} else { } else {
let bytes_hash = rgba_hash(&parts.rgba_bytes); // Servo's `read_pixels(gl::RGBA, gl::UNSIGNED_BYTE)`
Some(resolve_render_image( // writes R-G-B-A in memory order. GPUI's `RenderImage` is
parts.width, // documented as "in BGRA format" and uploads via
parts.height, // `MTLPixelFormat::BGRA8Unorm`, which reads B-G-R-A. Hand
parts.rgba_bytes, // the bytes across unchanged and the Metal sampler treats
bytes_hash, // 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 { Ok(Self {
@@ -241,6 +247,15 @@ pub(super) enum WebSurfaceError {
InvalidFrameBuffer { width: u32, height: u32 }, 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 { fn rgba_hash(bytes: &[u8]) -> u64 {
let mut hasher = AHasher::default(); let mut hasher = AHasher::default();
hasher.write(bytes); hasher.write(bytes);
@@ -327,6 +327,46 @@ fn click_survives_viewport_bounds_change_before_drain() -> Result<(), Box<dyn Er
Ok(()) Ok(())
} }
/// 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 the macOS Metal atlas uploads bytes as
/// `MTLPixelFormat::BGRA8Unorm` (B-G-R-A in memory). If the bytes
/// crossed the boundary unchanged, the Metal sampler would read the R
/// byte as B and vice versa — every coloured pixel would render with
/// R and B swapped. `WebSurfaceFrame::from_live_frame` is responsible
/// for swapping the two channels before handing the buffer to GPUI.
///
/// This test fed `(R=255, G=0, B=0, A=255)` — one red RGBA pixel —
/// through the live-frame conversion and asserts the resulting
/// `RenderImage` carries `(B=0, G=0, R=255, A=255)` byte-for-byte.
#[test]
fn live_frame_swaps_red_and_blue_bytes_for_gpui_bgra() -> Result<(), Box<dyn Error>> {
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<gpui::Pixels> { fn web_bounds() -> Bounds<gpui::Pixels> {
Bounds::new(point(px(0.0), px(0.0)), size(px(640.0), px(480.0))) Bounds::new(point(px(0.0), px(0.0)), size(px(640.0), px(480.0)))
} }