From 4643b192ab06cca1396bff80172009b6d027e4bf Mon Sep 17 00:00:00 2001 From: Cameron Billingham Date: Sun, 3 Nov 2024 11:27:40 -0800 Subject: [PATCH] Fix ComputeExtent for UsdGeomPointInstancer with prototypes under an over Fixes GH-3395. As described at https://openusd.org/docs/api/class_usd_geom_point_instancer.html#UsdGeomPointInstancer_protoProcessing PointInstancers can define prototypes under a parent prim specified as an over. The ComputeExtent plugin registered for PointInstancer was not correctly computing prototype bboxes in this case. This extends UsdGeomBBoxCache to allow for a custom prim predicate to be passed, which the updated ComputeExtent plugin takes advantage of to traverse prototype prims in this case. --- pxr/usd/usdGeom/bboxCache.cpp | 43 ++++++++++++++----- pxr/usd/usdGeom/bboxCache.h | 11 +++++ pxr/usd/usdGeom/pointInstancer.cpp | 14 ++++-- .../testenv/testUsdGeomPointInstancer.py | 23 +++++++++- .../instancerOverProtos.usda | 22 ++++++++++ pxr/usd/usdGeom/wrapBBoxCache.cpp | 4 ++ 6 files changed, 102 insertions(+), 15 deletions(-) create mode 100644 pxr/usd/usdGeom/testenv/testUsdGeomPointInstancer/instancerOverProtos.usda diff --git a/pxr/usd/usdGeom/bboxCache.cpp b/pxr/usd/usdGeom/bboxCache.cpp index a186738ab23..503d8d40f0c 100644 --- a/pxr/usd/usdGeom/bboxCache.cpp +++ b/pxr/usd/usdGeom/bboxCache.cpp @@ -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" @@ -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 // -------------------------------------------------------------------------- // @@ -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) { @@ -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) { @@ -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; @@ -439,7 +465,7 @@ UsdGeomBBoxCache::_ComputeBoundWithOverridesHelper( } GfBBox3d result; - UsdPrimRange range(prim); + UsdPrimRange range(prim, _primPredicate); for (auto it = range.begin(); it != range.end(); ++it) { const UsdPrim &p = *it; const SdfPath &primPath = p.GetPath(); @@ -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); @@ -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 @@ -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. diff --git a/pxr/usd/usdGeom/bboxCache.h b/pxr/usd/usdGeom/bboxCache.h index a3d0e7957f3..6897ee06c3b 100644 --- a/pxr/usd/usdGeom/bboxCache.h +++ b/pxr/usd/usdGeom/bboxCache.h @@ -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" @@ -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); @@ -557,6 +567,7 @@ class UsdGeomBBoxCache TfTokenVector _includedPurposes; UsdGeomXformCache _ctmCache; _PrimBBoxHashMap _bboxCache; + Usd_PrimFlagsPredicate _primPredicate; bool _useExtentsHint; bool _ignoreVisibility; }; diff --git a/pxr/usd/usdGeom/pointInstancer.cpp b/pxr/usd/usdGeom/pointInstancer.cpp index c3b6ac4e57e..093917bdb3e 100644 --- a/pxr/usd/usdGeom/pointInstancer.cpp +++ b/pxr/usd/usdGeom/pointInstancer.cpp @@ -1196,10 +1196,16 @@ UsdGeomPointInstancer::_ComputeExtentFromTransforms( std::vector 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); diff --git a/pxr/usd/usdGeom/testenv/testUsdGeomPointInstancer.py b/pxr/usd/usdGeom/testenv/testUsdGeomPointInstancer.py index 17050d17ffe..1cca3bdff3a 100644 --- a/pxr/usd/usdGeom/testenv/testUsdGeomPointInstancer.py +++ b/pxr/usd/usdGeom/testenv/testUsdGeomPointInstancer.py @@ -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() diff --git a/pxr/usd/usdGeom/testenv/testUsdGeomPointInstancer/instancerOverProtos.usda b/pxr/usd/usdGeom/testenv/testUsdGeomPointInstancer/instancerOverProtos.usda new file mode 100644 index 00000000000..ce7432a4aa0 --- /dev/null +++ b/pxr/usd/usdGeom/testenv/testUsdGeomPointInstancer/instancerOverProtos.usda @@ -0,0 +1,22 @@ +#usda 1.0 +( + defaultPrim = "Instancer" + upAxis = "Z" +) + +def PointInstancer "Instancer" +{ + rel prototypes = [] + 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@ + ) + { + } + } +} + diff --git a/pxr/usd/usdGeom/wrapBBoxCache.cpp b/pxr/usd/usdGeom/wrapBBoxCache.cpp index ac80a74f1ce..53debf13cd7 100644 --- a/pxr/usd/usdGeom/wrapBBoxCache.cpp +++ b/pxr/usd/usdGeom/wrapBBoxCache.cpp @@ -107,6 +107,10 @@ void wrapUsdGeomBBoxCache() init >( (arg("time"), arg("includedPurposes"), arg("useExtentsHint"), arg("ignoreVisibility")))) + .def(init>( + (arg("time"), arg("includedPurposes"), arg("predicate"), + arg("useExtentsHint"), arg("ignoreVisibility")))) .def("ComputeWorldBound", &BBoxCache::ComputeWorldBound, arg("prim")) .def("ComputeWorldBoundWithOverrides", &BBoxCache::ComputeWorldBoundWithOverrides,