layout: Take into account sticky in ScrollTree node to world transform and cache the value

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Co-authored-by: Oriol Brufau <obrufau@igalia.com>

components/compositing/webview_renderer.rs

@@ -265,11 +265,7 @@ impl WebViewRenderer {
         self.pipelines
             .iter_mut()
             .filter(|(id, _)| !attached_pipelines.contains(id))
-            .for_each(|(_, details)| {
+            .for_each(|(_, details)| details.scroll_tree.reset_all_scroll_offsets());
-                details.scroll_tree.nodes.iter_mut().for_each(|node| {
-                    node.set_offset(LayoutVector2D::zero());
-                })
-            })
     }
 
     /// Sets or unsets the animations-running flag for the given pipeline. Returns

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),
             }),
         )
     }

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(

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<bool>,
 }
 
 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<LayoutTransform>,
+}
+
+#[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<ScrollTreeNodeId>,
 
+    /// The children of this [`ScrollTreeNode`].
+    pub children: Vec<ScrollTreeNodeId>,
+
     /// The WebRender id, which is filled in when this tree is serialiezd
     /// into a WebRender display list.
     pub webrender_id: Option<SpatialId>,
@@ -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<Option<ScrollTreeNodeTransformationCache>>,
 }
 
 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,
+    pub fn scroll_node_or_ancestor_inner(
-    /// 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,
@@ -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<LayoutVector2D> {
-        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.
-    ///
+    pub fn cumulative_node_to_root_transform(&self, node_id: &ScrollTreeNodeId) -> LayoutTransform {
-    /// TODO(stevennovaryo): Add caching mechanism for this.
+        let node = self.get_node(node_id);
-    pub fn cumulative_node_transform(&self, node_id: &ScrollTreeNodeId) -> LayoutTransform {
+        if let Some(cached_transforms) = node.transformation_cache.get() {
-        let current_node = self.get_node(node_id);
+            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<f32, LayoutPixel, LayoutPixel>, x: f32, y: f32, z: f32| {
+            |transform: &Transform3D<f32, LayoutPixel, LayoutPixel>, x: f32, y: f32| {
-                let pre_translation = Transform3D::translation(x, y, z);
+                let pre_translation = Transform3D::translation(x, y, 0.0);
-                let post_translation = Transform3D::translation(-x, -y, -z);
+                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
+        let (node_to_parent_transform, parent_to_node_transform) = match &node.info {
-        //                       transformation like translate as it could be done
+            SpatialTreeNodeInfo::ReferenceFrame(info) => {
-        //                       in smaller amount of operation compared to a normal
+                // To apply a transformation we need to make sure the rectangle's
-        //                       matrix multiplication.
+                // coordinate space is the same as reference frame's coordinate space.
-        let node_transform = match &current_node.info {
+                let node_to_parent_transform = change_basis(
-            // To apply a transformation we need to make sure the rectangle's
+                    &info.transform,
-            // coordinate space is the same as reference frame's coordinate space.
+                    info.frame_origin_for_query.x,
-            // TODO(stevennovaryo): contrary to how Firefox are handling the coordinate space,
+                    info.frame_origin_for_query.y,
-            //                      we are ignoring zoom in transforming the coordinate
+                );
-            //                      space, and we might need to consider zoom here if it was
+                let parent_to_node_transform = info.transform.inverse().map(|inverse_transform| {
-            //                      implemented completely.
+                    change_basis(&inverse_transform, -info.origin.x, info.origin.y)
-            SpatialTreeNodeInfo::ReferenceFrame(info) => change_basis(
+                });
-                &info.transform,
+
-                info.frame_origin_for_query.x,
+                sticky_info.nearest_scrolling_ancestor_viewport = sticky_info
-                info.frame_origin_for_query.y,
+                    .nearest_scrolling_ancestor_viewport
-                0.0,
+                    .translate(-info.origin.to_vector());
-            ),
+
-            SpatialTreeNodeInfo::Scroll(info) => {
+                (node_to_parent_transform, parent_to_node_transform)
-                Transform3D::translation(-info.offset.x, -info.offset.y, 0.0)
+            },
+            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 {
+        let node_to_root_transform =
-            // If a node is not a root, accumulate the transforms.
+            node_to_parent_transform.then(&parent_transforms.node_to_root_transform);
-            Some(parent_id) => {
+        let root_to_node_transform =
-                let ancestors_transform = self.cumulative_node_transform(&parent_id);
+            parent_transforms
-                node_transform.then(&ancestors_transform)
+                .root_to_node_transform
-            },
+                .and_then(|parent_transform| {
-            None => node_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),
             }),
         );
 

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),
         }),
     )
 }