From 381450fb719eaa834b66a17df06702b82ebcd345 Mon Sep 17 00:00:00 2001 From: Pierre Baillargeon Date: Mon, 27 Feb 2023 14:11:47 -0500 Subject: [PATCH 1/2] MAYA-127427 fix rename when staged multiple times - Protect UsdSceneItem against invalid prims. - Protect USD UI info handler against invalid prims. - These can happen for example when we're in the middle of a rename and the outliner gets updated or redrawn. - Fix the rename command to send rename notifications to all proxy shape that share the same stage. - At the same time, refactor the redo/undo code that was 99% identical. --- lib/mayaUsd/ufe/UsdSceneItem.cpp | 5 +- lib/mayaUsd/ufe/UsdUIInfoHandler.cpp | 6 +- lib/mayaUsd/ufe/UsdUndoRenameCommand.cpp | 147 ++++++++++++----------- 3 files changed, 87 insertions(+), 71 deletions(-) diff --git a/lib/mayaUsd/ufe/UsdSceneItem.cpp b/lib/mayaUsd/ufe/UsdSceneItem.cpp index 09eccd1a74..a16c0ce08c 100644 --- a/lib/mayaUsd/ufe/UsdSceneItem.cpp +++ b/lib/mayaUsd/ufe/UsdSceneItem.cpp @@ -47,13 +47,16 @@ UsdSceneItem::create(const Ufe::Path& path, const UsdPrim& prim, int instanceInd // Ufe::SceneItem overrides //------------------------------------------------------------------------------ -std::string UsdSceneItem::nodeType() const { return fPrim.GetTypeName(); } +std::string UsdSceneItem::nodeType() const { return fPrim ? fPrim.GetTypeName() : std::string(); } #ifdef UFE_V2_FEATURES_AVAILABLE std::vector UsdSceneItem::ancestorNodeTypes() const { std::vector strAncestorTypes; + if (!fPrim) + return strAncestorTypes; + #if PXR_VERSION < 2008 static const TfType schemaBaseType = TfType::Find(); const TfType schemaType = schemaBaseType.FindDerivedByName(fPrim.GetTypeName().GetString()); diff --git a/lib/mayaUsd/ufe/UsdUIInfoHandler.cpp b/lib/mayaUsd/ufe/UsdUIInfoHandler.cpp index afacc3fe03..fc1a3c7eee 100644 --- a/lib/mayaUsd/ufe/UsdUIInfoHandler.cpp +++ b/lib/mayaUsd/ufe/UsdUIInfoHandler.cpp @@ -141,7 +141,7 @@ bool UsdUIInfoHandler::treeViewCellInfo(const Ufe::SceneItem::Ptr& item, Ufe::Ce #if !defined(NDEBUG) assert(usdItem); #endif - if (usdItem) { + if (usdItem && usdItem->prim()) { if (!usdItem->prim().IsActive()) { changed = true; info.fontStrikeout = true; @@ -204,7 +204,7 @@ Ufe::UIInfoHandler::Icon UsdUIInfoHandler::treeViewIcon(const Ufe::SceneItem::Pt // Check if we have any composition meta data - if yes we display a special badge. UsdSceneItem::Ptr usdItem = std::dynamic_pointer_cast(item); - if (usdItem) { + if (usdItem && usdItem->prim()) { // Variants if (!usdItem->prim().GetVariantSets().GetNames().empty()) { icon.badgeIcon = "out_USD_CompArcBadgeV.png"; @@ -234,7 +234,7 @@ std::string UsdUIInfoHandler::treeViewTooltip(const Ufe::SceneItem::Ptr& item) c std::string tooltip; UsdSceneItem::Ptr usdItem = std::dynamic_pointer_cast(item); - if (usdItem) { + if (usdItem && usdItem->prim()) { // Composition related metadata. bool needComma = false; PXR_NS::SdfReferenceListOp referenceOp; diff --git a/lib/mayaUsd/ufe/UsdUndoRenameCommand.cpp b/lib/mayaUsd/ufe/UsdUndoRenameCommand.cpp index 184e5d3680..b3f44ab307 100644 --- a/lib/mayaUsd/ufe/UsdUndoRenameCommand.cpp +++ b/lib/mayaUsd/ufe/UsdUndoRenameCommand.cpp @@ -18,6 +18,8 @@ #include "private/UfeNotifGuard.h" #include "private/Utils.h" +#include +#include #include #include #include @@ -32,6 +34,8 @@ #include #include +#include +#include #include #include @@ -108,53 +112,89 @@ UsdUndoRenameCommand::create(const UsdSceneItem::Ptr& srcItem, const Ufe::PathCo UsdSceneItem::Ptr UsdUndoRenameCommand::renamedItem() const { return _ufeDstItem; } -bool UsdUndoRenameCommand::renameRedo() +static void sendNotificationToAllStageProxies( + const UsdStagePtr& stage, + const UsdPrim& prim, + const Ufe::Path& srcPath, + const Ufe::Path& dstPath) { - // If the new name is the same as the current name, do nothing. - // This is the same behavior as the Maya rename command for Maya nodes. - if (_newName.empty()) - return true; + const Ufe::Rtid mayaId = getMayaRunTimeId(); + for (const std::string& proxyName : ProxyShapeHandler::getAllNames()) { + UsdStagePtr proxyStage = ProxyShapeHandler::dagPathToStage(proxyName); + if (proxyStage != stage) + continue; - // get the stage's default prim path - auto defaultPrimPath = _stage->GetDefaultPrim().GetPath(); + const Ufe::PathSegment proxySegment(std::string("|world") + proxyName, mayaId, '|'); + + const Ufe::PathSegment& srcUsdSegment = srcPath.getSegments()[1]; + const Ufe::Path adjustedSrcPath(Ufe::Path::Segments({ proxySegment, srcUsdSegment })); + + const Ufe::PathSegment& dstUsdSegment = dstPath.getSegments()[1]; + const Ufe::Path adjustedDstPath(Ufe::Path::Segments({ proxySegment, dstUsdSegment })); + UsdSceneItem::Ptr newItem = UsdSceneItem::create(adjustedDstPath, prim); + + sendNotification(newItem, adjustedSrcPath); + } +} + +static bool doUsdRename( + const UsdStagePtr& stage, + const UsdPrim& prim, + std::string newName, + const Ufe::Path srcPath, + const Ufe::Path dstPath) +{ // 1- open a changeblock to delay sending notifications. // 2- update the Internal References paths (if any) first // 3- set the new name // Note: during the changeBlock scope we are still working with old items/paths/prims. // it's only after the scope ends that we start working with new items/paths/prims + SdfChangeBlock changeBlock; + + bool status + = MayaUsdUtils::updateReferencedPath(prim, SdfPath(dstPath.getSegments()[1].string())); + if (!status) + return false; + + // Make sure the load state of the renamed prim will be preserved. + // We copy all rules that applied to it specifically and remove the rules + // that applied to it specifically. { - SdfChangeBlock changeBlock; + auto fromPath = SdfPath(srcPath.getSegments()[1].string()); + auto destPath = SdfPath(dstPath.getSegments()[1].string()); + duplicateLoadRules(*stage, fromPath, destPath); + removeRulesForPath(*stage, fromPath); + } - const UsdPrim& prim = _stage->GetPrimAtPath(_ufeSrcItem->prim().GetPath()); + // set the new name + auto primSpec = MayaUsdUtils::getPrimSpecAtEditTarget(prim); + status = primSpec->SetName(newName); + if (!status) + return false; - auto ufeSiblingPath = _ufeSrcItem->path().sibling(Ufe::PathComponent(_newName)); - bool status = MayaUsdUtils::updateReferencedPath( - prim, SdfPath(ufeSiblingPath.getSegments()[1].string())); - if (!status) { - return false; - } + return true; +} - // Make sure the load state of the renamed prim will be preserved. - // We copy all rules that applied to it specifically and remove the rules - // that applied to it specifically. - { - auto fromPath = SdfPath(_ufeSrcItem->path().getSegments()[1].string()); - auto destPath = SdfPath(ufeSiblingPath.getSegments()[1].string()); - duplicateLoadRules(*_stage, fromPath, destPath); - removeRulesForPath(*_stage, fromPath); - } +bool UsdUndoRenameCommand::renameRedo() +{ + // If the new name is the same as the current name, do nothing. + // This is the same behavior as the Maya rename command for Maya nodes. + if (_newName.empty()) + return true; - // set the new name - auto primSpec = MayaUsdUtils::getPrimSpecAtEditTarget(prim); - status = primSpec->SetName(_newName); - if (!status) { - return false; - } - } + // get the stage's default prim path + auto defaultPrimPath = _stage->GetDefaultPrim().GetPath(); + + const Ufe::Path srcPath = _ufeSrcItem->path(); + const Ufe::Path dstPath = srcPath.sibling(Ufe::PathComponent(_newName)); + const UsdPrim& prim = _ufeSrcItem->prim(); + + if (!doUsdRename(_stage, prim, _newName, srcPath, dstPath)) + return false; // the renamed scene item is a "sibling" of its original name. - _ufeDstItem = createSiblingSceneItem(_ufeSrcItem->path(), _newName); + _ufeDstItem = createSiblingSceneItem(srcPath, _newName); // update stage's default prim if (_ufeSrcItem->prim().GetPath() == defaultPrimPath) { @@ -162,7 +202,7 @@ bool UsdUndoRenameCommand::renameRedo() } // send notification to update UFE data model - sendNotification(_ufeDstItem, _ufeSrcItem->path()); + sendNotificationToAllStageProxies(_stage, prim, srcPath, dstPath); return true; } @@ -177,40 +217,13 @@ bool UsdUndoRenameCommand::renameUndo() // get the stage's default prim path auto defaultPrimPath = _stage->GetDefaultPrim().GetPath(); - // 1- open a changeblock to delay sending notifications. - // 2- update the Internal References paths (if any) first - // 3- set the new name - // Note: during the changeBlock scope we are still working with old items/paths/prims. - // it's only after the scope ends that we start working with new items/paths/prims - { - SdfChangeBlock changeBlock; - - const UsdPrim& prim = _stage->GetPrimAtPath(_ufeDstItem->prim().GetPath()); + const Ufe::Path srcPath = _ufeDstItem->path(); + const Ufe::Path dstPath = _ufeSrcItem->path(); + const UsdPrim& prim = _ufeDstItem->prim(); + const std::string newName = _ufeSrcItem->prim().GetName(); - auto ufeSiblingPath - = _ufeSrcItem->path().sibling(Ufe::PathComponent(_ufeSrcItem->prim().GetName())); - bool status = MayaUsdUtils::updateReferencedPath( - prim, SdfPath(ufeSiblingPath.getSegments()[1].string())); - if (!status) { - return false; - } - - // Make sure the load state of the renamed prim will be preserved. - // We copy all rules that applied to it specifically and remove the rules - // that applied to it specifically. - { - auto fromPath = SdfPath(_ufeDstItem->path().getSegments()[1].string()); - auto destPath = SdfPath(ufeSiblingPath.getSegments()[1].string()); - duplicateLoadRules(*_stage, fromPath, destPath); - removeRulesForPath(*_stage, fromPath); - } - - auto primSpec = MayaUsdUtils::getPrimSpecAtEditTarget(prim); - status = primSpec->SetName(_ufeSrcItem->prim().GetName()); - if (!status) { - return false; - } - } + if (!doUsdRename(_stage, prim, newName, srcPath, dstPath)) + return false; // the renamed scene item is a "sibling" of its original name. _ufeSrcItem = createSiblingSceneItem(_ufeDstItem->path(), _ufeSrcItem->prim().GetName()); @@ -221,7 +234,7 @@ bool UsdUndoRenameCommand::renameUndo() } // send notification to update UFE data model - sendNotification(_ufeSrcItem, _ufeDstItem->path()); + sendNotificationToAllStageProxies(_stage, prim, srcPath, dstPath); return true; } From 09baf33d919e9dbd9ca9675c8b3686d0815cecfc Mon Sep 17 00:00:00 2001 From: Pierre Baillargeon Date: Mon, 27 Feb 2023 16:00:31 -0500 Subject: [PATCH 2/2] MAYA-127427 fix composite rename command The prim can be stale when a rename is in the middle of a composite command. For example, the assign-material command. --- lib/mayaUsd/ufe/UsdUndoRenameCommand.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/mayaUsd/ufe/UsdUndoRenameCommand.cpp b/lib/mayaUsd/ufe/UsdUndoRenameCommand.cpp index b3f44ab307..8bec7b679d 100644 --- a/lib/mayaUsd/ufe/UsdUndoRenameCommand.cpp +++ b/lib/mayaUsd/ufe/UsdUndoRenameCommand.cpp @@ -188,7 +188,10 @@ bool UsdUndoRenameCommand::renameRedo() const Ufe::Path srcPath = _ufeSrcItem->path(); const Ufe::Path dstPath = srcPath.sibling(Ufe::PathComponent(_newName)); - const UsdPrim& prim = _ufeSrcItem->prim(); + // Note: must fetch prim again from its path because undo/redo of composite commands + // (or doing multiple undo and then multiple redo) can make the cached prim + // stale. + const UsdPrim prim = _stage->GetPrimAtPath(_ufeSrcItem->prim().GetPath()); if (!doUsdRename(_stage, prim, _newName, srcPath, dstPath)) return false; @@ -217,9 +220,12 @@ bool UsdUndoRenameCommand::renameUndo() // get the stage's default prim path auto defaultPrimPath = _stage->GetDefaultPrim().GetPath(); - const Ufe::Path srcPath = _ufeDstItem->path(); - const Ufe::Path dstPath = _ufeSrcItem->path(); - const UsdPrim& prim = _ufeDstItem->prim(); + const Ufe::Path srcPath = _ufeDstItem->path(); + const Ufe::Path dstPath = _ufeSrcItem->path(); + // Note: must fetch prim again from its path because undo/redo of composite commands + // (or doing multiple undo and then multiple redo) can make the cached prim + // stale. + const UsdPrim prim = _stage->GetPrimAtPath(_ufeDstItem->prim().GetPath()); const std::string newName = _ufeSrcItem->prim().GetName(); if (!doUsdRename(_stage, prim, newName, srcPath, dstPath))