Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MAYA-105175: Rewrote the rename restriction algorithm from scratch to cover more edge cases. #786

Merged
merged 11 commits into from
Sep 24, 2020
10 changes: 3 additions & 7 deletions lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +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));
_usdDstPath = parentPrim.GetPath().AppendChild(TfToken(childName));

_childLayer = child->prim().GetStage()->GetEditTarget().GetLayer();
_childLayer = childPrim.GetStage()->GetEditTarget().GetLayer();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ppt-adsk clean up the legacy old code for getting the appropriate layer for _parentLayer .

ufe::applyCommandRestriction in line 71 and 72 will always put us in a position to use appropriate layer to use for performing the parent operation.


// 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);
_parentLayer = parentPrim.GetStage()->GetEditTarget().GetLayer();
}

UsdUndoInsertChildCommand::~UsdUndoInsertChildCommand()
Expand Down
89 changes: 48 additions & 41 deletions lib/mayaUsd/ufe/private/Utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@

#include <mayaUsdUtils/util.h>

#include <iostream>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just noticed I left out to clean up this iostream. Will have it remove in another PR.


PXR_NAMESPACE_USING_DIRECTIVE

MAYAUSD_NS_DEF {
Expand Down Expand Up @@ -129,54 +131,59 @@ 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 layers = MayaUsdUtils::layerInCompositionArcsWithSpec(prim);
std::string layerDisplayNames;
for (auto layer : layers) {
layerDisplayNames.append("[" + layer->GetDisplayName() + "]" + ",");
}
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());
// 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;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ppt-adsk per out offline discussion, PseudoRoot() is a special case and should be treated differently.

After some thinking, it makes more sense to simply return early if we are dealing with the root layer.

// 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());
}
else
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)
{
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;
const auto& layerName = spec->GetLayer()->GetDisplayName();

// 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() + "]" + ",");
});
// 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;
}

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());
// one reason for skipping the reference is to not clash
// 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
// 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 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";
break;
}
continue;
}
}

if(!layerDisplayName.empty()) {
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(),
message.c_str(),
layerDisplayName.c_str());
throw std::runtime_error(err.c_str());
}
Copy link
Contributor Author

@HamedSabri-adsk HamedSabri-adsk Sep 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ppt-adsk

  • polished the algorithm slightly to make it easier to reason about the code and added appropriate comments.
  • added proper messaging. we now only need to deal with two messages ( no more fancy words ) :
  1. Cannot rename something because It is defined on another layer
  2. Cannot rename something because It has a stronger opinion on another layer

please see 2472eec .

Also ignore my typo in the commit message: I meant "polished" not "published". Sigh..............

}
Copy link
Contributor Author

@HamedSabri-adsk HamedSabri-adsk Sep 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This algorithm heavily relies on using "Prim's PrimStack" rather than "Stage's LayerStack" which makes it much easier to interrogate about the SdfPrimSpec(s) in different composition arcs.


//------------------------------------------------------------------------------
Expand Down
81 changes: 0 additions & 81 deletions lib/usd/utils/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,53 +93,6 @@ defPrimSpecLayer(const UsdPrim& prim)
return defLayer;
}

std::vector<SdfLayerHandle>
layersWithContribution(const UsdPrim& prim)
{
UsdPrimCompositionQuery query(prim);

std::vector<SdfLayerHandle> 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)
Expand All @@ -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<SdfLayerHandle>
layerInCompositionArcsWithSpec(const UsdPrim& prim)
{
UsdPrimCompositionQuery query(prim);

std::vector<SdfLayerHandle> 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)
{
Expand Down
20 changes: 0 additions & 20 deletions lib/usd/utils/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<SdfLayerHandle> 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&);
Expand All @@ -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<SdfLayerHandle> layerInCompositionArcsWithSpec(const UsdPrim& prim);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleaned up the usd utility functions. We no longer use them.

UsdPrimCompositionQuery is a nice utility but doesn't deal with SubLayer comp arcs. For the sake of this task, I stopped using it altogether.

//! Convenience function for printing the list of queried composition arcs in order.
MAYA_USD_UTILS_PUBLIC
void printCompositionQuery(const UsdPrim&, std::ostream&);
Expand Down
23 changes: 0 additions & 23 deletions test/lib/ufe/testRename.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
ppt-adsk marked this conversation as resolved.
Show resolved Hide resolved

def testRenameRestrictionHasSpecs(self):
'''Restrict renaming USD node. Cannot rename a node that doesn't contribute to the final composed prim'''

Expand Down