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

Fix thread-use causing navigation polygon data corruption #93426

Merged
merged 1 commit into from
Jun 22, 2024
Merged

Fix thread-use causing navigation polygon data corruption #93426

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
12 changes: 3 additions & 9 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 @@ -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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading