Enable test suite rendering on Mac#2675
Enable test suite rendering on Mac#2675jstone-lucasfilm merged 15 commits intoAcademySoftwareFoundation:mainfrom
Conversation
…o main GraphEditor headless a bit more robust.
| TEST_RENDER_CONFIG="-DMATERIALX_TEST_RENDER=OFF" | ||
| if [ "${{ matrix.test_render }}" == "ON" ]; then | ||
| if [ "${{ runner.os }}" == "macOS" ]; then | ||
| TEST_RENDER_CONFIG="$TEST_RENDER_CONFIG -DMATERIALX_TEST_RENDER=ON -DMATERIALX_RENDER_MSL_ONLY=ON" |
There was a problem hiding this comment.
Key change: Turn on test rendering for MSL but not GLSL.
There was a problem hiding this comment.
Perhaps we can merge the existing EXTENDED_BUILD_CONFIG with your new TEST_RENDER_CONFIG, naming the combined variable ADDITIONAL_BUILD_CONFIG for generality?
That should help to keep the final cmake build call as compact and clear as possible, as it's becoming a bit hard to read!
| path: build/render/*.png | ||
|
|
||
| - name: Archive Resources (macOS) | ||
| if: matrix.test_render == 'ON' && runner.os == 'macOS' |
| endif() | ||
|
|
||
| # Validate MSL-only rendering option | ||
| if(MATERIALX_RENDER_MSL_ONLY) |
There was a problem hiding this comment.
Turn off GLSL rendering. For some reason there a renderGLSL module dependency for renderMSL. Not touching that dependency here.
| add_subdirectory(MaterialXRender) | ||
| target_link_libraries(MaterialXTest MaterialXRender) | ||
| if(MATERIALX_BUILD_GEN_GLSL) | ||
| if(MATERIALX_BUILD_GEN_GLSL AND NOT MATERIALX_RENDER_MSL_ONLY) |
There was a problem hiding this comment.
Turn off GLSL rendering if want MSL only
## Change - Turn on headless rendering on Mac for MaterialXView - Added to the MacOS_Xcode_16_Python313 workflow as that already does shader validation testing. Notes: - MaterialxGraphEditor rendering not enabled as it's using OpenGL which has issues running headless. (A[ new issue](#2672) has been logged to support Metal rendering here. If / when it is available then it can be turned on) - The Metal environment set up here is the same pre-requisite as for this [PR for full test suite enabling](#2675).
kwokcb
changed the title
ld-kerley
left a comment
ld-kerley
left a comment
There was a problem hiding this comment.
This looks like a great step forwards to me - the most testing we're able to do the better.
I hope we can even build on top of this to introduce regression testing for renders
| ls -la build/bin || true | ||
| exit 1 | ||
| fi | ||
| zip -r build/artifacts/resources.zip build/bin/resources |
There was a problem hiding this comment.
What is the motivation for manually generating a zip archive of the folder, rather than leveraging the built-in zip functionality in actions/upload-artifact?
Would it make sense to align this with our other upload steps (e.g. Upload Reference Shaders, Upload Renders) and use the existing system in actions/upload-artifact?
There was a problem hiding this comment.
Good catch. I had this "cherry picking" images at one point but reverted that as it's better to produce a "results" folder as part of test. Will revert this to the simpler rule.
kwokcb
left a comment
kwokcb
left a comment
There was a problem hiding this comment.
Review changes in.
Note: I will need to get some help to triage why the single case that renders the
the incorrect shader sporadically. 2 out of 3 runs were good.
| - name: CMake Generate | ||
| shell: bash | ||
| run: | | ||
| EXTENDED_BUILD_CONFIG="" |
There was a problem hiding this comment.
Small fix: This was not initialized so would error out if none of the "extended" conditions were set.
jstone-lucasfilm
left a comment
jstone-lucasfilm
left a comment
There was a problem hiding this comment.
This looks good to me, thanks @kwokcb!
jstone-lucasfilm
merged commit 9dc75c2
into
AcademySoftwareFoundation:main
3 participants
Change
Changes
MATERIALX_RENDER_MSL_ONLY) on render in paths supprtingMetal. This allows CI to run without also trying to run GLSL renders which are not supported.resourcesfolder results.Results
resources/Materials/TestSuite/pbrlib/surfaceshader/usd_normal_mapwhich for some reason is rendering the shader from the incorrect path. This occurred 1 out of 3 runs and left to be resolved separate from activation. One other test has a RMS diff of 0.00001Some other sample renderings (cannot attach full pdf results into github as file is too large)