Update to MaterialX v1.38.3

[seando-adsk]

Description of Change(s)

• Use CMake config directly from MaterialX install
• Update for API and library changes in v1.38.3
• Strengthen NodeDef discovery

This is the last update that is not version tagged in the C++ code.
Future updates will be bracketed using MATERIALX_MINOR_VERSION defines introduced in v1.38.3.

Notes:

• This PR replaces Update to MaterialX v1.38.3 #1738 which should be closed.
• Krystian has left Autodesk and his PR was updated to fix the merge conflicts in dev branch.

• I have submitted a signed Contributor License Agreement

[jilliene]

Filed as internal issue #USD-7254

[pablode]

Hi!
I made some efforts to port this PR to MaterialX 1.38.4. You can find the diff here:
autodesk-forks/USD@adsk/update_to_materialx_1.38.3...pablode:mtlx-1.38.4

Commit 93aed43 contains a change which was presumably missed in the transition to 1.38.3.

[spiffmon]

If Autodesk's preference is to jump right to 1.38.4, that would be good to know ASAP, and we'd ask that this PR be updated with additional changes? (And others following along please advise if you think that would not be desirable)

[JGamache-autodesk]

Hi @spiffmon , I will update to 1.38.4 and generate a patch for @seando-adsk . The code @pablode wrote is very good, but there is a subtle bug in mx::loadLibraries when passing { "libraries" } that I prefer not triggering. I will try to complete this ASAP.

[seando-adsk]

@JGamache-autodesk Thanks for the patch Jerry. I have applied it and uploaded the commit to this PR.
@spiffmon The update to v1.38.4 should be ready now.

[spiffmon]

Can either of you say whether the PR will leave the code valid also for 1.38.3, as well? That may affect the speed with which we’re able to take it, as we *just* got 1.38.3 installed, and it may be awhile before our infra team gets 1.38.4 in.

On Tue, Apr 19, 2022 at 11:25 AM Sean Donnelly ***@***.***> wrote: @JGamache-autodesk <https://github.com/JGamache-autodesk> Thanks for the patch Jerry. I have applied it and uploaded the commit to this PR. @spiffmon <https://github.com/spiffmon> The update to v1.38.4 should be ready now. — Reply to this email directly, view it on GitHub <#1792 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABPOU2C33EJL6D3CXMJNFCDVF33A7ANCNFSM5PSH26UA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- --spiffiPhone

[JGamache-autodesk]

@spiffmon sorry, this was 1.38.4 only. I will generate a code patch to stay backward compatible to 1.38.3.

[seando-adsk]

@JGamache-autodesk / @spiffmon Support for v1.38.3 added back in (patch from Jerry).

[spiffmon]

Much obliged, gentlemen!

…

On Tue, Apr 19, 2022 at 1:25 PM Sean Donnelly ***@***.***> wrote: @JGamache-autodesk <https://github.com/JGamache-autodesk> / @spiffmon <https://github.com/spiffmon> Support for v1.38.3 added back in (patch from Jerry). — Reply to this email directly, view it on GitHub <#1792 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABPOU2FVRUMV3FBNK2PITGLVF4JCPANCNFSM5PSH26UA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[seando-adsk]

Is there a chance this can make it into 22.05?

[spiffmon]

Sorry, @seando-adsk , 22.05 is basically cooked and nearly out the door. We can commit to getting this into our (next) summer release, though.

[sunyab]

I am reviewing this change now. One question I had: is the change to use the CMake module out of the MaterialX install necessary for upgrading to 1.38.3, or was it a nice-to-have? I've noticed that this causes the build to fail when building against 1.38.3 when the Python module is not installed -- this looks like a bug that was fixed in 1.38.4.

[JGamache-autodesk]

Hi @sunyab, using the CMake generated by MaterialX:

• Fixes dynamic library issues on the Windows platform
• Allows using a custom prefix in the library names

These are changes that are very hard to retrofit into FindMaterialX.cmake. Also fixes the issue of all those slightly incompatible FindMaterialX.cmake found in various projects that can now all be deleted.

If you look in the original 1.38.3 commit, you will see that build_usd.py had workarounds for these issues. Feel free to bring them back if you want to allow using build_usd.py with that MaterialX version.

[marktucker]

I recently used this PR to build 22.05 against Mtls 1.38.4. I just wanted to point out that pxr/usdImaging/bin/usdBakeMtlx also needs modifications similar to other included in this PR to be able to build.

I also had to add back the MATERIALX_INCLUDE_DIR to get any of the Mtlx-consuming libraries to build. I'm not sure where in this PR the Mtlx include dir is supposed to be added to the compiler command line. And I had to leave the old FindMaterialX.cmake file in place, and revert to using MATERIALX_LIBRARIES to get linking to work. But I'm on Windows, and building USD in a very unique environment, not using build_usd.py. Basically I didn't end up using most (any?) of the CMakeLists changes. So these problems/comments may not be valid for anyone else.

