cc: Use Element Id to Record Animation Changes

cc/trees/layer_tree_host_impl.cc

Index: cc/trees/layer_tree_host_impl.cc
diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
index ac4fd718574ea556dce7f456e7e726f38106bdec..67621146d7a271ecce9fc86be83b85fb2a749cde 100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -73,12 +73,14 @@
 #include "cc/tiles/software_image_decode_cache.h"
 #include "cc/trees/damage_tracker.h"
 #include "cc/trees/draw_property_utils.h"
+#include "cc/trees/effect_node.h"
 #include "cc/trees/latency_info_swap_promise_monitor.h"
 #include "cc/trees/layer_tree_host_common.h"
 #include "cc/trees/layer_tree_impl.h"
 #include "cc/trees/mutator_host.h"
 #include "cc/trees/scroll_node.h"
 #include "cc/trees/single_thread_proxy.h"
+#include "cc/trees/transform_node.h"
 #include "cc/trees/tree_synchronizer.h"
 #include "gpu/GLES2/gl2extchromium.h"
 #include "gpu/command_buffer/client/context_support.h"
@@ -4120,9 +4122,67 @@ void LayerTreeHostImpl::ElementIsAnimatingChanged(
       list_type == ElementListType::ACTIVE ? active_tree() : pending_tree();
   if (!tree)
     return;
-  LayerImpl* layer = tree->LayerByElementId(element_id);
-  if (layer)
-    layer->OnIsAnimatingChanged(mask, state);
+  PropertyTrees* property_trees = tree->property_trees();
+
+  for (int property = TargetProperty::FIRST_TARGET_PROPERTY;

[wkorman, 2017/03/20 21:59:57]

Much of this is near identical to the same in layer_tree_host.cc, can we share
code somehow? I see there are some differences for transform in particular (like
set_was_ever_ready..., and tree->set_needs_update_draw_properties). But bulk is
the same.

Even moving to three separate utility routines one for each case that live
somewhere affiliated with TargetProperty, PropertyTree, Animation, and we just
call them in each case here, then do impl-specific stuff remaining. Would make
maintaining easier.

We could create animation_property_utils.{h,cc} or somesuch. Just a thought.

[weiliangc, 2017/03/21 15:18:52]

We could create template function in utils class that is called here? Do you
think it makes sense to put in same CL or do another CL for that?

[wkorman, 2017/03/21 19:44:23]

Either ok IMO, could add a comment here and in other file to note the code
duplication and intent to move to be shared.

ajuma@ may have better idea on how to share the code, I also thought perhaps
layer_tree_host_common but that looks very reductionist/limited. But maybe
that's what it's for?

[ajuma, 2017/03/21 19:53:28]

Perhaps draw_property_utils or property_tree might be good places for this.
layer_tree_host_common was where the old CalcDrawProps used to live; recently
we've mostly been removing stuff from there rather than adding new things to it.

Doing this in another CL (and leaving a comment for now) sgtm.

+       property <= TargetProperty::LAST_TARGET_PROPERTY; ++property) {
+    if (!mask.currently_running[property] &&
+        !mask.potentially_animating[property])
+      continue;
+
+    switch (property) {
+      case TargetProperty::TRANSFORM:
+        if (TransformNode* transform_node =
+                property_trees->transform_tree.FindNodeFromElementId(
+                    element_id)) {
+          if (mask.currently_running[property])
+            transform_node->is_currently_animating =
+                state.currently_running[property];
+          if (mask.potentially_animating[property]) {
+            transform_node->has_potential_animation =
+                state.potentially_animating[property];
+            transform_node->has_only_translation_animations =
+                mutator_host()->HasOnlyTranslationTransforms(element_id,
+                                                             list_type);
+            property_trees->transform_tree.set_needs_update(true);
+            tree->set_needs_update_draw_properties();
+            // TODO(crbug.com/702777):
+            // was_ever_ready_since_last_transform_animation should not live on
+            // layers.
+            if (LayerImpl* layer = tree->LayerByElementId(element_id)) {
+              layer->set_was_ever_ready_since_last_transform_animation(false);
+            }
+          }
+        }
+        break;
+      case TargetProperty::OPACITY:
+        if (EffectNode* effect_node =
+                property_trees->effect_tree.FindNodeFromElementId(element_id)) {
+          if (mask.currently_running[property])
+            effect_node->is_currently_animating_opacity =
+                state.currently_running[property];
+          if (mask.potentially_animating[property]) {
+            effect_node->has_potential_opacity_animation =
+                state.potentially_animating[property];
+            property_trees->effect_tree.set_needs_update(true);
+          }
+        }
+        break;
+      case TargetProperty::FILTER:
+        if (EffectNode* effect_node =
+                property_trees->effect_tree.FindNodeFromElementId(element_id)) {
+          if (mask.currently_running[property])
+            effect_node->is_currently_animating_filter =
+                state.currently_running[property];
+          if (mask.potentially_animating[property])
+            effect_node->has_potential_filter_animation =
+                state.potentially_animating[property];

[wkorman, 2017/03/20 21:59:57]

Same comment as in layer_tree_host.cc, no set_needs_update(true) here, is that
intended?

[weiliangc, 2017/03/21 15:18:52]

It is because for opacity animation and transform animation the subtree could
care about whether they are inside the animation, but for filter animation the
change is only limited to the node. Should I add comment?

[wkorman, 2017/03/21 19:44:23]

Yes, I think worth adding a comment since I wondered.

+        }
+        break;
+      default:
+        break;
+    }
+  }
 }
 
 void LayerTreeHostImpl::ScrollOffsetAnimationFinished() {