Skip to content

Commit

Permalink
HYDRA-1102 : Fix instances selections being all wiped on an instancer…
Browse files Browse the repository at this point in the history
… when only one is removed (#158)

* HYDRA-1102 : Handle removing only specific selections rather than removing all

* HYDRA-1102 : Add test

* HYDRA-1102 : Remove unused function

* HYDRA-1102 : Add past-the-end iterator check

* HYDRA-1102 : Add comment in test

* HYDRA-1102 : Fix missing edge case handling
  • Loading branch information
debloip-adsk authored Aug 30, 2024
1 parent f496ea9 commit 7ee23a8
Show file tree
Hide file tree
Showing 11 changed files with 189 additions and 65 deletions.
16 changes: 16 additions & 0 deletions lib/flowViewport/fvpUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

#include "fvpUtils.h"

#include <pxr/imaging/hd/instanceIndicesSchema.h>
#include <pxr/imaging/hd/selectionSchema.h>

namespace FVP_NS_DEF {
Expand All @@ -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)
{
PXR_NS::HdInstanceIndicesSchema::Builder instanceIndicesBuilder;
instanceIndicesBuilder.SetInstancer(PXR_NS::HdRetainedTypedSampledDataSource<PXR_NS::SdfPath>::New(instancerPrimPath));
instanceIndicesBuilder.SetInstanceIndices(PXR_NS::HdRetainedTypedSampledDataSource<PXR_NS::VtArray<int>>::New({instanceIndex}));
PXR_NS::HdSelectionSchema::Builder selectionBuilder;
// Instancer is expected to be marked "fully selected" even if only certain instances are selected,
// based on USD's _AddToSelection function in selectionSceneIndexObserver.cpp :
// https://github.com/PixarAnimationStudios/OpenUSD/blob/f7b8a021ce3d13f91a0211acf8a64a8b780524df/pxr/imaging/hdx/selectionSceneIndexObserver.cpp#L212-L251
selectionBuilder.SetFullySelected(PXR_NS::HdRetainedTypedSampledDataSource<bool>::New(true));
auto instanceIndicesDataSource = PXR_NS::HdDataSourceBase::Cast(instanceIndicesBuilder.Build());
selectionBuilder.SetNestedInstanceIndices(PXR_NS::HdRetainedSmallVectorDataSource::New(1, &instanceIndicesDataSource));
return PXR_NS::HdDataSourceBase::Cast(selectionBuilder.Build());
}

} // namespace FVP_NS_DEF
2 changes: 2 additions & 0 deletions lib/flowViewport/fvpUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ class PrimvarDataSource final : public PXR_NS::HdContainerDataSource

PXR_NS::HdDataSourceBaseHandle FVP_API createFullySelectedDataSource();

PXR_NS::HdDataSourceBaseHandle FVP_API createInstanceSelectionDataSource(const PXR_NS::SdfPath& instancerPrimPath, int instanceIndex);

} // namespace FVP_NS_DEF

#endif // FVP_UTILS_H
7 changes: 6 additions & 1 deletion lib/flowViewport/sceneIndex/fvpPathInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@ namespace FVP_NS_DEF {
struct PrimSelection
{
PXR_NS::SdfPath primPath;
PXR_NS::HdDataSourceBaseHandle selectionDataSource;
std::optional<int> instanceIndex;

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

// Using TfSmallVector to optimize for selections that map to a few prims,
Expand Down
2 changes: 1 addition & 1 deletion lib/flowViewport/sceneIndex/fvpSelectionSceneIndex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
dirtiedPrims.emplace_back(primSelection.primPath, selectionsSchemaDefaultLocator);
}
}
Expand Down
4 changes: 1 addition & 3 deletions lib/flowViewport/selection/fvpPrefixPathMapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,7 @@ PrimSelections PrefixPathMapper::UfePathToPrimSelections(const Ufe::Path& appPat
primPath = primPath.AppendChild(TfToken(pathComponent.string()));
}

auto selectionDataSource = createFullySelectedDataSource();

return PrimSelections{{primPath, selectionDataSource}};
return PrimSelections{PrimSelection{primPath}};
}

}
76 changes: 51 additions & 25 deletions lib/flowViewport/selection/fvpSelection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,41 +38,53 @@

#include "flowViewport/selection/fvpSelection.h"

#include "flowViewport/fvpUtils.h"

#include "pxr/imaging/hd/selectionsSchema.h"

PXR_NAMESPACE_USING_DIRECTIVE

