diff --git a/pxr/usd/usd/clip.cpp b/pxr/usd/usd/clip.cpp index 99c53cf901..f248d5df51 100644 --- a/pxr/usd/usd/clip.cpp +++ b/pxr/usd/usd/clip.cpp @@ -104,7 +104,11 @@ Usd_Clip::Usd_Clip( const std::shared_ptr &timeMapping) : sourceLayerStack(clipSourceLayerStack) , sourcePrimPath(clipSourcePrimPath) - , sourceLayerIndex(clipSourceLayerIndex) + , sourceLayer( + TF_VERIFY(clipSourceLayerIndex + < clipSourceLayerStack->GetLayers().size()) ? + SdfLayerHandle(clipSourceLayerStack->GetLayers()[clipSourceLayerIndex]) : + SdfLayerHandle()) , assetPath(clipAssetPath) , primPath(clipPrimPath) , authoredStartTime(clipAuthoredStartTime) @@ -119,12 +123,11 @@ Usd_Clip::Usd_Clip( // This is important for change processing. Clip layers will be kept // alive during change processing, so any clips that are reconstructed // will have the opportunity to reuse the already-opened layer. - if (TF_VERIFY(sourceLayerIndex < sourceLayerStack->GetLayers().size())) { + if (sourceLayer) { const ArResolverContextBinder binder( sourceLayerStack->GetIdentifier().pathResolverContext); _layer = SdfLayer::FindRelativeToLayer( - sourceLayerStack->GetLayers()[sourceLayerIndex], - assetPath.GetAssetPath()); + sourceLayer, assetPath.GetAssetPath()); } _hasLayer = (bool)_layer; @@ -737,12 +740,11 @@ Usd_Clip::_GetLayerForClip() const SdfLayerRefPtr layer; - if (TF_VERIFY(sourceLayerIndex < sourceLayerStack->GetLayers().size())) { + if (TF_VERIFY(sourceLayer)) { const ArResolverContextBinder binder( sourceLayerStack->GetIdentifier().pathResolverContext); layer = SdfLayer::FindOrOpenRelativeToLayer( - sourceLayerStack->GetLayers()[sourceLayerIndex], - assetPath.GetAssetPath()); + sourceLayer, assetPath.GetAssetPath()); } if (!layer) { diff --git a/pxr/usd/usd/clip.h b/pxr/usd/usd/clip.h index acdb317fe5..e76dc80d0e 100644 --- a/pxr/usd/usd/clip.h +++ b/pxr/usd/usd/clip.h @@ -192,11 +192,10 @@ struct Usd_Clip /// open, it will generally be kept open for the life of the stage. SdfLayerHandle GetLayerIfOpen() const; - /// Layer stack, prim spec path, and index of layer where this clip - /// was introduced. + /// Layer stack, prim spec path, and layer where this clip was introduced. PcpLayerStackPtr sourceLayerStack; SdfPath sourcePrimPath; - size_t sourceLayerIndex; + SdfLayerHandle sourceLayer; /// Asset path for the clip and the path to the prim in the clip /// that provides data. diff --git a/pxr/usd/usd/clipSet.cpp b/pxr/usd/usd/clipSet.cpp index ff9af2e6b8..0ea49bb4f6 100644 --- a/pxr/usd/usd/clipSet.cpp +++ b/pxr/usd/usd/clipSet.cpp @@ -30,6 +30,8 @@ #include "pxr/usd/usd/usdaFileFormat.h" #include "pxr/usd/usd/valueUtils.h" +#include "pxr/usd/pcp/layerStack.h" + #include "pxr/base/tf/staticTokens.h" #include @@ -336,7 +338,9 @@ Usd_ClipSet::Usd_ClipSet( : name(name_) , sourceLayerStack(clipDef.sourceLayerStack) , sourcePrimPath(clipDef.sourcePrimPath) - , sourceLayerIndex(clipDef.indexOfLayerWhereAssetPathsFound) + , sourceLayer( + clipDef.sourceLayerStack->GetLayers()[ + clipDef.indexOfLayerWhereAssetPathsFound]) , clipPrimPath(SdfPath(*clipDef.clipPrimPath)) , interpolateMissingClipValues(false) { diff --git a/pxr/usd/usd/clipSet.h b/pxr/usd/usd/clipSet.h index a4cb74e5d8..84e46f774c 100644 --- a/pxr/usd/usd/clipSet.h +++ b/pxr/usd/usd/clipSet.h @@ -103,7 +103,7 @@ class Usd_ClipSet std::string name; PcpLayerStackPtr sourceLayerStack; SdfPath sourcePrimPath; - size_t sourceLayerIndex; + SdfLayerHandle sourceLayer; SdfPath clipPrimPath; Usd_ClipRefPtr manifestClip; Usd_ClipRefPtrVector valueClips; diff --git a/pxr/usd/usd/resolver.cpp b/pxr/usd/usd/resolver.cpp index 36aa74e7b1..ba7b3b0860 100644 --- a/pxr/usd/usd/resolver.cpp +++ b/pxr/usd/usd/resolver.cpp @@ -124,13 +124,6 @@ Usd_Resolver::_SkipEmptyNodes() } } -size_t -Usd_Resolver::GetLayerStackIndex() const -{ - return std::distance( - _curNode->GetLayerStack()->GetLayers().begin(), _curLayer); -} - void Usd_Resolver::NextNode() { diff --git a/pxr/usd/usd/resolver.h b/pxr/usd/usd/resolver.h index 5eaa478dca..15203c0a8d 100644 --- a/pxr/usd/usd/resolver.h +++ b/pxr/usd/usd/resolver.h @@ -118,13 +118,6 @@ class Usd_Resolver { return *_curLayer; } - /// Returns the index of the current layer in the current node's layer - /// stack for a valid resolver. - /// - /// The behavior is undefined if the resolver is not valid. - USD_API - size_t GetLayerStackIndex() const; - /// Returns a translated path for the current PcpNode and Layer for a valid /// resolver. /// diff --git a/pxr/usd/usd/stage.cpp b/pxr/usd/usd/stage.cpp index ea329a6234..67a93edf0d 100644 --- a/pxr/usd/usd/stage.cpp +++ b/pxr/usd/usd/stage.cpp @@ -7599,10 +7599,9 @@ struct UsdStage::_PropertyStackResolver { if (_withLayerOffsets) { // The layer offset for the clip is the layer offset of the // source layer of the clip set. - const auto layer = clipSet->sourceLayerStack->GetLayers()[ - clipSet->sourceLayerIndex]; propertyStackWithLayerOffsets.emplace_back( - propertySpec, _GetLayerToStageOffset(node, layer)); + propertySpec, + _GetLayerToStageOffset(node, clipSet->sourceLayer)); } else { propertyStack.push_back(propertySpec); } @@ -7979,11 +7978,10 @@ _GetResolvedValueAtTimeWithClipsImpl( } } - const size_t layerStackIndex = res->GetLayerStackIndex(); for (const Usd_ClipSetRefPtr& clipSet : clips) { // We only care about clips that were introduced at this // position within the LayerStack. - if (clipSet->sourceLayerIndex == layerStackIndex) { + if (clipSet->sourceLayer == res->GetLayer()) { // Look through clips to see if they have a time sample for // this attribute. If a time is given, examine just the clips // that are active at that time. diff --git a/pxr/usd/usd/testenv/testUsdValueClips.py b/pxr/usd/usd/testenv/testUsdValueClips.py index 531770e582..6d1719b8d0 100644 --- a/pxr/usd/usd/testenv/testUsdValueClips.py +++ b/pxr/usd/usd/testenv/testUsdValueClips.py @@ -2530,5 +2530,98 @@ def test_TemplateFileFormatArguments(self): self.assertTrue(layer) self.assertEqual(layer.GetFileFormatArguments(), {'a': '1', 'b': 'str'}) + def test_SublayerChanges(self): + """Test that clip layers are loaded successfully when sublayers + are added or removed before the clip layers are pulled on.""" + + def _test(stage): + # Query our test attribute's property stack and verify that it + # contains the opinions we expect. This will open the clip layer. + a = stage.GetAttributeAtPath('/SingleClip.attr_1') + propertyStack = a.GetPropertyStack(0) + + rootLayer = stage.GetRootLayer() + sublayerWithClip = Sdf.Layer.FindRelativeToLayer( + rootLayer, 'layers/sublayer.usda') + self.assertTrue(sublayerWithClip) + + clipLayer = Sdf.Layer.FindRelativeToLayer( + sublayerWithClip, 'clip.usda') + self.assertTrue(clipLayer) + + refLayer = Sdf.Layer.FindRelativeToLayer( + rootLayer, 'layers/ref.usda') + self.assertTrue(refLayer) + + self.assertEqual( + propertyStack, + [sublayerWithClip.GetAttributeAtPath('/SingleClip.attr_1'), + clipLayer.GetAttributeAtPath('/Model.attr_1'), + refLayer.GetAttributeAtPath('/Model.attr_1')]) + + # Test combinations of inserting and removing sublayers prior to + # pulling on attributes and opening clip layers. Clip layers are + # opened the first time the _test function is called, so these + # tests are separated into insert-first and remove-first to cover + # both cases. Empty and non-empty sublayers are also tested + # separately since there's an optimization that avoids significant + # resyncs in the former case. + + def _TestInsertAndRemoveEmptySublayer(): + dummySublayer = Sdf.Layer.CreateAnonymous('.usda') + rootLayer = Sdf.Layer.FindOrOpen('sublayerChanges/root.usda') + + stage = Usd.Stage.Open(rootLayer) + rootLayer.subLayerPaths.insert(0, dummySublayer.identifier) + _test(stage) + + del rootLayer.subLayerPaths[0] + _test(stage) + + def _TestRemoveAndInsertEmptySublayer(): + dummySublayer = Sdf.Layer.CreateAnonymous('.usda') + + rootLayer = Sdf.Layer.FindOrOpen('sublayerChanges/root.usda') + rootLayer.subLayerPaths.insert(0, dummySublayer.identifier) + + stage = Usd.Stage.Open(rootLayer) + del rootLayer.subLayerPaths[0] + _test(stage) + + rootLayer.subLayerPaths.insert(0, dummySublayer.identifier) + _test(stage) + + def _TestInsertAndRemoveNonEmptySublayer(): + dummySublayer = Sdf.Layer.CreateAnonymous('.usda') + Sdf.CreatePrimInLayer(dummySublayer, '/Dummy') + + rootLayer = Sdf.Layer.FindOrOpen('sublayerChanges/root.usda') + + stage = Usd.Stage.Open(rootLayer) + rootLayer.subLayerPaths.insert(0, dummySublayer.identifier) + _test(stage) + + del rootLayer.subLayerPaths[0] + _test(stage) + + def _TestRemoveAndInsertNonEmptySublayer(): + dummySublayer = Sdf.Layer.CreateAnonymous('.usda') + Sdf.CreatePrimInLayer(dummySublayer, '/Dummy') + + rootLayer = Sdf.Layer.FindOrOpen('sublayerChanges/root.usda') + rootLayer.subLayerPaths.insert(0, dummySublayer.identifier) + + stage = Usd.Stage.Open(rootLayer) + del rootLayer.subLayerPaths[0] + _test(stage) + + rootLayer.subLayerPaths.insert(0, dummySublayer.identifier) + _test(stage) + + _TestInsertAndRemoveNonEmptySublayer() + _TestRemoveAndInsertNonEmptySublayer() + _TestInsertAndRemoveEmptySublayer() + _TestRemoveAndInsertEmptySublayer() + if __name__ == "__main__": unittest.main() diff --git a/pxr/usd/usd/testenv/testUsdValueClips/sublayerChanges/layers/clip.usda b/pxr/usd/usd/testenv/testUsdValueClips/sublayerChanges/layers/clip.usda new file mode 100644 index 0000000000..8c0b3c29f9 --- /dev/null +++ b/pxr/usd/usd/testenv/testUsdValueClips/sublayerChanges/layers/clip.usda @@ -0,0 +1,9 @@ +#usda 1.0 + +def "Model" +{ + float attr_1.timeSamples = { + 0: 10, + } +} + diff --git a/pxr/usd/usd/testenv/testUsdValueClips/sublayerChanges/layers/clip_manifest.usda b/pxr/usd/usd/testenv/testUsdValueClips/sublayerChanges/layers/clip_manifest.usda new file mode 100644 index 0000000000..1262c8a1ba --- /dev/null +++ b/pxr/usd/usd/testenv/testUsdValueClips/sublayerChanges/layers/clip_manifest.usda @@ -0,0 +1,7 @@ +#usda 1.0 + +def "Model" +{ + float attr_1 +} + diff --git a/pxr/usd/usd/testenv/testUsdValueClips/sublayerChanges/layers/ref.usda b/pxr/usd/usd/testenv/testUsdValueClips/sublayerChanges/layers/ref.usda new file mode 100644 index 0000000000..d0934a99e3 --- /dev/null +++ b/pxr/usd/usd/testenv/testUsdValueClips/sublayerChanges/layers/ref.usda @@ -0,0 +1,7 @@ +#usda 1.0 + +def "Model" +{ + float attr_1 = 1 +} + diff --git a/pxr/usd/usd/testenv/testUsdValueClips/sublayerChanges/layers/sublayer.usda b/pxr/usd/usd/testenv/testUsdValueClips/sublayerChanges/layers/sublayer.usda new file mode 100644 index 0000000000..aab0574f55 --- /dev/null +++ b/pxr/usd/usd/testenv/testUsdValueClips/sublayerChanges/layers/sublayer.usda @@ -0,0 +1,19 @@ +#usda 1.0 + +over "SingleClip" ( + clips = { + dictionary default = { + double2[] active = [(0, 0)] + asset[] assetPaths = [@./clip.usda@] + asset manifestAssetPath = @./clip_manifest.usda@ + string primPath = "/Model" + } + } + references = @./ref.usda@ +) +{ + float attr_1.timeSamples = { + 0: 100, + } +} + diff --git a/pxr/usd/usd/testenv/testUsdValueClips/sublayerChanges/root.usda b/pxr/usd/usd/testenv/testUsdValueClips/sublayerChanges/root.usda new file mode 100644 index 0000000000..df7c63524c --- /dev/null +++ b/pxr/usd/usd/testenv/testUsdValueClips/sublayerChanges/root.usda @@ -0,0 +1,6 @@ +#usda 1.0 +( + subLayers = [ + @./layers/sublayer.usda@ + ] +)