Skip to content

Commit

Permalink
Adjust Hydra model schemas to address a recent performance
Browse files Browse the repository at this point in the history
regression involving UsdImaging Native Instance Aggregation.

Aggregation uses Hydra model schemas as part of the aggregation
key.  Originally this only covered UsdGeomModelAPI, which holds
fields related to draw mode that must be considered when
determining instance aggregation.

Recently, to support other model-related capabiliites, I added
UsdModelAPI to this Hydra model schema.  The reasoning at the
time was to align with how the USD API schemas both use the same
"model:" namespace for their USD attributes.

However, the "modelPath" portion of this defeated aggregation
effectiveness, because otherwise-equivalent native instances
might have distinct modelPaths.

To address this, we first considered removing "modelPath",
which would fix the aggregation issue.  Clients wanting to
find the model path would simply need to traverse the scene
index to find the prim where the closest model schema
data source was introduced.

However, further discussion lead to considering that we should
keep the UsdModelAPI and UsdGeomModelAPI representations in Hydra
schemas distinct.  This change implements that, introducing a
GeomModel schema in Hydra.

(Internal change: 2303046)
  • Loading branch information
blevin authored and pixar-oss committed Nov 3, 2023
1 parent e337356 commit dcbc1ca
Show file tree
Hide file tree
Showing 14 changed files with 614 additions and 402 deletions.
1 change: 1 addition & 0 deletions pxr/usdImaging/usdImaging/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ pxr_library(usdImaging
drawModeSceneIndex
extentResolvingSceneIndex
extentsHintSchema
geomModelSchema
indexProxy
materialParamUtils
modelSchema
Expand Down
9 changes: 1 addition & 8 deletions pxr/usdImaging/usdImaging/dataSourcePrim.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "pxr/usdImaging/usdImaging/dataSourcePrimvars.h"
#include "pxr/usdImaging/usdImaging/dataSourceUsdPrimInfo.h"
#include "pxr/usdImaging/usdImaging/extentsHintSchema.h"
#include "pxr/usdImaging/usdImaging/geomModelSchema.h"
#include "pxr/usdImaging/usdImaging/modelSchema.h"
#include "pxr/usdImaging/usdImaging/tokens.h"
#include "pxr/usdImaging/usdImaging/usdPrimInfoSchema.h"
Expand Down Expand Up @@ -556,7 +557,6 @@ UsdImagingDataSourcePrim_ModelAPI::GetNames()
UsdImagingModelSchemaTokens->assetIdentifier,
UsdImagingModelSchemaTokens->assetName,
UsdImagingModelSchemaTokens->assetVersion,
UsdImagingModelSchemaTokens->applyDrawMode,
};
}

Expand Down Expand Up @@ -594,13 +594,6 @@ UsdImagingDataSourcePrim_ModelAPI::Get(const TfToken &name)
::New(assetVersion);
}
return nullptr;
} else if (name == UsdImagingModelSchemaTokens->applyDrawMode) {
// We set applyDrawMode for kind = component prims even if the
// prim is not a UsdGeomModelAPI.
if (_model.IsKind(KindTokens->component)) {
return HdRetainedTypedSampledDataSource<bool>::New(true);
}
return nullptr;
}
return nullptr;
}
Expand Down
18 changes: 9 additions & 9 deletions pxr/usdImaging/usdImaging/drawModeSceneIndex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
#include "pxr/usdImaging/usdImaging/drawModeSceneIndex.h"
#include "pxr/usdImaging/usdImaging/drawModeStandin.h"

#include "pxr/usdImaging/usdImaging/modelSchema.h"
#include "pxr/usdImaging/usdImaging/geomModelSchema.h"
#include "pxr/usdImaging/usdImaging/usdPrimInfoSchema.h"

#include "pxr/base/trace/trace.h"
Expand Down Expand Up @@ -64,18 +64,18 @@ _GetDrawMode(const HdSceneIndexPrim &prim)
return empty;
}

