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 metadata is not parsed into Sdr metadata #1874

Closed
JGamache-autodesk opened this issue May 20, 2022 · 4 comments
Closed

MaterialX metadata is not parsed into Sdr metadata #1874

JGamache-autodesk opened this issue May 20, 2022 · 4 comments

Comments

@JGamache-autodesk
Copy link
Contributor

Description of Issue

We would like to offer rich parameter editing in our DCC, which requires some metadata information to be present in Sdr.

Let's start with Metadata that has a direct Sdr counterpart:

  1. MaterialX inputs of type "string" that have "enum" metadata should submit the options via NdrOptionVec.
  2. "uiname" metadata maps directly to SdrPropertyMetadata->Label
  3. "uifolder" metadata maps directly to SdrPropertyMetadata->Page

This should allow us to split the standard_surface parameters into their proper pages, offer a dropdown for enum-like string inputs, and use nicer labels on these controls.

This leaves a long list of metadata with no Sdr counterpart: uimin, uimax, uistep, uisoftmin, uisoftmax, unit, unittype, defaultgeomprop, enumvalues, colorspace. We would like to transfer them as-is into the SdrProperty metadata dictionary so DCCs that can make use of them will find them without having to reparse MaterialX definitions.

Would you be OK with that?

Steps to Reproduce

  1. Build with MaterialX enabled
  2. Run this script:
>>> n = Sdr.Registry().GetShaderNodeByIdentifier("ND_image_color3")
>>> n.GetInput("uaddressmode").GetOptions()
[]  # Expected: [('constant', ''), ('clamp', ''), ('periodic', ''), ('mirror', '')]
>>> n = Sdr.Registry().GetShaderNodeByIdentifier("ND_standard_surface_surfaceshader")
>>> n.GetInput("base_color").GetLabel()
''  # Expected: 'Base Color'
>>> n.GetInput("base_color").GetPage()
'' # Expected: 'Base'
>>> n.GetInput("specular_IOR").GetMetadata()
{'widget': 'default'} # Expected: {'widget': 'default', 'uimin': '0.0', 'uisoftmax': '3.0', 'label': 'Index of Refraction', 'page': 'Specular'}

System Information (OS, Hardware)

Package Versions

Build Flags

Enable MaterialX.

@spiffmon
Copy link
Member

Hi @JGamache-autodesk - this mostly sounds great! Few things to discuss:

  1. I think that defaultgeomprop probably wants to be surfaced as primvars on the ShaderNode ? Or possibly as "primvar-naming properties"? This is important to Hydra and other clients that want to be able to discover necessary primvars so that it can restrict its capture/processing to just the needed ones, for large-scene scaling.
  2. Wouldn't enumvalues also be consumed to create Sdr Options?
  3. We should probably discuss colorspace... there seems to have been some thought that it should be canonical, but it was not finished.
  4. SdrShaderProperty makes a distinction between GetMetadata() (inherited from Ndr) and GetHints(), and you can specify different dictionaries for each when creating the Property. The intent is that Hints are the things DCC's would use to configure UI that go beyond the things explicitly called out in API (page, label, widget, etc). If you peak under the hood there are some very questionable calls on that we look only in the metadata dictionary for those three named hints, and personally I'd love to see that addressed, as they are hints. But bottom line, all the other fields you mention seem like GUI hints, and it'd be great to see them reflected as such, and have Maya using GetHints() Specifically with respect to units, who would consume that, when, and for what purpose?

Thanks, Jerry!

@JGamache-autodesk
Copy link
Contributor Author

Hi @spiffmon,

  1. These primvars are already discovered in ShaderBuilder::AddProperty and added as SdrNodeMetadata->Primvars. Nothing to do here.
>>> Sdr.Registry().GetShaderNodeByIdentifier("ND_triplanarprojection_float").GetPrimvars()
['Pobject', 'Nobject']
  1. Yes, enumvalues should be used to properly populate the NdrOptionVec. I will add a test case similar to:
>>> n = Sdr.Registry().GetShaderNodeByIdentifier("ND_lama_dielectric")
>>> n.GetInput("fresnelMode").GetOptions()
[]  # Expected: [('Scientific', '0'), ('Artistic', '1')]
  1. I see multiple discussions about units and colorspace and things are starting to converge. On the shader definition side the most important is probably knowing that the rotations for rotate2d, rotate3d, and place2d are in degrees as this defines a slider naturally. There are only four uses of colorspace metadata on property definitions in the MaterialX base library, and I am not sure they provide interesting UI hints.

  2. I did have a look at the RenderMan parser and saw how it converts all uncategorized metadata to hints. Looks right, so I will probably do the same.

