Fix Metal shader errors with MaterialX 1.39

[brechtvl]

Description of Change(s)

mx_microfacet.glsl uses functions from mx_math.metal, so the latter must be included first.

Fixes Issue(s)

Checklist

• [x] I have created this PR based on the dev branch

• [x] I have followed the coding conventions

• [ ] I have added unit tests that exercise this functionality (Reference: testing guidelines)

• [ ] I have verified that all unit tests pass with the proposed changes

• [x] I have submitted a signed Contributor License Agreement (Reference: Contributor License Agreement instructions)

[brechtvl]

Here is an example USD file, producing the following errors:

Warning: in _ValidateCompilation at line 212 of usd/src/external_usd/pxr/imaging/hdSt/glslProgram.cpp -- Failed to compile shader (FRAGMENT_SHADER): program_source:2656:17:
    return vec3(mx_cos(phi) * sinTheta,
                ^
program_source:2657:17: error: use of undeclared identifier 'mx_sin'
                mx_sin(phi) * sinTheta,
                ^
program_source:2667:17: error: use of undeclared identifier 'mx_cos'
    return vec3(mx_cos(phi) * sinTheta,
                ^
program_source:2668:17: error: use of undeclared identifier 'mx_sin'
                mx_sin(phi) * sinTheta,
                ^
program_source:2785:12: error: no matching function for call to 'atan'
    return ::atan(y_over_x);
           ^~~~~~
program_source:153:3: note: candidate function template not viable: requires 2 arguments, but 1 was provided
T atan(T y, T x) { return atan2(y, x); }
  ^
program_source:2925:25: error: no member named 'get_num_mip_levels' in 'ProgramScope_Frag::MetalTexture'
    u_envRadianceMips = textureQueryLevels(u_envRadiance);
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
program_source:213:45: note: expanded from macro 'textureQueryLevels'
#define textureQueryLevels(texture) texture.get_num_mip_levels()
                                    ~~~~~~~ ^

I'm a bit confused about how this could have been working at all, as far as I can tell any scene using MaterialX has this problem.

[brechtvl]

CC @ld-kerley, since I guess you might want to be aware of this.

[dgovil]

I've asked Lee to take a look, but in the meantime these look good to me. Thanks for catching those, Brecht.

Do you know if these work backwards against 1.38 as well? Or do we need to guard accordingly?

@tcauchois this would likely need to get in for 25.5 if that is when you plan to switch to 1.39 by default.

[brechtvl]

I couldn't figure out if there are OpenUSD tests for MaterialX + Metal. So this is based on Blender tests.

• 1.38.8: Doesn't build with OpenUSD 25.02 anymore.
• 1.38.10: Seems to work with this PR. Some materials have pre-existing errors like:

program_source:2723:30: note: expanded from macro 'N4_file'
#define N4_file MetalTexture{HdGetSampler_N4_file(), samplerBind_N4_file}
                             ^
program_source:1036:7: note: 'HdGetScalar_N4_file' declared here
float HdGetScalar_N4_file() { return HdGet_N4_file(0).x; }
      ^
program_source:3603:21: error: no viable conversion from 'float' to 'texture2d<float>'
    mx_image_color4(N4_file, N4_layer, N4_default, N5_out, N4_uaddressmode, N4_vaddressmode, N4_filtertype, N4_framerange, N4_frameoffset, N4_frameendaction, N4_uv_scale, N4_uv_offset, N4_out);

• 1.39.2: No errors in the Blender tests.

[dgovil]

Awesome, thanks!

[jesschimein]

Filed as internal issue #USD-10652

(This is an automated message. See here for more information.)

[jesschimein]

/AzurePipelines run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[ld-kerley]

Firstly, I can confirm this PR appears to resolve the issues.

I was poking around at this from both the OpenUSD side and the MaterialX side and if we make some small changes on the MaterialX side then this PR can shrink significantly.

With the changes here, then this PR only needs to re-order the mx_math.metal include to come before the mx_microfacet.glsl include. Thats also something I think we should look at ways to fix on the MaterialX side too - but I don't think thats a trivial change.

[brechtvl]

Thanks! I'm happy to simplify or discard this PR if needed.

[jesschimein]

Hi! @ld-kerley @brechtvl would it be possible to update this PR to the simplified version mentioned above now that Lee's PR has been merged on the MaterialX side? 🙏 We'd like to be able to take this PR in order to upgrade to MatX 1.39.3. Any help would be much appreciated! cc: @klucknav

[brechtvl]

Sure, done.