-
-
Notifications
You must be signed in to change notification settings - Fork 3.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
Fix handling of double_sided
for normal maps
#10326
Conversation
double_sided
with normal mapsdouble_sided
in normal maps
double_sided
in normal mapsdouble_sided
for normal maps
Could this have any potential side effects with the mikktspace stuff right after? I don't really see where the issue would be but maybe I'm missing something |
Hmm... I honestly can't say I fully understand the Mikktspace tangent generation part, but all that the follow up code is doing is getting the existing tangents and normal and multiplying them by each component of the normal read from the map... In that case the (now) flipped normal from the map should match the flipped world normal to produce the (hopefully) correct results. It looked completely broken before, after this change it at least looks plausibly right. The one thing I'm not 100% sure is if we'd also need to flip |
This seems to have missed a few calls to
As a result, the |
# Objective - After #10326, examples `array_texture`, `ssao` and `shader_prepass` don't render correctly ``` error: failed to build a valid final module: Entry point fragment at Fragment is invalid ┌─ crates/bevy_pbr/src/render/pbr_prepass.wgsl:26:22 │ 26 │ let normal = evy_pbr::pbr_functions::31mapply_normal_mapping( │ ╭──────────────────────^ 27 │ │ bevy_pbr::pbr_bindings::material.flags, 28 │ │ world_normal, 29 │ │ · │ 36 │ │ 37 │ │ bevy_pbr::mesh_view_bindings::view.mip_bias, │ ╰───────────────────────────────────────────────────────────────────────────────────────^ invalid function call │ = Call to [9] is invalid = Requires 6 arguments, but 4 are provided ``` ## Solution - fix `apply_normal_mapping` calls
# Objective Right now, we flip the `world_normal` in response to `double_sided && !is_front`, however when calculating `N` from tangents and the normal map, we don't flip the normal read from the normal map, which produces extremely weird results. ## Solution - Pass `double_sided` and `is_front` flags to the `apply_normal_mapping()` function and use them to conditionally flip `Nt` ## Comparison Note: These are from a custom scene running with the `transmission` branch, (bevyengine#8015) I noticed lighting got pretty weird for the back side of translucent `double_sided` materials whenever I added a normal map. ### Before <img width="1392" alt="Screenshot 2023-10-31 at 01 26 06" src="https://github.com/bevyengine/bevy/assets/418473/d5f8c9c3-aca1-4c2f-854d-f0d0fd2fb19a"> ### After <img width="1392" alt="Screenshot 2023-10-31 at 01 25 42" src="https://github.com/bevyengine/bevy/assets/418473/fa0e1aa2-19ad-4c27-bb08-37299d97971c"> --- ## Changelog - Fixed a bug where `StandardMaterial::double_sided` would interact incorrectly with normal maps, producing broken results.
# Objective - After bevyengine#10326, examples `array_texture`, `ssao` and `shader_prepass` don't render correctly ``` error: failed to build a valid final module: Entry point fragment at Fragment is invalid ┌─ crates/bevy_pbr/src/render/pbr_prepass.wgsl:26:22 │ 26 │ let normal = evy_pbr::pbr_functions::31mapply_normal_mapping( │ ╭──────────────────────^ 27 │ │ bevy_pbr::pbr_bindings::material.flags, 28 │ │ world_normal, 29 │ │ · │ 36 │ │ 37 │ │ bevy_pbr::mesh_view_bindings::view.mip_bias, │ ╰───────────────────────────────────────────────────────────────────────────────────────^ invalid function call │ = Call to [9] is invalid = Requires 6 arguments, but 4 are provided ``` ## Solution - fix `apply_normal_mapping` calls
# Objective Right now, we flip the `world_normal` in response to `double_sided && !is_front`, however when calculating `N` from tangents and the normal map, we don't flip the normal read from the normal map, which produces extremely weird results. ## Solution - Pass `double_sided` and `is_front` flags to the `apply_normal_mapping()` function and use them to conditionally flip `Nt` ## Comparison Note: These are from a custom scene running with the `transmission` branch, (bevyengine#8015) I noticed lighting got pretty weird for the back side of translucent `double_sided` materials whenever I added a normal map. ### Before <img width="1392" alt="Screenshot 2023-10-31 at 01 26 06" src="https://github.com/bevyengine/bevy/assets/418473/d5f8c9c3-aca1-4c2f-854d-f0d0fd2fb19a"> ### After <img width="1392" alt="Screenshot 2023-10-31 at 01 25 42" src="https://github.com/bevyengine/bevy/assets/418473/fa0e1aa2-19ad-4c27-bb08-37299d97971c"> --- ## Changelog - Fixed a bug where `StandardMaterial::double_sided` would interact incorrectly with normal maps, producing broken results.
# Objective - After bevyengine#10326, examples `array_texture`, `ssao` and `shader_prepass` don't render correctly ``` error: failed to build a valid final module: Entry point fragment at Fragment is invalid ┌─ crates/bevy_pbr/src/render/pbr_prepass.wgsl:26:22 │ 26 │ let normal = evy_pbr::pbr_functions::31mapply_normal_mapping( │ ╭──────────────────────^ 27 │ │ bevy_pbr::pbr_bindings::material.flags, 28 │ │ world_normal, 29 │ │ · │ 36 │ │ 37 │ │ bevy_pbr::mesh_view_bindings::view.mip_bias, │ ╰───────────────────────────────────────────────────────────────────────────────────────^ invalid function call │ = Call to [9] is invalid = Requires 6 arguments, but 4 are provided ``` ## Solution - fix `apply_normal_mapping` calls
Objective
Right now, we flip the
world_normal
in response todouble_sided && !is_front
, however when calculatingN
from tangents and the normal map, we don't flip the normal read from the normal map, which produces extremely weird results.Solution
double_sided
andis_front
flags to theapply_normal_mapping()
function and use them to conditionally flipNt
Comparison
Note: These are from a custom scene running with the
transmission
branch, (#8015) I noticed lighting got pretty weird for the back side of translucentdouble_sided
materials whenever I added a normal map.Before
After
Changelog
StandardMaterial::double_sided
would interact incorrectly with normal maps, producing broken results.