From e0985c4daf9646f91dcbc2e7db1b203fd5d61144 Mon Sep 17 00:00:00 2001 From: Andreas Fabri Date: Thu, 8 Apr 2021 11:40:19 +0100 Subject: [PATCH 1/3] PMP Corefinement: Replace containters to make it faster --- .../internal/Corefinement/intersection_impl.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Corefinement/intersection_impl.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Corefinement/intersection_impl.h index eb59cf0357e2..76774187d3a2 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Corefinement/intersection_impl.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Corefinement/intersection_impl.h @@ -29,6 +29,7 @@ #include #include #include +#include #include @@ -192,7 +193,7 @@ class Intersection_of_triangle_meshes // we use Face_pair_and_int and not Face_pair to handle coplanar case. // Indeed the boundary of the intersection of two coplanar triangles // may contain several segments. - typedef std::map< Face_pair_and_int, Node_id_set > Faces_to_nodes_map; + typedef boost::unordered_map< Face_pair_and_int, Node_id_set > Faces_to_nodes_map; typedef Intersection_nodes Node_vector; @@ -1051,7 +1052,7 @@ class Intersection_of_triangle_meshes } struct Graph_node{ - std::set neighbors; + boost::container::flat_set neighbors; unsigned degree; Graph_node():degree(0){} From 05e4b56c886cae8abd737f994e9f0a28f3bd7726 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Wed, 14 Apr 2021 10:19:38 +0200 Subject: [PATCH 2/3] group intersecting segment of planar faces into a unique Node_set_id This makes the unordered_map change working for autoref --- .../internal/Corefinement/intersection_impl.h | 277 ++++++++++-------- 1 file changed, 156 insertions(+), 121 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Corefinement/intersection_impl.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Corefinement/intersection_impl.h index 76774187d3a2..02c2be886f7d 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Corefinement/intersection_impl.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Corefinement/intersection_impl.h @@ -128,10 +128,18 @@ struct Node_id_set { Node_id second; std::size_t size_; + std::vector > coplanar_segments; + Node_id_set() : size_(0) {} + void insert(std::size_t i, std::size_t j) + { + if (j >& ids) + { + ids.reserve( (size_==2?1:0) + coplanar_segments.size() ); + if (size_==2) + ids.push_back({first, second}); + std::copy(coplanar_segments.begin(), coplanar_segments.end(), std::back_inserter(ids)); + } }; template< class TriangleMesh, @@ -189,11 +205,10 @@ class Intersection_of_triangle_meshes typedef std::size_t Node_id; - typedef std::pair Face_pair_and_int; // we use Face_pair_and_int and not Face_pair to handle coplanar case. // Indeed the boundary of the intersection of two coplanar triangles // may contain several segments. - typedef boost::unordered_map< Face_pair_and_int, Node_id_set > Faces_to_nodes_map; + typedef boost::unordered_map< Face_pair, Node_id_set > Faces_to_nodes_map; typedef Intersection_nodes Node_vector; @@ -455,7 +470,7 @@ class Intersection_of_triangle_meshes ? Face_pair(f_1,f_2) : Face_pair(f_2,f_1); if ( coplanar_faces.count(face_pair)==0 ) - f_to_node[Face_pair_and_int(face_pair,0)].insert(node_id); + f_to_node[face_pair].insert(node_id); } } h_2 = opposite(h_2, tm2); @@ -469,7 +484,7 @@ class Intersection_of_triangle_meshes ? Face_pair(f_1,f_2) : Face_pair(f_2,f_1); if ( coplanar_faces.count(face_pair) == 0 ) - f_to_node[Face_pair_and_int(face_pair,0)].insert(node_id); + f_to_node[face_pair].insert(node_id); } } } @@ -776,17 +791,15 @@ class Intersection_of_triangle_meshes case 0: break; case 1: { - f_to_node[Face_pair_and_int(face_pair,1)].insert(cpln_nodes[0]); + f_to_node[face_pair].insert(cpln_nodes[0]); // TODO: really? } break; default: { + Node_id_set& node_id_set = f_to_node[face_pair]; std::size_t stop=nb_pts + (nb_pts<3?-1:0); - for (std::size_t k=0;k(k+1))]; - node_id_set.insert( cpln_nodes[k] ); - node_id_set.insert( cpln_nodes[(k+1)%nb_pts] ); - } + for (std::size_t k=0;kfirst.first.first, f2 = it->first.first.second; + face_descriptor f1 = it->first.first, f2 = it->first.second; CGAL_assertion( f1 < f2 ); std::vector& inter_faces=face_intersections[f1]; if (inter_faces.empty() || inter_faces.back()!=f2) @@ -1112,14 +1125,14 @@ class Intersection_of_triangle_meshes } // TODO AUTOREF_TAG find a better way to avoid too many queries. - std::map< std::pair< std::pair, int>, - std::vector > map_to_process; + std::map< Node_id_set*, std::vector, Node_id>> > map_to_process; typedef std::pair > Pair_type; for(Pair_type& p : face_intersections) { face_descriptor f1 = p.first; std::vector& inter_faces=p.second; + std::sort(inter_faces.begin(), inter_faces.end()); std::size_t nb_faces = inter_faces.size(); // TODO AUTOREF_TAG handle 4 and more faces intersecting (only 3 right now) @@ -1135,47 +1148,37 @@ class Intersection_of_triangle_meshes // use lower bound to get the first entry which key is equal or larger // the non-coplanar entry case. That way any coplanar entry will be // listed afterward - typename Faces_to_nodes_map::iterator find_it = - f_to_node.lower_bound(std::make_pair(std::make_pair(f2, f3),0)); + typename Faces_to_nodes_map::iterator it_seg3 = + f_to_node.find(std::make_pair(f2, f3)); - if (find_it!=f_to_node.end() && - find_it->first.first.first == f2 && - find_it->first.first.second == f3) + if (it_seg3!=f_to_node.end()) { + std::vector > f2f3_segments; + it_seg3->second.get_segments(f2f3_segments); + // look for edges between f1 and f2 typename Faces_to_nodes_map::iterator it_seg1 = - f_to_node.lower_bound(std::make_pair(std::make_pair(f1, f2),0)); - CGAL_assertion( it_seg1 != f_to_node.end() && - it_seg1->first.first.first == f1 && - it_seg1->first.first.second == f2 ); + f_to_node.find(std::make_pair(f1, f2)); + CGAL_assertion( it_seg1 != f_to_node.end() ); + std::vector > f1f2_segments; + it_seg1->second.get_segments(f1f2_segments); // look for edges between f1 and f3 typename Faces_to_nodes_map::iterator it_seg2 = - f_to_node.lower_bound(std::make_pair(std::make_pair(f1, f3),0)); - CGAL_assertion( it_seg2 != f_to_node.end() && - it_seg2->first.first.first == f1 && - it_seg2->first.first.second == f3 ); - - while(it_seg1 != f_to_node.end() && - it_seg1->first.first.first == f1 && - it_seg1->first.first.second == f2) + f_to_node.find(std::make_pair(f1, f3)); + CGAL_assertion( it_seg2 != f_to_node.end() ); + std::vector > f1f3_segments; + it_seg2->second.get_segments(f1f3_segments); + + /// TODO AUTOREF_TAG shall we ignore tangency points? + /// with the current code, Node_id_set::size()==1 is ignored as we only drop semgents + /// Actually it might be that it is not a tangency point if the third segment was considered! + /// so not handling it is a bug + + for (const std::array& ns1 : f1f2_segments) { - while(it_seg2 != f_to_node.end() && - it_seg2->first.first.first == f1 && - it_seg2->first.first.second == f3 ) + for (const std::array& ns2 : f1f3_segments) { - const Node_id_set& ns1 = it_seg1->second; - const Node_id_set& ns2 = it_seg2->second; - CGAL_assertion (ns1.size()!=0); - CGAL_assertion (ns2.size()!=0); - - if (ns1.size()==1 || ns2.size()==1){ - ++it_seg2; - continue; /// TODO AUTOREF_TAG shall we ignore tangency points? - /// Actually it might be that it is not a tangency point if the third segment was considered! - /// so not handling it is a bug - } - // handle cases of segments sharing an endpoint if (ns1[0]==ns2[0] || ns1[0]==ns2[1] || ns1[1]==ns2[0] || ns1[1]==ns2[1] ) @@ -1229,7 +1232,6 @@ class Intersection_of_triangle_meshes throw Triple_intersection_exception(); } } - ++it_seg2; continue; } @@ -1244,17 +1246,18 @@ class Intersection_of_triangle_meshes /// TODO AUTOREF_TAG it might be the end point of a segment! // we need to find which segment is the third one intersecting // with the point found. - typename Faces_to_nodes_map::iterator it_seg3 = find_it; + // first check if there is only one such edge (no test is needed then) - if (std::next(it_seg3)!=f_to_node.end() && - std::next(it_seg3)->first.first == it_seg3->first.first) + CGAL_assertion(!f2f3_segments.empty() || !"AUTOREF_TAG HANDLE ME"); + + std::array ns3 = f2f3_segments.front(); + + if (f2f3_segments.size()!=1) { - while(true) + std::size_t k=0; + for (;kfirst.first == find_it->first.first); - - CGAL_assertion(it_seg3->second.size()!=1 || !"AUTOREF_TAG HANDLE ME"); - const Node_id_set& ns3 = it_seg3->second; + ns3=f2f3_segments[k]; s3=Segment_3(nodes.exact_node(ns3[0]), nodes.exact_node(ns3[1])); if (do_intersect(s1, s3)) { @@ -1262,16 +1265,14 @@ class Intersection_of_triangle_meshes break; } } + CGAL_assertion(k!=f2f3_segments.size()); } else - { - CGAL_assertion(it_seg3->second.size()==2 || !"AUTOREF_TAG TODO: HANDLE ME"); - s3=Segment_3(nodes.exact_node(it_seg3->second[0]), - nodes.exact_node(it_seg3->second[1])); - } - CGAL_assertion(it_seg3->first.first == find_it->first.first); + s3=Segment_3(nodes.exact_node(ns3[0]), nodes.exact_node(ns3[1])); + + CGAL_assertion(do_intersect(s1, s3)); - // use it_seg3 + // use s3 ///TODO AUTOREF_TAG the collinear test could be factorized in the do-intersect /// TODO AUTOREF_TAG if we don't factorise maybe checking for collinearity with /// cross product would be cheaper? @@ -1295,13 +1296,11 @@ class Intersection_of_triangle_meshes #endif visitor.new_node_added_triple_face(node_id, f1, f2, f3, tm); - map_to_process[it_seg1->first].push_back(node_id); - map_to_process[it_seg2->first].push_back(node_id); - map_to_process[it_seg3->first].push_back(node_id); + map_to_process[&(it_seg1->second)].push_back(std::make_pair(ns1, node_id)); + map_to_process[&(it_seg2->second)].push_back(std::make_pair(ns2, node_id)); + map_to_process[&(it_seg3->second)].push_back(std::make_pair(ns3, node_id)); } - ++it_seg2; } - ++it_seg1; } } } @@ -1311,45 +1310,64 @@ class Intersection_of_triangle_meshes #ifdef CGAL_DEBUG_AUTOREFINEMENT std::cout << "\nAt the end of new node creation, current_node is " << current_node << "\n\n"; #endif - typedef std::pair, int>, - std::vector > Faces_and_nodes; - for(Faces_and_nodes& f_n_nids : map_to_process) + typedef std::pair, Node_id> > > Node_id_set_and_nodes; + for(Node_id_set_and_nodes& n_id_and_new_nodes : map_to_process) { - //get the original entry and remove it - typename Faces_to_nodes_map::iterator find_it = - f_to_node.find( f_n_nids.first ); // TODO AUTOREF_TAG saving the iterator above would avoid this find - CGAL_assertion(find_it!=f_to_node.end()); - CGAL_assertion(find_it->second.size()==2); // TODO AUTOREF_TAG handle case size = 1 - Node_id n1 = find_it->second[0]; - Node_id n2 = find_it->second[1]; - - // sort node ids along the edge - std::sort(f_n_nids.second.begin(), - f_n_nids.second.end(), - Less_for_nodes_along_an_edge(nodes, n1)); - - // insert new segments - Node_id prev = n1; - f_n_nids.second.push_back(n2); - int i=0; - typename Faces_to_nodes_map::iterator insert_it = find_it; -#ifdef CGAL_DEBUG_AUTOREFINEMENT - std::cout << n1 << " -> " << n2 << "\n"; -#endif - for(Node_id id : f_n_nids.second) + Node_id_set& nids = *(n_id_and_new_nodes.first); + CGAL_assertion(nids.size()!=1); // TODO AUTOREF_TAG handle case size = 1 + + // let's abuse coplanar_segments to store refinements + if (nids.size()==2) { - insert_it = f_to_node.insert(insert_it, std::make_pair( - std::make_pair(f_n_nids.first.first,--i), Node_id_set()) ); // I have picked negative int for refined edges - insert_it->second.insert(prev); - insert_it->second.insert(id); - CGAL_assertion(insert_it->second.size()==2); -#ifdef CGAL_DEBUG_AUTOREFINEMENT - std::cerr <<" adding " << prev << " " << id << " into " - << f_n_nids.first.first.first << " and " << f_n_nids.first.first.second << "\n"; -#endif - prev=id; + nids.coplanar_segments.push_back({nids[0], nids[1]}); + nids.size_=0; + } + + std::map, std::vector > local_map; + for (const std::pair, Node_id>& p : n_id_and_new_nodes.second) + { + CGAL_assertion(p.first[0], std::vector >& p : local_map) + { + //get the original entry and remove it + auto it = std::find(nids.coplanar_segments.begin(), nids.coplanar_segments.end(), p.first); + CGAL_assertion(it!=nids.coplanar_segments.end()); + nids.coplanar_segments.erase(it); + + std::vector& new_nodes = p.second; + Node_id n1 = p.first[0]; + Node_id n2 = p.first[1]; + + // sort node ids along the edge + std::sort(new_nodes.begin(), + new_nodes.end(), + Less_for_nodes_along_an_edge(nodes, n1)); + + // insert new segments + Node_id prev = n1; + new_nodes.push_back(n2); // add last node id to avoid having another case after the loop + #ifdef CGAL_DEBUG_AUTOREFINEMENT + std::cout << n1 << " -> " << n2 << "\n"; + #endif + for(Node_id id : new_nodes) + { + nids.coplanar_segments.push_back( {prev, id} ); + #ifdef CGAL_DEBUG_AUTOREFINEMENT + std::cerr <<" adding " << prev << " " << id << " into " + << n1 << " and " << n2 << "\n"; + #endif + prev=id; + } + } + + // update main entry + nids.first=nids.coplanar_segments.back()[0]; + nids.second=nids.coplanar_segments.back()[1]; + nids.size_=2; + nids.coplanar_segments.pop_back(); } } @@ -1362,7 +1380,7 @@ class Intersection_of_triangle_meshes bool isolated_point_seen=false; for (typename Faces_to_nodes_map::iterator it=f_to_node.begin();it!=f_to_node.end();++it){ const Node_id_set& segment=it->second; - CGAL_assertion(segment.size()==2 || segment.size()==1); + CGAL_assertion(segment.size()<=2); if (segment.size()==2){ Node_id i=segment.first; Node_id j=segment.second; @@ -1370,8 +1388,18 @@ class Intersection_of_triangle_meshes graph[j].insert(i); } else{ - CGAL_assertion(segment.size()==1); - isolated_point_seen=true; // NOT TRUE CAN BE END POINT OF POLYLINE FALLING ONTO AN INPUT EDGE + if (segment.size()==1) + isolated_point_seen=true; // NOT TRUE CAN BE END POINT OF POLYLINE FALLING ONTO AN INPUT EDGE + else + { + CGAL_assertion(!it->second.coplanar_segments.empty()); + } + } + + for (const std::array& ij : it->second.coplanar_segments) + { + graph[ij[0]].insert(ij[1]); + graph[ij[1]].insert(ij[0]); } } @@ -1466,24 +1494,31 @@ class Intersection_of_triangle_meshes } } - void remove_duplicated_intersecting_edges() { - std::set< std::pair > already_seen; + std::set< std::array > already_seen; std::vector to_erase; for (typename Faces_to_nodes_map::iterator it=f_to_node.begin(); it!=f_to_node.end(); ++it) { - if (it->second.size()==1) continue; - CGAL_precondition(it->second.size()==2); - CGAL_assertion((it->second.first)< (it->second.second)); - //it->second is a set so node ids are already sorted - bool is_new=already_seen.insert( std::make_pair( - it->second.first, - it->second.second ) - ).second; - - if (!is_new) + if (it->second.size()==2) + { + CGAL_assertion(it->second.first < it->second.second); + if (!already_seen.insert( {it->second.first, it->second.second} ).second) + it->second.size_=0; + } + + it->second.coplanar_segments.erase( + std::remove_if(it->second.coplanar_segments.begin(), + it->second.coplanar_segments.end(), + [&already_seen](const std::array& a) + { + CGAL_assertion(a[0]second.coplanar_segments.end()); + + if (it->second.size()==0 && it->second.coplanar_segments.empty()) to_erase.push_back(it); } @@ -1499,10 +1534,10 @@ class Intersection_of_triangle_meshes for (typename Faces_to_nodes_map::iterator it=f_to_node.begin(); it!=f_to_node.end(); ++it) { - if (it->second.size()==2) continue; - CGAL_precondition(it->second.size()==1); - halfedge_descriptor h1 = halfedge(it->first.first.first, tm); - halfedge_descriptor h2 = halfedge(it->first.first.second, tm); + if (it->second.size()!=1) continue; + + halfedge_descriptor h1 = halfedge(it->first.first, tm); + halfedge_descriptor h2 = halfedge(it->first.second, tm); for (int i=0; i<3; ++i) { From 33a9553a13308a9c21fe33477cecf347183610d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Wed, 14 Apr 2021 11:09:14 +0200 Subject: [PATCH 3/3] rename variables --- .../internal/Corefinement/intersection_impl.h | 92 +++++++++---------- 1 file changed, 46 insertions(+), 46 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Corefinement/intersection_impl.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Corefinement/intersection_impl.h index 02c2be886f7d..c89feac84add 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Corefinement/intersection_impl.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Corefinement/intersection_impl.h @@ -1148,70 +1148,70 @@ class Intersection_of_triangle_meshes // use lower bound to get the first entry which key is equal or larger // the non-coplanar entry case. That way any coplanar entry will be // listed afterward - typename Faces_to_nodes_map::iterator it_seg3 = + typename Faces_to_nodes_map::iterator it_seg23 = f_to_node.find(std::make_pair(f2, f3)); - if (it_seg3!=f_to_node.end()) + if (it_seg23!=f_to_node.end()) { std::vector > f2f3_segments; - it_seg3->second.get_segments(f2f3_segments); + it_seg23->second.get_segments(f2f3_segments); // look for edges between f1 and f2 - typename Faces_to_nodes_map::iterator it_seg1 = + typename Faces_to_nodes_map::iterator it_seg12 = f_to_node.find(std::make_pair(f1, f2)); - CGAL_assertion( it_seg1 != f_to_node.end() ); + CGAL_assertion( it_seg12 != f_to_node.end() ); std::vector > f1f2_segments; - it_seg1->second.get_segments(f1f2_segments); + it_seg12->second.get_segments(f1f2_segments); // look for edges between f1 and f3 - typename Faces_to_nodes_map::iterator it_seg2 = + typename Faces_to_nodes_map::iterator it_seg13 = f_to_node.find(std::make_pair(f1, f3)); - CGAL_assertion( it_seg2 != f_to_node.end() ); + CGAL_assertion( it_seg13 != f_to_node.end() ); std::vector > f1f3_segments; - it_seg2->second.get_segments(f1f3_segments); + it_seg13->second.get_segments(f1f3_segments); /// TODO AUTOREF_TAG shall we ignore tangency points? /// with the current code, Node_id_set::size()==1 is ignored as we only drop semgents /// Actually it might be that it is not a tangency point if the third segment was considered! /// so not handling it is a bug - for (const std::array& ns1 : f1f2_segments) + for (const std::array& ns12 : f1f2_segments) { - for (const std::array& ns2 : f1f3_segments) + for (const std::array& ns13 : f1f3_segments) { // handle cases of segments sharing an endpoint - if (ns1[0]==ns2[0] || ns1[0]==ns2[1] || - ns1[1]==ns2[0] || ns1[1]==ns2[1] ) + if (ns12[0]==ns13[0] || ns12[0]==ns13[1] || + ns12[1]==ns13[0] || ns12[1]==ns13[1] ) { Node_id common_nid, nid1, nid2; - if (ns1[0]==ns2[0]) + if (ns12[0]==ns13[0]) { - common_nid=ns1[0]; - nid1=ns1[1]; - nid2=ns2[1]; + common_nid=ns12[0]; + nid1=ns12[1]; + nid2=ns13[1]; } else { - if (ns1[0]==ns2[1]) + if (ns12[0]==ns13[1]) { - common_nid=ns1[0]; - nid1=ns1[1]; - nid2=ns2[0]; + common_nid=ns12[0]; + nid1=ns12[1]; + nid2=ns13[0]; } else { - if (ns1[1]==ns2[0]) + if (ns12[1]==ns13[0]) { - common_nid=ns1[1]; - nid1=ns1[0]; - nid2=ns2[1]; + common_nid=ns12[1]; + nid1=ns12[0]; + nid2=ns13[1]; } else { - common_nid=ns1[1]; - nid1=ns1[0]; - nid2=ns2[0]; + common_nid=ns12[1]; + nid1=ns12[0]; + nid2=ns13[0]; } } } @@ -1237,11 +1237,11 @@ class Intersection_of_triangle_meshes // TODO AUTOREF_TAG there might be a better test rather than relying on constructions typedef typename Node_vector::Exact_kernel::Segment_3 Segment_3; - Segment_3 s1(nodes.exact_node(ns1[0]), nodes.exact_node(ns1[1])), - s2(nodes.exact_node(ns2[0]), nodes.exact_node(ns2[1])), - s3; + Segment_3 s12(nodes.exact_node(ns12[0]), nodes.exact_node(ns12[1])), + s13(nodes.exact_node(ns13[0]), nodes.exact_node(ns13[1])), + s23; - if( do_intersect(s1, s2) ) + if( do_intersect(s12, s13) ) { /// TODO AUTOREF_TAG it might be the end point of a segment! // we need to find which segment is the third one intersecting @@ -1250,35 +1250,35 @@ class Intersection_of_triangle_meshes // first check if there is only one such edge (no test is needed then) CGAL_assertion(!f2f3_segments.empty() || !"AUTOREF_TAG HANDLE ME"); - std::array ns3 = f2f3_segments.front(); + std::array ns23 = f2f3_segments.front(); if (f2f3_segments.size()!=1) { std::size_t k=0; for (;ksecond)].push_back(std::make_pair(ns1, node_id)); - map_to_process[&(it_seg2->second)].push_back(std::make_pair(ns2, node_id)); - map_to_process[&(it_seg3->second)].push_back(std::make_pair(ns3, node_id)); + map_to_process[&(it_seg12->second)].push_back(std::make_pair(ns12, node_id)); + map_to_process[&(it_seg13->second)].push_back(std::make_pair(ns13, node_id)); + map_to_process[&(it_seg23->second)].push_back(std::make_pair(ns23, node_id)); } } }