UsdImagingModelSchema modelSchema =
UsdImagingModelSchema::GetFromParent(prim.dataSource);
UsdImagingGeomModelSchema geomModelSchema =
UsdImagingGeomModelSchema::GetFromParent(prim.dataSource);

HdBoolDataSourceHandle const applySrc = modelSchema.GetApplyDrawMode();
HdBoolDataSourceHandle const applySrc = geomModelSchema.GetApplyDrawMode();
if (!applySrc) {
return empty;
}
if (!applySrc->GetTypedValue(0.0f)) {
return empty;
}

HdTokenDataSourceHandle const modeSrc = modelSchema.GetDrawMode();
HdTokenDataSourceHandle const modeSrc = geomModelSchema.GetDrawMode();
if (!modeSrc) {
return empty;
}
Expand Down Expand Up @@ -357,10 +357,10 @@ UsdImagingDrawModeSceneIndex::_PrimsDirtied(
std::set<SdfPath> paths;

static const HdDataSourceLocatorSet drawModeLocators{
UsdImagingModelSchema::GetDefaultLocator().Append(
UsdImagingModelSchemaTokens->drawMode),
UsdImagingModelSchema::GetDefaultLocator().Append(
UsdImagingModelSchemaTokens->applyDrawMode)};
UsdImagingGeomModelSchema::GetDefaultLocator().Append(
UsdImagingGeomModelSchemaTokens->drawMode),
UsdImagingGeomModelSchema::GetDefaultLocator().Append(
UsdImagingGeomModelSchemaTokens->applyDrawMode)};

for (const HdSceneIndexObserver::DirtiedPrimEntry &entry : entries) {
if (drawModeLocators.Intersects(entry.dirtyLocators)) {
Expand Down
2 changes: 1 addition & 1 deletion pxr/usdImaging/usdImaging/drawModeSceneIndex.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ TF_DECLARE_REF_PTRS(UsdImagingDrawModeSceneIndex);
/// A scene index replacing geometry based on the draw mode.
///
/// Inspects a prim's values for drawMode and applyDrawMode (see
/// UsdImagingModelSchema).
/// UsdImagingGeomModelSchema).
/// If the drawMode is valid and not the default and applyDrawMode is true,
/// the prim and all its descendents are replaced by stand-in geometry
/// specified by the draw mode.
Expand Down
58 changes: 29 additions & 29 deletions pxr/usdImaging/usdImaging/drawModeStandin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

#include "pxr/usdImaging/usdImaging/drawModeStandin.h"

#include "pxr/usdImaging/usdImaging/modelSchema.h"
#include "pxr/usdImaging/usdImaging/geomModelSchema.h"
#include "pxr/usdImaging/usdImaging/tokens.h"

#include "pxr/imaging/hd/basisCurvesSchema.h"
Expand Down Expand Up @@ -193,12 +193,12 @@ class _DisplayColorVec3fDataSource final : public HdVec3fDataSource
}

private:
_DisplayColorVec3fDataSource(const UsdImagingModelSchema schema)
_DisplayColorVec3fDataSource(const UsdImagingGeomModelSchema schema)
: _schema(schema)
{
}

UsdImagingModelSchema _schema;
UsdImagingGeomModelSchema _schema;
};

/// A vec4f wrapper around a HdVec3fDataSource, for use when a vec4f
Expand Down Expand Up @@ -332,7 +332,7 @@ class _PrimvarsDataSource : public HdContainerDataSource
///
return _PrimvarDataSource::New(
_DisplayColorVec3fDataSource::New(
UsdImagingModelSchema::GetFromParent(_primSource)),
UsdImagingGeomModelSchema::GetFromParent(_primSource)),
HdPrimvarSchemaTokens->constant,
HdPrimvarSchemaTokens->color);
}
Expand Down Expand Up @@ -656,8 +656,8 @@ class _BoundsStandin : public UsdImaging_DrawModeStandin

