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

Silent crash when Sprite3D.texture is assigned while null and Sprite3D.extra_cull_margin is positive #80749

Closed
Wierdox opened this issue Aug 18, 2023 · 9 comments · Fixed by #95503

Comments

@Wierdox
Copy link
Contributor

Wierdox commented Aug 18, 2023

Godot version

v4.1.1.stable.official [bd6af8e] v4.0.3.stable.official [5222a99]

System information

Godot v4.1.1.stable - Windows 10.0.19044 - Vulkan (Forward+) - dedicated NVIDIA GeForce GTX 1660 SUPER (NVIDIA; 31.0.15.3168) - AMD Ryzen 5 5600G with Radeon Graphics (12 Threads)

Issue description

Under these circumstances a silent game crash will occur when running a scene while using the editor or in an exported release:

  1. $Sprite3D.extra_cull_margin = 0.25 or any positive number.
  2. $Sprite3D.texture == null.
  3. Crashes upon $Sprite3D.texture = preload("res://icon.svg"), or any texture, but only if not in _ready().

Unsure if related, but there is an alternative silent crash if extra_cull_margin is set to a large number like 100000000. However, it occurs the second time $Sprite3D.texture is assigned to null of all things.

When happening in my game, the crash was a bit weirder but I can't recreate it. It was instead randomly crashing over a period of minutes, after when the repro would have crashed. During that time, the Sprite3D would be correctly assigned a texture(checked via remote view and prints), but it would be invisible in game.

Notes:

  • I print just before the crashes, and the print is caught by the external console but not the editor console upon crash.
  • No crash if assigning the equivalent with $Sprite3D.custom_aabb.
  • $Sprite3D.extra_cull_margin = -1.0 will obviously error, so it doesn't crash.
  • Crash occurs with all renderers(4.1 and 4.0 tested).
  • Reproduced in 4.1 and 4.0, 3.5 acts as expected and is nominal.
  • Camera3D is not needed for reproduction, it is only for viewing the Sprite3D.

Steps to reproduce

Download and run main.tscn to see the crash occur. main.gd contains commentary and example code.
If you use an external console, you may see one more print compared to the editor's console upon crash.
Uncomment $Sprite3D.extra_cull_margin = 100000000.0 in main.gd to see alternative crash.

Minimal reproduction project

crash_reproduction.zip

@AThousandShips
Copy link
Member

Can you confirm it is a silent crash and just not showing in the editor? Open with console and check the console instead

@Wierdox
Copy link
Contributor Author

Wierdox commented Aug 18, 2023

@AThousandShips
This is what I did to confirm it, pretty sure this is how you do it.

8FOBYvUCxD.webm

@AThousandShips
Copy link
Member

Thank you!

To have videos play in your comment make sure to put empty lines around it, I've fixed this for you but keep it in mind

@clayjohn
Copy link
Member

Stacktrace

