compositor: Batch all pending scroll event updates into a single transaction (#36974)

When multiple WebViews are updating scroll events, instead of taking the
list of `WebView`s to avoid a double-borrow, batch scroll events into a
single transaction. This should make processing slightly more efficient
and avoids having to take the vector of WebViews.

Testing: No behavior change here and this aspect of WebView interaction
is untestable currently.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>

Cargo.lock

@@ -1103,6 +1103,7 @@ dependencies = [
  "servo_allocator",
  "servo_config",
  "servo_geometry",
+ "smallvec",
  "stylo_traits",
  "surfman",
  "tracing",

components/compositing/Cargo.toml

@@ -38,6 +38,7 @@ script_traits = { workspace = true }
 servo_allocator = { path = "../allocator" }
 servo_config = { path = "../config" }
 servo_geometry = { path = "../geometry" }
+smallvec = { workspace = true }
 stylo_traits = { workspace = true }
 tracing = { workspace = true, optional = true }
 webrender = { workspace = true }

components/compositing/compositor.rs

@@ -7,7 +7,6 @@ use std::collections::HashMap;
 use std::env;
 use std::fs::create_dir_all;
 use std::iter::once;
-use std::mem::take;
 use std::rc::Rc;
 use std::sync::Arc;
 use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
@@ -55,7 +54,7 @@ use webrender_api::{
 
 use crate::InitialCompositorState;
 use crate::webview_manager::WebViewManager;
-use crate::webview_renderer::{UnknownWebView, WebViewRenderer};
+use crate::webview_renderer::{PinchZoomResult, UnknownWebView, WebViewRenderer};
 
 #[derive(Debug, PartialEq)]
 enum UnableToComposite {
@@ -1668,11 +1667,39 @@ impl IOCompositor {
         if let Err(err) = self.rendering_context.make_current() {
             warn!("Failed to make the rendering context current: {:?}", err);
         }
-        let mut webview_renderers = take(&mut self.webview_renderers);
+
-        for webview_renderer in webview_renderers.iter_mut() {
+        let mut need_zoom = false;
-            webview_renderer.process_pending_scroll_events(self);
+        let scroll_offset_updates: Vec<_> = self
+            .webview_renderers
+            .iter_mut()
+            .filter_map(|webview_renderer| {
+                let (zoom, scroll_result) =
+                    webview_renderer.process_pending_scroll_and_pinch_zoom_events();
+                need_zoom = need_zoom || (zoom == PinchZoomResult::DidPinchZoom);
+                scroll_result
+            })
+            .collect();
+
+        if need_zoom || !scroll_offset_updates.is_empty() {
+            let mut transaction = Transaction::new();
+            if need_zoom {
+                self.send_root_pipeline_display_list_in_transaction(&mut transaction);
             }
-        self.webview_renderers = webview_renderers;
+            for update in scroll_offset_updates {
+                let offset = LayoutVector2D::new(-update.offset.x, -update.offset.y);
+                transaction.set_scroll_offsets(
+                    update.external_scroll_id,
+                    vec![SampledScrollOffset {
+                        offset,
+                        generation: 0,
+                    }],
+                );
+            }
+
+            self.generate_frame(&mut transaction, RenderReasons::APZ);
+            self.global.borrow_mut().send_transaction(transaction);
+        }
+
         self.global.borrow().shutdown_state() != ShutdownState::FinishedShuttingDown
     }
 

components/compositing/webview_renderer.rs

@@ -20,15 +20,11 @@ use fnv::FnvHashSet;
 use log::{debug, warn};
 use servo_geometry::DeviceIndependentPixel;
 use style_traits::{CSSPixel, PinchZoomFactor};
-use webrender::Transaction;
 use webrender_api::units::{
     DeviceIntPoint, DeviceIntRect, DevicePixel, DevicePoint, DeviceRect, LayoutVector2D,
 };
-use webrender_api::{
+use webrender_api::{ExternalScrollId, HitTestFlags, ScrollLocation};
-    ExternalScrollId, HitTestFlags, RenderReasons, SampledScrollOffset, ScrollLocation,
-};
 
-use crate::IOCompositor;
 use crate::compositor::{PipelineDetails, ServoRenderer};
 use crate::touch::{TouchHandler, TouchMoveAction, TouchMoveAllowed, TouchSequenceState};
 
@@ -55,6 +51,19 @@ enum ScrollZoomEvent {
     Scroll(ScrollEvent),
 }
 
+#[derive(Clone, Copy, Debug, PartialEq)]
+pub(crate) struct ScrollResult {
+    pub pipeline_id: PipelineId,
+    pub external_scroll_id: ExternalScrollId,
+    pub offset: LayoutVector2D,
+}
+
+#[derive(PartialEq)]
+pub(crate) enum PinchZoomResult {
+    DidPinchZoom,
+    DidNotPinchZoom,
+}
+
 /// A renderer for a libservo `WebView`. This is essentially the [`ServoRenderer`]'s interface to a
 /// libservo `WebView`, but the code here cannot depend on libservo in order to prevent circular
 /// dependencies, which is why we store a `dyn WebViewTrait` here instead of the `WebView` itself.
@@ -737,9 +746,17 @@ impl WebViewRenderer {
         self.on_scroll_window_event(scroll_location, cursor)
     }
 
-    pub(crate) fn process_pending_scroll_events(&mut self, compositor: &mut IOCompositor) {
+    /// Process pending scroll events for this [`WebViewRenderer`]. Returns a tuple containing:
+    ///
+    ///  - A boolean that is true if a zoom occurred.
+    ///  - An optional [`ScrollResult`] if a scroll occurred.
+    ///
+    /// It is up to the caller to ensure that these events update the rendering appropriately.
+    pub(crate) fn process_pending_scroll_and_pinch_zoom_events(
+        &mut self,
+    ) -> (PinchZoomResult, Option<ScrollResult>) {
         if self.pending_scroll_zoom_events.is_empty() {
-            return;
+            return (PinchZoomResult::DidNotPinchZoom, None);
         }
 
         // Batch up all scroll events into one, or else we'll do way too much painting.
@@ -790,37 +807,24 @@ impl WebViewRenderer {
             }
         }
 
-        let zoom_changed =
-            self.set_pinch_zoom_level(self.pinch_zoom_level().get() * combined_magnification);
         let scroll_result = combined_scroll_event.and_then(|combined_event| {
             self.scroll_node_at_device_point(
                 combined_event.cursor.to_f32(),
                 combined_event.scroll_location,
             )
         });
-        if !zoom_changed && scroll_result.is_none() {
+        if let Some(scroll_result) = scroll_result {
-            return;
+            self.send_scroll_positions_to_layout_for_pipeline(scroll_result.pipeline_id);
         }
 
-        let mut transaction = Transaction::new();
+        let pinch_zoom_result = match self
-        if zoom_changed {
+            .set_pinch_zoom_level(self.pinch_zoom_level().get() * combined_magnification)
-            compositor.send_root_pipeline_display_list_in_transaction(&mut transaction);
+        {
-        }
+            true => PinchZoomResult::DidPinchZoom,
+            false => PinchZoomResult::DidNotPinchZoom,
+        };
 
-        if let Some((pipeline_id, external_id, offset)) = scroll_result {
+        (pinch_zoom_result, scroll_result)
-            let offset = LayoutVector2D::new(-offset.x, -offset.y);
-            transaction.set_scroll_offsets(
-                external_id,
-                vec![SampledScrollOffset {
-                    offset,
-                    generation: 0,
-                }],
-            );
-            self.send_scroll_positions_to_layout_for_pipeline(pipeline_id);
-        }
-
-        compositor.generate_frame(&mut transaction, RenderReasons::APZ);
-        self.global.borrow_mut().send_transaction(transaction);
     }
 
     /// Perform a hit test at the given [`DevicePoint`] and apply the [`ScrollLocation`]
@@ -831,7 +835,7 @@ impl WebViewRenderer {
         &mut self,
         cursor: DevicePoint,
         scroll_location: ScrollLocation,
-    ) -> Option<(PipelineId, ExternalScrollId, LayoutVector2D)> {
+    ) -> Option<ScrollResult> {
         let scroll_location = match scroll_location {
             ScrollLocation::Delta(delta) => {
                 let device_pixels_per_page = self.device_pixels_per_page_pixel();
@@ -871,8 +875,12 @@ impl WebViewRenderer {
                 let scroll_result = pipeline_details
                     .scroll_tree
                     .scroll_node_or_ancestor(scroll_tree_node, scroll_location);
-                if let Some((external_id, offset)) = scroll_result {
+                if let Some((external_scroll_id, offset)) = scroll_result {
-                    return Some((*pipeline_id, external_id, offset));
+                    return Some(ScrollResult {
+                        pipeline_id: *pipeline_id,
+                        external_scroll_id,
+                        offset,
+                    });
                 }
             }
         }