LOOKDEVX-2403 - Recognize NodeGraph EnumString attributes #3568
Conversation
| return retVal; | ||
| } | ||
|
|
||
| UsdAttributeHolder::EnumOptions UsdAttributeHolder::getEnums() const |
There was a problem hiding this comment.
Moved from UsdAttributes to prevent duplicating the added "enum" handling code.
| public: | ||
| UsdAttributeHolder(const PXR_NS::UsdAttribute& usdAttr); |
There was a problem hiding this comment.
Allow creating a quick transient holder to access enum information.
There was a problem hiding this comment.
Another alternative would be to make my suggested function be static and pass the UsdAttribute to it and be public. (See my comment about moving enum-building to a function)
| UFE_ATTRIBUTES_BASE::Enums result; | ||
| PXR_NS::SdrShaderPropertyConstPtr shaderProp = _GetSdrPropertyAndType(fItem, attrName).first; | ||
| if (shaderProp) { | ||
| for (const auto& option : shaderProp->GetOptions()) { |
There was a problem hiding this comment.
Moved to UsdShaderAttributeHolder where it belongs.
| result.emplace_back(option.first.GetString(), option.second.GetString()); | ||
| auto shaderPropAndType = _GetSdrPropertyAndType(fItem, attrName); | ||
| if (shaderPropAndType.first) { | ||
| const auto shaderPropertyHolder = UsdShaderAttributeHolder( |
There was a problem hiding this comment.
Use transient holders to get the info.
| if (portType == UsdShadeAttributeType::Input) { | ||
| const auto input = UsdShadeInput(usdAttr); | ||
| if (!input.GetSdrMetadataByKey(TfToken("enum")).empty()) { | ||
| return Ufe::Attribute::kEnumString; |
There was a problem hiding this comment.
Recognize enum ports on NodeGraph and Material prims.
| retVal.emplace_back(token.GetString(), ""); | ||
| } | ||
| } | ||
| // We might have a propagated enum copied into the created the NodeGraph port, resulting |
There was a problem hiding this comment.
(minor) Grammar. Guessing just remove the "the".
| enums = testAttrs.getEnums("inputs:enumString") | ||
| self.assertEqual(len(enums), 3) | ||
| self.assertEqual(len(enums[0]), 2) | ||
| self.assertEqual(enums[0][0], "foo") |
There was a problem hiding this comment.
Should you be testing bar and baz too?
There was a problem hiding this comment.
I am confident the other two labels are correct. There is also a complete check at line 986.
| } | ||
| } | ||
|
|
||
| const bool hasValues = allLabels.size() == allValues.size(); |
There was a problem hiding this comment.
I'd be more comfortable with parentheses to hekp the reader be sure of the order of evaluation of = vs ==.
| // We might have a propagated enum copied into the created the NodeGraph port, resulting | ||
| // from connecting a shader enum property. | ||
| PXR_NS::UsdShadeNodeGraph ngPrim(_usdAttr.GetPrim()); | ||
| if (ngPrim && UsdShadeInput::IsInput(_usdAttr)) { |
There was a problem hiding this comment.
The code would be slightly clearer if this was moved to a function with a illuminating name name, like retrieveEnumValueNames() or some such. If you're going to have another version for otehr reason, it would be nice to do this refactor.
JGamache-autodesk
added
the
ready-for-merge
|
@seando-adsk ready for merge. Preflight passed and subsequent commit is only modifying a comment. |
seando-adsk
deleted the
gamaj/LOOKDEVX-2403/recognize_enum_ports_on_node_graphs
branch
Also refactored the enum discovery to prevent code duplication.