Skip to content
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

Add fileTextureVerticalFlip option for MDL generation #1418

Conversation

krohmerNV
Copy link
Contributor

When investigating the differences in the MaterialXTest I implemented the fileTextureVerticalFlip in the MDL shader gen.
In theory this option is not needed because the MaterialX and MDL define the image coordinate both in the lower left.
But it might be useful to align with existing but non standard conform renderers. It could for instance be used in the MaterialXTest to align with the flipped GLSL and OSL results.

WIP because I guess we want to address/discuss this in the context of the MaterialXTest fixes
and it will probably conflict with #1417

@krohmerNV krohmerNV force-pushed the PR-MDL-fileTextureVerticalFlip branch from f993ada to 79ccb9c Compare August 2, 2023 12:48
@krohmerNV krohmerNV changed the title WIP Add support for the fileTextureVerticalFlip option in the MDL generation Add support for the fileTextureVerticalFlip option in the MDL generation Oct 31, 2023
@krohmerNV
Copy link
Contributor Author

@jstone-lucasfilm, I believe the discussion was closed. We want to flip in the test renderer to align with GLSL and OSL. I added comments to explain why this flip is done and how this is to be handled in integrations.

@jstone-lucasfilm
Copy link
Member

This looks very promising, thanks @krohmerNV, and I'll plan to take a closer look soon!

Signed-off-by: Jonathan Stone <[email protected]>
Signed-off-by: Jonathan Stone <[email protected]>
@jstone-lucasfilm jstone-lucasfilm changed the title Add support for the fileTextureVerticalFlip option in the MDL generation Add fileTextureVerticalFlip option for MDL generation Oct 31, 2023
Copy link
Member

@jstone-lucasfilm jstone-lucasfilm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great to me, thanks @krohmerNV!

@jstone-lucasfilm jstone-lucasfilm merged commit 50062e5 into AcademySoftwareFoundation:main Oct 31, 2023
31 checks passed
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.

2 participants