Handle nested position:sticky elements correctly (compositor)

cc/trees/property_tree.cc

Index: cc/trees/property_tree.cc
diff --git a/cc/trees/property_tree.cc b/cc/trees/property_tree.cc
index b2ee4daa8170d1eab1bff1599f8802b688bec07e..45dec64c3fe3d429768ec9283810cc0f8755053c 100644
--- a/cc/trees/property_tree.cc
+++ b/cc/trees/property_tree.cc
@@ -348,8 +348,7 @@ bool TransformTree::CombineInversesBetween(int source_id,
 gfx::Vector2dF StickyPositionOffset(TransformTree* tree, TransformNode* node) {
   if (node->sticky_position_constraint_id == -1)
     return gfx::Vector2dF();
-  const StickyPositionNodeData* sticky_data =
-      tree->StickyPositionData(node->id);
+  StickyPositionNodeData* sticky_data = tree->StickyPositionData(node->id);
   const LayerStickyPositionConstraint& constraint = sticky_data->constraints;
   ScrollNode* scroll_node =
       tree->property_trees()->scroll_tree.Node(sticky_data->scroll_ancestor);
@@ -370,10 +369,36 @@ gfx::Vector2dF StickyPositionOffset(TransformTree* tree, TransformNode* node) {
       scroll_position,
       gfx::SizeF(tree->property_trees()->scroll_tree.scroll_clip_layer_bounds(
           scroll_node->id)));
-  gfx::Vector2dF sticky_offset(
-      constraint.scroll_container_relative_sticky_box_rect.OffsetFromOrigin());
   gfx::Vector2dF layer_offset(sticky_data->main_thread_offset);
 
+  gfx::Vector2dF ancestor_sticky_box_offset;
+  if (sticky_data->nearest_node_shifting_sticky_box !=
+      TransformTree::kInvalidNodeId) {
+    ancestor_sticky_box_offset =
+        tree->StickyPositionData(sticky_data->nearest_node_shifting_sticky_box)
+            ->total_sticky_box_sticky_offset;
+  }
+
+  gfx::Vector2dF ancestor_containing_block_offset;
+  if (sticky_data->nearest_node_shifting_containing_block !=
+      TransformTree::kInvalidNodeId) {
+    ancestor_containing_block_offset =
+        tree->StickyPositionData(
+                sticky_data->nearest_node_shifting_containing_block)
+            ->total_containing_block_sticky_offset;
+  }
+
+  // Compute the current position of the constraint rects based on the original
+  // positions and the offsets from ancestor sticky elements.
+  gfx::RectF sticky_box_rect =
+      gfx::RectF(constraint.scroll_container_relative_sticky_box_rect) +
+      ancestor_sticky_box_offset + ancestor_containing_block_offset;
+  gfx::RectF containing_block_rect =
+      gfx::RectF(constraint.scroll_container_relative_containing_block_rect) +
+      ancestor_containing_block_offset;

[chrishtr, 2017/03/21 23:26:05]

Your code adds on an offset from the ancestor sticky layer, and also an offset
related to the
containing block? Can you give me an example to think through where this offset
is not directly related
or computable from the ancestor sticky layer offset? I'm sure it's the case,
just need to see an
example.

[smcgruer, 2017/03/22 15:15:03]

If I understand correctly, you are asking about ancestor_sticky_box_offset vs
ancestor_containing_block_offset? (And more generally, the StickyPositionData
for these two ancestor layers).

With sticky elements (layers) there are always two questions to be asked: how
much do I need to offset to be where I want to be, and how much am I *allowed*
to be offset whilst staying inside my container. The former depends on the
'sticky box rect' (which is where we are *before* sticky), the latter depends on
the 'containing block rect'. Note that these are scroll-container relative
afaik.

The positions for these rects are set in the sticky constraints at layout time,
and then we might scroll the page. In the naive non-nested case, this is fine -
we are already calculating the required/allowed offset based on the original
positions plus the current viewport position, so that offset just changes as you
scroll. However in the nested case we run into the issue that both of our
original rects might be wrong, because some ancestor may have been shifted due
to its own sticky offset.

So, we need to correct the sticky box rect (which affects how much we need to
offset) and the containing block rect (which affects how much we can offset).
This is where the ancestor_sticky_box_offset and
ancestor_containing_block_offset come into play. The former accumulates only the
sticky offset of elements between the current sticky element and its containing
block. This is only relevant for elements whose containing block is not their
parent, most commonly tables and inline-divs. The latter accumulates the entire
sticky offset of elements between the containing block and the ancestor scroller
(this includes both of these offsets for ancestor elements, if you look at the
code on line 450 of this file, or 79 of StickyPositionScrollingConstraints.cpp).

So, which of these shifts our two constraint rects? Well, the sticky box rect is
affected by all of the sticky stuff above it, so that is both the
ancestor_sticky_box_offset *and* ancestor_containing_block_offset. On the other
hand, the containing block rect obviously doesn't shift by the sticky stuff
between the current element and the containing block, only the higher-level
sticky, so it only shifts by ancestor_containing_block_offset.

Does that make sense?

With regards to computing it from only the layer that provides the
ancestor_sticky_box_offset, I don't quite see how that could be done. Open to
suggestions though.

In general btw, this code is just a replica of
StickyPositionScrollingConstraints::computeStickyOffset. The only reason it
exists is that we can scroll on the cc side and main will only have applied
sticky offset for the main-thread scroll amount, so we have to fix the offset
for the additional cc scroll position.

+
+  gfx::Vector2dF sticky_offset(sticky_box_rect.OffsetFromOrigin());
+
   // In each of the following cases, we measure the limit which is the point
   // that the element should stick to, clamping on one side to 0 (because sticky
   // only pushes elements in one direction). Then we clamp to how far we can
@@ -385,55 +410,52 @@ gfx::Vector2dF StickyPositionOffset(TransformTree* tree, TransformNode* node) {
   // over bottom offset.
   if (constraint.is_anchored_right) {
     float right_limit = clip.right() - constraint.right_offset;
-    float right_delta = std::min<float>(
-        0, right_limit -
-               constraint.scroll_container_relative_sticky_box_rect.right());
-    float available_space = std::min<float>(
-        0, constraint.scroll_container_relative_containing_block_rect.x() -
-               constraint.scroll_container_relative_sticky_box_rect.x());
+    float right_delta =
+        std::min<float>(0, right_limit - sticky_box_rect.right());
+    float available_space =
+        std::min<float>(0, containing_block_rect.x() - sticky_box_rect.x());
     if (right_delta < available_space)
       right_delta = available_space;
     sticky_offset.set_x(sticky_offset.x() + right_delta);
   }
   if (constraint.is_anchored_left) {
     float left_limit = clip.x() + constraint.left_offset;
-    float left_delta = std::max<float>(
-        0,
-        left_limit - constraint.scroll_container_relative_sticky_box_rect.x());
+    float left_delta = std::max<float>(0, left_limit - sticky_box_rect.x());
     float available_space = std::max<float>(
-        0, constraint.scroll_container_relative_containing_block_rect.right() -
-               constraint.scroll_container_relative_sticky_box_rect.right());
+        0, containing_block_rect.right() - sticky_box_rect.right());
     if (left_delta > available_space)
       left_delta = available_space;
     sticky_offset.set_x(sticky_offset.x() + left_delta);
   }
   if (constraint.is_anchored_bottom) {
     float bottom_limit = clip.bottom() - constraint.bottom_offset;
-    float bottom_delta = std::min<float>(
-        0, bottom_limit -
-               constraint.scroll_container_relative_sticky_box_rect.bottom());
-    float available_space = std::min<float>(
-        0, constraint.scroll_container_relative_containing_block_rect.y() -
-               constraint.scroll_container_relative_sticky_box_rect.y());
+    float bottom_delta =
+        std::min<float>(0, bottom_limit - sticky_box_rect.bottom());
+    float available_space =
+        std::min<float>(0, containing_block_rect.y() - sticky_box_rect.y());
     if (bottom_delta < available_space)
       bottom_delta = available_space;
     sticky_offset.set_y(sticky_offset.y() + bottom_delta);
   }
   if (constraint.is_anchored_top) {
     float top_limit = clip.y() + constraint.top_offset;
-    float top_delta = std::max<float>(
-        0,
-        top_limit - constraint.scroll_container_relative_sticky_box_rect.y());
+    float top_delta = std::max<float>(0, top_limit - sticky_box_rect.y());
     float available_space = std::max<float>(
-        0, constraint.scroll_container_relative_containing_block_rect.bottom() -
-               constraint.scroll_container_relative_sticky_box_rect.bottom());
+        0, containing_block_rect.bottom() - sticky_box_rect.bottom());
     if (top_delta > available_space)
       top_delta = available_space;
     sticky_offset.set_y(sticky_offset.y() + top_delta);
   }
+
+  sticky_data->total_sticky_box_sticky_offset =
+      ancestor_sticky_box_offset + sticky_offset -
+      sticky_box_rect.OffsetFromOrigin();
+  sticky_data->total_containing_block_sticky_offset =
+      ancestor_sticky_box_offset + ancestor_containing_block_offset +
+      sticky_offset - sticky_box_rect.OffsetFromOrigin();
+
   return sticky_offset - layer_offset - node->source_to_parent -
-         constraint.scroll_container_relative_sticky_box_rect
-             .OffsetFromOrigin();
+         sticky_box_rect.OffsetFromOrigin();
 }
 
 void TransformTree::UpdateLocalTransform(TransformNode* node) {