Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix thread-use causing navigation source geometry data corruption #93407

Merged
merged 1 commit into from
Jun 21, 2024
Merged

Fix thread-use causing navigation source geometry data corruption #93407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions modules/navigation/2d/nav_mesh_generator_2d.cpp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading