From 7fe3a5aa4bdf98b707073d3b24baf591675423d2 Mon Sep 17 00:00:00 2001 From: Andreas Fabri Date: Wed, 12 Feb 2025 11:46:12 +0000 Subject: [PATCH 1/5] Polyline_simplification: Fix unremovble vertices --- .../CGAL/Polyline_simplification_2/simplify.h | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/Polyline_simplification_2/include/CGAL/Polyline_simplification_2/simplify.h b/Polyline_simplification_2/include/CGAL/Polyline_simplification_2/simplify.h index d5497788a02..f6435051313 100644 --- a/Polyline_simplification_2/include/CGAL/Polyline_simplification_2/simplify.h +++ b/Polyline_simplification_2/include/CGAL/Polyline_simplification_2/simplify.h @@ -143,9 +143,10 @@ public: } // endpoints of constraints are unremovable - // vertices which are not endpoint and have != 2 incident constrained edges are unremovable + // vertices which have more than 1 constraint passing through are unremovable void initialize_unremovable() { + std::unordered_map degrees; Constraint_iterator cit = pct.constraints_begin(), e = pct.constraints_end(); for(; cit!=e; ++cit){ Constraint_id cid = *cit; @@ -154,6 +155,8 @@ public: (*it)->set_removable(false); ++it; for(; it != ite; ++it){ + Vertex_handle vh = *it; + ++degrees[vh]; if((boost::next(it) != ite) && (boost::prior(it)== boost::next(it))){ (*it)->set_removable(false); } @@ -162,19 +165,8 @@ public: (*it)->set_removable(false); } - std::unordered_map degrees; - for (Constrained_edges_iterator it = pct.constrained_edges_begin(); it != pct.constrained_edges_end(); ++it) { - Edge e = *it; - Face_handle fh = e.first; - int ei = e.second; - Vertex_handle vh = fh->vertex(pct.cw(ei)); - ++degrees[vh]; - vh = fh->vertex(pct.ccw(ei)); - ++degrees[vh]; - } - for(Finite_vertices_iterator it = pct.finite_vertices_begin(); it != pct.finite_vertices_end(); ++it){ - if( it->is_removable() && (degrees[it] != 2) ){ + if( it->is_removable() && (degrees[it] > 1) ){ it->set_removable(false); } } From 33eec618303344d5ddb24f6ac7d1ca6ae65775f1 Mon Sep 17 00:00:00 2001 From: Andreas Fabri Date: Wed, 12 Feb 2025 12:01:03 +0000 Subject: [PATCH 2/5] Add testcase --- .../Polyline_simplification_2/CMakeLists.txt | 1 + .../Polyline_simplification_2/issue-8735.cpp | 34 +++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 Polyline_simplification_2/test/Polyline_simplification_2/issue-8735.cpp diff --git a/Polyline_simplification_2/test/Polyline_simplification_2/CMakeLists.txt b/Polyline_simplification_2/test/Polyline_simplification_2/CMakeLists.txt index 954afd626ab..82a1b98bee1 100644 --- a/Polyline_simplification_2/test/Polyline_simplification_2/CMakeLists.txt +++ b/Polyline_simplification_2/test/Polyline_simplification_2/CMakeLists.txt @@ -8,5 +8,6 @@ project(Polyline_simplification_2_Tests) find_package(CGAL REQUIRED) create_single_source_cgal_program( "issue-5774.cpp" ) +create_single_source_cgal_program( "issue-8735.cpp" ) create_single_source_cgal_program( "simplify_polygon_test.cpp" ) create_single_source_cgal_program( "simplify_polyline_with_duplicate_points.cpp" ) diff --git a/Polyline_simplification_2/test/Polyline_simplification_2/issue-8735.cpp b/Polyline_simplification_2/test/Polyline_simplification_2/issue-8735.cpp new file mode 100644 index 00000000000..11f2cb75643 --- /dev/null +++ b/Polyline_simplification_2/test/Polyline_simplification_2/issue-8735.cpp @@ -0,0 +1,34 @@ +#include +#include +#include +#include + +namespace PS = CGAL::Polyline_simplification_2; +typedef CGAL::Exact_predicates_exact_constructions_kernel K; +typedef PS::Vertex_base_2 Vb; +typedef CGAL::Constrained_triangulation_face_base_2 Fb; +typedef CGAL::Triangulation_data_structure_2 TDS; +typedef CGAL::Constrained_Delaunay_triangulation_2 CDT; +typedef CGAL::Constrained_triangulation_plus_2 CT; +typedef CT::Point Point; +typedef PS::Stop_above_cost_threshold Stop; +typedef PS::Squared_distance_cost Cost; + +int main() +{ + double tolerance = 100; + CT ct; + std::vector pts; + + pts.push_back(CT::Point(0, 0)); + pts.push_back(CT::Point(2, 0)); + pts.push_back(CT::Point(1, 0)); + pts.push_back(CT::Point(3, 0)); + pts.push_back(CT::Point(4, 1)); + tolerance = 100; + ct.insert_constraint(pts.begin(), pts.end(), false); + + PS::simplify(ct, Cost(), Stop(tolerance * tolerance), false); + + return 0; +} From bc6d19e11bb6e0b7cf22d209143c18e2bdfa04ce Mon Sep 17 00:00:00 2001 From: Andreas Fabri Date: Wed, 12 Feb 2025 17:35:28 +0000 Subject: [PATCH 3/5] The bug was to compare prior and next, whereas we have to compare *prior with *next --- .../CGAL/Polyline_simplification_2/simplify.h | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/Polyline_simplification_2/include/CGAL/Polyline_simplification_2/simplify.h b/Polyline_simplification_2/include/CGAL/Polyline_simplification_2/simplify.h index f6435051313..76f7468d469 100644 --- a/Polyline_simplification_2/include/CGAL/Polyline_simplification_2/simplify.h +++ b/Polyline_simplification_2/include/CGAL/Polyline_simplification_2/simplify.h @@ -143,10 +143,9 @@ public: } // endpoints of constraints are unremovable - // vertices which have more than 1 constraint passing through are unremovable + // vertices which are not endpoint and have != 2 incident constrained edges are unremovable void initialize_unremovable() { - std::unordered_map degrees; Constraint_iterator cit = pct.constraints_begin(), e = pct.constraints_end(); for(; cit!=e; ++cit){ Constraint_id cid = *cit; @@ -155,18 +154,30 @@ public: (*it)->set_removable(false); ++it; for(; it != ite; ++it){ - Vertex_handle vh = *it; - ++degrees[vh]; - if((boost::next(it) != ite) && (boost::prior(it)== boost::next(it))){ - (*it)->set_removable(false); + if(boost::next(it) != ite){ + Vertex_handle vp = *boost::prior(it), vn = *boost::next(it); + if(vp == vn){ + (*it)->set_removable(false); + } } } it = boost::prior(it); (*it)->set_removable(false); } + std::unordered_map degrees; + for (Constrained_edges_iterator it = pct.constrained_edges_begin(); it != pct.constrained_edges_end(); ++it) { + Edge e = *it; + Face_handle fh = e.first; + int ei = e.second; + Vertex_handle vh = fh->vertex(pct.cw(ei)); + ++degrees[vh]; + vh = fh->vertex(pct.ccw(ei)); + ++degrees[vh]; + } + for(Finite_vertices_iterator it = pct.finite_vertices_begin(); it != pct.finite_vertices_end(); ++it){ - if( it->is_removable() && (degrees[it] > 1) ){ + if( it->is_removable() && (degrees[it] != 2) ){ it->set_removable(false); } } From 98b6cf398d3a1cdc7d8f8457688cbd45162f6c19 Mon Sep 17 00:00:00 2001 From: Andreas Fabri Date: Wed, 12 Feb 2025 17:48:55 +0000 Subject: [PATCH 4/5] change iterator category to bidirectional --- STL_Extension/include/CGAL/Skiplist.h | 4 ++-- .../internal/Polyline_constraint_hierarchy_2.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/STL_Extension/include/CGAL/Skiplist.h b/STL_Extension/include/CGAL/Skiplist.h index 3be7908c90c..e7d7b60b422 100644 --- a/STL_Extension/include/CGAL/Skiplist.h +++ b/STL_Extension/include/CGAL/Skiplist.h @@ -75,7 +75,7 @@ public: all_iterator , typename all_list::iterator , T - > + , std::bidirectional_iterator_tag> { public: all_iterator() {} @@ -91,7 +91,7 @@ public: skip_iterator , typename skip_list::iterator , T - > + , std::bidirectional_iterator_tag> { public: skip_iterator() {} diff --git a/Triangulation_2/include/CGAL/Triangulation_2/internal/Polyline_constraint_hierarchy_2.h b/Triangulation_2/include/CGAL/Triangulation_2/internal/Polyline_constraint_hierarchy_2.h index 8154e26251c..0cba5ca0213 100644 --- a/Triangulation_2/include/CGAL/Triangulation_2/internal/Polyline_constraint_hierarchy_2.h +++ b/Triangulation_2/include/CGAL/Triangulation_2/internal/Polyline_constraint_hierarchy_2.h @@ -85,7 +85,7 @@ public: Vertex_it , typename Vertex_list::skip_iterator , Vertex_handle - , boost::use_default + , std::bidirectional_iterator_tag , Vertex_handle> { public: From c8e935708091b6daf84f4dabd9e2bf81baf374bf Mon Sep 17 00:00:00 2001 From: Andreas Fabri Date: Wed, 12 Feb 2025 17:50:35 +0000 Subject: [PATCH 5/5] Use std::prev() and std::next() instead of their boost equivalents --- .../include/CGAL/Polyline_simplification_2/simplify.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Polyline_simplification_2/include/CGAL/Polyline_simplification_2/simplify.h b/Polyline_simplification_2/include/CGAL/Polyline_simplification_2/simplify.h index 76f7468d469..3fd443d4e2d 100644 --- a/Polyline_simplification_2/include/CGAL/Polyline_simplification_2/simplify.h +++ b/Polyline_simplification_2/include/CGAL/Polyline_simplification_2/simplify.h @@ -154,8 +154,8 @@ public: (*it)->set_removable(false); ++it; for(; it != ite; ++it){ - if(boost::next(it) != ite){ - Vertex_handle vp = *boost::prior(it), vn = *boost::next(it); + if(std::next(it) != ite){ + Vertex_handle vp = *std::prev(it), vn = *std::next(it); if(vp == vn){ (*it)->set_removable(false); }