compositing: Send entire scene's scroll offsets when sending WebRende…

…r display lists WebRender does not preserve spatial tree offsets when updating the spatial tree. Updating the spatial tree of a pipeline can also update the spatial tree of child pipelines. This change ensures that WebRender always gets the scroll offsets of the entire scene when modifying display lists in a way that may rebuild the spatial tree. Fixes #31807.
servo · Mar 29, 2024 · dba60f1 · dba60f1
1 parent 2d5a1cd
commit dba60f1
Show file tree

Hide file tree

Showing 5 changed files with 130 additions and 13 deletions.
diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs
@@ -764,19 +764,7 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
                 let mut transaction = Transaction::new();
                 transaction
                     .set_display_list(display_list_info.epoch, (pipeline_id, built_display_list));
-
-                for node in details.scroll_tree.nodes.iter() {
-                    if let (Some(offset), Some(external_id)) = (node.offset(), node.external_id()) {
-                        let offset = LayoutVector2D::new(-offset.x, -offset.y);
-                        transaction.set_scroll_offsets(
-                            external_id,
-                            vec![SampledScrollOffset {
-                                offset,
-                                generation: 0,
-                            }],
-                        );
-                    }
-                }
+                self.update_transaction_with_all_scroll_offsets(&mut transaction);
                 self.generate_frame(&mut transaction, RenderReasons::SCENE);
                 self.webrender_api
                     .send_transaction(self.webrender_document, transaction);
@@ -1091,6 +1079,34 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
         // be an issue. WebRender will still update the scene and generate a new
         // frame even though the epoch hasn't changed.
         transaction.set_display_list(WebRenderEpoch(0), built_display_list);
+        self.update_transaction_with_all_scroll_offsets(transaction);
+    }
+
+    /// Update the given transaction with the scroll offsets of all active scroll nodes in
+    /// the WebRender scene. This is necessary because WebRender does not preserve scroll
+    /// offsets between scroll tree modifications. If a display list could potentially
+    /// modify a scroll tree branch, WebRender needs to have scroll offsets for that
+    /// branch.
+    ///
+    /// TODO(mrobinson): Could we only send offsets for the branch being modified
+    /// and not the entire scene?
+    fn update_transaction_with_all_scroll_offsets(&self, transaction: &mut Transaction) {
+        for details in self.pipeline_details.values() {
+            for node in details.scroll_tree.nodes.iter() {
+                let (Some(offset), Some(external_id)) = (node.offset(), node.external_id()) else {
+                    continue;
+                };
+
+                let offset = LayoutVector2D::new(-offset.x, -offset.y);
+                transaction.set_scroll_offsets(
+                    external_id,
+                    vec![SampledScrollOffset {
+                        offset,
+                        generation: 0,
+                    }],
+                );
+            }
+        }
     }
 
     fn set_frame_tree(&mut self, frame_tree: &SendableFrameTree) {

diff --git a/tests/wpt/meta/MANIFEST.json b/tests/wpt/meta/MANIFEST.json
@@ -260859,6 +260859,19 @@
        {}
       ]
      ],
+     "transform-iframe-scroll-position.html": [
+      "efb7bab532606cd9893a4cb4223dbf4b7baa6f1d",
+      [
+       null,
+       [
+        [
+         "/css/css-transforms/transform-iframe-scroll-position-ref.html",
+         "=="
+        ]
+       ],
+       {}
+      ]
+     ],
      "transform-image-001.html": [
       "0565b8dbeeb86b82993847a139c8f38b66c0b163",
       [
@@ -415416,6 +415429,10 @@
        "84f079c90bcb590e81ba39753edf723bcb123858",
        []
       ],
+      "transform-iframe-scroll-position-contents.html": [
+       "8efcdafc83cde63f89d56ea437a4852dd82cc206",
+       []
+      ],
       "transform-lime-square.png": [
        "8f939993332e1101b921615723ec6067f3bb90a3",
        []
@@ -415553,6 +415570,10 @@
       "b674c88d82f8a806a8a1cd20040302766d825202",
       []
      ],
+     "transform-iframe-scroll-position-ref.html": [
+      "e4d5da75d7a762b6c346640b2c72339a52d350ab",
+      []
+     ],
      "transform-image-ref.html": [
       "301c0f94bb7806caad2444583f3642d49aa4c969",
       []

diff --git a/tests/wpt/tests/css/css-transforms/support/transform-iframe-scroll-position-contents.html b/tests/wpt/tests/css/css-transforms/support/transform-iframe-scroll-position-contents.html
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+
+<html>
+    <head>
+        <title>CSS Test (Transforms): iframe scroll position</title>
+        <link rel="author" title="Martin Robinson" href="mailto:mrobinson@igalia.com">
+        <style>
+            html { background: red; }
+        </style>
+    </head>
+
+    <body>
+        <!-- Make a large red page with a small green and blue square that is scrolled to immediately. -->
+        <div style="position: absolute; width: 50px; height: 25px; top: 3000px; left: 3000px; background: green;"></div>
+        <div style="position: absolute; width: 50px; height: 25px; top: 3025px; left: 3000px; background: blue;"></div>
+        <div style="width: 10000px; height: 10000px;"></div>
+        <script>
+            window.scrollTo(3000, 3000);
+        </script>
+    </body>
+</html>
diff --git a/tests/wpt/tests/css/css-transforms/transform-iframe-scroll-position-ref.html b/tests/wpt/tests/css/css-transforms/transform-iframe-scroll-position-ref.html
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <title>CSS Test (Transforms): iframe scroll position</title>
+    <link rel="author" title="Martin Robinson" href="mailto:mrobinson@igalia.com">
+    <style>
+        #iframe {
+            border: 0;
+            width: 50px;
+            height: 50px;
+            border: solid;
+        }
+
+        #iframe div {
+            width: 25px;
+            height: 50px;
+            float: left;
+        }
+
+        .rotate {
+            transform: rotate(90deg);
+        }
+    </style>
+    <body onload="onLoad();">
+        <div id="iframe">
+            <div style="background: blue;"></div>
+            <div style="background: green;"></div>
+        </div>
+    </body>
+</html>
diff --git a/tests/wpt/tests/css/css-transforms/transform-iframe-scroll-position.html b/tests/wpt/tests/css/css-transforms/transform-iframe-scroll-position.html
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <title>CSS Test (Transforms): iframe scroll position</title>
+    <link rel="author" title="Martin Robinson" href="mailto:mrobinson@igalia.com">
+    <link rel="help" href="http://www.w3.org/TR/css-transforms-1/#transform-rendering">
+    <meta name="assert" content="This test ensures that when an iframe element is transformed, the scroll position of the iframe content is preserved.">
+    <link rel="match" href="transform-iframe-scroll-position-ref.html">
+    <style>
+        iframe {
+            border: 0;
+            width: 50px;
+            height: 50px;
+            border: solid;
+        }
+
+        .rotate {
+            transform: rotate(90deg);
+        }
+    </style>
+    <body onload="onLoad();">
+        <iframe id="iframe" src="support/transform-iframe-scroll-position-contents.html"></iframe>
+        <script>
+            function onLoad() {
+                iframe.classList.toggle("rotate");
+            }
+        </script>
+    </body>
+</html>