namespace FVP_NS_DEF {

HdDataSourceBaseHandle
Selection::_PrimSelectionState::GetVectorDataSource() const
{
return HdSelectionsSchema::BuildRetained(
selectionSources.size(), selectionSources.data()
);
};

bool
Selection::Add(const PrimSelection& primSelection)
{
if (primSelection.primPath.IsEmpty()) {
return false;
}

_pathToState[primSelection.primPath].selectionSources.push_back(primSelection.selectionDataSource);
_pathToSelections[primSelection.primPath].push_back(primSelection);

return true;
}

bool Selection::Remove(const PXR_NS::SdfPath& primPath)
bool Selection::Remove(const PrimSelection& primSelection)
{
return (!primPath.IsEmpty() && (_pathToState.erase(primPath) == 1));
if (primSelection.primPath.IsEmpty()) {
return false;
}

// Remove the specific selection
auto itSelection = std::find(
_pathToSelections[primSelection.primPath].begin(),
_pathToSelections[primSelection.primPath].end(),
primSelection);
if (itSelection != _pathToSelections[primSelection.primPath].end()) {
_pathToSelections[primSelection.primPath].erase(itSelection);
}

// If no selections remain, remove the entry entirely
if (_pathToSelections[primSelection.primPath].empty()) {
_pathToSelections.erase(primSelection.primPath);
}

return true;
}

void
Selection::Clear()
{
_pathToState.clear();
_pathToSelections.clear();
}

void Selection::Replace(const PrimSelections& primSelections)
Expand All @@ -83,34 +95,35 @@ void Selection::Replace(const PrimSelections& primSelections)
if (primSelection.primPath.IsEmpty()) {
continue;
}
_pathToState[primSelection.primPath].selectionSources.push_back(primSelection.selectionDataSource);
_pathToSelections[primSelection.primPath].push_back(primSelection);
}
}

