ArrayMesh::add_surface_from_arrays crashes

Hello, when trying to call ArrayMesh::add_surface_from_arrays from gdnative, I get a crash. Doing the exact same call from GDscript (with the array computed by gdnative) works well.

My setup

archlinux
gcc 11.1.0
godot version 3.4.2.stable.arch_linux (also tested with binary from godotengine.org 3.4.2.stable.official.45eaa2daf)
godot-cpp version godot-3.4.1-stable (also tested 3.3.4 and 3.1.1)

Minimal reproduction

I suggest to use gdnative_cpp_example because it is a good minimal reference, but really any gdnative plugin should do. It is just about writing 9 lines of codes, that make godot crashes at runtime.

Get the example repo https://github.com/BastiaanOlij/gdnative_cpp_example with submodules
Compile godot-cpp (scons platform=linux generate_bindings=yes in gdnative_cpp_example/godot-cpp)

Add the following to GDExample::_process implementation in src/gdexample.cpp (you need to #include <ArrayMesh.hpp> as well):

Array arr;
arr.resize(ArrayMesh::ARRAY_MAX);
ArrayMesh arrayMesh;
PoolVector3Array verts;
verts.append(Vector3(0, 0, 0));
verts.append(Vector3(0, 0, 1));
verts.append(Vector3(0, 0, 2));
arr[ArrayMesh::ARRAY_VERTEX] = verts;
arrayMesh.add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, arr);

Compile the gdnative lib with scons platform=linux in gdnative_cpp_example folder
Launch the demo project

The crash

Godot crashes with the following:

handle_crash: Program crashed with signal 11
Engine version: Godot Engine v3.4.2.stable.arch_linux
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[1] /usr/lib/libc.so.6(+0x3cda0) [0x7f2df9197da0] (??:0)
[2] godot(+0x2283be3) [0x55e184168be3] (??:0)
[3] godot(+0x2294e14) [0x55e184179e14] (??:0)
[4] godot::ArrayMesh::add_surface_from_arrays(long, godot::Array, godot::Array, long) (??:0)
[5] godot::GDExample::_process(float) (??:0)
[6] godot_variant godot::__wrapped_method<godot::GDExample, void, float>(void*, void*, void*, int, godot_variant**) (??:0)
[7] godot(+0xcf59e8) [0x55e182bda9e8] (??:0)
[8] godot(+0x1b8f13f) [0x55e183a7413f] (??:0)
[9] godot(+0x20acae8) [0x55e183f91ae8] (??:0)
[10] godot(+0x1ba93ee) [0x55e183a8e3ee] (??:0)
[11] godot(+0x1bc3eb1) [0x55e183aa8eb1] (??:0)
[12] godot(+0x8b9a25) [0x55e18279ea25] (??:0)
[13] godot(+0x8359a5) [0x55e18271a9a5] (??:0)
[14] /usr/lib/libc.so.6(__libc_start_main+0xd5) [0x7f2df9182b25] (??:0)
[15] godot(+0x84991e) [0x55e18272e91e] (??:0)
-- END OF BACKTRACE --

gdb gives a little more info. It says it comes from a SIGSEGV, but in my other project (too cumbersome to share here) it is a SIGBUS. Callstack for the minimal reproduction is:

(gdb) bt
#0  0x00005555577d7be3 in ?? ()
#1  0x00005555577e8e14 in ?? ()
#2  0x00007fffd887e2ec in godot::___godot_icall_void_int_Array_Array_int (arg3=<optimized out>, arg2=..., arg1=..., 
    arg0=<optimized out>, inst=0x7fffffffced0, mb=<optimized out>) at include/gen/__icalls.hpp:3348
#3  godot::ArrayMesh::add_surface_from_arrays (this=this@entry=0x7fffffffcf60, primitive=primitive@entry=4, arrays=..., 
    blend_shapes=..., compress_flags=compress_flags@entry=97280) at src/gen/ArrayMesh.cpp:57
#4  0x00007fffd879072a in godot::GDExample::_process (this=0x55555f7ec680, delta=0.149999976) at src/gdexample.cpp:38
#5  0x00007fffd8790ac4 in godot::_WrappedMethod<godot::GDExample, void, float>::apply<0> (args=0x7fffffffd0b8, obj=<optimized out>, 
    this=0x7fffffffcfd0) at godot-cpp/include/core/Godot.hpp:263
#6  godot::__wrapped_method<godot::GDExample, void, float> (method_data=0x7fffffffcfd0, user_data=<optimized out>, args=0x7fffffffd0b8)
    at godot-cpp/include/core/Godot.hpp:278
#7  0x00005555562499e8 in ?? ()
#8  0x00005555570e313f in ?? ()
#9  0x0000555557600ae8 in ?? ()
#10 0x00005555570fd3ee in ?? ()
#11 0x0000555557117eb1 in ?? ()
#12 0x0000555555e0da25 in ?? ()
#13 0x0000555555d899a5 in ?? ()
#14 0x00007ffff51a2b25 in __libc_start_main () from /usr/lib/libc.so.6
#15 0x0000555555d9d91e in ?? ()

