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

122 changes: 41 additions & 81 deletions lib/mayaUsd/ufe/UsdUndoRenameCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,35 +42,25 @@ namespace ufe {

UsdUndoRenameCommand::UsdUndoRenameCommand(const UsdSceneItem::Ptr& srcItem, const Ufe::PathComponent& newName)
: Ufe::UndoableCommand()
, _ufeSrcItem(srcItem)
, _ufeDstItem(nullptr)
, _prim(srcItem->prim())
, _stage(_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.

, _oldName(_prim.GetName().GetString())
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 _oldName that holds the _prim name before it is changed.

{
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());
}

// if the current layer doesn't have any opinions that affects selected prim
if (!MayaUsdUtils::doesEditTargetLayerHavePrimSpec(prim)) {
auto possibleTargetLayer = MayaUsdUtils::strongestLayerWithPrimSpec(prim);
if (!MayaUsdUtils::doesEditTargetLayerHavePrimSpec(_prim)) {
auto possibleTargetLayer = MayaUsdUtils::strongestLayerWithPrimSpec(_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(),
_prim.GetName().GetString().c_str(),
possibleTargetLayer->GetDisplayName().c_str());
throw std::runtime_error(err.c_str());
}
else
{
auto layers = MayaUsdUtils::layersWithPrimSpec(prim);
auto layers = MayaUsdUtils::layersWithPrimSpec(_prim);

if (layers.size() > 1) {
std::string layerDisplayNames;
Expand All @@ -79,7 +69,7 @@ UsdUndoRenameCommand::UsdUndoRenameCommand(const UsdSceneItem::Ptr& srcItem, con
}
layerDisplayNames.pop_back();
std::string err = TfStringPrintf("Cannot rename [%s] with definitions or opinions on other layers. "
"Opinions exist in %s", prim.GetName().GetString().c_str(), layerDisplayNames.c_str());
"Opinions exist in %s", _prim.GetName().GetString().c_str(), layerDisplayNames.c_str());
throw std::runtime_error(err.c_str());
}
}
Expand All @@ -101,79 +91,49 @@ 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());

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;
}

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);
}

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, _ufeDstItem->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(_oldName);
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(), _oldName);
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
17 changes: 7 additions & 10 deletions lib/mayaUsd/ufe/UsdUndoRenameCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,22 +48,19 @@ 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;

const UsdPrim& _prim;
UsdStageWeakPtr _stage;
std::string _newName;
std::string _oldName;

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
6 changes: 6 additions & 0 deletions lib/usd/utils/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,10 @@ strongestLayerWithPrimSpec(const UsdPrim& prim)
return targetLayer;
}

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

} // MayaUsdUtils
4 changes: 4 additions & 0 deletions lib/usd/utils/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ namespace MayaUsdUtils {
MAYA_USD_UTILS_PUBLIC
SdfLayerHandle strongestLayerWithPrimSpec(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&);

} // 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.


#endif // MAYAUSDUTILS_UTIL_H