void Selection::RemoveHierarchy(const PXR_NS::SdfPath& primPath)
{
auto it = _pathToState.lower_bound(primPath);
while (it != _pathToState.end() && it->first.HasPrefix(primPath)) {
it = _pathToState.erase(it);
auto it = _pathToSelections.lower_bound(primPath);
while (it != _pathToSelections.end() && it->first.HasPrefix(primPath)) {
it = _pathToSelections.erase(it);
}
}

bool Selection::IsEmpty() const
{
return _pathToState.empty();
return _pathToSelections.empty();
}

bool Selection::IsFullySelected(const SdfPath& primPath) const
{
return _pathToState.find(primPath) != _pathToState.end();
return _pathToSelections.find(primPath) != _pathToSelections.end()
&& !_pathToSelections.at(primPath).empty();
}

bool Selection::HasFullySelectedAncestorInclusive(const SdfPath& primPath, const SdfPath& topmostAncestor/* = SdfPath::AbsoluteRootPath()*/) const
{
// FLOW_VIEWPORT_TODO Prefix tree would be much higher performance
// than iterating over the whole selection, especially for a large
// selection. PPT, 13-Sep-2023.
for(const auto& entry : _pathToState) {
for(const auto& entry : _pathToSelections) {
if (primPath.HasPrefix(entry.first) && entry.first.HasPrefix(topmostAncestor)) {
return true;
}
Expand All @@ -124,7 +137,7 @@ SdfPathVector Selection::FindFullySelectedAncestorsInclusive(const SdfPath& prim
// than iterating over the whole selection, especially for a large
// selection. PPT, 13-Sep-2023.
SdfPathVector fullySelectedAncestors;
for(const auto& entry : _pathToState) {
for(const auto& entry : _pathToSelections) {
if (primPath.HasPrefix(entry.first) && entry.first.HasPrefix(topmostAncestor)) {
fullySelectedAncestors.push_back(entry.first);
}
Expand All @@ -135,8 +148,8 @@ SdfPathVector Selection::FindFullySelectedAncestorsInclusive(const SdfPath& prim
SdfPathVector Selection::GetFullySelectedPaths() const
{
SdfPathVector fullySelectedPaths;
fullySelectedPaths.reserve(_pathToState.size());
for(const auto& entry : _pathToState) {
fullySelectedPaths.reserve(_pathToSelections.size());
for(const auto& entry : _pathToSelections) {
fullySelectedPaths.emplace_back(entry.first);
}
return fullySelectedPaths;
Expand All @@ -146,9 +159,22 @@ HdDataSourceBaseHandle Selection::GetVectorDataSource(
const PXR_NS::SdfPath& primPath
) const
{
auto it = _pathToState.find(primPath);
return (it != _pathToState.end()) ?
it->second.GetVectorDataSource() : nullptr;
auto it = _pathToSelections.find(primPath);
if (it == _pathToSelections.end()) {
return nullptr;
}

std::vector<HdDataSourceBaseHandle> selectionDataSources;
for (const auto& selection : it->second) {
if (selection.instanceIndex.has_value()) {
selectionDataSources.push_back(createInstanceSelectionDataSource(selection.primPath, selection.instanceIndex.value()));
} else {
selectionDataSources.push_back(createFullySelectedDataSource());
}
}
return HdSelectionsSchema::BuildRetained(
selectionDataSources.size(), selectionDataSources.data()
);
}

}
13 changes: 3 additions & 10 deletions lib/flowViewport/selection/fvpSelection.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class Selection

// Returns true if the removal was successful, false otherwise.
FVP_API
bool Remove(const PXR_NS::SdfPath& primPath);
bool Remove(const PrimSelection& primSelection);

// Replace the selection with the contents of the argument primPath vector.
// Any empty primPath in the argument will be skipped.
Expand Down Expand Up @@ -93,17 +93,10 @@ class Selection
GetVectorDataSource(const PXR_NS::SdfPath& primPath) const;

private:

struct _PrimSelectionState {
// Container data sources conforming to HdSelectionSchema
std::vector<PXR_NS::HdDataSourceBaseHandle> selectionSources;

PXR_NS::HdDataSourceBaseHandle GetVectorDataSource() const;
};

// Maps prim path to data sources to be returned by the vector data
// Maps prim path to selections to be returned by the vector data
// source at locator selections.
std::map<PXR_NS::SdfPath, _PrimSelectionState> _pathToState;
std::map<PXR_NS::SdfPath, std::vector<PrimSelection>> _pathToSelections;
};

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -759,11 +759,7 @@ Fvp::PrimSelections MayaHydraSceneIndex::UfePathToPrimSelections(const Ufe::Path
// the UFE path to a string, then does a Dag path lookup with the string.
constexpr bool isSprim = false; // Can't handle sprims as of 15-Aug-2023.
SdfPath primPath = GetPrimPath(UfeExtensions::ufeToDagPath(appPath), isSprim);
HdSelectionSchema::Builder selectionBuilder;
selectionBuilder.SetFullySelected(HdRetainedTypedSampledDataSource<bool>::New(true));
auto selectionDataSource = HdDataSourceBase::Cast(selectionBuilder.Build());
Fvp::PrimSelection primSelection {primPath, selectionDataSource};
return Fvp::PrimSelections({primSelection});
return Fvp::PrimSelections({Fvp::PrimSelection{primPath}});
}

SdfPath MayaHydraSceneIndex::SetCameraViewport(const MDagPath& camPath, const GfVec4d& viewport)
Expand Down
26 changes: 6 additions & 20 deletions lib/mayaHydra/hydraExtensions/sceneIndex/registration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,21 +87,6 @@ class SceneObserver : public Observer
}
};

HdDataSourceBaseHandle createInstanceSelectionDataSource(const SdfPath& instancerPrimPath, int instanceIndex)
{
HdInstanceIndicesSchema::Builder instanceIndicesBuilder;
instanceIndicesBuilder.SetInstancer(HdRetainedTypedSampledDataSource<SdfPath>::New(instancerPrimPath));
instanceIndicesBuilder.SetInstanceIndices(HdRetainedTypedSampledDataSource<VtArray<int>>::New({instanceIndex}));
HdSelectionSchema::Builder selectionBuilder;
// Instancer is expected to be marked "fully selected" even if only certain instances are selected,
// based on USD's _AddToSelection function in selectionSceneIndexObserver.cpp :
// https://github.com/PixarAnimationStudios/OpenUSD/blob/f7b8a021ce3d13f91a0211acf8a64a8b780524df/pxr/imaging/hdx/selectionSceneIndexObserver.cpp#L212-L251
selectionBuilder.SetFullySelected(HdRetainedTypedSampledDataSource<bool>::New(true));
auto instanceIndicesDataSource = HdDataSourceBase::Cast(instanceIndicesBuilder.Build());
selectionBuilder.SetNestedInstanceIndices(HdRetainedSmallVectorDataSource::New(1, &instanceIndicesDataSource));
return HdDataSourceBase::Cast(selectionBuilder.Build());
}

/// \class PathInterfaceSceneIndex
///
/// Implement the path interface for plugin scene indices.
Expand Down Expand Up @@ -159,10 +144,11 @@ class PathInterfaceSceneIndex : public Fvp::PathInterfaceSceneIndexBase
}

const auto lastComponentString = secondSegment.components().back().string();
HdDataSourceBaseHandle selectionDataSource = lastComponentIsNumeric
? createInstanceSelectionDataSource(primPath, std::stoi(lastComponentString))
: Fvp::createFullySelectedDataSource();
Fvp::PrimSelections primSelections({{primPath, selectionDataSource}});
std::optional<int> instanceIndex = std::nullopt;
if (lastComponentIsNumeric) {
instanceIndex = std::stoi(lastComponentString);
}
Fvp::PrimSelections primSelections({{primPath, instanceIndex}});

// Propagate selection to propagated prototypes
auto ancestorsRange = primPath.GetAncestorsRange();
Expand All @@ -188,7 +174,7 @@ class PathInterfaceSceneIndex : public Fvp::PathInterfaceSceneIndexBase
// for another instancer B will only mark the geometry-drawing instancer A as selected. This can be changed.
// For now (2024/05/28), this only affects selection highlighting.
if (propagatedPrim.primType != HdPrimTypeTokens->instancer) {
primSelections.push_back({propagatedPrimPath, selectionDataSource});
primSelections.push_back({propagatedPrimPath, instanceIndex});
}
}
}
Expand Down
Loading

0 comments on commit 7ee23a8

Please sign in to comment.