// Check whether model:drawModeColor is dirty.
static const HdDataSourceLocator colorLocator =
UsdImagingModelSchema::GetDefaultLocator().Append(
UsdImagingModelSchemaTokens->drawModeColor);
UsdImagingGeomModelSchema::GetDefaultLocator().Append(
UsdImagingGeomModelSchemaTokens->drawModeColor);
const bool dirtyColor =
dirtyLocators.Intersects(colorLocator);

Expand Down Expand Up @@ -868,8 +868,8 @@ class _OriginStandin : public UsdImaging_DrawModeStandin

// Check whether model:drawModeColor is dirty.
static const HdDataSourceLocator colorLocator =
UsdImagingModelSchema::GetDefaultLocator().Append(
UsdImagingModelSchemaTokens->drawModeColor);
UsdImagingGeomModelSchema::GetDefaultLocator().Append(
UsdImagingGeomModelSchemaTokens->drawModeColor);
const bool dirtyColor =
dirtyLocators.Intersects(colorLocator);

Expand Down Expand Up @@ -950,7 +950,7 @@ using _MaterialsDict = std::unordered_map<
/// It is providing a mesh with a material. The mesh consists of up to 6 quads.
/// Besides points, it has the vertex-varying cardsUv and face-varying
/// cardsTexAssgin - determining where to sample which of the up to 6 textures
/// that can be specified by the UsdImagingModelSchema.
/// that can be specified by the UsdImagingGeomModelSchema.
///
/// Details vary based on the card geometry which is box, cross, or fromTexture.
///
Expand Down Expand Up @@ -1007,7 +1007,7 @@ class _CardsDataCache
}

private:
/// A helper extracing values from UsdImagingModelSchema.
/// A helper extracing values from UsdImagingGeomModelSchema.
///
/// Note that the order of the six given textures is assumed to be:
/// XPos, YPos, ZPos, XNeg, YNeg, ZNeg.
Expand All @@ -1018,7 +1018,7 @@ class _CardsDataCache
/// So we do not support motion-blur for these attributes.
struct _SchemaValues
{
_SchemaValues(UsdImagingModelSchema schema);
_SchemaValues(UsdImagingGeomModelSchema schema);

/// Card geometry, that is box, cross, or fromTexture.
TfToken cardGeometry;
Expand Down Expand Up @@ -1081,7 +1081,7 @@ class _CardsDataCache
return cached;
}
auto data = std::make_shared<_CardsData>(
_SchemaValues(UsdImagingModelSchema::GetFromParent(_primSource)),
_SchemaValues(UsdImagingGeomModelSchema::GetFromParent(_primSource)),
_primPath
);

Expand Down Expand Up @@ -1199,7 +1199,7 @@ GetWorldToScreenFromImageMetadata(
return false;
}

