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

Improve rename logics #483

133 changes: 55 additions & 78 deletions lib/mayaUsd/ufe/UsdUndoRenameCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,36 +42,39 @@ namespace ufe {

UsdUndoRenameCommand::UsdUndoRenameCommand(const UsdSceneItem::Ptr& srcItem, const Ufe::PathComponent& newName)
: Ufe::UndoableCommand()
, _ufeSrcItem(srcItem)
, _ufeDstItem(nullptr)
, _stage(_ufeSrcItem->prim().GetStage())
, _newName(newName.string())
Copy link
Contributor Author

@HamedSabri-adsk HamedSabri-adsk May 6, 2020

Choose a reason for hiding this comment

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

Using initializer list.

{
const UsdPrim& prim = srcItem->prim();
_stage = prim.GetStage();
_ufeSrcItem = srcItem;
_usdSrcPath = prim.GetPath();

// Every call to rename() (through execute(), undo() or redo()) removes
// a prim, which becomes expired. Since USD UFE scene items contain a
// prim, we must recreate them after every call to rename.
_usdDstPath = prim.GetParent().GetPath().AppendChild(TfToken(newName.string()));

_layer = MayaUsdUtils::defPrimSpecLayer(prim);
if (!_layer) {
std::string err = TfStringPrintf("No prim found at %s", prim.GetPath().GetString().c_str());
throw std::runtime_error(err.c_str());
}
const UsdPrim& prim = _stage->GetPrimAtPath(_ufeSrcItem->prim().GetPath());

// if the current layer doesn't have any opinions that affects selected prim
if (!MayaUsdUtils::doesEditTargetLayerHavePrimSpec(prim)) {
auto possibleTargetLayer = MayaUsdUtils::strongestLayerWithPrimSpec(prim);
// if the current layer doesn't have any contributions
if (!MayaUsdUtils::doesEditTargetLayerContribute(prim)) {
auto strongestContributingLayer = MayaUsdUtils::strongestContributingLayer(prim);
std::string err = TfStringPrintf("Cannot rename [%s] defined on another layer. "
"Please set [%s] as the target layer to proceed",
prim.GetName().GetString().c_str(),
possibleTargetLayer->GetDisplayName().c_str());
strongestContributingLayer->GetDisplayName().c_str());
throw std::runtime_error(err.c_str());
}
else
{
auto layers = MayaUsdUtils::layersWithPrimSpec(prim);
// account for internal vs external references
// internal references (references without a file path specified) from the same file
// should be renamable.
if (prim.HasAuthoredReferences()) {
auto primSpec = MayaUsdUtils::getPrimSpecAtEditTarget(_stage, prim);

if(!MayaUsdUtils::isInternalReference(primSpec)) {
std::string err = TfStringPrintf("Unable to rename referenced object [%s]",
prim.GetName().GetString().c_str());
throw std::runtime_error(err.c_str());
}
}

ppt-adsk marked this conversation as resolved.
Show resolved Hide resolved
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;
for (auto layer : layers) {
Expand Down Expand Up @@ -101,79 +104,53 @@ UsdSceneItem::Ptr UsdUndoRenameCommand::renamedItem() const

bool UsdUndoRenameCommand::renameRedo()
{
// Copy the source path using CopySpec, and remove the source.

// We use the source layer as the destination. An alternate workflow
// would be the edit target layer be the destination:
// _layer = _stage->GetEditTarget().GetLayer()
bool status = SdfCopySpec(_layer, _usdSrcPath, _layer, _usdDstPath);
if (status) {
// remove all scene description for the given path and
// its subtree in the current UsdEditTarget
{
UsdEditContext ctx(_stage, _layer);
status = _stage->RemovePrim(_ufeSrcItem->prim().GetPath());
}

if (status) {
// The renamed scene item is a "sibling" of its original name.
_ufeDstItem = createSiblingSceneItem(_ufeSrcItem->path(), _usdDstPath.GetElementString());
const UsdPrim& prim = _stage->GetPrimAtPath(_ufeSrcItem->prim().GetPath());

sendRenameNotification(_ufeDstItem, _ufeSrcItem->path());
}
auto primSpec = MayaUsdUtils::getPrimSpecAtEditTarget(_stage, prim);
if(!primSpec) {
return false;
}
else {
UFE_LOG(std::string("Warning: SdfCopySpec(") +
_usdSrcPath.GetString() + std::string(") failed."));

// set prim's name
// XXX: SetName successfuly returns true but when you examine the _prim.GetName()
// after the rename, the prim name shows the original name HS, 6-May-2020.
bool status = primSpec->SetName(_newName);
if (!status) {
return false;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do an early check for primSpec and SetName result.

@ppt-adsk there is something troubling happening here which I am trying to understand:

On Redo, I get the primspec for _prim object and rename it's name to the _newName. This operation return successful BUT when I query the _prim's name right after, I still get the _prim's original name. Huh???

I then set _ufeDstItem and when I print it's USD name and it gives me the _newName which is expected.


return status;
// the renamed scene item is a "sibling" of its original name.
_ufeDstItem = createSiblingSceneItem(_ufeSrcItem->path(), _newName);
sendRenameNotification(_ufeDstItem, _ufeSrcItem->path());

return true;
ppt-adsk marked this conversation as resolved.
Show resolved Hide resolved
}

bool UsdUndoRenameCommand::renameUndo()
{
// Copy the source path using CopySpec, and remove the source.
bool status = SdfCopySpec(_layer, _usdDstPath, _layer, _usdSrcPath);

if (status) {
// remove all scene description for the given path and
// its subtree in the current UsdEditTarget
{
UsdEditContext ctx(_stage, _layer);
status = _stage->RemovePrim(_usdDstPath);
}
const UsdPrim& prim = _stage->GetPrimAtPath(_ufeDstItem->prim().GetPath());

if (status) {
// create a new prim at _usdSrcPath
auto newPrim = _stage->DefinePrim(_usdSrcPath);

#ifdef UFE_V2_FEATURES_AVAILABLE
UFE_ASSERT_MSG(newPrim, "Invalid prim cannot be inactivated.");
#else
assert(newPrim);
#endif
auto primSpec = MayaUsdUtils::getPrimSpecAtEditTarget(_stage, prim);
if(!primSpec) {
return false;
}

// I shouldn't have to again create a sibling sceneItem here since we already have a valid _ufeSrcItem
// however, I get random crashes if I don't which needs furthur investigation. HS, 6-May-2020.
_ufeSrcItem = createSiblingSceneItem(_ufeDstItem->path(), _usdSrcPath.GetElementString());
// set prim's name
bool status = primSpec->SetName(_ufeSrcItem->prim().GetName());
if (!status) {
return false;
}

sendRenameNotification(_ufeSrcItem, _ufeDstItem->path());
// shouldn't have to again create a sibling sceneItem here since we already have a valid _ufeSrcItem
// however, I get random crashes if I don't which needs furthur investigation. HS, 6-May-2020.
_ufeSrcItem = createSiblingSceneItem(_ufeDstItem->path(), _ufeSrcItem->prim().GetName());
sendRenameNotification(_ufeSrcItem, _ufeDstItem->path());

_ufeDstItem = nullptr;
}
}
else {
UFE_LOG(std::string("Warning: SdfCopySpec(") +
_usdDstPath.GetString() + std::string(") failed."));
}
_ufeDstItem = nullptr;

return status;
return true;
Copy link
Contributor Author

@HamedSabri-adsk HamedSabri-adsk May 6, 2020

Choose a reason for hiding this comment

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

On Undo:
1- Get the SdfPrimSpec to the _ufeDstItem->prim() and check it's validity.
2- Set it's name to _prim's original name: primSpec->SetName(_prim.GetPath().GetElementString());
3- Set _ufeSrcItem and send UFE scene notification
4- Set _ufeDstItem = nullptr

}

//------------------------------------------------------------------------------
// UsdUndoRenameCommand overrides
//------------------------------------------------------------------------------

void UsdUndoRenameCommand::undo()
{
// MAYA-92264: Pixar bug prevents undo from working. Try again with USD
Expand Down
15 changes: 5 additions & 10 deletions lib/mayaUsd/ufe/UsdUndoRenameCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,22 +48,17 @@ class MAYAUSD_CORE_PUBLIC UsdUndoRenameCommand : public Ufe::UndoableCommand
UsdSceneItem::Ptr renamedItem() const;

private:

// UsdUndoRenameCommand overrides
void undo() override;
void redo() override;

bool renameRedo();
bool renameUndo();

UsdStageWeakPtr _stage;
SdfLayerHandle _layer;
void undo() override;
void redo() override;

UsdSceneItem::Ptr _ufeSrcItem;
SdfPath _usdSrcPath;

UsdSceneItem::Ptr _ufeDstItem;
SdfPath _usdDstPath;

UsdStageWeakPtr _stage;
std::string _newName;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simplified member variables. We no longer need _usdSrcPath, _usdDstPath, and _layer.

}; // UsdUndoRenameCommand

Expand Down
51 changes: 37 additions & 14 deletions lib/usd/utils/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
#include <pxr/usd/usd/prim.h>
#include <pxr/usd/usd/stage.h>

#include <vector>
#include <set>

PXR_NAMESPACE_USING_DIRECTIVE

Expand All @@ -46,30 +46,30 @@ defPrimSpecLayer(const UsdPrim& prim)
return defLayer;
}

std::vector<SdfLayerHandle>
layersWithPrimSpec(const UsdPrim& prim)
std::set<SdfLayerHandle>
layersWithContribution(const UsdPrim& prim)
{
// get the list of PrimSpecs that provide opinions for this prim
// ordered from strongest to weakest.
// get the list of all the specs that can
// contribute to the final composed prim
const auto& primStack = prim.GetPrimStack();

std::vector<SdfLayerHandle> layersWithPrimSpec;
std::set<SdfLayerHandle> layersWithContribution;
for (auto primSpec : primStack) {
layersWithPrimSpec.emplace_back(primSpec->GetLayer());
layersWithContribution.insert(primSpec->GetLayer());
}

return layersWithPrimSpec;
return layersWithContribution;
}

bool
doesEditTargetLayerHavePrimSpec(const UsdPrim& prim)
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 contains any opinions that
// affect a particular prim, there must be a primSpec for that prim
// 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;
}
Expand All @@ -78,14 +78,14 @@ doesEditTargetLayerHavePrimSpec(const UsdPrim& prim)
}

SdfLayerHandle
strongestLayerWithPrimSpec(const UsdPrim& prim)
strongestContributingLayer(const UsdPrim& prim)
{
SdfLayerHandle targetLayer;
auto layerStack = prim.GetStage()->GetLayerStack();
for (auto layer : layerStack)
{
// to know whether the target layer contains any opinions that
// affect a particular prim, there must be a primSpec for that prim
// 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;
Expand All @@ -95,4 +95,27 @@ strongestLayerWithPrimSpec(const UsdPrim& prim)
return targetLayer;
}

SdfPrimSpecHandle
getPrimSpecAtEditTarget(UsdStageWeakPtr stage, const UsdPrim& prim)
{
return stage->GetEditTarget().GetPrimSpecForScenePath(prim.GetPath());
}

bool
isInternalReference(const SdfPrimSpecHandle& primSpec)
{
bool isInternalRef{false};

for (const SdfReference& ref : primSpec->GetReferenceList().GetAddedOrExplicitItems()) {
// GetAssetPath returns the asset path to the root layer of the referenced layer
// this will be empty in the case of an internal reference.
if (ref.GetAssetPath().empty()) {
isInternalRef = true;
ppt-adsk marked this conversation as resolved.
Show resolved Hide resolved
break;
}
}

return isInternalRef;
}

} // MayaUsdUtils
20 changes: 14 additions & 6 deletions lib/usd/utils/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,25 @@ namespace MayaUsdUtils {
MAYA_USD_UTILS_PUBLIC
SdfLayerHandle defPrimSpecLayer(const UsdPrim&);

//! Return a list of layers in strength order that have opinions on the argument prim.
//! Return a list of layers in no strength order that can contribute to the argument prim.
MAYA_USD_UTILS_PUBLIC
std::vector<SdfLayerHandle> layersWithPrimSpec(const UsdPrim&);
std::set<SdfLayerHandle> layersWithContribution(const UsdPrim&);

//! Check if a layer has any opinions that affects on the argument prim.
//! Check if a layer has any contributions towards the argument prim.
MAYA_USD_UTILS_PUBLIC
bool doesEditTargetLayerHavePrimSpec(const UsdPrim&);
bool doesEditTargetLayerContribute(const UsdPrim&);

//! Return the strongest layer that has an opinion on the argument prim.
//! Return the strongest layer that can contribute to the argument prim.
MAYA_USD_UTILS_PUBLIC
SdfLayerHandle strongestLayerWithPrimSpec(const UsdPrim&);
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(UsdStageWeakPtr, const UsdPrim&);

//! Returns true if the prim spec has an internal reference.
MAYA_USD_UTILS_PUBLIC
bool isInternalReference(const SdfPrimSpecHandle&);

} // namespace MayaUsdUtils
Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a utility function for getting a PrimSpec from a prim path in the layer containing the stage's current edit target.


Expand Down
Loading