From 3320d27c7ce7f68effb483222215dd4d6b9d5874 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Wed, 30 Jul 2025 17:27:22 +0200 Subject: [PATCH] layout: Take into account sticky in ScrollTree node to world transform and cache the value Signed-off-by: Martin Robinson Co-authored-by: Oriol Brufau --- components/compositing/webview_renderer.rs | 6 +- .../layout/display_list/stacking_context.rs | 3 +- components/layout/query.rs | 2 +- components/shared/compositing/display_list.rs | 330 +++++++++++++++--- .../shared/compositing/tests/compositor.rs | 1 + 5 files changed, 282 insertions(+), 60 deletions(-) diff --git a/components/compositing/webview_renderer.rs b/components/compositing/webview_renderer.rs index f0e7ab003b5..27a4761413d 100644 --- a/components/compositing/webview_renderer.rs +++ b/components/compositing/webview_renderer.rs @@ -265,11 +265,7 @@ impl WebViewRenderer { self.pipelines .iter_mut() .filter(|(id, _)| !attached_pipelines.contains(id)) - .for_each(|(_, details)| { - details.scroll_tree.nodes.iter_mut().for_each(|node| { - node.set_offset(LayoutVector2D::zero()); - }) - }) + .for_each(|(_, details)| details.scroll_tree.reset_all_scroll_offsets()); } /// Sets or unsets the animations-running flag for the given pipeline. Returns diff --git a/components/layout/display_list/stacking_context.rs b/components/layout/display_list/stacking_context.rs index 060589fd5fd..e2e3eb6c671 100644 --- a/components/layout/display_list/stacking_context.rs +++ b/components/layout/display_list/stacking_context.rs @@ -3,7 +3,7 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ use core::f32; -use std::cell::RefCell; +use std::cell::{Cell, RefCell}; use std::mem; use std::sync::Arc; @@ -235,6 +235,7 @@ impl StackingContextTree { clip_rect, scroll_sensitivity, offset: LayoutVector2D::zero(), + offset_changed: Cell::new(false), }), ) } diff --git a/components/layout/query.rs b/components/layout/query.rs index 429f2d92da2..28d710b7ef8 100644 --- a/components/layout/query.rs +++ b/components/layout/query.rs @@ -65,7 +65,7 @@ fn root_transform_for_layout_node( .spatial_tree_node .borrow() .expect("Should always have a scroll tree node when querying bounding box."); - Some(scroll_tree.cumulative_node_transform(&scroll_tree_node_id)) + Some(scroll_tree.cumulative_node_to_root_transform(&scroll_tree_node_id)) } pub(crate) fn process_content_box_request( diff --git a/components/shared/compositing/display_list.rs b/components/shared/compositing/display_list.rs index af55dbd2449..eec69d03c34 100644 --- a/components/shared/compositing/display_list.rs +++ b/components/shared/compositing/display_list.rs @@ -4,6 +4,7 @@ //! Defines data structures which are consumed by the Compositor. +use std::cell::Cell; use std::collections::HashMap; use base::id::ScrollTreeNodeId; @@ -114,6 +115,10 @@ pub struct ScrollableNodeInfo { /// The current offset of this scroll node. pub offset: LayoutVector2D, + + /// Whether or not the scroll offset of this node has changed and it needs it's + /// cached transformations invalidated. + pub offset_changed: Cell, } impl ScrollableNodeInfo { @@ -140,6 +145,7 @@ impl ScrollableNodeInfo { } if self.offset != original_layer_scroll_offset { + self.offset_changed.set(true); Some(self.offset) } else { None @@ -166,6 +172,7 @@ impl ScrollableNodeInfo { } self.offset.y = 0.0; + self.offset_changed.set(true); return Some(self.offset); }, ScrollLocation::End => { @@ -176,6 +183,7 @@ impl ScrollableNodeInfo { } self.offset.y = end_pos; + self.offset_changed.set(true); return Some(self.offset); }, }; @@ -190,6 +198,24 @@ impl ScrollableNodeInfo { } } +/// A cached of transforms of a particular [`ScrollTree`] node in both directions: +/// mapping from node-relative points to root-relative points and vice-versa. +/// +/// Potential ideas for improvement: +/// - Test optimizing simple translations to avoid having to do full matrix +/// multiplication when transforms are not involved. +#[derive(Clone, Copy, Debug, Default, Deserialize, Serialize)] +pub struct ScrollTreeNodeTransformationCache { + node_to_root_transform: LayoutTransform, + root_to_node_transform: Option, +} + +#[derive(Default)] +struct AncestorStickyInfo { + nearest_scrolling_ancestor_offset: LayoutVector2D, + nearest_scrolling_ancestor_viewport: LayoutRect, +} + #[derive(Debug, Deserialize, Serialize)] /// A node in a tree of scroll nodes. This may either be a scrollable /// node which responds to scroll events or a non-scrollable one. @@ -198,6 +224,9 @@ pub struct ScrollTreeNode { /// None then this is the root node. pub parent: Option, + /// The children of this [`ScrollTreeNode`]. + pub children: Vec, + /// The WebRender id, which is filled in when this tree is serialiezd /// into a WebRender display list. pub webrender_id: Option, @@ -205,6 +234,10 @@ pub struct ScrollTreeNode { /// Specific information about this node, depending on whether it is a scroll node /// or a reference frame. pub info: SpatialTreeNodeInfo, + + /// Cached transformation information that's used to do things like hit testing + /// and viewport bounding box calculation. + transformation_cache: Cell>, } impl ScrollTreeNode { @@ -231,18 +264,10 @@ impl ScrollTreeNode { } } - /// Set the offset for this node, returns false if this was a - /// non-scrolling node for which you cannot set the offset. - pub fn set_offset(&mut self, new_offset: LayoutVector2D) { - if let SpatialTreeNodeInfo::Scroll(ref mut info) = self.info { - info.scroll_to_offset(new_offset, ScrollType::Script); - } - } - /// Scroll this node given a WebRender ScrollLocation. Returns a tuple that can /// be used to scroll an individual WebRender scroll frame if the operation /// actually changed an offset. - pub fn scroll( + fn scroll( &mut self, scroll_location: ScrollLocation, context: ScrollType, @@ -299,6 +324,27 @@ impl ScrollTreeNode { }, }; } + + fn invalidate_cached_transforms(&self, scroll_tree: &ScrollTree, ancestors_invalid: bool) { + let node_invalid = match &self.info { + SpatialTreeNodeInfo::Scroll(info) if info.offset_changed.get() => { + info.offset_changed.set(false); + true + }, + _ => false, + }; + + let invalid = node_invalid || ancestors_invalid; + if invalid { + self.transformation_cache.set(None); + } + + for child_id in &self.children { + scroll_tree + .get_node(child_id) + .invalidate_cached_transforms(scroll_tree, invalid); + } + } } /// A tree of spatial nodes, which mirrors the spatial nodes in the WebRender @@ -321,12 +367,21 @@ impl ScrollTree { ) -> ScrollTreeNodeId { self.nodes.push(ScrollTreeNode { parent: parent.cloned(), + children: Vec::new(), webrender_id: None, info, + transformation_cache: Cell::default(), }); - ScrollTreeNodeId { + + let new_node_id = ScrollTreeNodeId { index: self.nodes.len() - 1, + }; + + if let Some(parent_id) = parent { + self.get_node_mut(parent_id).children.push(new_node_id); } + + new_node_id } /// Once WebRender display list construction is complete for this [`ScrollTree`], update @@ -353,11 +408,7 @@ impl ScrollTree { self.get_node(id).webrender_id() } - /// Scroll the given scroll node on this scroll tree. If the node cannot be scrolled, - /// because it isn't a scrollable node or it's already scrolled to the maximum scroll - /// extent, try to scroll an ancestor of this node. Returns the node scrolled and the - /// new offset if a scroll was performed, otherwise returns None. - pub fn scroll_node_or_ancestor( + pub fn scroll_node_or_ancestor_inner( &mut self, scroll_node_id: &ScrollTreeNodeId, scroll_location: ScrollLocation, @@ -372,7 +423,26 @@ impl ScrollTree { node.parent }; - parent.and_then(|parent| self.scroll_node_or_ancestor(&parent, scroll_location, context)) + parent.and_then(|parent| { + self.scroll_node_or_ancestor_inner(&parent, scroll_location, context) + }) + } + + /// Scroll the given scroll node on this scroll tree. If the node cannot be scrolled, + /// because it isn't a scrollable node or it's already scrolled to the maximum scroll + /// extent, try to scroll an ancestor of this node. Returns the node scrolled and the + /// new offset if a scroll was performed, otherwise returns None. + pub fn scroll_node_or_ancestor( + &mut self, + scroll_node_id: &ScrollTreeNodeId, + scroll_location: ScrollLocation, + context: ScrollType, + ) -> Option<(ExternalScrollId, LayoutVector2D)> { + let result = self.scroll_node_or_ancestor_inner(scroll_node_id, scroll_location, context); + if result.is_some() { + self.invalidate_cached_transforms(); + } + result } /// Given an [`ExternalScrollId`] and an offset, update the scroll offset of the scroll node @@ -383,14 +453,20 @@ impl ScrollTree { offset: LayoutVector2D, context: ScrollType, ) -> Option { - self.nodes.iter_mut().find_map(|node| match node.info { + let result = self.nodes.iter_mut().find_map(|node| match node.info { SpatialTreeNodeInfo::Scroll(ref mut scroll_info) if scroll_info.external_id == external_scroll_id => { scroll_info.scroll_to_offset(offset, context) }, _ => None, - }) + }); + + if result.is_some() { + self.invalidate_cached_transforms(); + } + + result } /// Given a set of all scroll offsets coming from the Servo renderer, update all of the offsets @@ -403,6 +479,18 @@ impl ScrollTree { } } } + + self.invalidate_cached_transforms(); + } + + pub fn reset_all_scroll_offsets(&mut self) { + for node in self.nodes.iter_mut() { + if let SpatialTreeNodeInfo::Scroll(ref mut scroll_info) = node.info { + scroll_info.scroll_to_offset(LayoutVector2D::zero(), ScrollType::Script); + } + } + + self.invalidate_cached_transforms(); } /// Collect all of the scroll offsets of the scrolling nodes of this tree into a @@ -426,50 +514,185 @@ impl ScrollTree { } /// Traverse a scroll node to its root to calculate the transform. - /// - /// TODO(stevennovaryo): Add caching mechanism for this. - pub fn cumulative_node_transform(&self, node_id: &ScrollTreeNodeId) -> LayoutTransform { - let current_node = self.get_node(node_id); + pub fn cumulative_node_to_root_transform(&self, node_id: &ScrollTreeNodeId) -> LayoutTransform { + let node = self.get_node(node_id); + if let Some(cached_transforms) = node.transformation_cache.get() { + return cached_transforms.node_to_root_transform; + } + + let (transforms, _) = self.cumulative_node_transform_inner(node); + node.transformation_cache.set(Some(transforms)); + transforms.node_to_root_transform + } + + /// Traverse a scroll node to its root to calculate the transform. + fn cumulative_node_transform_inner( + &self, + node: &ScrollTreeNode, + ) -> (ScrollTreeNodeTransformationCache, AncestorStickyInfo) { + let (parent_transforms, mut sticky_info) = match node.parent { + Some(parent_id) => self.cumulative_node_transform_inner(self.get_node(&parent_id)), + None => (Default::default(), Default::default()), + }; let change_basis = - |transform: &Transform3D, x: f32, y: f32, z: f32| { - let pre_translation = Transform3D::translation(x, y, z); - let post_translation = Transform3D::translation(-x, -y, -z); + |transform: &Transform3D, x: f32, y: f32| { + let pre_translation = Transform3D::translation(x, y, 0.0); + let post_translation = Transform3D::translation(-x, -y, 0.0); post_translation.then(transform).then(&pre_translation) }; - // FIXME(stevennovaryo): Ideally we should optimize the computation of simpler - // transformation like translate as it could be done - // in smaller amount of operation compared to a normal - // matrix multiplication. - let node_transform = match ¤t_node.info { - // To apply a transformation we need to make sure the rectangle's - // coordinate space is the same as reference frame's coordinate space. - // TODO(stevennovaryo): contrary to how Firefox are handling the coordinate space, - // we are ignoring zoom in transforming the coordinate - // space, and we might need to consider zoom here if it was - // implemented completely. - SpatialTreeNodeInfo::ReferenceFrame(info) => change_basis( - &info.transform, - info.frame_origin_for_query.x, - info.frame_origin_for_query.y, - 0.0, - ), - SpatialTreeNodeInfo::Scroll(info) => { - Transform3D::translation(-info.offset.x, -info.offset.y, 0.0) + let (node_to_parent_transform, parent_to_node_transform) = match &node.info { + SpatialTreeNodeInfo::ReferenceFrame(info) => { + // To apply a transformation we need to make sure the rectangle's + // coordinate space is the same as reference frame's coordinate space. + let node_to_parent_transform = change_basis( + &info.transform, + info.frame_origin_for_query.x, + info.frame_origin_for_query.y, + ); + let parent_to_node_transform = info.transform.inverse().map(|inverse_transform| { + change_basis(&inverse_transform, -info.origin.x, info.origin.y) + }); + + sticky_info.nearest_scrolling_ancestor_viewport = sticky_info + .nearest_scrolling_ancestor_viewport + .translate(-info.origin.to_vector()); + + (node_to_parent_transform, parent_to_node_transform) + }, + SpatialTreeNodeInfo::Scroll(info) => { + sticky_info.nearest_scrolling_ancestor_viewport = info.clip_rect; + sticky_info.nearest_scrolling_ancestor_offset = -info.offset; + + ( + Transform3D::translation(-info.offset.x, -info.offset.y, 0.0), + Some(Transform3D::translation(info.offset.x, info.offset.y, 0.0)), + ) + }, + + SpatialTreeNodeInfo::Sticky(info) => { + let offset = Self::calculate_sticky_offset(&sticky_info, info); + sticky_info.nearest_scrolling_ancestor_offset += offset; + ( + Transform3D::translation(offset.x, offset.y, 0.0), + Some(Transform3D::translation(-offset.x, -offset.y, 0.0)), + ) }, - // TODO(stevennovaryo): Need to consider sticky frame accurately. - SpatialTreeNodeInfo::Sticky(_) => Default::default(), }; - match current_node.parent { - // If a node is not a root, accumulate the transforms. - Some(parent_id) => { - let ancestors_transform = self.cumulative_node_transform(&parent_id); - node_transform.then(&ancestors_transform) - }, - None => node_transform, + let node_to_root_transform = + node_to_parent_transform.then(&parent_transforms.node_to_root_transform); + let root_to_node_transform = + parent_transforms + .root_to_node_transform + .and_then(|parent_transform| { + parent_to_node_transform.map(|parent_to_node_transform| { + parent_transform.then(&parent_to_node_transform) + }) + }); + + let transforms = ScrollTreeNodeTransformationCache { + node_to_root_transform, + root_to_node_transform, + }; + (transforms, sticky_info) + } + + fn calculate_sticky_offset( + ancestor_sticky_info: &AncestorStickyInfo, + info: &StickyNodeInfo, + ) -> LayoutVector2D { + let viewport_scroll_offset = &ancestor_sticky_info.nearest_scrolling_ancestor_offset; + let viewport_rect = &ancestor_sticky_info.nearest_scrolling_ancestor_viewport; + + if info.margins.top.is_none() && + info.margins.bottom.is_none() && + info.margins.left.is_none() && + info.margins.right.is_none() + { + return LayoutVector2D::zero(); } + + // The viewport and margins of the item establishes the maximum amount that it can + // be offset in order to keep it on screen. Since we care about the relationship + // between the scrolled content and unscrolled viewport we adjust the viewport's + // position by the scroll offset in order to work with their relative positions on the + // page. + let mut sticky_rect = info.frame_rect.translate(*viewport_scroll_offset); + + let mut sticky_offset = LayoutVector2D::zero(); + if let Some(margin) = info.margins.top { + let top_viewport_edge = viewport_rect.min.y + margin; + if sticky_rect.min.y < top_viewport_edge { + // If the sticky rect is positioned above the top edge of the viewport (plus margin) + // we move it down so that it is fully inside the viewport. + sticky_offset.y = top_viewport_edge - sticky_rect.min.y; + } + } + + // If we don't have a sticky-top offset (sticky_offset.y + info.previously_applied_offset.y + // == 0), or if we have a previously-applied bottom offset (previously_applied_offset.y < 0) + // then we check for handling the bottom margin case. Note that the "don't have a sticky-top + // offset" case includes the case where we *had* a sticky-top offset but we reduced it to + // zero in the above block. + if sticky_offset.y <= 0.0 { + if let Some(margin) = info.margins.bottom { + // If sticky_offset.y is nonzero that means we must have set it + // in the sticky-top handling code above, so this item must have + // both top and bottom sticky margins. We adjust the item's rect + // by the top-sticky offset, and then combine any offset from + // the bottom-sticky calculation into sticky_offset below. + sticky_rect.min.y += sticky_offset.y; + sticky_rect.max.y += sticky_offset.y; + + // Same as the above case, but inverted for bottom-sticky items. Here + // we adjust items upwards, resulting in a negative sticky_offset.y, + // or reduce the already-present upward adjustment, resulting in a positive + // sticky_offset.y. + let bottom_viewport_edge = viewport_rect.max.y - margin; + if sticky_rect.max.y > bottom_viewport_edge { + sticky_offset.y += bottom_viewport_edge - sticky_rect.max.y; + } + } + } + + // Same as above, but for the x-axis. + if let Some(margin) = info.margins.left { + let left_viewport_edge = viewport_rect.min.x + margin; + if sticky_rect.min.x < left_viewport_edge { + sticky_offset.x = left_viewport_edge - sticky_rect.min.x; + } + } + + if sticky_offset.x <= 0.0 { + if let Some(margin) = info.margins.right { + sticky_rect.min.x += sticky_offset.x; + sticky_rect.max.x += sticky_offset.x; + let right_viewport_edge = viewport_rect.max.x - margin; + if sticky_rect.max.x > right_viewport_edge { + sticky_offset.x += right_viewport_edge - sticky_rect.max.x; + } + } + } + + // The total "sticky offset" (which is the sum that was already applied by + // the calling code, stored in info.previously_applied_offset, and the extra amount we + // computed as a result of scrolling, stored in sticky_offset) needs to be + // clamped to the provided bounds. + let clamp = + |value: f32, bounds: &StickyOffsetBounds| (value).max(bounds.min).min(bounds.max); + sticky_offset.y = clamp(sticky_offset.y, &info.vertical_offset_bounds); + sticky_offset.x = clamp(sticky_offset.x, &info.horizontal_offset_bounds); + + sticky_offset + } + + fn invalidate_cached_transforms(&self) { + let Some(root_node) = self.nodes.first() else { + return; + }; + root_node.invalidate_cached_transforms(self, false /* ancestors_invalid */); } } @@ -605,6 +828,7 @@ impl CompositorDisplayListInfo { ), scroll_sensitivity: viewport_scroll_sensitivity, offset: LayoutVector2D::zero(), + offset_changed: Cell::new(false), }), ); diff --git a/components/shared/compositing/tests/compositor.rs b/components/shared/compositing/tests/compositor.rs index 157b51be3a8..fdffc3feda9 100644 --- a/components/shared/compositing/tests/compositor.rs +++ b/components/shared/compositing/tests/compositor.rs @@ -33,6 +33,7 @@ fn add_mock_scroll_node(tree: &mut ScrollTree) -> ScrollTreeNodeId { y: ScrollType::Script | ScrollType::InputEvents, }, offset: LayoutVector2D::zero(), + offset_changed: Cell::new(false), }), ) }