_CardsDataCache::_SchemaValues::_SchemaValues(UsdImagingModelSchema schema)
_CardsDataCache::_SchemaValues::_SchemaValues(UsdImagingGeomModelSchema schema)
{
if (HdTokenDataSourceHandle src = schema.GetCardGeometry()) {
cardGeometry = src->GetTypedValue(0.0f);
Expand Down Expand Up @@ -2054,20 +2054,20 @@ class _CardsStandin : public UsdImaging_DrawModeStandin
// we send to the observer.

static const HdDataSourceLocatorSet cardLocators{
UsdImagingModelSchema::GetDefaultLocator().Append(
UsdImagingModelSchemaTokens->cardGeometry),
UsdImagingModelSchema::GetDefaultLocator().Append(
UsdImagingModelSchemaTokens->cardTextureXPos),
UsdImagingModelSchema::GetDefaultLocator().Append(
UsdImagingModelSchemaTokens->cardTextureYPos),
UsdImagingModelSchema::GetDefaultLocator().Append(
UsdImagingModelSchemaTokens->cardTextureZPos),
UsdImagingModelSchema::GetDefaultLocator().Append(
UsdImagingModelSchemaTokens->cardTextureXNeg),
UsdImagingModelSchema::GetDefaultLocator().Append(
UsdImagingModelSchemaTokens->cardTextureYNeg),
UsdImagingModelSchema::GetDefaultLocator().Append(
UsdImagingModelSchemaTokens->cardTextureZNeg) };
UsdImagingGeomModelSchema::GetDefaultLocator().Append(
UsdImagingGeomModelSchemaTokens->cardGeometry),
UsdImagingGeomModelSchema::GetDefaultLocator().Append(
UsdImagingGeomModelSchemaTokens->cardTextureXPos),
UsdImagingGeomModelSchema::GetDefaultLocator().Append(
UsdImagingGeomModelSchemaTokens->cardTextureYPos),
UsdImagingGeomModelSchema::GetDefaultLocator().Append(
UsdImagingGeomModelSchemaTokens->cardTextureZPos),
UsdImagingGeomModelSchema::GetDefaultLocator().Append(
UsdImagingGeomModelSchemaTokens->cardTextureXNeg),
UsdImagingGeomModelSchema::GetDefaultLocator().Append(
UsdImagingGeomModelSchemaTokens->cardTextureYNeg),
UsdImagingGeomModelSchema::GetDefaultLocator().Append(
UsdImagingGeomModelSchemaTokens->cardTextureZNeg) };

// Blast the entire thing.
if (dirtyLocators.Intersects(cardLocators)) {
Expand All @@ -2081,8 +2081,8 @@ class _CardsStandin : public UsdImaging_DrawModeStandin
}

static const HdDataSourceLocator colorLocator =
UsdImagingModelSchema::GetDefaultLocator().Append(
UsdImagingModelSchemaTokens->drawModeColor);
UsdImagingGeomModelSchema::GetDefaultLocator().Append(
UsdImagingGeomModelSchemaTokens->drawModeColor);
if (dirtyLocators.Intersects(colorLocator)) {
HdDataSourceLocatorSet primDirtyLocators = dirtyLocators;
static const HdDataSourceLocator displayColorValue =
Expand Down
4 changes: 2 additions & 2 deletions pxr/usdImaging/usdImaging/flattenedDataSourceProviders.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
#include "pxr/usdImaging/usdImaging/flattenedDataSourceProviders.h"

#include "pxr/usdImaging/usdImaging/flattenedModelDataSourceProvider.h"
#include "pxr/usdImaging/usdImaging/modelSchema.h"
#include "pxr/usdImaging/usdImaging/geomModelSchema.h"

#include "pxr/imaging/hd/flattenedDataSourceProviders.h"
#include "pxr/imaging/hd/overlayContainerDataSource.h"
Expand All @@ -40,7 +40,7 @@ UsdImagingFlattenedDataSourceProviders()
static HdContainerDataSourceHandle const result =
HdOverlayContainerDataSource::New(
HdRetainedContainerDataSource::New(
UsdImagingModelSchema::GetSchemaToken(),
UsdImagingGeomModelSchema::GetSchemaToken(),
Make<UsdImagingFlattenedModelDataSourceProvider>()),
HdFlattenedDataSourceProviders());
return result;
Expand Down
14 changes: 7 additions & 7 deletions pxr/usdImaging/usdImaging/flattenedModelDataSourceProvider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

#include "pxr/usdImaging/usdImaging/flattenedModelDataSourceProvider.h"

#include "pxr/usdImaging/usdImaging/modelSchema.h"
#include "pxr/usdImaging/usdImaging/geomModelSchema.h"

#include "pxr/usd/usdGeom/tokens.h"

Expand All @@ -35,7 +35,7 @@ bool
_ContainsDrawMode(const TfTokenVector &vec)
{
for (const TfToken &token : vec) {
if (token == UsdImagingModelSchemaTokens->drawMode) {
if (token == UsdImagingGeomModelSchemaTokens->drawMode) {
return true;
}
}
Expand All @@ -51,24 +51,24 @@ class _ModelDataSource : public HdContainerDataSource
TfTokenVector result = _primModel->GetNames();
if (!_ContainsDrawMode(result) &&
_ContainsDrawMode(_parentModel->GetNames())) {
result.push_back(UsdImagingModelSchemaTokens->drawMode);
result.push_back(UsdImagingGeomModelSchemaTokens->drawMode);
}

return result;
}

HdDataSourceBaseHandle Get(const TfToken &name) override {
if (name != UsdImagingModelSchemaTokens->drawMode) {
if (name != UsdImagingGeomModelSchemaTokens->drawMode) {
return _primModel->Get(name);
}
if (HdTokenDataSourceHandle ds =
UsdImagingModelSchema(_primModel).GetDrawMode()) {
UsdImagingGeomModelSchema(_primModel).GetDrawMode()) {
const TfToken drawMode = ds->GetTypedValue(0.0f);
if (!drawMode.IsEmpty() && drawMode != UsdGeomTokens->inherited) {
return ds;
}
}
return UsdImagingModelSchema(_parentModel).GetDrawMode();
return UsdImagingGeomModelSchema(_parentModel).GetDrawMode();
}

static
Expand Down Expand Up @@ -118,7 +118,7 @@ UsdImagingFlattenedModelDataSourceProvider::ComputeDirtyLocatorsForDescendants(
HdDataSourceLocatorSet * const locators) const
{
static const HdDataSourceLocator drawModeLocator(
UsdImagingModelSchemaTokens->drawMode);
UsdImagingGeomModelSchemaTokens->drawMode);
static const HdDataSourceLocatorSet drawModeLocatorSet{
drawModeLocator};

Expand Down
29 changes: 22 additions & 7 deletions pxr/usdImaging/usdImaging/geomModelAPIAdapter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@
#include "pxr/usdImaging/usdImaging/geomModelAPIAdapter.h"

#include "pxr/usdImaging/usdImaging/dataSourceSchemaBased.h"
#include "pxr/usdImaging/usdImaging/modelSchema.h"
#include "pxr/usdImaging/usdImaging/geomModelSchema.h"
#include "pxr/usdImaging/usdImaging/tokens.h"
#include "pxr/imaging/hd/extentSchema.h"
#include "pxr/imaging/hd/overlayContainerDataSource.h"
#include "pxr/imaging/hd/tokens.h"
#include "pxr/imaging/hd/retainedDataSource.h"
#include "pxr/usd/kind/registry.h"
#include "pxr/usd/usdGeom/modelAPI.h"
#include "pxr/usd/usd/modelAPI.h"

PXR_NAMESPACE_OPEN_SCOPE

Expand All @@ -54,7 +56,7 @@ struct _GeomModelTranslator
HdDataSourceLocator
GetContainerLocator()
{
return UsdImagingModelSchema::GetDefaultLocator();
return UsdImagingGeomModelSchema::GetDefaultLocator();
}
};

Expand Down Expand Up @@ -82,12 +84,25 @@ UsdImagingGeomModelAPIAdapter::GetImagingSubprimData(
}

if (subprim.IsEmpty()) {
UsdGeomModelAPI model(prim);
// Reflect UsdGeomModelAPI as UsdImagingGeomModelSchema.
HdContainerDataSourceHandle geomModelDs =
HdRetainedContainerDataSource::New(
UsdImagingGeomModelSchema::GetSchemaToken(),
_GeomModelDataSource::New(
prim.GetPath(), UsdGeomModelAPI(prim), stageGlobals));

// For model components, overlay applyDrawMode=true.
if (UsdModelAPI(prim).IsKind(KindTokens->component)) {
static HdContainerDataSourceHandle const applyDrawModeDs =
UsdImagingGeomModelSchema::Builder()
.SetApplyDrawMode(
HdRetainedTypedSampledDataSource<bool>::New(true))
.Build();
geomModelDs = HdOverlayContainerDataSource::
OverlayedContainerDataSources(applyDrawModeDs, geomModelDs);
}

return HdRetainedContainerDataSource::New(
UsdImagingModelSchema::GetSchemaToken(),
_GeomModelDataSource::New(
prim.GetPath(), model, stageGlobals));
return geomModelDs;
}

return nullptr;
Expand Down
Loading

0 comments on commit dcbc1ca

Please sign in to comment.