Merge pull request #3398 from Autodesk/bailp/EMSUSD-632/prevent-renam…

…ing-root-prim

EMSUSD-632 prevent root metadata changes when not targeting root

lib/usdUfe/python/wrapCommands.cpp

@@ -15,9 +15,11 @@
 //
 #include <usdUfe/ufe/UsdUndoAddPayloadCommand.h>
 #include <usdUfe/ufe/UsdUndoAddReferenceCommand.h>
+#include <usdUfe/ufe/UsdUndoClearDefaultPrimCommand.h>
 #include <usdUfe/ufe/UsdUndoClearPayloadsCommand.h>
 #include <usdUfe/ufe/UsdUndoClearReferencesCommand.h>
 #include <usdUfe/ufe/UsdUndoPayloadCommand.h>
+#include <usdUfe/ufe/UsdUndoSetDefaultPrimCommand.h>
 #include <usdUfe/ufe/UsdUndoToggleActiveCommand.h>
 #include <usdUfe/ufe/UsdUndoToggleInstanceableCommand.h>
 
@@ -71,10 +73,37 @@ UsdUfe::UsdUndoUnloadPayloadCommand* UnloadPayloadCommandInit(const PXR_NS::UsdP
     return new UsdUfe::UsdUndoUnloadPayloadCommand(prim);
 }
 
