MaterialX icon indicating copy to clipboard operation
MaterialX copied to clipboard

color3 to vector3 type decay issue with MDL backend

Open pablode opened this issue 2 years ago • 2 comments

I'm having trouble using the UsdPreviewSurface node together with the UsdUVTexture node to achieve normal mapping with the MDL backend.

UsdPreviewSurface exposes the normal input (of type vector3) https://github.com/AcademySoftwareFoundation/MaterialX/blob/a255e89372e768b806c63104336dd5e67edf24fc/libraries/bxdf/usd_preview_surface.mtlx#L20-L22 which is then connected to the rgb output of the UsdUVTexture node (type color3). https://github.com/AcademySoftwareFoundation/MaterialX/blob/a255e89372e768b806c63104336dd5e67edf24fc/libraries/bxdf/usd_preview_surface.mtlx#L48-L49

This is not a problem with GLSL, as color3 is essentially a typedef for vec3: https://github.com/AcademySoftwareFoundation/MaterialX/blob/a255e89372e768b806c63104336dd5e67edf24fc/source/MaterialXGenGlsl/GlslSyntax.cpp#L213-L223

However, in the MDL backend, color3 is translated to the native color type https://github.com/AcademySoftwareFoundation/MaterialX/blob/a255e89372e768b806c63104336dd5e67edf24fc/source/MaterialXGenMdl/mdl/materialx/core.mdl#L47-L49 which has no implicit conversions defined (MDL spec 6.12.2): https://raytracing-docs.nvidia.com/mdl/specification/MDL_spec_1.7.2_17Jan2022.pdf.

This means the generated code results in a compilation error.

There are multiple ways to solve this. I think an elegant solution would be to parameterize the UsdUVTexture node with an output type similar to how it's done for the native MaterialX image node.

For reference, the UsdPreviewSurface specification defines the output type of the UsdUVTexture node as float3, which is then implicitly converted to normal3f and color3f: https://graphics.pixar.com/usd/release/spec_usdpreviewsurface.html#texture-reader.

EDIT: I should probably note that a warning (but no error) is emitted when using the GLSL backend, and the shader compiles fine. Perhaps the spec can be clarified as to whether this kind of implicit conversion is allowed or not, and the validation logic adjusted?

Here's a test file: usd_preview_surface_brass_tiled_normalmap.txt

From an authoring viewpoint, the issue can be easily resolved by inserting a convertN node between UsdPreviewSurface and UsdUVTexture, however, I'm not sure if that's something which should be required.

EDIT 2: Turns out that if there's no mechanism to prevent color3f-vector3f mismatches, artists will accidentally create graphs where this is the case (in the wild, f.i. the original Chess Set asset). These graphs then are incompatible with the MDL backend. I think it needs to be clearly defined in the spec whether this implicit conversion is allowed or not. In the case it's allowed, the MDL implementation needs to be fixed. If it's not allowed, compilation/validation should fail, and connecting an input to an output of mismatching type should not be possible in an interactive context (f.i. Houdini.)

pablode avatar Jul 31 '22 12:07 pablode

the USD Preview Surface spec says the outputs of the texture reader should be:

r - float
g - float
b - float
a - float
rgb - float3

which does not really match the MaterialX definition:

  <nodedef name="ND_UsdUVTexture" node="UsdUVTexture" nodegroup="texture2d" version="2.2" inherit="ND_UsdUVTexture_23">
    <output name="r" type="float" />
    <output name="g" type="float" />
    <output name="b" type="float" />
    <output name="a" type="float" />
    <output name="rgb" type="color3" />  
    <output name="rgba" type="color4" />
  </nodedef>

https://openusd.org/release/spec_usdpreviewsurface.html#texture-reader

krohmerNV avatar Mar 20 '23 14:03 krohmerNV

changing the type of rgb to from color3 to vector3 makes the MDL code gen produce correct results. Giving the USD spec, I would consider this to be a good solution.

edit: I also changed int IMP_UsdUVTexture_22 and IMP_UsdUVTexture_23

krohmerNV avatar Mar 20 '23 14:03 krohmerNV