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

Fix UsdGeomPointInstancer ComputeExtents when prototypes are under an over #3396

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 33 additions & 10 deletions pxr/usd/usdGeom/bboxCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "pxr/usd/usdGeom/xform.h"

#include "pxr/usd/usd/modelAPI.h"
#include "pxr/usd/usd/primFlags.h"
#include "pxr/usd/usd/primRange.h"

#include "pxr/base/trace/trace.h"
Expand Down Expand Up @@ -280,6 +281,15 @@ _GetOrCreateExtentsHintQuery(UsdGeomModelAPI& geomModel, UsdAttributeQuery* q)
return *q;
}


// Default prim traversal predicate to use
// Note we do not exclude unloaded prims - we want them because they
// may have authored extentsHints we can use; thus we can have bboxes in
// model-hierarchy-only.
static const Usd_PrimFlagsPredicate _defaultPrimPredicate = (
UsdPrimIsActive && UsdPrimIsDefined && !UsdPrimIsAbstract
);

// -------------------------------------------------------------------------- //
// UsdGeomBBoxCache Public API
// -------------------------------------------------------------------------- //
Expand All @@ -290,6 +300,20 @@ UsdGeomBBoxCache::UsdGeomBBoxCache(
: _time(time)
, _includedPurposes(includedPurposes)
, _ctmCache(time)
, _primPredicate(_defaultPrimPredicate)
, _useExtentsHint(useExtentsHint)
, _ignoreVisibility(ignoreVisibility)
{
}

UsdGeomBBoxCache::UsdGeomBBoxCache(
UsdTimeCode time, TfTokenVector includedPurposes,
const Usd_PrimFlagsPredicate &predicate,
bool useExtentsHint, bool ignoreVisibility)
: _time(time)
, _includedPurposes(includedPurposes)
, _ctmCache(time)
, _primPredicate(predicate)
, _useExtentsHint(useExtentsHint)
, _ignoreVisibility(ignoreVisibility)
{
Expand All @@ -301,6 +325,7 @@ UsdGeomBBoxCache::UsdGeomBBoxCache(UsdGeomBBoxCache const &other)
, _includedPurposes(other._includedPurposes)
, _ctmCache(other._ctmCache)
, _bboxCache(other._bboxCache)
, _primPredicate(other._primPredicate)
, _useExtentsHint(other._useExtentsHint)
, _ignoreVisibility(other._ignoreVisibility)
{
Expand All @@ -316,6 +341,7 @@ UsdGeomBBoxCache::operator=(UsdGeomBBoxCache const &other)
_includedPurposes = other._includedPurposes;
_ctmCache = other._ctmCache;
_bboxCache = other._bboxCache;
_primPredicate = other._primPredicate;
_useExtentsHint = other._useExtentsHint;
_ignoreVisibility = other._ignoreVisibility;
return *this;
Expand Down Expand Up @@ -439,7 +465,7 @@ UsdGeomBBoxCache::_ComputeBoundWithOverridesHelper(
}

GfBBox3d result;
UsdPrimRange range(prim);
UsdPrimRange range(prim, _primPredicate);
Copy link
Author

Choose a reason for hiding this comment

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

I was a bit unsure about this change. Looking at where this is called, it seemed to me that this should be following the same predicate as the other traversals, but it was not before. Let me know if I need to roll this back

for (auto it = range.begin(); it != range.end(); ++it) {
const UsdPrim &p = *it;
const SdfPath &primPath = p.GetPath();
Expand Down Expand Up @@ -1016,14 +1042,13 @@ UsdGeomBBoxCache::_FindOrCreateEntriesForPrim(
entry->isIncluded = _ShouldIncludePrim(primContext.prim);

// Pre-populate all cache entries, note that some entries may already exist.
// Note also we do not exclude unloaded prims - we want them because they
// may have authored extentsHints we can use; thus we can have bboxes in
// model-hierarchy-only.
// Note also, with the default _primPredicate, we do not exclude unloaded
// prims - we want them because they may have authored extentsHints we can
// use; thus we can have bboxes in model-hierarchy-only.

TfHashSet<_PrimContext, _PrimContextHash> seenPrototypePrimContexts;

UsdPrimRange range(primContext.prim,
(UsdPrimIsActive && UsdPrimIsDefined && !UsdPrimIsAbstract));
UsdPrimRange range(primContext.prim, _primPredicate);
for (auto it = range.begin(); it != range.end(); ++it) {
_PrimContext cachePrimContext(
*it, primContext.instanceInheritablePurpose);
Expand Down Expand Up @@ -1312,8 +1337,7 @@ UsdGeomBBoxCache::_ResolvePrim(const _BBoxTask* task,
const bool primIsInstance = prim.IsInstance();
if (primIsInstance) {
const UsdPrim prototype = prim.GetPrototype();
children = prototype.GetFilteredChildren(
UsdPrimIsActive && UsdPrimIsDefined && !UsdPrimIsAbstract);
children = prototype.GetFilteredChildren(_primPredicate);
// Since we're using the prototype's children, we need to make sure
// we propagate this instance's inheritable purpose to the
// prototype's children so they inherit the correct purpose for this
Expand All @@ -1322,8 +1346,7 @@ UsdGeomBBoxCache::_ResolvePrim(const _BBoxTask* task,
entry->purposeInfo.GetInheritablePurpose();
}
else {
children = prim.GetFilteredChildren(
UsdPrimIsActive && UsdPrimIsDefined && !UsdPrimIsAbstract);
children = prim.GetFilteredChildren(_primPredicate);
// Otherwise for standard children that are not across an instance
// boundary, pass this prim's inheritable purpose along to its
// children.
Expand Down
11 changes: 11 additions & 0 deletions pxr/usd/usdGeom/bboxCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "pxr/usd/usdGeom/xformCache.h"
#include "pxr/usd/usdGeom/pointInstancer.h"
#include "pxr/usd/usd/attributeQuery.h"
#include "pxr/usd/usd/primFlags.h"
#include "pxr/base/gf/bbox3d.h"
#include "pxr/base/tf/hash.h"
#include "pxr/base/tf/hashmap.h"
Expand Down Expand Up @@ -91,6 +92,15 @@ class UsdGeomBBoxCache
UsdGeomBBoxCache(UsdTimeCode time, TfTokenVector includedPurposes,
bool useExtentsHint=false, bool ignoreVisibility=false);

/// Overload to pass in a custom prim traversal predicate.
///
/// This prim predicate will be used instead of the default when traversing
/// prims to compute extents.
USDGEOM_API
UsdGeomBBoxCache(UsdTimeCode time, TfTokenVector includedPurposes,
const Usd_PrimFlagsPredicate &predicate,
bool useExtentsHint=false, bool ignoreVisibility=false);

/// Copy constructor.
USDGEOM_API
UsdGeomBBoxCache(UsdGeomBBoxCache const &other);
Expand Down Expand Up @@ -557,6 +567,7 @@ class UsdGeomBBoxCache
TfTokenVector _includedPurposes;
UsdGeomXformCache _ctmCache;
_PrimBBoxHashMap _bboxCache;
Usd_PrimFlagsPredicate _primPredicate;
bool _useExtentsHint;
bool _ignoreVisibility;
};
Expand Down
14 changes: 10 additions & 4 deletions pxr/usd/usdGeom/pointInstancer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1196,10 +1196,16 @@ UsdGeomPointInstancer::_ComputeExtentFromTransforms(
std::vector<GfBBox3d> protoUntransformedBounds;
protoUntransformedBounds.reserve(protoPaths.size());

UsdGeomBBoxCache bboxCache(time,
/*purposes*/ {UsdGeomTokens->default_,
UsdGeomTokens->proxy,
UsdGeomTokens->render });
// Use a custom predicate for prototypes as they may not be defined
// since they can be under an over ancestor. This allows use to
// properly compute their bbox in this case.
UsdGeomBBoxCache bboxCache(time,
/*purposes*/ {UsdGeomTokens->default_,
UsdGeomTokens->proxy,
UsdGeomTokens->render},
/*predicate*/ (UsdPrimIsActive &&
UsdPrimHasDefiningSpecifier &&
!UsdPrimIsAbstract));
for (size_t protoId = 0 ; protoId < protoPaths.size() ; ++protoId) {
const SdfPath& protoPath = protoPaths[protoId];
const UsdPrim& protoPrim = stage->GetPrimAtPath(protoPath);
Expand Down
23 changes: 22 additions & 1 deletion pxr/usd/usdGeom/testenv/testUsdGeomPointInstancer.py
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,27 @@ def test_InstancerInVis(self):
instancer.InvisId(1, Usd.TimeCode.Default())
self.assertEqual(stage.GetRootLayer().GetAttributeAtPath(
'/Instance.invisibleIds').default, [1])


def test_ExtentWithOverPrototypes(self):
stage = Usd.Stage.Open('instancerOverProtos.usda')
instancer = UsdGeom.PointInstancer.Get(stage, '/Instancer')
expectedExtent = [
Gf.Vec3f(-2.5, -2.5, -2.5),
Gf.Vec3f(2.5, 2.5, 2.5),
]
self._ValidateExtent(instancer, expectedExtent)

expectedRange = [
Gf.Vec3d(-2.5, -2.5, -2.5),
Gf.Vec3d(2.5, 2.5, 2.5),
]
bboxCache = UsdGeom.BBoxCache(
Usd.TimeCode.Default(), includedPurposes=[UsdGeom.Tokens.default_])
bbox = bboxCache.ComputeUntransformedBound(instancer.GetPrim())
range = bbox.ComputeAlignedRange()
self.assertTrue(Gf.IsClose(range.GetMin(), expectedRange[0], 1e-6))
self.assertTrue(Gf.IsClose(range.GetMax(), expectedRange[1], 1e-6))


if __name__ == '__main__':
unittest.main()
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#usda 1.0
(
defaultPrim = "Instancer"
upAxis = "Z"
)

def PointInstancer "Instancer"
{
rel prototypes = [</Instancer/Prototypes/cube>]
int[] protoIndices = [0, 0, 0, 0]
point3f[] positions = [(2, 2, 2), (2, -2, -2), (-2, 2, 2), (-2, -2, -2)]

over "Prototypes"
{
def "cube" (
prepend references = @./CubeModel.usda@
)
{
}
}
}

4 changes: 4 additions & 0 deletions pxr/usd/usdGeom/wrapBBoxCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ void wrapUsdGeomBBoxCache()
init<UsdTimeCode, TfTokenVector, optional<bool, bool> >(
(arg("time"), arg("includedPurposes"),
arg("useExtentsHint"), arg("ignoreVisibility"))))
.def(init<UsdTimeCode, TfTokenVector, const Usd_PrimFlagsPredicate &,
optional<bool, bool>>(
(arg("time"), arg("includedPurposes"), arg("predicate"),
arg("useExtentsHint"), arg("ignoreVisibility"))))
.def("ComputeWorldBound", &BBoxCache::ComputeWorldBound, arg("prim"))
.def("ComputeWorldBoundWithOverrides",
&BBoxCache::ComputeWorldBoundWithOverrides,
Expand Down