Note

I don't know what is happening here or how to circonvent the issue. I have made some interesting finds though:

The same code written in GDscript works well, so it’s not an issue of the arguments beeing incorrect.
The call from add_surface_from_arrays does not always crash:
- If the input array is empty, it instead properly complains that this should be of size 9
- If the input array is of size 9 but without vertices, it instead properly complains it needs vertices
- And the strangest of all, I forgot to add the vertices to the array at some point, leading to this very weird discovery (read the comment):
```
Array arr;
arr.resize(ArrayMesh::ARRAY_MAX);
ArrayMesh arrayMesh;
PoolVector3Array verts;
verts.append(Vector3(0, 0, 0)); // <-- Without this line there is no crash, but with it there is!
// Even though the line should have no effect because "verts" is never used
arrayMesh.add_surface_from_arrays(Mesh::PRIMITIVE_TRIANGLES, arr);
```

What happens when you try adding indices for the ArrayMesh? My understanding is that the PRIMITIVE_TRIANGLES type requires, at minimum, a list of vertices and a list of indices for the triangles; when constructing a procedural mesh in this way, I haven't had any issues on 3.4.2 or the 4.0 master.

(As an aside, I've also found that constructing extensive procedural meshes like this incurs a huge performance hit from constructing Pool__Arrays as you go, likely due to how Godot is allocating memory for them. I was able to achieve a tenfold performance increase just by STL vectors of Vector3s and then copying them to Godot PoolVector3Arrays in one go.)

Hello, I took me a while to wrap my head again around this issue, but I had a complete look at it again.

Adding indices does not change anything. I have indices in my real scenario, here I just wanted to give a minimal reproduction. Even if the mesh would not be very relevant without indices, the real problem here is that godot crashes, which should not happen in any case.

I updated godot-cpp to 3.4.3 and the issue is still here (my godot is now 3.4.4).

You said you have some code that work constructing procedural mesh like this from gdnative? Do you have public code that I can check to see if it works on my machine? Did you try the minimal example from my post to see if you have an issue as well?

And thanks for the performance tips. I just used a append for now, but noted that I needed to check what was the fastest. Other options are resizing the array first, or use a writer.

@Shaac Here's an annotated example of what I use inside a helper class for constructing procedural meshes in my project. The data is populated elsewhere, but it's important to note that, if you have X vertices forming Y triangles, you need exactly:

X vertex locations
X vertex normals if you're using them
X vertex colors if you're using vertex colors
X*4 floats in the tangent array, if you're using tangents
Y*3 indices

Anything else produces undefined behavior, from what I've found.

Array ProceduralMeshData::get_mesh_for_ArrayMesh(bool color_to_srgb) {
    Array data = Array();
    PoolVector3Array verts, norms;
    PoolRealArray tangs;
    PoolColorArray cols;
    PoolIntArray inds;

    // The DataConverstion::copy_vector* functions just copy a std::vector<X> to a corresponding PoolXArray object.
    // I'm using this because in profiling, I found that using STL containers for calculating mesh values and then copying
    // them over to Godot containers in one step led to a significant (10x+) performance improvement. 
    DataConversion::copy_vector3(verts, vertices);
    DataConversion::copy_vector3(norms, normals);
    DataConversion::copy_vector_float(tangs, tangents);
    DataConversion::copy_vector_color(cols, colors);
    DataConversion::copy_vector_int(inds, indices);

    data.resize(ArrayMesh::ARRAY_MAX);
    data[ArrayMesh::ARRAY_VERTEX] = verts;
    data[ArrayMesh::ARRAY_NORMAL] = norms;
    data[ArrayMesh::ARRAY_INDEX] = inds;
    data[ArrayMesh::ARRAY_TANGENT] = tangs;
    data[ArrayMesh::ARRAY_COLOR] = cols;

    return data;
}

When returning an ArrayMesh, it uses the following function in the same helper object:

Ref<ArrayMesh> ProceduralMeshData::get_array_mesh() {
    Ref<ArrayMesh> new_mesh = nullptr;
    new_mesh.instance();

    new_mesh->add_surface_from_arrays(ArrayMesh::PRIMITIVE_TRIANGLES, get_mesh_for_ArrayMesh());
    return new_mesh;
}

Thank you for the quick response. I tried your snippet and it worked!

The only difference from what I used to do is the use of a Ref<ArrayMesh> instead of an ArrayMesh.

Most of the time it makes more sense to use a Ref<ArrayMesh> so I guess that is why no one noticed the issue. I myself will be better off using a Ref anyway so I guess this issue is fixed for me, but maybe this ticket should be kept open since godot crashing should not happen. Unless add_surface_from_arrays is only meant to be called from a reference, but then it should say so in the function documentation.

Thanks again

godotengine / godot-cpp