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

UDIMs uv_scale and uv_offset shader uniform parameters not exposed for tiledimage and UsdUVTexture nodes #1716

Open
JGamache-autodesk opened this issue Feb 16, 2024 · 0 comments · May be fixed by #1864

Comments

@JGamache-autodesk
Copy link
Contributor

JGamache-autodesk commented Feb 16, 2024

Looking at the current implementation of UDIMs there seems to be two choices:

1- Magic uv_scale and uv_offset ports appearing on top-level shader interface keyed to the image node name.
2- Global UDIM_SET_PROPERTY declaration affecting every single image in the MaterialX document.

Whenever an image node is hidden in a nodegraph, the uv_scale and uv_offset attributes do not propagate to the final shader interface, requiring option 2 to be used.

I would like to suggest expanding option 1, but this requires propagating scale/offset inputs thru parent nodegraph interfaces. Since I will be mentioning changes that will break the shader ABI this will be done only if explicitly requested by the DCC dev by setting a new ShaderGen option.

Let's take an arbitrary NodeGraph implementing the following NodeDef as an example:

  <nodedef name="ND_rustedimage_color3" node="rustedimage" nodegroup="texture2d">
    <input name="rustfile" type="filename" value="" uniform="true" />
    <input name="diffusefile" type="filename" value="" uniform="true" />
    <!-- Many other inputs... -->
    <output name="out" type="color3" default="0.0, 0.0, 0.0" />
  </nodedef>

The NodeGraph implementation of this definition will use two image nodes connected to the rustfile and diffusefile inputs. Could be other types of NodeGraph-implemented image nodes because the algo must be recursive to completely exit all the NodeGraph layers.

Let's say we are doing shadergen for a material using one rustedimage node called Rusty1.

We dig inside and end up at some point calling HwImageNode::addInputs() for the image nodes handling the rustfile.
1- The image ShaderNode being processed gets two new inputs, named file_uv_scale and file_uv_offset
2- We then look at the parent NodeGraph, find out that input rustfile is connected to the file input of the ShaderNode being processed, so we add rustfile_uv_scale connected to file_uv_scale and rustfile_uv_offset connected to file_uv_offset

Similarly we end up with diffusefile_uv_scale and diffusefile_uv_offset for the other image ShaderNode connected to the diffusefile filename input. Now you see why we use the name of the filename port as prefix.

When shadergen is done, I expect 4 new shader inputs to appear on the final shader ABI, matching the filename inputs. The shadergen will prefix these uniform parameters with the node name, so we end up with:

  • Rusty1_rustfile
  • Rusty1_rustfile_uv_scale
  • Rusty1_rustfile_uv_offset
  • Rusty1_diffusefile
  • Rusty1_diffusefile_uv_scale
  • Rusty1_diffusefile_uv_offset

So when we feed texture data to this shader, we can find the right uv_scale and uv_offset attributes for each texture file encountered by simply appending to the filename input we are currently resolving, and we can expect those scale and offset values to reach the mx_image function call in the shader code.

Does this make sense?

@JGamache-autodesk JGamache-autodesk changed the title UDIMs uv_scale and uv_offset shader uniform parameters not exposed for tiledimage and triplanar nodes UDIMs uv_scale and uv_offset shader uniform parameters not exposed for tiledimage and UsdUVTexture nodes Feb 16, 2024
JGamache-autodesk added a commit to autodesk-forks/MaterialX that referenced this issue May 27, 2024
Custom image nodes like ND_gltf_colorimage and ND_UsdUVTexture did not
produce uv_scale and uv_offset inputs. Fix that by propagating the added
inputs found inside a CompoundNode which represents a NodeGraph
implementation of a NodeDef.

Fixes AcademySoftwareFoundation#1716
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

Successfully merging a pull request may close this issue.

1 participant