Allow displaying of unloaded prims as bounding boxes #1145
Conversation
…aded prims with payloads as bounding boxes. Requires that the prim is of Model kind and has an extentsHint authored, or is a UsdGeomBoundable and has an extent authored. Built on the work from Animal Logic here: https://github.com/AnimalLogic/USD/tree/display_unloaded_prims And taking into account Pixar's recommendations here: PixarAnimationStudios#788 And also adding support for native and point instanced primitives.
|
I'm not sure how to create regression tests for this... Here is a sample file I've been using to test this feature. It contains straightforward payloads, payloads with native and point instancers, and payloads with nested point instancing and native instancing. There is one case that seems to not work properly, which is the nested native instancing. But that case doesn't seem to work even without any of these changes. Unloading the payloads causes the geometry to disappear (and with these changes, no bounding boxes are visible), but loading the payloads again does not make the geometry reappear (with this feature enabled or disabled). So I think that's an existing bug with instances/payloads/hydra? The other aspect of this commit not reflected in this sample file is a payload with a large extentsHint array, or where the payload is directly on a Boundable prim with an "extent" attribute lofted out of the payload. But I did run tests with both these cases to make sure that UsdImagingGLDrawModeAdapter::_ComputeExtent is working properly. |
|
Filed as internal issue #USD-5935 |
pxr/usdImaging/usdImaging/delegate.h
Outdated
| @@ -252,6 +252,19 @@ class UsdImagingDelegate : public HdSceneDelegate, public TfWeakBase { | |||
| USDIMAGING_API | |||
| void SetWindowPolicy(CameraUtilConformWindowPolicy policy); | |||
|
|
|||
| /// Sets display of unloaded prims as bounding boxes. | |||
| /// Unloaded prims will need to satisfay one of the following set of | |||
pxr/usdImaging/usdImaging/delegate.h
Outdated
| /// 1. The prim is a UsdGeomBoundable with an authored 'extent' attribute. | ||
| /// 2. The prim is a UsdPrim.IsModel() and has an authored 'extentsHint' | ||
| /// attribute (see UsdGeomModelAPI::GetExtentsHint). | ||
| /// Effective only for delegates that support draw modes. |
There was a problem hiding this comment.
could you add "(see GetUsdDrawModesEnabled())" ?
| UsdGeomBBoxCache bboxCache(UsdTimeCode::EarliestTime(), purposes, true); | ||
| return bboxCache.ComputeUntransformedBound(prim).ComputeAlignedBox(); | ||
|
|
||
| if (prim.IsLoaded()) { |
There was a problem hiding this comment.
I'm leaving this note partly for other reviewers, but can you add an XXX comment here saying that it seems problematic that we are using using EarliestTime() here?
Not something to change as part of this feature, but it definitely strikes me as a problem for animated models.
| extentsHint.size() == 2) { | ||
| extent = GfRange3d(extentsHint[0], extentsHint[1]); | ||
| } | ||
| else if ((attr = UsdGeomModelAPI(prim).GetExtentsHintAttr()) && |
There was a problem hiding this comment.
Oof... Can you leave an XXX comment that ideally we'd push the "compute unloaded" bit into UsdGeomBBoxCache, and therby not need to replicate all this logic?
There was a problem hiding this comment.
Yeah, I was pretty uncomfortable using the std::find over purposes in here, which is part of why I didn't want to push this into UsdGeomBBoxCache myself. But it was the only way I could think of to achieve generality in the face of the possibility of GetOrderedPurposeTokens changing in some way.
Come to think of it, the test for size() < 8 is an unnecessary hard coding of a value, so I'll change that too... Would you suggest completely removing the test for an upper bound on the size of extentsHint, or test that size() <= purposeTokens.size() * 2?
There was a problem hiding this comment.
Yeah, given the way we've defined extentsHint, and the iteration you're doing through purposeTokens, I don't think you need an upper bounds check at all.
future work, and removing the upper bound check on the size of the extentsHint array.
…raytracing-metal-2 Update acceleration structure implementation and shader code management
Provide an optional feature in the UsdImagingDelegate to display unloaded
prims with payloads as bounding boxes. Requires that the prim is of Model
kind and has an extentsHint authored, or is a UsdGeomBoundable and has
an extent authored.
Built on the work from Animal Logic here:
https://github.com/AnimalLogic/USD/tree/display_unloaded_prims
And taking into account Pixar's recommendations here:
#788
And also adding support for native and point instanced primitives.