Add documentation on cc/Blink nodes, ids, and indices.

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 036586341f78c05f5737cfd7675d1a319dd5883c..fec64316a3862a360fa1926da6a75a6f3e14a747 100644
--- a/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
+++ b/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
@@ -603,6 +603,44 @@ class PropertyTreeManager {
   void setupRootEffectNode();
   void setupRootScrollNode();
 
+  // A brief discourse on cc property tree nodes, identifiers, and current and
+  // future design evolution envisioned:
+  //
+  // cc property trees identify nodes by their |id|, which implementation-wise
+  // is actually its index in the property tree's vector of its node type. More
+  // recent cc code now refers to these as 'node indexes', or 'property tree

[chrishtr, 2017/01/04 20:54:28]

Could you land a patch that changes them to only have one naming scheme, rather
than both "node indexes" and "property tree indexes"?

[wkorman, 2017/01/04 21:22:55]

Sure, I'll look at that in a separate change and can update this comment
as part of that.

+  // indexes'. |parent_id| and |owner_id| in the cc property tree nodes are the
+  // same sort of node ids, all of which are stitched together to produce a tree
+  // representation.
+  //
+  // Note there are two other primary types of 'ids' referenced in cc property
+  // tree related logic: (1) ElementId, also known Blink-side as
+  // CompositorElementId, used by the animation system to allow tying an element
+  // to its respective layer, and (2) layer ids. There are other ancillary ids
+  // not relevant to any of the above, such as
+  // cc::TransformNode::sorting_context_id
+  // (a.k.a. blink::TransformPaintPropertyNode::renderingContextId()).
+  //
+  // There is a vision to move toward every layer having a |stable_id| instead
+  // of its current |id|. This could be represented by the layer's ElementId,

[ajuma, 2017/01/04 20:57:52]

"current |owner_id|"

[wkorman, 2017/01/04 21:22:55]

Rephrased this and next paragraph, let me know how it is now.

[ajuma, 2017/01/04 22:00:48]

Looks great, thanks!

+  // excepting that currently for performance reasons these are only assigned to
+  // layers with a composited animation. Once all layers have one, this could
+  // become the |stable_id| and the |owner_id| in each cc property tree node
+  // could then be the |stable_id| of its associated layer.

[ajuma, 2017/01/04 20:57:52]

The plan is that there won't be an associated layer, and the ElementId will come
from something like the layout object responsible for creating the property tree
node.

[wkorman, 2017/01/04 21:22:55]

See above comment.

+  //
+  // So, eventually cc property trees should not have to know anything about
+  // layer ids (once it is not assumed that each node is owned by a layer). That
+  // is, property tree nodes will have a |stable_id| and an index, but neither
+  // will correspond to a layer id. We would also like to explore moving to use
+  // a single shared property tree representation across both cc and Blink. See
+  // platform/graphics/paint/README.md for more.
+  //
+  // With the above as background, we can now state more clearly a description
+  // of the below set of compositor id generation methods: they take Blink paint
+  // property tree nodes as input and produce a corresponding cc 'node id',
+  // a.k.a., 'node index', for use as we build out the corresponding cc property
+  // tree representation.
+
   int compositorIdForTransformNode(const TransformPaintPropertyNode*);
   int compositorIdForClipNode(const ClipPaintPropertyNode*);
   int switchToEffectNode(const EffectPaintPropertyNode& nextEffect);
@@ -640,6 +678,8 @@ class PropertyTreeManager {
 
   struct BlinkEffectAndCcIdPair {
     const EffectPaintPropertyNode* effect;
+    // The cc property tree effect node id, or 'node index', for the cc effect
+    // node corresponding to the above Blink effect paint property node.
     int id;
   };
   Vector<BlinkEffectAndCcIdPair> m_effectStack;
@@ -661,7 +701,7 @@ void PropertyTreeManager::setupRootTransformNode() {
   transformTree.SetTargetId(transformNode.id, kRealRootNodeId);
   transformTree.SetContentTargetId(transformNode.id, kRealRootNodeId);
 
-  // TODO(jaydasika): We shouldn't set ToScreeen and FromScreen of root
+  // TODO(jaydasika): We shouldn't set ToScreen and FromScreen of root
   // transform node here. They should be set while updating transform tree in
   // cc.
   float deviceScaleFactor = m_rootLayer->GetLayerTree()->device_scale_factor();