From 12c0ec0935b1d42a880d93a6b916c4edb688ac8e Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Tue, 19 May 2020 17:11:18 +0200 Subject: [PATCH 1/3] New debug code in ... tested in `test/Triangulation_2/test_cdt_degenerate_case.cpp`. --- .../CGAL/Constrained_triangulation_2.h | 110 +++++++++++++++++- .../test/Triangulation_2/issue_4405.cpp | 32 ++++- .../test_cdt_degenerate_case.cpp | 38 +++++- 3 files changed, 176 insertions(+), 4 deletions(-) diff --git a/Triangulation_2/include/CGAL/Constrained_triangulation_2.h b/Triangulation_2/include/CGAL/Constrained_triangulation_2.h index a433e3ddd5c..3d496107b55 100644 --- a/Triangulation_2/include/CGAL/Constrained_triangulation_2.h +++ b/Triangulation_2/include/CGAL/Constrained_triangulation_2.h @@ -52,6 +52,23 @@ struct Exact_predicates_tag{}; // to be used with filtered exact number namespace internal { +#ifdef CGAL_CDT_2_DEBUG_INTERSECTIONS + struct Indentation_level { + int n = 0; + friend std::ostream& operator<<(std::ostream& os, Indentation_level level) { + return os << std::string(2*level.n, ' '); + } + Indentation_level& operator++() { ++n; return *this; } + Indentation_level& operator--() { --n; return *this; } + struct Exit_guard { + Indentation_level& level; + ~Exit_guard() { --level; } + }; + Exit_guard exit_guard() { return Exit_guard{*this}; } + Exit_guard open_new_scope() { return Exit_guard{++*this}; } + } cdt_2_indent_level; +#endif // CGAL_CDT_2_DEBUG_INTERSECTIONS + template struct Itag { typedef typename boost::mpl::if_::Is_exact, @@ -690,10 +707,25 @@ insert_constraint(Vertex_handle vaa, Vertex_handle vbb) std::stack > stack; stack.push(std::make_pair(vaa,vbb)); +#ifdef CGAL_CDT_2_DEBUG_INTERSECTIONS + std::cerr << CGAL::internal::cdt_2_indent_level + << "CT_2::insert_constraint( #" << vaa->time_stamp() << "= " << vaa->point() + << " , #" << vbb->time_stamp() << "= " << vbb->point() + << " )\n"; + auto exit_guard = CGAL::internal::cdt_2_indent_level.open_new_scope(); +#endif // CGAL_CDT_2_DEBUG_INTERSECTIONS while(! stack.empty()){ boost::tie(vaa,vbb) = stack.top(); stack.pop(); CGAL_triangulation_precondition( vaa != vbb); +#ifdef CGAL_CDT_2_DEBUG_INTERSECTIONS + std::cerr << CGAL::internal::cdt_2_indent_level + << "CT_2::insert_constraint, stack pop=( #" << vaa->time_stamp() << "= " << vaa->point() + << " , #" << vbb->time_stamp() << "= " << vbb->point() + << " ) remaining stack size: " + << stack.size() << '\n'; + CGAL_assertion(this->is_valid()); +#endif // CGAL_CDT_2_DEBUG_INTERSECTIONS Vertex_handle vi; Face_handle fr; @@ -716,10 +748,26 @@ insert_constraint(Vertex_handle vaa, Vertex_handle vbb) vi); if ( intersection) { if (vi != vaa && vi != vbb) { +#ifdef CGAL_CDT_2_DEBUG_INTERSECTIONS + std::cerr << CGAL::internal::cdt_2_indent_level + << "CT_2::insert_constraint stask push [vaa, vi] ( #" << vaa->time_stamp() << "= " << vaa->point() + << " , #" << vi->time_stamp() << "= " << vi->point() + << " )\n"; + std::cerr << CGAL::internal::cdt_2_indent_level + << "CT_2::insert_constraint stask push [vi, vbb] ( #" << vi->time_stamp() << "= " << vi->point() + << " , #" << vbb->time_stamp() << "= " << vbb->point() + << " )\n"; +#endif // CGAL_CDT_2_DEBUG_INTERSECTIONS stack.push(std::make_pair(vaa,vi)); stack.push(std::make_pair(vi,vbb)); } else{ +#ifdef CGAL_CDT_2_DEBUG_INTERSECTIONS + std::cerr << CGAL::internal::cdt_2_indent_level + << "CT_2::insert_constraint stask push [vaa, vbb]( #" << vaa->time_stamp() << "= " << vaa->point() + << " , #" << vbb->time_stamp() << "= " << vbb->point() + << " )\n"; +#endif // CGAL_CDT_2_DEBUG_INTERSECTIONS stack.push(std::make_pair(vaa,vbb)); } continue; @@ -763,6 +811,30 @@ find_intersected_faces(Vertex_handle vaa, // to deal with the case where the first crossed edge // is constrained +#ifdef CGAL_CDT_2_DEBUG_INTERSECTIONS + std::cerr << CGAL::internal::cdt_2_indent_level + << "CT_2::find_intersected_faces ( #" << vaa->time_stamp() << "= " << vaa->point() + << " , #" << vbb->time_stamp() << "= " << vbb->point() + << " )\n" + << CGAL::internal::cdt_2_indent_level + << "> current constrained edges are:\n"; + for(Constrained_edges_iterator edge_it = this->constrained_edges_begin(), + end = this->constrained_edges_end(); + edge_it != end; ++edge_it) + { + std::cerr < (#" + << edge_it->first->vertex(cw(edge_it->second))->time_stamp() + << ", #" + << edge_it->first->vertex(ccw(edge_it->second))->time_stamp() + << ")\n"; + } + std::cerr << CGAL::internal::cdt_2_indent_level + << "> current face is ( #" << current_face->vertex(0)->time_stamp() + << " #" << current_face->vertex(1)->time_stamp() + << " #" << current_face->vertex(2)->time_stamp() << " )\n"; +#endif // CGAL_CDT_2_DEBUG_INTERSECTIONS + if(current_face->is_constrained(ind)) { vi=intersect(current_face, ind, vaa, vbb); return true; @@ -909,6 +981,14 @@ intersect(Face_handle f, int i, const Point& pc = vcc->point(); const Point& pd = vdd->point(); +#ifdef CGAL_CDT_2_DEBUG_INTERSECTIONS + std::cerr << CGAL::internal::cdt_2_indent_level + << "CT_2::intersect segment ( #" << vaa->time_stamp() << "= " << vaa->point() + << " , #" << vbb->time_stamp() << "= " << vbb->point() + << " ) with edge ( #"<< vcc->time_stamp() << "= " << vcc->point() + << " , #" << vdd->time_stamp() << "= " << vdd->point() + << " )\n"; +#endif // CGAL_CDT_2_DEBUG_INTERSECTIONS Point pi; //creator for point is required here Itag itag = Itag(); bool ok = intersection(geom_traits(), pa, pb, pc, pd, pi, itag ); @@ -930,6 +1010,11 @@ intersect(Face_handle f, int i, remove_constrained_edge(f, i); vi = virtual_insert(pi, f); } +#ifdef CGAL_CDT_2_DEBUG_INTERSECTIONS + std::cerr << CGAL::internal::cdt_2_indent_level + << "CT_2::intersect, `vi` is ( #" << vi->time_stamp() << "= " << vi->point() + << " )\n"; +#endif // CGAL_CDT_2_DEBUG_INTERSECTIONS // vi == vc or vi == vd may happen even if intersection==true // due to approximate construction of the intersection @@ -1185,6 +1270,14 @@ void Constrained_triangulation_2:: remove_constrained_edge(Face_handle f, int i) { +#ifdef CGAL_CDT_2_DEBUG_INTERSECTIONS + std::cerr << CGAL::internal::cdt_2_indent_level + << "CT_2::remove_constrained_edge ( #" + << f->vertex(cw(i))->time_stamp() + << ", #" + << f->vertex(ccw(i))->time_stamp() + << ")\n"; +#endif // CGAL_CDT_2_DEBUG_INTERSECTIONS f->set_constraint(i, false); if (dimension() == 2) (f->neighbor(i))->set_constraint(this->mirror_index(f,i), false); @@ -1443,7 +1536,8 @@ intersection(const Gt& gt, if(!result) return result; if(pi == pa || pi == pb || pi == pc || pi == pd) { #ifdef CGAL_CDT_2_DEBUG_INTERSECTIONS - std::cerr << " CT_2::intersection: intersection is an existing point " + std::cerr << CGAL::internal::cdt_2_indent_level + << " CT_2::intersection: intersection is an existing point " << pi << std::endl; #endif return result; @@ -1465,7 +1559,8 @@ intersection(const Gt& gt, if(do_overlap(bb, bbox(pd))) pi = pd; #ifdef CGAL_CDT_2_DEBUG_INTERSECTIONS if(pi == pa || pi == pb || pi == pc || pi == pd) { - std::cerr << " CT_2::intersection: intersection SNAPPED to an existing point " + std::cerr << CGAL::internal::cdt_2_indent_level + << " CT_2::intersection: intersection SNAPPED to an existing point " << pi << std::endl; } #endif @@ -1503,6 +1598,17 @@ compute_intersection(const Gt& gt, construct_segment=gt.construct_segment_2_object(); Object result = compute_intersec(construct_segment(pa,pb), construct_segment(pc,pd)); +#ifdef CGAL_CDT_2_DEBUG_INTERSECTIONS + typename Gt::Segment_2 s; + if(assign(s, result)) { + std::cerr << CGAL::internal::cdt_2_indent_level + << "compute_intersection: " << s << '\n'; + } + if(assign(pi, result)) { + std::cerr << CGAL::internal::cdt_2_indent_level + << "compute_intersection: " << pi << '\n'; + } +#endif // CGAL_CDT_2_DEBUG_INTERSECTIONS return assign(pi, result); } diff --git a/Triangulation_2/test/Triangulation_2/issue_4405.cpp b/Triangulation_2/test/Triangulation_2/issue_4405.cpp index 89199d56141..770cd6c761c 100644 --- a/Triangulation_2/test/Triangulation_2/issue_4405.cpp +++ b/Triangulation_2/test/Triangulation_2/issue_4405.cpp @@ -9,13 +9,43 @@ typedef CGAL::Epick Kernel; typedef Kernel::FT FieldNumberType; typedef Kernel::Point_2 Point2; typedef Kernel::Point_3 Point3; + +template +class My_vertex_base : public Vb { + std::size_t time_stamp_; +public: + My_vertex_base() : Vb(), time_stamp_(-1) { + } + + My_vertex_base(const My_vertex_base& other) : + Vb(other), + time_stamp_(other.time_stamp_) + {} + + typedef CGAL::Tag_true Has_timestamp; + + std::size_t time_stamp() const { + return time_stamp_; + } + void set_time_stamp(const std::size_t& ts) { + time_stamp_ = ts; + } + + template < class TDS > + struct Rebind_TDS { + typedef typename Vb::template Rebind_TDS::Other Vb2; + typedef My_vertex_base Other; + }; +}; + struct FaceInfo2 { unsigned long long m_id; }; typedef CGAL::Projection_traits_xy_3 TriangulationTraits; typedef CGAL::Triangulation_vertex_base_with_id_2 VertexBaseWithId; -typedef CGAL::Triangulation_vertex_base_2 VertexBase; +typedef My_vertex_base Vb2; +typedef CGAL::Triangulation_vertex_base_2 VertexBase; typedef CGAL::Triangulation_face_base_with_info_2 FaceBaseWithInfo; typedef CGAL::Constrained_triangulation_face_base_2 FaceBase; typedef CGAL::Triangulation_data_structure_2 TriangulationData; diff --git a/Triangulation_2/test/Triangulation_2/test_cdt_degenerate_case.cpp b/Triangulation_2/test/Triangulation_2/test_cdt_degenerate_case.cpp index 5dc1d1c7afa..952a7428f5b 100644 --- a/Triangulation_2/test/Triangulation_2/test_cdt_degenerate_case.cpp +++ b/Triangulation_2/test/Triangulation_2/test_cdt_degenerate_case.cpp @@ -1,3 +1,4 @@ +#define CGAL_CDT_2_DEBUG_INTERSECTIONS 1 #include #include #include @@ -5,7 +6,41 @@ typedef CGAL::Exact_predicates_inexact_constructions_kernel EPIC; typedef EPIC::Point_2 Point_2; -typedef CGAL::Triangulation_vertex_base_2 Vb; + +template +class My_vertex_base : public Vb { + std::size_t time_stamp_; +public: + My_vertex_base() : Vb(), time_stamp_(-1) { + } + + My_vertex_base(const My_vertex_base& other) : + Vb(other), + time_stamp_(other.time_stamp_) + {} + + typedef CGAL::Tag_true Has_timestamp; + + std::size_t time_stamp() const { + return time_stamp_; + } + void set_time_stamp(const std::size_t& ts) { + time_stamp_ = ts; + } + + template < class TDS > + struct Rebind_TDS { + typedef typename Vb::template Rebind_TDS::Other Vb2; + typedef My_vertex_base Other; + }; +}; + +#ifdef CGAL_CDT_2_DEBUG_INTERSECTIONS +using Vb = My_vertex_base>; +#else +using Vb = CGAL::Triangulation_vertex_base_2; +#endif + typedef CGAL::Constrained_triangulation_face_base_2 Fb; typedef CGAL::Triangulation_data_structure_2 TDS; typedef CGAL::Exact_predicates_tag Itag; @@ -14,6 +49,7 @@ typedef CGAL::Constrained_triangulation_plus_2 CDTp2; template void test() { + std::cerr.precision(17); CDT cdt; cdt.insert_constraint(Point_2( 48.0923419883269, 299.7232779774145 ), Point_2( 66.05373710316852, 434.231770798343 )); From 3cd52cdc3e4eddc17ad33fe68fdc6db2b8c67028 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Tue, 19 May 2020 17:12:02 +0200 Subject: [PATCH 2/3] Fix a bug in The function parameter was locally shadowed by a local `int i` variable! Then the edge `(f, i)` in the call `remove_constrained_edge(f, i)` was the wrong one! The bug was introduced by ecfd82e287378977a325ae34e21087a2075a05cd, ten years ago. The bug was only triggered on degenerated cases, when two constraints intersect, but the code failed to compute the intersection. Then, if the intersection vertex `vi` was either `vaa` or `vbb` (and not `vcc` or `vdd`), then the edge `(f, i)` was the wrong one. --- Triangulation_2/include/CGAL/Constrained_triangulation_2.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Triangulation_2/include/CGAL/Constrained_triangulation_2.h b/Triangulation_2/include/CGAL/Constrained_triangulation_2.h index 3d496107b55..916acdcfd97 100644 --- a/Triangulation_2/include/CGAL/Constrained_triangulation_2.h +++ b/Triangulation_2/include/CGAL/Constrained_triangulation_2.h @@ -995,8 +995,8 @@ intersect(Face_handle f, int i, Vertex_handle vi; if ( !ok) { //intersection detected but not computed - int i = limit_intersection(geom_traits(), pa, pb, pc, pd, itag); - switch(i){ + int int_index = limit_intersection(geom_traits(), pa, pb, pc, pd, itag); + switch(int_index){ case 0 : vi = vaa; break; case 1 : vi = vbb; break; case 2 : vi = vcc; break; From 0ea385712fd158cd86c9cc20a84a40402a441b49 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Fri, 22 May 2020 14:52:18 +0200 Subject: [PATCH 3/3] Fix compatibility with C++03 --- STL_Extension/include/CGAL/Compact_container.h | 2 +- .../include/CGAL/Constrained_triangulation_2.h | 10 ++++++---- .../test/Triangulation_2/test_cdt_degenerate_case.cpp | 4 ++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/STL_Extension/include/CGAL/Compact_container.h b/STL_Extension/include/CGAL/Compact_container.h index 02d21c775ea..0e9d638a33d 100644 --- a/STL_Extension/include/CGAL/Compact_container.h +++ b/STL_Extension/include/CGAL/Compact_container.h @@ -1320,7 +1320,7 @@ namespace handle { template struct Hash_functor; template - struct Hash_functor>{ + struct Hash_functor >{ std::size_t operator()(const CC_iterator& i) { diff --git a/Triangulation_2/include/CGAL/Constrained_triangulation_2.h b/Triangulation_2/include/CGAL/Constrained_triangulation_2.h index 916acdcfd97..6fc2ff05675 100644 --- a/Triangulation_2/include/CGAL/Constrained_triangulation_2.h +++ b/Triangulation_2/include/CGAL/Constrained_triangulation_2.h @@ -54,18 +54,20 @@ namespace internal { #ifdef CGAL_CDT_2_DEBUG_INTERSECTIONS struct Indentation_level { - int n = 0; + int n; + Indentation_level() : n(0) {} friend std::ostream& operator<<(std::ostream& os, Indentation_level level) { return os << std::string(2*level.n, ' '); } Indentation_level& operator++() { ++n; return *this; } Indentation_level& operator--() { --n; return *this; } struct Exit_guard { + Exit_guard(Indentation_level& level): level(level) { ++level; } + Exit_guard(const Exit_guard& other) : level(other.level) { ++level; } Indentation_level& level; ~Exit_guard() { --level; } }; - Exit_guard exit_guard() { return Exit_guard{*this}; } - Exit_guard open_new_scope() { return Exit_guard{++*this}; } + Exit_guard open_new_scope() { return Exit_guard(*this); } } cdt_2_indent_level; #endif // CGAL_CDT_2_DEBUG_INTERSECTIONS @@ -712,7 +714,7 @@ insert_constraint(Vertex_handle vaa, Vertex_handle vbb) << "CT_2::insert_constraint( #" << vaa->time_stamp() << "= " << vaa->point() << " , #" << vbb->time_stamp() << "= " << vbb->point() << " )\n"; - auto exit_guard = CGAL::internal::cdt_2_indent_level.open_new_scope(); + internal::Indentation_level::Exit_guard exit_guard = CGAL::internal::cdt_2_indent_level.open_new_scope(); #endif // CGAL_CDT_2_DEBUG_INTERSECTIONS while(! stack.empty()){ boost::tie(vaa,vbb) = stack.top(); diff --git a/Triangulation_2/test/Triangulation_2/test_cdt_degenerate_case.cpp b/Triangulation_2/test/Triangulation_2/test_cdt_degenerate_case.cpp index 952a7428f5b..b4de0e1e0f7 100644 --- a/Triangulation_2/test/Triangulation_2/test_cdt_degenerate_case.cpp +++ b/Triangulation_2/test/Triangulation_2/test_cdt_degenerate_case.cpp @@ -36,9 +36,9 @@ public: }; #ifdef CGAL_CDT_2_DEBUG_INTERSECTIONS -using Vb = My_vertex_base>; +typedef My_vertex_base > Vb; #else -using Vb = CGAL::Triangulation_vertex_base_2; +typedef CGAL::Triangulation_vertex_base_2 Vb; #endif typedef CGAL::Constrained_triangulation_face_base_2 Fb;