Relocate data library files#2512
Relocate data library files#2512ld-kerley wants to merge 3 commits intoAcademySoftwareFoundation:mainfrom
Conversation
…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.
|
two questions:
|
|
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. |
|
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. |
|
I would recommend that we consider folding these shader source files into the MaterialX document. Raw glsl files on disk are flagged as attack vectors and can be misused. If they are the document we alteast have a checksum of the document. Also, there are performance concerns to think of due to small file IO during shader generation. |
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
targetsfolder. 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
emitLibraryIncludeto 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)