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

Normalize only nonzero normals for mikktspace normal maps #10905

Merged
merged 1 commit into from
Dec 10, 2023

Conversation

Jondolf
Copy link
Contributor

@Jondolf Jondolf commented Dec 7, 2023

Objective

Fixes #5891.

For mikktspace normal maps, normals must be renormalized in vertex shaders to match the way mikktspace bakes vertex tangents and normal maps so that the exact inverse process is applied when shading.

However, for invalid normals like vec3<f32>(0.0, 0.0, 0.0), this normalization causes NaN values, and because it's in the vertex shader, it affects the entire triangle and causes it to be shaded as black:

incorrectly shaded cone

A cone with a tip that has a vertex normal of [0, 0, 0], causing the mesh to be shaded as black.

In some cases, normals of zero are actually useful. For example, a smoothly shaded cone without creases requires the apex vertex normal to be zero, because there is no singular normal that works correctly, so the apex shouldn't contribute to the overall shading. Duplicate vertices for the apex fix some shading issues, but it causes visible creases and is more expensive. See #5891 and #10298 for more details.

For correctly shaded cones and other similar low-density shapes with sharp tips, vertex normals of zero can not be normalized in the vertex shader.

Solution

Only normalize the vertex normals and tangents in the vertex shader if the normal isn't [0, 0, 0]. This way, mikktspace normal maps should still work for everything except the zero normals, and the zero normals will only be normalized in the fragment shader.

This allows us to render cones correctly:

smooth cone with some banding

Notice how there is still a weird shadow banding effect in one area. I noticed that it can be fixed by normalizing here, which produces a perfectly smooth cone without duplicate vertices:

smooth cone

I didn't add this change yet, because it seems a bit arbitrary. I can add it here if that'd be useful or make another PR though.

@Jondolf Jondolf added C-Feature A new feature, making something new possible A-Rendering Drawing game state to the screen labels Dec 7, 2023
@superdump superdump added this pull request to the merge queue Dec 10, 2023
Merged via the queue into bevyengine:main with commit 4b1865f Dec 10, 2023
25 checks passed
@Jondolf Jondolf deleted the mikktspace-normalize-only-valid branch December 10, 2023 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Impossible to render smooth cone mesh properly after #5666
3 participants