cc: Compute draw transforms dynamically.

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 156 matching lines @@
 
 #if DCHECK_IS_ON()
   TransformTree tree;
   // TODO(jaydasika) : Move tests that expect source_to_parent_updates_allowed
   // to be true on impl thread to main thread and set it to is_main_thread here.
   tree.source_to_parent_updates_allowed_ = source_to_parent_updates_allowed_;
   DCHECK(tree == *this);
 #endif
 }
 
+void TransformTree::set_needs_update(bool needs_update) {
+  if (needs_update && !needs_update_)
+    property_trees()->UpdateCachedNumber();
+  needs_update_ = needs_update;

[ajuma, 2016/10/12 20:08:47]

Can we make needs_update_ private to make sure it's only changed by this method?

[sunxd, 2016/10/12 21:08:19]

I think needs_update_ is already private. TransformTree can access that because
it's a friend class of PropertyTree<>.

+}
+
 bool TransformTree::ComputeTransform(int source_id,
                                      int dest_id,
                                      gfx::Transform* transform) const {
   transform->MakeIdentity();
 
   if (source_id == dest_id)
     return true;
 
   if (source_id > dest_id) {
     CombineTransformsBetween(source_id, dest_id, transform);
@@ skipping 70 matching lines @@
     TransformNode* node = Node(id);
     node->transform_changed = false;
   }
 }
 
 void TransformTree::UpdateTransforms(int id) {
   TransformNode* node = Node(id);
   TransformNode* parent_node = parent(node);
   TransformNode* target_node = Node(TargetId(id));
   TransformNode* source_node = Node(node->source_node_id);
-  property_trees()->UpdateCachedNumber();
   // 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 {
     UndoSnapping(node);
   }
   UpdateScreenSpaceTransform(node, parent_node, target_node);
   UpdateSurfaceContentsScale(node);
   UpdateAnimationProperties(node, parent_node);
   UpdateSnapping(node);
