CSGMesh is not updated when it's properties are changed

[Einlander]

v3.2.4.rc3.official

Windows 10 PRO; Radeon RX 570 8GB; Driver 20.12.1; GLES3

Issue description:
CSGMesh is not updated when it's properties are changed

Steps to reproduce:

1. Add a CSGMesh to the scene.
2. Change the mesh property. Some of them show up immediatly, some do not.
3. If the mesh is not visible, changing to a different tab and coming back will fix it.

• If you undo changing the mesh change, the meshes wireframe stays visible.

Minimal reproduction project:

Godot.Engine.-.Evaluation.Abridged.2021-03-02.22-49-15-1.mp4

[gongpha]

Regression from e8804b9. And caused on chosen CubeMesh, PlaneMesh, PrismMesh or QuadMesh

[akien-mga]

Is it reproducible in 3.2.4 RC 5? Parts of #39635 were missing in 3.2.4 RC 3, added in RC 4.

CC @Meriipu

[gongpha]

It can reproduce it from 3.2.4 beta2 to 3.2.4 RC 5.

I have no idea about this commit is about viewport input will breaking mesh generating.

[akien-mga]

Regression from e8804b9. And caused on chosen CubeMesh, PlaneMesh, PrismMesh or QuadMesh

Tested and I can confirm the regression seems to be caused by e8804b9 (reverting that commit solves it), even though I really don't see the link between the two :|

Probably causes the 3D viewport not to be properly refreshed as it should be. Maybe here:

godot/editor/plugins/spatial_editor_plugin.cpp

Lines 4148 to 4152 in 8dd6fd0

	if (p_what == NOTIFICATION_MOUSE_ENTER || p_what == NOTIFICATION_MOUSE_EXIT) {
	mouseover = (p_what == NOTIFICATION_MOUSE_ENTER);
	update();
	}

CC @Meriipu @RandomShaper @JFonS

[akien-mga]

BTW this error is raised when adding a CubeMesh as mesh to a CSGMesh:

ERROR: mesh_surface_get_array: Index p_surface = 0 is out of bounds (mesh->surfaces.size() = 0).
   At: drivers/gles3/rasterizer_storage_gles3.cpp:4084.
ERROR: mesh_surface_get_arrays: Condition "vertex_data.size() == 0" is true. Returned: Array()
   At: servers/visual_server.cpp:1595.
ERROR: _build_brush: Condition "arrays.size() == 0" is true. Returned: _post_initialize(new ("") CSGBrush)
   At: modules/csg/csg_shape.cpp:738

There's also some more errors when adding or updating a CSGMesh though these don't seem caused by e8804b9 (also happen if reverted) and might not be linked to this issue:

ERROR: mesh_add_surface_from_arrays: Condition "array_len == 0" is true.
   At: servers/visual_server.cpp:965.
ERROR: add_surface_from_arrays: Condition "len == 0" is true.
   At: scene/resources/mesh.cpp:851.

[akien-mga]

For the reference, this issue doesn't seem reproducible in master (which also had the changes from #39635), maybe the regression was fixed there or did not happen in the first place.

[madmiraal]

This is a multi-thread syncing issue.

The multi-threading allows calls to the VisualSever to be queued and return before they are completed, and subsequent calls to be made before previously calls have completed. This causes a problem with simple Primitive Meshes here:

godot/scene/resources/primitive_meshes.cpp

Lines 87 to 90 in 5942a70

	VisualServer::get_singleton()->mesh_add_surface_from_arrays(mesh, (VisualServer::PrimitiveType)primitive_type, arr);
	VisualServer::get_singleton()->mesh_surface_set_material(mesh, 0, material.is_null() ? RID() : material->get_rid());
	pending_request = false;

The mesh_add_surface_from_arrays() returns and pending_request is set to false before it has created the surface on the RasterizerStorage:

godot/drivers/gles3/rasterizer_storage_gles3.cpp

Lines 3977 to 3978 in 5942a70

	mesh->surfaces.push_back(surface);
	mesh->instance_change_notify(true, true);

If an attempt is made to retrieve the array before the surface is created, if fails with

ERROR: mesh_surface_get_array: Index p_surface = 0 is out of bounds (mesh->surfaces.size() = 0).
   At: drivers/gles2/rasterizer_storage_gles2.cpp:2756.

because there is nothing to retrieve yet:

godot/scene/resources/primitive_meshes.cpp

Lines 129 to 136 in 5942a70

	Array PrimitiveMesh::surface_get_arrays(int p_surface) const {
	ERR_FAIL_INDEX_V(p_surface, 1, Array());
	if (pending_request) {
	_update();
	}
	return VisualServer::get_singleton()->mesh_surface_get_arrays(mesh, 0);
	}

Ironically, it "works" with more complex meshes, because the call to create the mesh doesn't reach the call to add the surface before the call to retrieve the array is made; so pending_request is still true. This causes a second (unnecessary) call to be made to update the mesh which doesn't return before the first call to update the mesh has finished creating the surface.

I suspect, e8804b9 exposed this preexisting bug, by bringing forward the call to retrieve the array, called by the CSGShapeSpatialGizmoPlugin:

godot/modules/csg/csg_gizmos.cpp

Line 318 in 5942a70

void CSGShapeSpatialGizmoPlugin::redraw(EditorSpatialGizmo *p_gizmo) {

This happens on 3.3 with both GLES2 and GLES3. It probably doesn't happen on master because of the entire VisualServer rewrite.