Crash when configuring the import of a gltf file and enabling Physics in a MeshInstance3D and switching body and shape types

[Dancovich]

Godot version

v4.2.rc2.official [1ba920f]

System information

Windows 11, Intel Core i5 10300H, nVidia GTX 1650

Issue description

When configuring the import of a gltf file, I'm trying to configure a mesh to use physics. When switching the body and shape type of this mesh, the engine will sometimes crash.

I couldn't detect the specific action that triggered the crash. Switching possible shape types appears to make the crash happen more often, but sometimes it will crash on the first shape type I choose and sometimes I can switch the shape type several times before it crashes.

The bug also happens in 4.2_RC1, but it's WAY easier to reproduce there. Sometimes merely checking Physics on will already trigger the crash. At most, doing one switch of body type (not even shape type) will also trigger the crash.

Steps to reproduce

• Open the sample project
• Double click room_model.glb
• Click the ComputerScreen mesh instance
• Check Physics on
• Change body type to Area
• Change shape types

It can take a few changes to shape type to trigger the bug in RC2, but in RC1 sometimes merely checking Physics on can trigger it.

Minimal reproduction project

Project to reproduce the bug.

bug_project.zip

Video of bug happening on my machine (I was "lucky" and it happened right away, sometimes it takes a few switches to shape type before it happens).

2023-11-27.13-28-30.mp4

[akien-mga]

Tested on Mageia 9 Linux (KDE/X11, Radeon RX Vega M), I couldn't reproduce the crash in 4.2-rc1 or 4.2-rc2.

Might be a Windows-specific or driver specific issue.

Can you reproduce the issue in 4.1.3-stable, and on earlier 4.2 dev/beta snapshots?

[Dancovich]

Could reproduce it in 4.1.3. Behaved like 4.2_RC1 (instant crash, where in RC2 it takes more work to trigger the crash).

Also happened in 4.2 Beta 6, but it didn't happen right away. I had to change shape types a few times. Then I had the idea of deleting the .godot folder before testing and the bug happened right away when I changed the body type in the import settings window.

Then I had the idea of testing the same thing on RC2. I delete the .godot folder, open the project in RC2 and Godot re-imports all assets as usual, but if I try to configure the import and change the body and shape type, editor crashes immediately.

Isn't this bug related to the recent mingw bug that RC2 was supposed to fix?

Edit: Just to make it clear, the first time the bug happened to me I did nothing to the .godot folder. I don't think erasing this folder is a requirement, but it did make the bug easier to reproduce for me.

[jsjtxietian]

For me delete the .godot folder will trigger this crash, it's hit this assertion message located in the thirdparty vhacd:

godot/thirdparty/vhacd/inc/vhacdVolume.h

Line 365 in f82bf35

assert(i < m_dim[0] && i >= 0 && j < m_dim[1] && j >= 0 && k < m_dim[2] && k >= 0);

i = 63 which is probably fine but j = k = 9223372036854775808 which is insane.

[akien-mga]

Still can't reproduce after deleting the .godot folder, but this is like Windows/compiler specific.

The assert shouldn't trigger in official builds as we define NDEBUG, which should disable the assert macro and make it a no-op. But apparently this isn't working here.

godot/SConstruct

Lines 440 to 441 in f82bf35

	# Disable assert() for production targets (only used in thirdparty code).
	env_base.Append(CPPDEFINES=["NDEBUG"])

@YuriSizov also mentioned seeing this on Windows, though I can't find the bug report anymore, if it was filed.

And #60357 seems to be the same problem, reproducible on Linux too.

[akien-mga]

For the record, VHACD had a major rewrite as a 4.0 release last year (now 4.1): https://github.com/kmammou/v-hacd/releases

We're still on an older version of the library. Porting to the new version might help solve this, but that's a big undertaking as everything was rewritten and it's now a single header library.

We can likely also debug and find a workaround for this assert in the current version to have a safe fix rapidly.

[YuriSizov]

@YuriSizov also mentioned seeing this on Windows, though I can't find the bug report anymore, if it was filed.

It was discovered in #85201 with the Tilt5 test project https://github.com/Malcolmnixon/TiltFiveMapTest. I didn't get to extracting this problem into a separate issue, but I guess we're here now :)

It was consistently reproduceable on reimport of the project, at the same exact time in the process, so specific to some particular assets, though I never noted down which ones.

Edit: Note that the same project showed multiple but sporadic issues with the physics server. If this issue is related to physics, then #85217 may also be related to this validation check not passing on some assets/assets being invalid.

[Malcolmnixon]

I've found the following model reliably causes the crash for me windturbine_tall.zip from the https://godotengine.org/asset-library/asset/2124 assets.

The assert is caused by the ludicrous voxel index because it encounters a point smaller than the minimum bounding-box coordinate, resulting in a negative position which when cast to a size_t blows up to a ludicrous positive size. I'm debugging to try and find out why any point is outside the bounding box.

[Malcolmnixon]

I believe the issue is ImporterMesh::convex_decompose line 972:

godot/scene/resources/importer_mesh.cpp

Lines 972 to 974 in d76c1d0

	index = ++vertex_count;
	vertex_map[vertex] = index;
	vertex_w[index] = vertex;

The index is pre-incremented resulting in the vertex array being populated from 1 up to (and including) vertex_count. Then the vertices array is resized to vertex_count entries truncating the last entry.

My testing shows that changing this to a post-increment fixes the issue on my specific mesh and appears to populate the vertex array correctly; however I'd like confirmation it's not caused by the code containing a mix of 0-based and 1-based code and an intent to "pad" a leading zero.

[yythlj]

I import building glb from sketchfab，and it still crash when choose enable physix and select the type
Is it has fix on version 4.2 or 4.3
I use both this two ver nouse

[yythlj]

like this glb, can rightly import and use, but if i Check Physics on, godot will no respond and only can kill process.My version is godot 4.2.1.The glb download in sketchfab.com and another glb occur the same problem too.Please help me.
shop_house.zip

[Malcolmnixon]

Hi @yythlj. I'm not able to get it to "crash", but it does take a truly vast amount of time to update the colliders. Analyzing the model you provided (shop_house.glb) shows it consists of:

• 68513 vertices
• 56289 triangles
• 1176 discrete objects/meshes

The vertices and triangles aren't a problem; but I don't think I've ever seen a model with this many discrete parts - every individual leaf, roof-tile, and stone is a separate object. Is this some kind of stress-test model? I can't imagine it doing anything other than causing graphics pipelines to scream in agony 😊

What I can say is that SceneImportSettingsDialog doesn't seem to handle this use-case well. From what I'm reading if ANY mesh setting changes on ANY node (even turning off physics collision) then generate_collider causes SceneImportSettingsDialog::_update_view_gizmos() to recalculating the colliders for EVERY node (in this case all 1176) even though only one node changed.

This behavior in SceneImportSettingsDialog::_update_view_gizmos() seems odd to me. Rather than hitting every node (by iterating through the node_map) to update the colliders; the selected_id field contains the name of the node being edited. I feel this could be an O(1) operation hitting just this node, rather than an O(n), but I didn't write this routine.

I did an ugly experimental hack to just update the one selected_id node, and to only calculate the collider when it wants to be shown. The result is an truly enormous speed-up for this use-case; however I'm not sure if I'm missing any complexities/intricacies around the behavior. This behavior was introduced by @AndreaCatania in #52266 so I think it would be best to consult with them to confirm if my analysis is missing anything.

PhysicsSceneImport.mp4