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

Refine MaterialX render module build configuration #1970

Conversation

ld-kerley
Copy link
Contributor

Add more refined control over building MaterialX render modules. Specifically USD only needs the core render module in some build configurations (iOS).

Also add a guard to ensure that MaterialXGenGlsl is built if MaterialXGenMsl is built, because of the dependency in the data library. USD uses MaterialX::GlslShaderGenerator::TARGET to locate those resources.

@jstone-lucasfilm
Copy link
Member

@ld-kerley I like the idea behind this proposal, though the split between MATERIALX_BUILD_RENDER and MATERIALX_BUILD_RENDER_CORE seems counterintuitive to me, as there's no corresponding module named MaterialXRenderCore.

As an alternative, what if we added a MATERIALX_BUILD_RENDER_PLATFORMS flag, defaulting to ON, with all platform-specific render modules gated by this flag?

This would allow USD developers to opt out of platform-specific render modules such as MaterialXRenderGlsl and MaterialXRenderOsl, without losing access to the MaterialXRender module that they still need.

…ifically USD only needs the core render module in some build configuration.

Also add a guard to ensure that MaterialXGenGlsl is built if MaterialXGenMsl is built, because of the dependency in the data library.
@ld-kerley ld-kerley force-pushed the refine_materialx_render_build branch from d58b75c to c5a4ace Compare December 20, 2024 00:05
@jstone-lucasfilm
Copy link
Member

Thanks for the updates, @ld-kerley, and this looks really promising! I'll take a closer look in upcoming days, and let's see if we can get this change ready to include in MaterialX v1.39.2.

Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Copy link
Member

@jstone-lucasfilm jstone-lucasfilm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, thanks @ld-kerley!

@jstone-lucasfilm jstone-lucasfilm merged commit 1d6f148 into AcademySoftwareFoundation:main Dec 20, 2024
34 checks passed
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 this pull request may close these issues.

2 participants