-
Notifications
You must be signed in to change notification settings - Fork 202
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
Changes from all commits
7ad2e02
26e0c57
54f9d3f
0bba128
f452956
315af0e
edeb68d
b8f7104
1e7e92d
e0e5410
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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): | ||
# 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: | ||
|
@@ -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. | ||
|
@@ -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. | ||
|
@@ -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') | ||
|
@@ -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. | ||
|
@@ -507,6 +496,46 @@ def sectionNameFromSchema(self, schemaTypeName): | |
|
||
return schemaTypeName | ||
|
||
def createShaderAttributesSection(self): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should any form of error handling be done in these cases? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you show this to Natalia? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. We found some issues:
One of these will need a fix in Maya, I will investigate the other one. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
|
@@ -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', | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -191,7 +191,11 @@ U getUsdAttributeVectorAsUfe( | |
#else | ||
!attr.hasValue()) { | ||
#endif | ||
vt = MayaUsd::ufe::vtValueFromString(attr.typeName(), attr.defaultValue()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||
} | ||
|
@@ -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(); | ||
} | ||
|
@@ -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(); | ||
} | ||
|
@@ -366,6 +378,12 @@ namespace ufe { | |
UsdAttribute::UsdAttribute(const PXR_NS::UsdPrim& prim, const Ufe::AttributeDef::ConstPtr& attrDef) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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( | ||
|
@@ -583,16 +601,15 @@ Ufe::Value UsdAttribute::getMetadata(const std::string& key) const | |
return Ufe::Value(ss.str()); | ||
} | ||
} | ||
return Ufe::Value(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
@@ -805,8 +822,11 @@ UsdAttributeFilename::create(const UsdSceneItem::Ptr& item, const PXR_NS::UsdAtt | |
|
||
std::string UsdAttributeFilename::get() const | ||
{ | ||
if (!hasValue()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||
} | ||
|
@@ -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; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -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 | ||
|
@@ -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) { \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); \ | ||
} \ | ||
} \ | ||
} | ||
|
There was a problem hiding this comment.
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 asmayaUsdLib.Util.prettifyName()