-
Notifications
You must be signed in to change notification settings - Fork 366
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
Create material from textures #1589
Create material from textures #1589
Conversation
Hey @kwokcb , I apologize for the delay, there were some issues with the old PR. First and foremost, thank you for your suggestions. They mean a lot to me. I've made several improvements to the code. Utilizing MaterialX modules like Initially, this function was built for standard surface shading models, but it's a great idea to consider extending it to support other shading models in later stages of development. I've also added a The same json configuration file can be further extended to support different DCCs and library conventions. It has been successfully tested in Substance Painter (feel free to test it :)) ). I'm planning to structure the json configuration file to incorporate the concept of inheritance and overrides, similar to houdini packages. This will enable us to override specific patterns by creating separate files as needed. Thank you! |
Hi @Cinifreak, No need to apologize -- I only did a quick look with some possible suggestions :). I will leave some thoughts (not work!) for other shader models since I'll probably forget about it later on :).
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great!
Some minor niggly items.
The only item I think would be nice to address is if the json
file cannot be found then the script will halt prematurely.
5149292
to
675acb3
Compare
@Cinifreak @Michaelredaa @kwokcb I like the idea behind this proposal, where a Python script would attempt to map a set of textures to a known shading model, and then construct a MaterialX document with its best guess at the material that corresponds to the author's original intent. I'm not sure that the approach in this PR is the one that I would recommend for achieving this goal, though, and I have a few thoughts on how it could be implemented in a more forward-looking way. The most important change that I would recommend is to make this script fully driven by the open definitions of shading models in the MaterialX Data Libraries, rather than leveraging a custom JSON file that duplicates this information in a new form. By directly using the MaterialX data libraries to determine which shading model is the closest match to the input textures, your script would work automatically with all shading models that MaterialX supports, including Autodesk Standard Surface, glTF PBR, UsdPreviewSurface, and even the upcoming OpenPBR shading model, without needing to extract aspects of each shading model and store it in a custom JSON file. With the MaterialX data libraries as a guide, I believe it should be possible to write very general-purpose string heuristics that find the closest match between a texture set and all available shading models, without needing to hardcode any details about the naming conventions of any specific shading model. If you have the bandwidth to take this on, I'd be happy to continue the conversation and provide further ideas on how to achieve this, but of course there's no pressure if you're too busy with other projects. |
@jstone-lucasfilm I think the idea of this proposal is what you recommended on Slack. If MaterialX data libraries have a solution to map between shader modules and texture set names patterns, it will be better to use it instead of the JSON mapping. So if you have any extra information about those libraries, I will be happy to know about them and use them. |
@Michaelredaa Just to set the historical context for this pull request, here's the original thread where @Cinifreak mentioned the idea: https://academysoftwarefdn.slack.com/archives/C0230LWBE2X/p1684944149082959 And here was my original recommendation in that thread:
I still believe this is the right approach, and I can provide a few thoughts on how this might be implemented in practice. As a starting point, I would recommend the By comparing the filenames in a texture set against the inputs of each shading model in the MaterialX data libraries, it ought to be possible to determine the likelihood that the texture set belongs to a specific shading model, and then to determine which texture ought to be assigned to each input that model. There are some subtleties to think through, for example you may find that common synonyms such as "albedo" and "diffuse" should be accommodated in the script, but otherwise it ought to work for all shading models that MaterialX supports, and should extend automatically to both custom studio models and future models such as OpenPBR. Let me know how this approach sounds to you! |
@jstone-lucasfilm SequenceMatcher seems like a smart way to get the likeness between 2 strings. I see your approach to making it dynamically auto-guess the texture type based on the shader module input names. I like it, but I have some concerns about it.
I made a quick script to iterate on inputs of mtlx standard surface, then get the max matched texture to easy to understand, we can ignore the UDIMs for now, and this the result:
What do you think about that? |
This looks very promising, @Michaelredaa! To reduce the impact of non-varying prefixes such as But otherwise, this looks like an approach that should provide good starting matches between textures and shading model inputs, with lots of possibilities for additional refinements over time. Further in the future, we should consider including additional shading models such as Unreal Default Lit and Disney Principled in MaterialX, since both of those models would be even closer matches than Autodesk Standard Surface for your example texture set. |
@jstone-lucasfilm I've implemented the initial logic and integrated it into the repository. Overall, it's functioning good for me with the exception of the
let me know your thoughts about this. |
@Michaelredaa Is this new work merged into the current pull request, or should I look at another location to find it? |
…nifreak/MaterialX into create-material-from-textures
This is really starting to come together, @Michaelredaa, and one key simplification that I would recommend is to move all of the Python code into a single module such as This will help us to refine the script and its helper functions as a self-contained unit, and we can consider moving to a helper library pattern further in the future, as we learn more about which functionality is most beneficial to share across scripts. For testing this new script, I would recommend the pattern we currently use for |
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
I've had a chance to make a few minor refinements to this proposal, and I believe it's looking very close to the bar we'd need to hit for merging. Here are the latest test results in GitHub Actions, where we're processing both
One shader input that the script still struggles with is "roughness", where it maps "chessboard_roughness.jpg" to There are a number of ways that we could potentially improve this, and one option that comes to mind is to define a "synonym" map in the script, where the isolated term "roughness" would be considered equivalent to "specular_roughness", following the common usage of "roughness" in computer graphics. More specific terms such as "sheen_roughness" should ideally be left alone, so that textures including this more precise string can correctly find the "sheen_roughness" input of each shading model. At the moment, I believe the matching logic would only see the word "roughness" in this specific term, and we may want to refine the logic so that the name "sheen_roughness" is fully preserved. Let me know what your thoughts are! |
@jstone-lucasfilm Thanks for the review and for the updates. |
@Michaelredaa Since we're aiming to include this feature in MaterialX 1.39, would you mind making this a pull request for the |
To avoid making new pull request, maybe this request for @Cinifreak because he is the pull request owner and he have the ability to change the pull request merge branch. |
@Cinifreak If you have a chance to retarget this pull request to |
@jstone-lucasfilm I tried to do that. but it seems i don't have permission to do that. |
@Cinifreak Feel free to create a new pull request to |
Thanks @Michaelredaa, I'll go ahead and close this, as it's been superseded by #1746. |
This PR recreated after closing #1572 as the previous PR had some problems with force pushing and CLA.