godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.04k stars 21.18k forks source link

Editor crashes when generating ArrayMesh #23790

Closed Toshiwoz closed 5 years ago

Toshiwoz commented 5 years ago

Godot version:

Godot 3.1 custom build with hash 9eb4b6d

OS/device including version: Windows 10 (64 bits) AMD Radeon R5 M330

Issue description: I have a scene with a script that works with arraymesh when it ends generating the array I get this crash dump: ERROR: CowData::get: FATAL: Index p_index=0 out of size (size()=0) At: C:\Users\Home\Documents\TRABAJO-LEO\Godot Game Engine\godot\core/cowdata.h:149 CrashHandlerException: Program crashed Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues [0] CowData::get (c:\users\home\documents\trabajo-leo\godot game engine\godot\core\cowdata.h:149) [1] Vector::operator[] (c:\users\home\documents\trabajo-leo\godot game engine\godot\core\vector.h:89) [2] RasterizerSceneGLES3::_fill_render_list (c:\users\home\documents\trabajo-leo\godot game engine\godot\drivers\gles3\rasterizer_scene_gles3.cpp:3155) [3] RasterizerSceneGLES3::render_scene (c:\users\home\documents\trabajo-leo\godot game engine\godot\drivers\gles3\rasterizer_scene_gles3.cpp:4131) [4] VisualServerScene::_render_scene (c:\users\home\documents\trabajo-leo\godot game engine\godot\servers\visual\visual_server_scene.cpp:2140) [5] VisualServerScene::render_camera (c:\users\home\documents\trabajo-leo\godot game engine\godot\servers\visual\visual_server_scene.cpp:1714) [6] VisualServerViewport::_draw_viewport (c:\users\home\documents\trabajo-leo\godot game engine\godot\servers\visual\visual_server_viewport.cpp:70) [7] VisualServerViewport::draw_viewports (c:\users\home\documents\trabajo-leo\godot game engine\godot\servers\visual\visual_server_viewport.cpp:308) [8] VisualServerRaster::draw (c:\users\home\documents\trabajo-leo\godot game engine\godot\servers\visual\visual_server_raster.cpp:107) [9] VisualServerWrapMT::draw (c:\users\home\documents\trabajo-leo\godot game engine\godot\servers\visual\visual_server_wrap_mt.cpp:104) [10] Main::iteration (c:\users\home\documents\trabajo-leo\godot game engine\godot\main\main.cpp:1867) [11] OS_Windows::run (c:\users\home\documents\trabajo-leo\godot game engine\godot\platform\windows\os_windows.cpp:2736) [12] widechar_main (c:\users\home\documents\trabajo-leo\godot game engine\godot\platform\windows\godot_win.cpp:150) [13] _main (c:\users\home\documents\trabajo-leo\godot game engine\godot\platform\windows\godot_win.cpp:172) [14] main (c:\users\home\documents\trabajo-leo\godot game engine\godot\platform\windows\godot_win.cpp:184) [15] __scrt_common_main_seh (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:288) [16] BaseThreadInitThunk -- END OF BACKTRACE --

Steps to reproduce:

Minimal reproduction project: GodotTerrainPrimitive.zip

I have looked for similar issues, and found the following (stack trace is similar):

21892

I might be wrong but, to me it seems like the problem is here: drivers\gles3\rasterizer_scene_gles3.cpp:3155

This part of the code:

int ssize = mesh->surfaces.size();

                for (int i = 0; i < ssize; i++) {

                    int mat_idx = inst->materials[i].is_valid() ? i : -1;
                    RasterizerStorageGLES3::Surface *s = mesh->surfaces[i];
                    _add_geometry(s, inst, NULL, mat_idx, p_depth_pass, p_shadow_pass);
                }

It cycles through surfaces but it uses the index of the surface as the material index (but, is there always a material set into a mesh?), and as I place a material override then I managed to avoid the error.

Toshiwoz commented 5 years ago

Dang, I can't reproduce anymore the error..

akien-mga commented 5 years ago

That seems to be the same issue as the first crash mentioned in #24635, which #24650 aims at fixing.

BastiaanOlij commented 5 years ago

This only happens when you create everything from scratch. There is a delay between adding the surface and an (empty) entry being added for the material override list for that surface. When it then tries to look it up it crashes. Once that entry is there, everything works.

Basically the way to reproduce this is to start with adding a mesh instance to the scene, then adding an array mesh to the mesh instance, then calling add_surface_from_arrays on that array mesh.

The add_surface_from_arrays will create all the mesh data for the mesh, all good so far, it then sends out a changed signal.

This changed signal will cause MeshInstance::_mesh_changed being called and inside of this method we find the code that will resize the materials override array.

This is where it goes wrong, when I last tested there were about 6 frames that got rendered between calling add_surface_from_arrays, and the signal being processed. In those 6 frames the materials override array isn't large enough leading to the crash.

@reduz any idea what could cause the delay? I thought signals were pretty much executed right away?

