MAYA-106780 - Editing USD should mark scene as dirty #1061
Conversation
* Added catch-all condition in response to USD stage changes. If no UFE notif is sent, but we were given paths, send a UFE subtreeInvalidate notif.
seando-adsk
added
bug
lib/mayaUsd/ufe/StagesSubject.cpp
Outdated
| } | ||
|
|
||
| auto sceneItem = Ufe::Hierarchy::createItem(ufePath); | ||
| Ufe::Scene::instance().notify(Ufe::SubtreeInvalidate(sceneItem)); |
There was a problem hiding this comment.
This feels very awkward. SubtreeInvalidate is meant to imply a structural change to the hierarchy, but you're adding info-only changes to it. I don't think we should be doing that.
There was a problem hiding this comment.
We discussed this internally and since we have no other UFE notif to send this was the only thing we could do. One idea was the creation of a new UFE notif (such as SceneDirty) but we decided against that. @kxl-adsk Can you provide more info?
There was a problem hiding this comment.
I agree that the catch-all UFE notification for structural change (i.e. ResyncedPaths) should be SubtreeInvalidate --- but isn't that the case already?
For info-only changes, we should be able to extract an attribute out of the notice, and shouldn't need SubtreeInvalidate --- no?
There was a problem hiding this comment.
Okay I reworked it as we discussed to send Ufe attribute changed notif for the ChangedInfoOnlyPaths. I also incorporated the upcoming UFE changes (.36) to change the notif() method.
I also added the special case when both resync and changed only paths are empty.
lib/mayaUsd/ufe/StagesSubject.cpp
Outdated
| #ifdef UFE_V2_FEATURES_AVAILABLE | ||
| // If we get here and didn't send any UFE notif, but yet have paths | ||
| // then we'll send a catch-all subtree invalidate notification. | ||
| if (!anyUfeV2NotifSent) { | ||
| SdfPathVector allPaths = (SdfPathVector)notice.GetResyncedPaths(); | ||
| allPaths.insert( | ||
| allPaths.end(), | ||
| notice.GetChangedInfoOnlyPaths().begin(), | ||
| notice.GetChangedInfoOnlyPaths().end()); | ||
| SdfPath::RemoveDescendentPaths(&allPaths); | ||
| for (const auto& changedPath : allPaths) { | ||
| Ufe::Path ufePath; | ||
| if (changedPath == SdfPath::AbsoluteRootPath()) { | ||
| ufePath = stagePath(sender); | ||
| } else { | ||
| const std::string& usdPrimPathStr = changedPath.GetPrimPath().GetString(); | ||
| ufePath = stagePath(sender) + Ufe::PathSegment(usdPrimPathStr, g_USDRtid, '/'); | ||
| } | ||
|
|
||
| auto sceneItem = Ufe::Hierarchy::createItem(ufePath); | ||
| Ufe::Scene::instance().notify(Ufe::SubtreeInvalidate(sceneItem)); | ||
| } | ||
| } | ||
| #endif |
There was a problem hiding this comment.
The bulk of the change is here. I added a new boolean flag to know if any UFE notif is sent above. If not, then we merge the ResyncedPaths and ChangedInfoOnlyPaths and only keep the highest common ancestor paths. Then send a UFE subtree invalidate for each remaining path.
There was a problem hiding this comment.
As an example if you add an anonymous layer (via the Layer Editor) this stageChanged() method is called with a few ChangedInfoOnlyPaths (no ResyncPaths). Nothing is done with these notifs as they don't meet any of our specific conditions. So they will now be caught in this new block and send a subtree invalidate notif. This is used to refresh and and set dirty the Maya scene.
There was a problem hiding this comment.
Ufe::SubtreeInvalidate is v2 only, thus all this code is ifdef protected (including the new bool).
lib/mayaUsd/ufe/StagesSubject.cpp
Outdated
| } | ||
|
|
||
| auto sceneItem = Ufe::Hierarchy::createItem(ufePath); | ||
| Ufe::Scene::instance().notify(Ufe::SubtreeInvalidate(sceneItem)); |
There was a problem hiding this comment.
We discussed this internally and since we have no other UFE notif to send this was the only thing we could do. One idea was the creation of a new UFE notif (such as SceneDirty) but we decided against that. @kxl-adsk Can you provide more info?
* Reworked to send ufe attribute value changed for the ChangedInfoOnlyPaths (instead of subtreeInvalidate).
* Oops forgot to commit test
|
@ppt-adsk Oops sorry I forgot to commit the change to the test. |
* Fixed mistake in test for older UFE version.
| if(os.getenv('UFE_PREVIEW_VERSION_NUM', '0000') >= '2036'): | ||
| if isinstance(notification, ufe.AttributeValueChanged): | ||
| self._notifications += 1 | ||
| else: | ||
| if isinstance(notification, ufe.AttributeChanged): | ||
| self._notifications += 1 |
There was a problem hiding this comment.
Oops sorry. Of course the test passed locally for me because I had UFE 0.2.36. I forgot to account for older UFE versions.
seando-adsk
added
the
ready-for-merge
|
@kxl-adsk I had a random test timeout on master Windows python3 only. Please merge. In the meantime, I'll re-run the preflight just to verify that it was indeed random. |
kxl-adsk
deleted the
donnels/MAYA-106780/editing_usd_should_mark_scene_as_dirty
branch
MAYA-106780 - Editing USD should mark scene as dirty