From 4d67ff3d8b04a9defde1e16a508c6f4cf8b9aa0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mael=20Rouxel-Labb=C3=A9?= Date: Mon, 12 Mar 2018 11:13:35 +0100 Subject: [PATCH 01/13] Avoid a segfault when is_simple() is called with an empty polygon/range --- Polygon/include/CGAL/Polygon_2/Polygon_2_algorithms_impl.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Polygon/include/CGAL/Polygon_2/Polygon_2_algorithms_impl.h b/Polygon/include/CGAL/Polygon_2/Polygon_2_algorithms_impl.h index 03d79f002b7..f7248f89dff 100644 --- a/Polygon/include/CGAL/Polygon_2/Polygon_2_algorithms_impl.h +++ b/Polygon/include/CGAL/Polygon_2/Polygon_2_algorithms_impl.h @@ -49,6 +49,8 @@ bool is_simple_2(ForwardIterator first, ForwardIterator last, const PolygonTraits& traits) { + if (first == last) return true; + return is_simple_polygon(first, last, traits); } From efb6794a6c8f29c9b92b850eef9065d9bae47f8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mael=20Rouxel-Labb=C3=A9?= Date: Mon, 12 Mar 2018 11:14:23 +0100 Subject: [PATCH 02/13] Add a test --- Polygon/test/Polygon/AlgorithmTest.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Polygon/test/Polygon/AlgorithmTest.cpp b/Polygon/test/Polygon/AlgorithmTest.cpp index 15aaa44f3ab..cc5fac6a76e 100644 --- a/Polygon/test/Polygon/AlgorithmTest.cpp +++ b/Polygon/test/Polygon/AlgorithmTest.cpp @@ -66,6 +66,10 @@ void test_polygon(const R&, const Point&, const char* FileName) CGAL::Orientation orientation = CGAL::orientation_2(polygon.begin(), polygon.end()); + polygon.clear(); + assert(CGAL::is_simple_2(polygon.begin(), polygon.end())); + assert(CGAL::is_convex_2(polygon.begin(), polygon.end())); + cout << "left = " << *left << endl; cout << "right = " << *right << endl; cout << "top = " << *top << endl; From b080804f09d78cc462e1e0a611a6a3d5ae20e2d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mael=20Rouxel-Labb=C3=A9?= Date: Tue, 13 Mar 2018 11:14:06 +0100 Subject: [PATCH 03/13] Fixed missing bound check in the sequential insertion phase If the input point set is degenerate, dimension() < 3 is always 'true' and we will eventually try to read beyond the end of the points vector. See issue https://github.com/CGAL/cgal/issues/2922 --- Triangulation_3/include/CGAL/Delaunay_triangulation_3.h | 4 ++-- Triangulation_3/include/CGAL/Regular_triangulation_3.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Triangulation_3/include/CGAL/Delaunay_triangulation_3.h b/Triangulation_3/include/CGAL/Delaunay_triangulation_3.h index 3f18c2f5e57..01d8930fa03 100644 --- a/Triangulation_3/include/CGAL/Delaunay_triangulation_3.h +++ b/Triangulation_3/include/CGAL/Delaunay_triangulation_3.h @@ -381,7 +381,7 @@ public: // Insert "num_points_seq" points sequentially // (or more if dim < 3 after that) size_t num_points_seq = (std::min)(num_points, (size_t)100); - while (dimension() < 3 || i < num_points_seq) + while (i < num_points_seq || (dimension() < 3 && i < num_points)) { hint = insert(points[i], hint); ++i; @@ -464,7 +464,7 @@ private: // Insert "num_points_seq" points sequentially // (or more if dim < 3 after that) size_t num_points_seq = (std::min)(num_points, (size_t)100); - while (dimension() < 3 || i < num_points_seq) + while (i < num_points_seq || (dimension() < 3 && i < num_points)) { hint = insert(points[indices[i]], hint); if (hint != Vertex_handle()) hint->info() = infos[indices[i]]; diff --git a/Triangulation_3/include/CGAL/Regular_triangulation_3.h b/Triangulation_3/include/CGAL/Regular_triangulation_3.h index 7287944d586..bdb6d561bba 100644 --- a/Triangulation_3/include/CGAL/Regular_triangulation_3.h +++ b/Triangulation_3/include/CGAL/Regular_triangulation_3.h @@ -360,7 +360,7 @@ namespace CGAL { // Insert "num_points_seq" points sequentially // (or more if dim < 3 after that) size_t num_points_seq = (std::min)(num_points, (size_t)100); - while (dimension() < 3 || i < num_points_seq) + while (i < num_points_seq || (dimension() < 3 && i < num_points)) { Locate_type lt; Cell_handle c; @@ -483,7 +483,7 @@ namespace CGAL { // Insert "num_points_seq" points sequentially // (or more if dim < 3 after that) size_t num_points_seq = (std::min)(num_points, (size_t)100); - while (dimension() < 3 || i < num_points_seq) + while (i < num_points_seq || (dimension() < 3 && i < num_points)) { Locate_type lt; Cell_handle c; From 991987926c1627dedb21cf255d1f7bf7f7b8c9c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mael=20Rouxel-Labb=C3=A9?= Date: Thu, 15 Mar 2018 12:23:33 +0100 Subject: [PATCH 04/13] Fixed cutoff parameter not being properly propagated and simplified the default initilization functions --- .../include/CGAL/box_intersection_d.h | 32 +++++++------------ 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/Box_intersection_d/include/CGAL/box_intersection_d.h b/Box_intersection_d/include/CGAL/box_intersection_d.h index 80ae97ca9d0..eb42e6741f6 100644 --- a/Box_intersection_d/include/CGAL/box_intersection_d.h +++ b/Box_intersection_d/include/CGAL/box_intersection_d.h @@ -179,13 +179,14 @@ template< class RandomAccessIter, class Callback, class BoxTraits > void box_self_intersection_d( RandomAccessIter begin, RandomAccessIter end, Callback callback, - BoxTraits box_traits) + BoxTraits box_traits, + std::ptrdiff_t cutoff, + Box_intersection_d::Topology topology) { typedef typename std::iterator_traits::value_type val_t; std::vector< val_t> i( begin, end); box_intersection_d( begin, end, i.begin(), i.end(), - callback, box_traits, 10, - Box_intersection_d::CLOSED, + callback, box_traits, cutoff, topology, Box_intersection_d::COMPLETE); } @@ -196,26 +197,17 @@ void box_self_intersection_d( BoxTraits box_traits, std::ptrdiff_t cutoff) { - typedef typename std::iterator_traits::value_type val_t; - std::vector< val_t> i( begin, end); - box_intersection_d( begin, end, i.begin(), i.end(), - callback, box_traits, cutoff, - Box_intersection_d::CLOSED, - Box_intersection_d::COMPLETE); + return box_self_intersection_d(begin, end, callback, box_traits, cutoff, + Box_intersection_d::CLOSED); } template< class RandomAccessIter, class Callback, class BoxTraits > void box_self_intersection_d( RandomAccessIter begin, RandomAccessIter end, Callback callback, - BoxTraits box_traits, - std::ptrdiff_t cutoff, - Box_intersection_d::Topology topology) + BoxTraits box_traits) { - typedef typename std::iterator_traits::value_type val_t; - std::vector< val_t> i( begin, end); - box_intersection_d( begin, end, i.begin(), i.end(), - callback, box_traits, cutoff, topology, Box_intersection_d::COMPLETE); + return box_self_intersection_d(begin, end, callback, box_traits, 10); } // Specialized call with default box traits, specialized for self-intersection. @@ -227,20 +219,18 @@ void box_self_intersection_d( { typedef typename std::iterator_traits::value_type val_t; typedef Box_intersection_d::Box_traits_d< val_t> Box_traits; - box_self_intersection_d(begin, end, callback, - Box_traits(), 10, Box_intersection_d::CLOSED); + box_self_intersection_d(begin, end, callback, Box_traits()); } template< class RandomAccessIter, class Callback > void box_self_intersection_d( RandomAccessIter begin, RandomAccessIter end, Callback callback, - std::ptrdiff_t) + std::ptrdiff_t cutoff) { typedef typename std::iterator_traits::value_type val_t; typedef Box_intersection_d::Box_traits_d< val_t> Box_traits; - box_self_intersection_d(begin, end, callback, - Box_traits(), 10, Box_intersection_d::CLOSED); + box_self_intersection_d(begin, end, callback, Box_traits(), cutoff); } template< class RandomAccessIter, class Callback > From 3ae153eeb74761f7a5abba2f4417274aa5c05aea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mael=20Rouxel-Labb=C3=A9?= Date: Thu, 15 Mar 2018 12:25:14 +0100 Subject: [PATCH 05/13] Added a comment to explain why there is duplication --- Box_intersection_d/include/CGAL/box_intersection_d.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Box_intersection_d/include/CGAL/box_intersection_d.h b/Box_intersection_d/include/CGAL/box_intersection_d.h index eb42e6741f6..ed44ca03908 100644 --- a/Box_intersection_d/include/CGAL/box_intersection_d.h +++ b/Box_intersection_d/include/CGAL/box_intersection_d.h @@ -183,6 +183,9 @@ void box_self_intersection_d( std::ptrdiff_t cutoff, Box_intersection_d::Topology topology) { + // Copying rather than calling 'box_intersection_d(begin, end, begin, end, ...' + // is necessary because the 'std::partition' and range splits on the first range + // would be messed up by sorts on the second range otherwise. typedef typename std::iterator_traits::value_type val_t; std::vector< val_t> i( begin, end); box_intersection_d( begin, end, i.begin(), i.end(), From 1dfbec363a1a65e091de2a49bb0d63a4a3c7fea1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mael=20Rouxel-Labb=C3=A9?= Date: Thu, 15 Mar 2018 12:25:41 +0100 Subject: [PATCH 06/13] Minor output fix --- Box_intersection_d/test/Box_intersection_d/test_box_grid.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Box_intersection_d/test/Box_intersection_d/test_box_grid.cpp b/Box_intersection_d/test/Box_intersection_d/test_box_grid.cpp index 1c87cf90cb0..4d51695b9bd 100644 --- a/Box_intersection_d/test/Box_intersection_d/test_box_grid.cpp +++ b/Box_intersection_d/test/Box_intersection_d/test_box_grid.cpp @@ -116,7 +116,7 @@ void test_box_intersection() { std::ptrdiff_t(1), CGAL::Box_intersection_d::HALF_OPEN, CGAL::Box_intersection_d::COMPLETE); - check_result( "Box self inters. 3x3+2, half-open", result, check5, 2); + check_result( "Box inters. 3x3+2, half-open", result, check5, 2); // compare this with the bipartite case // self intersect 3x3+2 query boxes, half-open boxes From d87295515f5b4c7321f015b99258286695a6fa23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mael=20Rouxel-Labb=C3=A9?= Date: Thu, 15 Mar 2018 13:49:00 +0100 Subject: [PATCH 07/13] Various doc improvements --- .../CGAL/Box_intersection_d/Box_d.h | 18 +-- .../CGAL/Box_intersection_d/Box_traits_d.h | 8 +- .../Box_intersection_d/Box_with_handle_d.h | 6 +- .../CGAL/box_intersection_d.h | 143 ++++++++++-------- .../Concepts/BoxIntersectionTraits_d.h | 1 + 5 files changed, 95 insertions(+), 81 deletions(-) diff --git a/Box_intersection_d/doc/Box_intersection_d/CGAL/Box_intersection_d/Box_d.h b/Box_intersection_d/doc/Box_intersection_d/CGAL/Box_intersection_d/Box_d.h index aecf017cdbc..20f27bb125f 100644 --- a/Box_intersection_d/doc/Box_intersection_d/CGAL/Box_intersection_d/Box_d.h +++ b/Box_intersection_d/doc/Box_intersection_d/CGAL/Box_intersection_d/Box_d.h @@ -12,15 +12,14 @@ need to provide a unique `id`-number. The policy parameter `IdPolicy` offers several choices. The template parameters have to comply with the following requirements: -
    -
  • `NT`: number type for the box boundaries, needs to be a model -of the `Assignable` and the `LessThanComparable` concept. -
  • `int D`: the dimension of the box. -
  • `IdPolicy`: specifies how the `id`-number will be -provided. Can be one of the following types, where +\tparam NT is the number type for the box boundaries. It must meet the requierements +of the concepts `Assignable` and `LessThanComparable`. +\tparam D is an integer and the dimension of the box. +\tparam IdPolicy specifies how the `id`-number will be +provided and can be one of the following types, where `ID_EXPLICIT` is the default for this parameter:
      -
    • `ID_NONE`: no `id`-number is provided. Can be useful +
    • `ID_NONE`: no `id`-number is provided. This can be useful if `Box_d` is used as a base class for a different implementation of `id`-numbers than the ones provided here. @@ -32,15 +31,14 @@ the old one, which is the behavior needed by the `box_self_intersection_d()` algorithm. This is therefore the safe default implementation.
    • `ID_FROM_BOX_ADDRESS`: casts the address of the box into a -`std::ptrdiff_t` to create the `id`-number. Works fine +`std::ptrdiff_t` to create the `id`-number. This works fine if the intersection algorithms work effectively with pointers to boxes, but not in the case where the algorithms work with box values, because the algorithms modify the order of the boxes, and the `box_self_intersection_d()` algorithm creates copies of the boxes that would not have identical `id`-numbers. -
    -