+UsdUfe::UsdUndoClearDefaultPrimCommand*
+ClearDefaultPrimCommandInit(const PXR_NS::UsdStageRefPtr& stage)
+{
+    return new UsdUfe::UsdUndoClearDefaultPrimCommand(stage);
+}
+
+UsdUfe::UsdUndoSetDefaultPrimCommand* SetDefaultPrimCommandInit(const PXR_NS::UsdPrim& prim)
+{
+    return new UsdUfe::UsdUndoSetDefaultPrimCommand(prim);
+}
+
 } // namespace
 
 void wrapCommands()
 {
+    {
+        using This = UsdUfe::UsdUndoClearDefaultPrimCommand;
+        class_<This, boost::noncopyable>("ClearDefaultPrimCommand", no_init)
+            .def("__init__", make_constructor(ClearDefaultPrimCommandInit))
+            .def("execute", &UsdUfe::UsdUndoClearDefaultPrimCommand::execute)
+            .def("undo", &UsdUfe::UsdUndoClearDefaultPrimCommand::undo)
+            .def("redo", &UsdUfe::UsdUndoClearDefaultPrimCommand::redo);
+    }
+    {
+        using This = UsdUfe::UsdUndoSetDefaultPrimCommand;
+        class_<This, boost::noncopyable>("SetDefaultPrimCommand", no_init)
+            .def("__init__", make_constructor(SetDefaultPrimCommandInit))
+            .def("execute", &UsdUfe::UsdUndoSetDefaultPrimCommand::execute)
+            .def("undo", &UsdUfe::UsdUndoSetDefaultPrimCommand::undo)
+            .def("redo", &UsdUfe::UsdUndoSetDefaultPrimCommand::redo);
+    }
     {
         using This = UsdUfe::UsdUndoAddPayloadCommand;
         class_<This, boost::noncopyable>("AddPayloadCommand", no_init)

lib/usdUfe/ufe/UsdUndoClearDefaultPrimCommand.cpp

@@ -27,28 +27,28 @@ namespace USDUFE_NS_DEF {
 
 UsdUndoClearDefaultPrimCommand::UsdUndoClearDefaultPrimCommand(const UsdPrim& prim)
     : Ufe::UndoableCommand()
-    , _prim(prim)
+    , _stage(prim.GetStage())
+{
+}
+
+UsdUndoClearDefaultPrimCommand::UsdUndoClearDefaultPrimCommand(const PXR_NS::UsdStageRefPtr& stage)
+    : _stage(stage)
 {
 }
 
 UsdUndoClearDefaultPrimCommand::~UsdUndoClearDefaultPrimCommand() { }
 
 void UsdUndoClearDefaultPrimCommand::execute()
 {
-    UsdUndoBlock undoBlock(&_undoableItem);
+    if (!_stage)
+        return;
 
-    PXR_NS::UsdStageWeakPtr stage = _prim.GetStage();
+    // Check if the default prim can be cleared.
+    applyRootLayerMetadataRestriction(_stage, "clear default prim");
 
-    // Check if the layer selected is the root layer.
-    if (!UsdUfe::isRootLayer(stage)) {
-        TF_WARN(
-            "Stage metadata [defaultPrim] can only be modified when the root layer is targeted "
-            "[%s]",
-            stage->GetRootLayer()->GetDisplayName().c_str());
-        return;
-    }
     // Clear the stage's default prim.
-    stage->ClearDefaultPrim();
+    UsdUndoBlock undoBlock(&_undoableItem);
+    _stage->ClearDefaultPrim();
 }
 
 void UsdUndoClearDefaultPrimCommand::redo() { _undoableItem.redo(); }

lib/usdUfe/ufe/UsdUndoClearDefaultPrimCommand.h

@@ -30,6 +30,7 @@ class USDUFE_PUBLIC UsdUndoClearDefaultPrimCommand : public Ufe::UndoableCommand
 public:
     // Public for std::make_shared() access, use create() instead.
     UsdUndoClearDefaultPrimCommand(const PXR_NS::UsdPrim& prim);
+    UsdUndoClearDefaultPrimCommand(const PXR_NS::UsdStageRefPtr& stage);
     ~UsdUndoClearDefaultPrimCommand() override;
 
     // Delete the copy/move constructors assignment operators.
@@ -38,12 +39,12 @@ class USDUFE_PUBLIC UsdUndoClearDefaultPrimCommand : public Ufe::UndoableCommand
     UsdUndoClearDefaultPrimCommand(UsdUndoClearDefaultPrimCommand&&) = delete;
     UsdUndoClearDefaultPrimCommand& operator=(UsdUndoClearDefaultPrimCommand&&) = delete;
 
-private:
     void execute() override;
     void undo() override;
     void redo() override;
 
-    PXR_NS::UsdPrim _prim;
+private:
+    PXR_NS::UsdStageRefPtr _stage;
 
     UsdUndoableItem _undoableItem;
 

lib/usdUfe/ufe/UsdUndoSetDefaultPrimCommand.cpp

@@ -35,18 +35,15 @@ UsdUndoSetDefaultPrimCommand::~UsdUndoSetDefaultPrimCommand() { }
 
 void UsdUndoSetDefaultPrimCommand::execute()
 {
-    UsdUndoBlock            undoBlock(&_undoableItem);
-    PXR_NS::UsdStageWeakPtr stage = _prim.GetStage();
-
-    // Check if the layer selected is the root layer.
-    if (!UsdUfe::isRootLayer(stage)) {
-        TF_WARN(
-            "Stage metadata [defaultPrim] can only be modified when the root layer is targeted "
-            "[%s]",
-            stage->GetRootLayer()->GetDisplayName().c_str());
+    const PXR_NS::UsdStageRefPtr stage = _prim.GetStage();
+    if (!stage)
         return;
-    }
+
+    // Check if the default prim can be set.
+    applyRootLayerMetadataRestriction(stage, "set default prim");
+
     // Set the stage's default prim to the given prim.
+    UsdUndoBlock undoBlock(&_undoableItem);
     stage->SetDefaultPrim(_prim);
 }
 

lib/usdUfe/ufe/UsdUndoSetDefaultPrimCommand.h

@@ -38,11 +38,11 @@ class USDUFE_PUBLIC UsdUndoSetDefaultPrimCommand : public Ufe::UndoableCommand
     UsdUndoSetDefaultPrimCommand(UsdUndoSetDefaultPrimCommand&&) = delete;
     UsdUndoSetDefaultPrimCommand& operator=(UsdUndoSetDefaultPrimCommand&&) = delete;
 
-private:
     void execute() override;
     void undo() override;
     void redo() override;
 
+private:
     PXR_NS::UsdPrim _prim;
 
     UsdUndoableItem _undoableItem;

lib/usdUfe/ufe/Utils.cpp

@@ -410,6 +410,66 @@ Ufe::BBox3d combineUfeBBox(const Ufe::BBox3d& ufeBBox1, const Ufe::BBox3d& ufeBB
     return combinedBBox;
 }
 
+void applyRootLayerMetadataRestriction(const UsdPrim& prim, const std::string& commandName)
+{
+    // return early if prim is the pseudo-root.
+    // this is a special case and could happen when one tries to drag a prim under the
+    // proxy shape in outliner. Also note if prim is the pseudo-root, no def primSpec will be found.
+    if (prim.IsPseudoRoot()) {
+        return;
+    }
+
+    const auto stage = prim.GetStage();
+    if (!stage)
+        return;
+
+    // If the target layer is the root layer, then the restrictions
+    // do not apply since the edit target is on the layer that contains
+    // the metadata.
+    const SdfLayerHandle targetLayer = stage->GetEditTarget().GetLayer();
+    const SdfLayerHandle rootLayer = stage->GetRootLayer();
+    if (targetLayer == rootLayer)
+        return;
+
+    // Enforce the restriction that we cannot change the default prim
+    // from a layer other than the root layer.
+    if (prim == stage->GetDefaultPrim()) {
+        const std::string layerName = rootLayer->GetDisplayName();
+        const std::string err = TfStringPrintf(
+            "Cannot %s [%s]. This prim is defined as the default prim on [%s]",
+            commandName.c_str(),
+            prim.GetName().GetString().c_str(),
+            layerName.c_str());
+        throw std::runtime_error(err.c_str());
+    }
+}
+
+void applyRootLayerMetadataRestriction(
+    const PXR_NS::UsdStageRefPtr& stage,
+    const std::string&            commandName)
+{
+    if (!stage)
+        return;
+
+    // If the target layer is the root layer, then the restrictions
+    // do not apply since the edit target is on the layer that contains
+    // the metadata.
+    const SdfLayerHandle targetLayer = stage->GetEditTarget().GetLayer();
+    const SdfLayerHandle rootLayer = stage->GetRootLayer();
+    if (targetLayer == rootLayer)
+        return;
+
+    // Enforce the restriction that we cannot change the default prim
+    // from a layer other than the root layer.
+    const std::string layerName = rootLayer->GetDisplayName();
+    const std::string err = TfStringPrintf(
+        "Cannot %s. The stage default prim metadata can only be modified when the root layer [%s] "
+        "is targeted.",
+        commandName.c_str(),
+        layerName.c_str());
+    throw std::runtime_error(err.c_str());
+}
+
 void applyCommandRestriction(
     const UsdPrim&     prim,
     const std::string& commandName,
@@ -439,7 +499,7 @@ void applyCommandRestriction(
     // the target.
     std::string message = allowStronger ? "It is defined on another layer. " : "";
     std::string instructions = allowStronger ? "Please set %s as the target layer to proceed."
-                                             : "It would orphan opinions on the layer %s.";
+                                             : "It would orphan opinions on the layer %s";
 
     // iterate over the prim stack, starting at the highest-priority layer.
     for (const auto& spec : primStack) {
@@ -497,7 +557,7 @@ void applyCommandRestriction(
             if (allowedInStrongerLayer(prim, primStack, sessionLayers, allowStronger))
                 return;
             std::string err = TfStringPrintf(
-                "Cannot %s [%s] because it is defined inside the variant composition arc %s.",
+                "Cannot %s [%s] because it is defined inside the variant composition arc %s",
                 commandName.c_str(),
                 prim.GetName().GetString().c_str(),
                 layerDisplayName.c_str());
@@ -518,6 +578,8 @@ void applyCommandRestriction(
             formattedInstructions.c_str());
         throw std::runtime_error(err.c_str());
     }
+
+    applyRootLayerMetadataRestriction(prim, commandName);
 }
 
 bool applyCommandRestrictionNoThrow(
@@ -800,13 +862,4 @@ Ufe::Selection recreateDescendants(const Ufe::Selection& src, const Ufe::Path& f
     return dst;
 }
 
-bool isRootLayer(const PXR_NS::UsdStageWeakPtr stage)
-{
-    // Check if the layer selected is the root layer.
-    if (stage->GetRootLayer() != stage->GetEditTarget().GetLayer()) {
-        return false;
-    }
-    return true;
-}
-
 } // namespace USDUFE_NS_DEF

lib/usdUfe/ufe/Utils.h

@@ -264,6 +264,16 @@ bool applyCommandRestrictionNoThrow(
     const std::string&     commandName,
     bool                   allowStronger = false);
 
+//! Apply restriction rules for root layer metadata on the given prim
+USDUFE_PUBLIC
+void applyRootLayerMetadataRestriction(const PXR_NS::UsdPrim& prim, const std::string& commandName);
+
+//! Apply restriction rules for root layer metadata on the given stage
+USDUFE_PUBLIC
+void applyRootLayerMetadataRestriction(
+    const PXR_NS::UsdStageRefPtr& stage,
+    const std::string&            commandName);
+
 //! Check if the edit target in the stage is allowed to be changed.
 //! \return True, if the edit target layer in the stage is allowed to be changed
 USDUFE_PUBLIC
@@ -275,7 +285,4 @@ bool isEditTargetLayerModifiable(
 USDUFE_PUBLIC
 Ufe::BBox3d combineUfeBBox(const Ufe::BBox3d& ufeBBox1, const Ufe::BBox3d& ufeBBox2);
 
-USDUFE_PUBLIC
-bool isRootLayer(const PXR_NS::UsdStageWeakPtr prim);
-
 } // namespace USDUFE_NS_DEF

plugin/adsk/scripts/AETemplateHelpers.py

@@ -1,7 +1,8 @@
-import functools
 import os.path
 import maya.cmds as cmds
-import ufe
+import maya.api.OpenMaya as OpenMaya
+import maya.internal.ufeSupport.ufeCmdWrapper as ufeCmd
+import usdUfe
 import mayaUsd.ufe
 import mayaUsd.lib as mayaUsdLib
 import re
@@ -13,10 +14,12 @@ def debugMessage(msg):
     if DEBUG:
         print(msg)
 
+__naturalOrderRE = re.compile(r'([0-9]+)')
+
 def GetAllRootPrimNamesNaturalOrder(proxyShape):
     # Custom comparator key
     def natural_key(item):
-        return [int(s) if s.isdigit() else s.lower() for s in re.split(r'([0-9]+)', item)]
+        return [int(s) if s.isdigit() else s.lower() for s in __naturalOrderRE.split(item)]
     try:
         proxyStage = mayaUsd.ufe.getStage(proxyShape)
         primNames = []
@@ -47,19 +50,26 @@ def GetDefaultPrimName(proxyShape):
 def SetDefaultPrim(proxyShape, primName):
     try:
         proxyStage = mayaUsd.ufe.getStage(proxyShape)
-        if(not primName):
-           proxyStage.ClearDefaultPrim()
-        defautlPrim = None
-        if proxyStage:
-            for prim in proxyStage.TraverseAll():
-                if(primName == prim.GetName()):
-                    defautlPrim = prim
-        if defautlPrim:
-            proxyStage.SetDefaultPrim(defautlPrim)
+        if not proxyStage:
+            return False
+
+        cmd = None
+        if not primName:
+            cmd = usdUfe.ClearDefaultPrimCommand(proxyStage)
+        else:
+            defaultPrim = proxyStage.GetPrimAtPath('/' + primName)
+            if defaultPrim:
+                cmd = usdUfe.SetDefaultPrimCommand(defaultPrim)
+
+        if cmd is None:
+            return False
+
+        ufeCmd.execute(cmd)
+        return True
     except Exception as e:
-        debugMessage('SetDefaultPrim() - Error: %s' % str(e))
-        pass
-    return ''
+        # Note: we do want to tell the user why the set or clear failed.
+        OpenMaya.MGlobal.displayError(str(e))
+        return False
 
 def GetRootLayerName(proxyShape):
     try:

plugin/adsk/scripts/AEmayaUsdProxyShapeBaseTemplate.mel

@@ -20,10 +20,17 @@ global proc string AEmayaUsdProxyShapeInfoChanged(string $input)
 
     // Call python helpers to get the info we want to display.
     string $rootLayer   = `python("import AETemplateHelpers; AETemplateHelpers.GetRootLayerName('" + $fullNodeName + "')")`;
-    string $defaultPrim = `python("import AETemplateHelpers; AETemplateHelpers.GetDefaultPrimName('" + $fullNodeName + "')")`;
-    string $allRootPrims[] = `python("import AETemplateHelpers; AETemplateHelpers.GetAllRootPrimNamesNaturalOrder('" + $fullNodeName + "')")`;
 
     textFieldGrp -edit -text $rootLayer mayaUsdProxyShapeRootLayer;
+    mayaUsd_DefaultPrimInit($fullNodeName);
+
+    return $fullNodeName;
+}
+
+global proc mayaUsd_DefaultPrimInit(string $fullNodeName)
+{
+    string $defaultPrim = `python("import AETemplateHelpers; AETemplateHelpers.GetDefaultPrimName('" + $fullNodeName + "')")`;
+    string $allRootPrims[] = `python("import AETemplateHelpers; AETemplateHelpers.GetAllRootPrimNamesNaturalOrder('" + $fullNodeName + "')")`;
 
     optionMenuGrp -edit -deleteAllItems -changeCommand ("mayaUsd_DefaultPrimChanged(\"" + $fullNodeName + "\"\)") mayaUsdProxyShapeDefaultPrim;
     string $menuName = `optionMenuGrp -q -fullPathName mayaUsdProxyShapeDefaultPrim`;
@@ -43,14 +50,17 @@ global proc string AEmayaUsdProxyShapeInfoChanged(string $input)
         $index = 1;
     }
     optionMenuGrp -e -select $index mayaUsdProxyShapeDefaultPrim;
-
-    return $fullNodeName;
 }
 
 global proc mayaUsd_DefaultPrimChanged(string $fullNodeName)
 {
     string $primName = `optionMenuGrp -q -value mayaUsdProxyShapeDefaultPrim`;
-    python("import AETemplateHelpers; AETemplateHelpers.SetDefaultPrim('" + $fullNodeName + "', primName ='" + $primName + "')");
+    int $result = `python("import AETemplateHelpers; AETemplateHelpers.SetDefaultPrim('" + $fullNodeName + "', primName ='" + $primName + "')")`;
+    // On failure, we re-initialize the UI since the command failed
+    // and thus we need to restore the previous UI state.
+    if ($result == 0) {
+        mayaUsd_DefaultPrimInit($fullNodeName);
+    }
 }
 
 global proc AEmayaUsdProxyShapeInfoReplace(string $input)

test/lib/ufe/CMakeLists.txt

@@ -8,6 +8,7 @@ set(TEST_SCRIPT_FILES
     testChildFilter.py
     testComboCmd.py
     testContextOps.py
+    testDefaultPrimCmds.py
     testDuplicateCmd.py
     testEditRouting.py
     testGroupCmd.py