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

LOOKDEVX-679 - Propagate MatX metadata info #2441

Merged
merged 10 commits into from
Jul 5, 2022
22 changes: 22 additions & 0 deletions cmake/modules/FindUSD.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,28 @@ if(USD_INCLUDE_DIR)
endif()
endif()

# See if MaterialX shaders with color4 inputs exist natively in Sdr:
# Not yet in a tagged USD version: https://github.com/PixarAnimationStudios/USD/pull/1894
set(USD_HAS_COLOR4_SDR_SUPPORT FALSE CACHE INTERNAL "USD.Sdr.PropertyTypes.Color4")
if (USD_INCLUDE_DIR AND EXISTS "${USD_INCLUDE_DIR}/pxr/usd/sdr/shaderProperty.h")
file(STRINGS ${USD_INCLUDE_DIR}/pxr/usd/sdr/shaderProperty.h USD_HAS_API REGEX "Color4")
if(USD_HAS_API)
set(USD_HAS_COLOR4_SDR_SUPPORT TRUE CACHE INTERNAL "USD.Sdr.PropertyTypes.Color4")
message(STATUS "USD has new Sdr.PropertyTypes.Color4")
endif()
endif()

# See if MaterialX shaders have full Metadata imported:
# Not yet in a tagged USD version: https://github.com/PixarAnimationStudios/USD/pull/1895
set(USD_HAS_MX_METADATA_SUPPORT FALSE CACHE INTERNAL "USD.MaterialX.Metadata")
if (USD_LIBRARY_DIR AND EXISTS "${USD_LIBRARY_DIR}/${USD_LIB_PREFIX}usdMtlx${CMAKE_SHARED_LIBRARY_SUFFIX}")
file(STRINGS ${USD_LIBRARY_DIR}/${USD_LIB_PREFIX}usdMtlx${CMAKE_SHARED_LIBRARY_SUFFIX} USD_HAS_API REGEX "uisoftmin")
if(USD_HAS_API)
set(USD_HAS_MX_METADATA_SUPPORT TRUE CACHE INTERNAL "USD.MaterialX.Metadata")
message(STATUS "USD has MaterialX metadata support")
endif()
endif()

message(STATUS "USD include dir: ${USD_INCLUDE_DIR}")
message(STATUS "USD library dir: ${USD_LIBRARY_DIR}")
message(STATUS "USD version: ${USD_VERSION}")
Expand Down
8 changes: 8 additions & 0 deletions lib/mayaUsd/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,14 @@ if (MAYA_HAS_GET_MEMBER_PATHS)
)
endif()

message(STATUS "USD_HAS_COLOR4_SDR_SUPPORT is ${USD_HAS_COLOR4_SDR_SUPPORT}")
if (USD_HAS_COLOR4_SDR_SUPPORT)
target_compile_definitions(${PROJECT_NAME}
PRIVATE
USD_HAS_COLOR4_SDR_SUPPORT=1
)
endif()

