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

More geometrically-accurate smooth normals. #18383

Open
Waridley opened this issue Mar 18, 2025 · 2 comments · May be fixed by #18552
Open

More geometrically-accurate smooth normals. #18383

Waridley opened this issue Mar 18, 2025 · 2 comments · May be fixed by #18552
Labels
A-glTF Related to the glTF 3D scene/model format C-Feature A new feature, making something new possible D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Design This issue requires design work to think about how it would best be accomplished

Comments

@Waridley
Copy link
Contributor

Waridley commented Mar 18, 2025

Problem

#16050 added a much-needed fix to computed smooth normals, where the previous implementation (still in 0.15) resulted in normals weighted by the count of triangles in each plane connected to a vertex. The current implementation instead weights them by the area of each triangle, which is sometimes entirely desired.

However, in a lot of cases, the area of a triangle connected to a vertex has nothing to do with the area of the entire face on that side, like it would in a full 3D modelling program like Blender. And even if it were the same circumstance as Blender, non-face-weighted normals seems to be the default in most cases anyway. For example, the normal of the maximum corner of a mesh generated from an AABB should always be equal to Vec3::ONE.normalize(), no matter the dimensions of the box or the direction the quads are divided into triangles. (In one direction, a face would contribute half of its area, and in the other, it would contribute the full area).

Solution

IMHO, the normals should by default be weighted by the angle of the corner of the triangle at that vertex, as explained in this StackOverflow answer. I have made a demo comparing the current implementation with one that uses corner angles rather than face area: https://github.com/Waridley/bevy_smooth_normals_comparison?tab=readme-ov-file

Open questions

Should it actually be the default?

main branch impl on the left, angle-weighted on the right

This view does highlight another potential drawback of this approach: The normal in the concave corner where the gable meets the spire is almost entirely influenced by the spire face and not the ridge of the gable. It's unclear that that is strictly incorrect, but since the far left vertex of the ridge of the gable is pointing significantly more in the upward direction, scaling the mesh along the normals would result in a more tilted ridge. It also results in a more pronounced faux-ambient occlusion effect. I still think the perfectly-vertical spire peak normal is by far the best option, though.

Naming

If the angle-weighted method does not become the default, but does get added as another method, that method would need a name. I've been calling them "geometric" normals, but I'm not sure how clear that is.

If the area-weighted version remains an option, is it really clear that only the triangles connected to the vertex are incorporated, not the whole plane on that side of the object? Maybe its name could stay but the docs could be updated to clarify that.

Additional context

I discovered the issue with 0.15's smooth normals when I tried to generate outline meshes and they were very lumpy. It turns out bevy_mod_outline also had to deal with that issue and ended up adding an entire separate mesh attribute for it. They might want to keep it around for other reasons anyway, but it seems to me like generating "geometric" smooth normals by default would be less surprising.

I can go ahead and make the PR to update Bevy's implementation, either going with my gut on the unanswered questions and updating later if necessary, or including any feedback provided before I actually make the PR.

@Waridley Waridley added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Mar 18, 2025
@komadori
Copy link
Contributor

Naturally, I agree that angle-weighted smooth normals work very well having adopted them in bevy_mod_outline and would support that being the default in Bevy.

Note that the purpose of bevy_mod_outline::ATTRIBUTE_OUTLINE_NORMAL is that non-smooth vertex normals may be authored for aesthetic reasons, but the normals used for vertex extrusion outlines must be smooth otherwise the outline mesh will have holes.

@alice-i-cecile alice-i-cecile added A-glTF Related to the glTF 3D scene/model format S-Needs-Design This issue requires design work to think about how it would best be accomplished D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes and removed S-Needs-Triage This issue needs to be labelled labels Mar 20, 2025
@alice-i-cecile
Copy link
Member

I think that this is a reasonable option to provide. I'm not sure on which default behavior is better however. Please feel free to open a PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-glTF Related to the glTF 3D scene/model format C-Feature A new feature, making something new possible D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Design This issue requires design work to think about how it would best be accomplished
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants