From 8aaff613342568c13e9141758b770788694d2f84 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Wed, 3 Apr 2024 08:34:16 +0200 Subject: [PATCH] compositing: Send entire scene's scroll offsets when sending WebRender display lists (#31892) 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. --- components/compositing/compositor.rs | 42 +++++++++++++------ tests/wpt/meta/MANIFEST.json | 21 ++++++++++ ...sform-iframe-scroll-position-contents.html | 21 ++++++++++ .../transform-iframe-scroll-position-ref.html | 30 +++++++++++++ .../transform-iframe-scroll-position.html | 29 +++++++++++++ 5 files changed, 130 insertions(+), 13 deletions(-) create mode 100644 tests/wpt/tests/css/css-transforms/support/transform-iframe-scroll-position-contents.html create mode 100644 tests/wpt/tests/css/css-transforms/transform-iframe-scroll-position-ref.html create mode 100644 tests/wpt/tests/css/css-transforms/transform-iframe-scroll-position.html diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index ccb70d7b64e..90f5b1768f0 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -764,19 +764,7 @@ impl IOCompositor { 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 IOCompositor { // 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 index a5731bd38f0..0f111e8d255 100644 --- a/tests/wpt/meta/MANIFEST.json +++ b/tests/wpt/meta/MANIFEST.json @@ -261166,6 +261166,19 @@ {} ] ], + "transform-iframe-scroll-position.html": [ + "efb7bab532606cd9893a4cb4223dbf4b7baa6f1d", + [ + null, + [ + [ + "/css/css-transforms/transform-iframe-scroll-position-ref.html", + "==" + ] + ], + {} + ] + ], "transform-image-001.html": [ "0565b8dbeeb86b82993847a139c8f38b66c0b163", [ @@ -416071,6 +416084,10 @@ "84f079c90bcb590e81ba39753edf723bcb123858", [] ], + "transform-iframe-scroll-position-contents.html": [ + "8efcdafc83cde63f89d56ea437a4852dd82cc206", + [] + ], "transform-lime-square.png": [ "8f939993332e1101b921615723ec6067f3bb90a3", [] @@ -416208,6 +416225,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 new file mode 100644 index 00000000000..8efcdafc83c --- /dev/null +++ b/tests/wpt/tests/css/css-transforms/support/transform-iframe-scroll-position-contents.html @@ -0,0 +1,21 @@ + + + + + CSS Test (Transforms): iframe scroll position + + + + + + +
+
+
+ + + 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 new file mode 100644 index 00000000000..e4d5da75d7a --- /dev/null +++ b/tests/wpt/tests/css/css-transforms/transform-iframe-scroll-position-ref.html @@ -0,0 +1,30 @@ + + + + CSS Test (Transforms): iframe scroll position + + + +
+
+
+
+ + 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 new file mode 100644 index 00000000000..efb7bab5326 --- /dev/null +++ b/tests/wpt/tests/css/css-transforms/transform-iframe-scroll-position.html @@ -0,0 +1,29 @@ + + + + CSS Test (Transforms): iframe scroll position + + + + + + + + + +