message(STATUS "MAYA_HAS_DISPLAY_LAYER_API is ${MAYA_HAS_DISPLAY_LAYER_API}")
if (MAYA_HAS_DISPLAY_LAYER_API)
target_compile_definitions(${PROJECT_NAME}
Expand Down
9 changes: 5 additions & 4 deletions lib/mayaUsd/python/wrapUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,9 @@ VtDictionary getDictionaryFromEncodedOptions(const std::string& textOptions)

void wrapUtil()
{
scope s(class_<UsdMayaUtilScope>("Util"));

def("IsAuthored", UsdMayaUtil::IsAuthored);
def("getDictionaryFromEncodedOptions", getDictionaryFromEncodedOptions);
scope s = class_<UsdMayaUtilScope>("Util", no_init)
.def("IsAuthored", UsdMayaUtil::IsAuthored)
.def("prettifyName", &UsdMayaUtil::prettifyName)
.staticmethod("prettifyName")
.def("getDictionaryFromEncodedOptions", getDictionaryFromEncodedOptions);
}
84 changes: 58 additions & 26 deletions lib/mayaUsd/resources/ae/usdschemabase/ae_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,31 +35,12 @@

# We manually import all the classes which have a 'GetSchemaAttributeNames'
# method so we have access to it and the 'pythonClass' method.
from pxr import Usd, UsdGeom, UsdLux, UsdRender, UsdRi, UsdShade, UsdSkel, UsdUI, UsdVol, Kind, Tf
from pxr import Usd, UsdGeom, UsdLux, UsdRender, UsdRi, UsdShade, UsdSkel, UsdUI, UsdVol, Kind, Tf, Sdr

nameTxt = 'nameTxt'
attrValueFld = 'attrValueFld'
attrTypeFld = 'attrTypeFld'

def getPrettyName(name):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved to C++ function UsdMayaUtil::prettifyName() exported to Python as mayaUsdLib.Util.prettifyName()

# Put a space in the name when preceded by a capital letter.
# Exceptions: Number followed by capital
# Multiple capital letters together
prettyName = str(name[0])
nbChars = len(name)
for i in range(1, nbChars):
if name[i].isupper() and not name[i-1].isdigit():
if (i < (nbChars-1)) and not name[i+1].isupper():
prettyName += ' '
prettyName += name[i]
elif name[i] == '_':
continue
else:
prettyName += name[i]

# Make each word start with an uppercase.
return prettyName.title()

# Helper class to push/pop the Attribute Editor Template. This makes
# sure that controls are aligned properly.
class AEUITemplate:
Expand Down Expand Up @@ -165,7 +146,7 @@ def onCreate(self, *args):
for k in allMetadata:
# All extra metadata is for display purposes only - it is not editable, but we
# allow keyboard focus so you copy the value.
mdLabel = getPrettyName(k) if self.useNiceName else k
mdLabel = mayaUsdLib.Util.prettifyName(k) if self.useNiceName else k
self.extraMetadata[k] = cmds.textFieldGrp(label=mdLabel, editable=False, enableKeyboardFocus=True)

# Update all metadata values.
Expand Down Expand Up @@ -262,7 +243,7 @@ def onCreate(self, *args):
typeNameStr = str(typeName.scalarType)
typeNameStr += ("[" + str(len(values)) + "]") if hasValue else "[]"

attrLabel = getPrettyName(self.attrName) if self.useNiceName else self.attrName
attrLabel = mayaUsdLib.Util.prettifyName(self.attrName) if self.useNiceName else self.attrName
singleWidgetWidth = mel.eval('global int $gAttributeEditorTemplateSingleWidgetWidth; $gAttributeEditorTemplateSingleWidgetWidth += 0')
with AEUITemplate():
# See comment in ConnectionsCustomControl below for why nc=5.
Expand Down Expand Up @@ -324,7 +305,15 @@ def __init__(self, ufeItem, prim, attrName, useNiceName):
def onCreate(self, *args):
frontPath = self.path.popSegment()
attr = self.prim.GetAttribute(self.attrName)
attrLabel = getPrettyName(self.attrName) if self.useNiceName else self.attrName
attrLabel = self.attrName
if self.useNiceName:
ufeItem = ufe.SceneItem(self.path)
ufeAttrS = ufe.Attributes.attributes(ufeItem)
ufeAttr = ufeAttrS.attribute(self.attrName)
attrLabel = str(ufeAttr.getMetadata("uiname"))

if not attrLabel:
attrLabel = mayaUsdLib.Util.prettifyName(self.attrName)
attrType = attr.GetMetadata('typeName')

singleWidgetWidth = mel.eval('global int $gAttributeEditorTemplateSingleWidgetWidth; $gAttributeEditorTemplateSingleWidgetWidth += 0')
Expand Down Expand Up @@ -497,7 +486,7 @@ def sectionNameFromSchema(self, schemaTypeName):
schemaTypeName = schemaTypeName.replace(p, r, 1)
break

schemaTypeName = getPrettyName(schemaTypeName)
schemaTypeName = mayaUsdLib.Util.prettifyName(schemaTypeName)

# if the schema name ends with "api", replace it with
# "API" and make sure to only replace the last occurence.
Expand All @@ -507,6 +496,46 @@ def sectionNameFromSchema(self, schemaTypeName):

return schemaTypeName

def createShaderAttributesSection(self):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Move all shader parameters to top of the AE. They used to reside in "Extra Attributes".

"""Use Sdr node information to populate the shader section"""
shader = UsdShade.Shader(self.prim)
nodeId = shader.GetIdAttr().Get()
if not nodeId:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should any form of error handling be done in these cases?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If we have the shader information we display a nice page. If the definition is missing (or running on an older Maya version without Ufe 4), the attributes end up in the "Extra Attributes" section as they did before.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you show this to Natalia?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. We found some issues:

  • Float sliders do not align perfectly with color sliders
  • Ordering of attributes in UsdPreviewSurface is alphabetical, which is wrong

One of these will need a fix in Maya, I will investigate the other one.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The default ordering of the UsdPreviewSurface attributes is alphabetical in USD. Nothing can be done in MayaUSD.

return
nodeDef = Sdr.Registry().GetShaderNodeByIdentifier(nodeId)
if not nodeDef:
return
label = nodeDef.GetLabel()
if not label:
label = nodeDef.GetFamily()
# Hide all outputs:
for name in nodeDef.GetOutputNames():
self.suppress(UsdShade.Utils.GetFullName(name, UsdShade.AttributeType.Output))
with ufeAeTemplate.Layout(self, "Shader: " + mayaUsdLib.Util.prettifyName(label)):
pages = nodeDef.GetPages()
if len(pages) == 1 and not pages[0]:
pages = []
if pages:
for page in pages:
collapse = False
pageLabel = page
if not page:
pageLabel = 'Unused Shader Attributes'
collapse = True
attrsToAdd = []
for name in nodeDef.GetPropertyNamesForPage(page):
if nodeDef.GetInput(name):
name = UsdShade.Utils.GetFullName(name, UsdShade.AttributeType.Input)
attrsToAdd.append(name)
if attrsToAdd:
with ufeAeTemplate.Layout(self, mayaUsdLib.Util.prettifyName(pageLabel), collapse):
self.addControls(attrsToAdd)
else:
attrsToAdd = []
for name in nodeDef.GetInputNames():
attrsToAdd.append(UsdShade.Utils.GetFullName(name, UsdShade.AttributeType.Input))
self.addControls(attrsToAdd)

def createTransformAttributesSection(self, sectionName, attrsToAdd):
# Get the xformOp order and add those attributes (in order)
# followed by the xformOp order attribute.
Expand Down Expand Up @@ -638,9 +667,12 @@ def buildUI(self):
sectionName = self.sectionNameFromSchema(schemaTypeName)
if schemaType.pythonClass:
attrsToAdd = schemaType.pythonClass.GetSchemaAttributeNames(False)

if schemaTypeName == 'UsdShadeShader' and hasattr(ufe, "NodeDef"):
# Shader attributes are special, but we requires Ufe knowledge of NodeDef to
# show unauthored attributes.
self.createShaderAttributesSection()
# We have a special case when building the Xformable section.
if schemaTypeName == 'UsdGeomXformable':
elif schemaTypeName == 'UsdGeomXformable':
self.createTransformAttributesSection(sectionName, attrsToAdd)
else:
sectionsToCollapse = ['Curves', 'Point Based', 'Geometric Prim', 'Boundable',
Expand Down
42 changes: 31 additions & 11 deletions lib/mayaUsd/ufe/UsdAttribute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,11 @@ U getUsdAttributeVectorAsUfe(
#else
!attr.hasValue()) {
#endif
vt = MayaUsd::ufe::vtValueFromString(attr.typeName(), attr.defaultValue());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do not crash if there is no default value. And do even report an error. These are problems coming from projects out of the control of Autodesk, so there is not much the end user can do about them.

if (!attr.defaultValue().empty()) {
vt = MayaUsd::ufe::vtValueFromString(attr.typeName(), attr.defaultValue());
} else {
return U();
}
} else if (!attr.get(vt, time) || !vt.IsHolding<T>()) {
return U();
}
Expand Down Expand Up @@ -223,7 +227,11 @@ U getUsdAttributeColorAsUfe(const MayaUsd::ufe::UsdAttribute& attr, const PXR_NS
#else
!attr.hasValue()) {
#endif
vt = MayaUsd::ufe::vtValueFromString(attr.typeName(), attr.defaultValue());
if (!attr.defaultValue().empty()) {
vt = MayaUsd::ufe::vtValueFromString(attr.typeName(), attr.defaultValue());
} else {
return U();
}
} else if (!attr.get(vt, time) || !vt.IsHolding<T>()) {
return U();
}
Expand Down Expand Up @@ -257,7 +265,11 @@ U getUsdAttributeMatrixAsUfe(
#else
!attr.hasValue()) {
#endif
vt = MayaUsd::ufe::vtValueFromString(attr.typeName(), attr.defaultValue());
if (!attr.defaultValue().empty()) {
vt = MayaUsd::ufe::vtValueFromString(attr.typeName(), attr.defaultValue());
} else {
return U();
}
} else if (!attr.get(vt, time) || !vt.IsHolding<T>()) {
return U();
}
Expand Down Expand Up @@ -366,6 +378,12 @@ namespace ufe {
UsdAttribute::UsdAttribute(const PXR_NS::UsdPrim& prim, const Ufe::AttributeDef::ConstPtr& attrDef)
Copy link
Collaborator

Choose a reason for hiding this comment

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

My recollection was that there were two UsdAttribute constructors, one that takes an attribute and the other that takes a prim and attribute definition. When the prim version is called there shouldn't be an attribute on the prim, no? In which case the GetAttribute(...) code below should fail. Is that not correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No. You can have both an Attribute and an AttributeDef. If that causes the Attribute-only CTOR to be called, then you lose all the rich information the AttributeDef provides.

I either had to add a 3rd CTOR for Attribute+AttributeDef, or simply leverage the fact that you don't even need the attribute to call the Prim+AttributeDef CTOR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So I had a discussion with Patrick a little while ago about changing the attribute constructor to also take an attribute definition as well (nullptr for the definition if there isn't one). We scheduled work for this with a Jira ticket, but it hasn't been implemented yet. If you wanted to you could make that change.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The more we delay making interface changes that support the code in a more natural, better encapsulated way, with proper separation of concerns, the more we make client code hard to understand, full of unnecessary concepts and dependencies.

: fPrim(prim)
, fAttrDef(attrDef)
, fUsdAttr(prim.GetAttribute(PXR_NS::UsdShadeUtils::GetFullName(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As was said in a previous review, the attribute can always be retrieved from the UsdPrim and the attribute name. Put that in application here to have one CTOR for Usd Attribute only, and another one for when we have an Attribute def. As was previously discussed, these two constructors should be in two classes, one derived from the other one, but that will not happen in this PR yet.

TfToken(attrDef->name()),
attrDef->ioType() == Ufe::AttributeDef::INPUT_ATTR ? PXR_NS::UsdShadeAttributeType::Input
: PXR_NS::UsdShadeAttributeType::Output

)))
{
PXR_NAMESPACE_USING_DIRECTIVE
TF_VERIFY(
Expand Down Expand Up @@ -583,16 +601,15 @@ Ufe::Value UsdAttribute::getMetadata(const std::string& key) const
return Ufe::Value(ss.str());
}
}
return Ufe::Value();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do not return immediately here. There might be information coming from the AttributeDef.

}
#ifdef UFE_V4_FEATURES_AVAILABLE
#if (UFE_PREVIEW_VERSION_NUM >= 4008)
} else if (fAttrDef && fAttrDef->hasMetadata(key)) {
if (fAttrDef && fAttrDef->hasMetadata(key)) {
return fAttrDef->getMetadata(key);
}
#endif
#endif
} else {
return Ufe::Value();
}
return Ufe::Value();
}

