[BGPT] Optimize animation element id collection

Prevously we collected animation element ids after updating cc paint
properties, and traversed the parent chain until we find a node whose
element id had been collected. This might repeatedly traverse many
nodes without element ids unnecessarily.

Now collect animation element ids just after creating cc property
node to avoid the repeated traversal.

Bug: 954520
Change-Id: I923b610766d04d80041b8ddf46cc9828621d9237
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1598328
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#657450}

third_party/blink/renderer/core/frame/local_frame_view.cc

@@ -265,7 +265,6 @@ LocalFrameView::LocalFrameView(LocalFrame& frame, IntRect frame_rect)
           base::WrapUnique(g_initial_track_all_paint_invalidations
                                ? new Vector<ObjectPaintInvalidation>
                                : nullptr)),
-      composited_element_ids_(CompositorElementIdSet()),
       main_thread_scrolling_reasons_(0),
       forced_layout_stack_depth_(0),
       forced_layout_start_time_(TimeTicks()),
@@ -2465,16 +2464,14 @@ void LocalFrameView::RunPaintLifecyclePhase() {
   if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled() ||
       RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled()) {
     if (!print_mode_enabled) {
-      auto& composited_element_ids = composited_element_ids_;
       bool needed_update = !paint_artifact_compositor_ ||
                            paint_artifact_compositor_->NeedsUpdate();
-      PushPaintArtifactToCompositor(composited_element_ids.value());
-      ForAllNonThrottledLocalFrameViews(
-          [&composited_element_ids](LocalFrameView& frame_view) {
-            DocumentAnimations::UpdateAnimations(
-                frame_view.GetLayoutView()->GetDocument(),
-                DocumentLifecycle::kPaintClean, composited_element_ids);
-          });
+      PushPaintArtifactToCompositor();
+      ForAllNonThrottledLocalFrameViews([this](LocalFrameView& frame_view) {
+        DocumentAnimations::UpdateAnimations(
+            frame_view.GetLayoutView()->GetDocument(),
+            DocumentLifecycle::kPaintClean, animation_element_ids_);
+      });
 
       // Initialize animation properties in the newly created paint property
       // nodes according to the current animation state. This is mainly for
@@ -2730,8 +2727,7 @@ const cc::Layer* LocalFrameView::RootCcLayer() const {
   return nullptr;
 }
 
-void LocalFrameView::PushPaintArtifactToCompositor(
-    CompositorElementIdSet& composited_element_ids) {
+void LocalFrameView::PushPaintArtifactToCompositor() {
   TRACE_EVENT0("blink", "LocalFrameView::pushPaintArtifactToCompositor");
 
   DCHECK(RuntimeEnabledFeatures::CompositeAfterPaintEnabled() ||
@@ -2811,7 +2807,7 @@ void LocalFrameView::PushPaintArtifactToCompositor(
   }
 
   paint_artifact_compositor_->Update(
-      paint_controller_->GetPaintArtifactShared(), composited_element_ids,
+      paint_controller_->GetPaintArtifactShared(), animation_element_ids_,
       viewport_properties, settings);
 
   probe::LayerTreePainted(&GetFrame());

third_party/blink/renderer/core/frame/local_frame_view.h

@@ -797,8 +797,7 @@ class CORE_EXPORT LocalFrameView final
   void PaintTree();
   void UpdateStyleAndLayoutIfNeededRecursive();
 
-  void PushPaintArtifactToCompositor(
-      CompositorElementIdSet& composited_element_ids);
+  void PushPaintArtifactToCompositor();
 
   void ClearLayoutSubtreeRootsAndMarkContainingBlocks();
 
@@ -994,12 +993,13 @@ class CORE_EXPORT LocalFrameView final
   std::unique_ptr<PaintController> paint_controller_;
   std::unique_ptr<PaintArtifactCompositor> paint_artifact_compositor_;
 
-  // The set of ElementIds that were composited by PaintArtifactCompositor
-  // during the Paint lifecycle phase. Only used by BlinkGenPropertyTrees and
-  // CompositeAfterPaint. These are stored here because sometimes
-  // PaintArtifactCompositor::Update() does not run (if the dirty bit is not
-  // set) and in that case, the element ids from the prior run are retained.
-  base::Optional<CompositorElementIdSet> composited_element_ids_;
+  // The set of ElementIds that were composited for animation by
+  // PaintArtifactCompositor during the Paint lifecycle phase. Only used by
+  // BlinkGenPropertyTrees and CompositeAfterPaint. These are stored here
+  // because sometimes PaintArtifactCompositor::Update() does not run (if the
+  // dirty bit is not set) and in that case, the element ids from the prior run
+  // are retained.
+  CompositorElementIdSet animation_element_ids_;
 
   MainThreadScrollingReasons main_thread_scrolling_reasons_;
 

third_party/blink/renderer/platform/graphics/compositing/paint_artifact_compositor.cc

@@ -34,42 +34,6 @@
 
 namespace blink {
 
-namespace {
-
-// Returns true if the given element namespace is one of the ones needed for
-// running animations on the compositor. These are the only element_ids the
-// compositor needs to track existence of in the element id set.
-bool IsAnimationNamespace(CompositorElementIdNamespace element_namespace) {
-  return element_namespace == CompositorElementIdNamespace::kPrimaryTransform ||
-         element_namespace == CompositorElementIdNamespace::kPrimaryEffect ||
-         element_namespace == CompositorElementIdNamespace::kEffectFilter;
-}
-
-// Inserts the element ids of the given node and all of its ancestors into the
-// given |composited_element_ids| set. Filters out specifically element ids
-// which are needed for animations. Returns once it finds an id which already
-// exists as this implies that all of those ancestor nodes have already been
-// inserted.
-template <typename NodeType>
-void InsertAncestorElementIdsForAnimation(
-    const NodeType& node,
-    CompositorElementIdSet& composited_element_ids) {
-  for (const auto* n = &node; n; n = SafeUnalias(n->Parent())) {
-    const CompositorElementId& element_id = n->GetCompositorElementId();
-    if (element_id && n->RequiresCompositingForAnimation() &&
-        IsAnimationNamespace(NamespaceFromCompositorElementId(element_id))) {
-      if (composited_element_ids.count(element_id)) {
-        // Once we reach a node already counted we can stop traversing the
-        // parent chain.
-        return;
-      }
-      composited_element_ids.insert(element_id);
-    }
-  }
-}
-
-}  // namespace
-
 // cc property trees make use of a sequence number to identify when tree
 // topology changes. For now we naively increment the sequence number each time
 // we update the property trees. We should explore optimizing our management of
@@ -886,7 +850,7 @@ void PaintArtifactCompositor::DecompositeTransforms() {
 
 void PaintArtifactCompositor::Update(
     scoped_refptr<const PaintArtifact> paint_artifact,
-    CompositorElementIdSet& composited_element_ids,
+    CompositorElementIdSet& animation_element_ids,
     const ViewportProperties& viewport_properties,
     const Settings& settings) {
   DCHECK(NeedsUpdate());
@@ -913,9 +877,9 @@ void PaintArtifactCompositor::Update(
       g_s_property_tree_sequence_number);
 
   LayerListBuilder layer_list_builder;
-  PropertyTreeManager property_tree_manager(*this, *host->property_trees(),
-                                            *root_layer_, layer_list_builder,
-                                            g_s_property_tree_sequence_number);
+  PropertyTreeManager property_tree_manager(
+      *this, *host->property_trees(), *root_layer_, layer_list_builder,
+      animation_element_ids, g_s_property_tree_sequence_number);
   CollectPendingLayers(*paint_artifact, settings);
 
   UpdateCompositorViewportProperties(viewport_properties, property_tree_manager,
@@ -936,7 +900,7 @@ void PaintArtifactCompositor::Update(
   DecompositeTransforms();
 
   // Clear prior frame ids before inserting new ones.
-  composited_element_ids.clear();
+  animation_element_ids.clear();
   for (auto& pending_layer : pending_layers_) {
     const auto& property_state = pending_layer.property_tree_state;
     const auto& transform = property_state.Transform();
@@ -998,11 +962,8 @@ void PaintArtifactCompositor::Update(
         pending_layer.FirstPaintChunk(*paint_artifact).id.client.OwnerNodeId());
     // TODO(wangxianzhu): cc_picture_layer_->set_compositing_reasons(...);
 
-    InsertAncestorElementIdsForAnimation(property_state.Effect(),
-                                         composited_element_ids);
-    InsertAncestorElementIdsForAnimation(transform, composited_element_ids);
     if (layer->scrollable())
-      composited_element_ids.insert(layer->element_id());
+      animation_element_ids.insert(layer->element_id());
 
     // If the property tree state has changed between the layer and the root, we
     // need to inform the compositor so damage can be calculated.
@@ -1040,7 +1001,7 @@ void PaintArtifactCompositor::Update(
   root_layer_->SetChildLayerList(std::move(layers));
 
   // Update the host's active registered element ids.
-  host->SetActiveRegisteredElementIds(composited_element_ids);
+  host->SetActiveRegisteredElementIds(animation_element_ids);
 
   // Mark the property trees as having been rebuilt.
   host->property_trees()->needs_rebuild = false;

third_party/blink/renderer/platform/graphics/compositing/paint_artifact_compositor.h

@@ -131,10 +131,10 @@ class PLATFORM_EXPORT PaintArtifactCompositor final
 
   // Updates the layer tree to match the provided paint artifact.
   //
-  // Populates |composited_element_ids| with the CompositorElementId of all
+  // Populates |animation_element_ids| with the CompositorElementId of all
   // animations for which we saw a paint chunk and created a layer.
   void Update(scoped_refptr<const PaintArtifact>,
-              CompositorElementIdSet& composited_element_ids,
+              CompositorElementIdSet& animation_element_ids,
               const ViewportProperties& viewport_properties,
               const Settings& settings);
 

third_party/blink/renderer/platform/graphics/compositing/property_tree_manager.cc

@@ -39,15 +39,18 @@ PropertyTreeManager::EffectState::Transform() const {
                                               : clip->LocalTransformSpace();
 }
 
-PropertyTreeManager::PropertyTreeManager(PropertyTreeManagerClient& client,
-                                         cc::PropertyTrees& property_trees,
-                                         cc::Layer& root_layer,
-                                         LayerListBuilder& layer_list_builder,
-                                         int new_sequence_number)
+PropertyTreeManager::PropertyTreeManager(
+    PropertyTreeManagerClient& client,
+    cc::PropertyTrees& property_trees,
+    cc::Layer& root_layer,
+    LayerListBuilder& layer_list_builder,
+    CompositorElementIdSet& animation_element_ids,
+    int new_sequence_number)
     : client_(client),
       property_trees_(property_trees),
       root_layer_(root_layer),
       layer_list_builder_(layer_list_builder),
+      animation_element_ids_(animation_element_ids),
       new_sequence_number_(new_sequence_number) {
   SetupRootTransformNode();
   SetupRootClipNode();
@@ -459,6 +462,9 @@ int PropertyTreeManager::EnsureCompositorTransformNode(
     property_trees_.element_id_to_transform_node_index[compositor_element_id] =
         id;
     compositor_node.element_id = compositor_element_id;
+
+    if (transform_node.RequiresCompositingForAnimation())
+      CollectAnimationElementId(compositor_element_id);
   }
 
   // If this transform is a scroll offset translation, create the associated
@@ -934,6 +940,20 @@ SkBlendMode PropertyTreeManager::SynthesizeCcEffectsForClipsIfNeeded(
   return delegated_blend;
 }
 
+void PropertyTreeManager::CollectAnimationElementId(
+    CompositorElementId element_id) {
+  // Collect the element id if the namespace is one of the ones needed for
+  // running animations on the compositor. These are the only element_ids the
+  // compositor needs to track existence of in the element id set.
+  auto element_namespace = NamespaceFromCompositorElementId(element_id);
+  if (element_namespace == CompositorElementIdNamespace::kPrimaryTransform ||
+      element_namespace == CompositorElementIdNamespace::kPrimaryEffect ||
+      element_namespace == CompositorElementIdNamespace::kEffectFilter) {
+    DCHECK_EQ(0u, animation_element_ids_.count(element_id));
+    animation_element_ids_.insert(element_id);
+  }
+}
+
 void PropertyTreeManager::BuildEffectNodesRecursively(
     const EffectPaintPropertyNode& next_effect_arg) {
   const auto& next_effect = next_effect_arg.Unalias();
@@ -978,6 +998,9 @@ void PropertyTreeManager::BuildEffectNodesRecursively(
         compositor_element_id));
     property_trees_.element_id_to_effect_node_index[compositor_element_id] =
         effect_node.id;
+
+    if (next_effect.RequiresCompositingForAnimation())
+      CollectAnimationElementId(compositor_element_id);
   }
 
   effect_stack_.emplace_back(current_);

third_party/blink/renderer/platform/graphics/compositing/property_tree_manager.h

@@ -53,6 +53,7 @@ class PropertyTreeManager {
                       cc::PropertyTrees& property_trees,
                       cc::Layer& root_layer,
                       LayerListBuilder& layer_list_builder,
+                      CompositorElementIdSet& animation_element_ids,
                       int new_sequence_number);
   ~PropertyTreeManager();
 
@@ -198,6 +199,7 @@ class PropertyTreeManager {
     const TransformPaintPropertyNode& Transform() const;
   };
 
+  void CollectAnimationElementId(CompositorElementId);
   void BuildEffectNodesRecursively(const EffectPaintPropertyNode& next_effect);
   void ForceRenderSurfaceIfSyntheticRoundedCornerClip(EffectState& state);
   SkBlendMode SynthesizeCcEffectsForClipsIfNeeded(
@@ -253,6 +255,7 @@ class PropertyTreeManager {
   cc::Layer& root_layer_;
 
   LayerListBuilder& layer_list_builder_;
+  CompositorElementIdSet& animation_element_ids_;
 
   int new_sequence_number_;