Merge pull request #93407 from smix8/lock_this_geometry_up

Fix thread-use causing navigation source geometry data corruption

modules/navigation/2d/nav_mesh_generator_2d.cpp

@@ -852,8 +852,15 @@ void NavMeshGenerator2D::generator_bake_from_source_geometry_data(Ref<Navigation
 	}
 
 	int outline_count = p_navigation_mesh->get_outline_count();
-	const Vector<Vector<Vector2>> &traversable_outlines = p_source_geometry_data->_get_traversable_outlines();
+
-	const Vector<Vector<Vector2>> &obstruction_outlines = p_source_geometry_data->_get_obstruction_outlines();
+	Vector<Vector<Vector2>> traversable_outlines;
+	Vector<Vector<Vector2>> obstruction_outlines;
+	Vector<NavigationMeshSourceGeometryData2D::ProjectedObstruction> projected_obstructions;
+
+	p_source_geometry_data->get_data(
+			traversable_outlines,
+			obstruction_outlines,
+			projected_obstructions);
 
 	if (outline_count == 0 && traversable_outlines.size() == 0) {
 		return;
@@ -898,8 +905,6 @@ void NavMeshGenerator2D::generator_bake_from_source_geometry_data(Ref<Navigation
 		obstruction_polygon_paths.push_back(clip_path);
 	}
 
-	const Vector<NavigationMeshSourceGeometryData2D::ProjectedObstruction> &projected_obstructions = p_source_geometry_data->_get_projected_obstructions();
-
 	if (!projected_obstructions.is_empty()) {
 		for (const NavigationMeshSourceGeometryData2D::ProjectedObstruction &projected_obstruction : projected_obstructions) {
 			if (projected_obstruction.carve) {

modules/navigation/3d/nav_mesh_generator_3d.cpp

@@ -672,10 +672,16 @@ void NavMeshGenerator3D::generator_bake_from_source_geometry_data(Ref<Navigation
 		return;
 	}
 
-	const Vector<float> &vertices = p_source_geometry_data->get_vertices();
+	Vector<float> source_geometry_vertices;
-	const Vector<int> &indices = p_source_geometry_data->get_indices();
+	Vector<int> source_geometry_indices;
+	Vector<NavigationMeshSourceGeometryData3D::ProjectedObstruction> projected_obstructions;
 
-	if (vertices.size() < 3 || indices.size() < 3) {
+	p_source_geometry_data->get_data(
+			source_geometry_vertices,
+			source_geometry_indices,
+			projected_obstructions);
+
+	if (source_geometry_vertices.size() < 3 || source_geometry_indices.size() < 3) {
 		return;
 	}
 
@@ -691,10 +697,10 @@ void NavMeshGenerator3D::generator_bake_from_source_geometry_data(Ref<Navigation
 
 	bake_state = "Setting up Configuration..."; // step #1
 
-	const float *verts = vertices.ptr();
+	const float *verts = source_geometry_vertices.ptr();
-	const int nverts = vertices.size() / 3;
+	const int nverts = source_geometry_vertices.size() / 3;
-	const int *tris = indices.ptr();
+	const int *tris = source_geometry_indices.ptr();
-	const int ntris = indices.size() / 3;
+	const int ntris = source_geometry_indices.size() / 3;
 
 	float bmin[3], bmax[3];
 	rcCalcBounds(verts, nverts, bmin, bmax);
@@ -818,8 +824,6 @@ void NavMeshGenerator3D::generator_bake_from_source_geometry_data(Ref<Navigation
 	rcFreeHeightField(hf);
 	hf = nullptr;
 
-	const Vector<NavigationMeshSourceGeometryData3D::ProjectedObstruction> &projected_obstructions = p_source_geometry_data->_get_projected_obstructions();
-
 	// Add obstacles to the source geometry. Those will be affected by e.g. agent_radius.
 	if (!projected_obstructions.is_empty()) {
 		for (const NavigationMeshSourceGeometryData3D::ProjectedObstruction &projected_obstruction : projected_obstructions) {

scene/resources/2d/navigation_mesh_source_geometry_data_2d.cpp

@@ -33,37 +33,58 @@
 #include "scene/resources/mesh.h"
 
 void NavigationMeshSourceGeometryData2D::clear() {
+	RWLockWrite write_lock(geometry_rwlock);
 	traversable_outlines.clear();
 	obstruction_outlines.clear();
-	clear_projected_obstructions();
+	_projected_obstructions.clear();
 }
 
+bool NavigationMeshSourceGeometryData2D::has_data() {
+	RWLockRead read_lock(geometry_rwlock);
+	return traversable_outlines.size();
+};
+
 void NavigationMeshSourceGeometryData2D::clear_projected_obstructions() {
 	RWLockWrite write_lock(geometry_rwlock);
 	_projected_obstructions.clear();
 }
 
 void NavigationMeshSourceGeometryData2D::_set_traversable_outlines(const Vector<Vector<Vector2>> &p_traversable_outlines) {
+	RWLockWrite write_lock(geometry_rwlock);
 	traversable_outlines = p_traversable_outlines;
 }
 
 void NavigationMeshSourceGeometryData2D::_set_obstruction_outlines(const Vector<Vector<Vector2>> &p_obstruction_outlines) {
+	RWLockWrite write_lock(geometry_rwlock);
 	obstruction_outlines = p_obstruction_outlines;
 }
 
+const Vector<Vector<Vector2>> &NavigationMeshSourceGeometryData2D::_get_traversable_outlines() const {
+	RWLockRead read_lock(geometry_rwlock);
+	return traversable_outlines;
+}
+
+const Vector<Vector<Vector2>> &NavigationMeshSourceGeometryData2D::_get_obstruction_outlines() const {
+	RWLockRead read_lock(geometry_rwlock);
+	return obstruction_outlines;
+}
+
 void NavigationMeshSourceGeometryData2D::_add_traversable_outline(const Vector<Vector2> &p_shape_outline) {
 	if (p_shape_outline.size() > 1) {
+		RWLockWrite write_lock(geometry_rwlock);
 		traversable_outlines.push_back(p_shape_outline);
 	}
 }
 
 void NavigationMeshSourceGeometryData2D::_add_obstruction_outline(const Vector<Vector2> &p_shape_outline) {
 	if (p_shape_outline.size() > 1) {
+		RWLockWrite write_lock(geometry_rwlock);
 		obstruction_outlines.push_back(p_shape_outline);
 	}
 }
 
 void NavigationMeshSourceGeometryData2D::set_traversable_outlines(const TypedArray<Vector<Vector2>> &p_traversable_outlines) {
+	RWLockWrite write_lock(geometry_rwlock);
 	traversable_outlines.resize(p_traversable_outlines.size());
 	for (int i = 0; i < p_traversable_outlines.size(); i++) {
 		traversable_outlines.write[i] = p_traversable_outlines[i];
@@ -71,6 +92,7 @@ void NavigationMeshSourceGeometryData2D::set_traversable_outlines(const TypedArr
 }
 
 TypedArray<Vector<Vector2>> NavigationMeshSourceGeometryData2D::get_traversable_outlines() const {
+	RWLockRead read_lock(geometry_rwlock);
 	TypedArray<Vector<Vector2>> typed_array_traversable_outlines;
 	typed_array_traversable_outlines.resize(traversable_outlines.size());
 	for (int i = 0; i < typed_array_traversable_outlines.size(); i++) {
@@ -81,6 +103,7 @@ TypedArray<Vector<Vector2>> NavigationMeshSourceGeometryData2D::get_traversable_
 }
 
 void NavigationMeshSourceGeometryData2D::set_obstruction_outlines(const TypedArray<Vector<Vector2>> &p_obstruction_outlines) {
+	RWLockWrite write_lock(geometry_rwlock);
 	obstruction_outlines.resize(p_obstruction_outlines.size());
 	for (int i = 0; i < p_obstruction_outlines.size(); i++) {
 		obstruction_outlines.write[i] = p_obstruction_outlines[i];
@@ -88,6 +111,7 @@ void NavigationMeshSourceGeometryData2D::set_obstruction_outlines(const TypedArr
 }
 
 TypedArray<Vector<Vector2>> NavigationMeshSourceGeometryData2D::get_obstruction_outlines() const {
+	RWLockRead read_lock(geometry_rwlock);
 	TypedArray<Vector<Vector2>> typed_array_obstruction_outlines;
 	typed_array_obstruction_outlines.resize(obstruction_outlines.size());
 	for (int i = 0; i < typed_array_obstruction_outlines.size(); i++) {
@@ -117,6 +141,7 @@ void NavigationMeshSourceGeometryData2D::append_obstruction_outlines(const Typed
 
 void NavigationMeshSourceGeometryData2D::add_traversable_outline(const PackedVector2Array &p_shape_outline) {
 	if (p_shape_outline.size() > 1) {
+		RWLockWrite write_lock(geometry_rwlock);
 		Vector<Vector2> traversable_outline;
 		traversable_outline.resize(p_shape_outline.size());
 		for (int i = 0; i < p_shape_outline.size(); i++) {
@@ -128,6 +153,7 @@ void NavigationMeshSourceGeometryData2D::add_traversable_outline(const PackedVec
 
 void NavigationMeshSourceGeometryData2D::add_obstruction_outline(const PackedVector2Array &p_shape_outline) {
 	if (p_shape_outline.size() > 1) {
+		RWLockWrite write_lock(geometry_rwlock);
 		Vector<Vector2> obstruction_outline;
 		obstruction_outline.resize(p_shape_outline.size());
 		for (int i = 0; i < p_shape_outline.size(); i++) {
@@ -140,29 +166,16 @@ void NavigationMeshSourceGeometryData2D::add_obstruction_outline(const PackedVec
 void NavigationMeshSourceGeometryData2D::merge(const Ref<NavigationMeshSourceGeometryData2D> &p_other_geometry) {
 	ERR_FAIL_NULL(p_other_geometry);
 
-	// No need to worry about `root_node_transform` here as the data is already xformed.
+	Vector<Vector<Vector2>> other_traversable_outlines;
-	traversable_outlines.append_array(p_other_geometry->traversable_outlines);
+	Vector<Vector<Vector2>> other_obstruction_outlines;
-	obstruction_outlines.append_array(p_other_geometry->obstruction_outlines);
+	Vector<ProjectedObstruction> other_projected_obstructions;
 
-	if (p_other_geometry->_projected_obstructions.size() > 0) {
+	p_other_geometry->get_data(other_traversable_outlines, other_obstruction_outlines, other_projected_obstructions);
-		RWLockWrite write_lock(geometry_rwlock);
 
-		for (const ProjectedObstruction &other_projected_obstruction : p_other_geometry->_projected_obstructions) {
+	RWLockWrite write_lock(geometry_rwlock);
-			ProjectedObstruction projected_obstruction;
+	traversable_outlines.append_array(other_traversable_outlines);
-			projected_obstruction.vertices.resize(other_projected_obstruction.vertices.size());
+	obstruction_outlines.append_array(other_obstruction_outlines);
-
+	_projected_obstructions.append_array(other_projected_obstructions);
-			const float *other_obstruction_vertices_ptr = other_projected_obstruction.vertices.ptr();
-			float *obstruction_vertices_ptrw = projected_obstruction.vertices.ptrw();
-
-			for (int j = 0; j < other_projected_obstruction.vertices.size(); j++) {
-				obstruction_vertices_ptrw[j] = other_obstruction_vertices_ptr[j];
-			}
-
-			projected_obstruction.carve = other_projected_obstruction.carve;
-
-			_projected_obstructions.push_back(projected_obstruction);
-		}
-	}
 }
 
 void NavigationMeshSourceGeometryData2D::add_projected_obstruction(const Vector<Vector2> &p_vertices, bool p_carve) {
@@ -248,6 +261,20 @@ bool NavigationMeshSourceGeometryData2D::_get(const StringName &p_name, Variant
 	return false;
 }
 
+void NavigationMeshSourceGeometryData2D::set_data(const Vector<Vector<Vector2>> &p_traversable_outlines, const Vector<Vector<Vector2>> &p_obstruction_outlines, Vector<ProjectedObstruction> &p_projected_obstructions) {
+	RWLockWrite write_lock(geometry_rwlock);
+	traversable_outlines = p_traversable_outlines;
+	obstruction_outlines = p_obstruction_outlines;
+	_projected_obstructions = p_projected_obstructions;
+}
+
+void NavigationMeshSourceGeometryData2D::get_data(Vector<Vector<Vector2>> &r_traversable_outlines, Vector<Vector<Vector2>> &r_obstruction_outlines, Vector<ProjectedObstruction> &r_projected_obstructions) {
+	RWLockRead read_lock(geometry_rwlock);
+	r_traversable_outlines = traversable_outlines;
+	r_obstruction_outlines = obstruction_outlines;
+	r_projected_obstructions = _projected_obstructions;
+}
+
 void NavigationMeshSourceGeometryData2D::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("clear"), &NavigationMeshSourceGeometryData2D::clear);
 	ClassDB::bind_method(D_METHOD("has_data"), &NavigationMeshSourceGeometryData2D::has_data);

scene/resources/2d/navigation_mesh_source_geometry_data_2d.h

@@ -62,10 +62,10 @@ public:
 	};
 
 	void _set_traversable_outlines(const Vector<Vector<Vector2>> &p_traversable_outlines);
-	const Vector<Vector<Vector2>> &_get_traversable_outlines() const { return traversable_outlines; }
+	const Vector<Vector<Vector2>> &_get_traversable_outlines() const;
 
 	void _set_obstruction_outlines(const Vector<Vector<Vector2>> &p_obstruction_outlines);
-	const Vector<Vector<Vector2>> &_get_obstruction_outlines() const { return obstruction_outlines; }
+	const Vector<Vector<Vector2>> &_get_obstruction_outlines() const;
 
 	void _add_traversable_outline(const Vector<Vector2> &p_shape_outline);
 	void _add_obstruction_outline(const Vector<Vector2> &p_shape_outline);
@@ -88,7 +88,7 @@ public:
 	void add_traversable_outline(const PackedVector2Array &p_shape_outline);
 	void add_obstruction_outline(const PackedVector2Array &p_shape_outline);
 
-	bool has_data() { return traversable_outlines.size(); };
+	bool has_data();
 	void clear();
 	void clear_projected_obstructions();
 
@@ -100,6 +100,9 @@ public:
 
 	void merge(const Ref<NavigationMeshSourceGeometryData2D> &p_other_geometry);
 
+	void set_data(const Vector<Vector<Vector2>> &p_traversable_outlines, const Vector<Vector<Vector2>> &p_obstruction_outlines, Vector<ProjectedObstruction> &p_projected_obstructions);
+	void get_data(Vector<Vector<Vector2>> &r_traversable_outlines, Vector<Vector<Vector2>> &r_obstruction_outlines, Vector<ProjectedObstruction> &r_projected_obstructions);
+
 	NavigationMeshSourceGeometryData2D() {}
 	~NavigationMeshSourceGeometryData2D() { clear(); }
 };

scene/resources/3d/navigation_mesh_source_geometry_data_3d.cpp

@@ -31,14 +31,26 @@
 #include "navigation_mesh_source_geometry_data_3d.h"
 
 void NavigationMeshSourceGeometryData3D::set_vertices(const Vector<float> &p_vertices) {
+	RWLockWrite write_lock(geometry_rwlock);
 	vertices = p_vertices;
 }
 
+const Vector<float> &NavigationMeshSourceGeometryData3D::get_vertices() const {
+	RWLockRead read_lock(geometry_rwlock);
+	return vertices;
+}
+
 void NavigationMeshSourceGeometryData3D::set_indices(const Vector<int> &p_indices) {
 	ERR_FAIL_COND(vertices.size() < p_indices.size());
+	RWLockWrite write_lock(geometry_rwlock);
 	indices = p_indices;
 }
 
+const Vector<int> &NavigationMeshSourceGeometryData3D::get_indices() const {
+	RWLockRead read_lock(geometry_rwlock);
+	return indices;
+}
+
 void NavigationMeshSourceGeometryData3D::append_arrays(const Vector<float> &p_vertices, const Vector<int> &p_indices) {
 	RWLockWrite write_lock(geometry_rwlock);
 
@@ -53,10 +65,16 @@ void NavigationMeshSourceGeometryData3D::append_arrays(const Vector<float> &p_ve
 	}
 }
 
+bool NavigationMeshSourceGeometryData3D::has_data() {
+	RWLockRead read_lock(geometry_rwlock);
+	return vertices.size() && indices.size();
+};
+
 void NavigationMeshSourceGeometryData3D::clear() {
+	RWLockWrite write_lock(geometry_rwlock);
 	vertices.clear();
 	indices.clear();
-	clear_projected_obstructions();
+	_projected_obstructions.clear();
 }
 
 void NavigationMeshSourceGeometryData3D::clear_projected_obstructions() {
@@ -187,40 +205,37 @@ void NavigationMeshSourceGeometryData3D::add_mesh(const Ref<Mesh> &p_mesh, const
 
 void NavigationMeshSourceGeometryData3D::add_mesh_array(const Array &p_mesh_array, const Transform3D &p_xform) {
 	ERR_FAIL_COND(p_mesh_array.size() != Mesh::ARRAY_MAX);
+	RWLockWrite write_lock(geometry_rwlock);
 	_add_mesh_array(p_mesh_array, root_node_transform * p_xform);
 }
 
 void NavigationMeshSourceGeometryData3D::add_faces(const PackedVector3Array &p_faces, const Transform3D &p_xform) {
 	ERR_FAIL_COND(p_faces.size() % 3 != 0);
+	RWLockWrite write_lock(geometry_rwlock);
 	_add_faces(p_faces, root_node_transform * p_xform);
 }
 
 void NavigationMeshSourceGeometryData3D::merge(const Ref<NavigationMeshSourceGeometryData3D> &p_other_geometry) {
 	ERR_FAIL_NULL(p_other_geometry);
 
-	append_arrays(p_other_geometry->vertices, p_other_geometry->indices);
+	Vector<float> other_vertices;
+	Vector<int> other_indices;
+	Vector<ProjectedObstruction> other_projected_obstructions;
 
-	if (p_other_geometry->_projected_obstructions.size() > 0) {
+	p_other_geometry->get_data(other_vertices, other_indices, other_projected_obstructions);
-		RWLockWrite write_lock(geometry_rwlock);
 
-		for (const ProjectedObstruction &other_projected_obstruction : p_other_geometry->_projected_obstructions) {
+	RWLockWrite write_lock(geometry_rwlock);
-			ProjectedObstruction projected_obstruction;
+	const int64_t number_of_vertices_before_merge = vertices.size();
-			projected_obstruction.vertices.resize(other_projected_obstruction.vertices.size());
+	const int64_t number_of_indices_before_merge = indices.size();
 
-			const float *other_obstruction_vertices_ptr = other_projected_obstruction.vertices.ptr();
+	vertices.append_array(other_vertices);
-			float *obstruction_vertices_ptrw = projected_obstruction.vertices.ptrw();
+	indices.append_array(other_indices);
 
-			for (int j = 0; j < other_projected_obstruction.vertices.size(); j++) {
+	for (int64_t i = number_of_indices_before_merge; i < indices.size(); i++) {
-				obstruction_vertices_ptrw[j] = other_obstruction_vertices_ptr[j];
+		indices.set(i, indices[i] + number_of_vertices_before_merge / 3);
-			}
-
-			projected_obstruction.elevation = other_projected_obstruction.elevation;
-			projected_obstruction.height = other_projected_obstruction.height;
-			projected_obstruction.carve = other_projected_obstruction.carve;
-
-			_projected_obstructions.push_back(projected_obstruction);
-		}
 	}
+
+	_projected_obstructions.append_array(other_projected_obstructions);
 }
 
 void NavigationMeshSourceGeometryData3D::add_projected_obstruction(const Vector<Vector3> &p_vertices, float p_elevation, float p_height, bool p_carve) {
@@ -316,6 +331,20 @@ bool NavigationMeshSourceGeometryData3D::_get(const StringName &p_name, Variant
 	return false;
 }
 
+void NavigationMeshSourceGeometryData3D::set_data(const Vector<float> &p_vertices, const Vector<int> &p_indices, Vector<ProjectedObstruction> &p_projected_obstructions) {
+	RWLockWrite write_lock(geometry_rwlock);
+	vertices = p_vertices;
+	indices = p_indices;
+	_projected_obstructions = p_projected_obstructions;
+}
+
+void NavigationMeshSourceGeometryData3D::get_data(Vector<float> &r_vertices, Vector<int> &r_indices, Vector<ProjectedObstruction> &r_projected_obstructions) {
+	RWLockRead read_lock(geometry_rwlock);
+	r_vertices = vertices;
+	r_indices = indices;
+	r_projected_obstructions = _projected_obstructions;
+}
+
 void NavigationMeshSourceGeometryData3D::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("set_vertices", "vertices"), &NavigationMeshSourceGeometryData3D::set_vertices);
 	ClassDB::bind_method(D_METHOD("get_vertices"), &NavigationMeshSourceGeometryData3D::get_vertices);

scene/resources/3d/navigation_mesh_source_geometry_data_3d.h

@@ -75,14 +75,14 @@ public:
 	Transform3D root_node_transform;
 
 	void set_vertices(const Vector<float> &p_vertices);
-	const Vector<float> &get_vertices() const { return vertices; }
+	const Vector<float> &get_vertices() const;
 
 	void set_indices(const Vector<int> &p_indices);
-	const Vector<int> &get_indices() const { return indices; }
+	const Vector<int> &get_indices() const;
 
 	void append_arrays(const Vector<float> &p_vertices, const Vector<int> &p_indices);
 
-	bool has_data() { return vertices.size() && indices.size(); };
+	bool has_data();
 	void clear();
 	void clear_projected_obstructions();
 
@@ -98,6 +98,9 @@ public:
 	void set_projected_obstructions(const Array &p_array);
 	Array get_projected_obstructions() const;
 
+	void set_data(const Vector<float> &p_vertices, const Vector<int> &p_indices, Vector<ProjectedObstruction> &p_projected_obstructions);
+	void get_data(Vector<float> &r_vertices, Vector<int> &r_indices, Vector<ProjectedObstruction> &r_projected_obstructions);
+
 	NavigationMeshSourceGeometryData3D() {}
 	~NavigationMeshSourceGeometryData3D() { clear(); }
 };