From 80e99153dcbeb30ac1c3c1301e7c530cf8de53da Mon Sep 17 00:00:00 2001 From: Simon Giraudot Date: Thu, 28 Jan 2021 10:30:55 +0100 Subject: [PATCH 1/8] Use Compact Container in DCEL base instead of in place list --- .../include/CGAL/Arr_dcel_base.h | 106 +++++++----------- .../include/CGAL/Compact_container.h | 2 + 2 files changed, 42 insertions(+), 66 deletions(-) diff --git a/Arrangement_on_surface_2/include/CGAL/Arr_dcel_base.h b/Arrangement_on_surface_2/include/CGAL/Arr_dcel_base.h index 16a15d7ea56..e4bae24bf7e 100644 --- a/Arrangement_on_surface_2/include/CGAL/Arr_dcel_base.h +++ b/Arrangement_on_surface_2/include/CGAL/Arr_dcel_base.h @@ -28,7 +28,7 @@ #include #include #include -#include +#include #include #include #include @@ -91,6 +91,9 @@ public: /*! Destructor. */ virtual ~Arr_vertex_base() {} + void* for_compact_container() const { return static_cast(p_pt); } + void for_compact_container(void* ptr) { p_pt = static_cast(ptr); } + // Access/modification for pointer squatting void* inc() const { return p_inc; } void set_inc(void * inc) const @@ -183,6 +186,9 @@ public: /*! Destructor. */ virtual ~Arr_halfedge_base() {} + void* for_compact_container() const { return static_cast(p_cv); } + void for_compact_container(void* ptr) { p_cv = static_cast(ptr); } + /*! Check if the curve pointer is nullptr. */ bool has_null_curve() const { return (p_cv == nullptr); } @@ -284,7 +290,7 @@ template class Arr_isolated_vertex; * The default arrangement DCEL vertex class. */ template -class Arr_vertex : public V, public In_place_list_base > +class Arr_vertex : public V { public: @@ -351,8 +357,7 @@ public: * The default arrangement DCEL halfedge class. */ template -class Arr_halfedge : public H, - public In_place_list_base > +class Arr_halfedge : public H { public: typedef H Base; @@ -504,7 +509,7 @@ public: */ template class Arr_face : public F, - public In_place_list_base > + public Compact_container_base { public: typedef F Base; @@ -695,7 +700,7 @@ public: * Representation of an outer CCB. */ template -class Arr_outer_ccb : public In_place_list_base > { +class Arr_outer_ccb { public: typedef Arr_outer_ccb Self; typedef Arr_halfedge Halfedge; @@ -716,6 +721,9 @@ public: p_f(other.p_f), iter_is_not_singular(other.iter_is_not_singular) { if (other.iter_is_not_singular) iter = other.iter; } + void* for_compact_container() const { return static_cast(p_f); } + void for_compact_container(void* ptr) { p_f = static_cast(ptr); } + /*! Get a halfedge along the component (const version). */ const Halfedge* halfedge() const { return (*iter); } @@ -760,7 +768,7 @@ public: * Representation of an inner CCB. */ template -class Arr_inner_ccb : public In_place_list_base > +class Arr_inner_ccb : public Compact_container_base { public: typedef Arr_inner_ccb Self; @@ -826,8 +834,7 @@ public: * Representation of an isolated vertex. */ template -class Arr_isolated_vertex : -public In_place_list_base > { +class Arr_isolated_vertex { public: typedef Arr_isolated_vertex Self; typedef Arr_face Face; @@ -847,6 +854,9 @@ public: p_f(other.p_f), iter_is_not_singular(other.iter_is_not_singular) { if (other.iter_is_not_singular) iv_it = other.iv_it; } + void* for_compact_container() const { return static_cast(p_f); } + void for_compact_container(void* ptr) { p_f = static_cast(ptr); } + /*! Get the containing face (const version). */ const Face* face() const { return (p_f); } @@ -882,7 +892,8 @@ public: * The arrangement DCEL class. */ template > +// class Allocator = boost::fast_pool_allocator > + class Allocator = CGAL_ALLOCATOR(int)> class Arr_dcel_base { public: // Define the vertex, halfedge and face types. @@ -897,13 +908,6 @@ public: typedef Inner_ccb Hole; protected: - // The vetices, halfedges and faces are stored in three in-place lists. - typedef In_place_list Vertex_list; - typedef In_place_list Halfedge_list; - typedef In_place_list Face_list; - typedef In_place_list Outer_ccb_list; - typedef In_place_list Inner_ccb_list; - typedef In_place_list Iso_vert_list; typedef std::allocator_traits Allocator_traits; typedef typename Allocator_traits::template rebind_alloc Vertex_allocator; @@ -913,6 +917,13 @@ protected: typedef typename Allocator_traits::template rebind_alloc Inner_ccb_allocator; typedef typename Allocator_traits::template rebind_alloc Iso_vert_allocator; + typedef Compact_container Vertex_list; + typedef Compact_container Halfedge_list; + typedef Compact_container Face_list; + typedef Compact_container Outer_ccb_list; + typedef Compact_container Inner_ccb_list; + typedef Compact_container Iso_vert_list; + public: typedef typename Halfedge_list::size_type Size; typedef typename Halfedge_list::size_type size_type; @@ -929,13 +940,6 @@ protected: Inner_ccb_list in_ccbs; // The inner CCBs. Iso_vert_list iso_verts; // The isolated vertices. - Vertex_allocator vertex_alloc; // An allocator for vertices. - Halfedge_allocator halfedge_alloc; // An allocator for halfedges. - Face_allocator face_alloc; // An allocator for faces. - Outer_ccb_allocator out_ccb_alloc; // An allocator for outer CCBs. - Inner_ccb_allocator in_ccb_alloc; // An allocator for inner CCBs. - Iso_vert_allocator iso_vert_alloc; // Allocator for isolated vertices. - public: // Definitions of iterators. typedef typename Vertex_list::iterator Vertex_iterator; @@ -1058,10 +1062,7 @@ public: /*! Create a new vertex. */ Vertex* new_vertex() { - Vertex* v = vertex_alloc.allocate(1); - std::allocator_traits::construct(vertex_alloc,v); - vertices.push_back(*v); - return v; + return &*vertices.emplace(); } /*! Create a new pair of opposite halfedges. */ @@ -1081,37 +1082,25 @@ public: /*! Create a new face. */ Face* new_face() { - Face* f = face_alloc.allocate(1); - std::allocator_traits::construct(face_alloc, f); - faces.push_back (*f); - return(f); + return &*faces.emplace(); } /*! Create a new outer CCB. */ Outer_ccb* new_outer_ccb() { - Outer_ccb* oc = out_ccb_alloc.allocate(1); - std::allocator_traits::construct(out_ccb_alloc, oc); - out_ccbs.push_back(*oc); - return (oc); + return &*out_ccbs.emplace(); } /*! Create a new inner CCB. */ Inner_ccb* new_inner_ccb() { - Inner_ccb* ic = in_ccb_alloc.allocate(1); - std::allocator_traits::construct(in_ccb_alloc, ic); - in_ccbs.push_back(*ic); - return (ic); + return &*in_ccbs.emplace(); } /*! Create a new isolated vertex. */ Isolated_vertex* new_isolated_vertex() { - Isolated_vertex* iv = iso_vert_alloc.allocate(1); - std::allocator_traits::construct(iso_vert_alloc, iv); - iso_verts.push_back(*iv); - return (iv); + return &*iso_verts.emplace(); } //@} @@ -1120,9 +1109,7 @@ public: /*! Delete an existing vertex. */ void delete_vertex(Vertex* v) { - vertices.erase(v); - std::allocator_traits::destroy(vertex_alloc, v); - vertex_alloc.deallocate(v,1); + vertices.erase (vertices.iterator_to(*v)); } /*! Delete an existing pair of opposite halfedges. */ @@ -1136,33 +1123,25 @@ public: /*! Delete an existing face. */ void delete_face(Face* f) { - faces.erase(f); - std::allocator_traits::destroy(face_alloc, f); - face_alloc.deallocate(f, 1); + faces.erase (faces.iterator_to(*f)); } /*! Delete an existing outer CCB. */ void delete_outer_ccb(Outer_ccb* oc) { - out_ccbs.erase(oc); - std::allocator_traits::destroy(out_ccb_alloc, oc); - out_ccb_alloc.deallocate(oc, 1); + out_ccbs.erase (out_ccbs.iterator_to(*oc)); } /*! Delete an existing inner CCB. */ void delete_inner_ccb(Inner_ccb* ic) { - in_ccbs.erase(ic); - std::allocator_traits::destroy(in_ccb_alloc, ic); - in_ccb_alloc.deallocate(ic, 1); + in_ccbs.erase (in_ccbs.iterator_to(*ic)); } /*! Delete an existing isolated vertex. */ void delete_isolated_vertex(Isolated_vertex* iv) { - iso_verts.erase(iv); - std::allocator_traits::destroy(iso_vert_alloc, iv); - iso_vert_alloc.deallocate(iv, 1); + iso_verts.erase (iso_verts.iterator_to(*iv)); } /*! Delete all DCEL features. */ @@ -1421,18 +1400,13 @@ protected: /*! Create a new halfedge. */ Halfedge* _new_halfedge() { - Halfedge* h = halfedge_alloc.allocate(1); - std::allocator_traits::construct(halfedge_alloc, h); - halfedges.push_back(*h); - return (h); + return &*halfedges.emplace(); } /*! Delete an existing halfedge. */ void _delete_halfedge(Halfedge* h) { - halfedges.erase(h); - std::allocator_traits::destroy(halfedge_alloc,h); - halfedge_alloc.deallocate(h, 1); + halfedges.erase (halfedges.iterator_to(*h)); } }; diff --git a/STL_Extension/include/CGAL/Compact_container.h b/STL_Extension/include/CGAL/Compact_container.h index 62751003b5a..9273df1556e 100644 --- a/STL_Extension/include/CGAL/Compact_container.h +++ b/STL_Extension/include/CGAL/Compact_container.h @@ -875,6 +875,8 @@ namespace internal { m_ptr = nullptr; } + CC_iterator(pointer ptr) : m_ptr(ptr) { } + // Converting constructor from mutable to constant iterator template CC_iterator(const CC_iterator< From 2e98995b29e8c010ad94831394f093b506b4cb69 Mon Sep 17 00:00:00 2001 From: Simon Giraudot Date: Thu, 28 Jan 2021 14:24:07 +0100 Subject: [PATCH 2/8] Clean up and use fast pool allocator (faster) --- Arrangement_on_surface_2/include/CGAL/Arr_dcel_base.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Arrangement_on_surface_2/include/CGAL/Arr_dcel_base.h b/Arrangement_on_surface_2/include/CGAL/Arr_dcel_base.h index e4bae24bf7e..4945a52adeb 100644 --- a/Arrangement_on_surface_2/include/CGAL/Arr_dcel_base.h +++ b/Arrangement_on_surface_2/include/CGAL/Arr_dcel_base.h @@ -892,8 +892,7 @@ public: * The arrangement DCEL class. */ template > - class Allocator = CGAL_ALLOCATOR(int)> + class Allocator = boost::fast_pool_allocator > class Arr_dcel_base { public: // Define the vertex, halfedge and face types. From 6bbd72cfde33f76eba3cf692ba969ad3735d802c Mon Sep 17 00:00:00 2001 From: Simon Giraudot Date: Thu, 28 Jan 2021 14:50:26 +0100 Subject: [PATCH 3/8] Fix default constructor of iterators --- HalfedgeDS/include/CGAL/HalfedgeDS_iterator.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/HalfedgeDS/include/CGAL/HalfedgeDS_iterator.h b/HalfedgeDS/include/CGAL/HalfedgeDS_iterator.h index 23d1a135eea..e5aa9493fbf 100644 --- a/HalfedgeDS/include/CGAL/HalfedgeDS_iterator.h +++ b/HalfedgeDS/include/CGAL/HalfedgeDS_iterator.h @@ -181,7 +181,7 @@ public: // CREATION // -------- - _HalfedgeDS_facet_circ() : It(0) {} + _HalfedgeDS_facet_circ() : It(nullptr) {} //_HalfedgeDS_facet_circ( pointer p) : It(p) {} _HalfedgeDS_facet_circ( It i) : It(i) {} @@ -241,7 +241,7 @@ public: // CREATION // -------- - _HalfedgeDS_facet_const_circ() : It(0) {} + _HalfedgeDS_facet_const_circ() : It(nullptr) {} _HalfedgeDS_facet_const_circ( pointer p) : It(p) {} _HalfedgeDS_facet_const_circ( It i) : It(i) {} @@ -306,7 +306,7 @@ public: // CREATION // -------- - _HalfedgeDS_vertex_circ() : It(0) {} + _HalfedgeDS_vertex_circ() : It(nullptr) {} //_HalfedgeDS_vertex_circ( pointer p) : It(p) {} _HalfedgeDS_vertex_circ( It i) : It(i) {} @@ -366,7 +366,7 @@ public: // CREATION // -------- - _HalfedgeDS_vertex_const_circ() : It(0) {} + _HalfedgeDS_vertex_const_circ() : It(nullptr) {} _HalfedgeDS_vertex_const_circ( pointer p) : It(p) {} _HalfedgeDS_vertex_const_circ( It i) : It(i) {} From 3448035fc691cbc2259e6115fa17117aa1d94fe1 Mon Sep 17 00:00:00 2001 From: Simon Giraudot Date: Wed, 24 Mar 2021 08:25:29 +0100 Subject: [PATCH 4/8] Fix CC_iterator by making constructor from pointer explicit --- .../Arrangement_2/Arrangement_2_iterators.h | 28 +++++++++++++++++++ HalfedgeDS/include/CGAL/HalfedgeDS_iterator.h | 12 ++++---- .../include/CGAL/Compact_container.h | 2 +- 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/Arrangement_on_surface_2/include/CGAL/Arrangement_2/Arrangement_2_iterators.h b/Arrangement_on_surface_2/include/CGAL/Arrangement_2/Arrangement_2_iterators.h index e210813d47f..219c445bdc1 100644 --- a/Arrangement_on_surface_2/include/CGAL/Arrangement_2/Arrangement_2_iterators.h +++ b/Arrangement_on_surface_2/include/CGAL/Arrangement_2/Arrangement_2_iterators.h @@ -288,6 +288,12 @@ public: iend (nt) {} + template + I_Filtered_iterator (T* p) : + nt (pointer(p)), + iend (nt) + {} + I_Filtered_iterator (Iterator it, Iterator end) : nt (it), iend (end) @@ -305,6 +311,14 @@ public: ++nt; } + template + I_Filtered_iterator& operator= (const P* p) + { + nt = pointer(p); + iend =nt; + return *this; + } + /*! Access operations. */ Iterator current_iterator() const { @@ -439,6 +453,12 @@ public: iend (it) {} + template + I_Filtered_const_iterator (T* p) : + nt (pointer(p)), + iend (nt) + {} + I_Filtered_const_iterator (Iterator it, Iterator end) : nt (it), iend (end) @@ -465,6 +485,14 @@ public: // ++nt; } + template + I_Filtered_const_iterator& operator= (const P* p) + { + nt = pointer(p); + iend =nt; + return *this; + } + /*! Access operations. */ Iterator current_iterator() const { diff --git a/HalfedgeDS/include/CGAL/HalfedgeDS_iterator.h b/HalfedgeDS/include/CGAL/HalfedgeDS_iterator.h index e5aa9493fbf..fe62a26e2c0 100644 --- a/HalfedgeDS/include/CGAL/HalfedgeDS_iterator.h +++ b/HalfedgeDS/include/CGAL/HalfedgeDS_iterator.h @@ -200,7 +200,7 @@ public: bool operator!=( const Self& i) const { return !(*this == i); } Self& operator++() { - this->nt = (*this->nt).next(); + this->nt = typename It::Iterator((*this->nt).next()); return *this; } Self operator++(int) { @@ -266,7 +266,7 @@ public: bool operator!=( const It& i) const { return !(*this == i); } Self& operator++() { - this->nt = (*this->nt).next(); + this->nt = typename It::Iterator((*this->nt).next()); return *this; } Self operator++(int) { @@ -325,7 +325,7 @@ public: bool operator!=( const Self& i) const { return !(*this == i); } Self& operator++() { - this->nt = (*this->nt).next()->opposite(); + this->nt = typename It::Iterator((*this->nt).next()->opposite()); return *this; } Self operator++(int) { @@ -338,7 +338,7 @@ public: // --------------------------------- Self& operator--() { - this->nt = (*this->nt).opposite()->prev(); + this->nt = typename It::Iterator((*this->nt).opposite()->prev()); return *this; } Self operator--(int) { @@ -389,7 +389,7 @@ public: bool operator!=( const Self& i) const { return !(*this == i); } Self& operator++() { - this->nt = (*this->nt).next()->opposite(); + this->nt = typename It::Iterator((*this->nt).next()->opposite()); return *this; } Self operator++(int) { @@ -402,7 +402,7 @@ public: // --------------------------------- Self& operator--() { - this->nt = (*this->nt).opposite()->prev(); + this->nt = typename It::Iterator((*this->nt).opposite()->prev()); return *this; } Self operator--(int) { diff --git a/STL_Extension/include/CGAL/Compact_container.h b/STL_Extension/include/CGAL/Compact_container.h index 9273df1556e..470918b5f80 100644 --- a/STL_Extension/include/CGAL/Compact_container.h +++ b/STL_Extension/include/CGAL/Compact_container.h @@ -875,7 +875,7 @@ namespace internal { m_ptr = nullptr; } - CC_iterator(pointer ptr) : m_ptr(ptr) { } + explicit CC_iterator(pointer ptr) : m_ptr(ptr) { } // Converting constructor from mutable to constant iterator template From afc6e4cae60d3285924534910ed887bf115bf5af Mon Sep 17 00:00:00 2001 From: Simon Giraudot Date: Wed, 24 Mar 2021 09:13:29 +0100 Subject: [PATCH 5/8] Use nullptr instead of NULL to fix ambiguous construction --- Linear_cell_complex/demo/Linear_cell_complex/import_moka.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Linear_cell_complex/demo/Linear_cell_complex/import_moka.h b/Linear_cell_complex/demo/Linear_cell_complex/import_moka.h index b375423d4fb..f838c085b8a 100644 --- a/Linear_cell_complex/demo/Linear_cell_complex/import_moka.h +++ b/Linear_cell_complex/demo/Linear_cell_complex/import_moka.h @@ -20,7 +20,7 @@ struct GDart Dart_handle dh; LCC::Vertex_attribute_handle vh; - GDart() : dh(NULL), vh(NULL) + GDart() : dh(nullptr), vh(nullptr) {} GDart(const GDart& adart) : dh(adart.dh), From 2923ea4a2b3d9c9ec62bbc710e9393db9a7b0ccd Mon Sep 17 00:00:00 2001 From: Simon Giraudot Date: Thu, 25 Mar 2021 10:34:21 +0100 Subject: [PATCH 6/8] Fix errors with iterator conversions --- HalfedgeDS/include/CGAL/HalfedgeDS_iterator.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/HalfedgeDS/include/CGAL/HalfedgeDS_iterator.h b/HalfedgeDS/include/CGAL/HalfedgeDS_iterator.h index fe62a26e2c0..0eb0856a890 100644 --- a/HalfedgeDS/include/CGAL/HalfedgeDS_iterator.h +++ b/HalfedgeDS/include/CGAL/HalfedgeDS_iterator.h @@ -213,7 +213,7 @@ public: // --------------------------------- Self& operator--() { - this->nt = (*this->nt).prev(); + this->nt = typename It::Iterator((*this->nt).prev()); return *this; } Self operator--(int) { @@ -279,7 +279,7 @@ public: // --------------------------------- Self& operator--() { - this->nt = (*this->nt).prev(); + this->nt = typename It::Iterator((*this->nt).prev()); return *this; } Self operator--(int) { From c11d6316b1518fe7e068b1983a068c6dc3fe27ab Mon Sep 17 00:00:00 2001 From: Simon Giraudot Date: Mon, 29 Mar 2021 15:00:44 +0200 Subject: [PATCH 7/8] Fix example (avoid incrementing end()) --- .../examples/Arrangement_on_surface_2/isolated_vertices.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Arrangement_on_surface_2/examples/Arrangement_on_surface_2/isolated_vertices.cpp b/Arrangement_on_surface_2/examples/Arrangement_on_surface_2/isolated_vertices.cpp index 9568b33952f..b8a53d7d784 100644 --- a/Arrangement_on_surface_2/examples/Arrangement_on_surface_2/isolated_vertices.cpp +++ b/Arrangement_on_surface_2/examples/Arrangement_on_surface_2/isolated_vertices.cpp @@ -27,9 +27,10 @@ int main() arr.insert_at_vertices(s4, v4, v1); // Remove the isolated vertices located in the unbounded face. - Arrangement::Vertex_iterator curr, next = arr.vertices_begin(); - for (curr = next++; curr != arr.vertices_end(); curr = next++) { + Arrangement::Vertex_iterator iter = arr.vertices_begin(); + while (iter != arr.vertices_end()) { // Keep an iterator to the next vertex, as curr might be deleted. + Arrangement::Vertex_iterator curr = iter ++; if (curr->is_isolated() && curr->face() == uf) arr.remove_isolated_vertex(curr); } From 4e4a93de3720ceafe1795b1307e8ea43ef87a8b8 Mon Sep 17 00:00:00 2001 From: Simon Giraudot Date: Wed, 31 Mar 2021 15:48:05 +0200 Subject: [PATCH 8/8] Resolve ambiguous call to operator== by overloading it with base type --- HalfedgeDS/include/CGAL/HalfedgeDS_iterator.h | 1 + 1 file changed, 1 insertion(+) diff --git a/HalfedgeDS/include/CGAL/HalfedgeDS_iterator.h b/HalfedgeDS/include/CGAL/HalfedgeDS_iterator.h index 0eb0856a890..b2bcac25a33 100644 --- a/HalfedgeDS/include/CGAL/HalfedgeDS_iterator.h +++ b/HalfedgeDS/include/CGAL/HalfedgeDS_iterator.h @@ -321,6 +321,7 @@ public: return It::operator==( It(nullptr)); } bool operator!=( std::nullptr_t p) const { return !(*this == p); } + bool operator==( const It& i) const { return It::operator==(i); } bool operator==( const Self& i) const { return It::operator==(i); } bool operator!=( const Self& i) const { return !(*this == i); }