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-1271 : Selection highlighting rework #240

Merged
merged 151 commits into from
Jan 27, 2025

Conversation

debloip-adsk
Copy link
Collaborator

@debloip-adsk debloip-adsk commented Jan 21, 2025

Rework of the selection highlighting infrastructure.

Instead of having a single scene index handling all cases, the logic is now split up across multiple scene indices, each dedicated to handling highlights for a specific type of prim. The common logic between them is extracted out in a base class, which makes each scene index independent of each other. See the BaseWhSi.h doc comment for an explanation on how selection highlights are structured.

Note that sets & maps keyed by SdfPaths are heavily used in order to accelerate finding the children and/or parents of paths among a given collection. Generally speaking, lower_bound will be used to find children of a given path (or the path itself), while upper_bound followed by an iterator decrement will be used to find a parent of a path.

Existing highlighting tests were replaced by image-based tests, and new tests were added for native instancing highlighting. Reasoning behind going with image-based tests is that we don't care all that much about how the highlight prims are structured as opposed to how the result looks. And since there are a lot of cases to handle, it is much faster & simpler to go with image-based tests (and it also allows us to easily see if a case actually works or not).

Possible improvements/alternatives for the future :

  • Currently, to handle wireframe color changes, the MhDirtyLeadObjectSceneIndex also forwards the dirty notifications to instancers' prototypes. However, this could be handled by the instancing-related highlight scene indices instead in order to reduce the amount of notifications sent.
  • Extract out the common logic between the point instancer scene index and point instance prototype scene index.
  • The mesh scene index currently includes all of the mesh's parents and children in its sub-hierarchy, but we could potentially do like the geomSubset scene index and only return the mesh itself.
  • Extract out the shared logic found in the BaseWhSi class into a single driver class, which would control the specialized scene indices. This would avoid having each scene index repeat the processing of the BaseWhSi and maintain separate copies of the fully selected paths.
  • Instead of tracking fully selected paths in a standalone manner and re-implementing HasFullySelectedAncestorInclusive, hook the scene indices to the Flow Viewport selection and use the Fvp::Selection's version of the method. This would also require the Fvp::Selection to send out notifications whenever a path starts or stops being fully selected to accomodate ProcessFullySelectedChange.
  • HYDRA-1407 : When both a mesh and a geomSubset highlight coexist, the mesh highlight will always show up on top of the geomSubset highlight, which can lead to incorrect colors.

@debloip-adsk debloip-adsk added ready-for-merge Development process is finished, PR is ready for merge feature New feature or request fvp-toolkit Flow Viewport Toolkit and removed feature New feature or request labels Jan 24, 2025
@debloip-adsk debloip-adsk merged commit aad36a1 into dev Jan 27, 2025
10 checks passed
@debloip-adsk debloip-adsk deleted the debloip/HYDRA-1271/selection-highlight-refactor-2 branch January 27, 2025 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fvp-toolkit Flow Viewport Toolkit 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.

2 participants