From 5907c8152ee36a7d95f638e456b67d0d4f59820f Mon Sep 17 00:00:00 2001 From: Pierre Baillargeon Date: Fri, 8 Sep 2023 13:48:42 -0400 Subject: [PATCH 1/2] EMSUSD-277 fix lost muted anonymous layers - Add a global functionality to hold onto muted layers. - This is to work around the fact that OpenUSD will not keep muted layers in memory. - Make sure to clean up the list of muted layers when a new Maya scene is created. - Use the muted layer holder in the MuteLayer command. - The base class of the command was trying to hold onto layer, but when the undo stack is cleared, so would be the anonymous layers. - In particular, when unloading a Maya Reference, Maya always clears the undo stack. - Add a test that check if muted anonymous layers are lost when editing a Maya Reference. - The test fails without my fix, works with it. - Improved test helper functions to enable creating anonymous sub-layers. --- lib/mayaUsd/commands/layerEditorCommand.cpp | 9 ++- lib/mayaUsd/utils/layerMuting.cpp | 73 +++++++++++++++++++ lib/mayaUsd/utils/layerMuting.h | 24 ++++++ .../mayaUsd/fileio/testAddMayaReference.py | 63 +++++++++++++++- test/testUtils/usdUtils.py | 52 ++++++++++--- 5 files changed, 206 insertions(+), 15 deletions(-) diff --git a/lib/mayaUsd/commands/layerEditorCommand.cpp b/lib/mayaUsd/commands/layerEditorCommand.cpp index 43f36bca6e..0fc27d2b4d 100644 --- a/lib/mayaUsd/commands/layerEditorCommand.cpp +++ b/lib/mayaUsd/commands/layerEditorCommand.cpp @@ -17,6 +17,7 @@ #include "layerEditorCommand.h" #include +#include #include #include @@ -620,7 +621,7 @@ class MuteLayer : public BaseCmd // we perfer not holding to pointers needlessly, but we need to hold on to the layer if we // mute it otherwise usd will let go of it and its modifications, and any dirty children // will also be lost - _mutedLayer = layer; + addMutedLayer(layer); return true; } @@ -638,8 +639,9 @@ class MuteLayer : public BaseCmd saveSelection(); stage->MuteLayer(layer->GetIdentifier()); } + // we can release the pointer - _mutedLayer = nullptr; + removeMutedLayer(layer); return true; } @@ -683,8 +685,7 @@ class MuteLayer : public BaseCmd globalSn->replaceWith(UsdUfe::recreateDescendants(_savedSn, path)); } - Ufe::Selection _savedSn; - PXR_NS::SdfLayerRefPtr _mutedLayer; + Ufe::Selection _savedSn; }; // We assume the indexes given to the command are the original indexes diff --git a/lib/mayaUsd/utils/layerMuting.cpp b/lib/mayaUsd/utils/layerMuting.cpp index 271258804f..2a787afc57 100644 --- a/lib/mayaUsd/utils/layerMuting.cpp +++ b/lib/mayaUsd/utils/layerMuting.cpp @@ -16,6 +16,10 @@ #include "layerMuting.h" +#include + +#include + namespace MAYAUSD_NS_DEF { MStatus copyLayerMutingToAttribute(const PXR_NS::UsdStage& stage, MayaUsdProxyShapeBase& proxyShape) @@ -32,4 +36,73 @@ copyLayerMutingFromAttribute(const MayaUsdProxyShapeBase& proxyShape, PXR_NS::Us return MS::kSuccess; } +namespace { + +// Automatic reset of recorded muted layers when the Maya scene is reset. +struct SceneResetListener : public PXR_NS::TfWeakBase +{ + SceneResetListener() + { + PXR_NS::TfWeakPtr me(this); + PXR_NS::TfNotice::Register(me, &SceneResetListener::OnSceneReset); + } + + void OnSceneReset(const UsdMayaSceneResetNotice&) + { + // Make sure we don't hold onto muted layers now that the + // Maya scene is reset. + forgetMutedLayers(); + } +}; + +// The set of muted layers. +// +// Kept in a function to avoid problem with the order of construction +// of global variables in C++. +using MutedLayers = std::set; +MutedLayers& getMutedLayers() +{ + // Note: C++ guarantees correct multi-thread protection for static + // variables initialization in functions. + static SceneResetListener onSceneResetListener; + static MutedLayers layers; + return layers; +} +} // namespace + +void addMutedLayer(const PXR_NS::SdfLayerRefPtr& layer) +{ + if (!layer) + return; + + // Non-dirty, non-anonymous layers can be reloaded, so we + // won't hold onto them. + const bool needHolding = (layer->IsDirty() || layer->IsAnonymous()); + if (!needHolding) + return; + + MutedLayers& layers = getMutedLayers(); + layers.insert(layer); + + // Hold onto sub-layers as well, in case they are dirty or anonymous. + // Note: the GetSubLayerPaths function returns proxies, so we have to + // hold the std::string by value, not reference. + for (const std::string subLayerPath : layer->GetSubLayerPaths()) { + auto subLayer = SdfLayer::FindRelativeToLayer(layer, subLayerPath); + addMutedLayer(subLayer); + } +} + +void removeMutedLayer(const PXR_NS::SdfLayerRefPtr& layer) +{ + MutedLayers& layers = getMutedLayers(); + layers.erase(layer); +} + +void forgetMutedLayers() +{ + MutedLayers& layers = getMutedLayers(); + layers.clear(); +} + } // namespace MAYAUSD_NS_DEF diff --git a/lib/mayaUsd/utils/layerMuting.h b/lib/mayaUsd/utils/layerMuting.h index 881bee2fd9..619e37dc3c 100644 --- a/lib/mayaUsd/utils/layerMuting.h +++ b/lib/mayaUsd/utils/layerMuting.h @@ -19,6 +19,7 @@ #include #include +#include #include namespace MAYAUSD_NS_DEF { @@ -47,6 +48,29 @@ MAYAUSD_CORE_PUBLIC MStatus copyLayerMutingFromAttribute(const MayaUsdProxyShapeBase& proxyShape, PXR_NS::UsdStage& stage); +// OpenUSD forget everything about muted layers. The OpenUSD documentation for +// the MuteLayer function says: +// +// Note that muting a layer will cause this stage to release all references +// to that layer. If no other client is holding on to references to that +// layer, it will be unloaded.In this case, if there are unsaved edits to +// the muted layer, those edits are lost. +// +// Since anonymous layers are not serialized, muting an anonymous layer will +// cause that layer and its contents to be lost in this case. +// +// So we need to hold on to muted layers. We do this in a private global list +// of muted layers. That list gets cleared when a new Maya scene is created. + +MAYAUSD_CORE_PUBLIC +void addMutedLayer(const PXR_NS::SdfLayerRefPtr& layer); + +MAYAUSD_CORE_PUBLIC +void removeMutedLayer(const PXR_NS::SdfLayerRefPtr& layer); + +MAYAUSD_CORE_PUBLIC +void forgetMutedLayers(); + } // namespace MAYAUSD_NS_DEF #endif diff --git a/test/lib/mayaUsd/fileio/testAddMayaReference.py b/test/lib/mayaUsd/fileio/testAddMayaReference.py index 4fa856579d..eeb50c26b4 100644 --- a/test/lib/mayaUsd/fileio/testAddMayaReference.py +++ b/test/lib/mayaUsd/fileio/testAddMayaReference.py @@ -23,9 +23,10 @@ import ufeUtils import usdUtils -from pxr import Tf, Usd, Kind +from pxr import Tf, Usd, Kind, Sdf from maya import cmds +import maya.mel as mel from maya import standalone from maya.api import OpenMaya as om @@ -288,6 +289,66 @@ def testDiscardMayaRefUnderDeactivatedPrim(self): self.assertTrue(mayaRefPrim.IsActive()) + def testEditAndDiscardMayaRefWithMutedLayer(self): + ''' + Test editing then discarding a Maya Reference when an anonymous + has been muted does not lose the muted layer. + + Add a Maya Reference using auto-edit, create a new layer, mute it, + then discard the edits. Verify the muted layer still exists. + ''' + kDefaultPrimName = mayaRefUtils.defaultMayaReferencePrimName() + + # Create a new layer, and mute it. Make sure not to keep a reference + # to it to verify it does not get lost later. Use the layer editor + # command to mute it, as it is the one responsible to hold onto the + # anonymous layer. + anonLayer = usdUtils.addNewLayerToStage(self.stage, anonymous=True) + anonLayerId = anonLayer.identifier + mel.eval('mayaUsdLayerEditor -edit -muteLayer 1 "%s" "%s"' % (self.proxyShapePathStr, anonLayerId)) + anonLayer = None + + # Since this is a brand new prim, it should not have variant sets. + primTestDefault = self.stage.DefinePrim('/Test_Default', 'Xform') + primPathStr = self.proxyShapePathStr + ',/Test_Default' + self.assertFalse(primTestDefault.HasVariantSets()) + + mayaRefPrim = mayaUsdAddMayaReference.createMayaReferencePrim( + primPathStr, + self.mayaSceneStr, + self.kDefaultNamespace, + mayaAutoEdit=True) + + # The prim should not have any variant set. + self.assertFalse(primTestDefault.HasVariantSets()) + + # Verify that a Maya Reference prim was created. + self.assertTrue(mayaRefPrim.IsValid()) + self.assertEqual(str(mayaRefPrim.GetName()), kDefaultPrimName) + self.assertEqual(mayaRefPrim, primTestDefault.GetChild(kDefaultPrimName)) + self.assertTrue(mayaRefPrim.GetPrimTypeInfo().GetTypeName(), 'MayaReference') + + attr = mayaRefPrim.GetAttribute('mayaAutoEdit') + self.assertTrue(attr.IsValid()) + self.assertEqual(attr.Get(), True) + + # Discard Maya edits. + aMayaItem = ufe.GlobalSelection.get().front() + aMayaPath = aMayaItem.path() + aMayaPathStr = ufe.PathString.string(aMayaPath) + with mayaUsd.lib.OpUndoItemList(): + self.assertTrue(mayaUsd.lib.PrimUpdaterManager.discardEdits(aMayaPathStr)) + + # Verify that the auto-edit has been turned off + attr = mayaRefPrim.GetAttribute('mayaAutoEdit') + self.assertTrue(attr.IsValid()) + self.assertEqual(attr.Get(), False) + + # Verify that the muted layer stil exists. + layerIdentifiers = [layer.identifier for layer in Sdf.Layer.GetLoadedLayers()] + self.assertIn(anonLayerId, layerIdentifiers) + + def testEditAndMergeMayaRef(self): '''Test editing then merge a Maya Reference. diff --git a/test/testUtils/usdUtils.py b/test/testUtils/usdUtils.py index 8074448501..68e967dfd8 100644 --- a/test/testUtils/usdUtils.py +++ b/test/testUtils/usdUtils.py @@ -210,7 +210,46 @@ def createDuoXformScene(): aXlateOp, aXlation, aUsdUfePathStr, aUsdUfePath, aUsdItem, bXlateOp, bXlation, bUsdUfePathStr, bUsdUfePath, bUsdItem) -def createLayeredStage(layersCount = 3): +def addNewLayersToLayer(parentLayer, layersCount=1, anonymous=False): + ''' + Create multiple new layers under the given layer. + Return the list of new layers. + ''' + createdLayers = [] + for i in range(layersCount): + if anonymous: + newLayer = Sdf.Layer.CreateAnonymous() + else: + newLayerName = 'Layer_%d' % (i+1) + usdFormat = Sdf.FileFormat.FindByExtension('usd') + newLayer = Sdf.Layer.New(usdFormat, newLayerName) + parentLayer.subLayerPaths.append(newLayer.identifier) + createdLayers.append(newLayer) + parentLayer = newLayer + return createdLayers + +def addNewLayerToLayer(parentLayer, anonymous=False): + ''' + Create a new layer under the given layer. + Return the new layer. + ''' + return addNewLayersToLayer(parentLayer, 1, anonymous)[0] + +def addNewLayersToStage(stage, layersCount=1, anonymous=False): + ''' + Create multiple new layers under the root layer of a stage. + Return the list of new layers. + ''' + return addNewLayersToLayer(stage.GetRootLayer(), layersCount, anonymous) + +def addNewLayerToStage(stage, anonymous=False): + ''' + Create a new layer under the root layer of a stage. + Return the new layer. + ''' + return addNewLayersToStage(stage, 1, anonymous)[0] + +def createLayeredStage(layersCount = 3, anonymous=False): '''Create a stage with multiple layers, by default 3 extra layers: Returns a tuple of: @@ -222,15 +261,8 @@ def createLayeredStage(layersCount = 3): (psPathStr, psPath, ps) = createSimpleStage() stage = mayaUsd.lib.GetPrim(psPathStr).GetStage() - layer = stage.GetRootLayer() - layers = [layer] - for i in range(layersCount): - newLayerName = 'Layer_%d' % (i+1) - usdFormat = Sdf.FileFormat.FindByExtension('usd') - newLayer = Sdf.Layer.New(usdFormat, newLayerName) - layer.subLayerPaths.append(newLayer.identifier) - layer = newLayer - layers.append(layer) + rootLayer = stage.GetRootLayer() + layers = [rootLayer] + addNewLayersToLayer(rootLayer, layersCount, anonymous) return (psPathStr, psPath, ps, layers) From eb5a5ae25a6e5b9ce89834bbfde078042079081f Mon Sep 17 00:00:00 2001 From: Pierre Baillargeon Date: Tue, 12 Sep 2023 15:50:59 -0400 Subject: [PATCH 2/2] EMSUSD-277 remove muted layers recusrsively. Also, improve comments explaining why we add and remove muted layers. --- lib/mayaUsd/commands/layerEditorCommand.cpp | 8 ++++---- lib/mayaUsd/utils/layerMuting.cpp | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/lib/mayaUsd/commands/layerEditorCommand.cpp b/lib/mayaUsd/commands/layerEditorCommand.cpp index 0fc27d2b4d..95152e019e 100644 --- a/lib/mayaUsd/commands/layerEditorCommand.cpp +++ b/lib/mayaUsd/commands/layerEditorCommand.cpp @@ -618,9 +618,9 @@ class MuteLayer : public BaseCmd restoreSelection(); } - // we perfer not holding to pointers needlessly, but we need to hold on to the layer if we - // mute it otherwise usd will let go of it and its modifications, and any dirty children - // will also be lost + // We prefer not holding to pointers needlessly, but we need to hold on + // to the muted layer. OpenUSD let go of muted layers, so anonymous + // layers and any dirty children would be lost if not explicitly held on. addMutedLayer(layer); return true; } @@ -640,7 +640,7 @@ class MuteLayer : public BaseCmd stage->MuteLayer(layer->GetIdentifier()); } - // we can release the pointer + // We can release the now unmuted layers. removeMutedLayer(layer); return true; } diff --git a/lib/mayaUsd/utils/layerMuting.cpp b/lib/mayaUsd/utils/layerMuting.cpp index 2a787afc57..55a837629b 100644 --- a/lib/mayaUsd/utils/layerMuting.cpp +++ b/lib/mayaUsd/utils/layerMuting.cpp @@ -95,8 +95,25 @@ void addMutedLayer(const PXR_NS::SdfLayerRefPtr& layer) void removeMutedLayer(const PXR_NS::SdfLayerRefPtr& layer) { + if (!layer) + return; + MutedLayers& layers = getMutedLayers(); layers.erase(layer); + + // Stop holding onto sub-layers as well, in case they were previously + // dirty or anonymous. + // + // Note: we don't check the dirty or anonymous status while removing + // in case the status changed. Trying to remove a layer that + // was not held has no consequences. + // + // Note: the GetSubLayerPaths function returns proxies, so we have to + // hold the std::string by value, not reference. + for (const std::string subLayerPath : layer->GetSubLayerPaths()) { + auto subLayer = SdfLayer::FindRelativeToLayer(layer, subLayerPath); + removeMutedLayer(subLayer); + } } void forgetMutedLayers()