Bug 1886739 - Give position: sticky items their own picture cache slice r=gfx-reviewers,gw

position: sticky items scroll independently of their containing
content, resulting in large amounts of picture cache invalidation when
scrolling, causing poor performance. This patch attempts to give them
their own picture cache slice in order to avoid that invalidation.

Differential Revision: https://phabricator.services.mozilla.com/D206502

gfx/wr/webrender/src/spatial_tree.rs

@@ -338,6 +338,7 @@ impl SceneSpatialTree {
     ) -> SpatialNodeIndex {
         let mut real_scroll_root = self.root_reference_frame_index;
         let mut outermost_scroll_root = self.root_reference_frame_index;
+        let mut current_scroll_root_is_sticky = false;
         let mut node_index = spatial_node_index;
 
         while node_index != self.root_reference_frame_index {
@@ -354,10 +355,19 @@ impl SceneSpatialTree {
                             // we have encountered, as they may end up with a non-axis-aligned transform.
                             real_scroll_root = self.root_reference_frame_index;
                             outermost_scroll_root = self.root_reference_frame_index;
+                            current_scroll_root_is_sticky = false;
                         }
                     }
                 }
-                SpatialNodeType::StickyFrame(..) => {}
+                SpatialNodeType::StickyFrame(..) => {
+                    // Though not a scroll frame, we treat sticky frames as scroll roots to ensure they
+                    // are given a separate picture cache slice.
+                    outermost_scroll_root = node_index;
+                    real_scroll_root = node_index;
+                    // Set this true so that we don't select an ancestor scroll frame as the scroll root
+                    // on a subsequent iteration.
+                    current_scroll_root_is_sticky = true;
+                }
                 SpatialNodeType::ScrollFrame(ref info) => {
                     match info.frame_kind {
                         ScrollFrameKind::PipelineRoot { is_root_pipeline } => {
@@ -371,24 +381,29 @@ impl SceneSpatialTree {
                             // later on, even if it's not actually scrollable.
                             outermost_scroll_root = node_index;
 
-                            // If the scroll root has no scrollable area, we don't want to
-                            // consider it. This helps pages that have a nested scroll root
-                            // within a redundant scroll root to avoid selecting the wrong
-                            // reference spatial node for a picture cache.
-                            if info.scrollable_size.width > MIN_SCROLLABLE_AMOUNT ||
-                               info.scrollable_size.height > MIN_SCROLLABLE_AMOUNT {
-                                // Since we are skipping redundant scroll roots, we may end up
-                                // selecting inner scroll roots that are very small. There is
-                                // no performance benefit to creating a slice for these roots,
-                                // as they are cheap to rasterize. The size comparison is in
-                                // local-space, but makes for a reasonable estimate. The value
-                                // is arbitrary, but is generally small enough to ignore things
-                                // like scroll roots around text input elements.
-                                if info.viewport_rect.width() > MIN_SCROLL_ROOT_SIZE &&
-                                   info.viewport_rect.height() > MIN_SCROLL_ROOT_SIZE {
-                                    // If we've found a root that is scrollable, and a reasonable
-                                    // size, select that as the current root for this node
-                                    real_scroll_root = node_index;
+                            // If the previously identified scroll root is sticky then we don't
+                            // want to choose an ancestor scroll root, as we want the sticky item
+                            // to have its own picture cache slice.
+                            if !current_scroll_root_is_sticky {
+                                // If the scroll root has no scrollable area, we don't want to
+                                // consider it. This helps pages that have a nested scroll root
+                                // within a redundant scroll root to avoid selecting the wrong
+                                // reference spatial node for a picture cache.
+                                if info.scrollable_size.width > MIN_SCROLLABLE_AMOUNT ||
+                                   info.scrollable_size.height > MIN_SCROLLABLE_AMOUNT {
+                                    // Since we are skipping redundant scroll roots, we may end up
+                                    // selecting inner scroll roots that are very small. There is
+                                    // no performance benefit to creating a slice for these roots,
+                                    // as they are cheap to rasterize. The size comparison is in
+                                    // local-space, but makes for a reasonable estimate. The value
+                                    // is arbitrary, but is generally small enough to ignore things
+                                    // like scroll roots around text input elements.
+                                    if info.viewport_rect.width() > MIN_SCROLL_ROOT_SIZE &&
+                                       info.viewport_rect.height() > MIN_SCROLL_ROOT_SIZE {
+                                        // If we've found a root that is scrollable, and a reasonable
+                                        // size, select that as the current root for this node
+                                        real_scroll_root = node_index;
+                                    }
                                 }
                             }
                         }
@@ -2008,6 +2023,58 @@ fn test_find_scroll_root_2d_scale() {
     assert_eq!(st.find_scroll_root(sub_scroll), sub_scroll);
 }
 
+/// Tests that a sticky spatial node is chosen as the scroll root rather than
+/// its parent scroll frame
+#[test]
+fn test_find_scroll_root_sticky() {
+    let mut st = SceneSpatialTree::new();
+    let pid = PipelineInstanceId::new(0);
+
+    let root = st.add_reference_frame(
+        st.root_reference_frame_index(),
+        TransformStyle::Flat,
+        PropertyBinding::Value(LayoutTransform::identity()),
+        ReferenceFrameKind::Transform {
+            is_2d_scale_translation: true,
+            should_snap: true,
+            paired_with_perspective: false,
+        },
+        LayoutVector2D::new(0.0, 0.0),
+        PipelineId::dummy(),
+        SpatialNodeUid::external(SpatialTreeItemKey::new(0, 0), PipelineId::dummy(), pid),
+    );
+
+    let scroll = st.add_scroll_frame(
+        root,
+        ExternalScrollId(1, PipelineId::dummy()),
+        PipelineId::dummy(),
+        &LayoutRect::from_size(LayoutSize::new(400.0, 400.0)),
+        &LayoutSize::new(400.0, 800.0),
+        ScrollFrameKind::Explicit,
+        LayoutVector2D::new(0.0, 0.0),
+        APZScrollGeneration::default(),
+        HasScrollLinkedEffect::No,
+        SpatialNodeUid::external(SpatialTreeItemKey::new(0, 1), PipelineId::dummy(), pid),
+    );
+
+    let sticky = st.add_sticky_frame(
+        scroll,
+        StickyFrameInfo {
+            frame_rect: LayoutRect::from_size(LayoutSize::new(400.0, 100.0)),
+            margins: euclid::SideOffsets2D::new(Some(0.0), None, None, None),
+            vertical_offset_bounds: api::StickyOffsetBounds::new(0.0, 0.0),
+            horizontal_offset_bounds: api::StickyOffsetBounds::new(0.0, 0.0),
+            previously_applied_offset: LayoutVector2D::zero(),
+            current_offset: LayoutVector2D::zero(),
+        },
+        PipelineId::dummy(),
+        SpatialTreeItemKey::new(0, 2),
+        pid,
+    );
+
+    assert_eq!(st.find_scroll_root(sticky), sticky);
+}
+
 #[test]
 fn test_world_transforms() {
   // Create a spatial tree with a scroll frame node with scroll offset (0, 200).

layout/reftests/async-scrolling/reftest.list

@@ -68,7 +68,7 @@ fuzzy-if(Android,0-13,0-465) fuzzy-if(gtkWidget,17-28,24-60) fuzzy-if(cocoaWidge
 fuzzy-if(Android,0-13,0-465) fuzzy-if(gtkWidget,17-29,24-60) fuzzy-if(cocoaWidget,15-19,40-75) skip-if(useDrawSnapshot) == fixed-pos-scrolled-clip-4.html fixed-pos-scrolled-clip-4-ref.html # Bug 1604338
 skip-if(useDrawSnapshot) == fixed-pos-scrolled-clip-5.html fixed-pos-scrolled-clip-5-ref.html
 skip-if(useDrawSnapshot) == position-sticky-bug1434250.html position-sticky-bug1434250-ref.html
-fuzzy-if(Android,0-12,0-11) fuzzy-if(gtkWidget,16-25,12-32) fuzzy-if(cocoaWidget,13-16,20-44) skip-if(useDrawSnapshot) == position-sticky-scrolled-clip-1.html position-sticky-scrolled-clip-1-ref.html # Bug 1604338
+fuzzy-if(Android,0-12,0-45) fuzzy-if(gtkWidget,16-25,12-32) fuzzy-if(cocoaWidget,13-16,20-44) skip-if(useDrawSnapshot) == position-sticky-scrolled-clip-1.html position-sticky-scrolled-clip-1-ref.html # Bug 1604338
 fuzzy-if(Android,0-6,0-4) skip == position-sticky-scrolled-clip-2.html position-sticky-scrolled-clip-2-ref.html # bug ?????? - incorrectly applying clip to sticky contents
 fuzzy-if(Android,0-8,0-27) fuzzy-if(cocoaWidget,9-11,20-44) skip-if(useDrawSnapshot) == curtain-effect-1.html curtain-effect-1-ref.html
 fuzzy-if(Android,0-7,0-9) fuzzy-if(gtkWidget,10-15,12-32) fuzzy-if(cocoaWidget,5-9,20-42) skip-if(useDrawSnapshot) == transformed-1.html transformed-1-ref.html # Bug 1604338

layout/reftests/position-sticky/reftest.list

@@ -51,5 +51,5 @@ fuzzy(0-1,0-220) == block-in-inline-3.html block-in-inline-3-ref.html
 == iframe-1.html iframe-1-ref.html
 == transformed-1.html transformed-1-ref.html
 fuzzy-if(Android,0-8,0-9) fuzzy-if(gtkWidget,10-17,12-32) fuzzy-if(cocoaWidget,7-8,18-42) skip-if(useDrawSnapshot) fails-if(useDrawSnapshot) == transformed-2.html transformed-2-ref.html # Bug 1604644
-skip-if(useDrawSnapshot) fuzzy-if(Android,0-14,0-11) fuzzy-if(gtkWidget,19-30,12-32) fuzzy-if(cocoaWidget,13-16,20-44) fails-if(useDrawSnapshot) == nested-sticky-1.html nested-sticky-1-ref.html # Bug 1604644
-skip-if(useDrawSnapshot) fuzzy-if(Android,0-14,0-11) fuzzy-if(gtkWidget,19-30,12-32) fuzzy-if(cocoaWidget,13-16,20-44) fails-if(useDrawSnapshot) == nested-sticky-2.html nested-sticky-2-ref.html # Bug 1604644
+skip-if(useDrawSnapshot) fuzzy-if(Android,0-14,0-17) fuzzy-if(gtkWidget,19-30,12-32) fuzzy-if(cocoaWidget,13-16,20-44) fails-if(useDrawSnapshot) == nested-sticky-1.html nested-sticky-1-ref.html # Bug 1604644
+skip-if(useDrawSnapshot) fuzzy-if(Android,0-14,0-96) fuzzy-if(gtkWidget,19-30,12-32) fuzzy-if(cocoaWidget,13-16,20-44) fails-if(useDrawSnapshot) == nested-sticky-2.html nested-sticky-2-ref.html # Bug 1604644

testing/web-platform/meta/css/css-position/sticky/position-sticky-fractional-offset.html.ini

@@ -0,0 +1,3 @@
+[position-sticky-fractional-offset.html]
+  fuzzy:
+    if (os == "android"): maxDifference=0-1;totalPixels=0-54