LOOKDEVX-679 - Propagate MatX metadata info #2441

JGamache-autodesk · 2022-06-23T19:26:18Z

Add detection code for new USD features that might appear earlier than next Pixar release
Show shader parameters at the top of AE
Implement grouping in AE if metadata is available
Fix missing Ufe types from LOOKDEVX-670
Add underscore support to UI label prettification code

- Add detection code for new USD features that might appear earlier than next Pixar release - Show shader parameters at the top of AE - Implement grouping in AE if metadata is available - Fix missing Ufe types from LOOKDEVX-670 - Add underscore support to UI label prettification code

JGamache-autodesk · 2022-06-23T19:28:38Z

lib/mayaUsd/resources/ae/usdschemabase/ae_template.py


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

-def getPrettyName(name):


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

JGamache-autodesk · 2022-06-23T19:29:51Z

lib/mayaUsd/resources/ae/usdschemabase/ae_template.py

@@ -507,6 +496,46 @@ def sectionNameFromSchema(self, schemaTypeName):

        return schemaTypeName

+    def createShaderAttributesSection(self):


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

JGamache-autodesk · 2022-06-23T19:32:07Z

lib/mayaUsd/ufe/UsdAttribute.cpp

@@ -177,7 +177,11 @@ U getUsdAttributeVectorAsUfe(
 {
    VtValue vt;
    if (!attr.isValid() || !attr.hasValue()) {
-        vt = MayaUsd::ufe::vtValueFromString(attr.typeName(), attr.defaultValue());


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.

JGamache-autodesk · 2022-06-23T19:35:05Z

lib/mayaUsd/ufe/UsdAttribute.cpp

@@ -335,6 +347,12 @@ namespace ufe {
 UsdAttribute::UsdAttribute(const PXR_NS::UsdPrim& prim, const Ufe::AttributeDef::ConstPtr& attrDef)
    : fPrim(prim)
    , fAttrDef(attrDef)
+    , fUsdAttr(prim.GetAttribute(PXR_NS::UsdShadeUtils::GetFullName(


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.

JGamache-autodesk · 2022-06-23T19:35:43Z

lib/mayaUsd/ufe/UsdAttribute.cpp

@@ -532,14 +550,13 @@ Ufe::Value UsdAttribute::getMetadata(const std::string& key) const
                return Ufe::Value(ss.str());
            }
        }
-        return Ufe::Value();


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

JGamache-autodesk · 2022-06-23T19:42:36Z

lib/mayaUsd/ufe/UsdAttributes.cpp

@@ -181,10 +195,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) {                                            \


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.

JGamache-autodesk · 2022-06-23T19:43:23Z

lib/mayaUsd/ufe/UsdShaderAttributeDef.cpp

+namespace {
+typedef std::unordered_map<std::string, std::function<Ufe::Value(const PXR_NS::SdrShaderProperty&)>>
+                         MetadataMap;
+static const MetadataMap _metaMap = {


The metadata in USD is using Pixar's metadata nomenclature. This converts it back to MaterialX nomenclature.

JGamache-autodesk · 2022-06-23T19:44:27Z

lib/mayaUsd/ufe/Utils.cpp

@@ -600,6 +600,9 @@ Ufe::Attribute::Type usdTypeToUfe(const PXR_NS::SdrShaderPropertyConstPtr& shade
                { PXR_NS::SdrPropertyTypes->String, PXR_NS::SdfValueTypeNames->String },
                { PXR_NS::SdrPropertyTypes->Float, PXR_NS::SdfValueTypeNames->Float },
                { PXR_NS::SdrPropertyTypes->Color, PXR_NS::SdfValueTypeNames->Color3f },
+#if defined(USD_HAS_COLOR4_SDR_SUPPORT)
+                { PXR_NS::SdrPropertyTypes->Color4, PXR_NS::SdfValueTypeNames->Color4f },
+#endif


Without that patched USD with Color4 support, a MaterialX color4 will look and act as a Float4.

JGamache-autodesk · 2022-06-23T19:45:01Z

lib/mayaUsd/ufe/Utils.cpp

+            { Ufe::Attribute::kFloat3, PXR_NS::SdfValueTypeNames->Float3 },       // GfVec3f
+            { Ufe::Attribute::kDouble3, PXR_NS::SdfValueTypeNames->Double3 },     // GfVec3d
+            { Ufe::Attribute::kColorFloat3, PXR_NS::SdfValueTypeNames->Color3f }, // GfVec3f
+#if (UFE_PREVIEW_VERSION_NUM >= 4015)


Updated to support new Ufe types.

JGamache-autodesk · 2022-06-23T19:45:32Z

lib/mayaUsd/utils/util.cpp

@@ -722,6 +724,32 @@ std::string UsdMayaUtil::SanitizeName(const std::string& name)
    return TfStringReplace(name, ":", "_");
 }

+std::string UsdMayaUtil::prettifyName(const std::string& name)
+{


Moved from Python AE template code.

feldstj · 2022-06-23T19:36:39Z

lib/mayaUsd/resources/ae/usdschemabase/ae_template.py

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


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

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.

Did you show this to Natalia?

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.

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

feldstj · 2022-06-23T19:37:35Z

lib/mayaUsd/python/wrapUtil.cpp

@@ -50,5 +50,6 @@ void wrapUtil()
    scope s(class_<UsdMayaUtilScope>("Util"));

    def("IsAuthored", UsdMayaUtil::IsAuthored);
+    def("prettifyName", UsdMayaUtil::prettifyName);


(minor) - why is IsAuthored capitalized while prettifyName is lower case?

The coding standards requires CamelCase, I will update the code.

Jerry I believe the way you named yours "lowerCamelCase" is correct and how our coding standard is. The previous function "IsAuthored" came from old Pixar code which is why it has uppercase Is.

Yes. I rechecked and it is indeed lowerCamelCase.

feldstj · 2022-06-23T19:43:34Z

lib/mayaUsd/ufe/UsdAttribute.cpp

@@ -335,6 +347,12 @@ namespace ufe {
 UsdAttribute::UsdAttribute(const PXR_NS::UsdPrim& prim, const Ufe::AttributeDef::ConstPtr& attrDef)


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?

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.

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.

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.

feldstj · 2022-06-23T19:46:14Z

lib/mayaUsd/ufe/UsdAttributes.cpp

@@ -112,6 +112,11 @@ static PXR_NS::UsdAttribute _GetAttributeType(const PXR_NS::UsdPrim& prim, const

 Ufe::Attribute::Type UsdAttributes::attributeType(const std::string& name)
 {
+    // early return if name is empty.
+    if (name.empty()) {
+        throw std::invalid_argument("Empty name.");


I'm not really familiar with how MayaUsd exceptions are thrown. Is this enough information, or do you need to somehow include where this is coming from?

feldstj · 2022-06-23T19:49:47Z

lib/mayaUsd/ufe/Utils.cpp

+            { Ufe::Attribute::kDouble3, PXR_NS::SdfValueTypeNames->Double3 },     // GfVec3d
+            { Ufe::Attribute::kColorFloat3, PXR_NS::SdfValueTypeNames->Color3f }, // GfVec3f
+#if (UFE_PREVIEW_VERSION_NUM >= 4015)
+            { Ufe::Attribute::kFilename, PXR_NS::SdfValueTypeNames->Asset },


Should you have comments equivalent to those above that show the data type? ie. // GfVec3d

Are these comments adding value in this function?

Not sure, but I would think for consistency they should either be removed or added.

Also, isn't this a trivial change? If not, by all means ignore this.

ppt-adsk

Just question about error management going from return value to exception.

ppt-adsk · 2022-06-23T19:43:54Z

lib/mayaUsd/ufe/UsdAttributes.cpp

 }

 Ufe::Attribute::Ptr UsdAttributes::attribute(const std::string& name)
 {
    // early return if name is empty.
    if (name.empty()) {
-        return nullptr;


We've gone from indicating failure by returning an illegal value to throwing an exception. What motivated the change, and are all call sites capable of dealing with this change?

The main Python call site is in UFE itself. It is one that I can actually point to on this public forum since it is installed in Maya.
Look for /runTime/Python/Lib/site-packages/ufe/__init__.py
When passing an empty string, or a name that does not exist, the code in typedAttributeFactory() calls attrs.attributeType(attrName), receives Unknown then complains that this is not found in typedAttributeDict. I found that to be a bit too obscure, hence the exception. I would have preferred KeyError instead of ValueError on the Python side though.

I do have to agree that switching error handling on short notice is bug-prone though, so I would have no issue reverting that change.

Since this is code that has no backward compatibility constraints (i.e. it will only work in a future version of Maya), shouldn't we fix the UFE Python binding to more helpfully deal with an empty or unknown attribute name?

Reverted. I will submit a fix in UFE at a later time.

- Also make slider ranges default to 0-1 for shaders.

…ropagate_materialx_metadata

seando-adsk · 2022-06-28T20:04:06Z

lib/mayaUsd/resources/ae/usdschemabase/ae_template.py

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


Did you show this to Natalia?

…ropagate_materialx_metadata

JGamache-autodesk requested review from feldstj, seando-adsk and ppt-adsk June 23, 2022 19:26

JGamache-autodesk commented Jun 23, 2022

View reviewed changes

JGamache-autodesk assigned ecp-maya-devops-adsk Jun 23, 2022

feldstj requested changes Jun 23, 2022

View reviewed changes

ppt-adsk reviewed Jun 23, 2022

View reviewed changes

JGamache-autodesk added 3 commits June 24, 2022 12:59

Capitalize PrettifyName

26e0c57

- Also make slider ranges default to 0-1 for shaders.

Merge remote-tracking branch 'github/dev' into t_gamaj/LOOKDEVX-679/p…

54f9d3f

…ropagate_materialx_metadata

Fix bad merge from double #ifdef

0bba128

JGamache-autodesk assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Jun 24, 2022

JGamache-autodesk requested review from feldstj and ppt-adsk June 24, 2022 17:22

ppt-adsk previously approved these changes Jun 27, 2022

View reviewed changes

seando-adsk previously approved these changes Jun 28, 2022

View reviewed changes

JGamache-autodesk added 2 commits July 1, 2022 14:48

Merge remote-tracking branch 'github/dev' into t_gamaj/LOOKDEVX-679/p…

f452956

…ropagate_materialx_metadata

Fix unit tests

315af0e

JGamache-autodesk dismissed stale reviews from seando-adsk and ppt-adsk via 315af0e July 1, 2022 19:24

JGamache-autodesk assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Jul 1, 2022

Using scopes in a Python 2 compatible way

edeb68d

JGamache-autodesk assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Jul 2, 2022

Fix static method Python 2 bindings

b8f7104

JGamache-autodesk assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Jul 2, 2022

Roll-back Attributes error handling change

1e7e92d

JGamache-autodesk assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Jul 4, 2022

JGamache-autodesk requested review from ppt-adsk and seando-adsk July 4, 2022 16:06

JGamache-autodesk assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Jul 4, 2022

seando-adsk previously approved these changes Jul 5, 2022

View reviewed changes

Back to lowerCamelCase for prettifyName

e0e5410

JGamache-autodesk dismissed seando-adsk’s stale review via e0e5410 July 5, 2022 14:40

JGamache-autodesk assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Jul 5, 2022

ppt-adsk approved these changes Jul 5, 2022

View reviewed changes

seando-adsk approved these changes Jul 5, 2022

View reviewed changes

JGamache-autodesk added the ready-for-merge Development process is finished, PR is ready for merge label Jul 5, 2022

seando-adsk added the ufe-usd Related to UFE-USD plugin in Maya-Usd label Jul 5, 2022

seando-adsk merged commit e223433 into dev Jul 5, 2022

seando-adsk deleted the t_gamaj/LOOKDEVX-679/propagate_materialx_metadata branch July 5, 2022 15:33

		@@ -507,6 +496,46 @@ def sectionNameFromSchema(self, schemaTypeName):

		return schemaTypeName

		def createShaderAttributesSection(self):

		@@ -335,6 +347,12 @@ namespace ufe {
		UsdAttribute::UsdAttribute(const PXR_NS::UsdPrim& prim, const Ufe::AttributeDef::ConstPtr& attrDef)

LOOKDEVX-679 - Propagate MatX metadata info #2441

LOOKDEVX-679 - Propagate MatX metadata info #2441

Conversation

JGamache-autodesk commented Jun 23, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ppt-adsk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment