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-1102 : Fix instances selections being all wiped on an instancer when only one is removed #158

Merged
merged 6 commits into from
Aug 30, 2024

Conversation

debloip-adsk
Copy link
Collaborator

No description provided.

@debloip-adsk debloip-adsk self-assigned this Aug 22, 2024
@@ -39,4 +40,19 @@ PXR_NS::HdDataSourceBaseHandle createFullySelectedDataSource()
return PXR_NS::HdDataSourceBase::Cast(selectionBuilder.Build());
}

PXR_NS::HdDataSourceBaseHandle createInstanceSelectionDataSource(const PXR_NS::SdfPath& instancerPrimPath, int instanceIndex)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved over as-is from registration.cpp

Comment on lines +38 to +43
std::optional<int> instanceIndex;

inline bool operator==(const PrimSelection &rhs) const {
return primPath == rhs.primPath
&& instanceIndex == rhs.instanceIndex;
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fundamental design change of the pull request : PrimSelection objects, returned by the different data producers, no longer return selection data sources directly, as data sources can't be efficiently compared. This posed problems when wanting to remove only specific selections (such as instance selections) on a prim in Fvp::Selection::Remove.

PrimSelections now only contain the core data required to create the selection data sources, which are now created by the Fvp::Selection. The information stored in PrimSelection is based off the HdSelectionSchema class, which currently only specifies full prim and instance selections.

@@ -184,7 +184,7 @@ void SelectionSceneIndex::RemoveSelection(const Ufe::Path& appPath)

HdSceneIndexObserver::DirtiedPrimEntries dirtiedPrims;
for (const auto& primSelection : primSelections) {
if (_selection->Remove(primSelection.primPath)) {
if (_selection->Remove(primSelection)) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Core change : we now only remove the specific selections corresponding to the deselected UFE path, rather than all selections on the prim.

// source at locator selections.
std::map<PXR_NS::SdfPath, _PrimSelectionState> _pathToState;
std::map<PXR_NS::SdfPath, std::vector<PrimSelection>> _pathToSelections;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Now only stores the PrimSelection objects directly, which are fairly small, and which each correspond to a selection data source, constructed in GetVectorDataSource.

Copy link
Collaborator

@ppt-adsk ppt-adsk left a comment

Choose a reason for hiding this comment

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

Great work, nice test, thanks!

@debloip-adsk debloip-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Aug 23, 2024
@roopavr-adsk roopavr-adsk merged commit 7ee23a8 into dev Aug 30, 2024
10 checks passed
@roopavr-adsk roopavr-adsk deleted the debloip/HYDRA-1102/instance-selection-mishandling branch August 30, 2024 13:31
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