For units I can see Maya using the degrees angle hint to build a slider. The distance metadata on most parameters is unitless, so not very useful for UI. The only one of interest is the thin film thickness being defined in nanometers, but I don't think we can do much more than adding some parameter documentation pop-up shown while hovering over the parameter.

That is for the Maya side of things. On the engineering side of things, a diamond patterned steel floor material will have a very strict definition of how many units of real world distance are covered per unit of texture space. Covering a factory floor correctly will require a scaling factor between ST coordinates and world space pixel positions. It is presently unclear to me how this will be handled and if this requires metadata awareness and UI hints.

@jilliene
Copy link

Filed as internal issue #USD-7380

@spiffmon
Copy link
Member

1 and 2 sound great, @JGamache-autodesk !

  1. I see multiple discussions about units and colorspace and things are starting to converge. On the shader definition side the most important is probably knowing that the rotations for rotate2d, rotate3d, and place2d are in degrees as this defines a slider naturally. There are only four uses of colorspace metadata on property definitions in the MaterialX base library, and I am not sure they provide interesting UI hints.
  • degrees : UsdGeom and UsdPhysics both document that all angles in USD are measured in degrees, though we didn't think to document that for UsdShade or Sdr, beyond the documented "rotation" of UsdTransform2d. Would it be a good idea to make a statement in UsdShade and/or Sdr?
  • colorspace , agreed I can't think of what value this provides as a hint. BTW we are aware that we still need to connect the dots from mtlx-translated colorspace metadata into Hydra-performed OCIO transformations.
  1. I did have a look at the RenderMan parser and saw how it converts all uncategorized metadata to hints. Looks right, so I will probably do the same.

For units I can see Maya using the degrees angle hint to build a slider. The distance metadata on most parameters is unitless, so not very useful for UI. The only one of interest is the thin film thickness being defined in nanometers, but I don't think we can do much more than adding some parameter documentation pop-up shown while hovering over the parameter.

Sdr Help metadata for the thickness property sounds right.

That is for the Maya side of things. On the engineering side of things, a diamond patterned steel floor material will have a very strict definition of how many units of real world distance are covered per unit of texture space. Covering a factory floor correctly will require a scaling factor between ST coordinates and world space pixel positions. It is presently unclear to me how this will be handled and if this requires metadata awareness and UI hints.

Yeah, so this is the fun part :-) As I've argued to @ashwin.bhat and @jstone-lucasfilm, from a USD perspective, it's preferred that any specification of units or scaling be encoded as a shader parameter rather than as metadata. But hierarchical scenegraphs (inherited scaling) and USD composition (referencing data encoded in different frames of reference, and sparse overrides) make this quite tricky.

The easy, wash-our-hands solution, I think, is something along the lines of always having (for materials sensitive to it) a textureCoordinate scaling node whose input is driven by a (constant) primvar that could be authored sparsely in the scenegraph (e.g. at referencing prims where we have applied a corrective transformation to bring referenced objects into the scale of the referencing scene, or could also be authored per primitive if/when needed. That way materials set up in assets could be "easily" adjusted in any consuming context. Possibly there's a system that could auto-compute these scale factors, but that seems possibly mysterious and difficult to debug when it goes wrong.

This quite likely merits a wider discussion... curious to hear what @jstone-lucasfilem and @meshula think.

seando-adsk pushed a commit to autodesk-forks/USD that referenced this issue Jan 17, 2023
Fixes PixarAnimationStudios#1874

- "enum" and "enumvalues" are parsed as options
- "doc" is parsed as help
- "uiname" is parsed as label
- "uifolder" is parsed as page
- all other MaterialX metadata is added as hints
seando-adsk pushed a commit to autodesk-forks/USD that referenced this issue Mar 1, 2024
Fixes PixarAnimationStudios#1874

- "enum" and "enumvalues" are parsed as options
- "doc" is parsed as help
- "uiname" is parsed as label
- "uifolder" is parsed as page
- all other MaterialX metadata is added as hints
This issue was closed.
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