[sunyab]

I'm continuing to test this change internally but ran into one issue that I'd love some help with.

usdImaging/usdImagingGL/CMakeLists.txt guards the MaterialX imaging tests with a check for MATERIALX_FOUND, which no longer exists with the removal of the CMake module. I don't think that check was really necessary, so could that please be removed?

Once that's removed, if you run the testUsdImagingGLMaterialX* tests, they will all claim to pass but the results for testUsdImagingGLMaterialX_usdPreviewSurface and testUsdImagingGLMaterialX_mxPreviewSurfaceVsNative_Gold are noticeably different. I've attached images below -- baseline on the left, result with these changes on the right. Are you able to reproduce this on your side? If so, I'd love some help understanding what's happening here and whether this is expected.

(missing highlight on the upper-left ball)

(brighter gold ball in the lower right)

[seando-adsk]

@marktucker You mention that this PR will require changes to pxr/usdImaging/bin/usdBakeMtlx to build. There are already changes in that folder. I have built 22.05 locally on Windows with these changes from this PR without any problems. We are using the build_usd.py script.

Sean

[marktucker]

My apologies, you are correct. I must have done something wrong when I brought your changes into my baseline. Please ignore that part of my comment. And as I said my build environment is... unique... so obviously feel free to ignore my comments about the CMake process. Which I guess means you can ignore everything I said. Sorry for the noise.

[JGamache-autodesk]

Hi @sunyab. Sorry for missing that MATERIALX_FOUND. Modern CMake stopped capitalizing library names, so I should have replaced it with MaterialX_FOUND.
As for the image differences.

• The smoother specular on the green sphere is expected since the default value for roughness was properly updated from 0.2 to 0.5 in 1.38.2 to match the UsdPreviewSurface standard.
• I can not directly explain the brighter prefiltered specular on the gold in the second scene. Might be due to changes in GGX computations between 1.38.1 and 1.38.4. I would recommend you try the recent FIS code though. It has improved a lot and stopped requiring an external texture for the albedo table. The code changes to enable this are minimal:

diff --git a/pxr/imaging/hdSt/materialXFilter.cpp b/pxr/imaging/hdSt/materialXFilter.cpp
index b0bf3c33f..54a9dc33e 100644
--- a/pxr/imaging/hdSt/materialXFilter.cpp
+++ b/pxr/imaging/hdSt/materialXFilter.cpp
@@ -79,7 +79,7 @@ _GenMaterialXShader(mx::GenContext & mxContext, mx::ElementPtr const& mxElem)

     // Use the domeLightPrefilter texture instead of sampling the Environment Map
     materialContext.getOptions().hwSpecularEnvironmentMethod =
-        mx::HwSpecularEnvironmentMethod::SPECULAR_ENVIRONMENT_PREFILTER;
+        mx::HwSpecularEnvironmentMethod::SPECULAR_ENVIRONMENT_FIS;

     return mx::createShader("Shader", materialContext, mxElem);
 }
diff --git a/pxr/imaging/hdSt/materialXShaderGen.cpp b/pxr/imaging/hdSt/materialXShaderGen.cpp
index 69f3abf8f..f0fc1e4a4 100644
--- a/pxr/imaging/hdSt/materialXShaderGen.cpp
+++ b/pxr/imaging/hdSt/materialXShaderGen.cpp
@@ -600,6 +600,8 @@ HdStMaterialXShaderGen::_EmitMxInitFunction(
         emitLine("u_envRadiance = HdGetSampler_domeLightPrefilter()", mxStage);
     }
     emitLine("u_envRadianceMips = textureQueryLevels(u_envRadiance)", mxStage);
+    emitLine("u_envRadianceSamples = 64", mxStage);
     emitLine("#endif", mxStage, false);
     emitLineBreak(mxStage);

This would generate a much improved specular experience for that test scene:

[seando-adsk]

@sunyab I added another patch from Jerry to Enable FIS lighting for MaterialX 1.38.3 and later. I had to merge in latest dev first.

[sunyab]

@JGamache-autodesk Thank you for the comments on those test cases, that was super helpful!

@seando-adsk Could I ask you to revert that FIS patch and file another PR for that so we can evaluate that separately?

Otherwise I think we're all set to land this PR once that last commit is reverted.

[seando-adsk]

@sunyab Sure I can revert the last commit. Question though, do you want the MaterialX_FOUND cmake change in this PR or with the new PR?

[sunyab]

@seando-adsk Ah please leave the MaterialX_FOUND change in this PR since it keeps tests running. Thanks!

[seando-adsk]

@sunyab Done, I've corrected the last commit to be just the cmake change.