From 92c2e05510c8be4c2e41881d3304a337379b30d6 Mon Sep 17 00:00:00 2001 From: Jerry Gamache Date: Fri, 24 Mar 2023 18:16:18 -0400 Subject: [PATCH 1/3] Add smart signaling for renderers. A renderer now only has to track the "updateid" and "resyncid" attributes of the stage to know if something significant happened. All UINodeGraphNodeAPI notifications were suppressed. --- lib/mayaUsd/nodes/proxyShapeBase.cpp | 160 +++++++++++++++++++++++++++ lib/mayaUsd/nodes/proxyShapeBase.h | 10 ++ test/lib/ufe/testContextOps.py | 57 ++++++++++ test/lib/ufe/testUINodeGraphNode.py | 7 ++ 4 files changed, 234 insertions(+) diff --git a/lib/mayaUsd/nodes/proxyShapeBase.cpp b/lib/mayaUsd/nodes/proxyShapeBase.cpp index d098e82d7c..80546d8fb7 100644 --- a/lib/mayaUsd/nodes/proxyShapeBase.cpp +++ b/lib/mayaUsd/nodes/proxyShapeBase.cpp @@ -50,17 +50,20 @@ #include #include #include +#include #include #include #include #include #include #include +#include #include #include #include #include #include +#include #include #include @@ -154,6 +157,9 @@ MObject MayaUsdProxyShapeBase::drawGuidePurposeAttr; MObject MayaUsdProxyShapeBase::sessionLayerNameAttr; MObject MayaUsdProxyShapeBase::rootLayerNameAttr; MObject MayaUsdProxyShapeBase::mutedLayersAttr; +// Change counter attributes +MObject MayaUsdProxyShapeBase::updateCounterAttr; +MObject MayaUsdProxyShapeBase::resyncCounterAttr; // Output attributes MObject MayaUsdProxyShapeBase::outTimeAttr; MObject MayaUsdProxyShapeBase::outStageDataAttr; @@ -388,6 +394,26 @@ MStatus MayaUsdProxyShapeBase::initialize() retValue = addAttribute(outStageDataAttr); CHECK_MSTATUS_AND_RETURN_IT(retValue); + updateCounterAttr + = numericAttrFn.create("updateid", "upid", MFnNumericData::kInt64, -1, &retValue); + CHECK_MSTATUS_AND_RETURN_IT(retValue); + numericAttrFn.setStorable(false); + numericAttrFn.setWritable(false); + numericAttrFn.setHidden(true); + numericAttrFn.setInternal(true); + retValue = addAttribute(updateCounterAttr); + CHECK_MSTATUS_AND_RETURN_IT(retValue); + + resyncCounterAttr + = numericAttrFn.create("resyncid", "rsid", MFnNumericData::kInt64, -1, &retValue); + CHECK_MSTATUS_AND_RETURN_IT(retValue); + numericAttrFn.setStorable(false); + numericAttrFn.setWritable(false); + numericAttrFn.setHidden(true); + numericAttrFn.setInternal(true); + retValue = addAttribute(resyncCounterAttr); + CHECK_MSTATUS_AND_RETURN_IT(retValue); + outStageCacheIdAttr = numericAttrFn.create("outStageCacheId", "ostcid", MFnNumericData::kInt, -1, &retValue); CHECK_MSTATUS_AND_RETURN_IT(retValue); @@ -524,6 +550,22 @@ void MayaUsdProxyShapeBase::postConstructor() MayaUsdProxyStageInvalidateNotice(*this).Send(); } +/* virtual */ +bool MayaUsdProxyShapeBase::getInternalValue(const MPlug& plug, MDataHandle& handle) +{ + bool retVal = true; + + if (plug == updateCounterAttr) { + handle.set(_UsdStageUpdateCounter); + } else if (plug == resyncCounterAttr) { + handle.set(_UsdStageResyncCounter); + } else { + retVal = MPxSurfaceShape::getInternalValue(plug, handle); + } + + return retVal; +} + /* virtual */ MStatus MayaUsdProxyShapeBase::compute(const MPlug& plug, MDataBlock& dataBlock) { @@ -1923,6 +1965,118 @@ void MayaUsdProxyShapeBase::_OnStageEditTargetChanged( _targetLayer = layer; } +namespace { + +// We have incoming changes that USD will consider either requiring an +// update (meaning the render delegate needs to refresh and redraw) or +// a resync (meaning the scene delegate needs to fetch new datum). We +// want external clients to be aware of these classes of updates in case +// they do not use the Hydra system for refreshing and drawing the scene. +enum class _UsdChangeType +{ + kIgnored, // Change does not require redraw: UI change, metadata change. + kUpdate, // Change requires redraw after refreshing parameter values + kResync // Change requires refreshing cached buffers +}; + +// If the notification is about prepending a UI schema, we don't want a refresh. These structures +// are quite large to inspect, but they hash easily, so let's compare known hashes. +bool _IsUiSchemaPrepend(const VtValue& v) +{ + static std::set UiSchemaPrependHashes; + std::once_flag hasHashes; + std::call_once(hasHashes, [&]() { + SdfTokenListOp op; + op.SetPrependedItems(TfTokenVector { TfToken("NodeGraphNodeAPI") }); + UiSchemaPrependHashes.insert(hash_value(op)); + }); + + if (v.IsHolding()) { + const size_t hash = hash_value(v.UncheckedGet()); + if (UiSchemaPrependHashes.count(hash)) { + return true; + } + } + return false; +} + +// This is a stripped down copy of UsdImagingDelegate::_OnUsdObjectsChanged which is the main USD +// notification handler where paths to refresh and paths to update are compiled for the next Hydra +// refresh. We do not gather paths as there is no simple way to know when tho flush these maps. +// +// This needs to stay as quick as possible since it is stuck in the middle of the notification code +// path. +// +// This is a work in progress. Some improvements might be necessary in the future. The following +// potential issues are already visible: +// +// - Changing a parameter value for the first time creates the attribute, which is a kResync +_UsdChangeType _ClassifyUsdObjectsChanged(UsdNotice::ObjectsChanged const& notice) +{ + using PathRange = UsdNotice::ObjectsChanged::PathRange; + + auto range = notice.GetResyncedPaths(); + if (!range.empty()) { + size_t ignoredCount = 0; + size_t resyncCount = 0; + for (auto it = range.begin(); it != range.end(); ++it) { + if (it->IsPropertyPath()) { + // We have a bunch of UI properties to ignore. Especially anything that comes from + // UI schemas. + if (it->GetName().rfind("ui:", 0) == 0) { + ++ignoredCount; + continue; + } + } + for (const SdfChangeList::Entry* entry : it.base()->second) { + for (auto&& infoChanged : entry->infoChanged) { + if (infoChanged.first == UsdTokens->apiSchemas + && _IsUiSchemaPrepend(infoChanged.second.second)) { + ++ignoredCount; + } else { + ++resyncCount; + } + } + } + } + + if (ignoredCount) { + return resyncCount ? _UsdChangeType::kResync : _UsdChangeType::kIgnored; + } else { + return _UsdChangeType::kResync; + } + } + + auto retVal = _UsdChangeType::kIgnored; + + const PathRange pathsToUpdate = notice.GetChangedInfoOnlyPaths(); + for (PathRange::const_iterator it = pathsToUpdate.begin(); it != pathsToUpdate.end(); ++it) { + if (it->IsAbsoluteRootOrPrimPath()) { + const TfTokenVector changedFields = it.GetChangedFields(); + if (!changedFields.empty()) { + retVal = _UsdChangeType::kUpdate; + } + } else if (it->IsPropertyPath()) { + // We have a bunch of UI properties to ignore. Especially anything that comes from UI + // schemas. + if (it->GetName().rfind("ui:", 0) == 0) { + continue; + } + retVal = _UsdChangeType::kUpdate; + for (const auto& entry : it.base()->second) { + if (entry->flags.didChangeAttributeConnection) { + retVal = _UsdChangeType::kResync; + break; + } + } + } + } + + return retVal; +} + +} // namespace + void MayaUsdProxyShapeBase::_OnStageObjectsChanged(const UsdNotice::ObjectsChanged& notice) { MProfilingScope profilingScope( @@ -1936,6 +2090,12 @@ void MayaUsdProxyShapeBase::_OnStageObjectsChanged(const UsdNotice::ObjectsChang ProxyAccessor::stageChanged(_usdAccessor, thisMObject(), notice); MayaUsdProxyStageObjectsChangedNotice(*this, notice).Send(); + switch (_ClassifyUsdObjectsChanged(notice)) { + case _UsdChangeType::kIgnored: return; + case _UsdChangeType::kUpdate: ++_UsdStageUpdateCounter; break; + case _UsdChangeType::kResync: ++_UsdStageResyncCounter; break; + } + // Recompute the extents of any UsdGeomBoundable that has authored extents const auto& stage = notice.GetStage(); if (stage != getUsdStage()) { diff --git a/lib/mayaUsd/nodes/proxyShapeBase.h b/lib/mayaUsd/nodes/proxyShapeBase.h index 57b9d56125..71b3f7deae 100644 --- a/lib/mayaUsd/nodes/proxyShapeBase.h +++ b/lib/mayaUsd/nodes/proxyShapeBase.h @@ -124,6 +124,12 @@ class MayaUsdProxyShapeBase MAYAUSD_CORE_PUBLIC static MObject mutedLayersAttr; + // Change counter attributes + MAYAUSD_CORE_PUBLIC + static MObject updateCounterAttr; + MAYAUSD_CORE_PUBLIC + static MObject resyncCounterAttr; + // Output attributes MAYAUSD_CORE_PUBLIC static MObject outTimeAttr; @@ -170,6 +176,8 @@ class MayaUsdProxyShapeBase MAYAUSD_CORE_PUBLIC void postConstructor() override; MAYAUSD_CORE_PUBLIC + bool getInternalValue(const MPlug&, MDataHandle&) override; + MAYAUSD_CORE_PUBLIC MStatus compute(const MPlug& plug, MDataBlock& dataBlock) override; MAYAUSD_CORE_PUBLIC bool isBounded() const override; @@ -397,6 +405,8 @@ class MayaUsdProxyShapeBase std::map _boundingBoxCache; size_t _excludePrimPathsVersion { 1 }; size_t _UsdStageVersion { 1 }; + MInt64 _UsdStageUpdateCounter { 1 }; + MInt64 _UsdStageResyncCounter { 1 }; MayaUsd::ProxyAccessor::Owner _usdAccessor; diff --git a/test/lib/ufe/testContextOps.py b/test/lib/ufe/testContextOps.py index dba12eea4c..1baadd4df2 100644 --- a/test/lib/ufe/testContextOps.py +++ b/test/lib/ufe/testContextOps.py @@ -1483,14 +1483,71 @@ def testGeomCoponentAssignment(self): self.assertEqual(topSubset.GetFamilyNameAttr().Get(), "componentTag") self.assertFalse(topSubset.GetPrim().HasAPI(UsdShade.MaterialBindingAPI)) + currentResyncCount = cmds.getAttr(psPathStr + '.resyncid') + contextOps = ufe.ContextOps.contextOps(topItem) cmd = contextOps.doOpCmd(['Assign New Material', 'USD', 'UsdPreviewSurface']) self.assertIsNotNone(cmd) ufeCmd.execute(cmd) + snIter = iter(ufe.GlobalSelection.get()) + shaderItem = next(snIter) + self.assertEqual(topSubset.GetFamilyNameAttr().Get(), "materialBind") self.assertTrue(topSubset.GetPrim().HasAPI(UsdShade.MaterialBindingAPI)) + # We expect a resync after this assignment: + self.assertGreater(cmds.getAttr(psPathStr + '.resyncid'), currentResyncCount) + + # setting a value the first time is a resync due to the creation of the attribute: + attrs = ufe.Attributes.attributes(shaderItem) + metallicAttr = attrs.attribute("inputs:metallic") + currentResyncCount = cmds.getAttr(psPathStr + '.resyncid') + ufeCmd.execute(metallicAttr.setCmd(0.5)) + self.assertGreater(cmds.getAttr(psPathStr + '.resyncid'), currentResyncCount) + + # Subsequent changes are updates: + currentResyncCount = cmds.getAttr(psPathStr + '.resyncid') + currentUpdateCount = cmds.getAttr(psPathStr + '.updateid') + ufeCmd.execute(metallicAttr.setCmd(0.7)) + self.assertEqual(cmds.getAttr(psPathStr + '.resyncid'), currentResyncCount) + self.assertGreater(cmds.getAttr(psPathStr + '.updateid'), currentUpdateCount) + + # First undo is an update: + currentResyncCount = cmds.getAttr(psPathStr + '.resyncid') + currentUpdateCount = cmds.getAttr(psPathStr + '.updateid') + cmds.undo() + self.assertEqual(cmds.getAttr(psPathStr + '.resyncid'), currentResyncCount) + self.assertGreater(cmds.getAttr(psPathStr + '.updateid'), currentUpdateCount) + + # Second undo is a resync: + currentResyncCount = cmds.getAttr(psPathStr + '.resyncid') + cmds.undo() + self.assertGreater(cmds.getAttr(psPathStr + '.resyncid'), currentResyncCount) + + # Third undo is also resync: + currentResyncCount = cmds.getAttr(psPathStr + '.resyncid') + cmds.undo() + self.assertGreater(cmds.getAttr(psPathStr + '.resyncid'), currentResyncCount) + + # First redo is resync: + currentResyncCount = cmds.getAttr(psPathStr + '.resyncid') + cmds.redo() + self.assertGreater(cmds.getAttr(psPathStr + '.resyncid'), currentResyncCount) + + # Second redo is resync: + currentResyncCount = cmds.getAttr(psPathStr + '.resyncid') + cmds.redo() + self.assertGreater(cmds.getAttr(psPathStr + '.resyncid'), currentResyncCount) + + # Third redo is update: + currentResyncCount = cmds.getAttr(psPathStr + '.resyncid') + currentUpdateCount = cmds.getAttr(psPathStr + '.updateid') + cmds.redo() + self.assertEqual(cmds.getAttr(psPathStr + '.resyncid'), currentResyncCount) + self.assertGreater(cmds.getAttr(psPathStr + '.updateid'), currentUpdateCount) + + if __name__ == '__main__': unittest.main(verbosity=2) diff --git a/test/lib/ufe/testUINodeGraphNode.py b/test/lib/ufe/testUINodeGraphNode.py index addaae3355..4beb33c780 100644 --- a/test/lib/ufe/testUINodeGraphNode.py +++ b/test/lib/ufe/testUINodeGraphNode.py @@ -60,6 +60,9 @@ def testUIInfo(self): ball3Path = ufe.PathString.path('|transform1|proxyShape1,/Ball_set/Props/Ball_3') ball3SceneItem = ufe.Hierarchy.createItem(ball3Path) + initialUpdateCount = cmds.getAttr('|transform1|proxyShape1.updateid') + initialResyncCount = cmds.getAttr('|transform1|proxyShape1.resyncid') + uiNodeGraphNode = ufe.UINodeGraphNode.uiNodeGraphNode(ball3SceneItem) self.assertFalse(uiNodeGraphNode.hasPosition()) @@ -85,5 +88,9 @@ def testUIInfo(self): self.assertEqual(pos4.x(), pos5.x()) self.assertEqual(pos4.y(), pos5.y()) + # None of these changes should force a render refresh: + self.assertEqual(initialUpdateCount, cmds.getAttr('|transform1|proxyShape1.updateid')) + self.assertEqual(initialResyncCount, cmds.getAttr('|transform1|proxyShape1.resyncid')) + if __name__ == '__main__': unittest.main(verbosity=2) From e04a28a5f308761b714ccc367853eee366a4452c Mon Sep 17 00:00:00 2001 From: Jerry Gamache Date: Mon, 27 Mar 2023 10:45:02 -0400 Subject: [PATCH 2/3] The update counter will increment on resync. So client that do not care about resync can tune in only on update and get the expected results. --- lib/mayaUsd/nodes/proxyShapeBase.cpp | 3 +- test/lib/ufe/testContextOps.py | 52 +++++++++++++++------------- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/lib/mayaUsd/nodes/proxyShapeBase.cpp b/lib/mayaUsd/nodes/proxyShapeBase.cpp index 80546d8fb7..e500d979ab 100644 --- a/lib/mayaUsd/nodes/proxyShapeBase.cpp +++ b/lib/mayaUsd/nodes/proxyShapeBase.cpp @@ -2092,8 +2092,9 @@ void MayaUsdProxyShapeBase::_OnStageObjectsChanged(const UsdNotice::ObjectsChang switch (_ClassifyUsdObjectsChanged(notice)) { case _UsdChangeType::kIgnored: return; + case _UsdChangeType::kResync: ++_UsdStageResyncCounter; + // [[fallthrough]]; // We want that fallthrough to have the update always triggered. case _UsdChangeType::kUpdate: ++_UsdStageUpdateCounter; break; - case _UsdChangeType::kResync: ++_UsdStageResyncCounter; break; } // Recompute the extents of any UsdGeomBoundable that has authored extents diff --git a/test/lib/ufe/testContextOps.py b/test/lib/ufe/testContextOps.py index 1baadd4df2..bb330f1162 100644 --- a/test/lib/ufe/testContextOps.py +++ b/test/lib/ufe/testContextOps.py @@ -1483,7 +1483,24 @@ def testGeomCoponentAssignment(self): self.assertEqual(topSubset.GetFamilyNameAttr().Get(), "componentTag") self.assertFalse(topSubset.GetPrim().HasAPI(UsdShade.MaterialBindingAPI)) - currentResyncCount = cmds.getAttr(psPathStr + '.resyncid') + counters= { "resync": cmds.getAttr(psPathStr + '.resyncid'), + "update" : cmds.getAttr(psPathStr + '.upid')} + + def assertIsOnlyUpdate(self, counters, shapePathStr): + resyncCounter = cmds.getAttr(shapePathStr + '.resyncid') + updateCounter = cmds.getAttr(shapePathStr + '.updateid') + self.assertEqual(resyncCounter, counters["resync"]) + self.assertGreater(updateCounter, counters["update"]) + counters["resync"] = resyncCounter + counters["update"] = updateCounter + + def assertIsResync(self, counters, shapePathStr): + resyncCounter = cmds.getAttr(shapePathStr + '.resyncid') + updateCounter = cmds.getAttr(shapePathStr + '.updateid') + self.assertGreater(resyncCounter, counters["resync"]) + self.assertGreater(updateCounter, counters["update"]) + counters["resync"] = resyncCounter + counters["update"] = updateCounter contextOps = ufe.ContextOps.contextOps(topItem) cmd = contextOps.doOpCmd(['Assign New Material', 'USD', 'UsdPreviewSurface']) @@ -1497,56 +1514,41 @@ def testGeomCoponentAssignment(self): self.assertTrue(topSubset.GetPrim().HasAPI(UsdShade.MaterialBindingAPI)) # We expect a resync after this assignment: - self.assertGreater(cmds.getAttr(psPathStr + '.resyncid'), currentResyncCount) + assertIsResync(self, counters, psPathStr) # setting a value the first time is a resync due to the creation of the attribute: attrs = ufe.Attributes.attributes(shaderItem) metallicAttr = attrs.attribute("inputs:metallic") - currentResyncCount = cmds.getAttr(psPathStr + '.resyncid') ufeCmd.execute(metallicAttr.setCmd(0.5)) - self.assertGreater(cmds.getAttr(psPathStr + '.resyncid'), currentResyncCount) + assertIsResync(self, counters, psPathStr) # Subsequent changes are updates: - currentResyncCount = cmds.getAttr(psPathStr + '.resyncid') - currentUpdateCount = cmds.getAttr(psPathStr + '.updateid') ufeCmd.execute(metallicAttr.setCmd(0.7)) - self.assertEqual(cmds.getAttr(psPathStr + '.resyncid'), currentResyncCount) - self.assertGreater(cmds.getAttr(psPathStr + '.updateid'), currentUpdateCount) + assertIsOnlyUpdate(self, counters, psPathStr) # First undo is an update: - currentResyncCount = cmds.getAttr(psPathStr + '.resyncid') - currentUpdateCount = cmds.getAttr(psPathStr + '.updateid') cmds.undo() - self.assertEqual(cmds.getAttr(psPathStr + '.resyncid'), currentResyncCount) - self.assertGreater(cmds.getAttr(psPathStr + '.updateid'), currentUpdateCount) + assertIsOnlyUpdate(self, counters, psPathStr) # Second undo is a resync: - currentResyncCount = cmds.getAttr(psPathStr + '.resyncid') cmds.undo() - self.assertGreater(cmds.getAttr(psPathStr + '.resyncid'), currentResyncCount) + assertIsResync(self, counters, psPathStr) # Third undo is also resync: - currentResyncCount = cmds.getAttr(psPathStr + '.resyncid') cmds.undo() - self.assertGreater(cmds.getAttr(psPathStr + '.resyncid'), currentResyncCount) + assertIsResync(self, counters, psPathStr) # First redo is resync: - currentResyncCount = cmds.getAttr(psPathStr + '.resyncid') cmds.redo() - self.assertGreater(cmds.getAttr(psPathStr + '.resyncid'), currentResyncCount) + assertIsResync(self, counters, psPathStr) # Second redo is resync: - currentResyncCount = cmds.getAttr(psPathStr + '.resyncid') cmds.redo() - self.assertGreater(cmds.getAttr(psPathStr + '.resyncid'), currentResyncCount) + assertIsResync(self, counters, psPathStr) # Third redo is update: - currentResyncCount = cmds.getAttr(psPathStr + '.resyncid') - currentUpdateCount = cmds.getAttr(psPathStr + '.updateid') cmds.redo() - self.assertEqual(cmds.getAttr(psPathStr + '.resyncid'), currentResyncCount) - self.assertGreater(cmds.getAttr(psPathStr + '.updateid'), currentUpdateCount) - + assertIsOnlyUpdate(self, counters, psPathStr) if __name__ == '__main__': From 98fe8431bf55243ce6a4ea5b681523a19489a947 Mon Sep 17 00:00:00 2001 From: Jerry Gamache Date: Mon, 27 Mar 2023 13:29:42 -0400 Subject: [PATCH 3/3] Responding to review comments --- lib/mayaUsd/nodes/CMakeLists.txt | 2 + lib/mayaUsd/nodes/proxyShapeBase.cpp | 131 +-------------- lib/mayaUsd/nodes/proxyShapeBase.h | 6 +- lib/mayaUsd/nodes/proxyShapeUpdateManager.cpp | 156 ++++++++++++++++++ lib/mayaUsd/nodes/proxyShapeUpdateManager.h | 47 ++++++ test/lib/ufe/testContextOps.py | 10 +- test/lib/ufe/testUINodeGraphNode.py | 8 +- 7 files changed, 225 insertions(+), 135 deletions(-) create mode 100644 lib/mayaUsd/nodes/proxyShapeUpdateManager.cpp create mode 100644 lib/mayaUsd/nodes/proxyShapeUpdateManager.h diff --git a/lib/mayaUsd/nodes/CMakeLists.txt b/lib/mayaUsd/nodes/CMakeLists.txt index 441e36cd7e..cdfe07ce6e 100644 --- a/lib/mayaUsd/nodes/CMakeLists.txt +++ b/lib/mayaUsd/nodes/CMakeLists.txt @@ -9,6 +9,7 @@ target_sources(${PROJECT_NAME} proxyShapeBase.cpp proxyShapePlugin.cpp proxyShapeStageExtraData.cpp + proxyShapeUpdateManager.cpp stageData.cpp stageNode.cpp usdPrimProvider.cpp @@ -22,6 +23,7 @@ set(HEADERS proxyShapePlugin.h proxyStageProvider.h proxyShapeStageExtraData.h + proxyShapeUpdateManager.h stageData.h stageNode.h usdPrimProvider.h diff --git a/lib/mayaUsd/nodes/proxyShapeBase.cpp b/lib/mayaUsd/nodes/proxyShapeBase.cpp index e500d979ab..0b8d07835e 100644 --- a/lib/mayaUsd/nodes/proxyShapeBase.cpp +++ b/lib/mayaUsd/nodes/proxyShapeBase.cpp @@ -50,20 +50,17 @@ #include #include #include -#include #include #include #include #include #include #include -#include #include #include #include #include #include -#include #include #include @@ -395,7 +392,7 @@ MStatus MayaUsdProxyShapeBase::initialize() CHECK_MSTATUS_AND_RETURN_IT(retValue); updateCounterAttr - = numericAttrFn.create("updateid", "upid", MFnNumericData::kInt64, -1, &retValue); + = numericAttrFn.create("updateId", "upid", MFnNumericData::kInt64, -1, &retValue); CHECK_MSTATUS_AND_RETURN_IT(retValue); numericAttrFn.setStorable(false); numericAttrFn.setWritable(false); @@ -405,7 +402,7 @@ MStatus MayaUsdProxyShapeBase::initialize() CHECK_MSTATUS_AND_RETURN_IT(retValue); resyncCounterAttr - = numericAttrFn.create("resyncid", "rsid", MFnNumericData::kInt64, -1, &retValue); + = numericAttrFn.create("resyncId", "rsid", MFnNumericData::kInt64, -1, &retValue); CHECK_MSTATUS_AND_RETURN_IT(retValue); numericAttrFn.setStorable(false); numericAttrFn.setWritable(false); @@ -556,9 +553,9 @@ bool MayaUsdProxyShapeBase::getInternalValue(const MPlug& plug, MDataHandle& han bool retVal = true; if (plug == updateCounterAttr) { - handle.set(_UsdStageUpdateCounter); + handle.set(_shapeUpdateManager.GetUpdateCount()); } else if (plug == resyncCounterAttr) { - handle.set(_UsdStageResyncCounter); + handle.set(_shapeUpdateManager.GetResyncCount()); } else { retVal = MPxSurfaceShape::getInternalValue(plug, handle); } @@ -1965,118 +1962,6 @@ void MayaUsdProxyShapeBase::_OnStageEditTargetChanged( _targetLayer = layer; } -namespace { - -// We have incoming changes that USD will consider either requiring an -// update (meaning the render delegate needs to refresh and redraw) or -// a resync (meaning the scene delegate needs to fetch new datum). We -// want external clients to be aware of these classes of updates in case -// they do not use the Hydra system for refreshing and drawing the scene. -enum class _UsdChangeType -{ - kIgnored, // Change does not require redraw: UI change, metadata change. - kUpdate, // Change requires redraw after refreshing parameter values - kResync // Change requires refreshing cached buffers -}; - -// If the notification is about prepending a UI schema, we don't want a refresh. These structures -// are quite large to inspect, but they hash easily, so let's compare known hashes. -bool _IsUiSchemaPrepend(const VtValue& v) -{ - static std::set UiSchemaPrependHashes; - std::once_flag hasHashes; - std::call_once(hasHashes, [&]() { - SdfTokenListOp op; - op.SetPrependedItems(TfTokenVector { TfToken("NodeGraphNodeAPI") }); - UiSchemaPrependHashes.insert(hash_value(op)); - }); - - if (v.IsHolding()) { - const size_t hash = hash_value(v.UncheckedGet()); - if (UiSchemaPrependHashes.count(hash)) { - return true; - } - } - return false; -} - -// This is a stripped down copy of UsdImagingDelegate::_OnUsdObjectsChanged which is the main USD -// notification handler where paths to refresh and paths to update are compiled for the next Hydra -// refresh. We do not gather paths as there is no simple way to know when tho flush these maps. -// -// This needs to stay as quick as possible since it is stuck in the middle of the notification code -// path. -// -// This is a work in progress. Some improvements might be necessary in the future. The following -// potential issues are already visible: -// -// - Changing a parameter value for the first time creates the attribute, which is a kResync -_UsdChangeType _ClassifyUsdObjectsChanged(UsdNotice::ObjectsChanged const& notice) -{ - using PathRange = UsdNotice::ObjectsChanged::PathRange; - - auto range = notice.GetResyncedPaths(); - if (!range.empty()) { - size_t ignoredCount = 0; - size_t resyncCount = 0; - for (auto it = range.begin(); it != range.end(); ++it) { - if (it->IsPropertyPath()) { - // We have a bunch of UI properties to ignore. Especially anything that comes from - // UI schemas. - if (it->GetName().rfind("ui:", 0) == 0) { - ++ignoredCount; - continue; - } - } - for (const SdfChangeList::Entry* entry : it.base()->second) { - for (auto&& infoChanged : entry->infoChanged) { - if (infoChanged.first == UsdTokens->apiSchemas - && _IsUiSchemaPrepend(infoChanged.second.second)) { - ++ignoredCount; - } else { - ++resyncCount; - } - } - } - } - - if (ignoredCount) { - return resyncCount ? _UsdChangeType::kResync : _UsdChangeType::kIgnored; - } else { - return _UsdChangeType::kResync; - } - } - - auto retVal = _UsdChangeType::kIgnored; - - const PathRange pathsToUpdate = notice.GetChangedInfoOnlyPaths(); - for (PathRange::const_iterator it = pathsToUpdate.begin(); it != pathsToUpdate.end(); ++it) { - if (it->IsAbsoluteRootOrPrimPath()) { - const TfTokenVector changedFields = it.GetChangedFields(); - if (!changedFields.empty()) { - retVal = _UsdChangeType::kUpdate; - } - } else if (it->IsPropertyPath()) { - // We have a bunch of UI properties to ignore. Especially anything that comes from UI - // schemas. - if (it->GetName().rfind("ui:", 0) == 0) { - continue; - } - retVal = _UsdChangeType::kUpdate; - for (const auto& entry : it.base()->second) { - if (entry->flags.didChangeAttributeConnection) { - retVal = _UsdChangeType::kResync; - break; - } - } - } - } - - return retVal; -} - -} // namespace - void MayaUsdProxyShapeBase::_OnStageObjectsChanged(const UsdNotice::ObjectsChanged& notice) { MProfilingScope profilingScope( @@ -2090,11 +1975,9 @@ void MayaUsdProxyShapeBase::_OnStageObjectsChanged(const UsdNotice::ObjectsChang ProxyAccessor::stageChanged(_usdAccessor, thisMObject(), notice); MayaUsdProxyStageObjectsChangedNotice(*this, notice).Send(); - switch (_ClassifyUsdObjectsChanged(notice)) { - case _UsdChangeType::kIgnored: return; - case _UsdChangeType::kResync: ++_UsdStageResyncCounter; - // [[fallthrough]]; // We want that fallthrough to have the update always triggered. - case _UsdChangeType::kUpdate: ++_UsdStageUpdateCounter; break; + // Also keeps track of the notification counters: + if (_shapeUpdateManager.CanIgnoreObjectsChanged(notice)) { + return; } // Recompute the extents of any UsdGeomBoundable that has authored extents diff --git a/lib/mayaUsd/nodes/proxyShapeBase.h b/lib/mayaUsd/nodes/proxyShapeBase.h index 71b3f7deae..9bb07d0790 100644 --- a/lib/mayaUsd/nodes/proxyShapeBase.h +++ b/lib/mayaUsd/nodes/proxyShapeBase.h @@ -55,6 +55,7 @@ constexpr char USD_UFE_SEPARATOR = '/'; #include #include #include +#include #include #include @@ -405,8 +406,9 @@ class MayaUsdProxyShapeBase std::map _boundingBoxCache; size_t _excludePrimPathsVersion { 1 }; size_t _UsdStageVersion { 1 }; - MInt64 _UsdStageUpdateCounter { 1 }; - MInt64 _UsdStageResyncCounter { 1 }; + + // This helper class keeps track of the notification counters: + MayaUsdProxyShapeUpdateManager _shapeUpdateManager; MayaUsd::ProxyAccessor::Owner _usdAccessor; diff --git a/lib/mayaUsd/nodes/proxyShapeUpdateManager.cpp b/lib/mayaUsd/nodes/proxyShapeUpdateManager.cpp new file mode 100644 index 0000000000..ceb490a0de --- /dev/null +++ b/lib/mayaUsd/nodes/proxyShapeUpdateManager.cpp @@ -0,0 +1,156 @@ +// +// Copyright 2023 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. +// +#include "proxyShapeUpdateManager.h" + +#include +#include +#include +#include + +using namespace MAYAUSD_NS_DEF; + +PXR_NAMESPACE_OPEN_SCOPE + +namespace { + +// We have incoming changes that USD will consider either requiring an +// update (meaning the render delegate needs to refresh and redraw) or +// a resync (meaning the scene delegate needs to fetch new datum). We +// want external clients to be aware of these classes of updates in case +// they do not use the Hydra system for refreshing and drawing the scene. +enum class _UsdChangeType +{ + kIgnored, // Change does not require redraw: UI change, metadata change. + kUpdate, // Change requires redraw after refreshing parameter values + kResync // Change requires refreshing cached buffers +}; + +// If the notification is about prepending a UI schema, we don't want a refresh. These structures +// are quite large to inspect, but they hash easily, so let's compare known hashes. +bool _IsUiSchemaPrepend(const VtValue& v) +{ + static std::set UiSchemaPrependHashes; + std::once_flag hasHashes; + std::call_once(hasHashes, [&]() { + SdfTokenListOp op; + op.SetPrependedItems(TfTokenVector { TfToken("NodeGraphNodeAPI") }); + UiSchemaPrependHashes.insert(hash_value(op)); + }); + + if (v.IsHolding()) { + const size_t hash = hash_value(v.UncheckedGet()); + if (UiSchemaPrependHashes.count(hash)) { + return true; + } + } + return false; +} + +// This is a stripped down copy of UsdImagingDelegate::_OnUsdObjectsChanged which is the main USD +// notification handler where paths to refresh and paths to update are compiled for the next Hydra +// refresh. We do not gather paths as there is no simple way to know when to flush these maps. +// +// This needs to stay as quick as possible since it is stuck in the middle of the notification code +// path. +// +// This is a work in progress. Some improvements might be necessary in the future. The following +// potential issues are already visible: +// +// - Changing a parameter value for the first time creates the attribute, which is a kResync +_UsdChangeType _ClassifyUsdObjectsChanged(UsdNotice::ObjectsChanged const& notice) +{ + using PathRange = UsdNotice::ObjectsChanged::PathRange; + + auto range = notice.GetResyncedPaths(); + if (!range.empty()) { + size_t ignoredCount = 0; + size_t resyncCount = 0; + for (auto it = range.begin(); it != range.end(); ++it) { + if (it->IsPropertyPath()) { + // We have a bunch of UI properties to ignore. Especially anything that comes from + // UI schemas. + if (it->GetName().rfind("ui:", 0) == 0) { + ++ignoredCount; + continue; + } + } + for (const SdfChangeList::Entry* entry : it.base()->second) { + for (auto&& infoChanged : entry->infoChanged) { + if (infoChanged.first == UsdTokens->apiSchemas + && _IsUiSchemaPrepend(infoChanged.second.second)) { + ++ignoredCount; + } else { + ++resyncCount; + } + } + } + } + + if (ignoredCount) { + return resyncCount ? _UsdChangeType::kResync : _UsdChangeType::kIgnored; + } else { + return _UsdChangeType::kResync; + } + } + + auto retVal = _UsdChangeType::kIgnored; + + const PathRange pathsToUpdate = notice.GetChangedInfoOnlyPaths(); + for (PathRange::const_iterator it = pathsToUpdate.begin(); it != pathsToUpdate.end(); ++it) { + if (it->IsAbsoluteRootOrPrimPath()) { + const TfTokenVector changedFields = it.GetChangedFields(); + if (!changedFields.empty()) { + retVal = _UsdChangeType::kUpdate; + } + } else if (it->IsPropertyPath()) { + // We have a bunch of UI properties to ignore. Especially anything that comes from UI + // schemas. + if (it->GetName().rfind("ui:", 0) == 0) { + continue; + } + retVal = _UsdChangeType::kUpdate; + for (const auto& entry : it.base()->second) { + if (entry->flags.didChangeAttributeConnection) { + retVal = _UsdChangeType::kResync; + break; + } + } + } + } + + return retVal; +} + +} // namespace + +bool MayaUsdProxyShapeUpdateManager::CanIgnoreObjectsChanged( + const UsdNotice::ObjectsChanged& notice) +{ + switch (_ClassifyUsdObjectsChanged(notice)) { + case _UsdChangeType::kIgnored: return true; + case _UsdChangeType::kResync: ++_UsdStageResyncCounter; + // [[fallthrough]]; // We want that fallthrough to have the update always triggered. + case _UsdChangeType::kUpdate: ++_UsdStageUpdateCounter; break; + } + + return false; +} + +MInt64 MayaUsdProxyShapeUpdateManager::GetUpdateCount() { return _UsdStageUpdateCounter; } + +MInt64 MayaUsdProxyShapeUpdateManager::GetResyncCount() { return _UsdStageResyncCounter; } + +PXR_NAMESPACE_CLOSE_SCOPE diff --git a/lib/mayaUsd/nodes/proxyShapeUpdateManager.h b/lib/mayaUsd/nodes/proxyShapeUpdateManager.h new file mode 100644 index 0000000000..7cb378fe56 --- /dev/null +++ b/lib/mayaUsd/nodes/proxyShapeUpdateManager.h @@ -0,0 +1,47 @@ +// +// Copyright 2023 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. +// +#ifndef PXRUSDMAYA_PROXY_SHAPE_UPDATE_MANAGER_H +#define PXRUSDMAYA_PROXY_SHAPE_UPDATE_MANAGER_H + +#include + +#include +#include + +#include + +PXR_NAMESPACE_OPEN_SCOPE + +class MayaUsdProxyShapeUpdateManager +{ +public: + MAYAUSD_CORE_PUBLIC + bool CanIgnoreObjectsChanged(const UsdNotice::ObjectsChanged& notice); + + MAYAUSD_CORE_PUBLIC + MInt64 GetUpdateCount(); + + MAYAUSD_CORE_PUBLIC + MInt64 GetResyncCount(); + +private: + MInt64 _UsdStageUpdateCounter { 1 }; + MInt64 _UsdStageResyncCounter { 1 }; +}; + +PXR_NAMESPACE_CLOSE_SCOPE + +#endif diff --git a/test/lib/ufe/testContextOps.py b/test/lib/ufe/testContextOps.py index bb330f1162..b0cd85846e 100644 --- a/test/lib/ufe/testContextOps.py +++ b/test/lib/ufe/testContextOps.py @@ -1483,20 +1483,20 @@ def testGeomCoponentAssignment(self): self.assertEqual(topSubset.GetFamilyNameAttr().Get(), "componentTag") self.assertFalse(topSubset.GetPrim().HasAPI(UsdShade.MaterialBindingAPI)) - counters= { "resync": cmds.getAttr(psPathStr + '.resyncid'), + counters= { "resync": cmds.getAttr(psPathStr + '.resyncId'), "update" : cmds.getAttr(psPathStr + '.upid')} def assertIsOnlyUpdate(self, counters, shapePathStr): - resyncCounter = cmds.getAttr(shapePathStr + '.resyncid') - updateCounter = cmds.getAttr(shapePathStr + '.updateid') + resyncCounter = cmds.getAttr(shapePathStr + '.resyncId') + updateCounter = cmds.getAttr(shapePathStr + '.updateId') self.assertEqual(resyncCounter, counters["resync"]) self.assertGreater(updateCounter, counters["update"]) counters["resync"] = resyncCounter counters["update"] = updateCounter def assertIsResync(self, counters, shapePathStr): - resyncCounter = cmds.getAttr(shapePathStr + '.resyncid') - updateCounter = cmds.getAttr(shapePathStr + '.updateid') + resyncCounter = cmds.getAttr(shapePathStr + '.resyncId') + updateCounter = cmds.getAttr(shapePathStr + '.updateId') self.assertGreater(resyncCounter, counters["resync"]) self.assertGreater(updateCounter, counters["update"]) counters["resync"] = resyncCounter diff --git a/test/lib/ufe/testUINodeGraphNode.py b/test/lib/ufe/testUINodeGraphNode.py index 4beb33c780..b89d5cfb08 100644 --- a/test/lib/ufe/testUINodeGraphNode.py +++ b/test/lib/ufe/testUINodeGraphNode.py @@ -60,8 +60,8 @@ def testUIInfo(self): ball3Path = ufe.PathString.path('|transform1|proxyShape1,/Ball_set/Props/Ball_3') ball3SceneItem = ufe.Hierarchy.createItem(ball3Path) - initialUpdateCount = cmds.getAttr('|transform1|proxyShape1.updateid') - initialResyncCount = cmds.getAttr('|transform1|proxyShape1.resyncid') + initialUpdateCount = cmds.getAttr('|transform1|proxyShape1.updateId') + initialResyncCount = cmds.getAttr('|transform1|proxyShape1.resyncId') uiNodeGraphNode = ufe.UINodeGraphNode.uiNodeGraphNode(ball3SceneItem) @@ -89,8 +89,8 @@ def testUIInfo(self): self.assertEqual(pos4.y(), pos5.y()) # None of these changes should force a render refresh: - self.assertEqual(initialUpdateCount, cmds.getAttr('|transform1|proxyShape1.updateid')) - self.assertEqual(initialResyncCount, cmds.getAttr('|transform1|proxyShape1.resyncid')) + self.assertEqual(initialUpdateCount, cmds.getAttr('|transform1|proxyShape1.updateId')) + self.assertEqual(initialResyncCount, cmds.getAttr('|transform1|proxyShape1.resyncId')) if __name__ == '__main__': unittest.main(verbosity=2)