Skip to content

Commit

Permalink
USD API schema strength order and composition changes:
Browse files Browse the repository at this point in the history 
1. Authored API schemas are no longer the strongest schemas and are now
   instead the weakest API schemas when a USD prim definition is
   constructed. Most importantly this means that an authored API schema
   can no longer override properties that are already declared from a
   prim's typed schema.
2. The "default" and "hidden" fields of properties can now be composed
   from a weaker API if a stronger schema with the property has no
   opinion for those fields.
3. API schema override properties are no longer rejected for a
   variability mismatch; the override's variability is just ignored.
   This is because we don't check attribute variability when determining
   if a default value from a weaker schema can be composed in, so it
   doesn't makes sense to require a variability match for API schema
   override properties.
4. The result from UsdPrim::GetAppliedSchemas will no longer contain
   duplicate schema names when there are authored API schemas that are
   the same as any of the prim's built-in API schemas.

A little background context on why we're only allowing "default" and
"hidden" to compose right now:
Our team discussed schema property definition composition and the
suggestion was made that we start limited in what fields we compose
since there are open questions about what to do with certain fields
(like the allowedTokens listOp which likely wants more than "stronegest
opinion wins" composition). So, the choice was to start with only the
fields we know we want now.

Allowing "default" was one of the main reasons we wanted schema property
composition in the first place. Default is also required for a
GenerativeProcedural prim with an authored applied
HydraGenerativeProceduralAPI to still work as it did previously after
making authored API schemas weaker than the prim type schema.

Allowing "hidden" came up because of a internal Pixar desire to hide the
"productName" property of the RenderProduct schema when our
internal renderman API schemas are auto-applied to it. The productName
value is expected to be generated at some point in the render submission
pipeline so being able to hide it in the schema via auto-apply is a
user experience win.

Fixes #2336

(Internal change: 2275114)
  • Loading branch information
@pixar-oss
pixar-oss committed May 15, 2023
1 parent 1d38703 commit 545aa62
Show file tree
Hide file tree
Showing 10 changed files with 1,238 additions and 821 deletions.
234 changes: 176 additions & 58 deletions pxr/usd/usd/primDefinition.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,29 +234,14 @@ bool UsdPrimDefinition::_PropertyTypesMatch(
return false;
}

// Compare variability
SdfVariability strongVariability, weakVariability;
strongLayerAndPath.HasField(SdfFieldKeys->Variability, &strongVariability);
weakLayerAndPath.HasField(SdfFieldKeys->Variability, &weakVariability);
if (weakVariability != strongVariability) {
TF_WARN("Property at path '%s' from stronger schema failed to override "
"property at path '%s' from weaker schema during schema prim "
"definition composition because their variability does not "
"match.",
strongLayerAndPath.path.GetText(),
weakLayerAndPath.path.GetText());
return false;
}

// Done comparing if it's not an attribute.
if (!specIsAttribute) {
return true;
}

// Compare the type name field of the attributes.
TfToken strongTypeName, weakTypeName;
strongLayerAndPath.HasField(SdfFieldKeys->TypeName, &strongTypeName);
weakLayerAndPath.HasField(SdfFieldKeys->TypeName, &weakTypeName);
const TfToken strongTypeName = strongLayerAndPath.GetTypeName();
const TfToken weakTypeName = weakLayerAndPath.GetTypeName();
if (weakTypeName != strongTypeName) {
TF_WARN("Attribute at path '%s' with type name '%s' from stronger "
"schema failed to override attribute at path '%s' with type "
Expand All @@ -273,34 +258,14 @@ bool UsdPrimDefinition::_PropertyTypesMatch(

void
UsdPrimDefinition::_ComposePropertiesFromPrimDef(
const UsdPrimDefinition &weakerPrimDef,
bool useWeakerPropertyForTypeConflict)
const UsdPrimDefinition &weakerPrimDef)
{
_properties.reserve(_properties.size() + weakerPrimDef._properties.size());

// Copy over property to path mappings from the weaker prim definition that
// aren't already in this prim definition.
// Copy over property to path mappings from the weaker prim definition,
// possibly creating composed definitions for properties that already exist.
for (const auto &it : weakerPrimDef._propLayerAndPathMap) {
// Note that the prop name may be empty as we use the empty path to
// map to the spec containing the prim level metadata. We need to
// make sure we don't add the empty name to properties list if
// we successfully insert a metadata mapping.
auto insertResult = _propLayerAndPathMap.insert(it);
if (insertResult.second){
if (!it.first.IsEmpty()) {
_properties.push_back(it.first);
}
} else {
// The property exists already. If we need to use the weaker
// property in the event of a property type conflict, then we
// check if the weaker property's type matches the existing, and
// replace the existing with the weaker property if the types
// do not match.
if (useWeakerPropertyForTypeConflict &&
!_PropertyTypesMatch(insertResult.first->second, it.second)) {
insertResult.first->second = it.second;
}
}
_AddOrComposeProperty(it.first, it.second);
}
}

Expand All @@ -311,19 +276,172 @@ UsdPrimDefinition::_ComposePropertiesFromPrimDefInstance(
{
_properties.reserve(_properties.size() + weakerPrimDef._properties.size());

// Copy over property to path mappings from the weaker prim definition that
// aren't already in this prim definition.
// Copy over property to path mappings from the weaker prim definition,
// possibly creating composed definitions for properties that already exist.
for (const auto &it : weakerPrimDef._propLayerAndPathMap) {
// Apply the prefix to each property name before adding it.
const TfToken instancedPropName =
UsdSchemaRegistry::MakeMultipleApplyNameInstance(
it.first, instanceName);
auto insertResult = _propLayerAndPathMap.emplace(
instancedPropName, it.second);
if (insertResult.second) {
_properties.push_back(instancedPropName);
_AddOrComposeProperty(instancedPropName, it.second);
}
}

void UsdPrimDefinition::_AddOrComposeProperty(
const TfToken &propName,
const _LayerAndPath &layerAndPath)
{
// Note that the prop name may be empty as we use the empty path to
// map to the spec containing the prim level metadata. We need to
// make sure we don't add the empty name to properties list if
// we successfully insert a metadata mapping.
auto insertResult = _propLayerAndPathMap.emplace(propName, layerAndPath);
if (insertResult.second){
if (!propName.IsEmpty()) {
_properties.push_back(propName);
}
} else {
// The property exists already. Some fields may be able to be
// composed in from the new weaker property definition so we try to
// do that here.
_LayerAndPath &existingProp = insertResult.first->second;
if (SdfPropertySpecHandle composedPropSpec =
_CreatedComposedPropertyIfNeeded(
propName, existingProp, layerAndPath)) {
// If a composed property was created, replace the existing
// property definition. Otherwise, we just leave the existing
// property as is.
existingProp = {
get_pointer(composedPropSpec->GetLayer()),
composedPropSpec->GetPath()};
}
}
}

// We limit which fields are allowed to be composed in from a property defined
// in a weaker prim definition when a prim definition already has a property
// with the same name.
static
const TfTokenVector &_GetAllowedComposeFromWeakerPropertyFields()
{
// Right now we only allow the "default" value (of attributes) and the
// "hidden" field to be composed from a weaker property. We may selectively
// expand this set of fields if it becomes necessary.
static const TfTokenVector fields = {
SdfFieldKeys->Default,
SdfFieldKeys->Hidden
};
return fields;
}

SdfPropertySpecHandle
UsdPrimDefinition::_FindOrCreatePropertySpecForComposition(
const TfToken &propName,
const _LayerAndPath &srcLayerAndPath)
{
// Arbitrary prim path for this definition's composed property specs. Only
// this prim definition will use the layer we find or create here so we
// don't need unique prim spec names/paths.
static const SdfPath primPath("/ComposedProperties");

SdfPropertySpecHandle destProp;

// If we have a composed layer, we can check if we've already created
// a spec for the composed property and return it if we have. Otherwise,
// we create a new layer for this prim definition to write its composed
// properties.
if (_composedPropertyLayer) {
if (destProp = _composedPropertyLayer->GetPropertyAtPath(
primPath.AppendProperty(propName))) {
return destProp;
}
} else {
_composedPropertyLayer = SdfLayer::CreateAnonymous(
"schema-composed-properties");
}

SdfChangeBlock block;

// Find or create the prim spec that will hold the composed property specs.
SdfPrimSpecHandle destPrim = _composedPropertyLayer->GetPrimAtPath(primPath);
if (!destPrim) {
destPrim = SdfPrimSpec::New(
_composedPropertyLayer, primPath.GetName(), SdfSpecifierDef);
}

// Create a copy of the source attribute or relationship spec. We do this
// manually as the copy utils for Sdf specs are more generalized than what
// we need here.
const SdfSpecType specType = srcLayerAndPath.GetSpecType();
if (specType == SdfSpecTypeAttribute) {
destProp = SdfAttributeSpec::New(
destPrim,
propName,
_composedPropertyLayer->GetSchema().FindOrCreateType(
srcLayerAndPath.GetTypeName()),
srcLayerAndPath.GetVariability());
} else if (specType == SdfSpecTypeRelationship) {
destProp = SdfRelationshipSpec::New(
destPrim,
propName,
srcLayerAndPath.GetVariability());
} else {
TF_CODING_ERROR("Cannot create a property spec from spec at layer "
"'%s' and path '%s'. The spec type is not an attribute or "
"relationship.",
srcLayerAndPath.layer->GetIdentifier().c_str(),
srcLayerAndPath.path.GetText());
return destProp;
}

// Copy all the metadata fields from the source spec to the new spec.
for (const TfToken &field : srcLayerAndPath.ListMetadataFields()) {
VtValue value;
srcLayerAndPath.HasField(field, &value);
destProp->SetField(field, value);
}

return destProp;
}

SdfPropertySpecHandle
UsdPrimDefinition::_CreatedComposedPropertyIfNeeded(
const TfToken &propName,
const _LayerAndPath &strongProp,
const _LayerAndPath &weakProp)
{
SdfPropertySpecHandle destProp;

// If the property types don't match, then we can't compose the properties
// together.
if (!_PropertyTypesMatch(strongProp, weakProp)) {
return destProp;
}

for (const TfToken &field : _GetAllowedComposeFromWeakerPropertyFields()) {
// If the stronger property already has the field, skip it.
if (strongProp.HasField<VtValue>(field, nullptr)) {
continue;
}

// Get the field's value from the weaker property. If it doesn't have
// the field, we skip it too.
VtValue weakValue;
if (!weakProp.HasField(field, &weakValue)) {
continue;
}

// If we get here we need to compose a property definition so create a
// a copy of the stronger property if we haven't already and add the
// field.
if (!destProp) {
destProp = _FindOrCreatePropertySpecForComposition(
propName, strongProp);
}
destProp->SetField(field, weakValue);
}

return destProp;
}

void
Expand Down Expand Up @@ -362,6 +480,15 @@ UsdPrimDefinition::_ComposeOverAndReplaceExistingProperty(
}
}

// There's one exception to override fields being stonger; an override
// cannot change the defined property's variability. So we may have to
// set the variability to match the defined property.
SdfVariability variability = defLayerAndPath->GetVariability();
if (overLayerAndPath.GetVariability() != variability) {
overLayer->SetField(
overLayerAndPath.path, SdfFieldKeys->Variability, variability);
}

// With the override spec composed, set the definition's path for the
// property to the composed override spec path.
*defLayerAndPath = std::move(overLayerAndPath);
Expand All @@ -371,8 +498,7 @@ bool
UsdPrimDefinition::_ComposeWeakerAPIPrimDefinition(
const UsdPrimDefinition &apiPrimDef,
const TfToken &instanceName,
_FamilyAndInstanceToVersionMap *alreadyAppliedSchemaFamilyVersions,
bool allowDupes)
_FamilyAndInstanceToVersionMap *alreadyAppliedSchemaFamilyVersions)
{
// Helper for appending the given schemas to our applied schemas list while
// checking for version conflicts. Returns true if the schemas are appended
Expand Down Expand Up @@ -415,15 +541,7 @@ UsdPrimDefinition::_ComposeWeakerAPIPrimDefinition(
// can undo the addition if we have to.
_appliedAPISchemas.push_back(std::move(apiSchemaName));
newlyAddedFamilies.push_back(std::move(familyAndInstance));
} else if (result.first->second == schemaInfo->version) {
// The family and instance were already added but the versions
// are the same. This is allowed. If we allow the API schema
// name to show up in the list more than once, we add it.
// Otherwise we safely skip it.
if (allowDupes) {
_appliedAPISchemas.push_back(std::move(apiSchemaName));
}
} else {
} else if (result.first->second != schemaInfo->version) {
// If we get here, the family and instance name were already
// added with a different version of the schema. This is a
// conflict and we will not add ANY of the schemas that are
Expand Down
40 changes: 33 additions & 7 deletions pxr/usd/usd/primDefinition.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,18 @@ class UsdPrimDefinition
return layer->HasFieldDictKey(path, fieldName, keyPath, value);
}

SdfVariability GetVariability() const {
SdfVariability variability;
HasField(SdfFieldKeys->Variability, &variability);
return variability;
}

TfToken GetTypeName() const {
TfToken typeName;
HasField(SdfFieldKeys->TypeName, &typeName);
return typeName;
}

USD_API
TfTokenVector ListMetadataFields() const;
};
Expand Down Expand Up @@ -325,6 +337,7 @@ class UsdPrimDefinition
}

UsdPrimDefinition() = default;
UsdPrimDefinition(const UsdPrimDefinition &) = default;

USD_API
void _IntializeForTypedSchema(
Expand Down Expand Up @@ -358,16 +371,26 @@ class UsdPrimDefinition
TfTokenVector _ListMetadataFields(const TfToken &propName) const;

// Helpers for constructing the prim definition.
USD_API
void _ComposePropertiesFromPrimDef(
const UsdPrimDefinition &weakerPrimDef,
bool useWeakerPropertyForTypeConflict = false);
const UsdPrimDefinition &weakerPrimDef);

USD_API
void _ComposePropertiesFromPrimDefInstance(
const UsdPrimDefinition &weakerPrimDef,
const std::string &instanceName);

void _AddOrComposeProperty(
const TfToken &propName,
const _LayerAndPath &layerAndPath);

SdfPropertySpecHandle _FindOrCreatePropertySpecForComposition(
const TfToken &propName,
const _LayerAndPath &srcLayerAndPath);

SdfPropertySpecHandle _CreatedComposedPropertyIfNeeded(
const TfToken &propName,
const _LayerAndPath &strongProp,
const _LayerAndPath &weakProp);

USD_API
void _ComposeOverAndReplaceExistingProperty(
const TfToken &propName,
Expand All @@ -381,10 +404,8 @@ class UsdPrimDefinition
bool _ComposeWeakerAPIPrimDefinition(
const UsdPrimDefinition &apiPrimDef,
const TfToken &instanceName,
_FamilyAndInstanceToVersionMap *alreadyAppliedSchemaFamilyVersions,
bool allowDupes = false);
_FamilyAndInstanceToVersionMap *alreadyAppliedSchemaFamilyVersions);

USD_API
static bool _PropertyTypesMatch(
const _LayerAndPath &strongerProp,
const _LayerAndPath &weakerProp);
Expand All @@ -401,6 +422,11 @@ class UsdPrimDefinition

// Cached list of property names.
TfTokenVector _properties;

// Layer that may be created for this prim definition if it's necessary to
// compose any new property specs for this definition from multiple
// property specs from other definitions.
SdfLayerRefPtr _composedPropertyLayer;
};

PXR_NAMESPACE_CLOSE_SCOPE
Expand Down
Loading

0 comments on commit 545aa62

Please sign in to comment.