From c945d27dc2c772f599549e9b90107d17c6b5b484 Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Thu, 12 Jul 2018 16:42:21 +0200 Subject: [PATCH 1/5] Fix Convex_hull_3 so it becomes minimal. --- .../Convex_hull_3/CGAL/Convex_hull_traits_3.h | 5 ++ .../Concepts/ConvexHullTraits_3.h | 7 ++ .../include/CGAL/Convex_hull_traits_3.h | 32 ++++++++ Convex_hull_3/include/CGAL/convex_hull_3.h | 80 ++++++++++++++++--- .../convex_hull_traits_3_fp_bug.cpp | 18 ++++- 5 files changed, 129 insertions(+), 13 deletions(-) diff --git a/Convex_hull_3/doc/Convex_hull_3/CGAL/Convex_hull_traits_3.h b/Convex_hull_3/doc/Convex_hull_3/CGAL/Convex_hull_traits_3.h index 8ba87fd6ed1..c03fa746511 100644 --- a/Convex_hull_3/doc/Convex_hull_3/CGAL/Convex_hull_traits_3.h +++ b/Convex_hull_3/doc/Convex_hull_3/CGAL/Convex_hull_traits_3.h @@ -98,6 +98,11 @@ unspecified_type Has_on_positive_side_3; /*! +*/ +unspecified_type Has_on_negative_side_3; + +/*! + */ unspecified_type Less_signed_distance_to_plane_3; diff --git a/Convex_hull_3/doc/Convex_hull_3/Concepts/ConvexHullTraits_3.h b/Convex_hull_3/doc/Convex_hull_3/Concepts/ConvexHullTraits_3.h index 9b857d2165c..e4d4af444c1 100644 --- a/Convex_hull_3/doc/Convex_hull_3/Concepts/ConvexHullTraits_3.h +++ b/Convex_hull_3/doc/Convex_hull_3/Concepts/ConvexHullTraits_3.h @@ -84,6 +84,13 @@ Predicate object type that provides `q` is on the positive side of the halfspace `h`. */ typedef unspecified_type Has_on_positive_side_3; + + /*! + Predicate object type that provides + `bool operator()(Plane_3 h, Point_3 q)`, which determines if the point + `q` is on the negative side of the halfspace `h`. + */ + typedef unspecified_type Has_on_negative_side_3; /*! Predicate object type that provides diff --git a/Convex_hull_3/include/CGAL/Convex_hull_traits_3.h b/Convex_hull_3/include/CGAL/Convex_hull_traits_3.h index caa065b2b58..1df55294e3f 100644 --- a/Convex_hull_3/include/CGAL/Convex_hull_traits_3.h +++ b/Convex_hull_3/include/CGAL/Convex_hull_traits_3.h @@ -95,6 +95,23 @@ public: typedef bool result_type; }; + +template +class Point_triple_has_on_negative_side_3 { + +public: + typedef typename K::Point_3 Point_3; + typedef Point_triple Plane_3; + bool + operator()( const Plane_3& pl, const Point_3& p) const + { + typename K::Orientation_3 o; + return ( o(pl.p(), pl.q(), pl.r(), p) == CGAL::NEGATIVE); + } + + typedef bool result_type; +}; + template class Point_triple_construct_orthogonal_vector_3 { @@ -167,6 +184,8 @@ struct GT3_for_CH3 { template struct Convex_hull_traits_base_3 { typedef Point_triple_has_on_positive_side_3 Has_on_positive_side_3; + typedef Point_triple_has_on_negative_side_3 Has_on_negative_side_3; + typedef Point_triple_less_signed_distance_to_plane_3 Less_signed_distance_to_plane_3; @@ -180,6 +199,13 @@ struct Convex_hull_traits_base_3{ Point_triple_converter, Point_triple_converter > Has_on_positive_side_3; + + typedef Filtered_predicate< + Point_triple_has_on_negative_side_3< typename R_::Exact_kernel_rt >, + Point_triple_has_on_negative_side_3< typename R_::Approximate_kernel >, + Point_triple_converter, + Point_triple_converter + > Has_on_negative_side_3; typedef Filtered_predicate< Point_triple_less_signed_distance_to_plane_3< typename R_::Exact_kernel_rt >, @@ -230,6 +256,8 @@ class Convex_hull_traits_3 : typedef typename Convex_hull_traits_base_3 ::Has_on_positive_side_3 Has_on_positive_side_3; + typedef typename Convex_hull_traits_base_3 + ::Has_on_negative_side_3 Has_on_negative_side_3; typedef typename Convex_hull_traits_base_3 ::Less_signed_distance_to_plane_3 Less_signed_distance_to_plane_3; @@ -289,6 +317,10 @@ class Convex_hull_traits_3 : Has_on_positive_side_3 has_on_positive_side_3_object() const { return Has_on_positive_side_3(); } + + Has_on_negative_side_3 + has_on_negative_side_3_object() const + { return Has_on_negative_side_3(); } Oriented_side_3 oriented_side_3_object() const diff --git a/Convex_hull_3/include/CGAL/convex_hull_3.h b/Convex_hull_3/include/CGAL/convex_hull_3.h index 732893b58bb..e73e6b10286 100644 --- a/Convex_hull_3/include/CGAL/convex_hull_3.h +++ b/Convex_hull_3/include/CGAL/convex_hull_3.h @@ -358,8 +358,8 @@ find_visible_set(TDS_2& tds, typedef typename Traits::Plane_3 Plane_3; typedef typename TDS_2::Face_handle Face_handle; typedef typename TDS_2::Vertex_handle Vertex_handle; - typename Traits::Has_on_positive_side_3 has_on_positive_side = - traits.has_on_positive_side_3_object(); + typename Traits::Has_on_negative_side_3 has_on_negative_side = + traits.has_on_negative_side_3_object(); std::vector vertices; vertices.reserve(10); @@ -387,7 +387,7 @@ find_visible_set(TDS_2& tds, f->info() = VISITED; Plane_3 plane(f->vertex(0)->point(),f->vertex(1)->point(),f->vertex(2)->point()); int ind = f->index(*vis_it); - if ( has_on_positive_side(plane, point) ){ // is visible + if ( !has_on_negative_side(plane, point) ){ // is visible visible.push_back(f); Vertex_handle vh = f->vertex(ind); if(vh->info() == 0){ vertices.push_back(vh); vh->info() = VISITED;} @@ -717,6 +717,56 @@ ch_quickhull_polyhedron_3(std::list& points, } +template +void init_iterators(P3_iterator& minx, + P3_iterator& maxx, + P3_iterator& miny, + const P3_iterator& start, + const Point_3_list& points) +{ + P3_iterator it = start; + for(; it != points.end(); ++it){ + if(it->x() < minx->x()) minx = it; + else if(it->x() == minx->x()) + { + if(it->y() < minx->y()) minx = it; + else if(it->y() == minx->y()) + { + if(it->z() < minx->z()) minx = it; + } + } + } + it = start; + for(; it != points.end(); ++it){ + if(it == minx ) + continue; + if(it->x() > maxx->x()) maxx = it; + else if(it->x() == maxx->x()) + { + if(it->y() > maxx->y()) maxx = it; + else if(it->y() == maxx->y()) + { + if(it->z() > maxx->z()) maxx = it; + } + } + } + it = start; + for(; it != points.end(); ++it){ + if(it == minx || it == maxx) + continue; + if(it->y() < miny->y()) miny = it; + else if(it->y() == miny->y()) + { + if(it->x() > miny->x()) miny = it; + else if(it->x() == miny->x()) + { + if(it->z() < miny->z()) miny = it; + } + } + } +} + } } //namespace internal::Convex_hull_3 @@ -810,13 +860,7 @@ convex_hull_3(InputIterator first, InputIterator beyond, Polyhedron P; P3_iterator minx, maxx, miny, it; - minx = maxx = miny = it = points.begin(); - ++it; - for(; it != points.end(); ++it){ - if(it->x() < minx->x()) minx = it; - if(it->x() > maxx->x()) maxx = it; - if(it->y() < miny->y()) miny = it; - } + internal::Convex_hull_3::init_iterators(minx, maxx, miny, it, points); if(! collinear(*minx, *maxx, *miny) ){ internal::Convex_hull_3::ch_quickhull_polyhedron_3(points, minx, maxx, miny, P, traits); } else { @@ -893,8 +937,20 @@ void convex_hull_3(InputIterator first, InputIterator beyond, clear(polyhedron); // result will be a polyhedron - internal::Convex_hull_3::ch_quickhull_polyhedron_3(points, point1_it, point2_it, point3_it, - polyhedron, traits); + P3_iterator minx, maxx, miny, it; + minx = maxx = miny = it = points.begin(); + ++it; + + + //take extreme points to begin with. + internal::Convex_hull_::init_iterators(minx, maxx, miny, it, points); + if(! collinear(*minx, *maxx, *miny) ){ + internal::Convex_hull_3::ch_quickhull_polyhedron_3(points, minx, maxx, miny, + polyhedron, traits); + } else {//to do : this case leads to bad init a risk of non minimal convex hull + internal::Convex_hull_3::ch_quickhull_polyhedron_3(points, point1_it, point2_it, point3_it, + polyhedron, traits); + } } diff --git a/Convex_hull_3/test/Convex_hull_3/convex_hull_traits_3_fp_bug.cpp b/Convex_hull_3/test/Convex_hull_3/convex_hull_traits_3_fp_bug.cpp index 7eb9d41e1bd..3ca7be941c2 100644 --- a/Convex_hull_3/test/Convex_hull_3/convex_hull_traits_3_fp_bug.cpp +++ b/Convex_hull_3/test/Convex_hull_3/convex_hull_traits_3_fp_bug.cpp @@ -1,10 +1,14 @@ #include +#include #include #include +#include +#include #include #include typedef CGAL::Epick K; +typedef CGAL::Epeck EK; int main() { @@ -18,8 +22,20 @@ int main() CGAL::Polyhedron_3 r; CGAL::convex_hull_3(pointset.begin(), pointset.end(), r); - assert(r.size_of_vertices()==82); + + CGAL::Polyhedron_3 s; + CGAL::copy_face_graph(r,s); + assert(CGAL::is_strongly_convex_3(s)); + + CGAL::Cartesian_converter to_EK; + CGAL::Side_of_triangle_mesh, EK> sotm(s); + + + BOOST_FOREACH(K::Point_3 p, pointset) + { + assert(sotm(to_EK(p)) != CGAL::ON_UNBOUNDED_SIDE); + } return 0; } From d3a8a07df3bf5ea8e099552227d2b636fa4c4e40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Fri, 13 Jul 2018 16:47:11 +0200 Subject: [PATCH 2/5] do not use negative side change plane's orientation instead --- .../Convex_hull_3/CGAL/Convex_hull_traits_3.h | 5 --- .../Concepts/ConvexHullTraits_3.h | 7 ---- .../include/CGAL/Convex_hull_traits_3.h | 32 ------------------- Convex_hull_3/include/CGAL/convex_hull_3.h | 8 ++--- 4 files changed, 4 insertions(+), 48 deletions(-) diff --git a/Convex_hull_3/doc/Convex_hull_3/CGAL/Convex_hull_traits_3.h b/Convex_hull_3/doc/Convex_hull_3/CGAL/Convex_hull_traits_3.h index c03fa746511..8ba87fd6ed1 100644 --- a/Convex_hull_3/doc/Convex_hull_3/CGAL/Convex_hull_traits_3.h +++ b/Convex_hull_3/doc/Convex_hull_3/CGAL/Convex_hull_traits_3.h @@ -98,11 +98,6 @@ unspecified_type Has_on_positive_side_3; /*! -*/ -unspecified_type Has_on_negative_side_3; - -/*! - */ unspecified_type Less_signed_distance_to_plane_3; diff --git a/Convex_hull_3/doc/Convex_hull_3/Concepts/ConvexHullTraits_3.h b/Convex_hull_3/doc/Convex_hull_3/Concepts/ConvexHullTraits_3.h index e4d4af444c1..9b857d2165c 100644 --- a/Convex_hull_3/doc/Convex_hull_3/Concepts/ConvexHullTraits_3.h +++ b/Convex_hull_3/doc/Convex_hull_3/Concepts/ConvexHullTraits_3.h @@ -84,13 +84,6 @@ Predicate object type that provides `q` is on the positive side of the halfspace `h`. */ typedef unspecified_type Has_on_positive_side_3; - - /*! - Predicate object type that provides - `bool operator()(Plane_3 h, Point_3 q)`, which determines if the point - `q` is on the negative side of the halfspace `h`. - */ - typedef unspecified_type Has_on_negative_side_3; /*! Predicate object type that provides diff --git a/Convex_hull_3/include/CGAL/Convex_hull_traits_3.h b/Convex_hull_3/include/CGAL/Convex_hull_traits_3.h index 1df55294e3f..caa065b2b58 100644 --- a/Convex_hull_3/include/CGAL/Convex_hull_traits_3.h +++ b/Convex_hull_3/include/CGAL/Convex_hull_traits_3.h @@ -95,23 +95,6 @@ public: typedef bool result_type; }; - -template -class Point_triple_has_on_negative_side_3 { - -public: - typedef typename K::Point_3 Point_3; - typedef Point_triple Plane_3; - bool - operator()( const Plane_3& pl, const Point_3& p) const - { - typename K::Orientation_3 o; - return ( o(pl.p(), pl.q(), pl.r(), p) == CGAL::NEGATIVE); - } - - typedef bool result_type; -}; - template class Point_triple_construct_orthogonal_vector_3 { @@ -184,8 +167,6 @@ struct GT3_for_CH3 { template struct Convex_hull_traits_base_3 { typedef Point_triple_has_on_positive_side_3 Has_on_positive_side_3; - typedef Point_triple_has_on_negative_side_3 Has_on_negative_side_3; - typedef Point_triple_less_signed_distance_to_plane_3 Less_signed_distance_to_plane_3; @@ -199,13 +180,6 @@ struct Convex_hull_traits_base_3{ Point_triple_converter, Point_triple_converter > Has_on_positive_side_3; - - typedef Filtered_predicate< - Point_triple_has_on_negative_side_3< typename R_::Exact_kernel_rt >, - Point_triple_has_on_negative_side_3< typename R_::Approximate_kernel >, - Point_triple_converter, - Point_triple_converter - > Has_on_negative_side_3; typedef Filtered_predicate< Point_triple_less_signed_distance_to_plane_3< typename R_::Exact_kernel_rt >, @@ -256,8 +230,6 @@ class Convex_hull_traits_3 : typedef typename Convex_hull_traits_base_3 ::Has_on_positive_side_3 Has_on_positive_side_3; - typedef typename Convex_hull_traits_base_3 - ::Has_on_negative_side_3 Has_on_negative_side_3; typedef typename Convex_hull_traits_base_3 ::Less_signed_distance_to_plane_3 Less_signed_distance_to_plane_3; @@ -317,10 +289,6 @@ class Convex_hull_traits_3 : Has_on_positive_side_3 has_on_positive_side_3_object() const { return Has_on_positive_side_3(); } - - Has_on_negative_side_3 - has_on_negative_side_3_object() const - { return Has_on_negative_side_3(); } Oriented_side_3 oriented_side_3_object() const diff --git a/Convex_hull_3/include/CGAL/convex_hull_3.h b/Convex_hull_3/include/CGAL/convex_hull_3.h index e73e6b10286..431d8df227b 100644 --- a/Convex_hull_3/include/CGAL/convex_hull_3.h +++ b/Convex_hull_3/include/CGAL/convex_hull_3.h @@ -358,8 +358,8 @@ find_visible_set(TDS_2& tds, typedef typename Traits::Plane_3 Plane_3; typedef typename TDS_2::Face_handle Face_handle; typedef typename TDS_2::Vertex_handle Vertex_handle; - typename Traits::Has_on_negative_side_3 has_on_negative_side = - traits.has_on_negative_side_3_object(); + typename Traits::Has_on_positive_side_3 has_on_positive_side = + traits.has_on_positive_side_3_object(); std::vector vertices; vertices.reserve(10); @@ -385,9 +385,9 @@ find_visible_set(TDS_2& tds, // if haven't already seen this facet if (f->info() == 0) { f->info() = VISITED; - Plane_3 plane(f->vertex(0)->point(),f->vertex(1)->point(),f->vertex(2)->point()); + Plane_3 plane(f->vertex(0)->point(),f->vertex(2)->point(),f->vertex(1)->point()); int ind = f->index(*vis_it); - if ( !has_on_negative_side(plane, point) ){ // is visible + if ( !has_on_positive_side(plane, point) ){ // is visible visible.push_back(f); Vertex_handle vh = f->vertex(ind); if(vh->info() == 0){ vertices.push_back(vh); vh->info() = VISITED;} From 0037edc52978abba1e13dd3f237443606d28025c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Fri, 13 Jul 2018 16:49:40 +0200 Subject: [PATCH 3/5] use statically filtered predicate --- Convex_hull_3/include/CGAL/convex_hull_3.h | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/Convex_hull_3/include/CGAL/convex_hull_3.h b/Convex_hull_3/include/CGAL/convex_hull_3.h index 431d8df227b..d3992ee3ecf 100644 --- a/Convex_hull_3/include/CGAL/convex_hull_3.h +++ b/Convex_hull_3/include/CGAL/convex_hull_3.h @@ -355,11 +355,8 @@ find_visible_set(TDS_2& tds, std::map& outside, const Traits& traits) { - typedef typename Traits::Plane_3 Plane_3; typedef typename TDS_2::Face_handle Face_handle; typedef typename TDS_2::Vertex_handle Vertex_handle; - typename Traits::Has_on_positive_side_3 has_on_positive_side = - traits.has_on_positive_side_3_object(); std::vector vertices; vertices.reserve(10); @@ -385,9 +382,10 @@ find_visible_set(TDS_2& tds, // if haven't already seen this facet if (f->info() == 0) { f->info() = VISITED; - Plane_3 plane(f->vertex(0)->point(),f->vertex(2)->point(),f->vertex(1)->point()); + Is_on_positive_side_of_plane_3 is_on_positive_side( + traits,f->vertex(0)->point(),f->vertex(2)->point(),f->vertex(1)->point()); int ind = f->index(*vis_it); - if ( !has_on_positive_side(plane, point) ){ // is visible + if ( !is_on_positive_side(point) ){ // is visible visible.push_back(f); Vertex_handle vh = f->vertex(ind); if(vh->info() == 0){ vertices.push_back(vh); vh->info() = VISITED;} @@ -943,7 +941,7 @@ void convex_hull_3(InputIterator first, InputIterator beyond, //take extreme points to begin with. - internal::Convex_hull_::init_iterators(minx, maxx, miny, it, points); + internal::Convex_hull_3::init_iterators(minx, maxx, miny, it, points); if(! collinear(*minx, *maxx, *miny) ){ internal::Convex_hull_3::ch_quickhull_polyhedron_3(points, minx, maxx, miny, polyhedron, traits); From 2f319d332e3da25c21675fc83bfc2c9ba0472ed5 Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Mon, 16 Jul 2018 09:58:15 +0200 Subject: [PATCH 4/5] Remove init_iterators. It works without and like this there is no need to change the traits --- Convex_hull_3/include/CGAL/convex_hull_3.h | 74 +++------------------- 1 file changed, 9 insertions(+), 65 deletions(-) diff --git a/Convex_hull_3/include/CGAL/convex_hull_3.h b/Convex_hull_3/include/CGAL/convex_hull_3.h index d3992ee3ecf..2352d474447 100644 --- a/Convex_hull_3/include/CGAL/convex_hull_3.h +++ b/Convex_hull_3/include/CGAL/convex_hull_3.h @@ -715,56 +715,6 @@ ch_quickhull_polyhedron_3(std::list& points, } -template -void init_iterators(P3_iterator& minx, - P3_iterator& maxx, - P3_iterator& miny, - const P3_iterator& start, - const Point_3_list& points) -{ - P3_iterator it = start; - for(; it != points.end(); ++it){ - if(it->x() < minx->x()) minx = it; - else if(it->x() == minx->x()) - { - if(it->y() < minx->y()) minx = it; - else if(it->y() == minx->y()) - { - if(it->z() < minx->z()) minx = it; - } - } - } - it = start; - for(; it != points.end(); ++it){ - if(it == minx ) - continue; - if(it->x() > maxx->x()) maxx = it; - else if(it->x() == maxx->x()) - { - if(it->y() > maxx->y()) maxx = it; - else if(it->y() == maxx->y()) - { - if(it->z() > maxx->z()) maxx = it; - } - } - } - it = start; - for(; it != points.end(); ++it){ - if(it == minx || it == maxx) - continue; - if(it->y() < miny->y()) miny = it; - else if(it->y() == miny->y()) - { - if(it->x() > miny->x()) miny = it; - else if(it->x() == miny->x()) - { - if(it->z() < miny->z()) miny = it; - } - } - } -} - } } //namespace internal::Convex_hull_3 @@ -858,7 +808,13 @@ convex_hull_3(InputIterator first, InputIterator beyond, Polyhedron P; P3_iterator minx, maxx, miny, it; - internal::Convex_hull_3::init_iterators(minx, maxx, miny, it, points); + minx = maxx = miny = it = points.begin(); + ++it; + for(; it != points.end(); ++it){ + if(it->x() < minx->x()) minx = it; + if(it->x() > maxx->x()) maxx = it; + if(it->y() < miny->y()) miny = it; + } if(! collinear(*minx, *maxx, *miny) ){ internal::Convex_hull_3::ch_quickhull_polyhedron_3(points, minx, maxx, miny, P, traits); } else { @@ -935,20 +891,8 @@ void convex_hull_3(InputIterator first, InputIterator beyond, clear(polyhedron); // result will be a polyhedron - P3_iterator minx, maxx, miny, it; - minx = maxx = miny = it = points.begin(); - ++it; - - - //take extreme points to begin with. - internal::Convex_hull_3::init_iterators(minx, maxx, miny, it, points); - if(! collinear(*minx, *maxx, *miny) ){ - internal::Convex_hull_3::ch_quickhull_polyhedron_3(points, minx, maxx, miny, - polyhedron, traits); - } else {//to do : this case leads to bad init a risk of non minimal convex hull - internal::Convex_hull_3::ch_quickhull_polyhedron_3(points, point1_it, point2_it, point3_it, - polyhedron, traits); - } + internal::Convex_hull_3::ch_quickhull_polyhedron_3(points, point1_it, point2_it, point3_it, + polyhedron, traits); } From d1ac382a87f8742e6b122e4810e6b5944ad7b582 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Mon, 16 Jul 2018 13:59:51 +0200 Subject: [PATCH 5/5] update changes --- Installation/CHANGES.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Installation/CHANGES.md b/Installation/CHANGES.md index e2683e3b1fd..884a368b13b 100644 --- a/Installation/CHANGES.md +++ b/Installation/CHANGES.md @@ -49,6 +49,10 @@ Release date: September 2018 to reflect the real needs of the code (some types and operators were used in the code but did not appear in the concepts). +### Convex hull 3 + +- Fix a bug in the computation of the 3D convex hull that was leaving extra points + within subset of coplanar points that do not belong to the minimal convex hull. ### Point Set Processing