Only allow change-of-basis between transform nodes on the same ancestor chain

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 <set>
 #include <vector>
 
 #include "base/logging.h"
 #include "cc/base/math_util.h"
 #include "cc/trees/property_tree.h"
@@ skipping 43 matching lines @@
 TransformNodeData::~TransformNodeData() {
 }
 
 ClipNodeData::ClipNodeData() : transform_id(-1), target_id(-1) {
 }
 
 bool TransformTree::ComputeTransform(int source_id,
                                      int dest_id,
                                      gfx::Transform* transform) const {
   transform->MakeIdentity();
-

[ajuma, 2015/04/09 14:43:09]

Nit: this whitespace should stay

[jaydasika, 2015/04/09 15:24:29]

Done.

   if (source_id == dest_id)
     return true;
 
   if (source_id > dest_id && IsDescendant(source_id, dest_id))

[ajuma, 2015/04/09 14:43:09]

Since we don't expect the IsDescendant check to ever return false now, it can be
changed to a DCHECK. Same thing below too.

If we're doing that, the 'if' below can go away (since if we reach there, it
must be the case that dest_id > source_id), and we can unconditionally
DCHECK(IsDescendant(dest_id, source_id) and then "return CombineInverses...".

[jaydasika, 2015/04/09 15:24:29]

Done.

     return CombineTransformsBetween(source_id, dest_id, transform);
 
   if (dest_id > source_id && IsDescendant(dest_id, source_id))
     return CombineInversesBetween(source_id, dest_id, transform);
 
-  int lca = LowestCommonAncestor(source_id, dest_id);
-
-  bool no_singular_matrices_to_lca =
-      CombineTransformsBetween(source_id, lca, transform);
-
-  bool no_singular_matrices_from_lca =
-      CombineInversesBetween(lca, dest_id, transform);
-
-  return no_singular_matrices_to_lca && no_singular_matrices_from_lca;
+  NOTREACHED();
+  return 0;

[ajuma, 2015/04/09 14:43:09]

Style nit: use 'false' instead of 0. (But this code will go away with the
changes suggested above.)

[jaydasika, 2015/04/09 15:24:29]

Done.

 }
 
 bool TransformTree::Are2DAxisAligned(int source_id, int dest_id) const {
   gfx::Transform transform;
   return ComputeTransform(source_id, dest_id, &transform) &&
          transform.Preserves2dAxisAlignment();
 }
 
 void TransformTree::UpdateTransforms(int id) {
   TransformNode* node = Node(id);
@@ skipping 10 matching lines @@
 
 bool TransformTree::IsDescendant(int desc_id, int source_id) const {
   while (desc_id != source_id) {
     if (desc_id < 0)
       return false;
     desc_id = Node(desc_id)->parent_id;
   }
   return true;
 }
 
-int TransformTree::LowestCommonAncestor(int a, int b) const {
-  std::set<int> chain_a;
-  std::set<int> chain_b;
-  while (a || b) {
-    if (a) {
-      a = Node(a)->parent_id;
-      if (a > -1 && chain_b.find(a) != chain_b.end())
-        return a;
-      chain_a.insert(a);
-    }
-    if (b) {
-      b = Node(b)->parent_id;
-      if (b > -1 && chain_a.find(b) != chain_a.end())
-        return b;
-      chain_b.insert(b);
-    }
-  }
-  NOTREACHED();
-  return 0;
-}
-
 bool TransformTree::CombineTransformsBetween(int source_id,
                                              int dest_id,
                                              gfx::Transform* transform) const {
   const TransformNode* current = Node(source_id);
   const TransformNode* dest = Node(dest_id);
   // Combine transforms to and from the screen when possible. Since flattening
   // is a non-linear operation, we cannot use this approach when there is
   // non-trivial flattening between the source and destination nodes. For
   // example, consider the tree R->A->B->C, where B flattens its inherited
   // transform, and A has a non-flat transform. Suppose C is the source and A is
@@ skipping 180 matching lines @@
   node->data.from_target.matrix().postTranslate(-translation.x(),
                                                 -translation.y(), 0);
   node->data.to_screen.Translate(translation.x(), translation.y());
   node->data.from_screen.matrix().postTranslate(-translation.x(),
                                                 -translation.y(), 0);
 
   node->data.scroll_snap = translation;
 }
 
 }  // namespace cc