#ifdef UFE_V4_FEATURES_AVAILABLE
Expand Down Expand Up @@ -805,8 +822,11 @@ UsdAttributeFilename::create(const UsdSceneItem::Ptr& item, const PXR_NS::UsdAtt

std::string UsdAttributeFilename::get() const
{
if (!hasValue())
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated to use the same unauthored attribute logic as the other classes.

return std::string();

PXR_NS::VtValue vt;
if (fUsdAttr.Get(&vt, getCurrentTime(sceneItem())) && vt.IsHolding<SdfAssetPath>()) {
if (UsdAttribute::get(vt, getCurrentTime(sceneItem())) && vt.IsHolding<SdfAssetPath>()) {
SdfAssetPath path = vt.UncheckedGet<SdfAssetPath>();
return path.GetAssetPath();
}
Expand All @@ -826,8 +846,8 @@ Ufe::UndoableCommand::Ptr UsdAttributeFilename::setCmd(const std::string& value)
if (!TF_VERIFY(self, kErrorMsgInvalidType))
return nullptr;

std::string errMsg;
if (!MayaUsd::ufe::isAttributeEditAllowed(fUsdAttr, &errMsg)) {
const std::string errMsg = isEditAllowedMsg();
if (!errMsg.empty()) {
MGlobal::displayError(errMsg.c_str());
return nullptr;
}
Expand Down
25 changes: 14 additions & 11 deletions lib/mayaUsd/ufe/UsdAttributes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,11 @@ Ufe::Attribute::Type UsdAttributes::attributeType(const std::string& name)
#ifdef UFE_V4_FEATURES_AVAILABLE
#if (UFE_PREVIEW_VERSION_NUM >= 4008)
Ufe::NodeDef::Ptr nodeDef = UsdAttributes::nodeDef();
if (!nodeDef) {
return Ufe::Attribute::kInvalid;
}
Ufe::AttributeDef::ConstPtr attrDef = nameToAttrDef(tok, nodeDef);
if (attrDef) {
return attrDef->type();
if (nodeDef) {
Ufe::AttributeDef::ConstPtr attrDef = nameToAttrDef(tok, nodeDef);
if (attrDef) {
return attrDef->type();
}
}
#endif
#endif
Expand Down Expand Up @@ -167,16 +166,20 @@ Ufe::Attribute::Ptr UsdAttributes::attribute(const std::string& name)
}
#endif
#endif
bool canCreateAttribute = usdAttr.IsValid();
if (usdAttr.IsValid()) {
newAttrType = attributeType(name);
}
#ifdef UFE_V4_FEATURES_AVAILABLE
#if (UFE_PREVIEW_VERSION_NUM >= 4008)
else if (!nodeDef) {
return nullptr;
if (nodeDef) {
canCreateAttribute = true;
}
#endif
#endif
if (!canCreateAttribute) {
return nullptr;
}

// Use a map of constructors to reduce the number of string comparisons. Since the naming
// convention is extremely uniform, let's use a macro to simplify definition (and prevent
Expand All @@ -189,10 +192,10 @@ Ufe::Attribute::Ptr UsdAttributes::attribute(const std::string& name)
const PXR_NS::UsdPrim& prim, \
const Ufe::AttributeDef::ConstPtr& attrDef, \
const PXR_NS::UsdAttribute& usdAttr) { \
if (usdAttr) { \
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The code was assuming that an existing USD attribute would not require an AttributeDef. This is incorrect since there is metadata information we might want to get. Changed to select a CTOR based on the presence of an AttributeDef.

return UsdAttribute##TYPE::create(si, usdAttr); \
} else { \
if (attrDef) { \
return UsdAttribute##TYPE::create(si, prim, attrDef); \
} else { \
return UsdAttribute##TYPE::create(si, usdAttr); \
} \
} \
}
Expand Down
Loading