cc : Snap texture layers to pixel boundary

cc/trees/property_tree.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <stddef.h>
 
 #include <set>
 #include <vector>
 
 #include "base/logging.h"
@@ skipping 407 matching lines @@
     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::PointF scroll_position(scroll_offset.x(), scroll_offset.y());
-  // The scroll position does not include snapping which shifts the scroll
-  // offset to align to a pixel boundary, we need to manually include it here.
-  scroll_position -= tree->property_trees()
-                         ->transform_tree.Node(scroll_node->transform_id)
-                         ->scroll_snap;
+  TransformNode* scroll_ancestor_transform_node =
+      tree->Node(scroll_node->transform_id);
+  if (scroll_ancestor_transform_node->scrolls) {
+    // The scroll position does not include snapping which shifts the scroll
+    // offset to align to a pixel boundary, we need to manually include it here.
+    // In this case, snapping is caused by a scroll.
+    scroll_position -= scroll_ancestor_transform_node->snap_amount;
+  }
 
   gfx::RectF clip(
       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);
 
   // In each of the following cases, we measure the limit which is the point
@@ skipping 59 matching lines @@
 }
 
 void TransformTree::UpdateLocalTransform(TransformNode* node) {
   gfx::Transform transform = node->post_local;
   if (NeedsSourceToParentUpdate(node)) {
     gfx::Transform to_parent;
     ComputeTranslation(node->source_node_id, node->parent_id, &to_parent);
     gfx::Vector2dF unsnapping;
     TransformNode* current;
     TransformNode* parent_node;
+    // Since we are calculating the adjustment for fixed position node or a
+    // scroll child, we need to unsnap only if the snap was caused by a scroll.
     for (current = Node(node->source_node_id); current->id > node->parent_id;
          current = parent(current)) {
-      unsnapping.Subtract(current->scroll_snap);
+      if (current->scrolls)
+        unsnapping.Subtract(current->snap_amount);

[ajuma, 2016/10/26 21:19:39]

Could we DCHECK that the snap_amount is non-zero only if current->scrolls? I
don't think Blink will ever construct a layer tree where a canvas' layer has a
fixed-pos descendant or a scroll child (but if it ever did, we'd need to account
for snapping of those layers too here in order to get the right source_to_parent
value).

[jaydasika, 2016/10/26 21:49:13]

Done.

     }
     for (parent_node = Node(node->parent_id);
          parent_node->id > node->source_node_id;
          parent_node = parent(parent_node)) {
-      unsnapping.Add(parent_node->scroll_snap);
+      if (parent_node->scrolls)
+        unsnapping.Add(parent_node->snap_amount);
     }
     // If a node NeedsSourceToParentUpdate, the node is either a fixed position
     // node or a scroll child.
     // If the node has a fixed position, the parent of the node is an ancestor
     // of source node, current->id should be equal to node->parent_id.
     // Otherwise, the node's source node is always an ancestor of the node owned
     // by the scroll parent, so parent_node->id should be equal to
     // node->source_node_id.
     DCHECK(current->id == node->parent_id ||
            parent_node->id == node->source_node_id);
@@ skipping 96 matching lines @@
 void TransformTree::UpdateAnimationProperties(TransformNode* node,
                                               TransformNode* parent_node) {
   bool ancestor_is_animating = false;
   if (parent_node)
     ancestor_is_animating = parent_node->to_screen_is_potentially_animated;
   node->to_screen_is_potentially_animated =
       node->has_potential_animation || ancestor_is_animating;
 }
 
 void TransformTree::UndoSnapping(TransformNode* node) {
-  // to_parent transform has the scroll snap from previous frame baked in.
+  // to_parent transform has snapping from previous frame baked in.
   // We need to undo it and use the un-snapped transform to compute current
   // target and screen space transforms.
-  node->to_parent.Translate(-node->scroll_snap.x(), -node->scroll_snap.y());
+  node->to_parent.Translate(-node->snap_amount.x(), -node->snap_amount.y());
 }
 
 void TransformTree::UpdateSnapping(TransformNode* node) {
-  if (!node->scrolls || node->to_screen_is_potentially_animated ||
+  if (!node->should_be_snapped || node->to_screen_is_potentially_animated ||
       !ToScreen(node->id).IsScaleOrTranslation() ||
       !node->ancestors_are_invertible) {
     return;
   }
 
-  // Scroll snapping must be done in screen space (the pixels we care about).
+  // Snapping must be done in screen space (the pixels we care about).

[enne (OOO), 2016/10/26 20:32:33]

This comment is not actually true.  Snapping really needs to be done in target
space (and then that render pass snapped as well, if necessary).  I think we've
always done it screen space because it's easier and works most of the time if
there's not an intermediate render pass with a snap-destroying transform.

[jaydasika, 2016/10/26 21:49:13]

Done.

   // This means we effectively snap the screen space transform. If ST is the
   // screen space transform and ST' is ST with its translation components
   // rounded, then what we're after is the scroll delta X, where ST * X = ST'.
-  // I.e., we want a transform that will realize our scroll snap. It follows
-  // that X = ST^-1 * ST'. We cache ST and ST^-1 to make this more efficient.
+  // I.e., we want a transform that will realize our snap. It follows that
+  // X = ST^-1 * ST'. We cache ST and ST^-1 to make this more efficient.
   gfx::Transform rounded = ToScreen(node->id);
   rounded.RoundTranslationComponents();
   gfx::Transform delta = FromScreen(node->id);
   delta *= rounded;
 
   DCHECK(delta.IsApproximatelyIdentityOrTranslation(SkDoubleToMScalar(1e-4)))
       << delta.ToString();
 
   gfx::Vector2dF translation = delta.To2dTranslation();
 
-  // Now that we have our scroll delta, we must apply it to each of our
-  // combined, to/from matrices.
+  // Now that we have our delta, we must apply it to each of our combined,
+  // to/from matrices.
   SetToScreen(node->id, rounded);
   node->to_parent.Translate(translation.x(), translation.y());
   gfx::Transform from_screen = FromScreen(node->id);
   from_screen.matrix().postTranslate(-translation.x(), -translation.y(), 0);
   SetFromScreen(node->id, from_screen);
-  node->scroll_snap = translation;
+  node->snap_amount = translation;
 }
 
 void TransformTree::UpdateTransformChanged(TransformNode* node,
                                            TransformNode* parent_node,
                                            TransformNode* source_node) {
   if (parent_node && parent_node->transform_changed) {
     node->transform_changed = true;
     return;
   }
 
@@ skipping 1685 matching lines @@
 
   const EffectNode* effect_node = effect_tree.Node(effect_id);
 
   bool success = GetFromTarget(transform_id, effect_id, transform);
   transform->Scale(effect_node->surface_contents_scale.x(),
                    effect_node->surface_contents_scale.y());
   return success;
 }
 
 }  // namespace cc