cc: Compute draw transforms correctly when non root surfaces disabled

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 d88ebd3ae76dae9b45ddc2f9426779ab0911ed1b..be7c4a65ba8eb4df70c67d6dba5dc502993d2ab1 100644
--- a/cc/trees/property_tree.cc
+++ b/cc/trees/property_tree.cc
@@ -260,6 +260,8 @@ void TransformTree::CombineTransformsBetween(int source_id,
 
   gfx::Transform combined_transform;
   if (current->id > dest_id) {
+    // TODO(sunxd): Instead of using target space transform, only use to_parent
+    // here when we fully implement computing draw transforms on demand.
     combined_transform = ToTarget(current->id, kInvalidNodeId);
     // The stored target space transform has surface contents scale baked in,
     // but we need the unscaled transform.
@@ -1798,9 +1800,14 @@ CombinedAnimationScale PropertyTrees::GetAnimationScales(
 
     // Computing maximum animated scale in the presence of non-scale/translation
     // transforms isn't supported.
-    bool failed_for_non_scale_or_translation =
-        !transform_tree.ToTarget(transform_node_id, effect_tree.kInvalidNodeId)
-             .IsScaleOrTranslation();
+    bool failed_for_non_scale_or_translation = false;
+    if (node && node->id != transform_tree.kRootNodeId)

[ajuma, 2016/08/04 22:55:58]

Is |node| ever null here? Can we just use |transform_node_id|?

[sunxd, 2016/08/05 15:40:42]

Done.

+      failed_for_non_scale_or_translation =
+          !transform_tree
+               .ToTarget(transform_node_id,

[ajuma, 2016/08/04 22:55:58]

Hmm. Do we need to use ToTarget here? What about using ToParent instead? If any
ancestor has a non-scale-or-translation transform, the code below should already
ensure that we fail to compute animation scales, so I think we only need to
handle the case of the node itself having a non-scale-or-translation.

[sunxd, 2016/08/05 15:40:42]

Done.

+                         layer_tree_impl->LayerById(node->owner_id)
+                             ->render_target_effect_tree_index())

[ajuma, 2016/08/04 22:55:58]

If we really do need this, it'd be better to pass it in rather than adding a
dependency on LayerImpl.

[sunxd, 2016/08/05 15:40:42]

Done.

+               .IsScaleOrTranslation();
 
     // We don't attempt to accumulate animation scale from multiple nodes with
     // scale animations, because of the risk of significant overestimation. For
@@ -1865,12 +1872,14 @@ CombinedAnimationScale PropertyTrees::GetAnimationScales(
             .combined_starting_animation_scale =
             max_local_scale * ancestor_starting_animation_scale;
       } else {
-        gfx::Vector2dF ancestor_scales =
-            parent_node ? MathUtil::ComputeTransform2dScaleComponents(
-                              transform_tree.ToTarget(
-                                  parent_node->id, effect_tree.kInvalidNodeId),
-                              0.f)
-                        : gfx::Vector2dF(1.f, 1.f);
+        gfx::Vector2dF ancestor_scales = gfx::Vector2dF(1.f, 1.f);
+        if (parent_node && parent_node->id != transform_tree.kRootNodeId)
+          ancestor_scales = MathUtil::ComputeTransform2dScaleComponents(
+              transform_tree.ToTarget(

[ajuma, 2016/08/04 22:55:58]

I think we might be able to use ToScreen here (since the scale components of
ToTarget and ToScreen should be the same).

[sunxd, 2016/08/05 15:40:42]

Done.

+                  parent_node->id,
+                  layer_tree_impl->LayerById(parent_node->owner_id)
+                      ->render_target_effect_tree_index()),
+              0.f);
         float max_ancestor_scale =
             std::max(ancestor_scales.x(), ancestor_scales.y());
         cached_data_.animation_scales[transform_node_id]