Skip to content

Conversation

ld-kerley
Copy link
Contributor

@ld-kerley ld-kerley commented Aug 1, 2025

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)

…tion files under the "targets" folder.

Introduce heuristic to emitLibraryInclude() to re-interpret an "old" style data library file path as its equivalent "new" style location.
@krohmerNV
Copy link
Contributor

two questions:

  1. is there a reason we check the old location first and fallback to the new one? the other way around feels more natural
  2. have you tried this with the genmdl tests? I would like to get them in first.

@ld-kerley
Copy link
Contributor Author

There idea here is to check whatever location you're passed first, and then if we don't find it there, then assume that it was an old location (because we didn't find it), and try the new location.

In the case where we're provided the new location - we shouldn't need to check the second location, and we'll just find it directly.

My hope here was that those people with "old" style paths in their shader generators will still work, with the incurred cost of a second file check - which they can remove by updating the file path.

I haven't done any testing with MDL yet - my hope by posting this is that it would allow those who are able to test this with different integrations and on different platforms.

I'm happy to wait to merge this until the genmdl tests are merged etc - then we can just test via CI.

@msuzuki-nvidia
Copy link
Contributor

Thank you for this PR, it looks good to me. I agree that it would be better to merge this after the genmdl tests PR.

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