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

Multiple fixes for compressed meshes #88738

Merged
merged 1 commit into from
Feb 27, 2024

Conversation

clayjohn
Copy link
Member

Fixes: #85406 (fixed by disabling compression when mesh has bogus tangents)
Fixes: #84270 (fixed by disabling compression when we generate bogus tangents because the mesh has bogus UVs)
Fixes: #83345
Fixes: #88619 (fixed by avoiding singularity when generating tangents and normal is exactly (0,1,0)

There are two different fixes here:

  1. When generating dummy tangents from normals, there is a singularity when the normal is exactly (0, 1, 0). In that case we generate bad tangents. So we avoid that by using the normal vector and scrambling the channels to generate the tangent vector. This works in all cases except for when normal is (0,0,0).

  2. When users import bogus meshes the normal/tangent compression fails. It requires that both normals and tangents are valid and are actually perpendicular to each other. Since it is difficult to discover the "force disable compression option", we just detect these cases and silently force disable compression for the user.

@clayjohn clayjohn added bug topic:rendering topic:import regression cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Feb 23, 2024
@clayjohn clayjohn added this to the 4.3 milestone Feb 23, 2024
@clayjohn clayjohn requested a review from fire February 23, 2024 21:59
@clayjohn clayjohn requested review from a team as code owners February 23, 2024 21:59
@clayjohn clayjohn force-pushed the mesh_compression-tangents branch from 33fc3c2 to 781cd27 Compare February 24, 2024 00:25
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really qualified to do a technical review here, but given the bugs it fixes and Clay's expertise on this area, I'm going to merge so we can get the fixes tested in the next dev snapshot.

Post merge code reviews from rendering/import contributors still welcome :)

@akien-mga akien-mga merged commit 8f98ed6 into godotengine:master Feb 27, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 4.2.2.

@akien-mga akien-mga removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Mar 9, 2024
@clayjohn clayjohn deleted the mesh_compression-tangents branch April 24, 2024 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants