cc: Use sublayer scale from effect tree (2)

cc/trees/draw_property_utils.cc

Index: cc/trees/draw_property_utils.cc
diff --git a/cc/trees/draw_property_utils.cc b/cc/trees/draw_property_utils.cc
index 1fe57213e6c7cc02c3146a455bd348c4b1830a4d..8600cf6a0d16ee580a3ac0d831be66d85ae70b3d 100644
--- a/cc/trees/draw_property_utils.cc
+++ b/cc/trees/draw_property_utils.cc
@@ -93,20 +93,27 @@ bool ComputeClipRectInTargetSpace(const LayerType* layer,
                                         target_node_id, &clip_to_target)) {
       // We don't have to apply sublayer scale when target is root.
       if (target_node_id != 0) {
-        PostConcatSublayerScale(layer->effect_tree_index(), effect_tree,
-                                &clip_to_target);
+        const EffectNode* effect_node =
+            effect_tree.Node(layer->effect_tree_index());
+        const EffectNode* target_effect_node =
+            effect_node->has_render_surface
+                ? effect_node
+                : effect_tree.Node(effect_node->target_id);
+        PostConcatSublayerScale(target_effect_node, &clip_to_target);
 #if DCHECK_IS_ON()
         VerifySublayerScalesMatch(layer->effect_tree_index(), target_node_id,
                                   effect_tree, transform_tree);
 #endif
       }
 
-      const TransformNode* source_node =
-          transform_tree.Node(clip_node->target_transform_id);
-      if (source_node->sublayer_scale.x() != 0.f &&
-          source_node->sublayer_scale.y() != 0.f)
-        clip_to_target.Scale(1.0f / source_node->sublayer_scale.x(),
-                             1.0f / source_node->sublayer_scale.y());
+      const EffectNode* source_node =
+          effect_tree.Node(clip_node->target_effect_id);
+      ConcatInverseSublayerScale(source_node, &clip_to_target);
+#if DCHECK_IS_ON()
+      VerifySublayerScalesMatch(clip_node->target_effect_id,
+                                clip_node->target_transform_id, effect_tree,
+                                transform_tree);
+#endif
       *clip_rect_in_target_space = MathUtil::MapClippedRect(
           clip_to_target, clip_node->clip_in_target_space);
     } else {
@@ -132,13 +139,18 @@ struct ConditionalClip {
 static ConditionalClip ComputeTargetRectInLocalSpace(
     gfx::RectF rect,
     const TransformTree& transform_tree,
+    const EffectTree& effect_tree,
     int current_transform_id,
-    int target_transform_id) {
+    int target_transform_id,
+    const int current_effect_id) {
   gfx::Transform current_to_target;
-  if (!transform_tree.ComputeTransformWithSourceSublayerScale(
-          current_transform_id, target_transform_id, &current_to_target))
+  bool success = transform_tree.ComputeTransform(
+      current_transform_id, target_transform_id, &current_to_target);
+  if (!success)
     // If transform is not invertible, cannot apply clip.
     return ConditionalClip{false, gfx::RectF()};
+  const EffectNode* current_effect_node = effect_tree.Node(current_effect_id);
+  ConcatInverseSublayerScale(current_effect_node, &current_to_target);
 
   if (current_transform_id > target_transform_id)
     return ConditionalClip{true,  // is_clipped.
@@ -162,7 +174,8 @@ static ConditionalClip ComputeLocalRectInTargetSpace(
     return ConditionalClip{false, gfx::RectF()};
   // We don't have to apply sublayer scale when target is root.
   if (target_transform_id != 0) {
-    PostConcatSublayerScale(target_effect_id, effect_tree, &current_to_target);
+    const EffectNode* target_effect_node = effect_tree.Node(target_effect_id);
+    PostConcatSublayerScale(target_effect_node, &current_to_target);
 #if DCHECK_IS_ON()
     VerifySublayerScalesMatch(target_effect_id, target_transform_id,
                               effect_tree, transform_tree);
@@ -189,7 +202,7 @@ static ConditionalClip ComputeCurrentClip(const ClipNode* clip_node,
 
   gfx::RectF current_clip = clip_node->clip;
   gfx::Vector2dF sublayer_scale =
-      transform_tree.Node(target_transform_id)->sublayer_scale;
+      effect_tree.Node(target_effect_id)->sublayer_scale;
   if (sublayer_scale.x() > 0 && sublayer_scale.y() > 0)
     current_clip.Scale(sublayer_scale.x(), sublayer_scale.y());
   return ConditionalClip{true /* is_clipped */, current_clip};
@@ -374,9 +387,9 @@ void CalculateVisibleRects(
       const EffectNode* copy_request_effect_node =
           effect_tree.Node(effect_ancestor_with_copy_request);
       ConditionalClip clip_in_layer_space = ComputeTargetRectInLocalSpace(
-          accumulated_clip_in_copy_request_space, transform_tree,
-          copy_request_effect_node->transform_id,
-          layer->transform_tree_index());
+          accumulated_clip_in_copy_request_space, transform_tree, effect_tree,
+          copy_request_effect_node->transform_id, layer->transform_tree_index(),
+          copy_request_effect_node->id);
 
       if (clip_in_layer_space.is_clipped) {
         gfx::RectF clip_rect = clip_in_layer_space.clip_rect;
@@ -491,16 +504,29 @@ void CalculateVisibleRects(
     if (transform_node->ancestors_are_invertible) {
       target_to_layer = transform_tree.FromTarget(transform_node->id);
     } else {
-      if (!transform_tree.ComputeTransformWithSourceSublayerScale(
-              target_node_id, transform_node->id, &target_to_layer)) {
+      bool success = transform_tree.ComputeTransform(
+          target_node_id, transform_node->id, &target_to_layer);
+      if (!success) {
         // An animated singular transform may become non-singular during the
         // animation, so we still need to compute a visible rect. In this
         // situation, we treat the entire layer as visible.
         layer->set_visible_layer_rect(gfx::Rect(layer_bounds));
         continue;
       }
+      if (target_node_id != 0) {
+        const EffectNode* effect_node =
+            effect_tree.Node(layer->effect_tree_index());
+        const EffectNode* target_effect_node =
+            effect_node->has_render_surface
+                ? effect_node
+                : effect_tree.Node(effect_node->target_id);
+        ConcatInverseSublayerScale(target_effect_node, &target_to_layer);

[Ian Vollick, 2016/07/15 19:59:07]

This logic (grab the effect node, do a ternary, call Concat..) seems to be
repeated in a number of places. Could it be refactored (even if it's just the
bit to get the target effect node)?

[jaydasika, 2016/07/15 20:28:49]

Done.

+#if DCHECK_IS_ON()
+        VerifySublayerScalesMatch(target_effect_node->id, target_node_id,
+                                  effect_tree, transform_tree);
+#endif
+      }
     }
-
     gfx::Transform target_to_content;
     target_to_content.Translate(-layer->offset_to_transform_parent().x(),
                                 -layer->offset_to_transform_parent().y());
@@ -700,24 +726,16 @@ static void ResetIfHasNanCoordinate(gfx::RectF* rect) {
     *rect = gfx::RectF();
 }
 
-void PostConcatSublayerScale(const int effect_node_id,
-                             const EffectTree& effect_tree,
+void PostConcatSublayerScale(const EffectNode* effect_node,
                              gfx::Transform* transform) {
-  // TODO(jaydasika): This function should not compute target effect node id. It
-  // should receive it from callers.
-  const EffectNode* effect_node = effect_tree.Node(effect_node_id);
-  const EffectNode* target_effect_node =
-      effect_node->has_render_surface
-          ? effect_node
-          : effect_tree.Node(effect_node->target_id);
-  transform->matrix().postScale(target_effect_node->sublayer_scale.x(),
-                                target_effect_node->sublayer_scale.y(), 1.f);
+  DCHECK(effect_node->has_render_surface);
+  transform->matrix().postScale(effect_node->sublayer_scale.x(),
+                                effect_node->sublayer_scale.y(), 1.f);
 }
 
-void ConcatInverseSublayerScale(const int effect_node_id,
-                                const EffectTree& effect_tree,
+void ConcatInverseSublayerScale(const EffectNode* effect_node,
                                 gfx::Transform* transform) {
-  const EffectNode* effect_node = effect_tree.Node(effect_node_id);
+  DCHECK(effect_node->has_render_surface);
   if (effect_node->sublayer_scale.x() != 0.0 &&
       effect_node->sublayer_scale.y() != 0.0)
     transform->Scale(1.0 / effect_node->sublayer_scale.x(),
@@ -770,19 +788,26 @@ void ComputeClips(ClipTree* clip_tree,
           &parent_to_current);
       // We don't have to apply sublayer scale when target is root.
       if (clip_node->target_transform_id != 0) {
-        PostConcatSublayerScale(clip_node->target_effect_id, effect_tree,
-                                &parent_to_current);
+        const EffectNode* target_effect_node =
+            effect_tree.Node(clip_node->target_effect_id);
+        PostConcatSublayerScale(target_effect_node, &parent_to_current);
 #if DCHECK_IS_ON()
         VerifySublayerScalesMatch(clip_node->target_effect_id,
                                   clip_node->target_transform_id, effect_tree,
                                   transform_tree);
 #endif
       }
-      if (parent_target_transform_node->sublayer_scale.x() > 0 &&
-          parent_target_transform_node->sublayer_scale.y() > 0)
-        parent_to_current.Scale(
-            1.f / parent_target_transform_node->sublayer_scale.x(),
-            1.f / parent_target_transform_node->sublayer_scale.y());
+      if (parent_clip_node->target_transform_id != 0) {
+        const EffectNode* parent_target_effect_node =
+            effect_tree.Node(parent_clip_node->target_effect_id);
+        ConcatInverseSublayerScale(parent_target_effect_node,
+                                   &parent_to_current);
+#if DCHECK_IS_ON()
+        VerifySublayerScalesMatch(parent_clip_node->target_effect_id,
+                                  parent_clip_node->target_transform_id,
+                                  effect_tree, transform_tree);
+#endif
+      }
       // If we can't compute a transform, it's because we had to use the inverse
       // of a singular transform. We won't draw in this case, so there's no need
       // to compute clips.
@@ -844,8 +869,9 @@ void ComputeClips(ClipTree* clip_tree,
             &source_to_target);
         // We don't have to apply sublayer scale when target is root.
         if (clip_node->target_transform_id != 0) {
-          PostConcatSublayerScale(clip_node->target_effect_id, effect_tree,
-                                  &source_to_target);
+          const EffectNode* target_effect_node =
+              effect_tree.Node(clip_node->target_effect_id);
+          PostConcatSublayerScale(target_effect_node, &source_to_target);
 #if DCHECK_IS_ON()
           VerifySublayerScalesMatch(clip_node->target_effect_id,
                                     clip_node->target_transform_id, effect_tree,
@@ -1006,8 +1032,13 @@ static void VerifyDrawTransformsMatch(LayerImpl* layer,
                                                   &draw_transform);
   // We don't have to apply sublayer scale when target is root.
   if (destination_id != 0) {
-    PostConcatSublayerScale(layer->effect_tree_index(),
-                            property_trees->effect_tree, &draw_transform);
+    const EffectNode* effect_node =
+        property_trees->effect_tree.Node(layer->effect_tree_index());
+    const EffectNode* target_effect_node =
+        effect_node->has_render_surface
+            ? effect_node
+            : property_trees->effect_tree.Node(effect_node->target_id);
+    PostConcatSublayerScale(target_effect_node, &draw_transform);
 #if DCHECK_IS_ON()
     VerifySublayerScalesMatch(layer->effect_tree_index(), destination_id,
                               property_trees->effect_tree,
@@ -1203,19 +1234,20 @@ static void SetSurfaceDrawTransform(const TransformTree& transform_tree,
                                   &render_surface_transform);
   // We don't have to apply sublayer scale when target is root.
   if (target_transform_node->id != 0) {
-    PostConcatSublayerScale(effect_node->target_id, effect_tree,
-                            &render_surface_transform);
+    const EffectNode* target_effect_node =
+        effect_tree.Node(effect_node->target_id);
+    PostConcatSublayerScale(target_effect_node, &render_surface_transform);
 #if DCHECK_IS_ON()
     VerifySublayerScalesMatch(effect_node->target_id, target_transform_node->id,
                               effect_tree, transform_tree);
 #endif
   }
 
-  DCHECK(transform_node->sublayer_scale == effect_node->sublayer_scale);
-  if (effect_node->sublayer_scale.x() != 0.0 &&
-      effect_node->sublayer_scale.y() != 0.0)
-    render_surface_transform.Scale(1.0 / effect_node->sublayer_scale.x(),
-                                   1.0 / effect_node->sublayer_scale.y());
+  ConcatInverseSublayerScale(effect_node, &render_surface_transform);
+#if DCHECK_IS_ON()
+  VerifySublayerScalesMatch(effect_node->id, transform_node->id, effect_tree,
+                            transform_tree);
+#endif
   render_surface->SetDrawTransform(render_surface_transform);
 }
 
@@ -1261,8 +1293,9 @@ static void SetSurfaceClipRect(const ClipNode* parent_clip_node,
 
   // We don't have to apply sublayer scale when target is root.
   if (transform_tree.TargetId(transform_node->id) != 0) {
-    PostConcatSublayerScale(render_surface->EffectTreeIndex(), effect_tree,
-                            &clip_parent_target_to_target);
+    const EffectNode* effect_node =
+        effect_tree.Node(render_surface->EffectTreeIndex());
+    PostConcatSublayerScale(effect_node, &clip_parent_target_to_target);
 #if DCHECK_IS_ON()
     VerifySublayerScalesMatch(render_surface->EffectTreeIndex(),
                               transform_tree.TargetId(transform_node->id),