================================================================
handle_crash: Program crashed with signal 11
Engine version: Godot Engine v4.2.dev.custom_build (6b9f7e29945a792c6453cd70d0ab72ef5885e477)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] /lib/x86_64-linux-gnu/libc.so.6(+0x42520) [0x7f6863242520] (??:0)
[2] DynamicBVH::_remove_leaf(DynamicBVH::Node*) (/home/clayjohn/dev/godot/core/math/dynamic_bvh.cpp:105)
[3] DynamicBVH::update(DynamicBVH::ID const&, AABB const&) (/home/clayjohn/dev/godot/core/math/dynamic_bvh.cpp:374)
[4] RendererSceneCull::_update_instance(RendererSceneCull::Instance*) (/home/clayjohn/dev/godot/servers/rendering/renderer_scene_cull.cpp:1772)
[5] RendererSceneCull::_update_dirty_instance(RendererSceneCull::Instance*) (/home/clayjohn/dev/godot/servers/rendering/renderer_scene_cull.cpp:3991)
[6] RendererSceneCull::update_dirty_instances() (/home/clayjohn/dev/godot/servers/rendering/renderer_scene_cull.cpp:3996)
[7] RendererSceneCull::update() (/home/clayjohn/dev/godot/servers/rendering/renderer_scene_cull.cpp:4016)
[8] RenderingServerDefault::_draw(bool, double) (/home/clayjohn/dev/godot/servers/rendering/rendering_server_default.cpp:85)
[9] RenderingServerDefault::draw(bool, double) (/home/clayjohn/dev/godot/servers/rendering/rendering_server_default.cpp:387)
[10] Main::iteration() (/home/clayjohn/dev/godot/main/main.cpp:3468)
[11] OS_LinuxBSD::run() (/home/clayjohn/dev/godot/platform/linuxbsd/os_linuxbsd.cpp:912)
[12] /home/clayjohn/dev/godot/bin/master.llvm(main+0x1fe) [0x55a3b9a8c8ae] (/home/clayjohn/dev/godot/platform/linuxbsd/godot_linuxbsd.cpp:74)
[13] /lib/x86_64-linux-gnu/libc.so.6(+0x29d90) [0x7f6863229d90] (??:0)
[14] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x80) [0x7f6863229e40] (??:0)
[15] /home/clayjohn/dev/godot/bin/master.llvm(_start+0x25) [0x55a3b9a8c5e5] (??:?)
-- END OF BACKTRACE --
================================================================

@clayjohn clayjohn added this to the 4.x milestone Aug 18, 2023
@AThousandShips
Copy link
Member

AThousandShips commented Aug 18, 2023

Might be caused by an invalid AABB as it is empty if no texture was set, would be worth checking if the DynamicBVH breaks if provided empty AABB

@jsjtxietian
Copy link
Contributor

jsjtxietian commented Aug 22, 2023

parent is nullptr so it crashed, and bvh_root is nullptr too.

image

@jsjtxietian
Copy link
Contributor

jsjtxietian commented Aug 24, 2023

I think I might find where the bug is, but I'm not familiar to the bvh code logic so I might be wrong, feel free to correct me.

After $Sprite3D.texture = null, p_instance's base_type is INSTANCE_NONE so the bvh node is inserted into INDEXER_VOLUMES's bvh:

image

And after the assign($Sprite3D.texture = preload("res://icon.svg")), p_instance->base_type is INSTANCE_MESH, so it trys to remove the node ( the node belongs to INDEXER_VOLUMES's bvh ) from INDEXER_GEOMETRY's bvh.

image

And in the code of _remove_leaf, it assumes the leaf belongs to this bvh and if the leaf is not root, it must have a parent. But the node passed in belongs to INDEXER_VOLUMES ( in fact, it is the bvh_root ), so leaf->parent is nullptr, and thus cause the crash.

image

@clayjohn
Copy link
Member

@jsjtxietian That looks like it is likely the culprit. Do you why the Sprite3D is set to INSTANCE_NONE when the texture is removed? To me it seems like it should either be removed entirely and thus not get indexed, or it should maintain its status as geometry

@jsjtxietian
Copy link
Contributor

jsjtxietian commented Aug 28, 2023

@clayjohn Sorry my previous explain is wrong to some extent, yesterday I found the bug has something to do with time and I missed that when using a debugger. If one comment out the code in timer and just step thorugh the code in _ready the bug will occur too. Same reason, but not related to the second assign.

  1. After $Sprite3D.extra_cull_margin = 0.25, p_instance's base_type is INSTANCE_NONE, and it inserted into INDEXER_VOLUMES's bvh (at line 1684)
  2. After the first texture assignment, p_instance's base_type will be INSTANCE_MESH (set at line 611), and it trys to update INDEXER_GEOMETRY's bvh and thus cause the crash.

I think maybe when we change p_instance's base_type, we just clear and rebuild the bvh ?

Take a look at my draft,your suggestions are welcomed.

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