Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MAYA-114247 Mark everything dirty when display modes change. #1891

Merged
merged 6 commits into from
Jan 5, 2022

Conversation

williamkrick
Copy link
Contributor

This fixes the issue Grace found with #1752.

MayaUSD skips updating render items whose repr is not currently in use, instead saving dirty bits about the unused items so that when they are used they can be correctly updating. With the update to USD 21.11 we're not getting a sync call for these rprims which have some dirty reprs. We clean all the dirty bits off the rprim, so it makes sense that USD skips syncing them.

I don't want to leave the rprims dirty, because then we'll have to call sync on them every frame. Instead, when the reprs in use change mark every rprim "dirty" with a flag that will cause sync to be called, but which won't trigger any actual work to occur. This way any dirty reprs get a chance to update themselves.

perrauo-adsk
perrauo-adsk previously approved these changes Dec 8, 2021
@@ -69,6 +69,7 @@ struct MayaPrimCommon
{
DirtySelectionHighlight = HdChangeTracker::CustomBitsBegin,
DirtySelectionMode = (DirtySelectionHighlight << 1),
DirtyDisplayMode = (DirtySelectHighlight << 1),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this bit chosen for dirty display mode?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DirtyDisplayMode is just meant to be the next bit in the enum. The original changes were wrong, there is an updated version now which is correct.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok thanks, I'm just not used to seeing flags and plain enums mixed together.

@@ -69,6 +69,7 @@ struct MayaPrimCommon
{
DirtySelectionHighlight = HdChangeTracker::CustomBitsBegin,
DirtySelectionMode = (DirtySelectionHighlight << 1),
DirtyDisplayMode = (DirtySelectHighlight << 1),
DirtyBitLast = DirtySelectionMode
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to update DirtyBitLast?

@williamkrick williamkrick added ready-for-merge Development process is finished, PR is ready for merge and removed ready-for-merge Development process is finished, PR is ready for merge labels Dec 8, 2021
// if there are no repr's to update then don't even call sync.
if (reprSelector != HdReprSelector()) {
if (_defaultCollection->GetReprSelector() != reprSelector) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the new point snapping support when we do a selection pass we often end up with an empty reprSelector. If that happens don't save it to _defaultCollection, because doing so trashes the previous reprSelector we had saved, and causes the full scene MarkRprimDirty to run. We want to avoid running that as much as possible because the cost of even the empty sync calls is non-zero.

@williamkrick williamkrick added the ready-for-merge Development process is finished, PR is ready for merge label Dec 9, 2021
@jufrantz
Copy link
Contributor

Hello @williamkrick,

Commenting here as we are testing maya-usd x USD 21.11 and also noticed the selection highlighting problems reported in issue #1752.

First we tried this PR and it fixes on our side the wireframe highlighting after a display mode change.

But there is still a problem, we don't get selection highlighting updates when we are just changing selection while the viewport is in wireframe display modes. Not sure that it is related to this PR, but wanted to let you know what I found while the issue is beeing worked.

I noticed that ProxyRenderDelegate::_UpdateSelectionStates does not trigger HdVP2Mesh::Sync for wire representation on rprim whose selection status changed, only smoothHull items are synchronized.

By changing _UpdateSelectionStates to use the defaultCollection reprSelector (related to current maya display mode), it works fine on our side.

--- a/lib/mayaUsd/render/vp2RenderDelegate/proxyRenderDelegate.cpp
+++ b/lib/mayaUsd/render/vp2RenderDelegate/proxyRenderDelegate.cpp
@@ -1263,7 +1263,7 @@ void ProxyRenderDelegate::_UpdateSelectionStates()
 
         // now that the appropriate prims have been marked dirty trigger
         // a sync so that they all update.
-        HdRprimCollection collection(HdTokens->geometry, kSmoothHullReprSelector);
+        HdRprimCollection collection(HdTokens->geometry, _defaultCollection->GetReprSelector());
         collection.SetRootPaths(rootPaths);
         _taskController->SetCollection(collection);
         _engine.Execute(_renderIndex.get(), &_dummyTasks);

Hope that helps

@williamkrick williamkrick removed the ready-for-merge Development process is finished, PR is ready for merge label Dec 15, 2021
@williamkrick
Copy link
Contributor Author

@jufrantz that makes sense to me, thanks! I will test it out locally and update the PR.

@williamkrick williamkrick added the ready-for-merge Development process is finished, PR is ready for merge label Jan 5, 2022
@gracekumagai
Copy link

Hey @williamkrick,

I was testing the PR and I can no longer reproduce the issues I reported in #1752 - thank you!

However, I found another issue with the wireframe display mode and selection but it might be out of scope for this PR.

If you select the proxy shape in the wireframe display mode and clear your selection, the selection highlight is still displayed for its children. This does not occur in the smooth shaded mode. If you select one if its children, the selection highlight still displays for all the other children (even if the selection is cleared). If you select each child, it seems to reset the selection highlight and behave as expected.

wireframe_after_proxyshape_selected

Let me know if you'd like me to file a separate ticket.

@williamkrick
Copy link
Contributor Author

@gracekumagai I think it is probably related but lets log another one to get this at least partially working.

@williamkrick williamkrick removed the request for review from perrauo-adsk January 5, 2022 18:12
@seando-adsk seando-adsk merged commit 7e43868 into dev Jan 5, 2022
@seando-adsk seando-adsk deleted the krickw/MAYA-114247/update_every_repr branch January 5, 2022 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge Development process is finished, PR is ready for merge vp2renderdelegate Related to VP2RenderDelegate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants