From c6a922c9dc01c796c69cafec4da709d62ca12647 Mon Sep 17 00:00:00 2001 From: Jane Tournois Date: Mon, 18 May 2020 14:51:42 +0200 Subject: [PATCH 01/11] fix Facet_updater parallel `vertex_to_proj` was not locked and this was causing seg faults --- Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h b/Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h index 3f4421ed3e9..74cd1da3c9a 100644 --- a/Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h +++ b/Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h @@ -1267,6 +1267,7 @@ private: class Facet_updater { + const Self& m_c3t3_helpers; Vertex_set& vertex_to_proj; C3T3& c3t3_; Update_c3t3& c3t3_updater_; @@ -1275,8 +1276,10 @@ private: typedef Facet& reference; typedef const Facet& const_reference; - Facet_updater(C3T3& c3t3, Vertex_set& vertex_to_proj, Update_c3t3& c3t3_updater_) - : vertex_to_proj(vertex_to_proj), c3t3_(c3t3), c3t3_updater_(c3t3_updater_) + Facet_updater(const Self& c3t3_helpers, + C3T3& c3t3, Vertex_set& vertex_to_proj, Update_c3t3& c3t3_updater_) + : m_c3t3_helpers(c3t3_helpers), + vertex_to_proj(vertex_to_proj), c3t3_(c3t3), c3t3_updater_(c3t3_updater_) {} void @@ -1296,9 +1299,9 @@ private: const Vertex_handle& v = f.first->vertex((k+i)&3); if ( c3t3_.in_dimension(v) > 2 ) { - //lock_vertex_to_proj(); + m_c3t3_helpers.lock_vertex_to_proj(); vertex_to_proj.insert(v); - //unlock_vertex_to_proj(); + m_c3t3_helpers.unlock_vertex_to_proj(); } } } @@ -2732,7 +2735,7 @@ rebuild_restricted_delaunay(OutdatedCells& outdated_cells, // Note: ~42% of rebuild_restricted_delaunay time // Facet_vector facets; lock_vertex_to_proj(); - Facet_updater facet_updater(c3t3_,vertex_to_proj, updater); + Facet_updater facet_updater(*this, c3t3_,vertex_to_proj, updater); unlock_vertex_to_proj(); update_facets(outdated_cells_vector, facet_updater); @@ -2761,7 +2764,7 @@ rebuild_restricted_delaunay(OutdatedCells& outdated_cells, // Get facets (returns each canonical facet only once) // Note: ~42% of rebuild_restricted_delaunay time // Facet_vector facets; - Facet_updater facet_updater(c3t3_,vertex_to_proj, updater); + Facet_updater facet_updater(*this, c3t3_,vertex_to_proj, updater); update_facets(outdated_cells, facet_updater); // now we can clear @@ -2949,7 +2952,7 @@ move_point(const Vertex_handle& old_vertex, Cell_vector incident_cells_; incident_cells_.reserve(64); - tr_.incident_cells(old_vertex, std::back_inserter(incident_cells_)); + tr_.incident_cells_threadsafe(old_vertex, std::back_inserter(incident_cells_)); const Weighted_point& position = tr_.point(old_vertex); const Weighted_point& new_position = cwp(translate(cp(position), move)); @@ -3873,7 +3876,7 @@ get_conflict_zone_topo_change(const Vertex_handle& v, // Get triangulation_vertex incident cells : removal conflict zone // TODO: hasn't it already been computed in "perturb_vertex" (when getting the slivers)? // We don't try to lock the incident cells since they've already been locked - tr_.incident_cells(v, removal_conflict_cells); + tr_.incident_cells_threadsafe(v, removal_conflict_cells); // Get conflict_point conflict zone int li=0; From ad9c357f3302ed7f3f43a4a069ea42c1c5218ae0 Mon Sep 17 00:00:00 2001 From: Jane Tournois Date: Wed, 17 Jun 2020 07:13:35 +0200 Subject: [PATCH 02/11] add adjacent_vertices_threadsafe and use it in nearest_power_vertex() --- .../CGAL/Triangulation_data_structure_3.h | 46 +++++++++++++++++++ .../include/CGAL/Regular_triangulation_3.h | 2 +- .../include/CGAL/Triangulation_3.h | 6 +++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/TDS_3/include/CGAL/Triangulation_data_structure_3.h b/TDS_3/include/CGAL/Triangulation_data_structure_3.h index e971ee6a7db..c81a7cab524 100644 --- a/TDS_3/include/CGAL/Triangulation_data_structure_3.h +++ b/TDS_3/include/CGAL/Triangulation_data_structure_3.h @@ -1297,6 +1297,52 @@ public: return adjacent_vertices(v, vertices); } + template + OutputIterator + adjacent_vertices_threadsafe(Vertex_handle v, OutputIterator vertices) const + { + return adjacent_vertices_threadsafe(v, vertices); + } + + template + OutputIterator + adjacent_vertices_threadsafe(Vertex_handle v, OutputIterator vertices, + Filter f = Filter()) const + { + CGAL_triangulation_precondition(v != Vertex_handle()); + CGAL_triangulation_precondition(dimension() >= -1); + CGAL_triangulation_expensive_precondition(is_vertex(v)); + CGAL_triangulation_expensive_precondition(is_valid()); + + if (dimension() == -1) + return vertices; + + if (dimension() == 0) { + Vertex_handle v1 = v->cell()->neighbor(0)->vertex(0); + if (!f(v1)) *vertices++ = v1; + return vertices; + } + + if (dimension() == 1) { + CGAL_triangulation_assertion(number_of_vertices() >= 3); + Cell_handle n0 = v->cell(); + const int index_v_in_n0 = n0->index(v); + CGAL_assume(index_v_in_n0 <= 1); + Cell_handle n1 = n0->neighbor(1 - index_v_in_n0); + const int index_v_in_n1 = n1->index(v); + CGAL_assume(index_v_in_n1 <= 1); + Vertex_handle v1 = n0->vertex(1 - index_v_in_n0); + Vertex_handle v2 = n1->vertex(1 - index_v_in_n1); + if (!f(v1)) *vertices++ = v1; + if (!f(v2)) *vertices++ = v2; + return vertices; + } + return visit_incident_cells_threadsafe< + Vertex_extractor, OutputIterator, Filter, + internal::Has_member_visited::value>, + OutputIterator>(v, vertices, f); + } + template OutputIterator visit_incident_cells(Vertex_handle v, OutputIterator output, Filter f) const diff --git a/Triangulation_3/include/CGAL/Regular_triangulation_3.h b/Triangulation_3/include/CGAL/Regular_triangulation_3.h index a4867cf9946..b93970449ce 100644 --- a/Triangulation_3/include/CGAL/Regular_triangulation_3.h +++ b/Triangulation_3/include/CGAL/Regular_triangulation_3.h @@ -1711,7 +1711,7 @@ nearest_power_vertex(const Bare_point& p, Cell_handle start) const while(true) { Vertex_handle tmp = nearest; - adjacent_vertices(nearest, std::back_inserter(vs)); + adjacent_vertices_threadsafe(nearest, std::back_inserter(vs)); for(typename std::vector::const_iterator vsit = vs.begin(); vsit != vs.end(); ++vsit) tmp = nearest_power_vertex(p, tmp, *vsit); diff --git a/Triangulation_3/include/CGAL/Triangulation_3.h b/Triangulation_3/include/CGAL/Triangulation_3.h index 6941c6c8acf..b412b36471d 100644 --- a/Triangulation_3/include/CGAL/Triangulation_3.h +++ b/Triangulation_3/include/CGAL/Triangulation_3.h @@ -2138,6 +2138,12 @@ public: return _tds.adjacent_vertices(v, vertices); } + template + OutputIterator adjacent_vertices_threadsafe(Vertex_handle v, OutputIterator vertices) const + { + return _tds.adjacent_vertices_threadsafe(v, vertices); + } + template OutputIterator adjacent_vertices_and_cells_3(Vertex_handle v, OutputIterator vertices, std::vector& cells) const From 7cd18cd659d4389f538485c20145e3b589fa80d5 Mon Sep 17 00:00:00 2001 From: Jane Tournois Date: Wed, 17 Jun 2020 13:21:03 +0200 Subject: [PATCH 03/11] unlock after the move, even if there is no topological change to avoid making changes with another thread --- Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h b/Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h index 74cd1da3c9a..c09fc396c76 100644 --- a/Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h +++ b/Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h @@ -3070,11 +3070,12 @@ move_point(const Vertex_handle& old_vertex, lock_outdated_cells(); std::copy(incident_cells_.begin(),incident_cells_.end(), std::inserter(outdated_cells_set, outdated_cells_set.end())); - unlock_outdated_cells(); Vertex_handle new_vertex = move_point_no_topo_change(old_vertex, move, new_position); + unlock_outdated_cells(); + // Don't "unlock_all_elements" here, the caller may need it to do it himself return new_vertex; } From f55ffabbe082e11a979271ed28e2ffe2a4845de4 Mon Sep 17 00:00:00 2001 From: Jane Tournois Date: Wed, 17 Jun 2020 13:36:04 +0200 Subject: [PATCH 04/11] add an assertion in make_canonical it also helps to make the code more explicit --- Triangulation_3/include/CGAL/Triangulation_3.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Triangulation_3/include/CGAL/Triangulation_3.h b/Triangulation_3/include/CGAL/Triangulation_3.h index b412b36471d..795aca2d6a0 100644 --- a/Triangulation_3/include/CGAL/Triangulation_3.h +++ b/Triangulation_3/include/CGAL/Triangulation_3.h @@ -4229,19 +4229,21 @@ make_canonical(Vertex_triple& t) const Vertex_handle tmp; switch(i) { - case 0: return; + case 0: break; case 1: tmp = t.first; t.first = t.second; t.second = t.third; t.third = tmp; - return; + break; default: tmp = t.first; t.first = t.third; t.third = t.second; t.second = tmp; } + + CGAL_assertion(t.first < t.second && t.first < t.third); } template < class GT, class Tds, class Lds > From daaf92d0ac0a2ab219c4a9f4967bd082153dccb5 Mon Sep 17 00:00:00 2001 From: Jane Tournois Date: Thu, 18 Jun 2020 13:48:28 +0200 Subject: [PATCH 05/11] rename make_canonical() to make_canonical_oriented_triple() to make it more explicit --- .../include/CGAL/Triangulation_3.h | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/Triangulation_3/include/CGAL/Triangulation_3.h b/Triangulation_3/include/CGAL/Triangulation_3.h index 795aca2d6a0..a60b57f9676 100644 --- a/Triangulation_3/include/CGAL/Triangulation_3.h +++ b/Triangulation_3/include/CGAL/Triangulation_3.h @@ -1646,7 +1646,7 @@ private: Vertex_handle>::type Vertex_handle_unique_hash_map; Vertex_triple make_vertex_triple(const Facet& f) const; - void make_canonical(Vertex_triple& t) const; + void make_canonical_oriented_triple(Vertex_triple& t) const; template < class VertexRemover > VertexRemover& make_hole_2D(Vertex_handle v, std::list& hole, @@ -4215,7 +4215,7 @@ make_vertex_triple(const Facet& f) const template < class Gt, class Tds, class Lds > void Triangulation_3:: -make_canonical(Vertex_triple& t) const +make_canonical_oriented_triple(Vertex_triple& t) const { int i = (t.first < t.second) ? 0 : 1; if(i==0) @@ -4770,7 +4770,7 @@ make_hole_3D(Vertex_handle v, Cell_handle opp_cit = (*cit)->neighbor(indv); Facet f(opp_cit, opp_cit->index(*cit)); Vertex_triple vt = make_vertex_triple(f); - make_canonical(vt); + make_canonical_oriented_triple(vt); outer_map[vt] = f; for(int i=0; i<4; i++) { @@ -4797,7 +4797,7 @@ make_hole_3D(Vertex_handle v, Cell_handle opp_cit = (*cit)->neighbor(indv); Facet f(opp_cit, opp_cit->index(*cit)); Vertex_triple vt = make_vertex_triple(f); - make_canonical(vt); + make_canonical_oriented_triple(vt); outer_map[vt] = f; for(int i=0; i<4; i++) { @@ -4985,7 +4985,7 @@ remove_3D(Vertex_handle v, VertexRemover& remover) Facet f = std::pair(it,i); Vertex_triple vt_aux = make_vertex_triple(f); Vertex_triple vt(vmap[vt_aux.first], vmap[vt_aux.third], vmap[vt_aux.second]); - make_canonical(vt); + make_canonical_oriented_triple(vt); inner_map[vt]= f; } } @@ -5000,7 +5000,7 @@ remove_3D(Vertex_handle v, VertexRemover& remover) Facet f = std::pair(it,i); Vertex_triple vt_aux = make_vertex_triple(f); Vertex_triple vt(vmap[vt_aux.first], vmap[vt_aux.third], vmap[vt_aux.second]); - make_canonical(vt); + make_canonical_oriented_triple(vt); inner_map[vt]= f; } } @@ -5043,7 +5043,7 @@ remove_3D(Vertex_handle v, VertexRemover& remover) { Facet f = std::pair(new_ch,i); Vertex_triple vt = make_vertex_triple(f); - make_canonical(vt); + make_canonical_oriented_triple(vt); std::swap(vt.second,vt.third); typename Vertex_triple_Facet_map::iterator oit2 = outer_map.find(vt); @@ -5181,7 +5181,7 @@ remove_3D(Vertex_handle v, VertexRemover& remover, Facet f = std::pair(it,i); Vertex_triple vt_aux = make_vertex_triple(f); Vertex_triple vt(vmap[vt_aux.first],vmap[vt_aux.third],vmap[vt_aux.second]); - make_canonical(vt); + make_canonical_oriented_triple(vt); inner_map[vt]= f; } } @@ -5196,7 +5196,7 @@ remove_3D(Vertex_handle v, VertexRemover& remover, Facet f = std::pair(it,i); Vertex_triple vt_aux = make_vertex_triple(f); Vertex_triple vt(vmap[vt_aux.first],vmap[vt_aux.third],vmap[vt_aux.second]); - make_canonical(vt); + make_canonical_oriented_triple(vt); inner_map[vt]= f; } } @@ -5241,7 +5241,7 @@ remove_3D(Vertex_handle v, VertexRemover& remover, { Facet f = std::pair(new_ch,i); Vertex_triple vt = make_vertex_triple(f); - make_canonical(vt); + make_canonical_oriented_triple(vt); std::swap(vt.second,vt.third); typename Vertex_triple_Facet_map::iterator oit2 = outer_map.find(vt); @@ -5485,7 +5485,7 @@ remove_3D(Vertex_handle v, VertexRemover& remover, OutputItCells fit) Facet f = std::pair(it,i); Vertex_triple vt_aux = make_vertex_triple(f); Vertex_triple vt(vmap[vt_aux.first], vmap[vt_aux.third], vmap[vt_aux.second]); - make_canonical(vt); + make_canonical_oriented_triple(vt); inner_map[vt] = f; } } @@ -5499,7 +5499,7 @@ remove_3D(Vertex_handle v, VertexRemover& remover, OutputItCells fit) Facet f = std::pair(it,i); Vertex_triple vt_aux = make_vertex_triple(f); Vertex_triple vt(vmap[vt_aux.first], vmap[vt_aux.third], vmap[vt_aux.second]); - make_canonical(vt); + make_canonical_oriented_triple(vt); inner_map[vt] = f; } } @@ -5546,7 +5546,7 @@ remove_3D(Vertex_handle v, VertexRemover& remover, OutputItCells fit) { Facet f = std::pair(new_ch,i); Vertex_triple vt = make_vertex_triple(f); - make_canonical(vt); + make_canonical_oriented_triple(vt); std::swap(vt.second, vt.third); typename Vertex_triple_Facet_map::iterator oit2 = outer_map.find(vt); if(oit2 == outer_map.end()) @@ -5873,7 +5873,7 @@ move_if_no_collision(Vertex_handle v, const Point& p, Facet f = std::pair(it,i); Vertex_triple vt_aux = make_vertex_triple(f); Vertex_triple vt(vmap[vt_aux.first],vmap[vt_aux.third],vmap[vt_aux.second]); - make_canonical(vt); + make_canonical_oriented_triple(vt); inner_map[vt]= f; } } @@ -5888,7 +5888,7 @@ move_if_no_collision(Vertex_handle v, const Point& p, Facet f = std::pair(it,i); Vertex_triple vt_aux = make_vertex_triple(f); Vertex_triple vt(vmap[vt_aux.first],vmap[vt_aux.third],vmap[vt_aux.second]); - make_canonical(vt); + make_canonical_oriented_triple(vt); inner_map[vt]= f; } } @@ -5934,7 +5934,7 @@ move_if_no_collision(Vertex_handle v, const Point& p, { Facet f = std::pair(new_ch,i); Vertex_triple vt = make_vertex_triple(f); - make_canonical(vt); + make_canonical_oriented_triple(vt); std::swap(vt.second,vt.third); typename Vertex_triple_Facet_map::iterator oit2 = outer_map.find(vt); if(oit2 == outer_map.end()) @@ -6325,7 +6325,7 @@ move_if_no_collision_and_give_new_cells(Vertex_handle v, const Point& p, Facet f = std::pair(it,i); Vertex_triple vt_aux = make_vertex_triple(f); Vertex_triple vt(vmap[vt_aux.first], vmap[vt_aux.third], vmap[vt_aux.second]); - make_canonical(vt); + make_canonical_oriented_triple(vt); inner_map[vt]= f; } } @@ -6340,7 +6340,7 @@ move_if_no_collision_and_give_new_cells(Vertex_handle v, const Point& p, Facet f = std::pair(it,i); Vertex_triple vt_aux = make_vertex_triple(f); Vertex_triple vt(vmap[vt_aux.first], vmap[vt_aux.third], vmap[vt_aux.second]); - make_canonical(vt); + make_canonical_oriented_triple(vt); inner_map[vt]= f; } } @@ -6387,7 +6387,7 @@ move_if_no_collision_and_give_new_cells(Vertex_handle v, const Point& p, { Facet f = std::pair(new_ch, i); Vertex_triple vt = make_vertex_triple(f); - make_canonical(vt); + make_canonical_oriented_triple(vt); std::swap(vt.second,vt.third); typename Vertex_triple_Facet_map::iterator oit2 = outer_map.find(vt); if(oit2 == outer_map.end()) @@ -6519,7 +6519,7 @@ _make_big_hole_3D(Vertex_handle v, Facet f(opp_cit, opp_i); Vertex_triple vt = make_vertex_triple(f); - make_canonical(vt); + make_canonical_oriented_triple(vt); outer_map[vt] = f; v1->set_cell(opp_cit); v2->set_cell(opp_cit); @@ -6664,7 +6664,7 @@ _remove_cluster_3D(InputIterator first, InputIterator beyond, VertexRemover& rem Facet f = std::pair(it,index); Vertex_triple vt_aux = this->make_vertex_triple(f); Vertex_triple vt(vmap[vt_aux.first], vmap[vt_aux.third], vmap[vt_aux.second]); - this->make_canonical(vt); + this->make_canonical_oriented_triple(vt); inner_map[vt]= f; } } @@ -6679,7 +6679,7 @@ _remove_cluster_3D(InputIterator first, InputIterator beyond, VertexRemover& rem Facet f = std::pair(it,index); Vertex_triple vt_aux = this->make_vertex_triple(f); Vertex_triple vt(vmap[vt_aux.first], vmap[vt_aux.third], vmap[vt_aux.second]); - this->make_canonical(vt); + this->make_canonical_oriented_triple(vt); inner_map[vt]= f; } } @@ -6728,7 +6728,7 @@ _remove_cluster_3D(InputIterator first, InputIterator beyond, VertexRemover& rem { Facet f = std::pair(new_ch,index); Vertex_triple vt = this->make_vertex_triple(f); - this->make_canonical(vt); + this->make_canonical_oriented_triple(vt); std::swap(vt.second,vt.third); typename Vertex_triple_Facet_map::iterator oit2 = outer_map.find(vt); if(oit2 == outer_map.end()) From 3b8c06f83631c43b4bce1735d12f51dc729beab1 Mon Sep 17 00:00:00 2001 From: Jane Tournois Date: Thu, 18 Jun 2020 14:23:16 +0200 Subject: [PATCH 06/11] fix adjacent_vertices_threadsafe internal::Has_member_visited is not threadsafe --- TDS_3/include/CGAL/Triangulation_data_structure_3.h | 2 +- Triangulation_3/include/CGAL/Regular_triangulation_3.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/TDS_3/include/CGAL/Triangulation_data_structure_3.h b/TDS_3/include/CGAL/Triangulation_data_structure_3.h index c81a7cab524..1024efa0943 100644 --- a/TDS_3/include/CGAL/Triangulation_data_structure_3.h +++ b/TDS_3/include/CGAL/Triangulation_data_structure_3.h @@ -1339,7 +1339,7 @@ public: } return visit_incident_cells_threadsafe< Vertex_extractor, OutputIterator, Filter, - internal::Has_member_visited::value>, + false>, OutputIterator>(v, vertices, f); } diff --git a/Triangulation_3/include/CGAL/Regular_triangulation_3.h b/Triangulation_3/include/CGAL/Regular_triangulation_3.h index b93970449ce..0bf3b88d919 100644 --- a/Triangulation_3/include/CGAL/Regular_triangulation_3.h +++ b/Triangulation_3/include/CGAL/Regular_triangulation_3.h @@ -147,6 +147,7 @@ public: using Tr_Base::geom_traits; #endif using Tr_Base::adjacent_vertices; + using Tr_Base::adjacent_vertices_threadsafe; using Tr_Base::cw; using Tr_Base::ccw; using Tr_Base::construct_point; From 13c0719e8716de2293e35857e4cb321667edfad8 Mon Sep 17 00:00:00 2001 From: Jane Tournois Date: Thu, 18 Jun 2020 14:24:34 +0200 Subject: [PATCH 07/11] fix incident_edges_threadsafe internal::Has_member_visited::value is not threadsafe --- TDS_3/include/CGAL/Triangulation_data_structure_3.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/TDS_3/include/CGAL/Triangulation_data_structure_3.h b/TDS_3/include/CGAL/Triangulation_data_structure_3.h index 1024efa0943..6c5c7e3ee29 100644 --- a/TDS_3/include/CGAL/Triangulation_data_structure_3.h +++ b/TDS_3/include/CGAL/Triangulation_data_structure_3.h @@ -1225,7 +1225,7 @@ public: return visit_incident_cells_threadsafe< Vertex_extractor, OutputIterator, Filter, - internal::Has_member_visited::value>, + false>, OutputIterator>(v, edges, f); } From d4b7af22baf554a6f67612c081b629bb23e9c911 Mon Sep 17 00:00:00 2001 From: Jane Tournois Date: Fri, 29 May 2020 08:41:26 +0200 Subject: [PATCH 08/11] use tr_.try_lock_and_get_incident_cells() and remove a "todo" of CJ --- Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h | 71 ++--------------------- 1 file changed, 5 insertions(+), 66 deletions(-) diff --git a/Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h b/Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h index c09fc396c76..c642cb960ba 100644 --- a/Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h +++ b/Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h @@ -839,16 +839,6 @@ public: Moving_vertices_set& moving_vertices, bool *could_lock_zone) const; - /** - * Try to lock the incident cells and return them in \c cells - * Return value: - * - false: everything is unlocked and \c cells is empty - * - true: incident cells are locked and \c cells contains all of them - */ - bool - try_lock_and_get_incident_cells(const Vertex_handle& v, - Cell_vector &cells) const; - /** * Try to lock ALL the incident cells and return in \c cells the ones * whose \c filter says "true". @@ -3041,9 +3031,10 @@ move_point(const Vertex_handle& old_vertex, //======= Get incident cells ========== Cell_vector incident_cells_; incident_cells_.reserve(64); - if (try_lock_and_get_incident_cells(old_vertex, incident_cells_) == false) + if (tr_.try_lock_and_get_incident_cells(old_vertex, incident_cells_) == false) { *could_lock_zone = false; + unlock_all_elements(); return Vertex_handle(); } //======= /Get incident cells ========== @@ -3587,60 +3578,6 @@ get_incident_slivers_without_using_tds_data(const Vertex_handle& v, tr_.incident_cells_threadsafe(v, boost::make_function_output_iterator(f)); } -// CJTODO: call tr_.try_lock_and_get_incident_cells instead? -template -bool -C3T3_helpers:: -try_lock_and_get_incident_cells(const Vertex_handle& v, - Cell_vector &cells) const - { - // We need to lock v individually first, to be sure v->cell() is valid - if (!try_lock_vertex(v)) - return false; - - Cell_handle d = v->cell(); - if (!try_lock_element(d)) // LOCK - { - unlock_all_elements(); - return false; - } - cells.push_back(d); - d->tds_data().mark_in_conflict(); - int head=0; - int tail=1; - do { - Cell_handle c = cells[head]; - - for (int i=0; i<4; ++i) { - if (c->vertex(i) == v) - continue; - Cell_handle next = c->neighbor(i); - - if (!try_lock_element(next)) // LOCK - { - for(Cell_handle ch : cells) - { - ch->tds_data().clear(); - } - cells.clear(); - unlock_all_elements(); - return false; - } - if (! next->tds_data().is_clear()) - continue; - cells.push_back(next); - ++tail; - next->tds_data().mark_in_conflict(); - } - ++head; - } while(head != tail); - for(Cell_handle ch : cells) - { - ch->tds_data().clear(); - } - return true; - } - template template bool @@ -3651,7 +3588,7 @@ try_lock_and_get_incident_cells(const Vertex_handle& v, { std::vector tmp_cells; tmp_cells.reserve(64); - bool ret = try_lock_and_get_incident_cells(v, tmp_cells); + bool ret = tr_.try_lock_and_get_incident_cells(v, tmp_cells); if (ret) { for(Cell_handle ch : tmp_cells) @@ -3660,6 +3597,8 @@ try_lock_and_get_incident_cells(const Vertex_handle& v, cells.push_back(ch); } } + else + tr_.unlock_all_elements(); return ret; } From bce4b4e80a93a50de9d00caeb409464fb657128f Mon Sep 17 00:00:00 2001 From: Jane Tournois Date: Fri, 26 Jun 2020 14:57:02 +0200 Subject: [PATCH 09/11] Revert "add an assertion in make_canonical" This reverts commit f55ffabbe082e11a979271ed28e2ffe2a4845de4. In the exuder, it can happen that this function takes the triple (Vertex_handle(), Vertex_handle(), Vertex_handle()) so the assertion does not hold --- Triangulation_3/include/CGAL/Triangulation_3.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Triangulation_3/include/CGAL/Triangulation_3.h b/Triangulation_3/include/CGAL/Triangulation_3.h index a60b57f9676..20f76735e6b 100644 --- a/Triangulation_3/include/CGAL/Triangulation_3.h +++ b/Triangulation_3/include/CGAL/Triangulation_3.h @@ -4229,21 +4229,19 @@ make_canonical_oriented_triple(Vertex_triple& t) const Vertex_handle tmp; switch(i) { - case 0: break; + case 0: return; case 1: tmp = t.first; t.first = t.second; t.second = t.third; t.third = tmp; - break; + return; default: tmp = t.first; t.first = t.third; t.third = t.second; t.second = tmp; } - - CGAL_assertion(t.first < t.second && t.first < t.third); } template < class GT, class Tds, class Lds > From 615ac140866492397295c80bdfb66ac7bcbdeb0c Mon Sep 17 00:00:00 2001 From: Jane Tournois Date: Fri, 26 Jun 2020 15:51:15 +0200 Subject: [PATCH 10/11] protect incident_cells_threadsafe with macro Periodic_3_mesh_3 does not have an implementation of these functions because it does not have a parallel implementation --- Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h b/Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h index c642cb960ba..28cb28456f2 100644 --- a/Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h +++ b/Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h @@ -2942,7 +2942,11 @@ move_point(const Vertex_handle& old_vertex, Cell_vector incident_cells_; incident_cells_.reserve(64); +#ifdef CGAL_LINKED_WITH_TBB tr_.incident_cells_threadsafe(old_vertex, std::back_inserter(incident_cells_)); +#else + tr_.incident_cells(old_vertex, std::back_inserter(incident_cells_)); +#endif const Weighted_point& position = tr_.point(old_vertex); const Weighted_point& new_position = cwp(translate(cp(position), move)); @@ -3816,7 +3820,11 @@ get_conflict_zone_topo_change(const Vertex_handle& v, // Get triangulation_vertex incident cells : removal conflict zone // TODO: hasn't it already been computed in "perturb_vertex" (when getting the slivers)? // We don't try to lock the incident cells since they've already been locked +#ifdef CGAL_LINKED_WITH_TBB tr_.incident_cells_threadsafe(v, removal_conflict_cells); +#else + tr_.incident_cells(v, removal_conflict_cells); +#endif // Get conflict_point conflict zone int li=0; From 40668a297efabbfb29d5f98ea04c290f5a8b3ba8 Mon Sep 17 00:00:00 2001 From: Jane Tournois Date: Mon, 29 Jun 2020 07:07:13 +0200 Subject: [PATCH 11/11] fix protection of parallel code --- Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h | 36 ++++++++++++++++------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h b/Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h index 28cb28456f2..ec092058b1d 100644 --- a/Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h +++ b/Mesh_3/include/CGAL/Mesh_3/C3T3_helpers.h @@ -2942,11 +2942,19 @@ move_point(const Vertex_handle& old_vertex, Cell_vector incident_cells_; incident_cells_.reserve(64); -#ifdef CGAL_LINKED_WITH_TBB - tr_.incident_cells_threadsafe(old_vertex, std::back_inserter(incident_cells_)); -#else - tr_.incident_cells(old_vertex, std::back_inserter(incident_cells_)); -#endif + +# ifdef CGAL_LINKED_WITH_TBB + // Parallel + if (boost::is_convertible::value) + { + tr_.incident_cells_threadsafe(old_vertex, std::back_inserter(incident_cells_)); + } + // Sequential + else +# endif // CGAL_LINKED_WITH_TBB + { + tr_.incident_cells(old_vertex, std::back_inserter(incident_cells_)); + } const Weighted_point& position = tr_.point(old_vertex); const Weighted_point& new_position = cwp(translate(cp(position), move)); @@ -3820,11 +3828,19 @@ get_conflict_zone_topo_change(const Vertex_handle& v, // Get triangulation_vertex incident cells : removal conflict zone // TODO: hasn't it already been computed in "perturb_vertex" (when getting the slivers)? // We don't try to lock the incident cells since they've already been locked -#ifdef CGAL_LINKED_WITH_TBB - tr_.incident_cells_threadsafe(v, removal_conflict_cells); -#else - tr_.incident_cells(v, removal_conflict_cells); -#endif + +# ifdef CGAL_LINKED_WITH_TBB +// Parallel + if (boost::is_convertible::value) + { + tr_.incident_cells_threadsafe(v, removal_conflict_cells); + } + // Sequential + else +# endif // CGAL_LINKED_WITH_TBB + { + tr_.incident_cells(v, removal_conflict_cells); + } // Get conflict_point conflict zone int li=0;