Add effect node support in PaintArtifactCompositor for layer list mode [3/4]

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 2f97608af8e87ec0d5f89758bbfdf800e47ddb58..a81999e956570a91983eb0cb113035b78a0e1056 100644
--- a/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
+++ b/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
@@ -64,6 +64,7 @@ private:
 };
 
 PaintArtifactCompositor::PaintArtifactCompositor()
+    : m_propertyTreeSequenceNumber(0)
 {
     if (!RuntimeEnabledFeatures::slimmingPaintV2Enabled())
         return;
@@ -256,67 +257,6 @@ scoped_refptr<cc::Layer> foreignLayerForPaintChunk(const PaintArtifact& paintArt
     return layer;
 }
 
-static const int kInvalidNodeId = -1;
-static const int kRealRootNodeId = 0;
-static const int kSecondaryRootNodeId = 1;
-static const int kPropertyTreeSequenceNumber = 1;
-
-// Creates a minimal set of property trees for the compositor.
-void setMinimalPropertyTrees(cc::PropertyTrees* propertyTrees, int ownerId)
-{
-    // cc's property trees expect a child of the actual root to be used. So we
-    // need to create and populate an additional node for each type of tree.
-
-    cc::TransformTree& transformTree = propertyTrees->transform_tree;
-    if (transformTree.size() < 2) {
-        transformTree.Insert(cc::TransformNode(), kRealRootNodeId);
-        cc::TransformNode& transformNode = *transformTree.back();
-        transformNode.data.target_id = kRealRootNodeId;
-        transformNode.data.content_target_id = kSecondaryRootNodeId;
-        transformNode.data.source_node_id = kRealRootNodeId;
-        transformNode.data.needs_local_transform_update = true;
-        transformNode.owner_id = ownerId;
-        transformTree.set_needs_update(true);
-    }
-    DCHECK_EQ(transformTree.size(), 2u);
-
-    cc::ClipTree& clipTree = propertyTrees->clip_tree;
-    if (clipTree.size() < 2) {
-        clipTree.Insert(cc::ClipNode(), kRealRootNodeId);
-        cc::ClipNode& clipNode = *clipTree.back();
-        clipNode.data.transform_id = kSecondaryRootNodeId;
-        clipNode.data.target_id = kSecondaryRootNodeId;
-        clipNode.owner_id = ownerId;
-        clipTree.set_needs_update(true);
-    }
-    DCHECK_EQ(clipTree.size(), 2u);
-
-    cc::EffectTree& effectTree = propertyTrees->effect_tree;
-    if (effectTree.size() < 2) {
-        // This matches what cc does right now: the secondary root isn't a child
-        // of the first root (at index 0). This may not have been intentional.
-        effectTree.Insert(cc::EffectNode(), kInvalidNodeId);
-        cc::EffectNode& effectNode = *effectTree.back();
-        effectNode.data.has_render_surface = true;
-        effectNode.data.transform_id = kRealRootNodeId;
-        effectNode.data.clip_id = kRealRootNodeId;
-        effectNode.owner_id = ownerId;
-        effectTree.set_needs_update(true);
-    }
-    DCHECK_EQ(effectTree.size(), 2u);
-
-    cc::ScrollTree& scrollTree = propertyTrees->scroll_tree;
-    if (scrollTree.size() < 2) {
-        scrollTree.Insert(cc::ScrollNode(), kRealRootNodeId);
-        cc::ScrollNode& scrollNode = *scrollTree.back();
-        scrollNode.data.scrollable = false;
-        scrollNode.data.transform_id = kSecondaryRootNodeId;
-        scrollNode.owner_id = ownerId;
-        scrollTree.set_needs_update(true);
-    }
-    DCHECK_EQ(scrollTree.size(), 2u);
-}
-
 } // namespace
 
 void PaintArtifactCompositor::update(const PaintArtifact& paintArtifact)
@@ -385,19 +325,52 @@ scoped_refptr<cc::Layer> PaintArtifactCompositor::layerForPaintChunk(const Paint
     return layer;
 }
 
+constexpr int kRealRootNodeId = 0;

[jbroman, 2016/06/15 12:55:08]

static constexpr int, since this is not in an anonymous namespace (and below)

[trchen, 2016/06/29 01:24:32]

Done.

