Fix Servo IOSurface orientation and resize identity
This commit is contained in:
@@ -37,7 +37,13 @@ use thiserror::Error;
|
||||
/// Constructed lazily by the renderer-side client on the first
|
||||
/// hardware-path frame.
|
||||
pub(crate) struct IOSurfaceCache {
|
||||
pixel_buffers: HashMap<u64, CVPixelBuffer>,
|
||||
pixel_buffers: HashMap<u64, CachedPixelBuffer>,
|
||||
}
|
||||
|
||||
struct CachedPixelBuffer {
|
||||
pixel_buffer: CVPixelBuffer,
|
||||
width: u32,
|
||||
height: u32,
|
||||
}
|
||||
|
||||
#[derive(Debug, Error)]
|
||||
@@ -54,21 +60,15 @@ impl IOSurfaceCache {
|
||||
}
|
||||
|
||||
/// Import an IOSurface published by the sidecar's
|
||||
/// `surface_handle` field. Idempotent on `surface_id`: a second
|
||||
/// call with the same id immediately deallocates the duplicate
|
||||
/// mach port without re-importing. The sender's T10.3 dedup means
|
||||
/// the duplicate path should never fire in practice — it's here
|
||||
/// so a misbehaving sidecar can't quietly leak ports.
|
||||
/// `surface_handle` field. Idempotent on `surface_id` plus pixel
|
||||
/// dimensions: duplicate handles for the same sized IOSurface are
|
||||
/// discarded, while a resized IOSurface that reuses the same
|
||||
/// `surface_id` replaces the cached pixel buffer.
|
||||
pub fn import(
|
||||
&mut self,
|
||||
mach_port_name: u32,
|
||||
surface_id: u64,
|
||||
) -> Result<(), SurfaceImportError> {
|
||||
if self.pixel_buffers.contains_key(&surface_id) {
|
||||
deallocate_mach_port(mach_port_name);
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let Some(iosurface) = objc2_io_surface::IOSurfaceRef::lookup_from_mach_port(mach_port_name)
|
||||
else {
|
||||
return Err(SurfaceImportError::LookupFailed { port: mach_port_name });
|
||||
@@ -91,8 +91,19 @@ impl IOSurfaceCache {
|
||||
|
||||
let pixel_buffer = CVPixelBuffer::from_io_surface(&io_surface_view, None)
|
||||
.map_err(|status| SurfaceImportError::PixelBufferBuildFailed { status })?;
|
||||
let width = pixel_buffer.get_width() as u32;
|
||||
let height = pixel_buffer.get_height() as u32;
|
||||
|
||||
self.pixel_buffers.insert(surface_id, pixel_buffer);
|
||||
if self
|
||||
.pixel_buffers
|
||||
.get(&surface_id)
|
||||
.is_some_and(|cached| cached.width == width && cached.height == height)
|
||||
{
|
||||
deallocate_mach_port(mach_port_name);
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
self.pixel_buffers.insert(surface_id, CachedPixelBuffer { pixel_buffer, width, height });
|
||||
deallocate_mach_port(mach_port_name);
|
||||
Ok(())
|
||||
}
|
||||
@@ -104,7 +115,7 @@ impl IOSurfaceCache {
|
||||
/// atomic increment) so the caller can hand it to GPUI's
|
||||
/// `surface(...)` element without holding a borrow on the cache.
|
||||
pub fn pixel_buffer_for(&self, surface_id: u64) -> Option<CVPixelBuffer> {
|
||||
self.pixel_buffers.get(&surface_id).cloned()
|
||||
self.pixel_buffers.get(&surface_id).map(|cached| cached.pixel_buffer.clone())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -156,9 +167,6 @@ mod tests {
|
||||
};
|
||||
use std::os::raw::c_void;
|
||||
|
||||
const TEST_WIDTH: u32 = 64;
|
||||
const TEST_HEIGHT: u32 = 48;
|
||||
|
||||
/// Build a CPU-backed IOSurface from scratch, the same way
|
||||
/// surfman's macOS backend does.
|
||||
///
|
||||
@@ -167,13 +175,13 @@ mod tests {
|
||||
///
|
||||
/// The pointer-casts mirror
|
||||
/// `surfman::platform::macos::system::surface::create_io_surface`.
|
||||
fn build_local_iosurface() -> Result<CFRetained<IOSurfaceRef>, String> {
|
||||
fn build_local_iosurface(width: u32, height: u32) -> Result<CFRetained<IOSurfaceRef>, String> {
|
||||
let pixel_format: i32 = i32::from_be_bytes(*b"BGRA");
|
||||
let bytes_per_element: i32 = 4;
|
||||
let bytes_per_row: i32 = (TEST_WIDTH as i32) * bytes_per_element;
|
||||
let bytes_per_row: i32 = (width as i32) * bytes_per_element;
|
||||
|
||||
let width_num = CFNumber::new_i32(TEST_WIDTH as i32);
|
||||
let height_num = CFNumber::new_i32(TEST_HEIGHT as i32);
|
||||
let width_num = CFNumber::new_i32(width as i32);
|
||||
let height_num = CFNumber::new_i32(height as i32);
|
||||
let bpe_num = CFNumber::new_i32(bytes_per_element);
|
||||
let bpr_num = CFNumber::new_i32(bytes_per_row);
|
||||
let pf_num = CFNumber::new_i32(pixel_format);
|
||||
@@ -207,7 +215,7 @@ mod tests {
|
||||
#[test]
|
||||
fn imports_local_iosurface_into_pixel_buffer() -> Result<(), String> {
|
||||
let mut cache = IOSurfaceCache::new();
|
||||
let iosurface = build_local_iosurface()?;
|
||||
let iosurface = build_local_iosurface(64, 48)?;
|
||||
let mach_port = iosurface.create_mach_port();
|
||||
assert!(mach_port != 0, "IOSurfaceCreateMachPort must yield a real port");
|
||||
let surface_id: u64 = 0xDEAD_BEEFu64;
|
||||
@@ -219,12 +227,12 @@ mod tests {
|
||||
.ok_or_else(|| "imported pixel buffer was missing".to_string())?;
|
||||
assert_eq!(
|
||||
pixel_buffer.get_width() as u32,
|
||||
TEST_WIDTH,
|
||||
64,
|
||||
"CVPixelBuffer width must match the source IOSurface",
|
||||
);
|
||||
assert_eq!(
|
||||
pixel_buffer.get_height() as u32,
|
||||
TEST_HEIGHT,
|
||||
48,
|
||||
"CVPixelBuffer height must match the source IOSurface",
|
||||
);
|
||||
assert_eq!(cache.cached_surface_count(), 1);
|
||||
@@ -234,7 +242,7 @@ mod tests {
|
||||
#[test]
|
||||
fn second_import_with_same_surface_id_is_idempotent() -> Result<(), String> {
|
||||
let mut cache = IOSurfaceCache::new();
|
||||
let iosurface = build_local_iosurface()?;
|
||||
let iosurface = build_local_iosurface(64, 48)?;
|
||||
let port_a = iosurface.create_mach_port();
|
||||
let port_b = iosurface.create_mach_port();
|
||||
assert!(port_a != 0 && port_b != 0 && port_a != port_b);
|
||||
@@ -246,4 +254,23 @@ mod tests {
|
||||
assert_eq!(cache.cached_surface_count(), 1);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn same_surface_id_with_changed_dimensions_replaces_pixel_buffer() -> Result<(), String> {
|
||||
let mut cache = IOSurfaceCache::new();
|
||||
let initial = build_local_iosurface(64, 48)?;
|
||||
let resized = build_local_iosurface(96, 72)?;
|
||||
let surface_id = 0xBBBB_BBBB;
|
||||
|
||||
cache.import(initial.create_mach_port(), surface_id).map_err(|error| error.to_string())?;
|
||||
cache.import(resized.create_mach_port(), surface_id).map_err(|error| error.to_string())?;
|
||||
|
||||
let pixel_buffer = cache
|
||||
.pixel_buffer_for(surface_id)
|
||||
.ok_or_else(|| "resized pixel buffer was missing".to_string())?;
|
||||
assert_eq!(pixel_buffer.get_width() as u32, 96);
|
||||
assert_eq!(pixel_buffer.get_height() as u32, 72);
|
||||
assert_eq!(cache.cached_surface_count(), 1);
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user