-  UpdateTargetSpaceTransform(node, target_node);
   UpdateNodeAndAncestorsHaveIntegerTranslations(node, parent_node);
   UpdateTransformChanged(node, parent_node, source_node);
   UpdateNodeAndAncestorsAreAnimatedOrInvertible(node, parent_node);
 }
 
 bool TransformTree::IsDescendant(int desc_id, int source_id) const {
   while (desc_id != source_id) {
     if (desc_id == kInvalidNodeId)
       return false;
     desc_id = Node(desc_id)->parent_id;
@@ skipping 36 matching lines @@
   // surface contents scale baked in, but we need to compute an unscaled
   // transform.
   std::vector<int> source_to_destination;
   source_to_destination.push_back(current->id);
   current = parent(current);
   bool destination_has_non_zero_surface_contents_scale =
       dest->surface_contents_scale.x() != 0.f &&
       dest->surface_contents_scale.y() != 0.f;
   DCHECK(destination_has_non_zero_surface_contents_scale ||
          !dest->ancestors_are_invertible);
-  for (; current && current->id > dest_id; current = parent(current)) {
-    if (destination_has_non_zero_surface_contents_scale &&
-        TargetId(current->id) == dest_id &&
-        ContentTargetId(current->id) == dest_id)
-      break;
+  for (; current && current->id > dest_id; current = parent(current))
     source_to_destination.push_back(current->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.
     combined_transform.matrix().postScale(
         1.0f / dest->surface_contents_scale.x(),
         1.0f / dest->surface_contents_scale.y(), 1.0f);
   } else if (current->id < dest_id) {
     // We have reached the lowest common ancestor of the source and destination
     // nodes. This case can occur when we are transforming between a node
     // corresponding to a fixed-position layer (or its descendant) and the node
     // corresponding to the layer's render target. For example, consider the
@@ skipping 399 matching lines @@
 }
 
 bool TransformTree::HasNodesAffectedByInnerViewportBoundsDelta() const {
   return !nodes_affected_by_inner_viewport_bounds_delta_.empty();
 }
 
 bool TransformTree::HasNodesAffectedByOuterViewportBoundsDelta() const {
   return !nodes_affected_by_outer_viewport_bounds_delta_.empty();
 }
 
-const gfx::Transform& TransformTree::FromTarget(int node_id,
-                                                int effect_id) const {
-  DCHECK(static_cast<int>(cached_data_.size()) > node_id);
-  if (effect_id != kInvalidNodeId &&
-      property_trees()->verify_transform_tree_calculations) {
-    const gfx::Transform& transform =
-        property_trees()->GetDrawTransforms(node_id, effect_id).from_target;
-    CHECK(transform.ApproximatelyEqual(cached_data_[node_id].from_target));
-  }
-  return cached_data_[node_id].from_target;
+gfx::Transform TransformTree::FromTarget(int node_id, int effect_id) const {
+  gfx::Transform from_target;
+  property_trees()->GetFromTarget(node_id, effect_id, &from_target);
+  return from_target;
 }
 
 void TransformTree::SetFromTarget(int node_id,
                                   const gfx::Transform& transform) {
   DCHECK(static_cast<int>(cached_data_.size()) > node_id);
   cached_data_[node_id].from_target = transform;
 }
 
-const gfx::Transform& TransformTree::ToTarget(int node_id,
-                                              int effect_id) const {
-  DCHECK(static_cast<int>(cached_data_.size()) > node_id);
-  if (effect_id != kInvalidNodeId &&
-      property_trees()->verify_transform_tree_calculations) {
-    const gfx::Transform& transform =
-        property_trees()->GetDrawTransforms(node_id, effect_id).to_target;
-    if (property_trees()->non_root_surfaces_enabled)
-      CHECK(transform.ApproximatelyEqual(cached_data_[node_id].to_target));
-    else
-      CHECK(transform.ApproximatelyEqual(cached_data_[node_id].to_screen));
-  }
-  return cached_data_[node_id].to_target;
+gfx::Transform TransformTree::ToTarget(int node_id, int effect_id) const {
+  gfx::Transform to_target;
+  property_trees()->GetToTarget(node_id, effect_id, &to_target);
+  return to_target;
 }
 
 void TransformTree::SetToTarget(int node_id, const gfx::Transform& transform) {
   DCHECK(static_cast<int>(cached_data_.size()) > node_id);
   cached_data_[node_id].to_target = transform;
 }
 
 const gfx::Transform& TransformTree::FromScreen(int node_id) const {
   DCHECK(static_cast<int>(cached_data_.size()) > node_id);
   return cached_data_[node_id].from_screen;
@@ skipping 935 matching lines @@
 
 PropertyTreesCachedData::~PropertyTreesCachedData() {}
 
 PropertyTrees::PropertyTrees()
     : needs_rebuild(true),
       non_root_surfaces_enabled(true),
       changed(false),
       full_tree_damaged(false),
       sequence_number(0),
       is_main_thread(true),
-      is_active(false),
-      verify_transform_tree_calculations(false) {
+      is_active(false) {
   transform_tree.SetPropertyTrees(this);
   effect_tree.SetPropertyTrees(this);
   clip_tree.SetPropertyTrees(this);
   scroll_tree.SetPropertyTrees(this);
 }
 
 PropertyTrees::~PropertyTrees() {}
 
 bool PropertyTrees::operator==(const PropertyTrees& other) const {
   return transform_tree == other.transform_tree &&
@@ skipping 24 matching lines @@
       from.always_use_active_tree_opacity_effect_ids;
   clip_id_to_index_map = from.clip_id_to_index_map;
   scroll_id_to_index_map = from.scroll_id_to_index_map;
   needs_rebuild = from.needs_rebuild;
   changed = from.changed;
   full_tree_damaged = from.full_tree_damaged;
   non_root_surfaces_enabled = from.non_root_surfaces_enabled;
   sequence_number = from.sequence_number;
   is_main_thread = from.is_main_thread;
   is_active = from.is_active;
-  verify_transform_tree_calculations = from.verify_transform_tree_calculations;
   inner_viewport_container_bounds_delta_ =
       from.inner_viewport_container_bounds_delta();
   outer_viewport_container_bounds_delta_ =
       from.outer_viewport_container_bounds_delta();
   inner_viewport_scroll_bounds_delta_ =
       from.inner_viewport_scroll_bounds_delta();
   transform_tree.SetPropertyTrees(this);
   effect_tree.SetPropertyTrees(this);
   clip_tree.SetPropertyTrees(this);
   scroll_tree.SetPropertyTrees(this);
   ResetCachedData();
   return *this;
 }
 
 void PropertyTrees::ToProtobuf(proto::PropertyTrees* proto) const {
   // TODO(khushalsagar): Add support for sending diffs when serializaing
   // property trees. See crbug/555370.
   transform_tree.ToProtobuf(proto->mutable_transform_tree());
   effect_tree.ToProtobuf(proto->mutable_effect_tree());
   clip_tree.ToProtobuf(proto->mutable_clip_tree());
   scroll_tree.ToProtobuf(proto->mutable_scroll_tree());
   proto->set_needs_rebuild(needs_rebuild);
   proto->set_changed(changed);
   proto->set_full_tree_damaged(full_tree_damaged);
   proto->set_non_root_surfaces_enabled(non_root_surfaces_enabled);
   proto->set_is_main_thread(is_main_thread);
   proto->set_is_active(is_active);
-  proto->set_verify_transform_tree_calculations(
-      verify_transform_tree_calculations);
 
   // TODO(khushalsagar): Consider using the sequence number to decide if
   // property trees need to be serialized again for a commit. See crbug/555370.
   proto->set_sequence_number(sequence_number);
 
   for (auto i : always_use_active_tree_opacity_effect_ids)
     proto->add_always_use_active_tree_opacity_effect_ids(i);
 }
 
 // static
 void PropertyTrees::FromProtobuf(const proto::PropertyTrees& proto) {
   transform_tree.FromProtobuf(proto.transform_tree(),
                               &transform_id_to_index_map);
   effect_tree.FromProtobuf(proto.effect_tree(), &effect_id_to_index_map);
   clip_tree.FromProtobuf(proto.clip_tree(), &clip_id_to_index_map);
   scroll_tree.FromProtobuf(proto.scroll_tree(), &scroll_id_to_index_map);
 
   needs_rebuild = proto.needs_rebuild();
   changed = proto.changed();
   full_tree_damaged = proto.full_tree_damaged();
   non_root_surfaces_enabled = proto.non_root_surfaces_enabled();
   sequence_number = proto.sequence_number();
   is_main_thread = proto.is_main_thread();
   is_active = proto.is_active();
-  verify_transform_tree_calculations =
-      proto.verify_transform_tree_calculations();
 
   transform_tree.SetPropertyTrees(this);
   effect_tree.SetPropertyTrees(this);
   clip_tree.SetPropertyTrees(this);
   scroll_tree.SetPropertyTrees(this);
   for (auto i : proto.always_use_active_tree_opacity_effect_ids())
     always_use_active_tree_opacity_effect_ids.push_back(i);
 }
 
 void PropertyTrees::clear() {
@@ skipping 207 matching lines @@
         !node->has_only_translation_animations || ancestor_is_animating_scale;
 
     // Once we've failed to compute a maximum animated scale at an ancestor, we
     // continue to fail.
     bool failed_at_ancestor =
         ancestor_is_animating_scale && ancestor_maximum_target_scale == 0.f;
 
     // Computing maximum animated scale in the presence of non-scale/translation
     // transforms isn't supported.
     bool failed_for_non_scale_or_translation =
-        !transform_tree.Node(transform_node_id)
-             ->to_parent.IsScaleOrTranslation();
+        !node->to_parent.IsScaleOrTranslation();
 
     // We don't attempt to accumulate animation scale from multiple nodes with
     // scale animations, because of the risk of significant overestimation. For
     // example, one node might be increasing scale from 1 to 10 at the same time
     // as another node is decreasing scale from 10 to 1. Naively combining these
     // scales would produce a scale of 100.
     bool failed_for_multiple_scale_animations =
         ancestor_is_animating_scale && !node->has_only_translation_animations;
 
     if (failed_at_ancestor || failed_for_non_scale_or_translation ||
@@ skipping 84 matching lines @@
     float maximum_animation_scale,
     float starting_animation_scale) {
   cached_data_.animation_scales[transform_id]
       .combined_maximum_animation_target_scale = maximum_animation_scale;
   cached_data_.animation_scales[transform_id]
       .combined_starting_animation_scale = starting_animation_scale;
   cached_data_.animation_scales[transform_id].update_number =
       cached_data_.property_tree_update_number;
 }
 
-const DrawTransforms& PropertyTrees::GetDrawTransforms(int transform_id,
-                                                       int effect_id) const {
-  if (cached_data_.draw_transforms[effect_id][transform_id].update_number !=
-      cached_data_.property_tree_update_number) {
-    gfx::Transform target_space_transform;
-    gfx::Transform from_target;
-    const TransformNode* transform_node = transform_tree.Node(transform_id);
-    const EffectNode* effect_node = effect_tree.Node(effect_id);
-    const TransformNode* dest_node =
-        transform_tree.Node(effect_node->transform_id);
-    DCHECK(effect_id == effect_tree.kRootNodeId ||
-           effect_node->has_render_surface);
-    bool already_computed_inverse = false;
-    if (transform_id == effect_node->transform_id) {
-      target_space_transform.Scale(effect_node->surface_contents_scale.x(),
-                                   effect_node->surface_contents_scale.y());
-    } else if (!dest_node || (dest_node->ancestors_are_invertible &&
-                              dest_node->node_and_ancestors_are_flat)) {
-      // Compute transform from transform_id to effect_node->transform using
-      // screen space transforms.
-      target_space_transform.ConcatTransform(
-          transform_tree.ToScreen(transform_id));
-      if (dest_node)
-        target_space_transform.ConcatTransform(
-            transform_tree.FromScreen(dest_node->id));
-      if (dest_node->needs_surface_contents_scale)
-        target_space_transform.matrix().postScale(
-            dest_node->surface_contents_scale.x(),
-            dest_node->surface_contents_scale.y(), 1.f);
-    } else if (transform_node->id > dest_node->id) {
-      target_space_transform =
-          GetDrawTransforms(transform_node->parent_id, effect_id).to_target;
-      if (transform_node->flattens_inherited_transform)
-        target_space_transform.FlattenTo2d();
-      target_space_transform.PreconcatTransform(transform_node->to_parent);
-    } else {
-      const TransformNode* current = dest_node;
-      std::vector<int> source_to_destination;
-      source_to_destination.push_back(current->id);
-      current = transform_tree.parent(current);
-      for (; current && current->id > transform_node->id;
-           current = transform_tree.parent(current)) {
-        source_to_destination.push_back(current->id);
-      }
-      DCHECK_EQ(current, transform_node);
-      gfx::Transform combined_transform;
-      size_t source_to_destination_size = source_to_destination.size();
-      for (size_t i = 0; i < source_to_destination_size; ++i) {
-        size_t index = source_to_destination_size - 1 - i;
-        const TransformNode* node =
-            transform_tree.Node(source_to_destination[index]);
-        if (node->flattens_inherited_transform)
-          combined_transform.FlattenTo2d();
-        combined_transform.PreconcatTransform(node->to_parent);
-      }
-      if (effect_node->surface_contents_scale.x() != 0.f &&
-          effect_node->surface_contents_scale.y() != 0.f)
-        combined_transform.Scale(
-            1.0f / effect_node->surface_contents_scale.x(),
-            1.0f / effect_node->surface_contents_scale.y());
-      cached_data_.draw_transforms[effect_id][transform_id]
-          .transforms.invertible =
-          combined_transform.GetInverse(&target_space_transform);
-      from_target = combined_transform;
-      already_computed_inverse = true;
+bool PropertyTrees::GetToTarget(int transform_id,
+                                int effect_id,
+                                gfx::Transform* to_target) const {
+  DrawTransforms& transforms = GetDrawTransforms(transform_id, effect_id);
+  if (transforms.to_valid) {
+    *to_target = transforms.to_target;
+    return true;
+  } else if (!transforms.invertible) {
+    return false;
+  } else {
+    transforms.invertible = transforms.from_target.GetInverse(to_target);
+    transforms.to_valid = transforms.invertible;
+    transforms.to_target = *to_target;
+    return transforms.to_valid;
+  }
+}
+
+bool PropertyTrees::GetFromTarget(int transform_id,
+                                  int effect_id,
+                                  gfx::Transform* from_target) const {
+  DrawTransforms& transforms = GetDrawTransforms(transform_id, effect_id);
+  if (transforms.from_valid) {
+    *from_target = transforms.from_target;
+    return true;
+  } else if (!transforms.invertible) {
+    return false;
+  } else {
+    transforms.invertible = transforms.to_target.GetInverse(from_target);
+    transforms.from_valid = transforms.invertible;
+    transforms.from_target = *from_target;
+    return transforms.from_valid;
+  }
+}
+
+DrawTransformData& PropertyTrees::FetchDrawTransformsDataFromCache(
+    int transform_id,
+    int dest_id) const {
+  int cnt = 0;
+  for (const auto& transform_data :
+       cached_data_.draw_transforms[transform_id]) {
+    if (transform_data.target_id == dest_id || transform_data.target_id == -1) {

[ajuma, 2016/10/12 20:08:47]

Which cases is the "-1" check supposed to handle?

[sunxd, 2016/10/12 21:08:19]

In PropertyTrees::ResetCachedData, we initialize draw_transforms with 1 element.
The first time when we access the cache, we would hit a -1 target_id.

[ajuma, 2016/10/12 21:37:41]

Ah! Please add a comment about this.

+      return cached_data_.draw_transforms[transform_id][cnt];

[ajuma, 2016/10/12 20:08:47]

Would it work to return |transform_data| here (so no need for the counter
variable)?

[sunxd, 2016/10/12 21:08:19]

Done.

     }
-    if (!already_computed_inverse) {
-      cached_data_.draw_transforms[effect_id][transform_id]
-          .transforms.invertible =
-          target_space_transform.GetInverse(&from_target);
-    }
-    cached_data_.draw_transforms[effect_id][transform_id].update_number =
-        cached_data_.property_tree_update_number;
-    cached_data_.draw_transforms[effect_id][transform_id]
-        .transforms.from_target = from_target;
-    cached_data_.draw_transforms[effect_id][transform_id].transforms.to_target =
-        target_space_transform;
+    cnt++;
   }
-  return cached_data_.draw_transforms[effect_id][transform_id].transforms;
+  // Add an entry to the cache.
+  cached_data_.draw_transforms[transform_id].push_back(DrawTransformData());
+  DrawTransformData& data = cached_data_.draw_transforms[transform_id][cnt];
+  data.update_number = -1;
+  data.target_id = dest_id;
+  return data;
+}
+
+DrawTransforms& PropertyTrees::GetDrawTransforms(int transform_id,
+                                                 int effect_id) const {
+  const EffectNode* effect_node = effect_tree.Node(effect_id);
+  int dest_id = effect_node->transform_id;
+
+  DrawTransformData& data =
+      FetchDrawTransformsDataFromCache(transform_id, dest_id);
+
+  DCHECK(data.update_number != cached_data_.property_tree_update_number ||
+         data.target_id != -1);

[ajuma, 2016/10/12 20:08:47]

In what situations do we have target_id -1 (and why does that imply that the
update numbers don't match)?

[sunxd, 2016/10/12 21:08:19]

As stated in the comments in Fetch function, it is -1 when we compute for the
first time after a cache reset.

+  if (data.update_number == cached_data_.property_tree_update_number)
+    return data.transforms;
+
+  // Cache miss.
+  gfx::Transform target_space_transform;
+  gfx::Transform from_target;
+  bool already_computed_inverse = false;
+  if (transform_id == dest_id) {
+    target_space_transform.Scale(effect_node->surface_contents_scale.x(),
+                                 effect_node->surface_contents_scale.y());
+    data.transforms.to_valid = true;
+    data.transforms.from_valid = false;
+  } else if (transform_id > dest_id) {
+    transform_tree.CombineTransformsBetween(transform_id, dest_id,
+                                            &target_space_transform);
+    if (dest_id != TransformTree::kRootNodeId)

[ajuma, 2016/10/12 20:08:47]

https://codereview.chromium.org/2408243002/ might affect this logic

[sunxd, 2016/10/12 21:08:19]

Is it safe that we do not apply root surface contents scale as in that CL the
scale factor is stored in the transform root.

+      target_space_transform.matrix().postScale(
+          effect_node->surface_contents_scale.x(),
+          effect_node->surface_contents_scale.y(), 1.f);
+    data.transforms.to_valid = true;
+    data.transforms.from_valid = false;
+    data.transforms.invertible = true;
+  } else {
+    gfx::Transform combined_transform;
+    transform_tree.CombineTransformsBetween(dest_id, transform_id,
+                                            &combined_transform);
+    if (effect_node->surface_contents_scale.x() != 0.f &&
+        effect_node->surface_contents_scale.y() != 0.f)
+      combined_transform.Scale(1.0f / effect_node->surface_contents_scale.x(),
+                               1.0f / effect_node->surface_contents_scale.y());
+    bool invertible = combined_transform.GetInverse(&target_space_transform);
+    data.transforms.invertible = invertible;
+    data.transforms.to_valid = invertible;
+    data.transforms.from_valid = true;
+    from_target = combined_transform;
+    already_computed_inverse = true;
+  }
+
+  if (!already_computed_inverse)
+    data.transforms.to_valid = true;
+  data.update_number = cached_data_.property_tree_update_number;
+  data.target_id = dest_id;
+  data.transforms.from_target = from_target;
+  data.transforms.to_target = target_space_transform;
+  return data.transforms;
 }
 
 void PropertyTrees::ResetCachedData() {
   cached_data_.property_tree_update_number = 0;
   cached_data_.animation_scales = std::vector<AnimationScaleData>(
       transform_tree.nodes().size(), AnimationScaleData());
-  cached_data_.draw_transforms =
-      std::vector<std::unordered_map<int, DrawTransformData>>(
-          effect_tree.nodes().size(),
-          std::unordered_map<int, DrawTransformData>());
+  cached_data_.draw_transforms = std::vector<std::vector<DrawTransformData>>(
+      transform_tree.nodes().size(), std::vector<DrawTransformData>(1));
 }
 
 void PropertyTrees::UpdateCachedNumber() {
   cached_data_.property_tree_update_number++;
 }
 
 gfx::Transform PropertyTrees::ToScreenSpaceTransformWithoutSurfaceContentsScale(
     int transform_id,
     int effect_id) const {
   DCHECK_GT(transform_id, 0);
   if (transform_id == 1) {
     return gfx::Transform();
   }
   gfx::Transform screen_space_transform = transform_tree.ToScreen(transform_id);
   const EffectNode* effect_node = effect_tree.Node(effect_id);
 
   if (effect_node->surface_contents_scale.x() != 0.0 &&
       effect_node->surface_contents_scale.y() != 0.0)
     screen_space_transform.Scale(1.0 / effect_node->surface_contents_scale.x(),
                                  1.0 / effect_node->surface_contents_scale.y());
   return screen_space_transform;
 }
 
 bool PropertyTrees::ComputeTransformToTarget(int transform_id,
                                              int effect_id,
                                              gfx::Transform* transform) const {
   transform->MakeIdentity();
-
   if (transform_id == TransformTree::kInvalidNodeId)
     return true;
 
-  int target_transform_id;
   const EffectNode* effect_node = effect_tree.Node(effect_id);
-  if (effect_id == EffectTree::kInvalidNodeId) {
-    // This can happen when PaintArtifactCompositor builds property trees as
-    // it doesn't set effect ids on clip nodes. We want to compute transform
-    // to the root in this case.
-    target_transform_id = TransformTree::kRootNodeId;
-  } else {
-    DCHECK(effect_node->has_render_surface ||
-           effect_node->id == EffectTree::kRootNodeId);
-    target_transform_id = effect_node->transform_id;
-  }
 
-  bool success = transform_tree.ComputeTransform(
-      transform_id, target_transform_id, transform);
-  if (verify_transform_tree_calculations) {
-    gfx::Transform to_target;
-    to_target.ConcatTransform(
-        GetDrawTransforms(transform_id, effect_id).to_target);
-    if (effect_node->surface_contents_scale.x() != 0.f &&
-        effect_node->surface_contents_scale.y() != 0.f)
-      to_target.matrix().postScale(
-          1.0f / effect_node->surface_contents_scale.x(),
-          1.0f / effect_node->surface_contents_scale.y(), 1.0f);
-    DCHECK(to_target.ApproximatelyEqual(*transform));
-  }
+  bool success = GetToTarget(transform_id, effect_id, transform);
+  if (effect_node->surface_contents_scale.x() != 0.f &&
+      effect_node->surface_contents_scale.y() != 0.f)
+    transform->matrix().postScale(
+        1.0f / effect_node->surface_contents_scale.x(),
+        1.0f / effect_node->surface_contents_scale.y(), 1.0f);
   return success;
 }
 
 bool PropertyTrees::ComputeTransformFromTarget(
     int transform_id,
     int effect_id,
     gfx::Transform* transform) const {
   transform->MakeIdentity();
-
   if (transform_id == TransformTree::kInvalidNodeId)
     return true;
 
-  int target_transform_id;
   const EffectNode* effect_node = effect_tree.Node(effect_id);
-  if (effect_id == EffectTree::kInvalidNodeId) {
-    // This can happen when PaintArtifactCompositor builds property trees as
-    // it doesn't set effect ids on clip nodes. We want to compute transform
-    // to the root in this case.
-    target_transform_id = TransformTree::kRootNodeId;
-  } else {
-    DCHECK(effect_node->has_render_surface ||
-           effect_node->id == EffectTree::kRootNodeId);
-    target_transform_id = effect_node->transform_id;
-  }
 
-  bool success = transform_tree.ComputeTransform(target_transform_id,
-                                                 transform_id, transform);
-  if (verify_transform_tree_calculations) {
-    auto draw_transforms = GetDrawTransforms(transform_id, effect_id);
-    gfx::Transform from_target;
-    from_target.ConcatTransform(draw_transforms.from_target);
-    from_target.Scale(effect_node->surface_contents_scale.x(),
-                      effect_node->surface_contents_scale.y());
-    DCHECK(from_target.ApproximatelyEqual(*transform) ||
-           !draw_transforms.invertible);
-  }
+  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