-
Notifications
You must be signed in to change notification settings - Fork 202
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
Optimized repr update, wireframe and BBox display, proxy transformation and visibility #112
Optimized repr update, wireframe and BBox display, proxy transformation and visibility #112
Conversation
huidong-chen
commented
Nov 12, 2019
•
edited
Loading
edited
- Optimized VP2RenderDelegate repr update.
- Added wireframe display for USD mesh.
- Added bbox display for USD mesh (BBox will be hidden for Rprims without authored extent).
- Added proxy shape transformation and visibility support (refactoring_sandbox : VP2 Delegate : Hiding ProxyShape Does Not Work #70, refactoring_sandbox : VP2 Delegate : Moving Proxy Shape Does Not Update Prim Positions in Viewport #71, refactoring_sandbox : Hydra : Moved USD Proxy makes Prim selection highlight still behave as if at origin #82)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already reviewed it internally.
Thanks for the updates ! Here are my results : #70 ( hiding proxy shape) still does not work for me. It only seems to work after I do a However, I can confirm the following work:
|
AL Unit test works in UI, but crashes in prompt mode : Result: untitled.ma |
@cfmoore007 Thanks a lot for such quickly feedback. For visibility, could you add some repro steps? For the crash, it is one of the crash I hit and fixed on Windows. Let me see if it is still there on Linux. |
@HdC-adsk : here you go ...
|
side note : this also seems to fix the crash - #69 However, it still does not show the duplicate proxyShape's stage. Will log that after this is merged |
Duplicate crash - the complete fix has not been merged in yet. |
@cfmoore007 I can reproduce the hiding issue. It can work if you hit "h" without ctrl, I am investigating the difference between h and Ctrl-h. For the crash at HdVP2BBoxGeom, is there a repro step for me as well? |
Hiding :
For the crash :
|
@cfmoore007 Thanks for confirm. I tried this command below and saw no crash. How do you load maya in prompt mode? It looks like you can do it with two separate steps. |
It crashes here with the command you provided and in maya -prompt ( interactive ) and via -command. |
Hi @cfmoore007 , I am creating a branch with a potential fix (as I cannot reproduce it I cannot confirm). I will push it later and ask for your help to confirm. Appreciated as always. |
@cfmoore007 do you see this crash on other platforms e.g ( Windows or MacOSx )? Could be Linux specific. |
@kxl-adsk The new commit is to fix the crash in AL unit test crash in Maya prompt mode and the hiding issue (H works but Ctrl+H didn't). |
@@ -364,6 +364,13 @@ void HdVP2Mesh::Sync( | |||
HdDirtyBits* dirtyBits, | |||
const TfToken& reprToken) | |||
{ | |||
// We don't create a repr for the selection token because this token serves |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an optimization in Hydra that would skip sync if a Rprim is invisible and has no visibility dirty bit:
Due to this optimization, if we have a selection update before regular refresh, the scene dirty bit gets cleared in selection update and no sync can be invoked for the hiding proxy. Thus we need the earlier return for the selection update to reserver the scene dirty bits.
_sceneDelegate->SetRootVisibility(isVisible); | ||
|
||
// Trigger selection update when a hidden proxy shape gets shown. | ||
if (isVisible) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trigger the selection update when an invisible proxy gets shown. To make this change, I also make the explicit check to see whether there is visibility change (which is checked inside SetRootVisibility() as well).
To make code consistent, I also refactored root transform part, a side benefit of this is the matrix comparison is done by approximation (GfIsClose) rather than equality. This is a TODO item inside SetRootTransform().
FWIW, we shouldn't do similar code change to skip SetTime() call when the time code is equivalent. SetTime() will apply pending edits before its internal time equality check.
// future we should move this code out. | ||
if (TF_VERIFY(sSharedBBoxGeom == nullptr)) { | ||
sSharedBBoxGeom = new HdVP2BBoxGeom(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't repro the crash in AL unit test crash in Maya prompt mode. The callstack appears to indicate HdVP2BBoxGeom was instantiated before VP2 initialization. Thus I make this change to make sure it is instantiated after VP2 initialization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AND @cfmoore007 helped to validate the fix before I pushing the commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense!
Just re-tested this PR. Looks like everything came through successfully :
Thanks for the quick turn-around @HdC-adsk ! |
@cfmoore007 Thanks for retesting and feedback as fast as always. Very much appreciated! |
…mation-visibility