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

Made proxy shape scene index prefixes unique, and support rename / reparent #84

Merged
merged 8 commits into from
Feb 27, 2024

Conversation

ppt-adsk
Copy link
Collaborator

When adding to the scene index scene, proxy shapes have to create a unique name under their rprim parent. When translating application scene paths to rprim paths, any change in the application scene path to scene index prefix path must be tracked by the path interface scene index. We therefore track renaming the proxy shape, or reparenting the proxy shape itself or an ancestor.

@ppt-adsk ppt-adsk self-assigned this Feb 22, 2024
@ppt-adsk ppt-adsk assigned ppt-adsk and unassigned ppt-adsk Feb 26, 2024
Copy link
Collaborator

@debloip-adsk debloip-adsk left a comment

Choose a reason for hiding this comment

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

Approving, presumably the preflights are going to keep failing due to the compile-time MayaUSD API not found? If so and the tests pass locally, it might be worth force merging, as I have changes upcoming for HYDRA-834 which will likely have a few conflicts to resolve.

@ppt-adsk
Copy link
Collaborator Author

Approving, presumably the preflights are going to keep failing due to the compile-time MayaUSD API not found? If so and the tests pass locally, it might be worth force merging, as I have changes upcoming for HYDRA-834 which will likely have a few conflicts to resolve.

That would be awkward... It would mean that all pre-flights would fail until the mayaUsdAPI support is added. Could you not merge your branch with this branch and resolve conflicts that arise?

@debloip-adsk
Copy link
Collaborator

Ah yeah I didn't think this through at all now did I... Problem is I was about to open a PR for HYDRA-834, so by merging both branches it couldn't be merged as well. Though we might want to wait until we have a USD build with the configurable float/double printing to run in our preflights before merging the PR for HYDRA-834 anyways, otherwise we'll be merging tests that aren't run yet, but will suddenly be in the future. Just feels a bit annoying that we have a couple dependent things floating around

@ppt-adsk ppt-adsk assigned ppt-adsk and unassigned ppt-adsk Feb 27, 2024
@ppt-adsk ppt-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Feb 27, 2024
@lilike-adsk lilike-adsk merged commit 6678345 into dev Feb 27, 2024
10 checks passed
@lilike-adsk lilike-adsk deleted the tremblp/HYDRA-704/selection_highlighting_fix branch February 27, 2024 18:24
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants