Fixes from code review

lib/flowViewport/sceneIndex/fvpSelectionSceneIndex.cpp

@@ -275,9 +275,9 @@ PrimSelections SelectionSceneIndex::UfePathToPrimSelections(const Ufe::Path& app
             TF_WARN("SelectionSceneIndex::UfePathToPrimSelections(%s) returned no path, Hydra selection will be incorrect", Ufe::PathString::string(appPath).c_str());
         };
 
-        //Don't emit a warning if there is no mapper for this path, _selectionSceneIndex->ReplaceSelection may be called 
-        // later again if there are some usd stage scene index or custom producer scene indices added to the merging scene index.
-        if (mapper) {
+        if (!mapper) {
+            warnEmptyPath(appPath);
+        } else {
             primSelections = mapper->UfePathToPrimSelections(appPath);
             if (primSelections.empty()) {
                 warnEmptyPath(appPath);

lib/mayaHydra/hydraExtensions/CMakeLists.txt

@@ -16,7 +16,7 @@ target_sources(${TARGET_NAME}
         mhWireframeColorInterfaceImp.cpp
         mhLeadObjectPathTracker.cpp
         tokens.cpp
-		mhDataProducersMayaNodeToSdfPathRegistry.cpp
+        mhDataProducersMayaNodeToSdfPathRegistry.cpp
 )
 
 set(HEADERS
@@ -31,7 +31,7 @@ set(HEADERS
     mhWireframeColorInterfaceImp.h
     mhLeadObjectPathTracker.h
     tokens.h
-	mhDataProducersMayaNodeToSdfPathRegistry.h
+    mhDataProducersMayaNodeToSdfPathRegistry.h
 )
 
 # -----------------------------------------------------------------------------

lib/mayaHydra/hydraExtensions/mhDataProducersMayaNodeToSdfPathRegistry.cpp

@@ -15,64 +15,49 @@
 //
 
 #include "mhDataProducersMayaNodeToSdfPathRegistry.h"
+#include <pxr/base/tf/instantiateSingleton.h>
 
 PXR_NAMESPACE_USING_DIRECTIVE
+TF_INSTANTIATE_SINGLETON(MAYAHYDRA_NS_DEF::MhDataProducersMayaNodeToSdfPathRegistry);
 
 namespace MAYAHYDRA_NS_DEF {
 
-std::unique_ptr<MhDataProducersMayaNodeToSdfPathRegistry>   MhDataProducersMayaNodeToSdfPathRegistry::_instance;
-
-MhDataProducersMayaNodeToSdfPathRegistry& MhDataProducersMayaNodeToSdfPathRegistry::Get()
+/* static */
+MhDataProducersMayaNodeToSdfPathRegistry& MhDataProducersMayaNodeToSdfPathRegistry::Instance()
 {
-    if (! _instance){
-        _instance.reset(new MhDataProducersMayaNodeToSdfPathRegistry());
-    }
-    return *_instance;
+    return PXR_NS::TfSingleton<MhDataProducersMayaNodeToSdfPathRegistry>::GetInstance();
 }
 
-void MhDataProducersMayaNodeToSdfPathRegistry::Add(const MObjectHandle& objectHandle, const SdfPath& thePath)
+void MhDataProducersMayaNodeToSdfPathRegistry::Add(const unsigned long& objectHandleHashCode, const SdfPath& thePath)
 {
-    if (thePath.IsEmpty() || ! objectHandle.isValid() ) {
-        TF_CODING_WARNING("Sending an empty SdfPath or an invalid objectHandle in MhDataProducersMayaNodeToSdfPathRegistry::Add, ignoring");
+    if (thePath.IsEmpty() || 0 == objectHandleHashCode) {
+        TF_CODING_WARNING("Sending an empty SdfPath or an invalid objectHandle has code in MhDataProducersMayaNodeToSdfPathRegistry::Add, ignoring");
         return;
     }
 
-    _SdfPathByObjectHandle.insert({ objectHandle, thePath });
+    _SdfPathByHashCode.insert({ objectHandleHashCode, thePath });
 }
 
-void MhDataProducersMayaNodeToSdfPathRegistry::Remove(unsigned long& objectHandleHashCode) 
+void MhDataProducersMayaNodeToSdfPathRegistry::Remove(const unsigned long& objectHandleHashCode) 
 { 
     if (0 == objectHandleHashCode) {
         return;
     }
 
-    for (auto it = _SdfPathByObjectHandle.begin(); it != _SdfPathByObjectHandle.end(); ++it) {
-        if (it->first.hashCode() == objectHandleHashCode) {
-            _SdfPathByObjectHandle.erase(it);
-            return;
-        }
+    auto it = _SdfPathByHashCode.find(objectHandleHashCode);
+    if (it != _SdfPathByHashCode.end()) {
+        _SdfPathByHashCode.erase(it);
     }
 }
 
-void MhDataProducersMayaNodeToSdfPathRegistry::Remove(const MObjectHandle& objectHandle)
+SdfPath MhDataProducersMayaNodeToSdfPathRegistry::GetPath(const unsigned long& objectHandleHashCode) const
 {
-    if(! objectHandle.isValid()){
-        return;
-    }
-    auto it = _SdfPathByObjectHandle.find(objectHandle);
-    if (it != _SdfPathByObjectHandle.end()) { 
-        _SdfPathByObjectHandle.erase(it);
-    }
-}
-
-SdfPath MhDataProducersMayaNodeToSdfPathRegistry::GetPath(const MObjectHandle& objectHandle) const
-{
-    if (!objectHandle.isValid()) {
-        TF_CODING_WARNING("Sending an invalid objectHandle in MhDataProducersMayaNodeToSdfPathRegistry::GetPath");
+    if (0 == objectHandleHashCode) {
+        TF_CODING_WARNING("Sending an invalid objectHandleHashCode in MhDataProducersMayaNodeToSdfPathRegistry::GetPath");
         return {};
     }
-    auto it = _SdfPathByObjectHandle.find(objectHandle);
-    if (it != _SdfPathByObjectHandle.end()) {
+    auto it = _SdfPathByHashCode.find(objectHandleHashCode);
+    if (it != _SdfPathByHashCode.end()) {
         return (it->second);
     }
     return SdfPath();

lib/mayaHydra/hydraExtensions/mhDataProducersMayaNodeToSdfPathRegistry.h

@@ -20,6 +20,7 @@
 #include "mayaHydraLib/api.h"
 
 //Usd headers
+#include <pxr/base/tf/singleton.h>
 #include <pxr/usd/sdf/path.h>
 
 //Maya headers
@@ -29,41 +30,38 @@
 
 namespace MAYAHYDRA_NS_DEF {
 
-/// MhDataProducersMayaNodeToSdfPathRegistry does a mapping between Maya nodes and USD paths. 
+/// MhDataProducersMayaNodeToSdfPathRegistry does a mapping between Maya nodes and Hydra paths. 
 /// The maya nodes registered in this class are used by data producers as a parent to all primitives.
 /// The registration/unregistration in this class is automatic when you use the flow viewport API and provide a maya node as a parent.
 /// This class is used when we select one of these maya nodes to return the matching SdfPath so that all prims child of this maya node are highlighted.
 class MhDataProducersMayaNodeToSdfPathRegistry 
 {
 public:
+    // Access the singleton instance
     MAYAHYDRALIB_API
-    static MhDataProducersMayaNodeToSdfPathRegistry& Get();
+    static MhDataProducersMayaNodeToSdfPathRegistry& Instance();
 
     MAYAHYDRALIB_API
-    void Add(const MObjectHandle& objectHandle, const PXR_NS::SdfPath& thePath);
+    void Add(const unsigned long& objectHandlehashCode, const PXR_NS::SdfPath& thePath);
 
     MAYAHYDRALIB_API
-    void Remove(const MObjectHandle& objectHandle);
-
-    //When removing a node that has been deleted we cannot get its MObjectHandle, so we use the hash code
-    MAYAHYDRALIB_API
-    void Remove(unsigned long& objectHandleHashCode);
+    void Remove(const unsigned long& objectHandleHashCode);
 
     // Returns an empty SdfPath if the objectHandle is not registered or the matching SdfPath
     // otherwise
     MAYAHYDRALIB_API
-    PXR_NS::SdfPath GetPath(const MObjectHandle& objectHandle) const;
+    PXR_NS::SdfPath GetPath(const unsigned long& objectHandleHashCode) const;
 
 private:
      // Singleton, no public creation or copy.
     MhDataProducersMayaNodeToSdfPathRegistry() = default;
+    ~MhDataProducersMayaNodeToSdfPathRegistry() = default;
     MhDataProducersMayaNodeToSdfPathRegistry(const MhDataProducersMayaNodeToSdfPathRegistry& ) = delete;
+    MhDataProducersMayaNodeToSdfPathRegistry& operator=(const MhDataProducersMayaNodeToSdfPathRegistry& ) = delete;
+
+    friend class PXR_NS::TfSingleton<MhDataProducersMayaNodeToSdfPathRegistry>;
 
-    struct _HashObjectHandle
-    {
-        unsigned long operator()(const MObjectHandle& handle) const { return handle.hashCode(); }
-    };
-    std::unordered_map<MObjectHandle, PXR_NS::SdfPath, _HashObjectHandle> _SdfPathByObjectHandle;
+    std::unordered_map<unsigned long, PXR_NS::SdfPath> _SdfPathByHashCode;
 
     static std::unique_ptr<MhDataProducersMayaNodeToSdfPathRegistry>   _instance;
 };

lib/mayaHydra/hydraExtensions/sceneIndex/mayaHydraMayaDataProducerSceneIndexData.cpp

@@ -70,7 +70,7 @@ MayaDataProducerSceneIndexData::~MayaDataProducerSceneIndexData()
     }
     if (0 != _dccNodeHashCode){
         //Remove the node from the registry
-        MAYAHYDRA_NS::MhDataProducersMayaNodeToSdfPathRegistry::Get().Remove(_dccNodeHashCode);
+        MAYAHYDRA_NS::MhDataProducersMayaNodeToSdfPathRegistry::Instance().Remove(_dccNodeHashCode);
     }
 }
 
@@ -89,8 +89,8 @@ void MayaDataProducerSceneIndexData::SetupDCCNode()
             if (!dagPath.node().isNull()) {
                 MObjectHandle hdl(dagPath.node());
                 _dccNodeHashCode = hdl.hashCode();
-                MAYAHYDRA_NS::MhDataProducersMayaNodeToSdfPathRegistry::Get().Add(
-                    hdl, _prefix);
+                MAYAHYDRA_NS::MhDataProducersMayaNodeToSdfPathRegistry::Instance().Add(
+                    _dccNodeHashCode, _prefix);
             }
         }
 

lib/mayaHydra/hydraExtensions/sceneIndex/mayaHydraSceneIndex.cpp

@@ -771,7 +771,7 @@ Fvp::PrimSelections MayaHydraSceneIndex::UfePathToPrimSelections(const Ufe::Path
     //highlighted.
     MDagPath shapeDagPath(dagPath);
     shapeDagPath.extendToShape();
-    const SdfPath matchingPath = MhDataProducersMayaNodeToSdfPathRegistry::Get().GetPath(MObjectHandle(shapeDagPath.node()));
+    const SdfPath matchingPath = MhDataProducersMayaNodeToSdfPathRegistry::Instance().GetPath(MObjectHandle(shapeDagPath.node()).hashCode());
     if (! matchingPath.IsEmpty()) {
         primPath = matchingPath;
     }

lib/mayaHydra/mayaPlugin/renderOverride.cpp

@@ -671,7 +671,7 @@ MStatus MtohRenderOverride::Render(
             const bool dataProducerSceneIndicesAdded = manager.AddViewportInformation(hydraViewportInformation, _renderIndexProxy, _lastFilteringSceneIndexBeforeCustomFiltering);
             //Update the selection since we have added data producer scene indices through manager.AddViewportInformation to the merging scene index
             if (dataProducerSceneIndicesAdded && _selectionSceneIndex){
-                _selectionSceneIndex->ReplaceSelection(*Ufe::GlobalSelection::get());
+                _needToReplaceSelection = true;
             }
             //Update the leadObjectTacker in case it could not find the current lead object which could be in a custom data producer scene index or a maya usd proxy shape scene index
             if (_leadObjectPathTracker){
@@ -680,6 +680,11 @@ MStatus MtohRenderOverride::Render(
         }
     }
 
+    if (_needToReplaceSelection){
+        _selectionSceneIndex->ReplaceSelection(*Ufe::GlobalSelection::get());
+        _needToReplaceSelection = false;
+    }
+
     const unsigned int currentDisplayStyle = drawContext.getDisplayStyle();
     MayaHydraParams delegateParams = _globals.delegateParams;
     delegateParams.displaySmoothMeshes = !(currentDisplayStyle & MHWRender::MFrameContext::kFlatShaded);
@@ -1013,9 +1018,8 @@ void MtohRenderOverride::_InitHydraResources(const MHWRender::MDrawContext& draw
     _dirtyLeadObjectSceneIndex = MAYAHYDRA_NS::MhDirtyLeadObjectSceneIndex::New(_inputSceneIndexOfFilteringSceneIndicesChain);
     _inputSceneIndexOfFilteringSceneIndicesChain = _dirtyLeadObjectSceneIndex;
 
-    // Set the initial selection onto the selection scene index. 
-    // _selectionSceneIndex->ReplaceSelection may be called later again if there are some usd stage scene index or custom producer scene indices added to the merging scene index
-    _selectionSceneIndex->ReplaceSelection(*Ufe::GlobalSelection::get());
+    // Set the initial selection onto the selection scene index later. 
+    _needToReplaceSelection = true;
 
     _CreateSceneIndicesChainAfterMergingSceneIndex(drawContext);
 

lib/mayaHydra/mayaPlugin/renderOverride.h

@@ -308,6 +308,7 @@ class MtohRenderOverride : public MHWRender::MRenderOverride,
 
     int _currentOperation = -1;
 
+    bool _needToReplaceSelection = false;
     const bool _isUsingHdSt = false;
     bool       _initializationAttempted = false;
     bool       _initializationSucceeded = false;

test/lib/mayaUsd/render/mayaToHydra/testFlowPluginsHierarchicalProperties.py

@@ -61,6 +61,7 @@ def usdStageSetup(self):
     def usdStageAnimatedPrimSetup(self):
         usdScenePath = testUtils.getTestScene('testFlowPluginsHierarchicalProperties', 'usd_animated_prim.usda')
         stagePath =  usdUtils.createStageFromFile(usdScenePath)
+        cmds.select(clear=True)#Clear selection
         stageParent = cmds.group(empty=True)
         cmds.parent(stagePath, stageParent)
         stageTransform = stagePath.split('|')[1]
@@ -91,6 +92,7 @@ def test_Authoring_Locator(self):
         # Change the shape's transform directly
         cmds.xform(cmds.listRelatives(locatorShape, parent=True)[0], translation=[-3,2,-1], rotation=[-15,10,-5], scale=[-2.5, 2.0, -1.5])
         self.assertSnapshotClose("authoring_locator_shapeTransformChanged.png", self.IMAGE_DIFF_FAIL_THRESHOLD, self.IMAGE_DIFF_FAIL_PERCENT)
+        cmds.select(clear=True)#Clear selection
 
     def test_Authoring_UsdStage(self):
         self.setBasicCam(10)

test/lib/mayaUsd/render/mayaToHydra/testFlowViewportAPI.py

@@ -61,6 +61,7 @@ def test_AddingPrimitives(self):
 
             #Create a MhFlowViewportAPILocator node which adds a dataProducerSceneIndex and a Filtering scene index
             flowViewportNodeName = cmds.createNode("MhFlowViewportAPILocator")
+            cmds.select(clear=True)#Clear selection
 
             #Original images are located for example in maya-hydra\test\lib\mayaUsd\render\mayaToHydra\FlowViewportAPITest
             self.assertSnapshotClose("add_NodeCreated.png", self.IMAGE_DIFF_FAIL_THRESHOLD, self.IMAGE_DIFF_FAIL_PERCENT)
@@ -119,6 +120,7 @@ def test_FilteringPrimitives(self):
 
             #Create a MhFlowViewportAPILocator node which adds a dataProducerSceneIndex and a Filtering scene index
             flowViewportNodeName = cmds.createNode("MhFlowViewportAPILocator")
+            cmds.select(clear=True)#Clear selection
             cmds.refresh()
             #Original images are located for example in maya-hydra\test\lib\mayaUsd\render\mayaToHydra\FlowViewportAPITest
             self.assertSnapshotClose("filter_NodeCreated.png", self.IMAGE_DIFF_FAIL_THRESHOLD, self.IMAGE_DIFF_FAIL_PERCENT)
@@ -191,6 +193,7 @@ def test_CubeGrid(self):
 
             #Create a MhFlowViewportAPILocator node which adds a dataProducerSceneIndex and a Filtering scene index
             flowViewportNodeName = cmds.createNode("MhFlowViewportAPILocator")
+            cmds.select(clear=True)#Clear selection
             self.assertSnapshotClose("cubeGrid_BeforeModifs.png", self.IMAGE_DIFF_FAIL_THRESHOLD, self.IMAGE_DIFF_FAIL_PERCENT)
 
             #Get the matrix and set a rotation of 70 degress around Y axis.
@@ -239,6 +242,7 @@ def test_MultipleNodes(self):
 
             #Create a MhFlowViewportAPILocator node which adds a dataProducerSceneIndex and a Filtering scene index
             flowViewportNodeName1 = cmds.createNode("MhFlowViewportAPILocator", n="nodeShape1")
+            cmds.select(clear=True)#Clear selection
 
             #Get the matrix and set a rotation of 70 degress around Y axis.
             matrix = cmds.getAttr(flowViewportNodeName1 + '.cubeInitalTransform')
@@ -266,6 +270,7 @@ def test_MultipleNodes(self):
 
             #Create a MhFlowViewportAPILocator node which adds a dataProducerSceneIndex and a Filtering scene index
             flowViewportNodeName2 = cmds.createNode("MhFlowViewportAPILocator", n="nodeShape2")
+            cmds.select(clear=True)#Clear selection
 
             #Get the matrix and set a rotation of 70 degress around Y axis.
             matrix = cmds.getAttr(flowViewportNodeName2 + '.cubeInitalTransform')
@@ -344,6 +349,7 @@ def test_MultipleViewports(self):
 
             #Create a MhFlowViewportAPILocator node which adds a dataProducerSceneIndex and a Filtering scene index
             flowViewportNodeName1 = cmds.createNode("MhFlowViewportAPILocator", n="nodeShape1")
+            cmds.select(clear=True)#Clear selection
 
             #Modify the cube grid parameters
             cmds.setAttr(flowViewportNodeName1 + '.numCubesX', 3)

test/lib/mayaUsd/render/mayaToHydra/testFootPrintNode.py

@@ -49,7 +49,8 @@ def test_AddingPrimitives(self):
 
             #Create a mayaHydraFootPrintNode node which adds a dataProducerSceneIndex
             footPrintNodeName = cmds.createNode("MhFootPrint")
-
+            cmds.select(clear=True)#Clear selection
+
             #Increase its size
             cmds.setAttr(footPrintNodeName + '.size', 5)
             cmds.refresh()
@@ -104,6 +105,7 @@ def test_FootPrintAttributes(self):
 
             #Create a mayaHydraFootPrintNode node which adds a dataProducerSceneIndex and a Filtering scene index
             footPrintNodeName = cmds.createNode("MhFootPrint")
+            cmds.select(clear=True)#Clear selection
             cmds.refresh()
             self.assertSnapshotClose("footPrint_BeforeModifs.png", self.IMAGE_DIFF_FAIL_THRESHOLD, self.IMAGE_DIFF_FAIL_PERCENT)
 
@@ -126,7 +128,7 @@ def test_MultipleNodes(self):
 
             #Create a mayaHydraFootPrintNode node which adds a dataProducerSceneIndex and a Filtering scene index
             footPrintNodeName1 = cmds.createNode("MhFootPrint", n="nodeShape1")
-
+            
             #Modify the attributes
             cmds.setAttr(footPrintNodeName1 + '.size', 3)
             cmds.setAttr(footPrintNodeName1 + '.color', 1.0, 1.0, 1.0, type="double3")
@@ -152,7 +154,8 @@ def test_MultipleNodes(self):
             self.assertIsNotNone(transformNode2)
             cmds.move(2, 0, -2, transformNode2)
             cmds.refresh()
-
+            cmds.select(clear=True)#Clear selection
+
             self.assertSnapshotClose("multipleNodes_BeforeModifs.png", self.IMAGE_DIFF_FAIL_THRESHOLD, self.IMAGE_DIFF_FAIL_PERCENT)
 
             #Modify the color of node #2, it shouldn't change node's #1 color
@@ -163,6 +166,7 @@ def test_MultipleNodes(self):
             cmds.rotate(0, 45, 0)
             cmds.scale(4, 1, 1)
             cmds.refresh()
+            cmds.select(clear=True)#Clear selection
             self.assertSnapshotClose("multipleNodes_AfterModifs.png", self.IMAGE_DIFF_FAIL_THRESHOLD, self.IMAGE_DIFF_FAIL_PERCENT)
 
             #Hide node #1
@@ -202,6 +206,7 @@ def test_MultipleViewports(self):
 
             #Create a mayaHydraFootPrintNode node which adds a dataProducerSceneIndex and a Filtering scene index
             footPrintNodeName1 = cmds.createNode("MhFootPrint", n="nodeShape1")
+            cmds.select(clear=True)#Clear selection
 
             cmds.setAttr(footPrintNodeName1 + '.size', 6)
             cmds.setAttr(footPrintNodeName1 + '.color', 0.0, 1.0, 0.0, type="double3")

test/lib/mayaUsd/render/mayaToHydra/testRefinement.py

@@ -35,6 +35,8 @@ def test_usdPrim(self):
         usdScenePath = testUtils.getTestScene('testStagePayloadsReferences', 'cube.usda')
         usdUtils.createStageFromFile(usdScenePath)
 
+        cmds.select(clear=True)#Clear selection
+
         self.setBasicCam(1)
         self.setHdStormRenderer()
         cmds.mayaHydra(createRenderGlobals=1)