From 2341612609ff743dfe6079c39130db37e93c382e Mon Sep 17 00:00:00 2001 From: Simon Giraudot Date: Thu, 23 Jul 2020 11:47:52 +0200 Subject: [PATCH 1/3] Add chain of inner CCB handling to Arr_dcel_base --- .../include/CGAL/Arr_dcel_base.h | 85 ++++++++++++++++--- 1 file changed, 71 insertions(+), 14 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 2c685385abc..c30b0ddfca6 100644 --- a/Arrangement_on_surface_2/include/CGAL/Arr_dcel_base.h +++ b/Arrangement_on_surface_2/include/CGAL/Arr_dcel_base.h @@ -478,7 +478,16 @@ public: const Inner_ccb* inner_ccb() const { CGAL_precondition(is_on_inner_ccb()); - return (reinterpret_cast(_clean_pointer(this->p_comp))); + + const Inner_ccb* out = reinterpret_cast(_clean_pointer(this->p_comp)); + if (out->is_valid()) + return out; + // else + while (!out->is_valid()) + out = out->next(); + const_cast(this)->set_inner_ccb(out); + + return out; } /*! Get an incident inner CCB (non-const version). @@ -487,11 +496,20 @@ public: Inner_ccb* inner_ccb() { CGAL_precondition(is_on_inner_ccb()); - return (reinterpret_cast(_clean_pointer(this->p_comp))); + + Inner_ccb* out = reinterpret_cast(_clean_pointer(this->p_comp)); + if (out->is_valid()) + return out; + // else + while (!out->is_valid()) + out = out->next(); + set_inner_ccb(out); + + return out; } /*! Set the incident inner CCB. */ - void set_inner_ccb(Inner_ccb *ic) + void set_inner_ccb(const Inner_ccb *ic) { // Set the component pointer and set its LSB. this->p_comp = _set_lsb(ic); @@ -768,18 +786,27 @@ public: typedef typename Face::Inner_ccb_iterator Inner_ccb_iterator; private: - Face* p_f; // The face the contains the CCB in its interior. + union + { + Face* f; // The face the contains the CCB in its interior. + Arr_inner_ccb* icc; // next inner CCB in chain to valid icc + } f_or_icc; Inner_ccb_iterator iter; // The inner CCB identifier. - bool iter_is_not_singular; + enum + { + ITER_IS_SINGULAR, + ITER_IS_NOT_SINGULAR, + INVALID + } status; public: /*! Default constructor. */ - Arr_inner_ccb() : p_f(nullptr), iter_is_not_singular(false) {} + Arr_inner_ccb() : status(ITER_IS_SINGULAR) { f_or_icc.f = nullptr; } /*! Copy constructor. */ Arr_inner_ccb(const Arr_inner_ccb& other) : - p_f(other.p_f), iter_is_not_singular(other.iter_is_not_singular) - { if (other.iter_is_not_singular) iter = other.iter; } + f_or_icc(other.f_or_icc), status(other.status) + { if (other.status == ITER_IS_NOT_SINGULAR) iter = other.iter; } /*! Get a halfedge along the component (const version). */ const Halfedge* halfedge() const { return (*iter); } @@ -791,33 +818,63 @@ public: void set_halfedge(Halfedge *he) { *iter = he; } /*! Get the incident face (const version). */ - const Face* face() const { return (p_f); } + const Face* face() const + { + CGAL_assertion (status != INVALID); + return f_or_icc.f; + } /*! Get the incident face (non-const version). */ - Face* face() { return (p_f); } + Face* face() + { + CGAL_assertion (status != INVALID); + return f_or_icc.f; + } /*! Set the incident face. */ - void set_face(Face* f) { p_f = f; } + void set_face(Face* f) + { + CGAL_assertion (status != INVALID); + f_or_icc.f = f; + } /*! Get the iterator (const version). */ Inner_ccb_iterator iterator() const { - CGAL_assertion(iter_is_not_singular); + CGAL_assertion(status == ITER_IS_NOT_SINGULAR); return (iter); } /*! Get the iterator (non-const version). */ Inner_ccb_iterator iterator() { - CGAL_assertion(iter_is_not_singular); + CGAL_assertion(status == ITER_IS_NOT_SINGULAR); return (iter); } /*! Set the inner CCB iterator. */ void set_iterator(Inner_ccb_iterator it) { + CGAL_assertion (status != INVALID); iter = it; - iter_is_not_singular = true; + status = ITER_IS_NOT_SINGULAR; + } + + /*! Check validity */ + bool is_valid() const { return (status != INVALID); } + + /*! Get the next CCB to primary chain. */ + Arr_inner_ccb* next() const + { + CGAL_assertion (status == INVALID); + return f_or_icc.icc; + } + + /*! Set the next CCB to primary chain. */ + void set_next(Arr_inner_ccb* next) + { + status = INVALID; + f_or_icc.icc = next; } }; From 2f9cdd068b884b8b530b2c948c9defa1e2a3b1f9 Mon Sep 17 00:00:00 2001 From: Simon Giraudot Date: Thu, 23 Jul 2020 14:37:18 +0200 Subject: [PATCH 2/3] Sweep mode for Arrangement_2 --- .../include/CGAL/Arr_dcel_base.h | 53 ++++++++++++++----- .../Arrangement_on_surface_2_impl.h | 22 +++++--- .../include/CGAL/Arrangement_on_surface_2.h | 42 +++++++++++++++ .../Arr_construction_ss_visitor.h | 19 ++++++- .../Surface_sweep_2/Arr_overlay_ss_visitor.h | 2 + 5 files changed, 118 insertions(+), 20 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 c30b0ddfca6..a32d7eb0de7 100644 --- a/Arrangement_on_surface_2/include/CGAL/Arr_dcel_base.h +++ b/Arrangement_on_surface_2/include/CGAL/Arr_dcel_base.h @@ -482,11 +482,10 @@ public: const Inner_ccb* out = reinterpret_cast(_clean_pointer(this->p_comp)); if (out->is_valid()) return out; - // else - while (!out->is_valid()) - out = out->next(); - const_cast(this)->set_inner_ccb(out); + // else + out = const_cast(out)->reduce_path(); + const_cast(this)->set_inner_ccb(out); return out; } @@ -500,14 +499,19 @@ public: Inner_ccb* out = reinterpret_cast(_clean_pointer(this->p_comp)); if (out->is_valid()) return out; - // else - while (!out->is_valid()) - out = out->next(); - set_inner_ccb(out); + // else + out = out->reduce_path(); + set_inner_ccb(out); return out; } + Inner_ccb* inner_ccb_no_redirect() + { + CGAL_precondition(is_on_inner_ccb()); + return reinterpret_cast(_clean_pointer(this->p_comp)); + } + /*! Set the incident inner CCB. */ void set_inner_ccb(const Inner_ccb *ic) { @@ -809,13 +813,25 @@ public: { if (other.status == ITER_IS_NOT_SINGULAR) iter = other.iter; } /*! Get a halfedge along the component (const version). */ - const Halfedge* halfedge() const { return (*iter); } + const Halfedge* halfedge() const + { + CGAL_assertion (is_valid()); + return (*iter); + } /*! Get a halfedge along the component (non-const version). */ - Halfedge* halfedge() { return (*iter); } + Halfedge* halfedge() + { + CGAL_assertion (is_valid()); + return (*iter); + } /*! Set a representative halfedge for the component. */ - void set_halfedge(Halfedge *he) { *iter = he; } + void set_halfedge(Halfedge *he) + { + CGAL_assertion (is_valid()); + *iter = he; + } /*! Get the incident face (const version). */ const Face* face() const @@ -855,7 +871,7 @@ public: /*! Set the inner CCB iterator. */ void set_iterator(Inner_ccb_iterator it) { - CGAL_assertion (status != INVALID); + CGAL_assertion (is_valid()); iter = it; status = ITER_IS_NOT_SINGULAR; } @@ -876,6 +892,15 @@ public: status = INVALID; f_or_icc.icc = next; } + + Arr_inner_ccb* reduce_path() + { + if (is_valid()) + return this; + // else + f_or_icc.icc = f_or_icc.icc->reduce_path(); + return f_or_icc.icc; + } }; /*! \class @@ -999,6 +1024,7 @@ public: typedef typename Face_list::iterator Face_iterator; typedef CGAL::N_step_adaptor_derived Edge_iterator; + typedef typename Inner_ccb_list::iterator Inner_ccb_iterator; // Definitions of const iterators. typedef typename Vertex_list::const_iterator Vertex_const_iterator; @@ -1075,6 +1101,9 @@ public: { return make_prevent_deref_range(edges_begin(), edges_end()); } + + Inner_ccb_iterator inner_ccbs_begin() { return in_ccbs.begin(); } + Inner_ccb_iterator inner_ccbs_end() { return in_ccbs.end(); } //@} /// \name Obtaining constant iterators. diff --git a/Arrangement_on_surface_2/include/CGAL/Arrangement_2/Arrangement_on_surface_2_impl.h b/Arrangement_on_surface_2/include/CGAL/Arrangement_2/Arrangement_on_surface_2_impl.h index 8c3fd571803..7dca8f1abae 100644 --- a/Arrangement_on_surface_2/include/CGAL/Arrangement_2/Arrangement_on_surface_2_impl.h +++ b/Arrangement_on_surface_2/include/CGAL/Arrangement_2/Arrangement_on_surface_2_impl.h @@ -2739,14 +2739,22 @@ _insert_at_vertices(DHalfedge* he_to, he1->set_inner_ccb(ic1); he2->set_inner_ccb(ic1); - // Make all halfedges along ic2 to point to ic1. - DHalfedge* curr; + if (m_sweep_mode) + { + CGAL_assertion(ic1->is_valid()); + CGAL_assertion(ic2->is_valid()); + ic2->set_next(ic1); + } + else + { + // Make all halfedges along ic2 to point to ic1. + DHalfedge* curr; + for (curr = he2->next(); curr != he1; curr = curr->next()) + curr->set_inner_ccb(ic1); - for (curr = he2->next(); curr != he1; curr = curr->next()) - curr->set_inner_ccb(ic1); - - // Delete the redundant inner CCB. - _dcel().delete_inner_ccb(ic2); + // Delete the redundant inner CCB. + _dcel().delete_inner_ccb(ic2); + } // Notify the observers that we have merged the two inner CCBs. _notify_after_merge_inner_ccb(fh, (Halfedge_handle(he1))->ccb()); diff --git a/Arrangement_on_surface_2/include/CGAL/Arrangement_on_surface_2.h b/Arrangement_on_surface_2/include/CGAL/Arrangement_on_surface_2.h index ddcc1c6229b..e8756fe562d 100644 --- a/Arrangement_on_surface_2/include/CGAL/Arrangement_on_surface_2.h +++ b/Arrangement_on_surface_2/include/CGAL/Arrangement_on_surface_2.h @@ -909,6 +909,14 @@ protected: bool m_own_traits; // inidicates whether the geometry // traits should be freed up. + bool m_sweep_mode = false; + // sweep mode efficiently + // merges inner CCB but + // keeps invalid inner CCB + // and memory overhead that + // should be cleaned + // afterwards + public: /// \name Constructors. //@{ @@ -939,6 +947,9 @@ public: /*! Destructor. */ virtual ~Arrangement_on_surface_2(); + /*! Change mode. */ + void set_sweep_mode (bool mode) { m_sweep_mode = mode; } + /*! Clear the arrangement. */ virtual void clear(); //@} @@ -1516,6 +1527,37 @@ public: //@} + /*! + * Cleans the inner CCB if sweep mode was used, by removing all + * non-valid inner CCBs + */ + void clean_inner_ccbs() + { + for (DHalfedge_iter he = _dcel().halfedges_begin(); + he != _dcel().halfedges_end(); ++ he) + { + if (!he->is_on_inner_ccb()) + continue; + + DInner_ccb* ic1 = he->inner_ccb_no_redirect(); + if (ic1->is_valid()) + continue; + + DInner_ccb* ic2 = he->inner_ccb(); + if (!ic2->halfedge()->is_on_inner_ccb() + || ic2->halfedge()->inner_ccb_no_redirect() != ic2) + ic2->set_halfedge(&(*he)); + } + + typename Dcel::Inner_ccb_iterator it = _dcel().inner_ccbs_begin(); + while (it != _dcel().inner_ccbs_end()) + { + typename Dcel::Inner_ccb_iterator current = it ++; + if (!current->is_valid()) + _dcel().delete_inner_ccb(&*current); + } + } + protected: /// \name Determining the boundary-side conditions. //@{ diff --git a/Arrangement_on_surface_2/include/CGAL/Surface_sweep_2/Arr_construction_ss_visitor.h b/Arrangement_on_surface_2/include/CGAL/Surface_sweep_2/Arr_construction_ss_visitor.h index c8c9b9b57f0..da3f13d1934 100644 --- a/Arrangement_on_surface_2/include/CGAL/Surface_sweep_2/Arr_construction_ss_visitor.h +++ b/Arrangement_on_surface_2/include/CGAL/Surface_sweep_2/Arr_construction_ss_visitor.h @@ -142,6 +142,9 @@ public: /* A notification issued before the sweep process starts. */ inline void before_sweep(); + /* A notification issued after the sweep process stops. */ + inline void after_sweep(); + /*! * A notification invoked before the sweep-line starts handling the given * event. @@ -267,7 +270,21 @@ private: // Notifies the helper that the sweep process now starts. template void Arr_construction_ss_visitor::before_sweep() -{ m_helper.before_sweep(); } +{ + m_helper.before_sweep(); + m_arr->set_sweep_mode(true); +} + + +//----------------------------------------------------------------------------- +// A notification issued after the sweep process stops. +template +void Arr_construction_ss_visitor::after_sweep() +{ + m_arr->clean_inner_ccbs(); + m_arr->set_sweep_mode(false); +} + //----------------------------------------------------------------------------- // A notification invoked before the sweep-line starts handling the given diff --git a/Arrangement_on_surface_2/include/CGAL/Surface_sweep_2/Arr_overlay_ss_visitor.h b/Arrangement_on_surface_2/include/CGAL/Surface_sweep_2/Arr_overlay_ss_visitor.h index f9f697a3b40..8fbcc861d5f 100644 --- a/Arrangement_on_surface_2/include/CGAL/Surface_sweep_2/Arr_overlay_ss_visitor.h +++ b/Arrangement_on_surface_2/include/CGAL/Surface_sweep_2/Arr_overlay_ss_visitor.h @@ -552,6 +552,8 @@ Arr_overlay_ss_visitor::update_event(Event* e, template void Arr_overlay_ss_visitor::after_sweep() { + Base::after_sweep(); + // Notify boundary vertices: typename Vertex_map::iterator it; for (it = m_vertices_map.begin(); it != m_vertices_map.end(); ++it) { From becf548ee12a303f73c35079562df11f01b14c04 Mon Sep 17 00:00:00 2001 From: Simon Giraudot Date: Wed, 12 Aug 2020 12:46:49 +0200 Subject: [PATCH 3/3] Update from review --- .../include/CGAL/Arr_dcel_base.h | 38 +++++++++---------- .../Arrangement_on_surface_2_impl.h | 2 + .../include/CGAL/Arrangement_on_surface_2.h | 10 +++-- .../Arr_construction_ss_visitor.h | 2 +- 4 files changed, 28 insertions(+), 24 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 a32d7eb0de7..03381dcd760 100644 --- a/Arrangement_on_surface_2/include/CGAL/Arr_dcel_base.h +++ b/Arrangement_on_surface_2/include/CGAL/Arr_dcel_base.h @@ -483,10 +483,13 @@ public: if (out->is_valid()) return out; - // else - out = const_cast(out)->reduce_path(); - const_cast(this)->set_inner_ccb(out); - return out; + // else reduce path and get valid iccb + const Inner_ccb* valid = out->next(); + while (!valid->is_valid()) + valid = valid->next(); + const_cast(out)->set_next(const_cast(valid)); + const_cast(this)->set_inner_ccb(valid); + return valid; } /*! Get an incident inner CCB (non-const version). @@ -500,10 +503,13 @@ public: if (out->is_valid()) return out; - // else - out = out->reduce_path(); - set_inner_ccb(out); - return out; + // else reduce path and get valid iccb + Inner_ccb* valid = out->next(); + while (!valid->is_valid()) + valid = valid->next(); + out->set_next(valid); + set_inner_ccb(valid); + return valid; } Inner_ccb* inner_ccb_no_redirect() @@ -798,9 +804,11 @@ private: Inner_ccb_iterator iter; // The inner CCB identifier. enum { - ITER_IS_SINGULAR, - ITER_IS_NOT_SINGULAR, - INVALID + ITER_IS_SINGULAR, // singular = default iterator, not initialized + ITER_IS_NOT_SINGULAR, // not singular = iterator was assigned and is valid + INVALID // invalid = the inner CCB is invalid and + // only links to another inner CCB + // in chain to valid CCB } status; public: @@ -893,14 +901,6 @@ public: f_or_icc.icc = next; } - Arr_inner_ccb* reduce_path() - { - if (is_valid()) - return this; - // else - f_or_icc.icc = f_or_icc.icc->reduce_path(); - return f_or_icc.icc; - } }; /*! \class diff --git a/Arrangement_on_surface_2/include/CGAL/Arrangement_2/Arrangement_on_surface_2_impl.h b/Arrangement_on_surface_2/include/CGAL/Arrangement_2/Arrangement_on_surface_2_impl.h index 7dca8f1abae..8c1581f45e3 100644 --- a/Arrangement_on_surface_2/include/CGAL/Arrangement_2/Arrangement_on_surface_2_impl.h +++ b/Arrangement_on_surface_2/include/CGAL/Arrangement_2/Arrangement_on_surface_2_impl.h @@ -2741,6 +2741,8 @@ _insert_at_vertices(DHalfedge* he_to, if (m_sweep_mode) { + // Inner CCB are obtained using Halfedge::inner_ccb() which + // performs path reduction and always return valid iCCB CGAL_assertion(ic1->is_valid()); CGAL_assertion(ic2->is_valid()); ic2->set_next(ic1); diff --git a/Arrangement_on_surface_2/include/CGAL/Arrangement_on_surface_2.h b/Arrangement_on_surface_2/include/CGAL/Arrangement_on_surface_2.h index e8756fe562d..8df551ee7aa 100644 --- a/Arrangement_on_surface_2/include/CGAL/Arrangement_on_surface_2.h +++ b/Arrangement_on_surface_2/include/CGAL/Arrangement_on_surface_2.h @@ -1531,7 +1531,7 @@ public: * Cleans the inner CCB if sweep mode was used, by removing all * non-valid inner CCBs */ - void clean_inner_ccbs() + void clean_inner_ccbs_after_sweep() { for (DHalfedge_iter he = _dcel().halfedges_begin(); he != _dcel().halfedges_end(); ++ he) @@ -1543,10 +1543,12 @@ public: if (ic1->is_valid()) continue; + // Calling Halfedge::inner_ccb() reduces the path and makes the + // halfedge point to a correct CCB DInner_ccb* ic2 = he->inner_ccb(); - if (!ic2->halfedge()->is_on_inner_ccb() - || ic2->halfedge()->inner_ccb_no_redirect() != ic2) - ic2->set_halfedge(&(*he)); + CGAL_USE(ic2); + CGAL_assertion (ic2->halfedge()->is_on_inner_ccb() + && ic2->halfedge()->inner_ccb_no_redirect() == ic2); } typename Dcel::Inner_ccb_iterator it = _dcel().inner_ccbs_begin(); diff --git a/Arrangement_on_surface_2/include/CGAL/Surface_sweep_2/Arr_construction_ss_visitor.h b/Arrangement_on_surface_2/include/CGAL/Surface_sweep_2/Arr_construction_ss_visitor.h index da3f13d1934..1e3544ec566 100644 --- a/Arrangement_on_surface_2/include/CGAL/Surface_sweep_2/Arr_construction_ss_visitor.h +++ b/Arrangement_on_surface_2/include/CGAL/Surface_sweep_2/Arr_construction_ss_visitor.h @@ -281,7 +281,7 @@ void Arr_construction_ss_visitor::before_sweep() template void Arr_construction_ss_visitor::after_sweep() { - m_arr->clean_inner_ccbs(); + m_arr->clean_inner_ccbs_after_sweep(); m_arr->set_sweep_mode(false); }