From 27769c42b840c26c76b6424a3b0bb2d6f68f6191 Mon Sep 17 00:00:00 2001 From: Pierre Tremblay Date: Fri, 20 Dec 2019 13:53:18 -0800 Subject: [PATCH 1/8] WIP: rename fix support. --- lib/ufe/Global.cpp | 6 +- lib/ufe/Global.h | 6 ++ lib/ufe/ProxyShapeHierarchy.cpp | 6 +- lib/ufe/UsdStageMap.cpp | 84 ++++++++++++++++++++++--- lib/ufe/UsdStageMap.h | 35 ++++++++--- lib/ufe/UsdUndoRenameCommand.cpp | 2 +- lib/ufe/Utils.cpp | 21 +++++++ lib/ufe/wrapUtils.cpp | 83 +++++++++++++++++++++++++ test/lib/ufe/CMakeLists.txt | 1 + test/lib/ufe/testRenameProxyShape.py | 92 ++++++++++++++++++++++++++++ 10 files changed, 315 insertions(+), 21 deletions(-) create mode 100644 test/lib/ufe/testRenameProxyShape.py diff --git a/lib/ufe/Global.cpp b/lib/ufe/Global.cpp index 2f07448317..11a554fe97 100644 --- a/lib/ufe/Global.cpp +++ b/lib/ufe/Global.cpp @@ -22,7 +22,6 @@ #include "UsdTransform3dHandler.h" #include "UsdSceneItemOpsHandler.h" -#include #include #include #include @@ -128,5 +127,10 @@ MStatus finalize() return MS::kSuccess; } +Ufe::Rtid getUsdRunTimeId() +{ + return g_USDRtid; +} + } // namespace ufe } // namespace MayaUsd diff --git a/lib/ufe/Global.h b/lib/ufe/Global.h index 5dfb588424..2179b522fe 100644 --- a/lib/ufe/Global.h +++ b/lib/ufe/Global.h @@ -17,6 +17,8 @@ #include "../base/api.h" +#include + #include MAYAUSD_NS_DEF { @@ -32,5 +34,9 @@ MStatus initialize(); MAYAUSD_CORE_PUBLIC MStatus finalize(); +//! Return the run-time ID allocated to USD. +MAYAUSD_CORE_PUBLIC +Ufe::Rtid getUsdRunTimeId(); + } // namespace ufe } // namespace MayaUsd diff --git a/lib/ufe/ProxyShapeHierarchy.cpp b/lib/ufe/ProxyShapeHierarchy.cpp index 2b8359713f..e0363c8eff 100644 --- a/lib/ufe/ProxyShapeHierarchy.cpp +++ b/lib/ufe/ProxyShapeHierarchy.cpp @@ -25,6 +25,8 @@ #include +#include + MAYAUSD_NS_DEF { namespace ufe { @@ -95,8 +97,10 @@ Ufe::SceneItem::Ptr ProxyShapeHierarchy::sceneItem() const bool ProxyShapeHierarchy::hasChildren() const { const UsdPrim& rootPrim = getUsdRootPrim(); - if (!rootPrim.IsValid()) + if (!rootPrim.IsValid()) { + std::cout << "PPT: invalid root prim in ProxyShapeHierarchy::hasChildren()" << std::endl; return false; + } return !rootPrim.GetChildren().empty(); } diff --git a/lib/ufe/UsdStageMap.cpp b/lib/ufe/UsdStageMap.cpp index 91a0ff14f7..3a74817900 100644 --- a/lib/ufe/UsdStageMap.cpp +++ b/lib/ufe/UsdStageMap.cpp @@ -15,6 +15,46 @@ // #include "UsdStageMap.h" +#include "Utils.h" + +#include + +#include +#include + +namespace { + +int stageCalls = 0; +int pathCalls = 0; + +MObjectHandle proxyShapeHandle(const Ufe::Path& path) +{ + // Get the MObjectHandle from the tail of the MDagPath. Remove the leading + // '|world' component. + auto dagPath = MayaUsd::ufe::nameToDagPath(path.popHead().string()); + MObjectHandle handle(dagPath.node()); + if (!handle.isValid()) { + TF_CODING_ERROR("'%s' is not a path to a proxy shape node.", + path.string().c_str()); + } + return handle; +} + +// Assuming proxy shape nodes cannot be instanced, simply return the first path. +Ufe::Path firstPath(const MObjectHandle& handle) +{ + if (!TF_VERIFY(handle.isValid(), + "Cannot get path from invalid object handle")) { + return Ufe::Path(); + } + + MDagPath dagPath; + auto status = MFnDagNode(handle.object()).getPath(dagPath); + CHECK_MSTATUS(status); + return MayaUsd::ufe::dagPathToUfe(dagPath); +} + +} MAYAUSD_NS_DEF { namespace ufe { @@ -31,32 +71,58 @@ UsdStageMap g_StageMap; void UsdStageMap::addItem(const Ufe::Path& path, UsdStageWeakPtr stage) { - fPathToStage[path] = stage; - fStageToPath[stage] = path; + // We expect a path to the proxy shape node, therefore a single segment. + auto nbSegments = path.nbSegments(); + if (nbSegments != 1) { + TF_CODING_ERROR("A proxy shape node path can have only one segment, path '%s' has %d", path.string().c_str(), nbSegments); + return; + } + + // Convert the tail of the UFE path to an MObjectHandle. + auto proxyShape = proxyShapeHandle(path); + if (!proxyShape.isValid()) { + return; + } + + // Could get the stage from the proxy shape object in the stage() method, + // but since it's given here, simply store it. + fObjectToStage[proxyShape] = stage; + fStageToObject[stage] = proxyShape; } UsdStageWeakPtr UsdStageMap::stage(const Ufe::Path& path) const { + ++stageCalls; + std::cout << "PPT: stage calls is " << stageCalls << std::endl; + + auto proxyShape = proxyShapeHandle(path); + if (!proxyShape.isValid()) { + return nullptr; + } + // A stage is bound to a single Dag proxy shape. - auto iter = fPathToStage.find(path); - if (iter != std::end(fPathToStage)) + auto iter = fObjectToStage.find(proxyShape); + if (iter != std::end(fObjectToStage)) return iter->second; return nullptr; } Ufe::Path UsdStageMap::path(UsdStageWeakPtr stage) const { + ++pathCalls; + std::cout << "PPT: path calls is " << pathCalls << std::endl; + // A stage is bound to a single Dag proxy shape. - auto iter = fStageToPath.find(stage); - if (iter != std::end(fStageToPath)) - return iter->second; + auto iter = fStageToObject.find(stage); + if (iter != std::end(fStageToObject)) + return firstPath(iter->second); return Ufe::Path(); } void UsdStageMap::clear() { - fPathToStage.clear(); - fStageToPath.clear(); + fObjectToStage.clear(); + fStageToObject.clear(); } } // namespace ufe diff --git a/lib/ufe/UsdStageMap.h b/lib/ufe/UsdStageMap.h index d12897029c..954eb640e9 100644 --- a/lib/ufe/UsdStageMap.h +++ b/lib/ufe/UsdStageMap.h @@ -22,8 +22,19 @@ #include #include +#include + #include +// Allow for use of MObjectHandle with std::unordered_map. +namespace std { +template <> struct hash { + std::size_t operator()(const MObjectHandle& handle) const { + return static_cast(handle.hashCode()); + } +}; +} + PXR_NAMESPACE_USING_DIRECTIVE MAYAUSD_NS_DEF { @@ -31,13 +42,17 @@ namespace ufe { //! \brief USD Stage Map /*! - Map of AL_usdmaya_ProxyShape UFE path to corresponding stage. - - Map of stage to corresponding AL_usdmaya_ProxyShape UFE path. Ideally, we - would support dynamically computing the path for the AL_usdmaya_ProxyShape - node, but we assume here it will not be reparented. We will also assume that - a USD stage will not be instanced (even though nothing in the data model - prevents it). + Two-way map of proxy shape UFE path to corresponding stage. + + We will assume that a USD proxy shape will not be instanced (even though + nothing in the data model prevents it). To support renaming and repathing, + we store an MObjectHandle in the maps, which is invariant to renaming and + repathing, and compute the path on access. This is slower than a scheme + where we cache using the Ufe::Path, but such a cache must be refreshed on + rename and repath, which is non-trivial, since there is no guarantee on the + order of notification of Ufe observers. An earlier implementation with + rename observation had the Maya Outliner (which observes rename) access the + UsdStageMap on rename before the UsdStageMap had been updated. */ class MAYAUSD_CORE_PUBLIC UsdStageMap { @@ -64,8 +79,10 @@ class MAYAUSD_CORE_PUBLIC UsdStageMap private: // We keep two maps for fast lookup when there are many proxy shapes. - std::unordered_map fPathToStage; - TfHashMap fStageToPath; + using ObjectToStage = std::unordered_map; + using StageToObject = TfHashMap; + ObjectToStage fObjectToStage; + StageToObject fStageToObject; }; // UsdStageMap diff --git a/lib/ufe/UsdUndoRenameCommand.cpp b/lib/ufe/UsdUndoRenameCommand.cpp index e767118eee..8c932b24e3 100644 --- a/lib/ufe/UsdUndoRenameCommand.cpp +++ b/lib/ufe/UsdUndoRenameCommand.cpp @@ -41,7 +41,7 @@ UsdUndoRenameCommand::UsdUndoRenameCommand(const UsdSceneItem::Ptr& srcItem, con // prim, we must recreate them after every call to rename. fUsdDstPath = prim.GetParent().GetPath().AppendChild(TfToken(newName.string())); fLayer = defPrimSpecLayer(prim); - if (fLayer) { + if (!fLayer) { std::string err = TfStringPrintf("No prim found at %s", prim.GetPath().GetString().c_str()); throw std::runtime_error(err.c_str()); } diff --git a/lib/ufe/Utils.cpp b/lib/ufe/Utils.cpp index 6a786e6eca..8e91e3a66b 100644 --- a/lib/ufe/Utils.cpp +++ b/lib/ufe/Utils.cpp @@ -32,6 +32,8 @@ #include #include +#include + PXR_NAMESPACE_USING_DIRECTIVE #ifndef MAYA_MSTRINGARRAY_ITERATOR_CATEGORY @@ -103,13 +105,32 @@ SdfLayerHandle defPrimSpecLayer(const UsdPrim& prim) // The source layer is the one in which there exists a def primSpec, not // an over. + std::cout << "PPT: in defPrimSpecLayer, argument prim is " + << (prim.IsValid() ? "" : "NOT ") << "valid" << std::endl; + SdfLayerHandle defLayer; auto layerStack = prim.GetStage()->GetLayerStack(); + auto stage = prim.GetStage(); + auto primFromPath = stage->GetPrimAtPath(prim.GetPath()); + std::cout << "PPT: stage.GetPrimAtPath() returns a " + << (prim.IsValid() ? "VALID " : "INVALID ") << "prim" + << std::endl; + for (auto layer : layerStack) { + std::cout << "PPT: iterating over layer " << layer->GetIdentifier() + << std::endl; + std::cout << "PPT: prim path is " << prim.GetPath().GetString() + << std::endl; auto primSpec = layer->GetPrimAtPath(prim.GetPath()); + std::cout << "PPT: primSpec is " << (primSpec ? "NOT " : "") << "null" + << std::endl; + std::cout << "PPT: layer->HasSpec() returns " + << (layer->HasSpec(prim.GetPath()) ? "TRUE" : "FALSE") + << std::endl; if (primSpec && (primSpec->GetSpecifier() == SdfSpecifierDef)) { + std::cout << "PPT: found layer with def primSpec." << std::endl; defLayer = layer; break; } diff --git a/lib/ufe/wrapUtils.cpp b/lib/ufe/wrapUtils.cpp index aa75963035..7904f6b6f5 100644 --- a/lib/ufe/wrapUtils.cpp +++ b/lib/ufe/wrapUtils.cpp @@ -18,6 +18,7 @@ #include "UsdSceneItem.h" #include "Utils.h" +#include "Global.h" #include #include @@ -25,6 +26,36 @@ using namespace MayaUsd; using namespace boost::python; +namespace { + +std::size_t find( + const std::string& s, std::size_t size, std::size_t begin, char sep) +{ + std::size_t foundPos = s.find(sep, begin); + return foundPos == std::string::npos ? size : foundPos; +} + +// Copy elision (return value optimization) will avoid duplicate std::vector +// construction on return. +std::vector split(const std::string& src, char sep) +{ + std::vector names; + if (sep) { + std::size_t end = 0; + const std::size_t size = src.size(); + for (std::size_t begin = 0; begin < size; begin = end+1) { + end = find(src, size, begin, sep); + names.push_back(src.substr(begin, end-begin)); + } + } + else { + names.push_back(src); + } + return names; +} + +} + UsdPrim getPrimFromRawItem(uint64_t rawItem) { Ufe::SceneItem* item = reinterpret_cast(rawItem); @@ -59,6 +90,48 @@ std::string getNodeTypeFromRawItem(uint64_t rawItem) return type; } +UsdStageWeakPtr getStage(const std::string& ufePathString) +{ + // This function works on a single-segment path, i.e. the Maya Dag path + // segment to the proxy shape. We know the Maya run-time ID is 1, + // separator is '|'. + return ufe::getStage(Ufe::Path(Ufe::PathSegment(ufePathString, 1, '|'))); +} + +std::string stagePath(UsdStageWeakPtr stage) +{ + // Proxy shape node's UFE path is a single segment, so no need to separate + // segments with commas. + return ufe::stagePath(stage).string(); +} + +UsdPrim ufePathToPrim(const std::string& ufePathString) +{ + // The path string is a list of segment strings separated by ',' comma + // separator. + auto segmentStrings = split(ufePathString, ','); + + // If there's just one segment, it's the Maya Dag path segment, so it can't + // have a prim. + if (segmentStrings.size() == 1) { + return UsdPrim(); + } + + // We have the path string split into segments. Build up the Ufe::Path one + // segment at a time. The path segment separator is the first character + // of each segment. We know that USD's separator is '/' and Maya's + // separator is '|', so use a map to get the corresponding UFE run-time ID. + Ufe::Path path; + static std::map sepToRtid = { + {'/', ufe::getUsdRunTimeId()}, {'|', 1}}; + for (std::size_t i = 0; i < segmentStrings.size(); ++i) { + const auto& segmentString = segmentStrings[i]; + char sep = segmentString[0]; + path = path + Ufe::PathSegment(segmentString, sepToRtid.at(sep), sep); + } + return ufe::ufePathToPrim(path); +} + BOOST_PYTHON_MODULE(ufe) { def("getPrimFromRawItem", getPrimFromRawItem); @@ -68,4 +141,14 @@ BOOST_PYTHON_MODULE(ufe) #endif def("getNodeTypeFromRawItem", getNodeTypeFromRawItem); + + // Because mayaUsd and UFE have incompatible Python bindings that do not + // know about each other (provided by Boost Python and pybind11, + // respectively), we cannot pass in or return UFE objects such as Ufe::Path + // here, and are forced to use strings. Use the tentative string + // representation of Ufe::Path as comma-separated segments. We know that + // the USD path separator is '/'. PPT, 8-Dec-2019. + def("getStage", getStage); + def("stagePath", stagePath); + def("ufePathToPrim", ufePathToPrim); } diff --git a/test/lib/ufe/CMakeLists.txt b/test/lib/ufe/CMakeLists.txt index 5e971ba64f..ed6386d1ec 100644 --- a/test/lib/ufe/CMakeLists.txt +++ b/test/lib/ufe/CMakeLists.txt @@ -48,6 +48,7 @@ if(CMAKE_UFE_V2_FEATURES_AVAILABLE) # testRotateCmd.py # testScaleCmd.py # testTransform3dTranslate.py + testRenameProxyShape.py ) endif() diff --git a/test/lib/ufe/testRenameProxyShape.py b/test/lib/ufe/testRenameProxyShape.py new file mode 100644 index 0000000000..91702892e0 --- /dev/null +++ b/test/lib/ufe/testRenameProxyShape.py @@ -0,0 +1,92 @@ +#!/usr/bin/env python + +# +# Copyright 2019 Autodesk +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +import maya.cmds as cmds + +from ufeTestUtils import usdUtils, mayaUtils +import ufe +import mayaUsd.ufe + +import unittest + +class RenameProxyShapeTestCase(unittest.TestCase): + '''Test renaming the proxy shape and its ancestors. + + Renaming should not affect UFE lookup. + ''' + + pluginsLoaded = False + + @classmethod + def setUpClass(cls): + if not cls.pluginsLoaded: + cls.pluginsLoaded = mayaUtils.isMayaUsdPluginLoaded() + + def setUp(self): + ''' Called initially to set up the Maya test environment ''' + # Load plugins + self.assertTrue(self.pluginsLoaded) + + # Open top_layer.ma scene in test-samples + mayaUtils.openTopLayerScene() + + # Clear selection to start off + cmds.select(clear=True) + + def testRenameProxyShape(self): + '''Rename proxy shape, UFE lookup should succeed.''' + + mayaSegment = mayaUtils.createUfePathSegment( + "|world|transform1|proxyShape1") + usdSegment = usdUtils.createUfePathSegment("/Room_set/Props/Ball_35") + + ball35Path = ufe.Path([mayaSegment, usdSegment]) + ball35PathStr = ','.join( + [str(segment) for segment in ball35Path.segments]) + + # Because of difference in Python binding systems (pybind11 for UFE, + # Boost Python for mayaUsd and USD), need to pass in strings to + # mayaUsd functions. Multi-segment UFE paths need to have + # comma-separated segments. + def assertStageAndPrimAccess( + proxyShapeSegment, primUfePathStr, primSegment): + proxyShapePathStr = str(proxyShapeSegment) + + stage = mayaUsd.ufe.getStage(proxyShapePathStr) + prim = mayaUsd.ufe.ufePathToPrim(primUfePathStr) + stagePath = mayaUsd.ufe.stagePath(stage) + + self.assertIsNotNone(stage) + self.assertEqual(stagePath, proxyShapePathStr) + self.assertTrue(prim.IsValid()) + self.assertEqual(str(prim.GetPath()), str(primSegment)) + + assertStageAndPrimAccess(mayaSegment, ball35PathStr, usdSegment) + + # Rename the proxy shape node itself. Stage and prim access should + # still be valid, with the new path. + mayaSegment = mayaUtils.createUfePathSegment( + "|world|transform1|potato") + cmds.rename('|transform1|proxyShape1', 'potato') + self.assertEqual(len(cmds.ls('potato')), 1) + + ball35Path = ufe.Path([mayaSegment, usdSegment]) + ball35PathStr = ','.join( + [str(segment) for segment in ball35Path.segments]) + + assertStageAndPrimAccess(mayaSegment, ball35PathStr, usdSegment) From 12889e253cd9aa053ae1be10501bbc44ee0b9be7 Mon Sep 17 00:00:00 2001 From: Pierre Tremblay Date: Fri, 31 Jan 2020 12:01:09 -0800 Subject: [PATCH 2/8] Rework rename command implementation. --- lib/ufe/ProxyShapeHierarchy.cpp | 22 ++++-- lib/ufe/ProxyShapeHierarchy.h | 14 ++-- lib/ufe/ProxyShapeHierarchyHandler.cpp | 9 +-- lib/ufe/ProxyShapeHierarchyHandler.h | 2 - lib/ufe/UsdStageMap.cpp | 5 +- lib/ufe/UsdUndoRenameCommand.cpp | 76 +++++++++++++------ lib/ufe/UsdUndoRenameCommand.h | 10 +-- test/lib/ufe/CMakeLists.txt | 2 +- ...{testRenameProxyShape.py => testRename.py} | 69 ++++++++++++++++- 9 files changed, 158 insertions(+), 51 deletions(-) rename test/lib/ufe/{testRenameProxyShape.py => testRename.py} (57%) diff --git a/lib/ufe/ProxyShapeHierarchy.cpp b/lib/ufe/ProxyShapeHierarchy.cpp index e0363c8eff..2c492ec1f9 100644 --- a/lib/ufe/ProxyShapeHierarchy.cpp +++ b/lib/ufe/ProxyShapeHierarchy.cpp @@ -17,6 +17,7 @@ #include "ProxyShapeHierarchy.h" #include "Utils.h" +#include #include #include #include @@ -25,8 +26,6 @@ #include -#include - MAYAUSD_NS_DEF { namespace ufe { @@ -39,7 +38,9 @@ extern Ufe::Rtid g_USDRtid; // ProxyShapeHierarchy //------------------------------------------------------------------------------ -ProxyShapeHierarchy::ProxyShapeHierarchy(Ufe::HierarchyHandler::Ptr mayaHierarchyHandler) +ProxyShapeHierarchy::ProxyShapeHierarchy( + const Ufe::HierarchyHandler::Ptr& mayaHierarchyHandler +) : Ufe::Hierarchy() , fMayaHierarchyHandler(mayaHierarchyHandler) { @@ -50,11 +51,22 @@ ProxyShapeHierarchy::~ProxyShapeHierarchy() } /*static*/ -ProxyShapeHierarchy::Ptr ProxyShapeHierarchy::create(Ufe::HierarchyHandler::Ptr mayaHierarchyHandler) +ProxyShapeHierarchy::Ptr ProxyShapeHierarchy::create(const Ufe::HierarchyHandler::Ptr& mayaHierarchyHandler) { return std::make_shared(mayaHierarchyHandler); } +/*static*/ +ProxyShapeHierarchy::Ptr ProxyShapeHierarchy::create( + const Ufe::HierarchyHandler::Ptr& mayaHierarchyHandler, + const Ufe::SceneItem::Ptr& item +) +{ + auto hierarchy = create(mayaHierarchyHandler); + hierarchy->setItem(item); + return hierarchy; +} + void ProxyShapeHierarchy::setItem(const Ufe::SceneItem::Ptr& item) { // Our USD root prim is from the stage, which is from the item. So if we are @@ -98,7 +110,7 @@ bool ProxyShapeHierarchy::hasChildren() const { const UsdPrim& rootPrim = getUsdRootPrim(); if (!rootPrim.IsValid()) { - std::cout << "PPT: invalid root prim in ProxyShapeHierarchy::hasChildren()" << std::endl; + UFE_LOG("invalid root prim in ProxyShapeHierarchy::hasChildren()"); return false; } return !rootPrim.GetChildren().empty(); diff --git a/lib/ufe/ProxyShapeHierarchy.h b/lib/ufe/ProxyShapeHierarchy.h index 3f932ed0d0..5073e29b88 100644 --- a/lib/ufe/ProxyShapeHierarchy.h +++ b/lib/ufe/ProxyShapeHierarchy.h @@ -38,7 +38,7 @@ class MAYAUSD_CORE_PUBLIC ProxyShapeHierarchy : public Ufe::Hierarchy public: typedef std::shared_ptr Ptr; - ProxyShapeHierarchy(Ufe::HierarchyHandler::Ptr mayaHierarchyHandler); + ProxyShapeHierarchy(const Ufe::HierarchyHandler::Ptr& mayaHierarchyHandler); ~ProxyShapeHierarchy() override; // Delete the copy/move constructors assignment operators. @@ -48,7 +48,11 @@ class MAYAUSD_CORE_PUBLIC ProxyShapeHierarchy : public Ufe::Hierarchy ProxyShapeHierarchy& operator=(ProxyShapeHierarchy&&) = delete; //! Create a ProxyShapeHierarchy from a UFE hierarchy handler. - static ProxyShapeHierarchy::Ptr create(Ufe::HierarchyHandler::Ptr mayaHierarchyHandler); + static ProxyShapeHierarchy::Ptr create(const Ufe::HierarchyHandler::Ptr& mayaHierarchyHandler); + static ProxyShapeHierarchy::Ptr create( + const Ufe::HierarchyHandler::Ptr& mayaHierarchyHandler, + const Ufe::SceneItem::Ptr& item + ); void setItem(const Ufe::SceneItem::Ptr& item); @@ -68,9 +72,9 @@ class MAYAUSD_CORE_PUBLIC ProxyShapeHierarchy : public Ufe::Hierarchy const UsdPrim& getUsdRootPrim() const; private: - Ufe::SceneItem::Ptr fItem; - Hierarchy::Ptr fMayaHierarchy; - Ufe::HierarchyHandler::Ptr fMayaHierarchyHandler; + Ufe::SceneItem::Ptr fItem{nullptr}; + Hierarchy::Ptr fMayaHierarchy{nullptr}; + Ufe::HierarchyHandler::Ptr fMayaHierarchyHandler{nullptr}; // The root prim is initialized on first use and therefore mutable. mutable UsdPrim fUsdRootPrim; diff --git a/lib/ufe/ProxyShapeHierarchyHandler.cpp b/lib/ufe/ProxyShapeHierarchyHandler.cpp index 2728396f43..9c62e9e369 100644 --- a/lib/ufe/ProxyShapeHierarchyHandler.cpp +++ b/lib/ufe/ProxyShapeHierarchyHandler.cpp @@ -17,15 +17,15 @@ #include "ProxyShapeHierarchyHandler.h" #include "Utils.h" +#include + MAYAUSD_NS_DEF { namespace ufe { ProxyShapeHierarchyHandler::ProxyShapeHierarchyHandler(Ufe::HierarchyHandler::Ptr mayaHierarchyHandler) : Ufe::HierarchyHandler() , fMayaHierarchyHandler(mayaHierarchyHandler) -{ - fProxyShapeHierarchy = ProxyShapeHierarchy::create(mayaHierarchyHandler); -} +{} ProxyShapeHierarchyHandler::~ProxyShapeHierarchyHandler() { @@ -45,8 +45,7 @@ Ufe::Hierarchy::Ptr ProxyShapeHierarchyHandler::hierarchy(const Ufe::SceneItem:: { if (isAGatewayType(item->nodeType())) { - fProxyShapeHierarchy->setItem(item); - return fProxyShapeHierarchy; + return ProxyShapeHierarchy::create(fMayaHierarchyHandler, item); } else { diff --git a/lib/ufe/ProxyShapeHierarchyHandler.h b/lib/ufe/ProxyShapeHierarchyHandler.h index e7322c654b..62f129e5b3 100644 --- a/lib/ufe/ProxyShapeHierarchyHandler.h +++ b/lib/ufe/ProxyShapeHierarchyHandler.h @@ -18,7 +18,6 @@ #include "../base/api.h" #include -#include MAYAUSD_NS_DEF { namespace ufe { @@ -59,7 +58,6 @@ class MAYAUSD_CORE_PUBLIC ProxyShapeHierarchyHandler : public Ufe::HierarchyHand private: Ufe::HierarchyHandler::Ptr fMayaHierarchyHandler; - ProxyShapeHierarchy::Ptr fProxyShapeHierarchy; }; // ProxyShapeHierarchyHandler diff --git a/lib/ufe/UsdStageMap.cpp b/lib/ufe/UsdStageMap.cpp index 3a74817900..e984f26c0c 100644 --- a/lib/ufe/UsdStageMap.cpp +++ b/lib/ufe/UsdStageMap.cpp @@ -31,11 +31,12 @@ MObjectHandle proxyShapeHandle(const Ufe::Path& path) { // Get the MObjectHandle from the tail of the MDagPath. Remove the leading // '|world' component. - auto dagPath = MayaUsd::ufe::nameToDagPath(path.popHead().string()); + auto noWorld = path.popHead().string(); + auto dagPath = MayaUsd::ufe::nameToDagPath(noWorld); MObjectHandle handle(dagPath.node()); if (!handle.isValid()) { TF_CODING_ERROR("'%s' is not a path to a proxy shape node.", - path.string().c_str()); + noWorld.c_str()); } return handle; } diff --git a/lib/ufe/UsdUndoRenameCommand.cpp b/lib/ufe/UsdUndoRenameCommand.cpp index 8c932b24e3..b0927565a3 100644 --- a/lib/ufe/UsdUndoRenameCommand.cpp +++ b/lib/ufe/UsdUndoRenameCommand.cpp @@ -21,6 +21,8 @@ #include #include #include +#define UFE_ENABLE_ASSERTS +#include #include #include @@ -34,7 +36,7 @@ UsdUndoRenameCommand::UsdUndoRenameCommand(const UsdSceneItem::Ptr& srcItem, con { const UsdPrim& prim = srcItem->prim(); fStage = prim.GetStage(); - fUfeSrcPath = srcItem->path(); + fUfeSrcItem = srcItem; fUsdSrcPath = prim.GetPath(); // Every call to rename() (through execute(), undo() or redo()) removes // a prim, which becomes expired. Since USD UFE scene items contain a @@ -62,32 +64,56 @@ UsdSceneItem::Ptr UsdUndoRenameCommand::renamedItem() const return fUfeDstItem; } -bool UsdUndoRenameCommand::rename(SdfLayerHandle layer, const Ufe::Path& ufeSrcPath, const SdfPath& usdSrcPath, const SdfPath& usdDstPath) +bool UsdUndoRenameCommand::renameRedo() { - InPathChange pc; - return internalRename(layer, ufeSrcPath, usdSrcPath, usdDstPath); -} + // Copy the source path using CopySpec, and inactivate the source. -bool UsdUndoRenameCommand::internalRename(SdfLayerHandle layer, const Ufe::Path& ufeSrcPath, const SdfPath& usdSrcPath, const SdfPath& usdDstPath) -{ // We use the source layer as the destination. An alternate workflow // would be the edit target layer be the destination: - // layer = self._stage.GetEditTarget().GetLayer() - bool status = SdfCopySpec(layer, usdSrcPath, layer, usdDstPath); + // layer = self.fStage.GetEditTarget().GetLayer() + bool status = SdfCopySpec(fLayer, fUsdSrcPath, fLayer, fUsdDstPath); if (status) { - fStage->RemovePrim(usdSrcPath); - // The renamed scene item is a "sibling" of its original name. - fUfeDstItem = createSiblingSceneItem( - ufeSrcPath, usdDstPath.GetElementString()); - // USD sends two ResyncedPaths() notifications, one for the CopySpec - // call, the other for the RemovePrim call (new name added, old name - // removed). Unfortunately, the rename semantics are lost: there is - // no notion that the two notifications belong to the same atomic - // change. Provide a single Rename notification ourselves here. - Ufe::ObjectRename notification(fUfeDstItem, ufeSrcPath); - Ufe::Scene::notifyObjectPathChange(notification); + auto srcPrim = fStage->GetPrimAtPath(fUsdSrcPath); + UFE_ASSERT_MSG(srcPrim, "Invalid prim cannot be inactivated."); + status = srcPrim.SetActive(false); + + if (status) { + // The renamed scene item is a "sibling" of its original name. + auto ufeSrcPath = fUfeSrcItem->path(); + fUfeDstItem = createSiblingSceneItem( + ufeSrcPath, fUsdDstPath.GetElementString()); + + Ufe::ObjectRename notification(fUfeDstItem, ufeSrcPath); + Ufe::Scene::notifyObjectPathChange(notification); + } + } + else { + UFE_LOG(std::string("Warning: SdfCopySpec(") + + fUsdSrcPath.GetString() + std::string(") failed.")); + } + + return status; +} + +bool UsdUndoRenameCommand::renameUndo() +{ + bool status = fStage->RemovePrim(fUsdDstPath); + if (status) { + auto srcPrim = fStage->GetPrimAtPath(fUsdSrcPath); + UFE_ASSERT_MSG(srcPrim, "Invalid prim cannot be activated."); + status = srcPrim.SetActive(true); + + if (status) { + Ufe::ObjectRename notification(fUfeSrcItem, fUfeDstItem->path()); + Ufe::Scene::notifyObjectPathChange(notification); + fUfeDstItem = nullptr; + } } + else { + UFE_LOG(std::string("Warning: RemovePrim(") + + fUsdDstPath.GetString() + std::string(") failed.")); + } return status; } @@ -101,7 +127,10 @@ void UsdUndoRenameCommand::undo() // MAYA-92264: Pixar bug prevents undo from working. Try again with USD // version 0.8.5 or later. PPT, 7-Jul-2018. try { - rename(fLayer, fUfeDstItem->path(), fUsdDstPath, fUsdSrcPath); + InPathChange pc; + if (!renameUndo()) { + UFE_LOG("rename undo failed"); + } } catch (const std::exception& e) { UFE_LOG(e.what()); @@ -111,7 +140,10 @@ void UsdUndoRenameCommand::undo() void UsdUndoRenameCommand::redo() { - rename(fLayer, fUfeSrcPath, fUsdSrcPath, fUsdDstPath); + InPathChange pc; + if (!renameRedo()) { + UFE_LOG("rename redo failed"); + } } } // namespace ufe diff --git a/lib/ufe/UsdUndoRenameCommand.h b/lib/ufe/UsdUndoRenameCommand.h index 8440b26ed7..c22b2c28aa 100644 --- a/lib/ufe/UsdUndoRenameCommand.h +++ b/lib/ufe/UsdUndoRenameCommand.h @@ -51,20 +51,18 @@ class MAYAUSD_CORE_PUBLIC UsdUndoRenameCommand : public Ufe::UndoableCommand UsdSceneItem::Ptr renamedItem() const; - //! Rename the prim hierarchy at usdSrcPath to usdDstPath. - bool rename(SdfLayerHandle layer, const Ufe::Path& ufeSrcPath, const SdfPath& usdSrcPath, const SdfPath& usdDstPath); +private: // UsdUndoRenameCommand overrides void undo() override; void redo() override; -private: - bool internalRename(SdfLayerHandle layer, const Ufe::Path& ufeSrcPath, const SdfPath& usdSrcPath, const SdfPath& usdDstPath); + bool renameRedo(); + bool renameUndo(); -private: UsdStageWeakPtr fStage; SdfLayerHandle fLayer; - Ufe::Path fUfeSrcPath; + UsdSceneItem::Ptr fUfeSrcItem; SdfPath fUsdSrcPath; UsdSceneItem::Ptr fUfeDstItem; SdfPath fUsdDstPath; diff --git a/test/lib/ufe/CMakeLists.txt b/test/lib/ufe/CMakeLists.txt index ed6386d1ec..c9d30470e3 100644 --- a/test/lib/ufe/CMakeLists.txt +++ b/test/lib/ufe/CMakeLists.txt @@ -48,7 +48,7 @@ if(CMAKE_UFE_V2_FEATURES_AVAILABLE) # testRotateCmd.py # testScaleCmd.py # testTransform3dTranslate.py - testRenameProxyShape.py + testRename.py ) endif() diff --git a/test/lib/ufe/testRenameProxyShape.py b/test/lib/ufe/testRename.py similarity index 57% rename from test/lib/ufe/testRenameProxyShape.py rename to test/lib/ufe/testRename.py index 91702892e0..d573cdadf1 100644 --- a/test/lib/ufe/testRenameProxyShape.py +++ b/test/lib/ufe/testRename.py @@ -24,10 +24,10 @@ import unittest -class RenameProxyShapeTestCase(unittest.TestCase): - '''Test renaming the proxy shape and its ancestors. +class RenameTestCase(unittest.TestCase): + '''Test renaming a UFE scene item and its ancestors. - Renaming should not affect UFE lookup. + Renaming should not affect UFE lookup, including renaming the proxy shape. ''' pluginsLoaded = False @@ -37,6 +37,10 @@ def setUpClass(cls): if not cls.pluginsLoaded: cls.pluginsLoaded = mayaUtils.isMayaUsdPluginLoaded() + @classmethod + def tearDownClass(cls): + cmds.file(new=True, force=True) + def setUp(self): ''' Called initially to set up the Maya test environment ''' # Load plugins @@ -90,3 +94,62 @@ def assertStageAndPrimAccess( [str(segment) for segment in ball35Path.segments]) assertStageAndPrimAccess(mayaSegment, ball35PathStr, usdSegment) + + def testRename(self): + '''Rename USD node.''' + + # Select a USD object. + ball35Path = ufe.Path([ + mayaUtils.createUfePathSegment( + "|world|transform1|proxyShape1"), + usdUtils.createUfePathSegment("/Room_set/Props/Ball_35")]) + ball35Item = ufe.Hierarchy.createItem(ball35Path) + ball35Hierarchy = ufe.Hierarchy.hierarchy(ball35Item) + propsItem = ball35Hierarchy.parent() + + propsHierarchy = ufe.Hierarchy.hierarchy(propsItem) + propsChildrenPre = propsHierarchy.children() + + ufe.GlobalSelection.get().append(ball35Item) + + newName = 'Ball_35_Renamed' + cmds.rename(newName) + + # The renamed item is in the selection. + snIter = iter(ufe.GlobalSelection.get()) + ball35RenItem = next(snIter) + ball35RenName = str(ball35RenItem.path().back()) + + self.assertEqual(ball35RenName, newName) + + # MAYA-92350: should not need to re-bind hierarchy interface objects + # with their item. + propsHierarchy = ufe.Hierarchy.hierarchy(propsItem) + propsChildren = propsHierarchy.children() + + self.assertEqual(len(propsChildren), len(propsChildrenPre)) + self.assertIn(ball35RenItem, propsChildren) + + # Rename undo / redo does not work yet. PPT, 28-Jan-2020. + return + cmds.undo() + + def childrenNames(children): + return [str(child.path().back()) for child in children] + + propsHierarchy = ufe.Hierarchy.hierarchy(propsItem) + propsChildren = propsHierarchy.children() + propsChildrenNames = childrenNames(propsChildren) + + self.assertNotIn(ball35RenName, propsChildrenNames) + self.assertIn('Ball_35', propsChildrenNames) + + cmds.redo() + + propsHierarchy = ufe.Hierarchy.hierarchy(propsItem) + propsChildren = propsHierarchy.children() + propsChildrenNames = childrenNames(propsChildren) + + self.assertIn(ball35RenName, propsChildrenNames) + self.assertNotIn('Ball_35', propsChildrenNames) + From b4d49f4823100a353b3160e59d43919a784a39af Mon Sep 17 00:00:00 2001 From: Pierre Tremblay Date: Tue, 4 Feb 2020 07:47:21 -0800 Subject: [PATCH 3/8] Fixed rename undo / redo, and cleaned up for merge. --- lib/ufe/UsdStageMap.cpp | 10 ---------- lib/ufe/UsdUndoRenameCommand.cpp | 6 ++++++ lib/ufe/Utils.cpp | 18 ------------------ lib/ufe/wrapUtils.cpp | 2 -- test/lib/ufe/testRename.py | 4 ++-- 5 files changed, 8 insertions(+), 32 deletions(-) diff --git a/lib/ufe/UsdStageMap.cpp b/lib/ufe/UsdStageMap.cpp index e984f26c0c..c20c562014 100644 --- a/lib/ufe/UsdStageMap.cpp +++ b/lib/ufe/UsdStageMap.cpp @@ -19,14 +19,10 @@ #include -#include #include namespace { -int stageCalls = 0; -int pathCalls = 0; - MObjectHandle proxyShapeHandle(const Ufe::Path& path) { // Get the MObjectHandle from the tail of the MDagPath. Remove the leading @@ -93,9 +89,6 @@ void UsdStageMap::addItem(const Ufe::Path& path, UsdStageWeakPtr stage) UsdStageWeakPtr UsdStageMap::stage(const Ufe::Path& path) const { - ++stageCalls; - std::cout << "PPT: stage calls is " << stageCalls << std::endl; - auto proxyShape = proxyShapeHandle(path); if (!proxyShape.isValid()) { return nullptr; @@ -110,9 +103,6 @@ UsdStageWeakPtr UsdStageMap::stage(const Ufe::Path& path) const Ufe::Path UsdStageMap::path(UsdStageWeakPtr stage) const { - ++pathCalls; - std::cout << "PPT: path calls is " << pathCalls << std::endl; - // A stage is bound to a single Dag proxy shape. auto iter = fStageToObject.find(stage); if (iter != std::end(fStageToObject)) diff --git a/lib/ufe/UsdUndoRenameCommand.cpp b/lib/ufe/UsdUndoRenameCommand.cpp index b0927565a3..fa821746da 100644 --- a/lib/ufe/UsdUndoRenameCommand.cpp +++ b/lib/ufe/UsdUndoRenameCommand.cpp @@ -98,7 +98,13 @@ bool UsdUndoRenameCommand::renameRedo() bool UsdUndoRenameCommand::renameUndo() { + // Regardless of where the edit target is currently set, switch to the + // layer where we copied the source prim into the destination, then + // restore the edit target. + auto editTarget = fStage->GetEditTarget(); + fStage->SetEditTarget(fLayer); bool status = fStage->RemovePrim(fUsdDstPath); + fStage->SetEditTarget(editTarget); if (status) { auto srcPrim = fStage->GetPrimAtPath(fUsdSrcPath); UFE_ASSERT_MSG(srcPrim, "Invalid prim cannot be activated."); diff --git a/lib/ufe/Utils.cpp b/lib/ufe/Utils.cpp index 8e91e3a66b..936722f8e7 100644 --- a/lib/ufe/Utils.cpp +++ b/lib/ufe/Utils.cpp @@ -32,8 +32,6 @@ #include #include -#include - PXR_NAMESPACE_USING_DIRECTIVE #ifndef MAYA_MSTRINGARRAY_ITERATOR_CATEGORY @@ -105,32 +103,16 @@ SdfLayerHandle defPrimSpecLayer(const UsdPrim& prim) // The source layer is the one in which there exists a def primSpec, not // an over. - std::cout << "PPT: in defPrimSpecLayer, argument prim is " - << (prim.IsValid() ? "" : "NOT ") << "valid" << std::endl; - SdfLayerHandle defLayer; auto layerStack = prim.GetStage()->GetLayerStack(); auto stage = prim.GetStage(); auto primFromPath = stage->GetPrimAtPath(prim.GetPath()); - std::cout << "PPT: stage.GetPrimAtPath() returns a " - << (prim.IsValid() ? "VALID " : "INVALID ") << "prim" - << std::endl; for (auto layer : layerStack) { - std::cout << "PPT: iterating over layer " << layer->GetIdentifier() - << std::endl; - std::cout << "PPT: prim path is " << prim.GetPath().GetString() - << std::endl; auto primSpec = layer->GetPrimAtPath(prim.GetPath()); - std::cout << "PPT: primSpec is " << (primSpec ? "NOT " : "") << "null" - << std::endl; - std::cout << "PPT: layer->HasSpec() returns " - << (layer->HasSpec(prim.GetPath()) ? "TRUE" : "FALSE") - << std::endl; if (primSpec && (primSpec->GetSpecifier() == SdfSpecifierDef)) { - std::cout << "PPT: found layer with def primSpec." << std::endl; defLayer = layer; break; } diff --git a/lib/ufe/wrapUtils.cpp b/lib/ufe/wrapUtils.cpp index 7904f6b6f5..86587b00aa 100644 --- a/lib/ufe/wrapUtils.cpp +++ b/lib/ufe/wrapUtils.cpp @@ -35,8 +35,6 @@ std::size_t find( return foundPos == std::string::npos ? size : foundPos; } -// Copy elision (return value optimization) will avoid duplicate std::vector -// construction on return. std::vector split(const std::string& src, char sep) { std::vector names; diff --git a/test/lib/ufe/testRename.py b/test/lib/ufe/testRename.py index d573cdadf1..da86e5c409 100644 --- a/test/lib/ufe/testRename.py +++ b/test/lib/ufe/testRename.py @@ -130,8 +130,6 @@ def testRename(self): self.assertEqual(len(propsChildren), len(propsChildrenPre)) self.assertIn(ball35RenItem, propsChildren) - # Rename undo / redo does not work yet. PPT, 28-Jan-2020. - return cmds.undo() def childrenNames(children): @@ -143,6 +141,7 @@ def childrenNames(children): self.assertNotIn(ball35RenName, propsChildrenNames) self.assertIn('Ball_35', propsChildrenNames) + self.assertEqual(len(propsChildren), len(propsChildrenPre)) cmds.redo() @@ -152,4 +151,5 @@ def childrenNames(children): self.assertIn(ball35RenName, propsChildrenNames) self.assertNotIn('Ball_35', propsChildrenNames) + self.assertEqual(len(propsChildren), len(propsChildrenPre)) From f53f29c7258bc1d99897d843dbfef2e0316dd702 Mon Sep 17 00:00:00 2001 From: Pierre Tremblay Date: Thu, 6 Feb 2020 12:03:17 -0800 Subject: [PATCH 4/8] Implemented code review suggestions. --- lib/ufe/ProxyShapeHierarchy.cpp | 4 +- lib/ufe/ProxyShapeHierarchy.h | 6 +-- lib/ufe/StagesSubject.cpp | 44 ++++++++++-------- lib/ufe/UsdStageMap.cpp | 80 ++++++++++++++++---------------- lib/ufe/UsdUndoRenameCommand.cpp | 16 ++++--- lib/ufe/wrapUtils.cpp | 32 ++----------- 6 files changed, 81 insertions(+), 101 deletions(-) diff --git a/lib/ufe/ProxyShapeHierarchy.cpp b/lib/ufe/ProxyShapeHierarchy.cpp index 2c492ec1f9..1ca4ff52f1 100644 --- a/lib/ufe/ProxyShapeHierarchy.cpp +++ b/lib/ufe/ProxyShapeHierarchy.cpp @@ -110,9 +110,9 @@ bool ProxyShapeHierarchy::hasChildren() const { const UsdPrim& rootPrim = getUsdRootPrim(); if (!rootPrim.IsValid()) { - UFE_LOG("invalid root prim in ProxyShapeHierarchy::hasChildren()"); + UFE_LOG("invalid root prim in ProxyShapeHierarchy::hasChildren()"); return false; - } + } return !rootPrim.GetChildren().empty(); } diff --git a/lib/ufe/ProxyShapeHierarchy.h b/lib/ufe/ProxyShapeHierarchy.h index 5073e29b88..61f25dfa84 100644 --- a/lib/ufe/ProxyShapeHierarchy.h +++ b/lib/ufe/ProxyShapeHierarchy.h @@ -72,9 +72,9 @@ class MAYAUSD_CORE_PUBLIC ProxyShapeHierarchy : public Ufe::Hierarchy const UsdPrim& getUsdRootPrim() const; private: - Ufe::SceneItem::Ptr fItem{nullptr}; - Hierarchy::Ptr fMayaHierarchy{nullptr}; - Ufe::HierarchyHandler::Ptr fMayaHierarchyHandler{nullptr}; + Ufe::SceneItem::Ptr fItem; + Hierarchy::Ptr fMayaHierarchy; + Ufe::HierarchyHandler::Ptr fMayaHierarchyHandler; // The root prim is initialized on first use and therefore mutable. mutable UsdPrim fUsdRootPrim; diff --git a/lib/ufe/StagesSubject.cpp b/lib/ufe/StagesSubject.cpp index 48f8db0961..1ec2080898 100644 --- a/lib/ufe/StagesSubject.cpp +++ b/lib/ufe/StagesSubject.cpp @@ -174,32 +174,36 @@ void StagesSubject::stageChanged(UsdNotice::ObjectsChanged const& notice, UsdSta for (const auto& changedPath : notice.GetResyncedPaths()) { const std::string& usdPrimPathStr = changedPath.GetPrimPath().GetString(); + // Assume proxy shapes (and thus stages) cannot be instanced. We can + // therefore map the stage to a single UFE path. Lifting this + // restriction would mean sending one add or delete notification for + // each Maya Dag path instancing the proxy shape / stage. Ufe::Path ufePath = stagePath(sender) + Ufe::PathSegment(usdPrimPathStr, g_USDRtid, '/'); auto prim = stage->GetPrimAtPath(changedPath); // Changed paths could be xformOps. // These are considered as invalid null prims if (prim.IsValid() && !InPathChange::inPathChange()) { - auto sceneItem = Ufe::Hierarchy::createItem(ufePath); - - // AL LayerCommands.addSubLayer test will cause Maya to crash - // if we don't filter invalid sceneItems. This patch is provided - // to prevent crashes, but more investigation will have to be - // done to understand why ufePath in case of sub layer - // creation causes Ufe::Hierarchy::createItem to fail. - if (!sceneItem) - continue; - - if (prim.IsActive()) - { - auto notification = Ufe::ObjectAdd(sceneItem); - Ufe::Scene::notifyObjectAdd(notification); - } - else - { - auto notification = Ufe::ObjectPostDelete(sceneItem); - Ufe::Scene::notifyObjectDelete(notification); - } + auto sceneItem = Ufe::Hierarchy::createItem(ufePath); + + // AL LayerCommands.addSubLayer test will cause Maya to crash + // if we don't filter invalid sceneItems. This patch is provided + // to prevent crashes, but more investigation will have to be + // done to understand why ufePath in case of sub layer + // creation causes Ufe::Hierarchy::createItem to fail. + if (!sceneItem) + continue; + + if (prim.IsActive()) + { + auto notification = Ufe::ObjectAdd(sceneItem); + Ufe::Scene::notifyObjectAdd(notification); + } + else + { + auto notification = Ufe::ObjectPostDelete(sceneItem); + Ufe::Scene::notifyObjectDelete(notification); + } } } diff --git a/lib/ufe/UsdStageMap.cpp b/lib/ufe/UsdStageMap.cpp index c20c562014..15dc3e7e22 100644 --- a/lib/ufe/UsdStageMap.cpp +++ b/lib/ufe/UsdStageMap.cpp @@ -25,30 +25,30 @@ namespace { MObjectHandle proxyShapeHandle(const Ufe::Path& path) { - // Get the MObjectHandle from the tail of the MDagPath. Remove the leading - // '|world' component. - auto noWorld = path.popHead().string(); - auto dagPath = MayaUsd::ufe::nameToDagPath(noWorld); - MObjectHandle handle(dagPath.node()); - if (!handle.isValid()) { - TF_CODING_ERROR("'%s' is not a path to a proxy shape node.", - noWorld.c_str()); - } - return handle; + // Get the MObjectHandle from the tail of the MDagPath. Remove the leading + // '|world' component. + auto noWorld = path.popHead().string(); + auto dagPath = MayaUsd::ufe::nameToDagPath(noWorld); + MObjectHandle handle(dagPath.node()); + if (!handle.isValid()) { + TF_CODING_ERROR("'%s' is not a path to a proxy shape node.", + noWorld.c_str()); + } + return handle; } // Assuming proxy shape nodes cannot be instanced, simply return the first path. Ufe::Path firstPath(const MObjectHandle& handle) { - if (!TF_VERIFY(handle.isValid(), - "Cannot get path from invalid object handle")) { - return Ufe::Path(); - } - - MDagPath dagPath; - auto status = MFnDagNode(handle.object()).getPath(dagPath); - CHECK_MSTATUS(status); - return MayaUsd::ufe::dagPathToUfe(dagPath); + if (!TF_VERIFY(handle.isValid(), + "Cannot get path from invalid object handle")) { + return Ufe::Path(); + } + + MDagPath dagPath; + auto status = MFnDagNode(handle.object()).getPath(dagPath); + CHECK_MSTATUS(status); + return MayaUsd::ufe::dagPathToUfe(dagPath); } } @@ -68,31 +68,31 @@ UsdStageMap g_StageMap; void UsdStageMap::addItem(const Ufe::Path& path, UsdStageWeakPtr stage) { - // We expect a path to the proxy shape node, therefore a single segment. - auto nbSegments = path.nbSegments(); - if (nbSegments != 1) { - TF_CODING_ERROR("A proxy shape node path can have only one segment, path '%s' has %d", path.string().c_str(), nbSegments); - return; - } - - // Convert the tail of the UFE path to an MObjectHandle. - auto proxyShape = proxyShapeHandle(path); - if (!proxyShape.isValid()) { - return; - } - - // Could get the stage from the proxy shape object in the stage() method, - // but since it's given here, simply store it. - fObjectToStage[proxyShape] = stage; - fStageToObject[stage] = proxyShape; + // We expect a path to the proxy shape node, therefore a single segment. + auto nbSegments = path.nbSegments(); + if (nbSegments != 1) { + TF_CODING_ERROR("A proxy shape node path can have only one segment, path '%s' has %d", path.string().c_str(), nbSegments); + return; + } + + // Convert the tail of the UFE path to an MObjectHandle. + auto proxyShape = proxyShapeHandle(path); + if (!proxyShape.isValid()) { + return; + } + + // Could get the stage from the proxy shape object in the stage() method, + // but since it's given here, simply store it. + fObjectToStage[proxyShape] = stage; + fStageToObject[stage] = proxyShape; } UsdStageWeakPtr UsdStageMap::stage(const Ufe::Path& path) const { - auto proxyShape = proxyShapeHandle(path); - if (!proxyShape.isValid()) { - return nullptr; - } + auto proxyShape = proxyShapeHandle(path); + if (!proxyShape.isValid()) { + return nullptr; + } // A stage is bound to a single Dag proxy shape. auto iter = fObjectToStage.find(proxyShape); diff --git a/lib/ufe/UsdUndoRenameCommand.cpp b/lib/ufe/UsdUndoRenameCommand.cpp index fa821746da..0432e1940c 100644 --- a/lib/ufe/UsdUndoRenameCommand.cpp +++ b/lib/ufe/UsdUndoRenameCommand.cpp @@ -25,6 +25,7 @@ #include #include +#include #include #include @@ -98,13 +99,14 @@ bool UsdUndoRenameCommand::renameRedo() bool UsdUndoRenameCommand::renameUndo() { - // Regardless of where the edit target is currently set, switch to the - // layer where we copied the source prim into the destination, then - // restore the edit target. - auto editTarget = fStage->GetEditTarget(); - fStage->SetEditTarget(fLayer); - bool status = fStage->RemovePrim(fUsdDstPath); - fStage->SetEditTarget(editTarget); + bool status{false}; + { + // Regardless of where the edit target is currently set, switch to the + // layer where we copied the source prim into the destination, then + // restore the edit target. + UsdEditContext ctx(fStage, fLayer); + status = fStage->RemovePrim(fUsdDstPath); + } if (status) { auto srcPrim = fStage->GetPrimAtPath(fUsdSrcPath); UFE_ASSERT_MSG(srcPrim, "Invalid prim cannot be activated."); diff --git a/lib/ufe/wrapUtils.cpp b/lib/ufe/wrapUtils.cpp index 86587b00aa..a42bd88b5a 100644 --- a/lib/ufe/wrapUtils.cpp +++ b/lib/ufe/wrapUtils.cpp @@ -23,37 +23,11 @@ #include #include +#include + using namespace MayaUsd; using namespace boost::python; -namespace { - -std::size_t find( - const std::string& s, std::size_t size, std::size_t begin, char sep) -{ - std::size_t foundPos = s.find(sep, begin); - return foundPos == std::string::npos ? size : foundPos; -} - -std::vector split(const std::string& src, char sep) -{ - std::vector names; - if (sep) { - std::size_t end = 0; - const std::size_t size = src.size(); - for (std::size_t begin = 0; begin < size; begin = end+1) { - end = find(src, size, begin, sep); - names.push_back(src.substr(begin, end-begin)); - } - } - else { - names.push_back(src); - } - return names; -} - -} - UsdPrim getPrimFromRawItem(uint64_t rawItem) { Ufe::SceneItem* item = reinterpret_cast(rawItem); @@ -107,7 +81,7 @@ UsdPrim ufePathToPrim(const std::string& ufePathString) { // The path string is a list of segment strings separated by ',' comma // separator. - auto segmentStrings = split(ufePathString, ','); + auto segmentStrings = TfStringTokenize(ufePathString, ","); // If there's just one segment, it's the Maya Dag path segment, so it can't // have a prim. From e508bdb626a80dcf58d08e17c85867514a36dd5b Mon Sep 17 00:00:00 2001 From: Pierre Tremblay Date: Mon, 10 Feb 2020 11:19:10 -0800 Subject: [PATCH 5/8] Fixed UFE v1 compilation issue. --- lib/ufe/UsdStageMap.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/ufe/UsdStageMap.cpp b/lib/ufe/UsdStageMap.cpp index 15dc3e7e22..882dbbbf90 100644 --- a/lib/ufe/UsdStageMap.cpp +++ b/lib/ufe/UsdStageMap.cpp @@ -69,7 +69,12 @@ UsdStageMap g_StageMap; void UsdStageMap::addItem(const Ufe::Path& path, UsdStageWeakPtr stage) { // We expect a path to the proxy shape node, therefore a single segment. - auto nbSegments = path.nbSegments(); + auto nbSegments = +#ifdef UFE_V2_FEATURES_AVAILABLE + path.nbSegments(); +#else + path.getSegments().size(); +#endif if (nbSegments != 1) { TF_CODING_ERROR("A proxy shape node path can have only one segment, path '%s' has %d", path.string().c_str(), nbSegments); return; From 25bac95859ae76be409b4b37f141d0366df6d896 Mon Sep 17 00:00:00 2001 From: Pierre Tremblay Date: Tue, 11 Feb 2020 07:27:10 -0800 Subject: [PATCH 6/8] Fixed compilation for Maya preview release 112. --- lib/ufe/UsdStageMap.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ufe/UsdStageMap.cpp b/lib/ufe/UsdStageMap.cpp index 882dbbbf90..11aeafebf3 100644 --- a/lib/ufe/UsdStageMap.cpp +++ b/lib/ufe/UsdStageMap.cpp @@ -70,7 +70,7 @@ void UsdStageMap::addItem(const Ufe::Path& path, UsdStageWeakPtr stage) { // We expect a path to the proxy shape node, therefore a single segment. auto nbSegments = -#ifdef UFE_V2_FEATURES_AVAILABLE +#ifdef UFE_V0_2_6_FEATURES_AVAILABLE path.nbSegments(); #else path.getSegments().size(); From c0277f094be8ff9ad47f82f2b5212e4b48682670 Mon Sep 17 00:00:00 2001 From: Pierre Tremblay Date: Tue, 11 Feb 2020 07:49:48 -0800 Subject: [PATCH 7/8] Fix macOS compilation warning as error. --- lib/ufe/UsdStageMap.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ufe/UsdStageMap.cpp b/lib/ufe/UsdStageMap.cpp index 11aeafebf3..97b7f6d57d 100644 --- a/lib/ufe/UsdStageMap.cpp +++ b/lib/ufe/UsdStageMap.cpp @@ -76,7 +76,7 @@ void UsdStageMap::addItem(const Ufe::Path& path, UsdStageWeakPtr stage) path.getSegments().size(); #endif if (nbSegments != 1) { - TF_CODING_ERROR("A proxy shape node path can have only one segment, path '%s' has %d", path.string().c_str(), nbSegments); + TF_CODING_ERROR("A proxy shape node path can have only one segment, path '%s' has %lu", path.string().c_str(), nbSegments); return; } From 30db4e2a1b325e074e3659098f39151196a8a54c Mon Sep 17 00:00:00 2001 From: Pierre Tremblay Date: Tue, 11 Feb 2020 10:48:10 -0800 Subject: [PATCH 8/8] Fixed UFE v1 compilation. --- lib/ufe/UsdUndoRenameCommand.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lib/ufe/UsdUndoRenameCommand.cpp b/lib/ufe/UsdUndoRenameCommand.cpp index 0432e1940c..679623fbdf 100644 --- a/lib/ufe/UsdUndoRenameCommand.cpp +++ b/lib/ufe/UsdUndoRenameCommand.cpp @@ -21,8 +21,12 @@ #include #include #include +#ifdef UFE_V2_FEATURES_AVAILABLE #define UFE_ENABLE_ASSERTS #include +#else +#include +#endif #include #include @@ -76,7 +80,11 @@ bool UsdUndoRenameCommand::renameRedo() if (status) { auto srcPrim = fStage->GetPrimAtPath(fUsdSrcPath); +#ifdef UFE_V2_FEATURES_AVAILABLE UFE_ASSERT_MSG(srcPrim, "Invalid prim cannot be inactivated."); +#else + assert(srcPrim); +#endif status = srcPrim.SetActive(false); if (status) { @@ -109,7 +117,11 @@ bool UsdUndoRenameCommand::renameUndo() } if (status) { auto srcPrim = fStage->GetPrimAtPath(fUsdSrcPath); +#ifdef UFE_V2_FEATURES_AVAILABLE UFE_ASSERT_MSG(srcPrim, "Invalid prim cannot be activated."); +#else + assert(srcPrim); +#endif status = srcPrim.SetActive(true); if (status) {