cc : Add node and ancestors are animated or invertible to transform tree

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 de0096bca4ef9a192051c29a941135273c593540..00ede3ac0f99f75eec44f3f03aa9f0f2bebcbbd3 100644
--- a/cc/trees/property_tree.cc
+++ b/cc/trees/property_tree.cc
@@ -136,6 +136,7 @@ TransformNodeData::TransformNodeData()
       source_node_id(-1),
       sorting_context_id(0),
       needs_local_transform_update(true),
+      node_and_ancestors_are_animated_or_invertible(true),
       is_invertible(true),
       ancestors_are_invertible(true),
       is_animated(false),
@@ -174,6 +175,8 @@ bool TransformNodeData::operator==(const TransformNodeData& other) const {
          source_node_id == other.source_node_id &&
          sorting_context_id == other.sorting_context_id &&
          needs_local_transform_update == other.needs_local_transform_update &&
+         node_and_ancestors_are_animated_or_invertible ==
+             other.node_and_ancestors_are_animated_or_invertible &&
          is_invertible == other.is_invertible &&
          ancestors_are_invertible == other.ancestors_are_invertible &&
          is_animated == other.is_animated &&
@@ -256,6 +259,9 @@ void TransformNodeData::ToProtobuf(proto::TreeNode* proto) const {
 
   data->set_needs_local_transform_update(needs_local_transform_update);
 
+  data->set_node_and_ancestors_are_animated_or_invertible(
+      node_and_ancestors_are_animated_or_invertible);
+
   data->set_is_invertible(is_invertible);
   data->set_ancestors_are_invertible(ancestors_are_invertible);
 
@@ -323,6 +329,9 @@ void TransformNodeData::FromProtobuf(const proto::TreeNode& proto) {
 
   needs_local_transform_update = data.needs_local_transform_update();
 
+  node_and_ancestors_are_animated_or_invertible =
+      data.node_and_ancestors_are_animated_or_invertible();
+
   is_invertible = data.is_invertible();
   ancestors_are_invertible = data.ancestors_are_invertible();
 
@@ -690,6 +699,7 @@ void TransformTree::UpdateTransforms(int id) {
   UpdateSnapping(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 {
@@ -1062,6 +1072,28 @@ void TransformTree::UpdateTransformChanged(TransformNode* node,
     node->data.transform_changed = true;
 }
 
+void TransformTree::UpdateNodeAndAncestorsAreAnimatedOrInvertible(
+    TransformNode* node,
+    TransformNode* parent_node) {
+  if (!parent_node) {
+    DCHECK(node->data.node_and_ancestors_are_animated_or_invertible);

[jaydasika, 2016/04/11 20:42:38]

I just realized, this shouldn't be a DCHECK, we should be checking the node's
transform.

+    return;
+  }
+  if (!parent_node->data.node_and_ancestors_are_animated_or_invertible) {
+    node->data.node_and_ancestors_are_animated_or_invertible = false;
+    return;
+  }
+  bool is_invertible = node->data.is_invertible;
+  // Even when the current node's transform and the parent's screen space
+  // transform are invertible, the current node's screen space transform can
+  // become uninvertible due to floating-point arithmetic.
+  if (!node->data.ancestors_are_invertible &&
+      parent_node->data.ancestors_are_invertible)

[ajuma, 2016/04/11 20:21:44]

Is it sufficient to just use the value of node->data.ancestors_are_invertible
here (which, despite its name, really is about the invertibility of the node as
well as its ancestors)? That is, this is true iff both the screen space
transform and draw transform are invertible. So we don't need to additionally
check node->data.is_invertible and parent_node->data.ancestors_are_invertible.

[jaydasika, 2016/04/11 20:31:28]

For the case, A->B, A has animating singular transform, B's
ancestors_are_invertible will be false, but we want
node_and_ancestors_are_animated_or_invertible to be true for B 

[ajuma, 2016/04/11 20:35:58]

Ah, good point, thanks. If we don't have a test case with this structure, please
add one.

[jaydasika, 2016/04/11 20:42:38]

There's a test already :
LayerTreeHostCommonTest.VisibleContentRectForAnimatedLayerWithSingularTransform 

+    is_invertible = false;
+  node->data.node_and_ancestors_are_animated_or_invertible =
+      node->data.is_animated || is_invertible;
+}
+
 void TransformTree::SetDeviceTransform(const gfx::Transform& transform,
                                        gfx::PointF root_position) {
   gfx::Transform root_post_local = transform;