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

Fix visible seam on smoothed sphere and cylinder shapes #58208

Conversation

MythTitans
Copy link
Contributor

@MythTitans MythTitans commented Feb 16, 2022

Fix visible seam on smoothed sphere and cylinder shapes, cause by a bad normal computation at the junction between the first and last radial segment of the shapes (#58207).

The problem is due to the fact that smoothed normals are computed by accumulating the normals of all the faces shared by a single vertex, found from a lookup hash table, and then normalized (see vec_map in CSGShape3D::_update_shape()). But probably due to floating point errors and the way the hash of a Vector3D is computed, the vertices on the first and the last radial segment are not detected as sharing the same faces so the computed normal is wrong.

The fix here is to detect if we are on the last radial segment, and if it is the case compute the points exactly as it was done for the first radial segment, so the results are the same and they are matched together by the hash table.

Before

sphere_small_before
cylinder_small_before

After

sphere_small_after
cylinder_small_after

@MythTitans MythTitans requested a review from a team as a code owner February 16, 2022 22:16
@Calinou Calinou added bug topic:3d topic:core cherrypick:3.4 cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Feb 16, 2022
@Calinou Calinou added this to the 4.0 milestone Feb 16, 2022
@akien-mga akien-mga merged commit 1177bd6 into godotengine:master Mar 7, 2022
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@akien-mga
Copy link
Member

Cherry-picked for 3.5.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Mar 7, 2022
@akien-mga
Copy link
Member

Cherry-picked for 3.4.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants