From 0a7c9bc6d9628c4be9caf907111b2a5eee077eb0 Mon Sep 17 00:00:00 2001 From: Guillaume Damiand Date: Tue, 18 Dec 2018 10:54:23 +0100 Subject: [PATCH] Cleanup; plus avoid some path copy. --- .../include/CGAL/Path_on_surface_with_rle.h | 310 +++++---------- .../CGAL/Surface_mesh_curve_topology.h | 375 +++++++++--------- 2 files changed, 301 insertions(+), 384 deletions(-) diff --git a/Surface_mesh_topology/include/CGAL/Path_on_surface_with_rle.h b/Surface_mesh_topology/include/CGAL/Path_on_surface_with_rle.h index 4c25f0e00ba..23d9e3a537f 100644 --- a/Surface_mesh_topology/include/CGAL/Path_on_surface_with_rle.h +++ b/Surface_mesh_topology/include/CGAL/Path_on_surface_with_rle.h @@ -36,7 +36,9 @@ namespace CGAL { template class Path_on_surface; -// In this version, each flat (even those with length 0) have begin and end darts +// A flat is a sequence of darts given by its two extremities: begin and end, +// with +2 turns (if length>0) or -2 turns (if length<0). +// length==0 => begin==end. template class CFlat { @@ -92,7 +94,6 @@ public: } }; - typedef boost::unordered_set Set_of_it; #ifdef CGAL_PWRLE_TURN_V2 @@ -144,7 +145,7 @@ public: std::size_t i=0, j=0, starti=0, length=0; bool positive_flat=false; - bool negative_flat=false; + bool negative_flat=false; if (apath.is_closed()) { @@ -177,36 +178,8 @@ public: do { // Here dart i is the beginning of a flat part (maybe of length 0) - positive_flat=false; negative_flat=false; - if (apath.next_index_exists(i)) - { - if (!use_only_negative && apath.next_positive_turn(i)==2) - { positive_flat=true; } - else if (!use_only_positive && apath.next_negative_turn(i)==2) - { negative_flat=true; } - } - - if (!positive_flat && !negative_flat) - { - m_path.push_back(Flat(apath[i])); + push_back(apath[i]); i=apath.next_index(i); - } - else - { - j=i; - length=0; - while (apath.next_index_exists(j) && - ((positive_flat && apath.next_positive_turn(j)==2) || - (negative_flat && apath.next_negative_turn(j)==2))) - { - j=apath.next_index(j); - ++length; - } - CGAL_assertion(length>0); - m_path.push_back(Flat(apath[i], apath[j], - (positive_flat?length:-length))); - i=apath.next_index(j); - } } while(ibegin; int nb=0; while(nb!=flat->length) @@ -331,8 +305,10 @@ public: return dhend==flat->end; } + /// Display the given flat void display_flat(std::ostream& os, const List_iterator& flat) { + CGAL_assertion(is_valid_iterator(flat)); os<<"["<begin)<<", " <end)<<" (l="<length<<")]"; } @@ -373,8 +349,8 @@ public: /* std::cout<<"************* ERROR ************"<(*this).display_pos_and_neg_turns(); std::cout< - // (m_map, m_map.other_extremity(back()), dh))); + // This assert is too long CGAL_assertion(m_map.darts().owns(dh)); - m_path.push_back(dh); + if (is_empty()) return true; + + return CGAL::template belong_to_same_cell + (m_map, m_map.other_extremity(back()), dh); + } + + /// Add the given dart at the end of this path. + /// @pre can_be_pushed(dh) + void push_back(Dart_const_handle dh, bool update_isclosed=true) + { + CGAL_assertion(dh!=Map::null_handle); + // This assert is too long, it is tested in the is_valid method. + CGAL_assertion(can_be_pushed(dh)); + + List_iterator itlast=m_path.end(); + + bool positive_flat, negative_flat; + if (is_empty() || + !is_prev_flat_can_be_extended_at_end(itlast, dh, + positive_flat, negative_flat)) + { m_path.push_back(Flat(dh)); } // Create a new flat + else + { + --itlast; + set_end_of_flat(itlast, dh); // Move the last dart of the last flat + set_flat_length(itlast, flat_length(itlast)+(positive_flat?+1:-1)); // Increment the length of the flat + } + + ++m_length; if (update_isclosed) { update_is_closed(); } } - */ // Update m_is_closed to true iff the path is closed (i.e. the second // extremity of the last dart of the path is the same vertex than the one @@ -816,27 +819,6 @@ public: return m_path.begin(); } -#ifndef NDEBUG -/* WRONG ASSERT - * TODO either compute the real minimum ? or ? - * { - // The sequence of modified flats should be consecutive - List_iterator it=smallest_it; - unsigned int nb=modified_flats.size(); - while(nb>0) - { - if (modified_flats.count(it)==1) { --nb; } - else if (next_flat_exist(it) && modified_flats.count(next_iterator(it))==0) - { - std::cerr<<"ERROR in merge_modified_flats_when_possible: modified flats" - <<" are not consecutives."<(begin_of_flat(it1))); // TODO CHECK - set_end_of_flat(it1, m_map.template beta<2,1,2,0>(end_of_flat(it1))); // TODO CHECK + set_begin_of_flat(it1, m_map.template beta<2,0,2,1>(begin_of_flat(it1))); + set_end_of_flat(it1, m_map.template beta<2,1,2,0>(end_of_flat(it1))); set_flat_length(it1, (-flat_length(it1))-2); flat_modified(it1, modified_flats); it1=merge_modified_flats_when_possible(modified_flats); @@ -1211,12 +1205,6 @@ public: it1=merge_modified_flats_when_possible(modified_flats); - /* if (!is_valid()) - { - std::cout<<*next_iterator(it1)<0); - set_begin_of_flat(it1, m_map.template beta<1,2,0,2>(begin_of_flat(it1))); // TODO CHECK - set_end_of_flat(it1, m_map.template beta<0,2,1,2>(end_of_flat(it1))); // TODO CHECK + set_begin_of_flat(it1, m_map.template beta<1,2,0,2>(begin_of_flat(it1))); + set_end_of_flat(it1, m_map.template beta<0,2,1,2>(end_of_flat(it1))); set_flat_length(it1, -(flat_length(it1)-2)); - m_length-=2; flat_modified(it1, modified_flats); it1=merge_modified_flats_when_possible(modified_flats); + CGAL_assertion(m_length>1); + m_length-=2; return; } @@ -1264,14 +1253,6 @@ public: it1=merge_modified_flats_when_possible(modified_flats); - // if (is_beginning_of_non_null_flat(it1)) { advance_iterator(it1); } - /* if (!is_valid()) - { - std::cout<<*next_iterator(it1)<(*this).display_pos_and_neg_turns(); std::cout<(dh3)); + set_end_of_flat(it2, dh6); // TODO WHY THIS DOES NOT WORK ???? + //set_end_of_flat(it2, m_map.template beta<1, 2, 1>(dh3)); // TODO VERY STRANGE set_flat_length(it2, -flat_length(it1)); set_flat_length(it1, 0); -/* - { // TEMPO DEBUG - unsigned int NBTOTO=0; - Dart_const_handle dhtoto=dh2; - while (dhtoto!=dh3) - { - dhtoto=m_map.template beta<1, 2, 1>(dhtoto); - ++NBTOTO; - } - - } - - Dart_const_handle dhtoto2=m_map.template beta<1, 2, 1>(dh3); - Dart_const_handle dhtoto4=m_map.template beta<1, 2, 1>(dh5); - - CGAL_assertion((m_map.template beta<1, 2, 1>(dh3)==dh6));*/ - - // CGAL_assertion(is_flat_valid(it1)); - // CGAL_assertion(is_flat_valid(it2)); flat_modified(it1, modified_flats); flat_modified(it2, modified_flats); @@ -1660,56 +1609,28 @@ public: m_length+=2; flat_modified(it1, modified_flats); flat_modified(it2, modified_flats); - } + // 3) Move the first flat - /* if (first_flat_zero) - { - m_path.erase(it1); - it1=prev_iterator(it3); - } - else */ - { - CGAL_assertion(flat_length(it1)<0); - set_begin_of_flat(it1, dh2); - set_flat_length(it1, -(flat_length(it1))-1); - if (flat_length(it1)==0) { set_end_of_flat(it1, dh2); } - else { set_end_of_flat(it1, dh3); } // End of the moved flat - flat_modified(it1, modified_flats); - - // merge_with_next_flat_if_possible(prev_iterator(it1)); - } + CGAL_assertion(flat_length(it1)<0); + set_begin_of_flat(it1, dh2); + set_flat_length(it1, -(flat_length(it1))-1); + if (flat_length(it1)==0) { set_end_of_flat(it1, dh2); } + else { set_end_of_flat(it1, dh3); } // End of the moved flat + flat_modified(it1, modified_flats); // 4) Move the second flat - /* if (second_flat_zero) - { - CGAL_assertion(it3!=it1); - m_path.erase(it3); - } - else */ - { - CGAL_assertion(flat_length(it2)<0); - set_begin_of_flat(it2, dh4); - set_flat_length(it2, -(flat_length(it2))-1); - if (flat_length(it2)==0) { set_end_of_flat(it2, dh4); } - else { set_end_of_flat(it2, dh5); } // End of the moved flat - flat_modified(it2, modified_flats); - - // merge_with_next_flat_if_possible(prev_iterator(it3)); - } + CGAL_assertion(flat_length(it2)<0); + set_begin_of_flat(it2, dh4); + set_flat_length(it2, -(flat_length(it2))-1); + if (flat_length(it2)==0) { set_end_of_flat(it2, dh4); } + else { set_end_of_flat(it2, dh5); } // End of the moved flat + flat_modified(it2, modified_flats); + CGAL_assertion(m_length>1); m_length-=2; it1=merge_modified_flats_when_possible(modified_flats); - - // CGAL_assertion(is_valid()); - /*it2=next_flat(it1); - if (merge_with_next_flat_if_possible(prev_iterator(it1))) - { it1=prev_iterator(it2); } - merge_with_next_flat_if_possible(it1); */ - - /*if (it==m_path.end() && !is_empty()) - { it=m_path.begin(); } */ } /// Right push the path, if all all l-shape are pushed, otherwise only one. @@ -1726,9 +1647,8 @@ public: // CGAL_assertion(is_valid_iterator(it)); // CGAL_assertion(is_valid()); if (!all) { return true; } - // it=m_path.begin(); } - else { move_to_next_l_shape(it); } // ++it; } + else { move_to_next_l_shape(it); } } CGAL_assertion(is_valid()); return res; @@ -1739,34 +1659,24 @@ public: { CGAL_assertion(is_valid()); - // ?? if (!is_closed()) { return; } + if (is_empty()) { return; } + + CGAL_assertion(is_closed()); bool modified=false; remove_spurs(); - //do + do { - do - { - modified=remove_brackets(); - modified=modified || remove_spurs(); - } - while(modified); - - // std::cout<<"path="<(*this)<(*this).display_pos_and_neg_turns(); std::cout< UFTree; typedef typename UFTree::handle UFTree_handle; @@ -97,13 +99,15 @@ namespace CGAL { { t.start(); t2.start(); } // The mapping between darts of the original map into the copied map. - boost::unordered_map origin_to_copy; + boost::unordered_map + origin_to_copy; // We copy the original map, while keeping a mapping between darts. m_map.copy(m_original_map, &origin_to_copy); // The mapping between darts of the copy into darts of the original map. - boost::unordered_map copy_to_origin; + boost::unordered_map + copy_to_origin; for (auto it=origin_to_copy.begin(); it!=origin_to_copy.end(); ++it) { copy_to_origin[it->second]=it->first; } @@ -289,93 +293,44 @@ namespace CGAL { m_original_map.free_mark(m_mark_L); } - const CMap_for_surface_mesh_curve_topology& get_map() const - { return m_map; } - - /// Canonize the path - /* void canonize(Path_on_surface& path, bool display_time=false) const - { - if (!path.is_closed()) - { - std::cerr<<"Error: a non closed path cannot be canonized."< path2(path); - path2.canonize(); - path=path2; - - if (display_time) - { - t.stop(); - std::cout<<"[TIME] Canonize path: "<& path, - std::size_t& a, - std::size_t& b) - { - Dart_const_handle dha=m_map.darts().begin(); - if (dha>m_map.template beta<2>(dha)) { dha=m_map.template beta<2>(dha); } - - Dart_const_handle dhb=NULL; - auto it=m_map.darts().begin(); - while(dhb==NULL) - { - if (it!=dha && it!=m_map.template beta<2>(dha)) { dhb=it; } - else { ++it; } - } - - if (dhb>m_map.template beta<2>(dhb)) { dhb=m_map.template beta<2>(dhb); } - - if (dha(dha)) { --a; } - else if (path[i]==dhb) { ++b; } - else if (path[i]==m_map.template beta<2>(dhb)) { --b; } - } - } - /// @return true iff 'path' is contractible. bool is_contractible(const Path_on_surface& p, bool display_time=false) { + if (p.is_empty()) + { return true; } + + if (!p.is_closed()) + { + std::cerr<<"Error: is_contractible requires a closed path."< - pt=transform_original_path_into_quad_surface(p); - bool res=false; if (m_map.number_of_darts()==4) { // Case of torus + Path_on_surface + pt=transform_original_path_into_quad_surface(p); + std::size_t a, b; count_edges_of_path_on_torus(pt, a, b); res=(a==0 && b==0); } else { - Path_on_surface_with_rle path(pt - #ifdef CGAL_PWRLE_TURN_V2 - , m_dart_ids - #endif //CGAL_PWRLE_TURN_V2 - ); - path.canonize(); - res=path.is_empty(); - std::cout<<"Length of reduced paths: "< + pt=transform_original_path_into_quad_surface_with_rle(p); + + pt.canonize(); + res=pt.is_empty(); + std::cout<<"Length of reduced paths: "<& p2, bool display_time=false) { + if (p1.is_empty() && p2.is_empty()) { return true; } + + if ((!p1.is_empty() && !p1.is_closed()) || + (!p2.is_empty() && !p2.is_closed())) + { + std::cerr<<"Error: are_freely_homotopic requires two closed paths." + < - pt1=transform_original_path_into_quad_surface(p1); - Path_on_surface - pt2=transform_original_path_into_quad_surface(p2); - bool res=false; if (m_map.number_of_darts()==4) { // Case of torus + Path_on_surface + pt1=transform_original_path_into_quad_surface(p1); + Path_on_surface + pt2=transform_original_path_into_quad_surface(p2); + std::size_t a1, a2, b1, b2; count_edges_of_path_on_torus(pt1, a1, b1); count_edges_of_path_on_torus(pt2, a2, b2); @@ -413,25 +382,16 @@ namespace CGAL { else { Path_on_surface_with_rle - path1(pt1 -#ifdef CGAL_PWRLE_TURN_V2 - , m_dart_ids -#endif //CGAL_PWRLE_TURN_V2 - ); + pt1=transform_original_path_into_quad_surface_with_rle(p1); Path_on_surface_with_rle - path2(pt2 -#ifdef CGAL_PWRLE_TURN_V2 - , m_dart_ids -#endif //CGAL_PWRLE_TURN_V2 - ); - - path1.canonize(); - path2.canonize(); - res=(path1==path2); // Do here to be counted in the computation time + pt2=transform_original_path_into_quad_surface_with_rle(p2); + pt1.canonize(); + pt2.canonize(); + res=(pt1==pt2); // Do here to be counted in the computation time #ifdef CGAL_TRACE_PATH_TESTS - std::cout<<"Length of reduced paths: "<& path, + std::size_t& a, std::size_t& b) + { + Dart_const_handle dha=m_map.darts().begin(); + if (dha>m_map.template beta<2>(dha)) { dha=m_map.template beta<2>(dha); } + + Dart_const_handle dhb=NULL; + auto it=m_map.darts().begin(); + while(dhb==NULL) + { + if (it!=dha && it!=m_map.template beta<2>(dha)) { dhb=it; } + else { ++it; } + } + + if (dhb>m_map.template beta<2>(dhb)) { dhb=m_map.template beta<2>(dhb); } + + if (dha(dha)) { --a; } + else if (path[i]==dhb) { ++b; } + else if (path[i]==m_map.template beta<2>(dhb)) { --b; } + } + } + Path_on_surface transform_original_path_into_quad_surface - (const Path_on_surface& path) // TODO return directly a Path_on_durface_with_rle + (const Path_on_surface& path + #ifdef CGAL_PWRLE_TURN_V2 + , m_dart_ids + #endif //CGAL_PWRLE_TURN_V2 + ) { Path_on_surface res(m_map); if (path.is_empty()) return res; @@ -511,6 +513,36 @@ namespace CGAL { return res; } + Path_on_surface_with_rle + transform_original_path_into_quad_surface_with_rle + (const Path_on_surface& path) + { + Path_on_surface_with_rle + res(m_map + #ifdef CGAL_PWRLE_TURN_V2 + , m_dart_ids + #endif //CGAL_PWRLE_TURN_V2 + ); + + if (path.is_empty()) return res; + CGAL_assertion(path.is_closed()); + + for (std::size_t i=0; i& vertices) @@ -580,7 +612,8 @@ namespace CGAL { } /// Mark the edge containing adart in the original map. - void mark_edge(const Map& amap, typename Map::Dart_const_handle adart, std::size_t amark) + void mark_edge(const Map& amap, typename Map::Dart_const_handle adart, + std::size_t amark) { amap.mark(amap.template beta<2>(adart), amark); amap.mark(adart, amark); @@ -591,10 +624,13 @@ namespace CGAL { /// (which belongs to the map m_original_map) from the array origin_to_copy void erase_edge_from_associative_arrays (Dart_handle adart, - boost::unordered_map& origin_to_copy, - boost::unordered_map& copy_to_origin) + boost::unordered_map& + origin_to_copy, + boost::unordered_map& + copy_to_origin) { - origin_to_copy.erase(m_original_map.template beta<2>(copy_to_origin[adart])); + origin_to_copy.erase(m_original_map.template beta<2> + (copy_to_origin[adart])); origin_to_copy.erase(copy_to_origin[adart]); copy_to_origin.erase(m_map.template beta<2>(adart)); @@ -605,8 +641,10 @@ namespace CGAL { /// vertex. All edges contracted during this step belong to the spanning /// tree T, and thus corresponding edges in m_original_map are marked. void surface_simplification_in_one_vertex - (boost::unordered_map& origin_to_copy, - boost::unordered_map& copy_to_origin) + (boost::unordered_map& + origin_to_copy, + boost::unordered_map& + copy_to_origin) { UFTree uftrees; // uftree of vertices; one tree for each vertex, // contains one dart of the vertex @@ -627,11 +665,14 @@ namespace CGAL { erase_edge_from_associative_arrays(it, origin_to_copy, copy_to_origin); uftrees.unify_sets(get_uftree(uftrees, vertices, it), - get_uftree(uftrees, vertices, m_map.template beta<2>(it))); + get_uftree(uftrees, vertices, + m_map.template beta<2>(it))); //m_map.template contract_cell<1>(it); Dart_handle d1=it, d2=m_map.template beta<2>(it); - m_map.template link_beta<1>(m_map.template beta<0>(d1), m_map.template beta<1>(d1)); - m_map.template link_beta<1>(m_map.template beta<0>(d2), m_map.template beta<1>(d2)); + m_map.template link_beta<1>(m_map.template beta<0>(d1), + m_map.template beta<1>(d1)); + m_map.template link_beta<1>(m_map.template beta<0>(d2), + m_map.template beta<1>(d2)); m_map.erase_dart(d1); m_map.erase_dart(d2); } @@ -645,7 +686,8 @@ namespace CGAL { /// will be updated later (in surface_simplification_in_one_face() and in /// surface_quadrangulate() ) void compute_length_two_paths - (const boost::unordered_map& origin_to_copy) + (const boost::unordered_map& + origin_to_copy) { paths.clear(); @@ -669,7 +711,8 @@ namespace CGAL { #ifdef CGAL_TRACE_CMAP_TOOLS std::cout<<"Number of darts in paths: "<& origin_to_copy, - boost::unordered_map& copy_to_origin) + (boost::unordered_map& + origin_to_copy, + boost::unordered_map& + copy_to_origin) { UFTree uftrees; // uftree of faces; one tree for each face, // contains one dart of the face @@ -732,8 +777,7 @@ namespace CGAL { } else { - // update_length_two_paths_before_edge_removals_v1(toremove); - update_length_two_paths_before_edge_removals_v2(toremove, copy_to_origin); + update_length_two_paths_before_edge_removals(toremove, copy_to_origin); // We remove all the edges to remove. for (typename CMap_for_surface_mesh_curve_topology::Dart_range::iterator @@ -779,8 +823,6 @@ namespace CGAL { p.second=m_map.template beta<0>(p.second); //std::cout<<" -> "<(p.second)); } // 3) We remove all the old edges. @@ -795,63 +837,16 @@ namespace CGAL { m_map.free_mark(oldedges); } - /// Version1: quadratic in number of darts in the paths - void update_length_two_paths_before_edge_removals_v1(typename Map::size_type toremove) + /// Update all length two paths, before edge removal. Edges that will be + /// removed are marked with toremove mark. + void update_length_two_paths_before_edge_removals + (typename Map::size_type toremove, + const boost::unordered_map& copy_to_origin) { // std::cout<<"************************************************"<& p=itp->second; - //std::cout<<"Pair: "<(p.first); - Dart_const_handle initdart=p.first; - - if (!m_map.is_marked(p.first, toremove)) - { - p.second=m_map.template beta<1>(p.first); - initdart=p.second; - while (m_map.is_marked(p.second, toremove)) - { - p.second=m_map.template beta<2, 1>(p.second); - assert(p.second!=initdart); - } - } - else - { - while (m_map.is_marked(p.first, toremove)) - { - p.first=m_map.template beta<2, 1>(p.first); - //std::cout<(p.second); - initdart=p.second; - while (m_map.is_marked(p.second, toremove)) - { - p.second=m_map.template beta<2, 1>(p.second); - //std::cout< "<& copy_to_origin) - { - // std::cout<<"************************************************"<& p=paths[it]; Dart_handle initdart=m_map.darts().iterator_to - (const_cast(*(p.first))); + (const_cast + (*(p.first))); Dart_handle initdart2=m_map.template beta<2>(initdart); CGAL_assertion(initdart2==p.second); CGAL_assertion(!m_map.is_marked(initdart, toremove)); @@ -876,8 +872,10 @@ namespace CGAL { while (m_map.is_marked(initdart, toremove)) { assert(copy_to_origin.count(initdart)==1); - typename Map::Dart_const_handle d1=copy_to_origin.find(initdart)->second; - typename Map::Dart_const_handle d2=m_original_map.template beta<2>(d1); + typename Map::Dart_const_handle + d1=copy_to_origin.find(initdart)->second; + typename Map::Dart_const_handle + d2=m_original_map.template beta<2>(d1); if (d1(initdart); @@ -892,8 +890,10 @@ namespace CGAL { while (m_map.is_marked(initdart2, toremove)) { assert(copy_to_origin.count(initdart2)==1); - typename Map::Dart_const_handle d1=copy_to_origin.find(initdart2)->second; - typename Map::Dart_const_handle d2=m_original_map.template beta<2>(d1); + typename Map::Dart_const_handle + d1=copy_to_origin.find(initdart2)->second; + typename Map::Dart_const_handle + d2=m_original_map.template beta<2>(d1); if (d1(adart); + typename Map::Dart_const_handle + opposite=m_original_map.template beta<2>(adart); if (adart(adart); + typename Map::Dart_const_handle + opposite=m_original_map.template beta<2>(adart); if (adartsecond; } return paths.find(opposite)->second; } - Dart_const_handle get_first_dart_of_the_path(typename Map::Dart_const_handle adart) const + Dart_const_handle get_first_dart_of_the_path + (typename Map::Dart_const_handle adart) const { assert(!m_original_map.is_marked(adart, m_mark_T)); assert(is_edge_has_path(adart)); - typename Map::Dart_const_handle opposite=m_original_map.template beta<2>(adart); + typename Map::Dart_const_handle + opposite=m_original_map.template beta<2>(adart); if (adart& @@ -955,12 +959,14 @@ namespace CGAL { return m_map.template beta<2>(p.second); } - Dart_const_handle get_second_dart_of_the_path(typename Map::Dart_const_handle adart) const + Dart_const_handle get_second_dart_of_the_path + (typename Map::Dart_const_handle adart) const { assert(!m_original_map.is_marked(adart, m_mark_T)); assert(is_edge_has_path(adart)); - typename Map::Dart_const_handle opposite=m_original_map.template beta<2>(adart); + typename Map::Dart_const_handle + opposite=m_original_map.template beta<2>(adart); if (adart& @@ -991,8 +997,8 @@ namespace CGAL { { if (!is_edge_has_path(it)) { - std::cout<<"ERROR: an edge that does not belong to the spanning tree" - <<" T has no associated path."<