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

MaterialX: Two small issues with boolean inputs #1784

Closed
JGamache-autodesk opened this issue Feb 24, 2022 · 4 comments
Closed

MaterialX: Two small issues with boolean inputs #1784

JGamache-autodesk opened this issue Feb 24, 2022 · 4 comments

Comments

@JGamache-autodesk
Copy link
Contributor

Description of Issue

We found two issues with boolean inputs while developing MaterialX node definitions for use with USD

  • Default values are not copied to SdrNode inputs. Boolean is not a Sdr type, but is recognized as a Sdf one, so the fix is to add a check where the boolean type is handled by the parser:
            type = converted.valueTypeName.GetAsToken();
+            if (type == SdfValueTypeNames->Bool) {
+                defaultValue = UsdMtlxGetUsdValue(element, isOutput);
+            }

Steps to Reproduce

  1. Extract the attached MaterialX node definition file boolean_nodedef.zip in a location that will be read by MaterialX/USD (either in the MaterialX install, or in a location read via the PXR_MTLX_STDLIB_SEARCH_PATHS env var).
  2. Load the boolean_nodedef_defaultvalue.usda from these bundled scenes boolean_nodedef_scenes.zip and notice the capsule is green. It should be red because the default value of use1 is true.
  3. Load the boolean_nodedef_explicit.usda scene from the scene bundle and notice the capsule is red. It should be green since we explicitly set use1 to 0.

System Information (OS, Hardware)

Package Versions

Build Flags

@tallytalwar
Copy link
Contributor

Thanks @JGamache-autodesk for catching this.

For the first issue you found regarding Boolean not being a Sdr type and checking for SdfValueTypeNames->Bool could you do the following instead:

             type = converted.valueTypeName.GetAsToken();
+           // Do not use GetAsToken for comparison as recommended in the API
+           if (converted.valueTypeName == SdfValueTypeNames->Bool) {
+                defaultValue = UsdMtlxGetUsdValue(element, isOutput);
+                metadata.emplace(SdrPropertyMetadata->SdrUsdDefinitionType, 
+                          converted.valueTypeName.GetType().GetTypeName())
+           }

Without adding the sdrUsdDefinitionType metadata, a call to GetDefaultValueAsSdfType will incorrectly return the default of the Bool type, which is True.

Refer https:/PixarAnimationStudios/USD/blob/release/pxr/usd/sdr/shaderProperty.cpp#L536-L542 and https:/PixarAnimationStudios/USD/blob/release/pxr/usd/sdr/shaderProperty.cpp#L619 for more details.

A similar thing was recently done for UsdShadeShaderDef: 409b6e9

Please feel free to submit a PR with the fix and please include a test exhibiting this behavior.

Thanks

@jilliene
Copy link

Filed as internal issue #USD-7242

seando-adsk pushed a commit to autodesk-forks/USD that referenced this issue Feb 28, 2022
Fixes PixarAnimationStudios#1784
 - Remembers default values of boolean inputs in Sdr
 - Fixes swapped truth values in hdMtlx when creating inputs
@JGamache-autodesk
Copy link
Contributor Author

Hi @tallytalwar , thanks for the pre-review. The PR has been submitted.

@JGamache-autodesk
Copy link
Contributor Author

PR has been accepted. Closing issue. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants