Skip to content

Commit

Permalink
Fix thread-use causing navigation polygon data corruption
Browse files Browse the repository at this point in the history 
Fixes navigation polygon  data corruption caused by thread-use that changed vertices or polygons while the navigation polygon was processed, e.g. by server region sync, navmesh baking or user thread updates.
  • Loading branch information
@smix8
smix8 committed Jun 21, 2024
1 parent 04a530f commit eba3aca
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 13 deletions.
12 changes: 3 additions & 9 deletions modules/navigation/2d/nav_mesh_generator_2d.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1010,8 +1010,7 @@ void NavMeshGenerator2D::generator_bake_from_source_geometry_data(Ref<Navigation
}

if (new_baked_outlines.size() == 0) {
p_navigation_mesh->set_vertices(Vector<Vector2>());
p_navigation_mesh->clear_polygons();
p_navigation_mesh->clear();
return;
}

Expand Down Expand Up @@ -1045,8 +1044,7 @@ void NavMeshGenerator2D::generator_bake_from_source_geometry_data(Ref<Navigation
TPPLPartition tpart;
if (tpart.ConvexPartition_HM(&tppl_in_polygon, &tppl_out_polygon) == 0) { //failed!
ERR_PRINT("NavigationPolygon Convex partition failed. Unable to create a valid NavigationMesh from defined polygon outline paths.");
p_navigation_mesh->set_vertices(Vector<Vector2>());
p_navigation_mesh->clear_polygons();
p_navigation_mesh->clear();
return;
}

Expand All @@ -1071,11 +1069,7 @@ void NavMeshGenerator2D::generator_bake_from_source_geometry_data(Ref<Navigation
new_polygons.push_back(new_polygon);
}

p_navigation_mesh->set_vertices(new_vertices);
p_navigation_mesh->clear_polygons();
for (int i = 0; i < new_polygons.size(); i++) {
p_navigation_mesh->add_polygon(new_polygons[i]);
}
p_navigation_mesh->set_data(new_vertices, new_polygons);
}

