Reduced dirty notifications for isolate select. (#239)

* Reduced dirty notifications for isolate select.

* Fixed 2025 build.

lib/flowViewport/API/perViewportSceneIndicesData/fvpViewportInformationAndSceneIndicesPerViewportDataManager.cpp

@@ -174,6 +174,7 @@ void ViewportInformationAndSceneIndicesPerViewportDataManager::DisableIsolateSel
 )
 {
     _isolateSelection[viewportId] = nullptr;
+    _isolateSelectSceneIndex->SetViewport(viewportId, nullptr);
 }
 
 SelectionPtr

lib/flowViewport/sceneIndex/fvpIsolateSelectSceneIndex.cpp

@@ -108,6 +108,18 @@ bool isGeomSubset(const HdSceneIndexPrim& prim) {
 #endif
 }
 
+HdContainerDataSourceHandle setInstancerMaskDataSource(
+    const HdSceneIndexPrim& inputPrim,
+    const VtArray<bool>&    instanceMask
+)
+{
+    auto maskDs = HdRetainedTypedSampledDataSource<VtArray<bool>>::New(instanceMask);
+
+    return HdContainerDataSourceEditor(inputPrim.dataSource)
+        .Set(instancerMaskLocator, maskDs)
+        .Finish();
+}
+
 }
 
 namespace FVP_NS_DEF {
@@ -159,11 +171,8 @@ HdSceneIndexPrim IsolateSelectSceneIndex::GetPrim(const SdfPath& primPath) const
 
     auto instancerMask = _instancerMasks.find(primPath);
     if (instancerMask != _instancerMasks.end()) {
-        auto maskDs = HdRetainedTypedSampledDataSource<VtArray<bool>>::New(instancerMask->second);
-
-        inputPrim.dataSource = HdContainerDataSourceEditor(inputPrim.dataSource)
-            .Set(instancerMaskLocator, maskDs)
-            .Finish();
+        inputPrim.dataSource = 
+            setInstancerMaskDataSource(inputPrim, instancerMask->second);
 
         return inputPrim;
     }
@@ -179,6 +188,20 @@ HdSceneIndexPrim IsolateSelectSceneIndex::GetPrim(const SdfPath& primPath) const
     // HYDRA-1242: setting visibility on GeomSubset prim causes hang in Hydra
     // Storm.
     if (!included && !isGeomSubset(inputPrim)) {
+        // If an instancer is not included, none of its instances are, so set
+        // an instance mask that is entirely false.
+        if (inputPrim.primType == HdPrimTypeTokens->instancer) {
+            auto instancerTopologySchema = HdInstancerTopologySchema::GetFromParent(inputPrim.dataSource);
+            const auto nbInstances = getNbInstances(instancerTopologySchema);
+            VtArray<bool> instanceMask(nbInstances, false);
+
+            inputPrim.dataSource = setInstancerMaskDataSource(inputPrim, instanceMask);
+        }
+
+        // Unclear whether instancers need their visibility off.  As of OpenUSD
+        // 0.24.11, Hydra Storm seems to ignore visibility for instancers, but
+        // there seems to be no harm in setting it false.
+
         inputPrim.dataSource = HdContainerDataSourceEditor(inputPrim.dataSource)
             .Set(HdVisibilitySchema::GetDefaultLocator(), visOff)
             .Finish();
@@ -437,21 +460,45 @@ void IsolateSelectSceneIndex::_DirtyVisibilityRecursive(
     HdSceneIndexObserver::DirtiedPrimEntries* dirtiedEntries
 ) const
 {
-    TF_DEBUG(FVP_ISOLATE_SELECT_SCENE_INDEX)
-        .Msg("            %s: marking %s visibility locator dirty.\n", _viewportId.c_str(), primPath.GetText());
-
+    // If the prim is a material, early out: visibility is not
+    // relevant for materials
     auto prim = GetInputSceneIndex()->GetPrim(primPath);
+    if (prim.primType == HdPrimTypeTokens->material) {
+      return;
+    }
 
     // GeomSubset visibility must not be set (see GetPrim()), so no need to
     // dirty it.
     if (isGeomSubset(prim)) {
       return;
     }
 
+    TF_DEBUG(FVP_ISOLATE_SELECT_SCENE_INDEX)
+        .Msg("            %s: marking %s visibility locator dirty.\n", _viewportId.c_str(), primPath.GetText());
+
+    // Unclear whether instancers need their visibility off.  As of OpenUSD
+    // 0.24.11, Hydra Storm seems to ignore visibility for instancers, but
+    // there seems to be no harm in setting it false.
+
     dirtiedEntries->emplace_back(
         primPath, HdVisibilitySchema::GetDefaultLocator());
 
+    // If the prim is an instancer, set its mask dirty, but don't recurse, as
+    // the hierarchy beneath an instancer is its prototypes, whose visibility
+    // is managed by the instancer mask data source.
+    if (prim.primType == HdPrimTypeTokens->instancer) {
+        _AddDirtyInstancerMaskEntry(primPath, dirtiedEntries);
+        return;
+    }
+
     for (const auto& childPath : GetChildPrimPaths(primPath)) {
+#if PXR_VERSION >= 2403
+        // Recursing down to set visibility on a geomSubset child is wasteful.
+        auto childPrim = GetInputSceneIndex()->GetPrim(childPath);
+        if (childPrim.primType == HdPrimTypeTokens->geomSubset) {
+            continue;
+        }
+#endif
         _DirtyVisibilityRecursive(childPath, dirtiedEntries);
     }
 }

lib/mayaHydra/mayaPlugin/renderOverride.cpp

@@ -1831,15 +1831,50 @@ void MtohRenderOverride::_ViewSelectedChangedCb(
         return;
     }
 
+    auto found = instance->_isolateSelectState.find(viewName.asChar());
+    if (found == instance->_isolateSelectState.end()) {
+        found = instance->_isolateSelectState.insert(found, VpIsolateSelectStates::value_type(viewName.asChar(), IsolateSelectState::IsolateSelectOff));
+    }
+
     // The M3dView returns the list of view selected objects as strings.
     // If isolate select is turned off, we want to disable isolate selection.
     // Otherwise, replace with what is in the M3dView.
     auto& vpDataMgr = Fvp::ViewportDataMgr::Get();
     if (!view.viewSelected()) {
         vpDataMgr.DisableIsolateSelection(viewName.asChar());
+        found->second = IsolateSelectState::IsolateSelectOff;
         return;
     }
 
+    // Deal with IsolateSelectState changes.  The messages we receive are
+    // viewSelectedObjectsChanged true or false.
+    //
+    // State transitions:
+    // 
+    // off: message false --> go to pendingObjects, do not notify.
+    // off: message true --> illegal, warn, do not notify.
+    // 
+    // pendingObjects: message false --> illegal, warn, do not notify.
+    // pendingObjects: message true --> notify, go to on.
+    // 
+    // on: message false --> go to off, notify.
+    // on: message true --> stay on, notify.
+
+    if (found->second == IsolateSelectState::IsolateSelectOff) {
+        if (TF_VERIFY(!viewSelectedObjectsChanged)) {
+            found->second = IsolateSelectState::IsolateSelectPendingObjects;
+        }
+        return;
+    }
+    else if (found->second == IsolateSelectState::IsolateSelectPendingObjects) {
+        if (!TF_VERIFY(viewSelectedObjectsChanged)) {
+            return;
+        }
+        found->second = IsolateSelectState::IsolateSelectOn;
+    }
+
+    TF_VERIFY(found->second == IsolateSelectState::IsolateSelectOn);
+
     // The M3dView returns the list of view selected objects as strings.
     // Loop over the view selected objects and try to create UFE paths from
     // them.  If there is more than one element to the MStringArray, this

lib/mayaHydra/mayaPlugin/renderOverride.h

@@ -74,6 +74,7 @@
 #include <memory>
 #include <mutex>
 #include <vector>
+#include <map>
 
 #include <ufe/ufe.h>
 UFE_NS_DEF {
@@ -338,6 +339,21 @@ class MtohRenderOverride : public MHWRender::MRenderOverride,
 #ifdef MAYA_HAS_VIEW_SELECTED_OBJECT_API
     long int   _nbViewSelectedChangedCalls{0};
 #endif
+
+    // Maya has an awkward notification mechanism for isolate select,
+    // with a view selected objects changed boolean that indicates
+    // whether the state has changed (false), or the isolate selected
+    // objects have changed (true).  When changing a viewport from
+    // isolate select off to on, two notifications are therefore sent,
+    // first false (state change), then true (objects set).  To avoid
+    // double dirtying in Hydra, we track the following isolate select
+    // states per viewport:
+    // 
+    enum class IsolateSelectState {IsolateSelectOff, IsolateSelectPendingObjects,
+				   IsolateSelectOn};
+
+    using VpIsolateSelectStates = std::map<std::string, IsolateSelectState>;
+    VpIsolateSelectStates _isolateSelectState;
 };
 
 PXR_NAMESPACE_CLOSE_SCOPE