pcp: Small cleanups to instancing utility functions

- Update Pcp_PrimIndexIsInstanceable to use a subtree iterator instead
  of a stack of nodes for strength order traversals. This gives us the
  same behavior while simplifying the code and avoiding potential
  memory allocations.

  Note that the stack of nodes was originally used instead of a
  recursive approach to avoid potential stack overflows in large
  graphs. The subtree iterator maintains that property.

- The instancing traversal helpers now use the cached transitive
  dependency bit stored on each node instead of manually keeping
  track of whether

(Internal change: 2392191)

Author: sunyab

pxr/usd/pcp/instancing.cpp

@@ -8,7 +8,6 @@
 #include "pxr/usd/pcp/instancing.h"
 
 #include "pxr/base/tf/envSetting.h"
-#include "pxr/base/tf/smallVector.h"
 #include "pxr/base/trace/trace.h"
 
 PXR_NAMESPACE_OPEN_SCOPE
@@ -73,15 +72,11 @@ Pcp_PrimIndexIsInstanceable(
     // Compose the value of the 'instanceable' metadata to see if this
     // prim has been tagged as instanceable.
     bool isInstance = false;
-    static const TfToken instanceField = SdfFieldKeys->Instanceable;
-    // Stack of nodes left to visit, in strong-to-weak order.
-    // Strongest open node is top of the stack.
-    TfSmallVector<PcpNodeRef, 64> nodesToVisit;
-    nodesToVisit.push_back(primIndex.GetRootNode());
     bool opinionFound = false;
-    while (!nodesToVisit.empty()) {
-        PcpNodeRef node = nodesToVisit.back();
-        nodesToVisit.pop_back();
+    static const TfToken instanceField = SdfFieldKeys->Instanceable;
+    for (const PcpNodeRef& node : 
+             Pcp_GetSubtreeRange(primIndex.GetRootNode())) {
+
         if (node.CanContributeSpecs()) {
             const PcpLayerStackSite& site = node.GetSite();
             for (SdfLayerRefPtr const& layer: site.layerStack->GetLayers()) {
@@ -94,9 +89,6 @@ Pcp_PrimIndexIsInstanceable(
                 break;
             }
         }
-        TF_REVERSE_FOR_ALL(childIt, Pcp_GetChildrenRange(node)) {
-            nodesToVisit.push_back(*childIt);
-        }
     }
     return isInstance;
 }

pxr/usd/pcp/instancing.h

@@ -70,8 +70,7 @@ Pcp_TraverseInstanceableWeakToStrong(
 
 inline bool
 Pcp_ChildNodeIsInstanceable(
-    const PcpNodeRef& node,
-    bool *hasAnyDirectArcsInNodeChain)
+    const PcpNodeRef& node)
 {
     // Non-ancestral nodes are instanceable: they represent a direct
     // composition arc to a portion of scenegraph that could be shared
@@ -80,34 +79,22 @@ Pcp_ChildNodeIsInstanceable(
     // exist in the graph because they were composed in a subtree of direct 
     // arc to a subroot path. These nodes are also instanceable as they are 
     // considered part of the direct arc that brought them in. This is why we 
-    // keep track of and check whether there are any direct arcs in the node's 
-    // chain up to the root node when determining if a node is instanceable.
-    *hasAnyDirectArcsInNodeChain = 
-        *hasAnyDirectArcsInNodeChain || !node.IsDueToAncestor();
-
+    // check whether there are any direct arcs in the node's chain up to the
+    // root node when determining if a node is instanceable.
+    //
     // If a node has no specs or cannot contribute specs, it is not instanceable
     // since it has no opinions to contribute to the prim index. In particular,
     // this allows prim indexes with implied arcs in different layer stacks
     // that have no overrides to still be considered equivalent for sharing.
-    return *hasAnyDirectArcsInNodeChain && 
+    return node.HasTransitiveDirectDependency() && 
         node.CanContributeSpecs() && node.HasSpecs();
 }
 
 inline bool 
 Pcp_ChildNodeIsDirectOrInDirectArcSubtree(
     const PcpNodeRef& node)
 {
-    if (node.IsRootNode() || !node.IsDueToAncestor()) {
-        return true;
-    }
-    for (PcpNodeRef parent = node.GetParentNode(); 
-         !parent.IsRootNode(); 
-         parent = parent.GetParentNode()) {
-        if (!parent.IsDueToAncestor()) {
-            return true;
-        }
-    }
-    return false;
+    return node.IsRootNode() || node.HasTransitiveDirectDependency();
 }
 
 inline bool
@@ -122,8 +109,7 @@ template <class Visitor>
 inline void
 Pcp_TraverseInstanceableStrongToWeakHelper(
     const PcpNodeRef& node,
-    Visitor* visitor,
-    bool hasAnyDirectArcsInNodeChain)
+    Visitor* visitor)
 {
     // If the node is culled, the entire subtree rooted at this node
     // does not contribute to the prim index, so we can prune the 
@@ -132,16 +118,14 @@ Pcp_TraverseInstanceableStrongToWeakHelper(
         return;
     }
 
-    const bool isInstanceable = 
-        Pcp_ChildNodeIsInstanceable(node, &hasAnyDirectArcsInNodeChain);
+    const bool isInstanceable = Pcp_ChildNodeIsInstanceable(node);
     if (!visitor->Visit(node, isInstanceable)) {
         return;
     }
 
     TF_FOR_ALL(childIt, Pcp_GetChildrenRange(node)) {
         const PcpNodeRef& childNode = *childIt;
-        Pcp_TraverseInstanceableStrongToWeakHelper(
-            childNode, visitor, hasAnyDirectArcsInNodeChain);
+        Pcp_TraverseInstanceableStrongToWeakHelper(childNode, visitor);
     }
 }
 
@@ -158,17 +142,15 @@ Pcp_TraverseInstanceableStrongToWeak(
 
     TF_FOR_ALL(childIt, Pcp_GetChildrenRange(rootNode)) {
         const PcpNodeRef& childNode = *childIt;
-        Pcp_TraverseInstanceableStrongToWeakHelper(
-            childNode, visitor, /* hasAnyDirectArcsInNodeChain = */ false);
+        Pcp_TraverseInstanceableStrongToWeakHelper(childNode, visitor);
     }
 }
 
 template <class Visitor>
 inline void
 Pcp_TraverseInstanceableWeakToStrongHelper(
     const PcpNodeRef& node,
-    Visitor* visitor,
-    bool hasAnyDirectArcsInNodeChain)
+    Visitor* visitor)
 {
     // If the node is culled, the entire subtree rooted at this node
     // does not contribute to the prim index, so we can prune the 
@@ -177,13 +159,11 @@ Pcp_TraverseInstanceableWeakToStrongHelper(
         return;
     }
 
-    const bool isInstanceable = 
-        Pcp_ChildNodeIsInstanceable(node, &hasAnyDirectArcsInNodeChain);
+    const bool isInstanceable = Pcp_ChildNodeIsInstanceable(node);
 
     TF_REVERSE_FOR_ALL(childIt, Pcp_GetChildrenRange(node)) {
         const PcpNodeRef& childNode = *childIt;
-        Pcp_TraverseInstanceableWeakToStrongHelper(
-            childNode, visitor, hasAnyDirectArcsInNodeChain);
+        Pcp_TraverseInstanceableWeakToStrongHelper(childNode, visitor);
     }
 
     visitor->Visit(node, isInstanceable);
@@ -198,20 +178,12 @@ Pcp_TraverseInstanceableWeakToStrong(
     if (subtreeRootNode.IsRootNode()) {
         TF_REVERSE_FOR_ALL(childIt, Pcp_GetChildrenRange(subtreeRootNode)) {
             const PcpNodeRef& childNode = *childIt;
-            Pcp_TraverseInstanceableWeakToStrongHelper(
-                childNode, visitor, /* hasAnyDirectArcsInNodeChain = */ false);
+            Pcp_TraverseInstanceableWeakToStrongHelper(childNode, visitor);
         }
 
         visitor->Visit(subtreeRootNode, /* nodeIsInstanceable = */ false);
     } else {
-        // Because we're starting below the root node, we need to find out if
-        // there are any direct arcs between the subtree parent and the true
-        // root node so that we can correctly determine in there are any direct
-        // nodes in whole node chain for each subtree node.
-        const bool hasAnyDirectArcsInNodeChain = 
-            Pcp_ChildNodeIsDirectOrInDirectArcSubtree(subtreeRootNode);
-        Pcp_TraverseInstanceableWeakToStrongHelper(
-            subtreeRootNode, visitor, hasAnyDirectArcsInNodeChain);
+        Pcp_TraverseInstanceableWeakToStrongHelper(subtreeRootNode, visitor);
     }
 }