From 6c2a7989bd45d764bd8ad124f5ce6b39e4333854 Mon Sep 17 00:00:00 2001 From: Hamed Sabri Date: Mon, 21 Sep 2020 10:05:40 -0400 Subject: [PATCH 01/11] MAYA-105175: Rewrote the rename restriction algorithm from scratch to cover more edge cases. --- lib/mayaUsd/ufe/private/Utils.cpp | 88 +++++++++++++++++-------------- lib/usd/utils/util.cpp | 81 ---------------------------- lib/usd/utils/util.h | 20 ------- test/lib/ufe/testRename.py | 23 -------- 4 files changed, 48 insertions(+), 164 deletions(-) diff --git a/lib/mayaUsd/ufe/private/Utils.cpp b/lib/mayaUsd/ufe/private/Utils.cpp index 4c24c1efa4..80b9bd40a9 100644 --- a/lib/mayaUsd/ufe/private/Utils.cpp +++ b/lib/mayaUsd/ufe/private/Utils.cpp @@ -129,53 +129,61 @@ UsdGeomXformCommonAPI convertToCompatibleCommonAPI(const UsdPrim& prim) void applyCommandRestriction(const UsdPrim& prim, const std::string& commandName) { - // early check to see if a particular node has any specs to contribute - // to the final composed prim. e.g (a node in payload) - if(!MayaUsdUtils::hasSpecs(prim)){ + auto primSpec = MayaUsdUtils::getPrimSpecAtEditTarget(prim); + auto primStack = prim.GetPrimStack(); + std::string layerDisplayName; - auto layers = MayaUsdUtils::layerInCompositionArcsWithSpec(prim); - std::string layerDisplayNames; - for (auto layer : layers) { - layerDisplayNames.append("[" + layer->GetDisplayName() + "]" + ","); + // check to see if there is a spec at the edit target layer. + if (primSpec) + { + for (const auto& spec : primStack) + { + // skip if the spec already exist + if (primSpec == spec && spec->GetSpecifier() == SdfSpecifierDef && primSpec->GetLayer() == spec->GetLayer()) { + continue; + } + + // break out if we are dealing with an over that has a reference. + if(spec->GetSpecifier() == SdfSpecifierOver && spec->HasReferences()) { + break; + } + + // if the over exist in the session layer + if (spec->GetSpecifier() == SdfSpecifierOver) { + auto sessionLayer = prim.GetStage()->GetSessionLayer(); + if (sessionLayer == spec->GetLayer()){ + layerDisplayName.append("[" + spec->GetLayer()->GetDisplayName() + "]" + ","); + } + } + + // if the def primspec is in another layer other than current stage's local layer. + if (spec->GetSpecifier() == SdfSpecifierDef && primSpec->GetLayer() != spec->GetLayer()) { + layerDisplayName.append("[" + spec->GetLayer()->GetDisplayName() + "]" + ","); + break; + } } - layerDisplayNames.pop_back(); - std::string err = TfStringPrintf("Cannot %s [%s]. It does not make any contributions in the current layer " - "because its specs are in an external composition arc. Please open %s to make direct edits.", - commandName.c_str(), - prim.GetName().GetString().c_str(), - layerDisplayNames.c_str()); - throw std::runtime_error(err.c_str()); - } - // if the current layer doesn't have any contributions - if (!MayaUsdUtils::doesEditTargetLayerContribute(prim)) { - auto strongestContributingLayer = MayaUsdUtils::strongestContributingLayer(prim); - std::string err = TfStringPrintf("Cannot %s [%s]. It is defined on another layer. Please set [%s] as the target layer to proceed.", - commandName.c_str(), - prim.GetName().GetString().c_str(), - strongestContributingLayer->GetDisplayName().c_str()); - throw std::runtime_error(err.c_str()); + if(!layerDisplayName.empty()) + { + layerDisplayName.pop_back(); + std::string err = TfStringPrintf("Cannot %s [%s]. It is defined on another layer. Please set %s as the target layer to proceed.", + commandName.c_str(), + prim.GetName().GetString().c_str(), + layerDisplayName.c_str()); + throw std::runtime_error(err.c_str()); + } } else { - auto layers = MayaUsdUtils::layersWithContribution(prim); - // if we have more than 2 layers that contributes to the final composed prim - if (layers.size() > 1) { - std::string layerDisplayNames; - - // skip the the first arc which is PcpArcTypeRoot - // we are interested in all the arcs after root - std::for_each(std::next(layers.begin()),layers.end(), [&](const auto& it) { - layerDisplayNames.append("[" + it->GetDisplayName() + "]" + ","); - }); - - layerDisplayNames.pop_back(); - std::string err = TfStringPrintf("Cannot %s [%s]. It has definitions or opinions on other layers. Opinions exist in %s", - commandName.c_str(), - prim.GetName().GetString().c_str(), - layerDisplayNames.c_str()); - throw std::runtime_error(err.c_str()); + for (const auto& spec : primStack) { + layerDisplayName.append("[" + spec->GetLayer()->GetDisplayName() + "]" + ","); } + layerDisplayName.pop_back(); + std::string err = TfStringPrintf("Cannot %s [%s]. It is defined on another layer. Please set %s as the target layer to proceed.", + commandName.c_str(), + prim.GetName().GetString().c_str(), + layerDisplayName.c_str()); + throw std::runtime_error(err.c_str()); } } diff --git a/lib/usd/utils/util.cpp b/lib/usd/utils/util.cpp index 7ba7c15632..80cc23f025 100644 --- a/lib/usd/utils/util.cpp +++ b/lib/usd/utils/util.cpp @@ -93,53 +93,6 @@ defPrimSpecLayer(const UsdPrim& prim) return defLayer; } -std::vector -layersWithContribution(const UsdPrim& prim) -{ - UsdPrimCompositionQuery query(prim); - - std::vector layersWithContribution; - - for (const auto& arc : query.GetCompositionArcs()) { - layersWithContribution.emplace_back(arc.GetTargetNode().GetLayerStack()->GetIdentifier().rootLayer); - } - - return layersWithContribution; -} - -bool -doesEditTargetLayerContribute(const UsdPrim& prim) -{ - auto editTarget = prim.GetStage()->GetEditTarget(); - auto layer = editTarget.GetLayer(); - auto primSpec = layer->GetPrimAtPath(prim.GetPath()); - - // to know whether the target layer can contribute to the - // final composed prim, there must be a primSpec for that prim - if (!primSpec) { - return false; - } - - return true; -} - -SdfLayerHandle -strongestContributingLayer(const UsdPrim& prim) -{ - SdfLayerHandle targetLayer; - auto layerStack = prim.GetStage()->GetLayerStack(); - for (auto layer : layerStack) - { - // to know whether the target layer can contribute to the - // final composed prim, there must be a primSpec for that prim - auto primSpec = layer->GetPrimAtPath(prim.GetPath()); - if (primSpec) { - targetLayer = layer; - break; - } - } - return targetLayer; -} SdfPrimSpecHandle getPrimSpecAtEditTarget(const UsdPrim& prim) @@ -165,40 +118,6 @@ isInternalReference(const SdfPrimSpecHandle& primSpec) return isInternalRef; } -bool -hasSpecs(const UsdPrim& prim) -{ - bool found{true}; - - UsdPrimCompositionQuery query(prim); - - for (const auto& compQueryArc : query.GetCompositionArcs()) { - if (!compQueryArc.GetTargetNode().HasSpecs()) { - found = false; - break; - } - } - - return found; -} - -std::vector -layerInCompositionArcsWithSpec(const UsdPrim& prim) -{ - UsdPrimCompositionQuery query(prim); - - std::vector layersWithContribution; - - for (const auto& compQueryArc : query.GetCompositionArcs()) { - if (compQueryArc.GetTargetNode().HasSpecs()) { - layersWithContribution.emplace_back( - compQueryArc.GetTargetNode().GetLayerStack()->GetIdentifier().rootLayer); - } - } - - return layersWithContribution; -} - void printCompositionQuery(const UsdPrim& prim, std::ostream& os) { diff --git a/lib/usd/utils/util.h b/lib/usd/utils/util.h index c45f3d7c03..348c6e01a8 100644 --- a/lib/usd/utils/util.h +++ b/lib/usd/utils/util.h @@ -29,18 +29,6 @@ namespace MayaUsdUtils { MAYA_USD_UTILS_PUBLIC SdfLayerHandle defPrimSpecLayer(const UsdPrim&); - //! Return a list of layers in no strength order that can contribute to the argument prim. - MAYA_USD_UTILS_PUBLIC - std::vector layersWithContribution(const UsdPrim&); - - //! Check if a layer has any contributions towards the argument prim. - MAYA_USD_UTILS_PUBLIC - bool doesEditTargetLayerContribute(const UsdPrim&); - - //! Return the strongest layer that can contribute to the argument prim. - MAYA_USD_UTILS_PUBLIC - SdfLayerHandle strongestContributingLayer(const UsdPrim&); - //! Return a PrimSpec for the argument prim in the layer containing the stage's current edit target. MAYA_USD_UTILS_PUBLIC SdfPrimSpecHandle getPrimSpecAtEditTarget(const UsdPrim&); @@ -49,14 +37,6 @@ namespace MayaUsdUtils { MAYA_USD_UTILS_PUBLIC bool isInternalReference(const SdfPrimSpecHandle&); - //! Returns true if the target node has any specs to contribute to the composed prim. - MAYA_USD_UTILS_PUBLIC - bool hasSpecs(const UsdPrim&); - - //! Returns the layer in composition arc where HasSpecs is set to true - MAYA_USD_UTILS_PUBLIC - std::vector layerInCompositionArcsWithSpec(const UsdPrim& prim); - //! Convenience function for printing the list of queried composition arcs in order. MAYA_USD_UTILS_PUBLIC void printCompositionQuery(const UsdPrim&, std::ostream&); diff --git a/test/lib/ufe/testRename.py b/test/lib/ufe/testRename.py index 1b642c4830..ba58b7ca4f 100644 --- a/test/lib/ufe/testRename.py +++ b/test/lib/ufe/testRename.py @@ -270,29 +270,6 @@ def testRenameRestrictionSameLayerDef(self): newName = 'Ball_35_Renamed' cmds.rename(newName) - def testRenameRestrictionOtherLayerOpinions(self): - '''Restrict renaming USD node. Cannot rename a prim with definitions or opinions on other layers.''' - - # select a USD object. - mayaPathSegment = mayaUtils.createUfePathSegment('|world|transform1|proxyShape1') - usdPathSegment = usdUtils.createUfePathSegment('/Room_set/Props/Ball_35') - ball35Path = ufe.Path([mayaPathSegment, usdPathSegment]) - ball35Item = ufe.Hierarchy.createItem(ball35Path) - - ufe.GlobalSelection.get().append(ball35Item) - - # get the USD stage - stage = mayaUsd.ufe.getStage(str(mayaPathSegment)) - - # set the edit target to Assembly_room_set.usda - stage.SetEditTarget(stage.GetLayerStack()[2]) - self.assertEqual(stage.GetEditTarget().GetLayer().GetDisplayName(), "Assembly_room_set.usda") - - # expect the exception happens - with self.assertRaises(RuntimeError): - newName = 'Ball_35_Renamed' - cmds.rename(newName) - def testRenameRestrictionHasSpecs(self): '''Restrict renaming USD node. Cannot rename a node that doesn't contribute to the final composed prim''' From e50457c2d84272d6594d9367f265092aa22f2a75 Mon Sep 17 00:00:00 2001 From: Hamed Sabri Date: Mon, 21 Sep 2020 13:25:19 -0400 Subject: [PATCH 02/11] Remove redundant check to distinguish if the "over" specifier is in the session layer. --- lib/mayaUsd/ufe/private/Utils.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/mayaUsd/ufe/private/Utils.cpp b/lib/mayaUsd/ufe/private/Utils.cpp index 80b9bd40a9..8dcfd43ead 100644 --- a/lib/mayaUsd/ufe/private/Utils.cpp +++ b/lib/mayaUsd/ufe/private/Utils.cpp @@ -150,10 +150,7 @@ void applyCommandRestriction(const UsdPrim& prim, const std::string& commandName // if the over exist in the session layer if (spec->GetSpecifier() == SdfSpecifierOver) { - auto sessionLayer = prim.GetStage()->GetSessionLayer(); - if (sessionLayer == spec->GetLayer()){ - layerDisplayName.append("[" + spec->GetLayer()->GetDisplayName() + "]" + ","); - } + layerDisplayName.append("[" + spec->GetLayer()->GetDisplayName() + "]" + ","); } // if the def primspec is in another layer other than current stage's local layer. From a9226cdc896a944dff927d2b8198e6d0d74ed4e1 Mon Sep 17 00:00:00 2001 From: Hamed Sabri Date: Mon, 21 Sep 2020 13:37:15 -0400 Subject: [PATCH 03/11] Fix testParentCmd test. If parent prim is the pseudo-root, no def primSpec will be found and instead USD returns an "over". --- lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp b/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp index 9077086a95..c63f1e5ff2 100644 --- a/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp +++ b/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp @@ -67,10 +67,6 @@ UsdUndoInsertChildCommand::UsdUndoInsertChildCommand(const UsdSceneItem::Ptr& pa const auto& childPrim = child->prim(); const auto& parentPrim = parent->prim(); - // Apply restriction rules - ufe::applyCommandRestriction(childPrim, "reparent"); - ufe::applyCommandRestriction(parentPrim, "reparent"); - // First, check if we need to rename the child. const auto& childName = uniqueChildName(parent, child->path()); @@ -94,6 +90,12 @@ UsdUndoInsertChildCommand::UsdUndoInsertChildCommand(const UsdSceneItem::Ptr& pa _parentLayer = parentPrim.IsPseudoRoot() ? parent->prim().GetStage()->GetEditTarget().GetLayer() : MayaUsdUtils::defPrimSpecLayer(parentPrim); + + // Apply restriction rules + if(!parentPrim.IsPseudoRoot()){ + ufe::applyCommandRestriction(childPrim, "reparent"); + ufe::applyCommandRestriction(parentPrim, "reparent"); + } } UsdUndoInsertChildCommand::~UsdUndoInsertChildCommand() From 52d7585e018d6bf1bfcbe64c9c6795cd4ea9ccfb Mon Sep 17 00:00:00 2001 From: Hamed Sabri Date: Mon, 21 Sep 2020 17:28:29 -0400 Subject: [PATCH 04/11] MAYA-105175: Simplify the logic to avoid code redundancy. --- lib/mayaUsd/ufe/private/Utils.cpp | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/lib/mayaUsd/ufe/private/Utils.cpp b/lib/mayaUsd/ufe/private/Utils.cpp index 8dcfd43ead..8e8867308b 100644 --- a/lib/mayaUsd/ufe/private/Utils.cpp +++ b/lib/mayaUsd/ufe/private/Utils.cpp @@ -134,9 +134,12 @@ void applyCommandRestriction(const UsdPrim& prim, const std::string& commandName std::string layerDisplayName; // check to see if there is a spec at the edit target layer. - if (primSpec) + for (const auto& spec : primStack) { - for (const auto& spec : primStack) + if (primSpec != spec) { + layerDisplayName.append("[" + spec->GetLayer()->GetDisplayName() + "]" + ","); + } + else { // skip if the spec already exist if (primSpec == spec && spec->GetSpecifier() == SdfSpecifierDef && primSpec->GetLayer() == spec->GetLayer()) { @@ -159,22 +162,10 @@ void applyCommandRestriction(const UsdPrim& prim, const std::string& commandName break; } } - - if(!layerDisplayName.empty()) - { - layerDisplayName.pop_back(); - std::string err = TfStringPrintf("Cannot %s [%s]. It is defined on another layer. Please set %s as the target layer to proceed.", - commandName.c_str(), - prim.GetName().GetString().c_str(), - layerDisplayName.c_str()); - throw std::runtime_error(err.c_str()); - } } - else + + if(!layerDisplayName.empty()) { - for (const auto& spec : primStack) { - layerDisplayName.append("[" + spec->GetLayer()->GetDisplayName() + "]" + ","); - } layerDisplayName.pop_back(); std::string err = TfStringPrintf("Cannot %s [%s]. It is defined on another layer. Please set %s as the target layer to proceed.", commandName.c_str(), From 92f1c042018cfd19d32cbc3c73ecf5b710ec645b Mon Sep 17 00:00:00 2001 From: Hamed Sabri Date: Mon, 21 Sep 2020 18:54:41 -0400 Subject: [PATCH 05/11] Minor clean up: - remove unnecessary primSpec->GetLayer() == spec->GetLayer() check - comment clean up --- lib/mayaUsd/ufe/private/Utils.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/mayaUsd/ufe/private/Utils.cpp b/lib/mayaUsd/ufe/private/Utils.cpp index 8e8867308b..c7b36f0a45 100644 --- a/lib/mayaUsd/ufe/private/Utils.cpp +++ b/lib/mayaUsd/ufe/private/Utils.cpp @@ -142,11 +142,11 @@ void applyCommandRestriction(const UsdPrim& prim, const std::string& commandName else { // skip if the spec already exist - if (primSpec == spec && spec->GetSpecifier() == SdfSpecifierDef && primSpec->GetLayer() == spec->GetLayer()) { + if (primSpec == spec && spec->GetSpecifier() == SdfSpecifierDef) { continue; } - // break out if we are dealing with an over that has a reference. + // break out if this an over that has a reference. if(spec->GetSpecifier() == SdfSpecifierOver && spec->HasReferences()) { break; } From 8038469a6f4a855df28e50835d80ac072de574f0 Mon Sep 17 00:00:00 2001 From: Hamed Sabri Date: Mon, 21 Sep 2020 21:28:56 -0400 Subject: [PATCH 06/11] MAYA-105175: minor fix in the logic: Break out of loop if the spec already exist. --- lib/mayaUsd/ufe/private/Utils.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/mayaUsd/ufe/private/Utils.cpp b/lib/mayaUsd/ufe/private/Utils.cpp index c7b36f0a45..8841b991ad 100644 --- a/lib/mayaUsd/ufe/private/Utils.cpp +++ b/lib/mayaUsd/ufe/private/Utils.cpp @@ -141,12 +141,12 @@ void applyCommandRestriction(const UsdPrim& prim, const std::string& commandName } else { - // skip if the spec already exist + // break out if the spec already exist if (primSpec == spec && spec->GetSpecifier() == SdfSpecifierDef) { - continue; + break; } - // break out if this an over that has a reference. + // break out if this is an over that has a reference. if(spec->GetSpecifier() == SdfSpecifierOver && spec->HasReferences()) { break; } From 5adec49739bc011226bd182646f2d803f6083065 Mon Sep 17 00:00:00 2001 From: Hamed Sabri Date: Mon, 21 Sep 2020 22:42:45 -0400 Subject: [PATCH 07/11] Covering more edge cases. --- lib/mayaUsd/ufe/private/Utils.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/mayaUsd/ufe/private/Utils.cpp b/lib/mayaUsd/ufe/private/Utils.cpp index 8841b991ad..c43fed5c12 100644 --- a/lib/mayaUsd/ufe/private/Utils.cpp +++ b/lib/mayaUsd/ufe/private/Utils.cpp @@ -133,7 +133,7 @@ void applyCommandRestriction(const UsdPrim& prim, const std::string& commandName auto primStack = prim.GetPrimStack(); std::string layerDisplayName; - // check to see if there is a spec at the edit target layer. + // iterate over the prim stack, starting at the highest-priority layer. for (const auto& spec : primStack) { if (primSpec != spec) { @@ -141,12 +141,15 @@ void applyCommandRestriction(const UsdPrim& prim, const std::string& commandName } else { - // break out if the spec already exist - if (primSpec == spec && spec->GetSpecifier() == SdfSpecifierDef) { - break; + // if exists a def primSpec + if (spec->GetSpecifier() == SdfSpecifierDef) { + if(spec->HasReferences()) { + break; + } + continue; } - // break out if this is an over that has a reference. + // if exists an over that has a reference. if(spec->GetSpecifier() == SdfSpecifierOver && spec->HasReferences()) { break; } From eac389ac017edc044f318acf30cfd2561725daa1 Mon Sep 17 00:00:00 2001 From: Hamed Sabri Date: Tue, 22 Sep 2020 08:25:50 -0400 Subject: [PATCH 08/11] Simplify the algorithm further. --- lib/mayaUsd/ufe/private/Utils.cpp | 38 ++++++++++++++----------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/lib/mayaUsd/ufe/private/Utils.cpp b/lib/mayaUsd/ufe/private/Utils.cpp index c43fed5c12..29e01fafc0 100644 --- a/lib/mayaUsd/ufe/private/Utils.cpp +++ b/lib/mayaUsd/ufe/private/Utils.cpp @@ -136,39 +136,35 @@ void applyCommandRestriction(const UsdPrim& prim, const std::string& commandName // iterate over the prim stack, starting at the highest-priority layer. for (const auto& spec : primStack) { + const auto& layerName = spec->GetLayer()->GetDisplayName(); + if (primSpec != spec) { - layerDisplayName.append("[" + spec->GetLayer()->GetDisplayName() + "]" + ","); + layerDisplayName.append("[" + layerName + "]" + ","); } - else - { - // if exists a def primSpec - if (spec->GetSpecifier() == SdfSpecifierDef) { - if(spec->HasReferences()) { + else { + // if exist a primSpec with reference + if(spec->HasReferences()) { + break; + } + + // if exists a def sepc + if (spec->GetSpecifier() == SdfSpecifierDef) { + // if exists a def spec is in another layer other than current stage's local layer. + if(primSpec->GetLayer() != spec->GetLayer()) { + layerDisplayName.append("[" + layerName + "]" + ","); break; } continue; } - // if exists an over that has a reference. - if(spec->GetSpecifier() == SdfSpecifierOver && spec->HasReferences()) { - break; - } - - // if the over exist in the session layer + // if exists an over sepc if (spec->GetSpecifier() == SdfSpecifierOver) { - layerDisplayName.append("[" + spec->GetLayer()->GetDisplayName() + "]" + ","); - } - - // if the def primspec is in another layer other than current stage's local layer. - if (spec->GetSpecifier() == SdfSpecifierDef && primSpec->GetLayer() != spec->GetLayer()) { - layerDisplayName.append("[" + spec->GetLayer()->GetDisplayName() + "]" + ","); - break; + layerDisplayName.append("[" + layerName + "]" + ","); } } } - if(!layerDisplayName.empty()) - { + if(!layerDisplayName.empty()) { layerDisplayName.pop_back(); std::string err = TfStringPrintf("Cannot %s [%s]. It is defined on another layer. Please set %s as the target layer to proceed.", commandName.c_str(), From 2472eeca9c6cef9ef4cb357efb41744487738f50 Mon Sep 17 00:00:00 2001 From: Hamed Sabri Date: Tue, 22 Sep 2020 16:26:40 -0400 Subject: [PATCH 09/11] MAYA-105175: published the algorithm and added proper messaging. --- lib/mayaUsd/ufe/private/Utils.cpp | 49 +++++++++++++++++-------------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/lib/mayaUsd/ufe/private/Utils.cpp b/lib/mayaUsd/ufe/private/Utils.cpp index 29e01fafc0..b0fa9d7294 100644 --- a/lib/mayaUsd/ufe/private/Utils.cpp +++ b/lib/mayaUsd/ufe/private/Utils.cpp @@ -25,6 +25,8 @@ #include +#include + PXR_NAMESPACE_USING_DIRECTIVE MAYAUSD_NS_DEF { @@ -132,43 +134,46 @@ void applyCommandRestriction(const UsdPrim& prim, const std::string& commandName auto primSpec = MayaUsdUtils::getPrimSpecAtEditTarget(prim); auto primStack = prim.GetPrimStack(); std::string layerDisplayName; + std::string message{"It is defined on another layer"}; // iterate over the prim stack, starting at the highest-priority layer. for (const auto& spec : primStack) { const auto& layerName = spec->GetLayer()->GetDisplayName(); - if (primSpec != spec) { - layerDisplayName.append("[" + layerName + "]" + ","); + // skip if there is no primSpec for the selected prim in the current stage's local layer. + if(!primSpec){ + // add "," separator for multiple layers + if(!layerDisplayName.empty()) { layerDisplayName.append(","); } + layerDisplayName.append("[" + layerName + "]"); + continue; } - else { - // if exist a primSpec with reference - if(spec->HasReferences()) { - break; - } - // if exists a def sepc - if (spec->GetSpecifier() == SdfSpecifierDef) { - // if exists a def spec is in another layer other than current stage's local layer. - if(primSpec->GetLayer() != spec->GetLayer()) { - layerDisplayName.append("[" + layerName + "]" + ","); - break; - } - continue; - } + // one reason for skipping the reference is to not clash + // with the over that may be created in the stage's sessioLayer. + // another reason is that one should be able to edit a referenced prim that + // is tagged over/def as long as it has a primSpec in the selected edit target layer. + if(spec->HasReferences()) { + break; + } - // if exists an over sepc - if (spec->GetSpecifier() == SdfSpecifierOver) { - layerDisplayName.append("[" + layerName + "]" + ","); + // if exists a def/over specs + if (spec->GetSpecifier() == SdfSpecifierDef || spec->GetSpecifier() == SdfSpecifierOver) { + // if exists in another layer other than current stage's local layer. + if(primSpec->GetLayer() != spec->GetLayer()) { + layerDisplayName.append("[" + layerName + "]"); + message = "It has a stronger opinion on another layer"; + break; } + continue; } } if(!layerDisplayName.empty()) { - layerDisplayName.pop_back(); - std::string err = TfStringPrintf("Cannot %s [%s]. It is defined on another layer. Please set %s as the target layer to proceed.", + std::string err = TfStringPrintf("Cannot %s [%s]. %s. Please set %s as the target layer to proceed.", commandName.c_str(), - prim.GetName().GetString().c_str(), + prim.GetName().GetString().c_str(), + message.c_str(), layerDisplayName.c_str()); throw std::runtime_error(err.c_str()); } From b10a2b43bb512d56f58dd3d54d9dd6a557bec74b Mon Sep 17 00:00:00 2001 From: Hamed Sabri Date: Tue, 22 Sep 2020 21:14:03 -0400 Subject: [PATCH 10/11] MAYA-105175: clean up some comments. --- lib/mayaUsd/ufe/private/Utils.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/mayaUsd/ufe/private/Utils.cpp b/lib/mayaUsd/ufe/private/Utils.cpp index b0fa9d7294..2ef5468bb2 100644 --- a/lib/mayaUsd/ufe/private/Utils.cpp +++ b/lib/mayaUsd/ufe/private/Utils.cpp @@ -150,16 +150,16 @@ void applyCommandRestriction(const UsdPrim& prim, const std::string& commandName } // one reason for skipping the reference is to not clash - // with the over that may be created in the stage's sessioLayer. + // with the over that may be created in the stage's sessionLayer. // another reason is that one should be able to edit a referenced prim that - // is tagged over/def as long as it has a primSpec in the selected edit target layer. + // either as over/def as long as it has a primSpec in the selected edit target layer. if(spec->HasReferences()) { break; } // if exists a def/over specs if (spec->GetSpecifier() == SdfSpecifierDef || spec->GetSpecifier() == SdfSpecifierOver) { - // if exists in another layer other than current stage's local layer. + // if spec exists in another layer ( e.g sessionLayer or layer other than stage's local layers ). if(primSpec->GetLayer() != spec->GetLayer()) { layerDisplayName.append("[" + layerName + "]"); message = "It has a stronger opinion on another layer"; From 99851753fda32a2255914324ca0544b324d05c9f Mon Sep 17 00:00:00 2001 From: Hamed Sabri Date: Wed, 23 Sep 2020 11:34:11 -0400 Subject: [PATCH 11/11] MAYA-105175: clean up the logic around pseudo-root. --- lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp | 20 +++++++------------ lib/mayaUsd/ufe/private/Utils.cpp | 9 ++++++++- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp b/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp index c63f1e5ff2..f36d7d7bb5 100644 --- a/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp +++ b/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp @@ -67,6 +67,10 @@ UsdUndoInsertChildCommand::UsdUndoInsertChildCommand(const UsdSceneItem::Ptr& pa const auto& childPrim = child->prim(); const auto& parentPrim = parent->prim(); + // Apply restriction rules + ufe::applyCommandRestriction(childPrim, "reparent"); + ufe::applyCommandRestriction(parentPrim, "reparent"); + // First, check if we need to rename the child. const auto& childName = uniqueChildName(parent, child->path()); @@ -81,21 +85,11 @@ UsdUndoInsertChildCommand::UsdUndoInsertChildCommand(const UsdSceneItem::Ptr& pa _ufeDstPath = parent->path() + Ufe::PathSegment( Ufe::PathComponent(childName), cRtId, cSep); } - _usdDstPath = parent->prim().GetPath().AppendChild(TfToken(childName)); - - _childLayer = child->prim().GetStage()->GetEditTarget().GetLayer(); + _usdDstPath = parentPrim.GetPath().AppendChild(TfToken(childName)); - // If parent prim is the pseudo-root, no def primSpec will be found, so - // just use the edit target layer. - _parentLayer = parentPrim.IsPseudoRoot() - ? parent->prim().GetStage()->GetEditTarget().GetLayer() - : MayaUsdUtils::defPrimSpecLayer(parentPrim); + _childLayer = childPrim.GetStage()->GetEditTarget().GetLayer(); - // Apply restriction rules - if(!parentPrim.IsPseudoRoot()){ - ufe::applyCommandRestriction(childPrim, "reparent"); - ufe::applyCommandRestriction(parentPrim, "reparent"); - } + _parentLayer = parentPrim.GetStage()->GetEditTarget().GetLayer(); } UsdUndoInsertChildCommand::~UsdUndoInsertChildCommand() diff --git a/lib/mayaUsd/ufe/private/Utils.cpp b/lib/mayaUsd/ufe/private/Utils.cpp index 2ef5468bb2..9f996da57a 100644 --- a/lib/mayaUsd/ufe/private/Utils.cpp +++ b/lib/mayaUsd/ufe/private/Utils.cpp @@ -131,13 +131,20 @@ UsdGeomXformCommonAPI convertToCompatibleCommonAPI(const UsdPrim& prim) void applyCommandRestriction(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; + } + auto primSpec = MayaUsdUtils::getPrimSpecAtEditTarget(prim); auto primStack = prim.GetPrimStack(); std::string layerDisplayName; std::string message{"It is defined on another layer"}; // iterate over the prim stack, starting at the highest-priority layer. - for (const auto& spec : primStack) + for (const auto& spec : primStack) { const auto& layerName = spec->GetLayer()->GetDisplayName();