FOR DISCUSSION: Implement indirect compositing reasons for effect nodes.

third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp

Index: third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp b/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
index 0e38a13cb0112b9985629e5e04c18b35d62462a5..549d995dc56443517c2ee295c2d90c5b3c8e0878 100644
--- a/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
+++ b/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
@@ -574,7 +574,8 @@ PaintArtifactCompositor::compositedLayerForPendingLayer(
 bool PaintArtifactCompositor::canMergeInto(
     const PaintArtifact& paintArtifact,
     const PaintChunk& newChunk,
-    const PendingLayer& candidatePendingLayer) {
+    const PendingLayer& candidatePendingLayer,
+    const HashSet<const EffectPaintPropertyNode*>* compositedEffectNodes) {
   const PaintChunk& pendingLayerFirstChunk =
       *candidatePendingLayer.paintChunks[0];
   if (paintArtifact.getDisplayItemList()[newChunk.beginIndex].isForeignLayer())
@@ -597,6 +598,12 @@ bool PaintArtifactCompositor::canMergeInto(
       return true;
     if (currentState->hasDirectCompositingReasons())
       return false;
+    if (compositedEffectNodes &&
+        compositedEffectNodes->find(currentState->effect()) !=
+            compositedEffectNodes->end()) {

[trchen, 2017/03/01 23:02:59]

I think this line will over-composite many of the chunks.

Chunk only needs compositing if the state switch across a compositing boundary,
but is okay to have a node that is right on the boundary. This should fix it:

if (currentState->innerMostNode() == Effect &&
    compositedEffectNodes &&
    compositedEffectNodes->find(currentState->effect()) !=
        compositedEffectNodes->end()) {

+      LOG(ERROR) << "newly composited";
+      return false;
+    }
   }
   return false;
 }
@@ -658,7 +665,8 @@ void PaintArtifactCompositor::PendingLayer::add(
 void PaintArtifactCompositor::collectPendingLayers(
     const PaintArtifact& paintArtifact,
     Vector<PendingLayer>& pendingLayers,
-    GeometryMapper& geometryMapper) {
+    GeometryMapper& geometryMapper,
+    const HashSet<const EffectPaintPropertyNode*>& compositedEffectNodes) {
   // n = # of paint chunks. Memoizing canMergeInto() can get it to O(n^2), and
   // other heuristics can make worst-case behavior better.
   for (const PaintChunk& paintChunk : paintArtifact.paintChunks()) {
@@ -667,7 +675,8 @@ void PaintArtifactCompositor::collectPendingLayers(
              pendingLayers.rbegin();
          candidatePendingLayer != pendingLayers.rend();
          ++candidatePendingLayer) {
-      if (canMergeInto(paintArtifact, paintChunk, *candidatePendingLayer)) {
+      if (canMergeInto(paintArtifact, paintChunk, *candidatePendingLayer,
+                       &compositedEffectNodes)) {
         candidatePendingLayer->add(paintChunk, &geometryMapper);
         createNew = false;
         break;
@@ -681,6 +690,53 @@ void PaintArtifactCompositor::collectPendingLayers(
   }
 }
 
+static void addEffectNodesUpToRoot(
+    const PaintChunk& paintChunk,
+    HashSet<const EffectPaintPropertyNode*>& compositedEffectNodes) {
+  const EffectPaintPropertyNode* effect =
+      paintChunk.properties.propertyTreeState.effect();
+  while (effect) {
+    compositedEffectNodes.insert(effect);
+    effect = effect->parent();
+  }
+}
+
+static void checkEffectNodesUpToRoot(
+    const PaintChunk& paintChunk,
+    unsigned currentIndex,
+    HashSet<const EffectPaintPropertyNode*>& compositedEffectNodes,
+    HashMap<const EffectPaintPropertyNode*, unsigned>& effectNodeLayerIndex) {
+  const EffectPaintPropertyNode* effect =
+      paintChunk.properties.propertyTreeState.effect();
+  while (effect) {
+    auto it = effectNodeLayerIndex.find(effect);
+    if (it != effectNodeLayerIndex.end() && it->value != currentIndex)
+      compositedEffectNodes.insert(effect);
+    else
+      effectNodeLayerIndex.set(effect, currentIndex);
+    effect = effect->parent();
+  }
+}
+
+void PaintArtifactCompositor::collectCompositedEffectNodes(
+    const Vector<PendingLayer>& pendingLayers,
+    HashSet<const EffectPaintPropertyNode*>& compositedEffectNodes) {
+  HashMap<const EffectPaintPropertyNode*, unsigned> effectNodeLayerIndex;
+  for (unsigned currentIndex = 0; currentIndex < pendingLayers.size();
+       ++currentIndex) {
+    const PendingLayer& pendingLayer = pendingLayers[currentIndex];
+    for (const auto* paintChunk : pendingLayer.paintChunks) {
+      if (paintChunk->properties.propertyTreeState
+              .hasDirectCompositingReasons()) {

[trchen, 2017/03/01 23:02:59]

propertyTreeState.hasDirectCompositingReasons() only look at the innermost node.
There can be compositing reason in the chain between chunk state and the
effect's input space. For example:

<div style="opacity:0.5;">
  <div style="will-change:transform;">
    <div style="overflow:hidden;">Chunk</div>
  </div>
</div>

We also want to check the output space of each effect in the chain, for example:

<div style="opacity:0.5">
  <div style="will-change:transform;">
    <div style="overflow:scroll">
      <div style="opacity:0.5">Chunk</div>
    </div>
  </div>
</div>

+        addEffectNodesUpToRoot(*paintChunk, compositedEffectNodes);
+      } else {
+        checkEffectNodesUpToRoot(*paintChunk, currentIndex,
+                                 compositedEffectNodes, effectNodeLayerIndex);
+      }
+    }
+  }
+}
+
 void PaintArtifactCompositor::update(
     const PaintArtifact& paintArtifact,
     RasterInvalidationTrackingMap<const PaintChunk>* rasterChunkInvalidations,
@@ -711,7 +767,18 @@ void PaintArtifactCompositor::update(
                                           sPropertyTreeSequenceNumber);
 
   Vector<PendingLayer, 0> pendingLayers;
-  collectPendingLayers(paintArtifact, pendingLayers, geometryMapper);
+  {
+    HashSet<const EffectPaintPropertyNode*> compositedEffectNodes;
+    Vector<PendingLayer, 0> tentativePendingLayers;
+    LOG(ERROR) << "first pass";
+    collectPendingLayers(paintArtifact, tentativePendingLayers, geometryMapper,
+                         compositedEffectNodes);
+    collectCompositedEffectNodes(tentativePendingLayers, compositedEffectNodes);
+    LOG(ERROR) << "# composited effect nodes: " << compositedEffectNodes.size();
+    LOG(ERROR) << "second pass";
+    collectPendingLayers(paintArtifact, pendingLayers, geometryMapper,
+                         compositedEffectNodes);
+  }
 
   Vector<std::unique_ptr<ContentLayerClientImpl>> newContentLayerClients;
   newContentLayerClients.reserveCapacity(paintArtifact.paintChunks().size());