-
-
Notifications
You must be signed in to change notification settings - Fork 21k
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
Fix refraction offset by manually unpacking normal mappings #48478
Fix refraction offset by manually unpacking normal mappings #48478
Conversation
scene/resources/material.cpp
Outdated
@@ -1032,6 +1032,8 @@ void BaseMaterial3D::_update_shader() { | |||
|
|||
if (features[FEATURE_REFRACTION]) { | |||
if (features[FEATURE_NORMAL_MAPPING]) { | |||
code += "\tNORMAL_MAP.xy = NORMAL_MAP.xy * 2.0 - 1.0;\n"; |
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 should be using a local variable such as normal_unpacked
like what Arnklit suggested in the issue. If you override the NORMAL_MAP
variable then the unpack will happen twice for actually calculating the normals of the object.
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.
Got it! Yea that actually makes sense... copy the normal mapping, unpack it, and feed it to the shader for when it needs to consider refraction. Leave the original normal mapping unedited for the remainder of the shader.
Something like
normal_unpacked = NORMAL_MAP;
normal_unpacked = ...;
...
code += "\tvec3 ref_normal = normalize( mix(NORMAL,TANGENT * normal_unpacked.x + BINORMAL * normal_unpacked.y + NORMAL * normal_unpacked.z,NORMAL_MAP_DEPTH) );\n";
If so I'll get that fixed and ammended to the commit/pr as soon as I get back home! Thanks!
…ffset rather than distorted.
264cd2c
to
31f2d94
Compare
Thanks! And congrats on your first merged Godot contribution 🎉 |
Cherry-picked for 3.4. |
Cherry-picked for 3.3.1. |
Implemented the suggested changes from @clayjohn in issue #44571. Changes were implemented to manually unpack the normal maps to enable refraction rather than just an offset when a normal mapping is provided.
Fixes: #44571