From 4dded465a45cb0e3a8b35549e85cf60cb752bdcf Mon Sep 17 00:00:00 2001 From: sagudev <16504129+sagudev@users.noreply.github.com> Date: Thu, 26 Jun 2025 16:57:15 +0200 Subject: [PATCH] compositor: only `UpdateImages` that accepts `SmallVec` and add helpers for single image (#37730) Before we only offered helper to add single image (no update or delete) that got special IPC message, now we simplify this by offering all ops helpers for dealing with single image (that happens most of the time), that simply uses `update_images` under the hood. We also optimize for this use case by using `SmallVec<[ImageUpdate; 1]>` to avoid alloc. Testing: Just refactor, but code is covered by WPT tests --------- Signed-off-by: sagudev <16504129+sagudev@users.noreply.github.com> --- Cargo.lock | 1 + components/canvas/canvas_data.rs | 13 ++++-------- components/compositing/compositor.rs | 6 ------ components/compositing/tracing.rs | 1 - components/script/dom/htmlmediaelement.rs | 2 +- components/shared/compositing/Cargo.toml | 1 + components/shared/compositing/lib.rs | 24 ++++++++++++++++------- components/webgl/webgl_thread.rs | 11 +++-------- components/webgpu/swapchain.rs | 19 +++++++++--------- 9 files changed, 36 insertions(+), 42 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 00f117f49d7..96160a3b2b4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1429,6 +1429,7 @@ dependencies = [ "serde", "servo_geometry", "servo_malloc_size_of", + "smallvec", "strum_macros", "stylo", "stylo_traits", diff --git a/components/canvas/canvas_data.rs b/components/canvas/canvas_data.rs index 0b9239dac92..0e54cd2ce0f 100644 --- a/components/canvas/canvas_data.rs +++ b/components/canvas/canvas_data.rs @@ -8,7 +8,7 @@ use std::sync::Arc; use app_units::Au; use canvas_traits::canvas::*; -use compositing_traits::{CrossProcessCompositorApi, ImageUpdate, SerializableImageData}; +use compositing_traits::{CrossProcessCompositorApi, SerializableImageData}; use euclid::default::{Box2D, Point2D, Rect, Size2D, Transform2D, Vector2D}; use euclid::point2; use fonts::{ @@ -431,7 +431,7 @@ impl<'a, B: Backend> CanvasData<'a, B> { }; let data = SerializableImageData::Raw(IpcSharedMemory::from_bytes(draw_target.bytes().as_ref())); - compositor_api.update_images(vec![ImageUpdate::AddImage(image_key, descriptor, data)]); + compositor_api.add_image(image_key, descriptor, data); CanvasData { state: backend.new_paint_state(), backend, @@ -1212,11 +1212,7 @@ impl<'a, B: Backend> CanvasData<'a, B> { )); self.compositor_api - .update_images(vec![ImageUpdate::UpdateImage( - self.image_key, - descriptor, - data, - )]); + .update_image(self.image_key, descriptor, data); } // https://html.spec.whatwg.org/multipage/#dom-context-2d-putimagedata @@ -1342,8 +1338,7 @@ impl<'a, B: Backend> CanvasData<'a, B> { impl Drop for CanvasData<'_, B> { fn drop(&mut self) { - self.compositor_api - .update_images(vec![ImageUpdate::DeleteImage(self.image_key)]); + self.compositor_api.delete_image(self.image_key); } } diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index 2505aba7603..79a74db93c7 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -941,12 +941,6 @@ impl IOCompositor { self.global.borrow_mut().send_transaction(transaction); }, - CompositorMsg::AddImage(key, desc, data) => { - let mut txn = Transaction::new(); - txn.add_image(key, desc, data.into(), None); - self.global.borrow_mut().send_transaction(txn); - }, - CompositorMsg::GenerateFontKeys( number_of_font_keys, number_of_font_instance_keys, diff --git a/components/compositing/tracing.rs b/components/compositing/tracing.rs index 65f9bd76c08..a130eaaa242 100644 --- a/components/compositing/tracing.rs +++ b/components/compositing/tracing.rs @@ -48,7 +48,6 @@ mod from_constellation { Self::SendDisplayList { .. } => target!("SendDisplayList"), Self::HitTest(..) => target!("HitTest"), Self::GenerateImageKey(..) => target!("GenerateImageKey"), - Self::AddImage(..) => target!("AddImage"), Self::UpdateImages(..) => target!("UpdateImages"), Self::GenerateFontKeys(..) => target!("GenerateFontKeys"), Self::AddFont(..) => target!("AddFont"), diff --git a/components/script/dom/htmlmediaelement.rs b/components/script/dom/htmlmediaelement.rs index 68ac43905f4..cc1c998db93 100644 --- a/components/script/dom/htmlmediaelement.rs +++ b/components/script/dom/htmlmediaelement.rs @@ -200,7 +200,7 @@ impl MediaFrameRenderer { impl VideoFrameRenderer for MediaFrameRenderer { fn render(&mut self, frame: VideoFrame) { - let mut updates = vec![]; + let mut updates = smallvec::smallvec![]; if let Some(old_image_key) = mem::replace(&mut self.very_old_frame, self.old_frame.take()) { updates.push(ImageUpdate::DeleteImage(old_image_key)); diff --git a/components/shared/compositing/Cargo.toml b/components/shared/compositing/Cargo.toml index 8578c7dabd2..8b1dde8e092 100644 --- a/components/shared/compositing/Cargo.toml +++ b/components/shared/compositing/Cargo.toml @@ -34,6 +34,7 @@ profile_traits = { path = '../profile' } raw-window-handle = { version = "0.6" } serde = { workspace = true } servo_geometry = { path = "../../geometry" } +smallvec = { workspace = true } strum_macros = { workspace = true } stylo = { workspace = true } stylo_traits = { workspace = true } diff --git a/components/shared/compositing/lib.rs b/components/shared/compositing/lib.rs index d6bf3ce2c70..607dda6cfe9 100644 --- a/components/shared/compositing/lib.rs +++ b/components/shared/compositing/lib.rs @@ -17,6 +17,7 @@ use ipc_channel::ipc::IpcSender; use log::warn; use malloc_size_of_derive::MallocSizeOf; use pixels::RasterImage; +use smallvec::SmallVec; use strum_macros::IntoStaticStr; use style_traits::CSSPixel; use webrender_api::DocumentId; @@ -147,10 +148,8 @@ pub enum CompositorMsg { /// Create a new image key. The result will be returned via the /// provided channel sender. GenerateImageKey(IpcSender), - /// Add an image with the given data and `ImageKey`. - AddImage(ImageKey, ImageDescriptor, SerializableImageData), /// Perform a resource update operation. - UpdateImages(Vec), + UpdateImages(SmallVec<[ImageUpdate; 1]>), /// Generate a new batch of font keys which can be used to allocate /// keys asynchronously. @@ -307,13 +306,24 @@ impl CrossProcessCompositorApi { descriptor: ImageDescriptor, data: SerializableImageData, ) { - if let Err(e) = self.0.send(CompositorMsg::AddImage(key, descriptor, data)) { - warn!("Error sending image update: {}", e); - } + self.update_images([ImageUpdate::AddImage(key, descriptor, data)].into()); + } + + pub fn update_image( + &self, + key: ImageKey, + descriptor: ImageDescriptor, + data: SerializableImageData, + ) { + self.update_images([ImageUpdate::UpdateImage(key, descriptor, data)].into()); + } + + pub fn delete_image(&self, key: ImageKey) { + self.update_images([ImageUpdate::DeleteImage(key)].into()); } /// Perform an image resource update operation. - pub fn update_images(&self, updates: Vec) { + pub fn update_images(&self, updates: SmallVec<[ImageUpdate; 1]>) { if let Err(e) = self.0.send(CompositorMsg::UpdateImages(updates)) { warn!("error sending image updates: {}", e); } diff --git a/components/webgl/webgl_thread.rs b/components/webgl/webgl_thread.rs index 3e1bf6b8dee..77ee8f507cd 100644 --- a/components/webgl/webgl_thread.rs +++ b/components/webgl/webgl_thread.rs @@ -23,7 +23,7 @@ use canvas_traits::webgl::{ WebGLVersion, WebGLVertexArrayId, YAxisTreatment, }; use compositing_traits::{ - CrossProcessCompositorApi, ImageUpdate, SerializableImageData, WebrenderExternalImageRegistry, + CrossProcessCompositorApi, SerializableImageData, WebrenderExternalImageRegistry, WebrenderImageHandlerType, }; use euclid::default::Size2D; @@ -719,8 +719,7 @@ impl WebGLThread { fn remove_webgl_context(&mut self, context_id: WebGLContextId) { // Release webrender image keys. if let Some(info) = self.cached_context_info.remove(&context_id) { - self.compositor_api - .update_images(vec![ImageUpdate::DeleteImage(info.image_key)]); + self.compositor_api.delete_image(info.image_key); } // We need to make the context current so its resources can be disposed of. @@ -929,11 +928,7 @@ impl WebGLThread { let image_data = Self::external_image_data(context_id, image_buffer_kind); self.compositor_api - .update_images(vec![ImageUpdate::UpdateImage( - info.image_key, - descriptor, - image_data, - )]); + .update_image(info.image_key, descriptor, image_data); } /// Helper function to create a `ImageDescriptor`. diff --git a/components/webgpu/swapchain.rs b/components/webgpu/swapchain.rs index a53f904fc59..9737963c371 100644 --- a/components/webgpu/swapchain.rs +++ b/components/webgpu/swapchain.rs @@ -9,7 +9,7 @@ use std::sync::{Arc, Mutex}; use arrayvec::ArrayVec; use compositing_traits::{ - CrossProcessCompositorApi, ImageUpdate, SerializableImageData, WebrenderExternalImageApi, + CrossProcessCompositorApi, SerializableImageData, WebrenderExternalImageApi, WebrenderImageSource, }; use euclid::default::Size2D; @@ -310,7 +310,7 @@ impl ContextData { warn!("Unable to send FreeBuffer({:?}) ({:?})", buffer_id, e); }; } - compositor_api.update_images(vec![ImageUpdate::DeleteImage(self.image_key)]); + compositor_api.delete_image(self.image_key); } /// Returns true if presentation id was updated (was newer) @@ -421,12 +421,11 @@ impl crate::WGPU { }; if needs_image_update { - self.compositor_api - .update_images(vec![ImageUpdate::UpdateImage( - context_data.image_key, - context_data.image_desc.0, - SerializableImageData::External(context_data.image_data), - )]); + self.compositor_api.update_image( + context_data.image_key, + context_data.image_desc.0, + SerializableImageData::External(context_data.image_data), + ); } } @@ -574,11 +573,11 @@ fn update_wr_image( return; }; let old_presentation_buffer = swap_chain.data.replace(presentation_buffer); - compositor_api.update_images(vec![ImageUpdate::UpdateImage( + compositor_api.update_image( context_data.image_key, context_data.image_desc.0, SerializableImageData::External(context_data.image_data), - )]); + ); if let Some(old_presentation_buffer) = old_presentation_buffer { context_data.unmap_old_buffer(old_presentation_buffer) }