+ \cgalModels `BoxIntersectionBox_d` diff --git a/Box_intersection_d/doc/Box_intersection_d/CGAL/Box_intersection_d/Box_traits_d.h b/Box_intersection_d/doc/Box_intersection_d/CGAL/Box_intersection_d/Box_traits_d.h index 309fcc24d36..c1f1d88520a 100644 --- a/Box_intersection_d/doc/Box_intersection_d/CGAL/Box_intersection_d/Box_traits_d.h +++ b/Box_intersection_d/doc/Box_intersection_d/CGAL/Box_intersection_d/Box_traits_d.h @@ -16,11 +16,9 @@ the `Box_parameter` type required in the `Box_parameter` to be of type `const B&`, while for the other cases it just uses the pointer type. -
    -
  • `BoxHandle`: either a class type `B`, a pointer `B*`, or a -const-pointer `const B*`, where `B` is a model of the -`BoxIntersectionBox_d` concept. -
+\tparam BoxHandle is either a class type `B`, a pointer `B*`, or a + const-pointer `const B*`, where `B` is a model of the + `BoxIntersectionBox_d` concept. \cgalModels `BoxIntersectionTraits_d` diff --git a/Box_intersection_d/doc/Box_intersection_d/CGAL/Box_intersection_d/Box_with_handle_d.h b/Box_intersection_d/doc/Box_intersection_d/CGAL/Box_intersection_d/Box_with_handle_d.h index 45a3a33e83d..5a851bbe395 100644 --- a/Box_intersection_d/doc/Box_intersection_d/CGAL/Box_intersection_d/Box_with_handle_d.h +++ b/Box_intersection_d/doc/Box_intersection_d/CGAL/Box_intersection_d/Box_with_handle_d.h @@ -19,9 +19,9 @@ of the `Assignable` and the `LessThanComparable` concept. \tparam D the dimension of the box. \tparam Handle Handle concept, e.g., a pointer, an iterator, or a circulator. \tparam IdPolicy specifies how the `id`-number will be -provided. Can be one of the following types, where +provided and can be one of the following types, where `ID_FROM_HANDLE` is the default for this parameter: - - `ID_NONE`: no `id`-number is provided. Can be useful + - `ID_NONE`: no `id`-number is provided. This can be useful to have this class as a base class for different implementations of `id`-numbers than the ones provided here. @@ -33,7 +33,7 @@ provided. Can be one of the following types, where `CGAL::box_self_intersection_d()` algorithm. This is therefore the safe default implementation. - `ID_FROM_BOX_ADDRESS`: casts the address of the box into a - `std::ptrdiff_t` to create the `id`-number. Works fine + `std::ptrdiff_t` to create the `id`-number. This works fine if the intersection algorithms work effectively with pointers to boxes, but not in the case where the algorithms work with box values, because the algorithms modify the order of the diff --git a/Box_intersection_d/doc/Box_intersection_d/CGAL/box_intersection_d.h b/Box_intersection_d/doc/Box_intersection_d/CGAL/box_intersection_d.h index 86441b44f23..89b6facc3cc 100644 --- a/Box_intersection_d/doc/Box_intersection_d/CGAL/box_intersection_d.h +++ b/Box_intersection_d/doc/Box_intersection_d/CGAL/box_intersection_d.h @@ -46,8 +46,26 @@ namespace CGAL { An important special application of this algorithm is the test for self-intersections where the second box sequence is an identical copy of the first sequence including the preserved `id`-number. We - offer a specialized implementation - `box_self_intersection_all_pairs` for this application. + offer a specialized implementation `box_self_intersection_all_pairs_d()` + for this application. + +\cgalHeading{Requirements} + +
    +
  • `ForwardIterator1`, and \f$ \ldots\f$ `2`, must meet + the requirements of `ForwardIterator` and both value types must be the same. + We call this value type `Box_handle` in the following. +
  • `Callback` must be of the `BinaryFunction` concept. + The `Box_handle` must be convertible to both argument types. The + return type is not used and can be `void`. +
  • The `Box_handle` must be a model of the `Assignable` concept. +
  • In addition, if the default box traits is used the `Box_handle` must + be a class type `T` or a pointer to a class type `T`, where + `T` must be a model of the `BoxIntersectionBox_d` concept. + In both cases, the default box traits specializes to a suitable + implementation. +
  • `BoxTraits` must be of the `BoxIntersectionTraits_d` concept. +
\sa \link PkgBoxIntersectionD_box_intersection_d `CGAL::box_intersection_d()` \endlink \sa \link PkgBoxIntersectionD_box_self_intersection_d `CGAL::box_self_intersection_d()` \endlink @@ -63,6 +81,40 @@ namespace CGAL { size of the second sequence. */ +/*! + \ingroup PkgBoxIntersectionD_box_intersection_all_pairs_d + + Invocation of box intersection with default box traits + `Box_intersection_d::Box_traits_d`, where + `Box_handle` corresponds to the iterator value type of + `ForwardIterator1`. + +*/ +template< class ForwardIterator1, + class ForwardIterator2, + class Callback > +void box_intersection_all_pairs_d( + ForwardIterator1 begin1, ForwardIterator1 end1, + ForwardIterator2 begin2, ForwardIterator2 end2, + Callback callback, + CGAL::Box_intersection_d::Topology topology = CGAL::Box_intersection_d::CLOSED); + +/*! + \ingroup PkgBoxIntersectionD_box_intersection_all_pairs_d + + Invocation with custom box traits. + +*/ +template< class ForwardIterator1, + class ForwardIterator2, + class Callback, class BoxTraits > +void box_intersection_all_pairs_d( + ForwardIterator1 begin1, ForwardIterator1 end1, + ForwardIterator2 begin2, ForwardIterator2 end2, + Callback callback, + BoxTraits box_traits, + CGAL::Box_intersection_d::Topology topology = CGAL::Box_intersection_d::CLOSED); + /*! \addtogroup PkgBoxIntersectionD_box_intersection_d @@ -112,7 +164,7 @@ namespace CGAL { concept and that the box handle, i.e., the iterators value type, is identical to the box type or a pointer to the box type. - An important special application of this algorithm is the test for + An important special application of this algorithm is the test for self-intersections where the second box sequence is an identical copy of the first sequence including the preserved `id`-number. Note that this implies that the address of the box is not sufficient for @@ -130,12 +182,16 @@ namespace CGAL { values `Box_intersection_d::COMPLETE` and `Box_intersection_d::BIPARTITE`. + \warning The two sequences of boxes passed to `box_intersection_d()` can be + ranges created from the same container, but these ranges must not contain + any common element. + \cgalHeading{Requirements}
    -
  • `RandomAccessIterator1`, and \f$ \ldots\f$ `2`, must be - mutable random-access iterators and both value types must be - the same. We call this value type `Box_handle` in the following. +
  • `RandomAccessIterator1`, and \f$ \ldots\f$ `2`, must meet + the requirements of `RandomAccessIterator` and both value types must be the same. + We call this value type `Box_handle` in the following.
  • `Callback` must be of the `BinaryFunction` concept. The `Box_handle` must be convertible to both argument types. The return type is not used and can be `void`. @@ -208,44 +264,6 @@ namespace CGAL { */ -/*! - \ingroup PkgBoxIntersectionD_box_intersection_all_pairs_d - - Invocation of box intersection with default box traits - `Box_intersection_d::Box_traits_d`, where - `Box_handle` corresponds to the iterator value type of - `ForwardIterator1`. - -*/ -template< class ForwardIterator1, - class ForwardIterator2, - class Callback > -void box_intersection_all_pairs_d( - ForwardIterator1 begin1, ForwardIterator1 end1, - ForwardIterator2 begin2, ForwardIterator2 end2, - Callback callback, - CGAL::Box_intersection_d::Topology topology = CGAL::Box_intersection_d::CLOSED); - -/*! - \ingroup PkgBoxIntersectionD_box_intersection_all_pairs_d - - Invocation with custom box traits. - -*/ -template< class ForwardIterator1, - class ForwardIterator2, - class Callback, class BoxTraits > -void box_intersection_all_pairs_d( - ForwardIterator1 begin1, ForwardIterator1 end1, - ForwardIterator2 begin2, ForwardIterator2 end2, - Callback callback, - BoxTraits box_traits, - CGAL::Box_intersection_d::Topology topology = CGAL::Box_intersection_d::CLOSED); - -} /* namespace CGAL */ - -namespace CGAL { - /*! \ingroup PkgBoxIntersectionD_box_intersection_d @@ -297,7 +315,7 @@ namespace CGAL { inferior to the fast `box_self_intersection_d()` algorithm. The sequence of boxes is given with a forward iterator range. The - sequences are not modified. For each intersecting pair of boxes a + sequence is not modified. For each intersecting pair of boxes a `callback` function object is called with the two intersecting boxes as argument. @@ -330,7 +348,7 @@ namespace CGAL { \cgalHeading{Requirements}
      -
    • `ForwardIterator` must be a forward iterator. We call its +
    • `ForwardIter` must meet the requirements of `ForwardIterator`. We call its value type `Box_handle` in the following.
    • `Callback` must be of the `BinaryFunction` concept. The `Box_handle` must be convertible to both argument types. The @@ -365,13 +383,11 @@ namespace CGAL { Invocation of box intersection with default box traits `Box_intersection_d::Box_traits_d`, where `Box_handle` corresponds to the iterator value type of - `ForwardIterator`. - - + `ForwardIter`. */ -template< class ForwardIterator, class Callback > +template< class ForwardIter, class Callback > void box_self_intersection_all_pairs_d( - ForwardIterator begin, ForwardIterator end, + ForwardIter begin, ForwardIter end, Callback callback, CGAL::Box_intersection_d::Topology topology = CGAL::Box_intersection_d::CLOSED); @@ -381,10 +397,10 @@ void box_self_intersection_all_pairs_d( */ -template< class ForwardIterator, +template< class ForwardIter, class Callback, class BoxTraits > void box_self_intersection_all_pairs_d( - ForwardIterator begin, ForwardIterator end, + ForwardIter begin, ForwardIter end, Callback callback, BoxTraits box_traits, CGAL::Box_intersection_d::Topology topology = CGAL::Box_intersection_d::CLOSED); @@ -430,16 +446,17 @@ namespace CGAL { `Box_intersection_d::HALF_OPEN` and `Box_intersection_d::CLOSED`. - In addition, a box has a unique `id`-number. It is used to order - boxes consistently in each dimension even if boxes have identical - coordinates. In consequence, the algorithm guarantees that a pair of - intersecting boxes is reported only once. This self-intersection - function creates internally a second copy of the box sequence. The - copying has to preserve the `id`-number of boxes. Note that this - implies that the address of the box is not sufficient for the - `id`-number if boxes are copied by value. Boxes of equal - `id`-number are not reported as intersecting pairs since they are - always intersecting trivially. + In addition, a box has a unique `id`-number. It is used to order + boxes consistently in each dimension even if boxes have identical + coordinates. In consequence, the algorithm guarantees that a pair of + intersecting boxes is reported only once. Boxes of equal + `id`-number are not reported as intersecting pairs since they are + always intersecting trivially. + + \warning This self-intersection function creates internally a second copy + of the box sequence. Note that this implies that an `id`-number based on the address + of the box is not acceptable if boxes are copied by value and one must either + pass boxes by pointer or use another type of box `id`-number such as `ID_EXPLICIT`. The algorithm uses a traits class of the `BoxIntersectionTraits_d` concept to access the boxes. A default traits class is provided that diff --git a/Box_intersection_d/doc/Box_intersection_d/Concepts/BoxIntersectionTraits_d.h b/Box_intersection_d/doc/Box_intersection_d/Concepts/BoxIntersectionTraits_d.h index 0b0614bd52a..f4203aca441 100644 --- a/Box_intersection_d/doc/Box_intersection_d/Concepts/BoxIntersectionTraits_d.h +++ b/Box_intersection_d/doc/Box_intersection_d/Concepts/BoxIntersectionTraits_d.h @@ -8,6 +8,7 @@ functions to the dimension, the `id`-number, and the boundaries of the boxes manipulated in these algorithms. \cgalRefines `Assignable` +\cgalRefines `DefaultConstructible` \cgalHasModel CGAL::Box_intersection_d::Box_traits_d From a443f85a52ac715b4f0d263988e898a7ef6f1846 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Fri, 16 Mar 2018 14:33:51 +0100 Subject: [PATCH 08/13] fix the determinism of the computation of point and base vectors possibly will return true in case the evaluation lead to true or false as possible values. If the exact value is computed or interval refined, false could be the true value which has a consequence will give a different result for point or base vector. --- Cartesian_kernel/include/CGAL/Cartesian/function_objects.h | 4 ++-- Cartesian_kernel/include/CGAL/constructions/kernel_ftC3.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cartesian_kernel/include/CGAL/Cartesian/function_objects.h b/Cartesian_kernel/include/CGAL/Cartesian/function_objects.h index cfce328606f..f92ab27d60d 100644 --- a/Cartesian_kernel/include/CGAL/Cartesian/function_objects.h +++ b/Cartesian_kernel/include/CGAL/Cartesian/function_objects.h @@ -1771,10 +1771,10 @@ namespace CartesianKernelFunctors { // to avoid badly defined vectors with coordinates all close // to 0 when the plane is almost horizontal, we ignore the // smallest coordinate instead of always ignoring Z - if (CGAL::possibly(CGAL_AND (a <= b, a <= c))) + if (a <= b && a <= c) return Vector_3(FT(0), -h.c(), h.b()); - if (CGAL::possibly(CGAL_AND (b <= a, b <= c))) + if (b <= a && b <= c) return Vector_3(-h.c(), FT(0), h.a()); return Vector_3(-h.b(), h.a(), FT(0)); diff --git a/Cartesian_kernel/include/CGAL/constructions/kernel_ftC3.h b/Cartesian_kernel/include/CGAL/constructions/kernel_ftC3.h index 22cd48f3af0..a43213be06c 100644 --- a/Cartesian_kernel/include/CGAL/constructions/kernel_ftC3.h +++ b/Cartesian_kernel/include/CGAL/constructions/kernel_ftC3.h @@ -272,9 +272,9 @@ point_on_planeC3(const FT &pa, const FT &pb, const FT &pc, const FT &pd, // to avoid badly defined point with an overly large coordinate when // the plane is almost orthogonal to one axis, we use the largest // scalar coordinate instead of always using the first non-null - if (CGAL::possibly(CGAL_AND (abs_pa >= abs_pb, abs_pa >= abs_pc))) + if (abs_pa >= abs_pb && abs_pa >= abs_pc) x = -pd/pa; - else if (CGAL::possibly(CGAL_AND (abs_pb >= abs_pa, abs_pb >= abs_pc))) + else if (abs_pb >= abs_pa && abs_pb >= abs_pc) y = -pd/pb; else z = -pd/pc; From 2854734bf54b37fc777c7fab637c4d3c57accddb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mael=20Rouxel-Labb=C3=A9?= Date: Tue, 20 Mar 2018 10:56:57 +0100 Subject: [PATCH 09/13] Moved is_simple_2()'s empty test to SimplicityTest's degenerate tests --- Polygon/test/Polygon/AlgorithmTest.cpp | 7 +++---- Polygon/test/Polygon/SimplicityTest.cpp | 3 +++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Polygon/test/Polygon/AlgorithmTest.cpp b/Polygon/test/Polygon/AlgorithmTest.cpp index cc5fac6a76e..a4c7e937cee 100644 --- a/Polygon/test/Polygon/AlgorithmTest.cpp +++ b/Polygon/test/Polygon/AlgorithmTest.cpp @@ -66,10 +66,6 @@ void test_polygon(const R&, const Point&, const char* FileName) CGAL::Orientation orientation = CGAL::orientation_2(polygon.begin(), polygon.end()); - polygon.clear(); - assert(CGAL::is_simple_2(polygon.begin(), polygon.end())); - assert(CGAL::is_convex_2(polygon.begin(), polygon.end())); - cout << "left = " << *left << endl; cout << "right = " << *right << endl; cout << "top = " << *top << endl; @@ -120,6 +116,9 @@ void test_polygon(const R&, const Point&, const char* FileName) cout << "the orientation is collinear" << endl; break; } + + polygon.clear(); + assert(CGAL::is_convex_2(polygon.begin(), polygon.end())); } //-----------------------------------------------------------------------// diff --git a/Polygon/test/Polygon/SimplicityTest.cpp b/Polygon/test/Polygon/SimplicityTest.cpp index e33a7741ef6..acbb61f69f4 100644 --- a/Polygon/test/Polygon/SimplicityTest.cpp +++ b/Polygon/test/Polygon/SimplicityTest.cpp @@ -63,6 +63,9 @@ void TestDegenerateCases() polygon.push_back(Point(1,2)); assert(CGAL::is_simple_2(polygon.begin(), polygon.end())); + + polygon.clear(); + assert(CGAL::is_simple_2(polygon.begin(), polygon.end())); } int main() From 9240f29ff8d657703353497e732a1c64a3bbe927 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Wed, 21 Mar 2018 16:48:45 +0100 Subject: [PATCH 10/13] update test to check both combinaisons --- .../Polygon_mesh_processing/test_corefine.cpp | 72 ++++++++++--------- 1 file changed, 39 insertions(+), 33 deletions(-) diff --git a/Polygon_mesh_processing/test/Polygon_mesh_processing/test_corefine.cpp b/Polygon_mesh_processing/test/Polygon_mesh_processing/test_corefine.cpp index 6f51922d838..4aee863f3a6 100644 --- a/Polygon_mesh_processing/test/Polygon_mesh_processing/test_corefine.cpp +++ b/Polygon_mesh_processing/test/Polygon_mesh_processing/test_corefine.cpp @@ -10,42 +10,48 @@ typedef CGAL::Exact_predicates_inexact_constructions_kernel K; typedef CGAL::Surface_mesh Surface_mesh; typedef CGAL::Polyhedron_3 Polyhedron_3; + +void test(const char* f1, const char* f2) +{ + std::cout << "Corefining " << f1 + << " and " << f2 << "\n"; + + std::cout << " with Surface_mesh\n"; + Surface_mesh sm1, sm2; + std::ifstream input(f1); + assert(input); + input >> sm1; + input.close(); + input.open(f2); + assert(input); + input >> sm2; + input.close(); + + CGAL::Polygon_mesh_processing::corefine(sm1, sm2); + + assert(sm1.is_valid()); + assert(sm2.is_valid()); + + std::cout << " with Polyhedron_3\n"; + Polyhedron_3 P, Q; + input.open(f1); + assert(input); + input >> P; + input.close(); + input.open(f2); + assert(input); + input >> Q; + + CGAL::Polygon_mesh_processing::corefine(P, Q); + + assert(P.is_valid()); + assert(Q.is_valid()); +} int main(int argc, char** argv) { for(int i=0; i< (argc-1)/2;++i) { - std::cout << "Corefining " << argv[2*i+1] - << " and " << argv[2*(i+1)] << "\n"; - - std::cout << " with Surface_mesh\n"; - Surface_mesh sm1, sm2; - std::ifstream input(argv[2*i+1]); - assert(input); - input >> sm1; - input.close(); - input.open(argv[2*(i+1)]); - assert(input); - input >> sm2; - input.close(); - - CGAL::Polygon_mesh_processing::corefine(sm1, sm2); - - assert(sm1.is_valid()); - assert(sm2.is_valid()); - - std::cout << " with Polyhedron_3\n"; - Polyhedron_3 P, Q; - input.open(argv[2*i+1]); - assert(input); - input >> P; - input.close(); - input.open(argv[2*(i+1)]); - assert(input); - input >> Q; - - CGAL::Polygon_mesh_processing::corefine(P, Q); - - assert(P.is_valid()); - assert(Q.is_valid()); + test(argv[2*i+1], argv[2*(i+1)]); + test(argv[2*(i+1)], argv[2*i+1]); } } From 6bb6f470c4cec207fe2489ba8bf08c3e91e02538 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Wed, 21 Mar 2018 16:49:12 +0100 Subject: [PATCH 11/13] update incorrectly handled coplanar intersection computation --- .../intersection_of_coplanar_triangles_3.h | 24 +++++++++++++++---- .../all_cases/bugreport/tr2-1.off | 6 +++++ .../all_cases/bugreport/tr2-2.off | 6 +++++ .../Polygon_mesh_processing/test_corefine.cmd | 1 + 4 files changed, 33 insertions(+), 4 deletions(-) create mode 100644 Polygon_mesh_processing/test/Polygon_mesh_processing/data-coref/coplanar_triangles/all_cases/bugreport/tr2-1.off create mode 100644 Polygon_mesh_processing/test/Polygon_mesh_processing/data-coref/coplanar_triangles/all_cases/bugreport/tr2-2.off diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Corefinement/intersection_of_coplanar_triangles_3.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Corefinement/intersection_of_coplanar_triangles_3.h index 3cfcf3326fd..489bbd963aa 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Corefinement/intersection_of_coplanar_triangles_3.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Corefinement/intersection_of_coplanar_triangles_3.h @@ -79,7 +79,7 @@ struct Intersect_coplanar_faces_3{ //constructor for intersection of edges. prev and curr are two points on an edge of the first facet (preserving the //orientation of the facet). This edge is intersected by h2 from the second facet. // - //The rational is the following: we first check whether curr and prev are on the same edge. I so we create + //The rational is the following: we first check whether curr and prev are on the same edge. If so we create //an intersection point between two edges. Otherwise, the point is a vertex of the second facet included into //the first facet. // @@ -102,9 +102,25 @@ struct Intersect_coplanar_faces_3{ res.info_2=h2; if (ipt_prev.type_1==ON_VERTEX && next(ipt_prev.info_1, tm1) == ipt_curr.info_1){ - CGAL_assertion(ipt_curr.type_1!=ON_FACE); - res.type_1=ON_EDGE; - res.info_1=ipt_curr.info_1; + if(ipt_curr.type_1!=ON_FACE) + { + res.type_1=ON_EDGE; + res.info_1=ipt_curr.info_1; + } + else + { + CGAL_assertion( ipt_curr.type_2==ON_VERTEX); + res.type_1=ON_FACE; + res.info_1=h1; + res.type_2=ON_VERTEX; + typename Exact_kernel::Collinear_3 is_collinear = Exact_kernel().collinear_3_object(); + if ( !is_collinear(ipt_prev.point,ipt_curr.point,to_exact(get(vpm2,target(res.info_2,tm2)) ) ) ){ + res.info_2=prev(res.info_2,tm2); + CGAL_assertion( is_collinear(ipt_prev.point,ipt_curr.point,to_exact(get(vpm2,target(res.info_2,tm2))) ) ); + } + res.point = to_exact( get(vpm2, target(res.info_2,tm2)) ); + return res; + } } else{ if(ipt_curr.type_1==ON_VERTEX && ipt_prev.info_1 == ipt_curr.info_1){ diff --git a/Polygon_mesh_processing/test/Polygon_mesh_processing/data-coref/coplanar_triangles/all_cases/bugreport/tr2-1.off b/Polygon_mesh_processing/test/Polygon_mesh_processing/data-coref/coplanar_triangles/all_cases/bugreport/tr2-1.off new file mode 100644 index 00000000000..692c64e5c3d --- /dev/null +++ b/Polygon_mesh_processing/test/Polygon_mesh_processing/data-coref/coplanar_triangles/all_cases/bugreport/tr2-1.off @@ -0,0 +1,6 @@ +OFF +3 1 0 +0 5310 100 +0 5400 100 +0 5310 150 +3 0 2 1 diff --git a/Polygon_mesh_processing/test/Polygon_mesh_processing/data-coref/coplanar_triangles/all_cases/bugreport/tr2-2.off b/Polygon_mesh_processing/test/Polygon_mesh_processing/data-coref/coplanar_triangles/all_cases/bugreport/tr2-2.off new file mode 100644 index 00000000000..21bb089ace4 --- /dev/null +++ b/Polygon_mesh_processing/test/Polygon_mesh_processing/data-coref/coplanar_triangles/all_cases/bugreport/tr2-2.off @@ -0,0 +1,6 @@ +OFF +3 1 0 +0 5375 105 +0 5350 110 +0 5375 110 +3 1 2 0 diff --git a/Polygon_mesh_processing/test/Polygon_mesh_processing/test_corefine.cmd b/Polygon_mesh_processing/test/Polygon_mesh_processing/test_corefine.cmd index a173eef4b20..e7e5fb3547b 100644 --- a/Polygon_mesh_processing/test/Polygon_mesh_processing/test_corefine.cmd +++ b/Polygon_mesh_processing/test/Polygon_mesh_processing/test_corefine.cmd @@ -64,6 +64,7 @@ data-coref/coplanar_triangles/all_cases/deg/tr15-1.off data-coref/coplanar_trian data-coref/coplanar_triangles/all_cases/deg/tr16-1.off data-coref/coplanar_triangles/all_cases/deg/tr16-2.off data-coref/coplanar_triangles/all_cases/deg/tr17-1.off data-coref/coplanar_triangles/all_cases/deg/tr17-2.off data-coref/coplanar_triangles/all_cases/bugreport/tr1-1.off data-coref/coplanar_triangles/all_cases/bugreport/tr1-2.off +data-coref/coplanar_triangles/all_cases/bugreport/tr2-1.off data-coref/coplanar_triangles/all_cases/bugreport/tr2-2.off data-coref/coplanar_triangles/all_cases/tr1-1.off data-coref/coplanar_triangles/all_cases/tr1-2.off data-coref/coplanar_triangles/all_cases/tr2-1.off data-coref/coplanar_triangles/all_cases/tr2-2.off data-coref/coplanar_triangles/all_cases/tr3-1.off data-coref/coplanar_triangles/all_cases/tr3-2.off From 5ac449dfd9c454c963541ba2242a89910c434005 Mon Sep 17 00:00:00 2001 From: Jane Tournois Date: Fri, 23 Mar 2018 11:45:13 +0100 Subject: [PATCH 12/13] remove function that makes no sense from the doc there cannot be 2 corners with the same index --- .../Concepts/MeshComplexWithFeatures_3InTriangulation_3.h | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Mesh_3/doc/Mesh_3/Concepts/MeshComplexWithFeatures_3InTriangulation_3.h b/Mesh_3/doc/Mesh_3/Concepts/MeshComplexWithFeatures_3InTriangulation_3.h index 0fcbb496951..dd7c71aee66 100644 --- a/Mesh_3/doc/Mesh_3/Concepts/MeshComplexWithFeatures_3InTriangulation_3.h +++ b/Mesh_3/doc/Mesh_3/Concepts/MeshComplexWithFeatures_3InTriangulation_3.h @@ -155,12 +155,6 @@ Returns the number of vertices which are corners of the complex. */ size_type number_of_corners() const; -/*! - -Returns the number of vertices which are corners of the complex with index `index`. -*/ -size_type number_of_corners(Corner_index index) const; - /*! Returns `true` iff edge `e` belongs to some curve segment. From b9f4a4e06beae9b8d538274aa248a6c24ca2362c Mon Sep 17 00:00:00 2001 From: Jane Tournois Date: Fri, 23 Mar 2018 11:45:49 +0100 Subject: [PATCH 13/13] implement number_of_corners(), that is documented --- Mesh_3/include/CGAL/Mesh_complex_3_in_triangulation_3.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Mesh_3/include/CGAL/Mesh_complex_3_in_triangulation_3.h b/Mesh_3/include/CGAL/Mesh_complex_3_in_triangulation_3.h index b12eaf4a948..13c9d2d1295 100644 --- a/Mesh_3/include/CGAL/Mesh_complex_3_in_triangulation_3.h +++ b/Mesh_3/include/CGAL/Mesh_complex_3_in_triangulation_3.h @@ -286,6 +286,10 @@ public: { return corners_.size(); } + size_type number_of_corners() const + { + return corners_.size(); + } void rescan_after_load_of_triangulation();