From d4722b9e1fcd88bc291fb2cfb3b2dbb94f8114b4 Mon Sep 17 00:00:00 2001 From: smix8 <52464204+smix8@users.noreply.github.com> Date: Thu, 20 Jun 2024 20:01:04 +0200 Subject: [PATCH] Fix thread-use causing navigation source geometry data corruption Fixes navigation source geometry data corruption caused by thread-use that changed vertices or indices while the source geometry data was used in a parsing process or read from by the navmesh baking. --- .../navigation/2d/nav_mesh_generator_2d.cpp | 13 ++-- .../navigation/3d/nav_mesh_generator_3d.cpp | 22 +++--- ...avigation_mesh_source_geometry_data_2d.cpp | 71 +++++++++++++------ .../navigation_mesh_source_geometry_data_2d.h | 9 ++- ...avigation_mesh_source_geometry_data_3d.cpp | 67 ++++++++++++----- .../navigation_mesh_source_geometry_data_3d.h | 9 ++- 6 files changed, 131 insertions(+), 60 deletions(-) diff --git a/modules/navigation/2d/nav_mesh_generator_2d.cpp b/modules/navigation/2d/nav_mesh_generator_2d.cpp index ace361a08a8d..aa4c7977237f 100644 --- a/modules/navigation/2d/nav_mesh_generator_2d.cpp +++ b/modules/navigation/2d/nav_mesh_generator_2d.cpp @@ -852,8 +852,15 @@ void NavMeshGenerator2D::generator_bake_from_source_geometry_data(Refget_outline_count(); - const Vector> &traversable_outlines = p_source_geometry_data->_get_traversable_outlines(); - const Vector> &obstruction_outlines = p_source_geometry_data->_get_obstruction_outlines(); + + Vector> traversable_outlines; + Vector> obstruction_outlines; + Vector 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 &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) { diff --git a/modules/navigation/3d/nav_mesh_generator_3d.cpp b/modules/navigation/3d/nav_mesh_generator_3d.cpp index df0bdc953764..a11284f6be7e 100644 --- a/modules/navigation/3d/nav_mesh_generator_3d.cpp +++ b/modules/navigation/3d/nav_mesh_generator_3d.cpp @@ -672,10 +672,16 @@ void NavMeshGenerator3D::generator_bake_from_source_geometry_data(Ref &vertices = p_source_geometry_data->get_vertices(); - const Vector &indices = p_source_geometry_data->get_indices(); + Vector source_geometry_vertices; + Vector source_geometry_indices; + Vector 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 &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) { diff --git a/scene/resources/2d/navigation_mesh_source_geometry_data_2d.cpp b/scene/resources/2d/navigation_mesh_source_geometry_data_2d.cpp index aee743ccf249..686560829be2 100644 --- a/scene/resources/2d/navigation_mesh_source_geometry_data_2d.cpp +++ b/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> &p_traversable_outlines) { + RWLockWrite write_lock(geometry_rwlock); traversable_outlines = p_traversable_outlines; } void NavigationMeshSourceGeometryData2D::_set_obstruction_outlines(const Vector> &p_obstruction_outlines) { + RWLockWrite write_lock(geometry_rwlock); obstruction_outlines = p_obstruction_outlines; } +const Vector> &NavigationMeshSourceGeometryData2D::_get_traversable_outlines() const { + RWLockRead read_lock(geometry_rwlock); + return traversable_outlines; +} + +const Vector> &NavigationMeshSourceGeometryData2D::_get_obstruction_outlines() const { + RWLockRead read_lock(geometry_rwlock); + return obstruction_outlines; +} + void NavigationMeshSourceGeometryData2D::_add_traversable_outline(const Vector &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 &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> &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> NavigationMeshSourceGeometryData2D::get_traversable_outlines() const { + RWLockRead read_lock(geometry_rwlock); TypedArray> 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> NavigationMeshSourceGeometryData2D::get_traversable_ } void NavigationMeshSourceGeometryData2D::set_obstruction_outlines(const TypedArray> &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> NavigationMeshSourceGeometryData2D::get_obstruction_outlines() const { + RWLockRead read_lock(geometry_rwlock); TypedArray> 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 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 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 &p_other_geometry) { ERR_FAIL_NULL(p_other_geometry); - // No need to worry about `root_node_transform` here as the data is already xformed. - traversable_outlines.append_array(p_other_geometry->traversable_outlines); - obstruction_outlines.append_array(p_other_geometry->obstruction_outlines); + Vector> other_traversable_outlines; + Vector> other_obstruction_outlines; + Vector other_projected_obstructions; - if (p_other_geometry->_projected_obstructions.size() > 0) { - RWLockWrite write_lock(geometry_rwlock); + p_other_geometry->get_data(other_traversable_outlines, other_obstruction_outlines, other_projected_obstructions); - for (const ProjectedObstruction &other_projected_obstruction : p_other_geometry->_projected_obstructions) { - ProjectedObstruction projected_obstruction; - projected_obstruction.vertices.resize(other_projected_obstruction.vertices.size()); - - 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); - } - } + RWLockWrite write_lock(geometry_rwlock); + traversable_outlines.append_array(other_traversable_outlines); + obstruction_outlines.append_array(other_obstruction_outlines); + _projected_obstructions.append_array(other_projected_obstructions); } void NavigationMeshSourceGeometryData2D::add_projected_obstruction(const Vector &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> &p_traversable_outlines, const Vector> &p_obstruction_outlines, Vector &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> &r_traversable_outlines, Vector> &r_obstruction_outlines, Vector &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); diff --git a/scene/resources/2d/navigation_mesh_source_geometry_data_2d.h b/scene/resources/2d/navigation_mesh_source_geometry_data_2d.h index aaa02ab40ec0..01e97eee48e3 100644 --- a/scene/resources/2d/navigation_mesh_source_geometry_data_2d.h +++ b/scene/resources/2d/navigation_mesh_source_geometry_data_2d.h @@ -62,10 +62,10 @@ public: }; void _set_traversable_outlines(const Vector> &p_traversable_outlines); - const Vector> &_get_traversable_outlines() const { return traversable_outlines; } + const Vector> &_get_traversable_outlines() const; void _set_obstruction_outlines(const Vector> &p_obstruction_outlines); - const Vector> &_get_obstruction_outlines() const { return obstruction_outlines; } + const Vector> &_get_obstruction_outlines() const; void _add_traversable_outline(const Vector &p_shape_outline); void _add_obstruction_outline(const Vector &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 &p_other_geometry); + void set_data(const Vector> &p_traversable_outlines, const Vector> &p_obstruction_outlines, Vector &p_projected_obstructions); + void get_data(Vector> &r_traversable_outlines, Vector> &r_obstruction_outlines, Vector &r_projected_obstructions); + NavigationMeshSourceGeometryData2D() {} ~NavigationMeshSourceGeometryData2D() { clear(); } }; diff --git a/scene/resources/3d/navigation_mesh_source_geometry_data_3d.cpp b/scene/resources/3d/navigation_mesh_source_geometry_data_3d.cpp index 3f3f8b44fdd2..4c9f381abadb 100644 --- a/scene/resources/3d/navigation_mesh_source_geometry_data_3d.cpp +++ b/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 &p_vertices) { + RWLockWrite write_lock(geometry_rwlock); vertices = p_vertices; } +const Vector &NavigationMeshSourceGeometryData3D::get_vertices() const { + RWLockRead read_lock(geometry_rwlock); + return vertices; +} + void NavigationMeshSourceGeometryData3D::set_indices(const Vector &p_indices) { ERR_FAIL_COND(vertices.size() < p_indices.size()); + RWLockWrite write_lock(geometry_rwlock); indices = p_indices; } +const Vector &NavigationMeshSourceGeometryData3D::get_indices() const { + RWLockRead read_lock(geometry_rwlock); + return indices; +} + void NavigationMeshSourceGeometryData3D::append_arrays(const Vector &p_vertices, const Vector &p_indices) { RWLockWrite write_lock(geometry_rwlock); @@ -53,10 +65,16 @@ void NavigationMeshSourceGeometryData3D::append_arrays(const Vector &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 &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 &p_other_geometry) { ERR_FAIL_NULL(p_other_geometry); - append_arrays(p_other_geometry->vertices, p_other_geometry->indices); + Vector other_vertices; + Vector other_indices; + Vector other_projected_obstructions; - if (p_other_geometry->_projected_obstructions.size() > 0) { - RWLockWrite write_lock(geometry_rwlock); + p_other_geometry->get_data(other_vertices, other_indices, other_projected_obstructions); - for (const ProjectedObstruction &other_projected_obstruction : p_other_geometry->_projected_obstructions) { - ProjectedObstruction projected_obstruction; - projected_obstruction.vertices.resize(other_projected_obstruction.vertices.size()); + RWLockWrite write_lock(geometry_rwlock); + const int64_t number_of_vertices_before_merge = vertices.size(); + const int64_t number_of_indices_before_merge = indices.size(); - const float *other_obstruction_vertices_ptr = other_projected_obstruction.vertices.ptr(); - float *obstruction_vertices_ptrw = projected_obstruction.vertices.ptrw(); + vertices.append_array(other_vertices); + indices.append_array(other_indices); - for (int j = 0; j < other_projected_obstruction.vertices.size(); j++) { - obstruction_vertices_ptrw[j] = other_obstruction_vertices_ptr[j]; - } - - 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); - } + for (int64_t i = number_of_indices_before_merge; i < indices.size(); i++) { + indices.set(i, indices[i] + number_of_vertices_before_merge / 3); } + + _projected_obstructions.append_array(other_projected_obstructions); } void NavigationMeshSourceGeometryData3D::add_projected_obstruction(const Vector &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 &p_vertices, const Vector &p_indices, Vector &p_projected_obstructions) { + RWLockWrite write_lock(geometry_rwlock); + vertices = p_vertices; + indices = p_indices; + _projected_obstructions = p_projected_obstructions; +} + +void NavigationMeshSourceGeometryData3D::get_data(Vector &r_vertices, Vector &r_indices, Vector &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); diff --git a/scene/resources/3d/navigation_mesh_source_geometry_data_3d.h b/scene/resources/3d/navigation_mesh_source_geometry_data_3d.h index 6c1ca760ea73..a8e613a51dec 100644 --- a/scene/resources/3d/navigation_mesh_source_geometry_data_3d.h +++ b/scene/resources/3d/navigation_mesh_source_geometry_data_3d.h @@ -75,14 +75,14 @@ public: Transform3D root_node_transform; void set_vertices(const Vector &p_vertices); - const Vector &get_vertices() const { return vertices; } + const Vector &get_vertices() const; void set_indices(const Vector &p_indices); - const Vector &get_indices() const { return indices; } + const Vector &get_indices() const; void append_arrays(const Vector &p_vertices, const Vector &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 &p_vertices, const Vector &p_indices, Vector &p_projected_obstructions); + void get_data(Vector &r_vertices, Vector &r_indices, Vector &r_projected_obstructions); + NavigationMeshSourceGeometryData3D() {} ~NavigationMeshSourceGeometryData3D() { clear(); } };