#endif // CLIPPER2_ENABLED
4 changes: 2 additions & 2 deletions modules/navigation/nav_region.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,11 @@ void NavRegion::set_transform(Transform3D p_transform) {

void NavRegion::set_navigation_mesh(Ref<NavigationMesh> p_navigation_mesh) {
#ifdef DEBUG_ENABLED
if (map && !Math::is_equal_approx(double(map->get_cell_size()), double(p_navigation_mesh->get_cell_size()))) {
if (map && p_navigation_mesh.is_valid() && !Math::is_equal_approx(double(map->get_cell_size()), double(p_navigation_mesh->get_cell_size()))) {
ERR_PRINT_ONCE(vformat("Attempted to update a navigation region with a navigation mesh that uses a `cell_size` of %s while assigned to a navigation map set to a `cell_size` of %s. The cell size for navigation maps can be changed by using the NavigationServer map_set_cell_size() function. The cell size for default navigation maps can also be changed in the ProjectSettings.", double(p_navigation_mesh->get_cell_size()), double(map->get_cell_size())));
}

if (map && !Math::is_equal_approx(double(map->get_cell_height()), double(p_navigation_mesh->get_cell_height()))) {
if (map && p_navigation_mesh.is_valid() && !Math::is_equal_approx(double(map->get_cell_height()), double(p_navigation_mesh->get_cell_height()))) {
ERR_PRINT_ONCE(vformat("Attempted to update a navigation region with a navigation mesh that uses a `cell_height` of %s while assigned to a navigation map set to a `cell_height` of %s. The cell height for navigation maps can be changed by using the NavigationServer map_set_cell_height() function. The cell height for default navigation maps can also be changed in the ProjectSettings.", double(p_navigation_mesh->get_cell_height()), double(map->get_cell_height())));
}
#endif // DEBUG_ENABLED
Expand Down
45 changes: 43 additions & 2 deletions scene/resources/2d/navigation_polygon.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@

#ifdef TOOLS_ENABLED
Rect2 NavigationPolygon::_edit_get_rect() const {
RWLockRead read_lock(rwlock);
if (rect_cache_dirty) {
item_rect = Rect2();
bool first = true;
Expand Down Expand Up @@ -65,6 +66,7 @@ Rect2 NavigationPolygon::_edit_get_rect() const {
}

bool NavigationPolygon::_edit_is_selected_on_click(const Point2 &p_point, double p_tolerance) const {
RWLockRead read_lock(rwlock);
for (int i = 0; i < outlines.size(); i++) {
const Vector<Vector2> &outline = outlines[i];
const int outline_size = outline.size();
Expand All @@ -80,6 +82,7 @@ bool NavigationPolygon::_edit_is_selected_on_click(const Point2 &p_point, double
#endif

void NavigationPolygon::set_vertices(const Vector<Vector2> &p_vertices) {
RWLockWrite write_lock(rwlock);
{
MutexLock lock(navigation_mesh_generation);
navigation_mesh.unref();
Expand All @@ -89,10 +92,12 @@ void NavigationPolygon::set_vertices(const Vector<Vector2> &p_vertices) {
}

Vector<Vector2> NavigationPolygon::get_vertices() const {
RWLockRead read_lock(rwlock);
return vertices;
}

void NavigationPolygon::_set_polygons(const TypedArray<Vector<int32_t>> &p_array) {
RWLockWrite write_lock(rwlock);
{
MutexLock lock(navigation_mesh_generation);
navigation_mesh.unref();
Expand All @@ -104,6 +109,7 @@ void NavigationPolygon::_set_polygons(const TypedArray<Vector<int32_t>> &p_array
}

TypedArray<Vector<int32_t>> NavigationPolygon::_get_polygons() const {
RWLockRead read_lock(rwlock);
TypedArray<Vector<int32_t>> ret;
ret.resize(polygons.size());
for (int i = 0; i < ret.size(); i++) {
Expand All @@ -114,6 +120,7 @@ TypedArray<Vector<int32_t>> NavigationPolygon::_get_polygons() const {
}

void NavigationPolygon::_set_outlines(const TypedArray<Vector<Vector2>> &p_array) {
RWLockWrite write_lock(rwlock);
outlines.resize(p_array.size());
for (int i = 0; i < p_array.size(); i++) {
outlines.write[i] = p_array[i];
Expand All @@ -122,6 +129,7 @@ void NavigationPolygon::_set_outlines(const TypedArray<Vector<Vector2>> &p_array
}

TypedArray<Vector<Vector2>> NavigationPolygon::_get_outlines() const {
RWLockRead read_lock(rwlock);
TypedArray<Vector<Vector2>> ret;
ret.resize(outlines.size());
for (int i = 0; i < ret.size(); i++) {
Expand All @@ -132,6 +140,7 @@ TypedArray<Vector<Vector2>> NavigationPolygon::_get_outlines() const {
}

void NavigationPolygon::add_polygon(const Vector<int> &p_polygon) {
RWLockWrite write_lock(rwlock);
Polygon polygon;
polygon.indices = p_polygon;
polygons.push_back(polygon);
Expand All @@ -142,20 +151,24 @@ void NavigationPolygon::add_polygon(const Vector<int> &p_polygon) {
}

void NavigationPolygon::add_outline_at_index(const Vector<Vector2> &p_outline, int p_index) {
RWLockWrite write_lock(rwlock);
outlines.insert(p_index, p_outline);
rect_cache_dirty = true;
}

int NavigationPolygon::get_polygon_count() const {
RWLockRead read_lock(rwlock);
return polygons.size();
}

Vector<int> NavigationPolygon::get_polygon(int p_idx) {
RWLockRead read_lock(rwlock);
ERR_FAIL_INDEX_V(p_idx, polygons.size(), Vector<int>());
return polygons[p_idx].indices;
}

void NavigationPolygon::clear_polygons() {
RWLockWrite write_lock(rwlock);
polygons.clear();
{
MutexLock lock(navigation_mesh_generation);
Expand All @@ -164,6 +177,7 @@ void NavigationPolygon::clear_polygons() {
}

void NavigationPolygon::clear() {
RWLockWrite write_lock(rwlock);
polygons.clear();
vertices.clear();
{
Expand All @@ -172,12 +186,31 @@ void NavigationPolygon::clear() {
}
}

void NavigationPolygon::set_data(const Vector<Vector2> &p_vertices, const Vector<Vector<int>> &p_polygons) {
RWLockWrite write_lock(rwlock);
vertices = p_vertices;
polygons.resize(p_polygons.size());
for (int i = 0; i < p_polygons.size(); i++) {
polygons.write[i].indices = p_polygons[i];
}
}

void NavigationPolygon::get_data(Vector<Vector2> &r_vertices, Vector<Vector<int>> &r_polygons) {
RWLockRead read_lock(rwlock);
r_vertices = vertices;
r_polygons.resize(polygons.size());
for (int i = 0; i < polygons.size(); i++) {
r_polygons.write[i] = polygons[i].indices;
}
}

Ref<NavigationMesh> NavigationPolygon::get_navigation_mesh() {
MutexLock lock(navigation_mesh_generation);

if (navigation_mesh.is_null()) {
navigation_mesh.instantiate();
Vector<Vector3> verts;
Vector<Vector<int>> polys;
{
verts.resize(get_vertices().size());
Vector3 *w = verts.ptrw();
Expand All @@ -188,50 +221,58 @@ Ref<NavigationMesh> NavigationPolygon::get_navigation_mesh() {
w[i] = Vector3(r[i].x, 0.0, r[i].y);
}
}
navigation_mesh->set_vertices(verts);

for (int i(0); i < get_polygon_count(); i++) {
navigation_mesh->add_polygon(get_polygon(i));
polys.push_back(get_polygon(i));
}

navigation_mesh->set_data(verts, polys);
navigation_mesh->set_cell_size(cell_size); // Needed to not fail the cell size check on the server
}

return navigation_mesh;
}

void NavigationPolygon::add_outline(const Vector<Vector2> &p_outline) {
RWLockWrite write_lock(rwlock);
outlines.push_back(p_outline);
rect_cache_dirty = true;
}

int NavigationPolygon::get_outline_count() const {
RWLockRead read_lock(rwlock);
return outlines.size();
}

void NavigationPolygon::set_outline(int p_idx, const Vector<Vector2> &p_outline) {
RWLockWrite write_lock(rwlock);
ERR_FAIL_INDEX(p_idx, outlines.size());
outlines.write[p_idx] = p_outline;
rect_cache_dirty = true;
}

void NavigationPolygon::remove_outline(int p_idx) {
RWLockWrite write_lock(rwlock);
ERR_FAIL_INDEX(p_idx, outlines.size());
outlines.remove_at(p_idx);
rect_cache_dirty = true;
}

Vector<Vector2> NavigationPolygon::get_outline(int p_idx) const {
RWLockRead read_lock(rwlock);
ERR_FAIL_INDEX_V(p_idx, outlines.size(), Vector<Vector2>());
return outlines[p_idx];
}

void NavigationPolygon::clear_outlines() {
RWLockWrite write_lock(rwlock);
outlines.clear();
rect_cache_dirty = true;
}

#ifndef DISABLE_DEPRECATED
void NavigationPolygon::make_polygons_from_outlines() {
RWLockWrite write_lock(rwlock);
WARN_PRINT("Function make_polygons_from_outlines() is deprecated."
"\nUse NavigationServer2D.parse_source_geometry_data() and NavigationServer2D.bake_from_source_geometry_data() instead.");

Expand Down
4 changes: 4 additions & 0 deletions scene/resources/2d/navigation_polygon.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@

class NavigationPolygon : public Resource {
GDCLASS(NavigationPolygon, Resource);
RWLock rwlock;

Vector<Vector2> vertices;
struct Polygon {
Expand Down Expand Up @@ -153,6 +154,9 @@ class NavigationPolygon : public Resource {

void clear();

void set_data(const Vector<Vector2> &p_vertices, const Vector<Vector<int>> &p_polygons);
void get_data(Vector<Vector2> &r_vertices, Vector<Vector<int>> &r_polygons);

NavigationPolygon() {}
~NavigationPolygon() {}
};
Expand Down

0 comments on commit eba3aca

Please sign in to comment.