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

Default to trimesh for generated collision shapes in Advanced Import Settings #89461

Merged

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Mar 13, 2024

This avoids using convex decomposition every time collision is enabled on a mesh, which can be extremely slow on complex meshes (such as entire levels).

This doesn't break compatibility since default values are saved to .import files, unlike .tscn/.tres and project.godot.

We may want to consider though that only StaticBody can use trimesh collision shapes. Other PhysicsBody types can only use convex shapes or primitive shapes, so perhaps we should automatically change the default according to the selected PhysicsBody type. (In this case, simple convex decomposition should be preferred for dynamic objects as it's faster).

@Calinou Calinou requested a review from a team as a code owner March 13, 2024 22:58
@Calinou Calinou added this to the 4.x milestone Mar 13, 2024
@Calinou Calinou marked this pull request as draft March 13, 2024 23:03
…Settings

This avoids using convex decomposition every time collision is enabled
on a mesh, which can be extremely slow on complex meshes (such as entire
levels).
@Calinou Calinou force-pushed the 3d-import-collision-default-trimesh branch from 68d31a2 to 51af186 Compare March 13, 2024 23:07
@Calinou Calinou marked this pull request as ready for review March 13, 2024 23:07
@smix8
Copy link
Contributor

smix8 commented Mar 14, 2024

Imo what should change is that you can turn on the "generate physics" property without it triggering the calculation. Instead, default it to NONE and let user select what shape they want generated before doing it. Trimesh collision is low quality and low performance so not exactly something that fits a good default.

Just changing the default shape is just adding lipstick on a pig because the real issue is that such a problematic mesh shouldnt be used for collision generation in the first place. If the source mesh is so bad that the convex decomposition takes so long the trimesh will be faster but it will be horrendously low quality and performance broken at runtime, just not in the editor. That is not really doing anyone a favor, it just hides the real issue for some time.

Copy link
Contributor

@lyuma lyuma left a comment

Choose a reason for hiding this comment

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

I think it's good. we should make sure the user does not select Trimesh if the body_type is Dynamic. possibly dynamically changing the default property value in that case.

@akien-mga akien-mga modified the milestones: 4.x, 4.3 May 3, 2024
@akien-mga akien-mga merged commit 8efe584 into godotengine:master May 4, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@Calinou Calinou deleted the 3d-import-collision-default-trimesh branch May 7, 2024 00:06
@Malcolmnixon
Copy link
Contributor

This is a breaking change - in that its causing problems loading GLB assets that were configured with SHAPE_TYPE_DECOMPOSE_CONVEX in Godot 4.2 or earlier.

In Godot 4.2 the default shape type was 0 (SHAPE_TYPE_DECOMPOSE_CONVEX), and as such this value wasn't saved in the .import file. When these files are now loaded into Godot 4.3 these GLB files are being given SHAPE_TYPE_TRIMESH colliders.

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.

Change default collision shape type in imported 3D scenes away from "convex decomposition"
5 participants