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

HYDRA-747 : Filtering scene index example can have prims get unfiltered at the wrong location #42

Merged
merged 9 commits into from
Feb 2, 2024

Conversation

debloip-adsk
Copy link
Collaborator

@debloip-adsk debloip-adsk commented Jan 18, 2024

The issue in this case was that when the prim was added, it was initially pruned out. Then, when changing the number of vertices so that it is no longer pruned, only a PrimsDirtied notification was sent on the mesh. This means that downstream scene indices were never made aware of the prim's xform data source, or of the prim's addition.

This issue is more general in that pruning scene indices such as this one need to be aware of which prims are initially pruned out, so that they can provide a coherent scene view to downstream scene indices. The current solution to this issue is to traverse the scene in the scene index ctor and store which prims are initially pruned, and update this set in PrimsAdded and PrimsRemoved. That way, in PrimsDirtied, we can always correctly determine for a given prim whether to forward the dirty notification, or if we should send a PrimsAdded/PrimsRemoved if a prim's pruning status changed.

I have asked a question on the AOUSD forums regarding this issue : https://forum.aousd.org/t/pruning-prims-based-on-their-data-source-contents-and-handling-dirty-notifications/1111. The conclusion is that scene indices should always present a coherent view to to their observers, and send notifications as needed to keep it as such. So in our example case, either we send PrimsAdded and PrimsRemoved notifications every time the vertices change, potentially causing additional prim re-syncs, or we traverse the scene hierarchy initially, store prims' filtering status and send notifications accordingly when they are dirtied, potentially incurring extra load-time cost (or whenever we'd modify the scene index chain). The current implementation uses the latter approach.

@debloip-adsk debloip-adsk marked this pull request as ready for review January 31, 2024 16:42
@debloip-adsk debloip-adsk self-assigned this Jan 31, 2024
@debloip-adsk debloip-adsk changed the title [WIP] HYDRA-747 : Filtering scene index example can have prims get unfiltered at the wrong location HYDRA-747 : Filtering scene index example can have prims get unfiltered at the wrong location Jan 31, 2024
@debloip-adsk
Copy link
Collaborator Author

Copy link
Collaborator

@lanierd-adsk lanierd-adsk left a comment

Choose a reason for hiding this comment

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

Can you please add a unit test for the different cases ? So we are sure it works and we don't break it any more.

@debloip-adsk
Copy link
Collaborator Author

Can you please add a unit test for the different cases ? So we are sure it works and we don't break it any more.

I've added a check for the renderer switching case which is the original bug that started this, as well as for filtering and unfiltering in similar cases. However, I can't really test that child prims of a filtered prim are also filtered out, because from what I see our mesh prims are always leaf prims, so they never have children.

@ppt-adsk
Copy link
Collaborator

ppt-adsk commented Feb 2, 2024

Can you please add a unit test for the different cases ? So we are sure it works and we don't break it any more.

I've added a check for the renderer switching case which is the original bug that started this, as well as for filtering and unfiltering in similar cases. However, I can't really test that child prims of a filtered prim are also filtered out, because from what I see our mesh prims are always leaf prims, so they never have children.

This will always be the case for Maya, since shape nodes must be leaves, and is the rule for well-constructed USD scenes as well --- see
https://openusd.org/release/glossary.html#usdglossary-gprim

@debloip-adsk debloip-adsk added ready-for-merge Development process is finished, PR is ready for merge bug Something isn't working labels Feb 2, 2024
@roopavr-adsk roopavr-adsk merged commit 1cd9715 into dev Feb 2, 2024
10 checks passed
@roopavr-adsk roopavr-adsk deleted the debloip/HYDRA-747/example-filter-prim-location branch February 2, 2024 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ready-for-merge Development process is finished, PR is ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants