Implement position: sticky updates on 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 912226b25e525f0e9045e26070783bbe0cb7c5eb..ba774a87e107772152bcb0ac3552585d3d8c9e19 100644
--- a/cc/trees/property_tree.cc
+++ b/cc/trees/property_tree.cc
@@ -149,6 +149,8 @@ void TransformTree::clear() {
   nodes_affected_by_outer_viewport_bounds_delta_.clear();
   cached_data_.clear();
   cached_data_.push_back(TransformCachedNodeData());
+  sticky_position_data_.clear();
+  sticky_position_data_free_ids_.clear();
 
 #if DCHECK_IS_ON()
   TransformTree tree;
@@ -250,10 +252,13 @@ void TransformTree::UpdateTransforms(int id) {
   TransformNode* target_node = Node(TargetId(id));
   TransformNode* source_node = Node(node->source_node_id);
   property_trees()->UpdateCachedNumber();
-  if (node->needs_local_transform_update || NeedsSourceToParentUpdate(node))
+  // TODO(flackr): Only dirty when scroll offset changes.
+  if (node->sticky_position_constraint_id >= 0 ||
+      node->needs_local_transform_update || NeedsSourceToParentUpdate(node)) {
     UpdateLocalTransform(node);
-  else
+  } else {
     UndoSnapping(node);
+  }
   UpdateScreenSpaceTransform(node, parent_node, target_node);
   UpdateSurfaceContentsScale(node);
   UpdateAnimationProperties(node, parent_node);
@@ -399,6 +404,78 @@ bool TransformTree::CombineInversesBetween(int source_id,
   return all_are_invertible;
 }
 
+TransformNode* ScrollingAncestor(TransformTree* tree, TransformNode* node) {

[ajuma, 2016/09/08 22:08:54]

This function seems to be unused?

[flackr, 2016/09/20 17:08:13]

Removed. Yes, this ended up not being needed as we set up the ancestor on the
constraint.

+  do {
+    node = tree->parent(node);
+  } while (!node->scrolls);
+  return node;
+}
+
+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);
+  const LayerStickyPositionConstraint& constraint = sticky_data->constraints;
+  ScrollNode* scroll_node =
+      tree->property_trees()->scroll_tree.Node(sticky_data->scroll_ancestor);
+  gfx::ScrollOffset scroll_offset =
+      tree->property_trees()->scroll_tree.current_scroll_offset(
+          scroll_node->owner_id);
+
+  gfx::RectF clip(gfx::PointF(scroll_offset.x(), scroll_offset.y()),
+                  gfx::SizeF(scroll_node->scroll_clip_layer_bounds));
+  gfx::Vector2dF sticky_offset(
+      constraint.absolute_sticky_box_rect.OffsetFromOrigin());
+  gfx::Vector2dF layer_offset(sticky_data->main_thread_offset);
+  if (constraint.is_anchored_right) {

[ajuma, 2016/09/08 22:08:54]

Please explain (in a comment) what the math here is trying to do.

[flackr, 2016/09/20 17:08:13]

Done. I tried to summarize above.

+    float right_limit = clip.right() - constraint.right_offset;
+    float right_delta = std::min<float>(
+        0, right_limit - constraint.absolute_sticky_box_rect.right());
+    float available_space =
+        std::min<float>(0, constraint.absolute_containing_block_rect.x() -
+                               constraint.absolute_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.absolute_sticky_box_rect.x());
+    float available_space =
+        std::max<float>(0, constraint.absolute_containing_block_rect.right() -
+                               constraint.absolute_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.absolute_sticky_box_rect.bottom());
+    float available_space =
+        std::min<float>(0, constraint.absolute_containing_block_rect.y() -
+                               constraint.absolute_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.absolute_sticky_box_rect.y());
+    float available_space =
+        std::max<float>(0, constraint.absolute_containing_block_rect.bottom() -
+                               constraint.absolute_sticky_box_rect.bottom());
+    if (top_delta > available_space)
+      top_delta = available_space;
+    sticky_offset.set_y(sticky_offset.y() + top_delta);
+  }
+  return sticky_offset - layer_offset -
+         constraint.absolute_sticky_box_rect.OffsetFromOrigin();
+}
+
 void TransformTree::UpdateLocalTransform(TransformNode* node) {
   gfx::Transform transform = node->post_local;
   if (NeedsSourceToParentUpdate(node)) {
@@ -450,6 +527,9 @@ void TransformTree::UpdateLocalTransform(TransformNode* node) {
                           fixed_position_adjustment.y());
   transform.PreconcatTransform(node->local);
   transform.PreconcatTransform(node->pre_local);
+
+  transform.Translate(StickyPositionOffset(this, node));

[ajuma, 2016/09/08 22:08:54]

I'd have expected this to happen a couple lines earlier, before we preconcat the
local transform (since this seems to be an adjustment to the layer's position,
and the position itself is part of the post_local transform). Try testing with
interesting values of node->local (e.g. consider the case where that's a scale;
then whether we translate before or after will change whether the translation is
affected by the scale).

[flackr, 2016/09/20 17:08:13]

Good catch, this is a position effect and shouldn't be affected by the local
transform. I've added a test case for this and for a scaled container.

+
   node->set_to_parent(transform);
   node->needs_local_transform_update = false;
 }
@@ -759,6 +839,31 @@ bool TransformTree::operator==(const TransformTree& other) const {
          cached_data_ == other.cached_data();
 }
 
+StickyPositionNodeData* TransformTree::StickyPositionData(int node_id) {
+  TransformNode* node = Node(node_id);
+  int sticky_data_id = node->sticky_position_constraint_id;
+  if (sticky_data_id == -1) {
+    if (!sticky_position_data_free_ids_.empty()) {
+      sticky_data_id = sticky_position_data_free_ids_.back();
+      sticky_position_data_free_ids_.pop_back();
+    } else {
+      sticky_data_id = sticky_position_data_.size();
+      sticky_position_data_.push_back(StickyPositionNodeData());
+    }
+    node->sticky_position_constraint_id = sticky_data_id;
+  }
+  return &sticky_position_data_[sticky_data_id];
+}
+
+void TransformTree::SetNotSticky(int node_id) {
+  TransformNode* node = Node(node_id);
+  int sticky_data_id = node->sticky_position_constraint_id;
+  if (sticky_data_id == -1)
+    return;
+  sticky_position_data_free_ids_.push_back(sticky_data_id);

[flackr, 2016/09/08 17:52:30]

I'm not sure if we always rebuild the tree when sticky status changes. If we
think we do, or if it's rare, we could remove the sticky_position_data_free_ids
and just always add new nodes at the end of the vector.

[ajuma, 2016/09/08 22:08:54]

Since Layer::SetStickyPositionConstraint calls SetNeedsCommit (rather than
SetNeedsCommitNoRebuild), we'll rebuild property trees whenever that gets called
with a changed sticky status. Rebuilding every time this changes seems fine
since I'd expect that to change rarely (if this turns out to be common, then it
would make sense to add logic for handling it without a rebuild).

[flackr, 2016/09/20 17:08:13]

Done. I agree, I expect sticky / not sticky changes to be rare.

+  node->sticky_position_constraint_id = -1;
+}
+
 void TransformTree::ToProtobuf(proto::PropertyTree* proto) const {

[flackr, 2016/09/08 17:52:30]

When is this used? Will the sticky position data need to be added to this?

[ajuma, 2016/09/08 22:08:54]

This was added for Blimp, though currently they don't actually use this code
(since their serialization point is now before cc builds property trees, so
property trees just get built on the client). But once property-tree-building
happens in Blink (as part of SPv2) I think they will need to serialize property
trees.

In any case, please do add the sticky position data to this.

[flackr, 2016/09/20 17:08:13]

Done.

   DCHECK(!proto->has_property_type());
   proto->set_property_type(proto::PropertyTree::Transform);