diff --git a/pxr/usd/pcp/changes.cpp b/pxr/usd/pcp/changes.cpp index a534461938..2d3256c082 100644 --- a/pxr/usd/pcp/changes.cpp +++ b/pxr/usd/pcp/changes.cpp @@ -2197,17 +2197,27 @@ PcpChanges::_DidAddOrRemoveSublayer( std::string* debugSummary, std::vector* significant) { + PcpCacheChanges& cacheChanges = _GetCacheChanges(cache); + + // Before processing any sublayer paths first check if we have encountered + // this layer / sublayer path before. If we have, it indicates that there + // is a cycle in this layer stack. + const auto key = std::make_pair(layer, sublayerPath); + if (!cacheChanges._processedLayerSublayerPathPairs.insert(key).second) { + significant->resize(layerStacks.size(), false); + return; + } + PCP_APPEND_DEBUG( " Layer @%s@ changed sublayers\n", layer ? layer->GetIdentifier().c_str() : "invalid"); const auto& processChanges = - [this, &cache, &sublayerPath, &debugSummary, &layer]( + [this, &cache, &sublayerPath, &debugSummary, &layer, &cacheChanges]( const SdfLayerRefPtr sublayer, const PcpLayerStackPtrVector& layerStacks, _SublayerChangeType sublayerChange) { - PcpCacheChanges& cacheChanges = _GetCacheChanges(cache); if (sublayer) { _lifeboat.Retain(sublayer); cacheChanges.didAddOrRemoveNonEmptySublayer |= !sublayer->IsEmpty(); diff --git a/pxr/usd/pcp/changes.h b/pxr/usd/pcp/changes.h index 96edfd3a48..99aa36beac 100644 --- a/pxr/usd/pcp/changes.h +++ b/pxr/usd/pcp/changes.h @@ -147,6 +147,15 @@ class PcpCacheChanges { friend class PcpCache; friend class PcpChanges; + using _ProcessedLayerSublayerPathPairsKey = + std::pair; + + // Set of hashed layer / sublayer path pairs that have been processed in + // in this round of changes. These values are checked in order to avoid + // recursively processing cycles created in layer stacks. + std::unordered_set<_ProcessedLayerSublayerPathPairsKey, TfHash> + _processedLayerSublayerPathPairs; + // Must rebuild the prim/property stacks at each path due to a change // that only affects the internal representation of the stack and // not its contents. Because this causes no externally-observable diff --git a/pxr/usd/usd/testenv/testUsdChangeProcessing.py b/pxr/usd/usd/testenv/testUsdChangeProcessing.py index 0d04c8cf85..91ca4aa3a8 100644 --- a/pxr/usd/usd/testenv/testUsdChangeProcessing.py +++ b/pxr/usd/usd/testenv/testUsdChangeProcessing.py @@ -5,10 +5,8 @@ # Licensed under the terms set forth in the LICENSE.txt file available at # https://openusd.org/license. -from __future__ import print_function - import sys -from pxr import Sdf,Usd,Tf +from pxr import Sdf,Usd,Pcp def RenamingSpec(): '''Test renaming a SdfPrimSpec.''' @@ -39,9 +37,62 @@ def ChangeInsignificantSublayer(): prim = stage.DefinePrim("/Foo") assert prim +def _TestStageErrors(stageAndErrorCounts): + for stage, errorCount in stageAndErrorCounts: + errors = stage.GetCompositionErrors() + assert len(errors) == errorCount + + for i in range(errorCount): + assert isinstance(errors[i], Pcp.ErrorSublayerCycle) + +def AddSublayerWithCycle(): + '''Tests that adding a sublayer resulting in a cycle produces warnings''' + + root = Usd.Stage.CreateNew("root.usda") + a = Usd.Stage.CreateNew("a.usda") + b = Usd.Stage.CreateNew("b.usda") + + root.GetRootLayer().subLayerPaths.append("b.usda") + b.GetRootLayer().subLayerPaths.append("a.usda") + + # Initial sanity test, there should be no errors on any stage + _TestStageErrors([(root, 0), (a, 0), (b, 0)]) + + # Add the sublayer which creates a cycle in all stages + a.GetRootLayer().subLayerPaths.append("b.usda") + _TestStageErrors([(root, 1), (a, 1), (b, 1)]) + +def UnmuteWithCycle(): + '''Tests that unmuting a sublayer resulting in a cycle produces warnings''' + + root = Usd.Stage.CreateNew("root.usda") + a = Usd.Stage.CreateNew("a.usda") + b = Usd.Stage.CreateNew("b.usda") + + # Mute layer b on the root stage, this will prevent a cycle from being + # created while the sublayers are assembled + root.MuteLayer(b.GetRootLayer().identifier) + + root.GetRootLayer().subLayerPaths.append("b.usda") + b.GetRootLayer().subLayerPaths.append("a.usda") + a.GetRootLayer().subLayerPaths.append("b.usda") + + # Initial sanity test, there should be no errors on root + _TestStageErrors([(root, 0)]) + + # Unmute layer b creating a cycle on root + root.UnmuteLayer(b.GetRootLayer().identifier) + _TestStageErrors([(root, 1)]) + + # Mute the layer ensuring that the cycle has been removed + root.MuteLayer(b.GetRootLayer().identifier) + _TestStageErrors([(root, 0)]) + def Main(argv): RenamingSpec() ChangeInsignificantSublayer() + AddSublayerWithCycle() + UnmuteWithCycle() if __name__ == "__main__": Main(sys.argv)