-
-
Notifications
You must be signed in to change notification settings - Fork 35.5k
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
NormalMap Shader: fix Adreno work-around #17586
Conversation
Merging now so @sunag can make progress with his changes. |
Thanks! |
/ping @JordyvanDortmont If you would like to extensively test, that would be great. :-) |
Back in the day, we used this pattern to avoid mapN.xy *= ( float( gl_FrontFacing ) * 2.0 - 1.0 ); If that is still an issue, this PR could be rewritten as: bool frontFacing = dot( cross( S, T ), N ) > 0.0;
mapN.xy *= ( float( frontFacing ) * 2.0 - 1.0 ); |
Works correctly for several material examples and also on an Adreno GPU. Thanks for fixing the fix! I think preventing instruction divergence is still very relevant for GPU performance, so it's definitely an improvement if the branching is not the same for each "fragment". |
If that is still an issue, this PR could be rewritten as: bool frontFacing = dot( cross( S, T ), N ) > 0.0;
mapN.xy *= ( float( frontFacing ) * 2.0 - 1.0 ); That looks good to me. |
T *= -1.0; | ||
|
||
} | ||
if ( dot( cross( S, T ), N ) < 0.0 ) mapN.xy *= - 1.0; |
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.
You almost had it. This should have been:
if ( dot( cross( q0, q1 ), N ) < 0.0 ) mapN.xy *= - 1.0;
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.
But we no longer need the workaround: #21205
We need a test to replace
gl_FrontFacing
. I believe this change is correct.It appears to work correctly even if the geometry and/or UVs are mirrored -- that is, even if the winding order of the UVs is clockwise when the winding order of the vertices is counterclockwise.