24650 works around the problem by checking if the material is in the material array and if not, skips trying to access it. That said, it is the correct solution if it is likely there is a delay between calling add_surface_from_arrays and the signals firing, something that is absolutely likely in a multi threading scenario.

@akien-mga the crash log is the first one:

OpenGL ES 3.0 Renderer: GeForce GTX 1060/PCIe/SSE2
ERROR: CowData<class RID>::get: FATAL: Index p_index=0 out of size (size()=0)
   At: C:\Users\basti\Development\godot3-git\core/cowdata.h:149
CrashHandlerException: Program crashed
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[0] CowData<RID>::get (c:\users\basti\development\godot3-git\core\cowdata.h:149)
[1] Vector<RID>::operator[] (c:\users\basti\development\godot3-git\core\vector.h:85)
[2] RasterizerSceneGLES3::_fill_render_list (c:\users\basti\development\godot3-git\drivers\gles3\rasterizer_scene_gles3.cpp:3170)
[3] RasterizerSceneGLES3::render_scene (c:\users\basti\development\godot3-git\drivers\gles3\rasterizer_scene_gles3.cpp:4147)
[4] VisualServerScene::_render_scene (c:\users\basti\development\godot3-git\servers\visual\visual_server_scene.cpp:2145)
[5] VisualServerScene::render_camera (c:\users\basti\development\godot3-git\servers\visual\visual_server_scene.cpp:1714)
[6] VisualServerViewport::_draw_viewport (c:\users\basti\development\godot3-git\servers\visual\visual_server_viewport.cpp:70)
[7] VisualServerViewport::draw_viewports (c:\users\basti\development\godot3-git\servers\visual\visual_server_viewport.cpp:308)
[8] VisualServerRaster::draw (c:\users\basti\development\godot3-git\servers\visual\visual_server_raster.cpp:107)
[9] VisualServerWrapMT::draw (c:\users\basti\development\godot3-git\servers\visual\visual_server_wrap_mt.cpp:104)
[10] Main::iteration (c:\users\basti\development\godot3-git\main\main.cpp:1874)
[11] OS_Windows::run (c:\users\basti\development\godot3-git\platform\windows\os_windows.cpp:2754)
[12] widechar_main (c:\users\basti\development\godot3-git\platform\windows\godot_win.cpp:150)
[13] _main (c:\users\basti\development\godot3-git\platform\windows\godot_win.cpp:172)
[14] main (c:\users\basti\development\godot3-git\platform\windows\godot_win.cpp:184)
[15] __scrt_common_main_seh (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:283)
[16] BaseThreadInitThunk
-- END OF BACKTRACE --
BastiaanOlij commented 5 years ago

Ok, so if you create the array mesh in _ready, it seems like everything is processed in time.

Perform it later on, say on a botton press however.........................

ArrayMeshCrash.zip

Run the project, press the button.. :)

BastiaanOlij commented 5 years ago

@reduz just a ping, I know you're traveling so you might not get to look at this again until you're back but this one is nasty. It would be good if you could at least try the example library in my last post maybe to offer some hints on how this may be solved?

kb173 commented 5 years ago

I found this thread while looking up a similar crash. It does not happen during the creation of an ArrayMesh, but the error message seems very similar:

E 0:01:01:0661   FATAL: Index p_index=2 out of size (((Vector<T> *)(this))->_cowdata.size()=0)
  <C Source>     ./core/vector.h:49 @ operator[]()

I'm still trying to trace back where exactly this is happening, but it seems to be related to setting shader parameters in threads.

BastiaanOlij commented 5 years ago

@kb173 it crashes because the array of materials on the meshinstance lags behind the number of surfaces you add. When you add a surface to the arraymesh it needs to be mirrored in the number of material overrides you have on the meshinstance. That is handled by signals but for some reason the signal is processed too late. When the meshinstance gets rendered without the material overrides being in place (even if they are empty/null variants) it crashes.

This only happens when the mesh instance and array mesh are created first, and the surface is added later. If everything is in place before the scene gets rendered it all works ok.

I've been trying to get @reduz to try out the example scene I added a few posts up as I'm not sure how to solve the delayed signal processing.

kb173 commented 5 years ago

@BastiaanOlij Thank you for the clarification! Interestingly, the p_index the Error spits out is different depending on where the call to set_shader_param is placed: In my own (GDScript) thread, it's 2, whereas if it happens in the main thread (in _process) right after my thread is done, it's 3.

I hope this is helpful information, I haven't dived too much into the engine code.

BastiaanOlij commented 5 years ago

@reduz nice one!

kb173 commented 5 years ago

Unfortunately, I'm still getting the error sometimes (FATAL: Index p_index=0 out of size (((Vector<T> *)(this))->_cowdata.size()=0)... I will try to provide more detail once I found where exactly it's coming from.

Toshiwoz commented 5 years ago

@kb173 you should try with beta 9 and the project Bastiaan has created, above: https://github.com/godotengine/godot/issues/23790#issuecomment-457566379

kb173 commented 5 years ago

@Toshiwoz That project works fine for me with beta 9, even with multithreading. I'll try to create an example project once I can find the cause.