+constexpr int kSecondaryRootNodeId = 1;
+
+// Creates a minimal set of property trees for the compositor.
+static void resetPropertyTrees(cc::PropertyTrees* propertyTrees, int ownerId)
+{
+    // cc is hardcoded to use transform node #1 for device scale and transform.

[jbroman, 2016/06/15 12:55:08]

#1? It's not clear if you're using zero- or one-based indexing here. Suggest
saying "transform node index 1" (or 0 if it's that), or something similar, to be
clearer.

[trchen, 2016/06/29 01:24:33]

Done.

+    cc::TransformTree& transformTree = propertyTrees->transform_tree;
+    transformTree.clear();
+    cc::TransformNode& transformNode = *transformTree.Node(transformTree.Insert(cc::TransformNode(), kRealRootNodeId));
+    DCHECK_EQ(transformNode.id, kSecondaryRootNodeId);
+
+    // cc is hardcoded to use clip node #1 for viewport clip.

[jbroman, 2016/06/15 12:55:08]

And it's okay for this to use the implicit root, rather than the secondary root,
for its transform node?

[trchen, 2016/06/29 01:24:33]

Yes, this matches what's done in cc::PropertyTreeBuilder.

What happens is that the LayerTreeHost gets viewport size (or damage rect) in
physical space, and the value will be assigned to the clip rect of clip node #1.

+    cc::ClipTree& clipTree = propertyTrees->clip_tree;
+    clipTree.clear();
+    cc::ClipNode& clipNode = *clipTree.Node(clipTree.Insert(cc::ClipNode(), kRealRootNodeId));
+    DCHECK_EQ(clipNode.id, kSecondaryRootNodeId);
+    clipNode.owner_id = ownerId;
+
+    // cc is hardcoded to use effect node #1 for root render surface.
+    cc::EffectTree& effectTree = propertyTrees->effect_tree;
+    effectTree.clear();
+    cc::EffectNode& effectNode = *effectTree.Node(effectTree.Insert(cc::EffectNode(), kRealRootNodeId));
+    DCHECK_EQ(effectNode.id, kSecondaryRootNodeId);
+    effectNode.owner_id = ownerId;
+    effectNode.data.clip_id = kSecondaryRootNodeId;
+    effectNode.data.has_render_surface = true;
+
+    cc::ScrollTree& scrollTree = propertyTrees->scroll_tree;
+    scrollTree.clear();
+}
+
 void PaintArtifactCompositor::updateInLayerListMode(const PaintArtifact& paintArtifact)
 {
+    const int sequenceNumber = ++m_propertyTreeSequenceNumber;
+
     cc::LayerTreeHost* host = m_rootLayer->layer_tree_host();
 
-    // The root layer must be the owner so that the render surface
-    // validation works. It's expected to own at least the effect node.
-    setMinimalPropertyTrees(host->property_trees(), m_rootLayer->id());
+    resetPropertyTrees(host->property_trees(), m_rootLayer->id());
+
     m_rootLayer->RemoveAllChildren();
-    m_rootLayer->set_property_tree_sequence_number(kPropertyTreeSequenceNumber);
+    m_rootLayer->set_property_tree_sequence_number(sequenceNumber);
     m_rootLayer->SetTransformTreeIndex(kSecondaryRootNodeId);
     m_rootLayer->SetClipTreeIndex(kSecondaryRootNodeId);
     m_rootLayer->SetEffectTreeIndex(kSecondaryRootNodeId);
-    m_rootLayer->SetScrollTreeIndex(kSecondaryRootNodeId);
+    m_rootLayer->SetScrollTreeIndex(kRealRootNodeId);
 
     m_contentLayerClients.clear();
     m_contentLayerClients.reserveCapacity(paintArtifact.paintChunks().size());
@@ -414,15 +387,15 @@ void PaintArtifactCompositor::updateInLayerListMode(const PaintArtifact& paintAr
             + layerOffset);
 
         m_rootLayer->AddChild(layer);
-        layer->set_property_tree_sequence_number(kPropertyTreeSequenceNumber);
+        layer->set_property_tree_sequence_number(sequenceNumber);
         layer->SetTransformTreeIndex(kSecondaryRootNodeId);
         layer->SetClipTreeIndex(kSecondaryRootNodeId);
         layer->SetEffectTreeIndex(kSecondaryRootNodeId);
-        layer->SetScrollTreeIndex(kSecondaryRootNodeId);
+        layer->SetScrollTreeIndex(kRealRootNodeId);
     }
 
     // Mark the property trees as having been rebuilt.
-    host->property_trees()->sequence_number = kPropertyTreeSequenceNumber;
+    host->property_trees()->sequence_number = sequenceNumber;
     host->property_trees()->needs_rebuild = false;
 }