Relocate data library files #2512
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As proposed by Masuo in this slack post, the idea here is to restructure the data library files. Consolidating all of the target shader language implementation files under the
targets
folder. This will allow us, in a later PR, to move the MDL sources inside the data library.There are downstream integrators of MaterialX that rely on the locations of these files in custom shader generators. Specifically USD, and so we cannot just move the files without impacting those projects. Initially the idea was to introduce a new API call to allow the redirection of those file requests. It turns out that USD already correctly uses
emitLibraryInclude
to access the necessary shader source code files for its code generation.Instead of introducing a new API call, I'm proposing here that we just introduce a heuristic that remaps the old data library file location to the new one, and check this if the original file is not located. This allows USD to continue to access the files at the old style path, while allowing this refactor to happen.
As a future point once the downstream consumer no longer rely on the old style location, the heuristic should be removed. Perhaps at a major number breaking change release of MaterialX.
I did already test this with ToT USD and MaterialX on Mac - but it would be great to validate this on other platforms, and with other MaterialX integrations.
(The diff here is mostly just moving files around - the interesting parts of the code change are right at the bottom)