Hardcoded libraries path in implementation files.

[ix-dcourtois]

Hi,

This commit 9cfdf37 removes a hard coded string from the C++ side of things (from what I understand of it) but the (big) problem is that in exchange it added it in all the file attributes of the implementation nodes... which completely broke our integration, because up until now we were deploying all the libraries into a <install_dir>/materialx/ folder, which was kinda nice/convenient (libraries was way too generic)

I can add deploy in <install_dir>/materialx/libraries/ to "fix" this, but it feels like it kinda goes against what the commit message says... now the root folder of all libraries of MaterialX have to be named libraries. At least in my case it feels like a step backward, where the library itself forces me to do something unnecessary.

Also, the other problem is that the path where the MaterialX libraries are located can be configured in the application's settings. So we have existing projects with the lib path already pointing to <install_dir>/materialx, which I will probably have to handle in a custom backward compatible check :/ But more importantly, what if a user decides to setup its libs in a folder where the root is not named libraries?

Is there a chance that the libraries part of the file attributes in the implementations could be reverted? Or is there a way to configure things so that we don't have to rename things and handle existing settings conversion?

Regards,
Damien.

[jstone-lucasfilm]

These are good points, @ix-dcourtois, and we're very open to generalizing this further for more flexible library integrations.

To my mind, it was important to remove the hardcoded libraries string in C++, since this hindered the integration of custom data libraries with their own folder structures. But you're correct that the standard data libraries now assume they're located within a libraries folder, and it would be ideal to remove this assumption.

In the short term, your proposal to deploy to <install_dir>/materialx/libraries/ would match the approach we're seeing with other teams, and it may indeed be the most direct solution.

Going forward, though, I think you're right that this should be generalized further, allowing packages to fully control the folder structure of their data libraries.

Ideas on this front are very welcome, and perhaps your own experiences can be used as a guide for what generalizations would be most useful.

[jstone-lucasfilm]

@ix-dcourtois One initial suggestion:

What if we added support for relative file references within MaterialX data libraries, and updated the standard data libraries to use only relative file references? This would remove all occurrences of the libraries string in the standard data libraries for MaterialX, replacing them with the appropriate relative file references.

[ix-dcourtois]

Hi,

I'm not a big fan of generalizations, especially when based on my (in)experiences :p And I'm not familiar enough with the codebase of MaterialX to give some precise ideas as to how to handle this, unfortunately.

I just have the feeling that changes required to give precedence to user-specified libraries/paths should have been a lot more contained. There is no mention to any issue or pull request in the commit, so I'm not sure what the exact problem that was solved by this commit is, but if you would be willing to share some details (e.g. a concrete use-case which was problematic on 1.38.3) I'd be interested in having a look.

But basically it feels like it should have been a "simple" (I know, nothing's really ever simple :p) matter of prepending user stuff (search paths and library folders) instead of appending like it was done on 1.38.3, without touching the include schemes of the whole library.

[jstone-lucasfilm]

@ix-dcourtois What would you think of my suggestion above, where we simply make all file references within the standard data libraries relative rather than absolute? That seems like a good step forward for all users of MaterialX, and it works equally well for standard and custom libraries, without imposing any specific folder hierarchy on applications or users.

[jstone-lucasfilm]

Providing a bit more detail on this proposal, we currently have support for relative XIncludes between MaterialX documents, so I believe we'd just need to add the following:

• Support for relative references from implementation elements to shader files.
• Support for relative references between shader files.

I think both of these features should be straightforward to implement, and they mirror existing functionality for referencing image files in MaterialX documents.

If there's time pressure for developers to make use of this feature, we could even consider pushing forward with a 1.38.5 release on an earlier timetable than usual.

[ix-dcourtois]

Hi,

Sorry I totally missed the second reply with the proposition to make paths relative! And yes, if it allows me to keep the same deployment structure as I'm using with 1.38.3, while preserving the added control mentioned in the commit, that would be perfect!
As for time, I had to revert back to 1.38.3 for now, so there is no rush. Thanks a lot for considering this change!

[jstone-lucasfilm]

I've written up a proposal for discussion as #926, and I'd be interested in your thoughts. Would this give your team the flexibility you'd need?

[ix-dcourtois]

Hi, I'm moving the discussion to the proposal, closing this one now! Thanks!