Skip to content

Commit

Permalink
Merge pull request #93407 from smix8/lock_this_geometry_up
Browse files Browse the repository at this point in the history 
Fix thread-use causing navigation source geometry data corruption
  • Loading branch information
@akien-mga
akien-mga committed Jun 21, 2024
2 parents 2e1b651 + d4722b9 commit 04a530f
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 60 deletions.
13 changes: 9 additions & 4 deletions modules/navigation/2d/nav_mesh_generator_2d.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
22 changes: 13 additions & 9 deletions modules/navigation/3d/nav_mesh_generator_3d.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
const Vector<int> &indices = p_source_geometry_data->get_indices();
Vector<float> source_geometry_vertices;
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;
}

Expand All @@ -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 int nverts = vertices.size() / 3;
const int *tris = indices.ptr();
const int ntris = indices.size() / 3;
const float *verts = source_geometry_vertices.ptr();
const int nverts = source_geometry_vertices.size() / 3;
const int *tris = source_geometry_indices.ptr();
const int ntris = source_geometry_indices.size() / 3;

float bmin[3], bmax[3];
rcCalcBounds(verts, nverts, bmin, bmax);
Expand Down Expand Up @@ -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) {
Expand Down
71 changes: 49 additions & 22 deletions scene/resources/2d/navigation_mesh_source_geometry_data_2d.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,44 +33,66 @@
#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];
}
}

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++) {
Expand All @@ -81,13 +103,15 @@ 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];
}
}

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++) {
Expand Down Expand Up @@ -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++) {
Expand All @@ -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++) {
Expand All @@ -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.
traversable_outlines.append_array(p_other_geometry->traversable_outlines);
obstruction_outlines.append_array(p_other_geometry->obstruction_outlines);

if (p_other_geometry->_projected_obstructions.size() > 0) {
RWLockWrite write_lock(geometry_rwlock);

for (const ProjectedObstruction &other_projected_obstruction : p_other_geometry->_projected_obstructions) {
ProjectedObstruction projected_obstruction;
projected_obstruction.vertices.resize(other_projected_obstruction.vertices.size());
Vector<Vector<Vector2>> other_traversable_outlines;
Vector<Vector<Vector2>> other_obstruction_outlines;
Vector<ProjectedObstruction> other_projected_obstructions;

const float *other_obstruction_vertices_ptr = other_projected_obstruction.vertices.ptr();
float *obstruction_vertices_ptrw = projected_obstruction.vertices.ptrw();
p_other_geometry->get_data(other_traversable_outlines, other_obstruction_outlines, other_projected_obstructions);

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<Vector2> &p_vertices, bool p_carve) {
Expand Down Expand Up @@ -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);
Expand Down
9 changes: 6 additions & 3 deletions scene/resources/2d/navigation_mesh_source_geometry_data_2d.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ class NavigationMeshSourceGeometryData2D : public Resource {
};

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);
Expand All @@ -88,7 +88,7 @@ class NavigationMeshSourceGeometryData2D : public Resource {
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();

Expand All @@ -100,6 +100,9 @@ class NavigationMeshSourceGeometryData2D : public Resource {

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(); }
};
Expand Down
67 changes: 48 additions & 19 deletions scene/resources/3d/navigation_mesh_source_geometry_data_3d.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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() {
Expand Down Expand Up @@ -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);

if (p_other_geometry->_projected_obstructions.size() > 0) {
RWLockWrite write_lock(geometry_rwlock);

for (const ProjectedObstruction &other_projected_obstruction : p_other_geometry->_projected_obstructions) {
ProjectedObstruction projected_obstruction;
projected_obstruction.vertices.resize(other_projected_obstruction.vertices.size());
Vector<float> other_vertices;
Vector<int> other_indices;
Vector<ProjectedObstruction> other_projected_obstructions;

const float *other_obstruction_vertices_ptr = other_projected_obstruction.vertices.ptr();
float *obstruction_vertices_ptrw = projected_obstruction.vertices.ptrw();
p_other_geometry->get_data(other_vertices, other_indices, other_projected_obstructions);

for (int j = 0; j < other_projected_obstruction.vertices.size(); j++) {
obstruction_vertices_ptrw[j] = other_obstruction_vertices_ptr[j];
}
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();

projected_obstruction.elevation = other_projected_obstruction.elevation;
projected_obstruction.height = other_projected_obstruction.height;
projected_obstruction.carve = other_projected_obstruction.carve;
vertices.append_array(other_vertices);
indices.append_array(other_indices);

_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<Vector3> &p_vertices, float p_elevation, float p_height, bool p_carve) {
Expand Down Expand Up @@ -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);
Expand Down
Loading

0 comments on commit 04a530f

Please sign in to comment.