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 calculation of skinned AABB for unused bones. #77265

Merged
merged 1 commit into from
May 22, 2023

Conversation

lyuma
Copy link
Contributor

@lyuma lyuma commented May 20, 2023

Fixes bug where bounding box of 1 unit was used in some models and had wrong LODs. (this could become very large if the mesh is scaled, such as FBX conversions)
Use LOD distance of 0 if camera is inside mesh bounds, rather than distance to surface.
Also fixes a mistake in calculating bone index.

This PR is part of #76184 by clayjohn

Fixes #67890
It might address part of #76436 but I have not tested MultiMesh

@lyuma lyuma requested a review from a team as a code owner May 20, 2023 01:53
@Calinou Calinou added this to the 4.1 milestone May 20, 2023
@lyuma lyuma force-pushed the aabb_bone_lod_inside branch 2 times, most recently from 4de2637 to a1c6cc6 Compare May 20, 2023 02:09
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Looks great!

@lyuma
Copy link
Contributor Author

lyuma commented May 20, 2023

Hmmm. I may have made a mistake by combining two changes here.

The first fix for the AABB -1,-1,-1 works.

The is_inside() check is not working. I'm still hitting cases where the incorrect LOD is used when the bounds are too large or I'm inside them. And it's very dependent on camera angle.

so there's something wrong with the math here that I'm not understanding.

Fixes bug where bounding box of 1 unit was used in some skinned models and had wrong LODs.
(this could become very large if the mesh is scaled, such as FBX conversions)
Also fixes a mistake in calcualting bone index.
@lyuma
Copy link
Contributor Author

lyuma commented May 20, 2023

malcomnixon reported some weird LOD issues with addons/godot-xr-tools/hands/scenes/lowpoly/left_hand_low.tscn from GodotVR/godot-xr-tools, so I'm testing this right now.

For now I have split it off so this just fixes the -1,-1,-1 bounding box size which we already tested. And if I figure out the other issue, I'll make a separate PR for it.

@akien-mga akien-mga merged commit 05ddc82 into godotengine:master May 22, 2023
@akien-mga
Copy link
Member

Thanks!

@YuriSizov
Copy link
Contributor

Cherry-picked for 4.0.4. While this is not a complete fix, code improvement seems good to me to include.

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.

Extra Cull Margin triggers worst LOD
5 participants