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

Update USD to 23.05 and add MaterialX #238

Merged
merged 2 commits into from
Jun 12, 2023
Merged

Conversation

johnhaddon
Copy link
Member

No description provided.

Not without some bother :

- MaterialX hardcodes `CMAKE_CXX_STANDARD` to `11` in a way that can't be overridden on the command line. OIIO (which MaterialX depends on) requires 14 minimum, and VFXPlatform requires 17. This is taken care of in `cxxStandard.patch`.
- MaterialX lets you specify where it puts all its library files using `MATERIALX_INSTALL_STDLIB_PATH`, but then advertises the default location instead in `MaterialXConfig.cmake`, preventing downstream builds (USD) from configuring correctly. This is taken care of by `config.patch`.
- The various MaterialX 1.38.x versions are not patch versions - they break source compatibility. USD is still using the old API, so we're just not using the latest MaterialX yet.
@johnhaddon johnhaddon requested a review from ericmehl June 8, 2023 11:16
@johnhaddon johnhaddon self-assigned this Jun 8, 2023
@ericmehl
Copy link
Contributor

ericmehl commented Jun 9, 2023

I got it building on Windows with a bit of patching, so all good there. I see a number of additional directories in the resources folder from either MaterialX or the USD integration of it. Do we want to keep those in the archive we build?

Otherwise LGTM, and I can upload a Windows archive whenever you want to do a new release.

@boberfly
Copy link
Contributor

Maybe we could sneak in the cmake stuff into USD's manifests into this PR? :)
#240

@johnhaddon
Copy link
Member Author

I see a number of additional directories in the resources folder from either MaterialX or the USD integration of it. Do we want to keep those in the archive we build?

I've verified that we don't need them in order to visualise MaterialX shaders in usdview, and the README in that directory also implies that they're optional. I also don't want MaterialX stuff existing in a top-level directory without being labelled as such - resources is way too generic, and clashes with Gaffer's own resources dir. So I think omitting is best for now. Thanks for the review - I'll merge now.

@johnhaddon johnhaddon merged commit 120a383 into GafferHQ:main Jun 12, 2023
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.

3 participants