From 80af4b3e747e362f9c0e1c30f1a51b6e0fe69eb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Mon, 26 Oct 2020 09:17:01 +0100 Subject: [PATCH 01/38] be more permissive for bisector with a FT with sqrt --- .../include/CGAL/constructions/kernel_ftC2.h | 4 ++-- .../include/CGAL/constructions/kernel_ftC3.h | 8 ++++---- .../doc/Kernel_23/CGAL/Kernel/global_functions.h | 12 ++++++++---- .../doc/Kernel_23/Concepts/FunctionObjectConcepts.h | 12 ++++++++---- 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/Cartesian_kernel/include/CGAL/constructions/kernel_ftC2.h b/Cartesian_kernel/include/CGAL/constructions/kernel_ftC2.h index c5066bec4d0..f5d27322792 100644 --- a/Cartesian_kernel/include/CGAL/constructions/kernel_ftC2.h +++ b/Cartesian_kernel/include/CGAL/constructions/kernel_ftC2.h @@ -246,8 +246,8 @@ bisector_of_linesC2(const FT &pa, const FT &pb, const FT &pc, FT &a, FT &b, FT &c) { // We normalize the equations of the 2 lines, and we then add them. - FT n1 = CGAL_NTS sqrt(CGAL_NTS square(pa) + CGAL_NTS square(pb)); - FT n2 = CGAL_NTS sqrt(CGAL_NTS square(qa) + CGAL_NTS square(qb)); + FT n1 = CGAL_NTS approximate_sqrt( FT(CGAL_NTS square(pa) + CGAL_NTS square(pb)) ); + FT n2 = CGAL_NTS approximate_sqrt( FT(CGAL_NTS square(qa) + CGAL_NTS square(qb)) ); a = n2 * pa + n1 * qa; b = n2 * pb + n1 * qb; c = n2 * pc + n1 * qc; diff --git a/Cartesian_kernel/include/CGAL/constructions/kernel_ftC3.h b/Cartesian_kernel/include/CGAL/constructions/kernel_ftC3.h index 63a1442f511..ac1dced802c 100644 --- a/Cartesian_kernel/include/CGAL/constructions/kernel_ftC3.h +++ b/Cartesian_kernel/include/CGAL/constructions/kernel_ftC3.h @@ -366,10 +366,10 @@ bisector_of_planesC3(const FT &pa, const FT &pb, const FT &pc, const FT &pd, FT &a, FT &b, FT &c, FT &d) { // We normalize the equations of the 2 planes, and we then add them. - FT n1 = CGAL_NTS sqrt(CGAL_NTS square(pa) + CGAL_NTS square(pb) + - CGAL_NTS square(pc)); - FT n2 = CGAL_NTS sqrt(CGAL_NTS square(qa) + CGAL_NTS square(qb) + - CGAL_NTS square(qc)); + FT n1 = CGAL_NTS approximate_sqrt( FT(CGAL_NTS square(pa) + CGAL_NTS square(pb) + + CGAL_NTS square(pc)) ); + FT n2 = CGAL_NTS approximate_sqrt( FT(CGAL_NTS square(qa) + CGAL_NTS square(qb) + + CGAL_NTS square(qc)) ); a = n2 * pa + n1 * qa; b = n2 * pb + n1 * qb; c = n2 * pc + n1 * qc; diff --git a/Kernel_23/doc/Kernel_23/CGAL/Kernel/global_functions.h b/Kernel_23/doc/Kernel_23/CGAL/Kernel/global_functions.h index 98ea9f7ed60..24b5d073631 100644 --- a/Kernel_23/doc/Kernel_23/CGAL/Kernel/global_functions.h +++ b/Kernel_23/doc/Kernel_23/CGAL/Kernel/global_functions.h @@ -355,8 +355,10 @@ through the intersection of `l1` and `l2`. If `l1` and `l2` are parallel, then the bisector is defined as the line which has the same direction as `l1`, and which is at the same distance from `l1` and `l2`. -This function requires that `Kernel::RT` supports the `sqrt()` -operation. +If `Kernel::FT` is not a model of `FieldWithSqrt` +an approximation of the square root will be used in this function, +impacting the exactness of the result even with a (exact) multiprecision +number type. */ template CGAL::Line_2 bisector(const CGAL::Line_2 &l1, @@ -379,8 +381,10 @@ passes through the intersection of `h1` and `h2`. If `h1` and `h2` are parallel, then the bisector is defined as the plane which has the same oriented normal vector as `l1`, and which is at the same distance from `h1` and `h2`. -This function requires that `Kernel::RT` supports the `sqrt()` -operation. +If `Kernel::FT` is not a model of `FieldWithSqrt` +an approximation of the square root will be used in this function, +impacting the exactness of the result even with a (exact) multiprecision +number type. */ template CGAL::Plane_3 bisector(const CGAL::Plane_3 &h1, diff --git a/Kernel_23/doc/Kernel_23/Concepts/FunctionObjectConcepts.h b/Kernel_23/doc/Kernel_23/Concepts/FunctionObjectConcepts.h index b63475c7188..a8ea38cd865 100644 --- a/Kernel_23/doc/Kernel_23/Concepts/FunctionObjectConcepts.h +++ b/Kernel_23/doc/Kernel_23/Concepts/FunctionObjectConcepts.h @@ -3848,8 +3848,10 @@ public: If `l1` and `l2` are parallel, then the bisector is defined as the line which has the same direction as `l1`, and which is at the same distance from `l1` and `l2`. - This function requires that `Kernel::RT` supports the `sqrt()` - operation. + If `Kernel::FT` is not a model of `FieldWithSqrt` + an approximation of the square root will be used in this function, + impacting the exactness of the result even with a (exact) multiprecision + number type. */ Kernel::Line_2 operator()(const Kernel::Line_2&l1, const Kernel::Line_2&l2); @@ -3891,8 +3893,10 @@ public: If `h1` and `h2` are parallel, then the bisector is defined as the plane which has the same oriented normal vector as `h1`, and which is at the same distance from `h1` and `h2`. - This function requires that `Kernel::RT` supports the `sqrt()` - operation. + If `Kernel::FT` is not a model of `FieldWithSqrt` + an approximation of the square root will be used in this function, + impacting the exactness of the result even with a (exact) multiprecision + number type. */ Kernel::Plane_3 operator()(const Kernel::Plane_3&h1, const Kernel::Plane_3&h2); From e685203cc60602d8cb5361aa4a9ec31fa22c304b Mon Sep 17 00:00:00 2001 From: Sebastien Loriot Date: Mon, 26 Oct 2020 11:08:56 +0100 Subject: [PATCH 02/38] a -> an --- Kernel_23/doc/Kernel_23/CGAL/Kernel/global_functions.h | 4 ++-- Kernel_23/doc/Kernel_23/Concepts/FunctionObjectConcepts.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Kernel_23/doc/Kernel_23/CGAL/Kernel/global_functions.h b/Kernel_23/doc/Kernel_23/CGAL/Kernel/global_functions.h index 24b5d073631..bb63612593e 100644 --- a/Kernel_23/doc/Kernel_23/CGAL/Kernel/global_functions.h +++ b/Kernel_23/doc/Kernel_23/CGAL/Kernel/global_functions.h @@ -357,7 +357,7 @@ which has the same direction as `l1`, and which is at the same distance from `l1` and `l2`. If `Kernel::FT` is not a model of `FieldWithSqrt` an approximation of the square root will be used in this function, -impacting the exactness of the result even with a (exact) multiprecision +impacting the exactness of the result even with an (exact) multiprecision number type. */ template @@ -383,7 +383,7 @@ plane which has the same oriented normal vector as `l1`, and which is at the same distance from `h1` and `h2`. If `Kernel::FT` is not a model of `FieldWithSqrt` an approximation of the square root will be used in this function, -impacting the exactness of the result even with a (exact) multiprecision +impacting the exactness of the result even with an (exact) multiprecision number type. */ template diff --git a/Kernel_23/doc/Kernel_23/Concepts/FunctionObjectConcepts.h b/Kernel_23/doc/Kernel_23/Concepts/FunctionObjectConcepts.h index a8ea38cd865..34c54c85e42 100644 --- a/Kernel_23/doc/Kernel_23/Concepts/FunctionObjectConcepts.h +++ b/Kernel_23/doc/Kernel_23/Concepts/FunctionObjectConcepts.h @@ -3850,7 +3850,7 @@ public: from `l1` and `l2`. If `Kernel::FT` is not a model of `FieldWithSqrt` an approximation of the square root will be used in this function, - impacting the exactness of the result even with a (exact) multiprecision + impacting the exactness of the result even with an (exact) multiprecision number type. */ Kernel::Line_2 operator()(const Kernel::Line_2&l1, @@ -3895,7 +3895,7 @@ public: the same distance from `h1` and `h2`. If `Kernel::FT` is not a model of `FieldWithSqrt` an approximation of the square root will be used in this function, - impacting the exactness of the result even with a (exact) multiprecision + impacting the exactness of the result even with an (exact) multiprecision number type. */ Kernel::Plane_3 operator()(const Kernel::Plane_3&h1, From 061c6684fb443e132a660faa4ab619a37424099e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Thu, 29 Oct 2020 16:08:23 +0100 Subject: [PATCH 03/38] use Sqrt function if available --- Algebraic_foundations/include/CGAL/number_utils.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Algebraic_foundations/include/CGAL/number_utils.h b/Algebraic_foundations/include/CGAL/number_utils.h index abb7894ccf4..7c1ac814af6 100644 --- a/Algebraic_foundations/include/CGAL/number_utils.h +++ b/Algebraic_foundations/include/CGAL/number_utils.h @@ -302,13 +302,13 @@ to_interval( const Real_embeddable& x) { } template -NT approximate_sqrt(const NT& nt, CGAL::Field_tag) +NT approximate_sqrt(const NT& nt, CGAL::Null_functor) { return NT(sqrt(CGAL::to_double(nt))); } -template -NT approximate_sqrt(const NT& nt, CGAL::Field_with_sqrt_tag) +template +NT approximate_sqrt(const NT& nt, Sqrt sqrt) { return sqrt(nt); } @@ -317,8 +317,8 @@ template NT approximate_sqrt(const NT& nt) { typedef CGAL::Algebraic_structure_traits AST; - typedef typename AST::Algebraic_category Algebraic_category; - return approximate_sqrt(nt, Algebraic_category()); + typedef typename AST::Sqrt Sqrt; + return approximate_sqrt(nt, Sqrt()); } CGAL_NTS_END_NAMESPACE From a4b0f88d41f58dcfb9b9fc6756fadd5a6e13dba3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Thu, 29 Oct 2020 16:08:47 +0100 Subject: [PATCH 04/38] test all now --- .../Kernel_23/include/CGAL/_test_fct_line_2.h | 25 +++---------------- .../include/CGAL/_test_fct_plane_3.h | 22 +++------------- 2 files changed, 7 insertions(+), 40 deletions(-) diff --git a/Kernel_23/test/Kernel_23/include/CGAL/_test_fct_line_2.h b/Kernel_23/test/Kernel_23/include/CGAL/_test_fct_line_2.h index e88c95fb0fe..28ec8a3d1ea 100644 --- a/Kernel_23/test/Kernel_23/include/CGAL/_test_fct_line_2.h +++ b/Kernel_23/test/Kernel_23/include/CGAL/_test_fct_line_2.h @@ -18,22 +18,9 @@ #ifndef CGAL__TEST_FCT_LINE_2_H #define CGAL__TEST_FCT_LINE_2_H -// Accessory function testing functions that require sqrt(). -// Doesn't instantiate anything if RT doesn't support sqrt(). template bool -_test_fct_line_sqrt_2(const R&, CGAL::Tag_false) -{ -// bool UNTESTED_STUFF_BECAUSE_SQRT_IS_NOT_SUPPORTED; - std::cout << std::endl - << "NOTE : FT doesn't support sqrt()," - " hence some functions are not tested." << std::endl; - return true; -} - -template -bool -_test_fct_line_sqrt_2(const R&, CGAL::Tag_true) +_test_fct_line_sqrt_2(const R&) { typedef typename R::Point_2 Point_2; typedef typename R::Line_2 Line_2; @@ -178,13 +165,9 @@ _test_fct_line_2(const R& ) assert(bl1.oriented_side(p2) == CGAL::ON_POSITIVE_SIDE); assert( CGAL::parallel(bl1, bl2) ); - // More tests, that require sqrt(). - { - typedef ::CGAL::Algebraic_structure_traits AST; - static const bool has_sqrt = - ! ::boost::is_same< ::CGAL::Null_functor, typename AST::Sqrt >::value; - _test_fct_line_sqrt_2(R(), ::CGAL::Boolean_tag()); - } + // More tests, that require sqrt() or use approx. + _test_fct_line_sqrt_2(R()); + std::cout << "done" << std::endl; return true; } diff --git a/Kernel_23/test/Kernel_23/include/CGAL/_test_fct_plane_3.h b/Kernel_23/test/Kernel_23/include/CGAL/_test_fct_plane_3.h index f39bc4d1ae7..a5502ad4c3b 100644 --- a/Kernel_23/test/Kernel_23/include/CGAL/_test_fct_plane_3.h +++ b/Kernel_23/test/Kernel_23/include/CGAL/_test_fct_plane_3.h @@ -17,22 +17,9 @@ #ifndef CGAL__TEST_FCT_PLANE_3_H #define CGAL__TEST_FCT_PLANE_3_H -// Accessory function testing functions that require sqrt(). -// Doesn't instantiate anything if RT doesn't support sqrt(). template bool -_test_fct_plane_sqrt_3(const R&, CGAL::Tag_false) -{ -// bool UNTESTED_STUFF_BECAUSE_SQRT_IS_NOT_SUPPORTED; - std::cout << std::endl - << "NOTE : FT doesn't support sqrt()," - " hence some functions are not tested." << std::endl; - return true; -} - -template -bool -_test_fct_plane_sqrt_3(const R&, CGAL::Tag_true) +_test_fct_plane_sqrt_3(const R&) { typedef typename R::Point_3 Point_3; typedef typename R::Plane_3 Plane_3; @@ -97,11 +84,8 @@ _test_fct_plane_3(const R& ) assert( CGAL::parallel(h1, h2) ); assert( ! CGAL::parallel(h1, h5) ); - // More tests, that require sqrt(). - typedef ::CGAL::Algebraic_structure_traits AST; - static const bool has_sqrt = - ! ::boost::is_same< ::CGAL::Null_functor, typename AST::Sqrt >::value; - _test_fct_plane_sqrt_3(R(), ::CGAL::Boolean_tag()); + // More tests, that require sqrt() or use approx. + _test_fct_plane_sqrt_3(R()); std::cout << "done" << std::endl; return true; From 0accdc3c7953319d90b5063a66e5c960a3c326ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Thu, 29 Oct 2020 16:32:23 +0100 Subject: [PATCH 05/38] add comment --- Algebraic_foundations/include/CGAL/number_utils.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Algebraic_foundations/include/CGAL/number_utils.h b/Algebraic_foundations/include/CGAL/number_utils.h index 7c1ac814af6..d29434ad603 100644 --- a/Algebraic_foundations/include/CGAL/number_utils.h +++ b/Algebraic_foundations/include/CGAL/number_utils.h @@ -316,6 +316,9 @@ NT approximate_sqrt(const NT& nt, Sqrt sqrt) template NT approximate_sqrt(const NT& nt) { + // the initial version of this function was using Algebraic_category + // for the dispatch but some ring type (like Gmpz) provides a Sqrt + // functor even if not being Field_with_sqrt. typedef CGAL::Algebraic_structure_traits AST; typedef typename AST::Sqrt Sqrt; return approximate_sqrt(nt, Sqrt()); From fa22cd972b660b54894c5907c19fdf06cff38d8e Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Thu, 14 Jan 2021 13:58:15 +0100 Subject: [PATCH 06/38] Change libpointmatcher_support so it won't fail when old boost has been used to build pointmatcher. --- Installation/cmake/modules/CGAL_pointmatcher_support.cmake | 7 +++---- .../examples/Point_set_processing_3/CMakeLists.txt | 1 + 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Installation/cmake/modules/CGAL_pointmatcher_support.cmake b/Installation/cmake/modules/CGAL_pointmatcher_support.cmake index c023102661f..520e4d3a2b2 100644 --- a/Installation/cmake/modules/CGAL_pointmatcher_support.cmake +++ b/Installation/cmake/modules/CGAL_pointmatcher_support.cmake @@ -1,7 +1,6 @@ if(libpointmatcher_FOUND AND NOT TARGET CGAL::pointmatcher_support) add_library(CGAL::pointmatcher_support INTERFACE IMPORTED) - set_target_properties(CGAL::pointmatcher_support PROPERTIES - INTERFACE_COMPILE_DEFINITIONS "CGAL_LINKED_WITH_POINTMATCHER" - INTERFACE_INCLUDE_DIRECTORIES "${libpointmatcher_INCLUDE_DIR}" - INTERFACE_LINK_LIBRARIES "${libpointmatcher_LIBRARIES}") + target_compile_definitions(CGAL::pointmatcher_support INTERFACE "CGAL_LINKED_WITH_POINTMATCHER") + target_include_directories(CGAL::pointmatcher_support INTERFACE "${libpointmatcher_INCLUDE_DIR}") + target_link_libraries(CGAL::pointmatcher_support INTERFACE "${libpointmatcher_LIBRARIES}") endif() diff --git a/Point_set_processing_3/examples/Point_set_processing_3/CMakeLists.txt b/Point_set_processing_3/examples/Point_set_processing_3/CMakeLists.txt index 83253368016..93dab1bc293 100644 --- a/Point_set_processing_3/examples/Point_set_processing_3/CMakeLists.txt +++ b/Point_set_processing_3/examples/Point_set_processing_3/CMakeLists.txt @@ -93,6 +93,7 @@ if ( CGAL_FOUND ) find_package(libpointmatcher QUIET) include(CGAL_pointmatcher_support) if (TARGET CGAL::pointmatcher_support) + find_package(Boost COMPONENTS thread filesystem system program_options date_time chrono REQUIRED) add_executable(registration_with_pointmatcher "registration_with_pointmatcher.cpp") target_link_libraries(registration_with_pointmatcher ${CGAL_libs} CGAL::pointmatcher_support) From ff9d714f85f5b356e70c1029f913ce94b6b36a44 Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Thu, 14 Jan 2021 14:06:18 +0100 Subject: [PATCH 07/38] Fix input --- .../Point_set_processing_3/registration_with_OpenGR.cpp | 4 ++-- .../registration_with_opengr_pointmatcher_pipeline.cpp | 4 ++-- .../Point_set_processing_3/registration_with_pointmatcher.cpp | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Point_set_processing_3/examples/Point_set_processing_3/registration_with_OpenGR.cpp b/Point_set_processing_3/examples/Point_set_processing_3/registration_with_OpenGR.cpp index ae11db41dbf..ff1aa3752d2 100644 --- a/Point_set_processing_3/examples/Point_set_processing_3/registration_with_OpenGR.cpp +++ b/Point_set_processing_3/examples/Point_set_processing_3/registration_with_OpenGR.cpp @@ -25,7 +25,7 @@ int main(int argc, const char** argv) const char* fname2 = (argc>2)?argv[2]:"data/hippo2.ply"; std::vector pwns1, pwns2; - std::ifstream input(fname1); + std::ifstream input(fname1, std::ios::binary); if (!input || !CGAL::read_ply_points(input, std::back_inserter(pwns1), CGAL::parameters::point_map (CGAL::First_of_pair_property_map()). @@ -36,7 +36,7 @@ int main(int argc, const char** argv) } input.close(); - input.open(fname2); + input.open(fname2, std::ios::binary); if (!input || !CGAL::read_ply_points(input, std::back_inserter(pwns2), CGAL::parameters::point_map (Point_map()). diff --git a/Point_set_processing_3/examples/Point_set_processing_3/registration_with_opengr_pointmatcher_pipeline.cpp b/Point_set_processing_3/examples/Point_set_processing_3/registration_with_opengr_pointmatcher_pipeline.cpp index 2af66826d73..a5ab2cc5c36 100644 --- a/Point_set_processing_3/examples/Point_set_processing_3/registration_with_opengr_pointmatcher_pipeline.cpp +++ b/Point_set_processing_3/examples/Point_set_processing_3/registration_with_opengr_pointmatcher_pipeline.cpp @@ -28,7 +28,7 @@ int main(int argc, const char** argv) const char* fname2 = (argc>2)?argv[2]:"data/hippo2.ply"; std::vector pwns1, pwns2; - std::ifstream input(fname1); + std::ifstream input(fname1, std::ios::binary); if (!input || !CGAL::read_ply_points(input, std::back_inserter(pwns1), CGAL::parameters::point_map (CGAL::First_of_pair_property_map()). @@ -39,7 +39,7 @@ int main(int argc, const char** argv) } input.close(); - input.open(fname2); + input.open(fname2, std::ios::binary); if (!input || !CGAL::read_ply_points(input, std::back_inserter(pwns2), CGAL::parameters::point_map (Point_map()). diff --git a/Point_set_processing_3/examples/Point_set_processing_3/registration_with_pointmatcher.cpp b/Point_set_processing_3/examples/Point_set_processing_3/registration_with_pointmatcher.cpp index f4de0556667..ea903b9b412 100644 --- a/Point_set_processing_3/examples/Point_set_processing_3/registration_with_pointmatcher.cpp +++ b/Point_set_processing_3/examples/Point_set_processing_3/registration_with_pointmatcher.cpp @@ -28,7 +28,7 @@ int main(int argc, const char** argv) const char* fname2 = (argc>2)?argv[2]:"data/hippo2.ply"; std::vector pwns1, pwns2; - std::ifstream input(fname1); + std::ifstream input(fname1, std::ios::binary); if (!input || !CGAL::read_ply_points(input, std::back_inserter(pwns1), CGAL::parameters::point_map (CGAL::First_of_pair_property_map()). @@ -39,7 +39,7 @@ int main(int argc, const char** argv) } input.close(); - input.open(fname2); + input.open(fname2, std::ios::binary); if (!input || !CGAL::read_ply_points(input, std::back_inserter(pwns2), CGAL::parameters::point_map (Point_map()). From 99a31e07fe31d26133eea6d1bcbd2aac5f24a122 Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Fri, 15 Jan 2021 14:03:12 +0100 Subject: [PATCH 08/38] Move the Boost components in the support file and condition it to windows. --- .../doc/Documentation/Third_party.txt | 4 ++++ .../modules/CGAL_pointmatcher_support.cmake | 21 +++++++++++++++---- .../Point_set_processing_3/CMakeLists.txt | 1 - 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/Documentation/doc/Documentation/Third_party.txt b/Documentation/doc/Documentation/Third_party.txt index 6019c3057ae..02bffde6a6b 100644 --- a/Documentation/doc/Documentation/Third_party.txt +++ b/Documentation/doc/Documentation/Third_party.txt @@ -154,6 +154,10 @@ executables should be linked with the CMake imported target The \sc{libpointmatcher} web site is `https://github.com/ethz-asl/libpointmatcher`. +\attention On Windows, we only test the following setup : PointMatcher 1.3.1 with Eigen 3.3.7. Also, to make it work, you should follow the instructions at +`https://github.com/ethz-asl/libpointmatcher/blob/master/doc/CompilationWindows.md`, but replace `NABO_INCLUDE_DIR` by `libnabo_INCLUDE_DIRS` +and `NABO_LIBRARY` by `libnabo_LIBRARIES` when configuring PointMatcher. + \subsection thirdpartyLeda LEDA Version 6.2 or later diff --git a/Installation/cmake/modules/CGAL_pointmatcher_support.cmake b/Installation/cmake/modules/CGAL_pointmatcher_support.cmake index 520e4d3a2b2..7247f399f18 100644 --- a/Installation/cmake/modules/CGAL_pointmatcher_support.cmake +++ b/Installation/cmake/modules/CGAL_pointmatcher_support.cmake @@ -1,6 +1,19 @@ if(libpointmatcher_FOUND AND NOT TARGET CGAL::pointmatcher_support) - add_library(CGAL::pointmatcher_support INTERFACE IMPORTED) - target_compile_definitions(CGAL::pointmatcher_support INTERFACE "CGAL_LINKED_WITH_POINTMATCHER") - target_include_directories(CGAL::pointmatcher_support INTERFACE "${libpointmatcher_INCLUDE_DIR}") - target_link_libraries(CGAL::pointmatcher_support INTERFACE "${libpointmatcher_LIBRARIES}") + if(WIN32 OR CMAKE_SYSTEM_NAME STREQUAL Windows) + find_package(Boost COMPONENTS thread filesystem system program_options date_time chrono) + endif() + if(NOT (WIN32 OR CMAKE_SYSTEM_NAME STREQUAL Windows) + OR ( Boost_chrono_FOUND + AND Boost_thread_FOUND + AND Boost_filesystem_FOUND + AND Boost_system_FOUND + AND Boost_program_options_FOUND + AND Boost_date_time_FOUND) ) + add_library(CGAL::pointmatcher_support INTERFACE IMPORTED) + target_compile_definitions(CGAL::pointmatcher_support INTERFACE "CGAL_LINKED_WITH_POINTMATCHER") + target_include_directories(CGAL::pointmatcher_support INTERFACE "${libpointmatcher_INCLUDE_DIR}") + target_link_libraries(CGAL::pointmatcher_support INTERFACE "${libpointmatcher_LIBRARIES}") + else() + message(STATUS "NOTICE : the libpointmatcher library requires the following boost components: thread filesystem system program_options date_time chrono.") + endif() endif() diff --git a/Point_set_processing_3/examples/Point_set_processing_3/CMakeLists.txt b/Point_set_processing_3/examples/Point_set_processing_3/CMakeLists.txt index 93dab1bc293..83253368016 100644 --- a/Point_set_processing_3/examples/Point_set_processing_3/CMakeLists.txt +++ b/Point_set_processing_3/examples/Point_set_processing_3/CMakeLists.txt @@ -93,7 +93,6 @@ if ( CGAL_FOUND ) find_package(libpointmatcher QUIET) include(CGAL_pointmatcher_support) if (TARGET CGAL::pointmatcher_support) - find_package(Boost COMPONENTS thread filesystem system program_options date_time chrono REQUIRED) add_executable(registration_with_pointmatcher "registration_with_pointmatcher.cpp") target_link_libraries(registration_with_pointmatcher ${CGAL_libs} CGAL::pointmatcher_support) From d257d999fb60f8cb54b315ee3ac33d36fc0a00db Mon Sep 17 00:00:00 2001 From: Simon Giraudot Date: Mon, 18 Jan 2021 15:45:39 +0100 Subject: [PATCH 09/38] Fix function properties_and_types() --- Point_set_3/include/CGAL/Point_set_3.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Point_set_3/include/CGAL/Point_set_3.h b/Point_set_3/include/CGAL/Point_set_3.h index 7a006c3d10a..0103389b0b6 100644 --- a/Point_set_3/include/CGAL/Point_set_3.h +++ b/Point_set_3/include/CGAL/Point_set_3.h @@ -17,6 +17,7 @@ #include +#include #include @@ -933,15 +934,15 @@ public: /*! \brief Returns a vector of pairs that describe properties and associated types. */ - std::vector > properties_and_types() const + std::vector > properties_and_types() const { std::vector prop = m_base.properties(); prop.erase (prop.begin()); // remove "index" prop.erase (prop.begin()); // remove "point" - std::vector > out; out.reserve (prop.size()); + std::vector > out; out.reserve (prop.size()); for (std::size_t i = 0; i < prop.size(); ++ i) - out.push_back (std::make_pair (prop[i], m_base.get_type(prop[i]))); + out.push_back (std::make_pair (prop[i], std::type_index(m_base.get_type(prop[i])))); return out; } From e962498b3ab5826804774a66a169adf7a1c4a1e3 Mon Sep 17 00:00:00 2001 From: Simon Giraudot Date: Mon, 18 Jan 2021 15:45:51 +0100 Subject: [PATCH 10/38] Add test for properties_and_types() --- Point_set_3/test/Point_set_3/point_set_test.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Point_set_3/test/Point_set_3/point_set_test.cpp b/Point_set_3/test/Point_set_3/point_set_test.cpp index 4accb458dfa..3089af3eca1 100644 --- a/Point_set_3/test/Point_set_3/point_set_test.cpp +++ b/Point_set_3/test/Point_set_3/point_set_test.cpp @@ -106,6 +106,11 @@ int main (int, char**) point_set.add_property_map ("label", 0); point_set.add_property_map ("intensity", 0.0); + auto pnt = point_set.properties_and_types(); + std::cerr << "Properties = " << std::endl; + for (const auto& p : pnt) + std::cerr << " * " << p.first << " with type " << p.second.name() << std::endl; + test (point_set.base().n_properties() == 4, "point set should have 4 properties."); Point p_before = *(point_set.points().begin()); From fbecff56922cae8f445ccc1f3cbac99492fd940b Mon Sep 17 00:00:00 2001 From: Andreas Fabri Date: Wed, 20 Jan 2021 16:06:32 +0000 Subject: [PATCH 11/38] Use CGAL::cpp98::random_shuffle() --- .../Segment_Delaunay_graph_2/include/CGAL/Constraints_loader.h | 3 ++- .../include/CGAL/Constraints_loader.h | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/GraphicsView/demo/Segment_Delaunay_graph_2/include/CGAL/Constraints_loader.h b/GraphicsView/demo/Segment_Delaunay_graph_2/include/CGAL/Constraints_loader.h index 1a3a2f3621f..66899ca4237 100644 --- a/GraphicsView/demo/Segment_Delaunay_graph_2/include/CGAL/Constraints_loader.h +++ b/GraphicsView/demo/Segment_Delaunay_graph_2/include/CGAL/Constraints_loader.h @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -96,7 +97,7 @@ class Constraints_loader { for(Points_iterator it = points.begin(); it != points.end(); ++it) { indices.push_back(it); } - std::random_shuffle(indices.begin(), indices.end()); + CGAL::cpp98::random_shuffle(indices.begin(), indices.end()); CGAL::spatial_sort(indices.begin(), indices.end(), sort_traits); diff --git a/GraphicsView/demo/Segment_Delaunay_graph_Linf_2/include/CGAL/Constraints_loader.h b/GraphicsView/demo/Segment_Delaunay_graph_Linf_2/include/CGAL/Constraints_loader.h index 1a3a2f3621f..66899ca4237 100644 --- a/GraphicsView/demo/Segment_Delaunay_graph_Linf_2/include/CGAL/Constraints_loader.h +++ b/GraphicsView/demo/Segment_Delaunay_graph_Linf_2/include/CGAL/Constraints_loader.h @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -96,7 +97,7 @@ class Constraints_loader { for(Points_iterator it = points.begin(); it != points.end(); ++it) { indices.push_back(it); } - std::random_shuffle(indices.begin(), indices.end()); + CGAL::cpp98::random_shuffle(indices.begin(), indices.end()); CGAL::spatial_sort(indices.begin(), indices.end(), sort_traits); From 631aa064eb8595804b0bdc4b013b6c9097e48492 Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Mon, 25 Jan 2021 15:28:35 +0100 Subject: [PATCH 12/38] Rephrase the doc --- Documentation/doc/Documentation/Third_party.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/doc/Documentation/Third_party.txt b/Documentation/doc/Documentation/Third_party.txt index 02bffde6a6b..f3001865058 100644 --- a/Documentation/doc/Documentation/Third_party.txt +++ b/Documentation/doc/Documentation/Third_party.txt @@ -154,9 +154,9 @@ executables should be linked with the CMake imported target The \sc{libpointmatcher} web site is `https://github.com/ethz-asl/libpointmatcher`. -\attention On Windows, we only test the following setup : PointMatcher 1.3.1 with Eigen 3.3.7. Also, to make it work, you should follow the instructions at -`https://github.com/ethz-asl/libpointmatcher/blob/master/doc/CompilationWindows.md`, but replace `NABO_INCLUDE_DIR` by `libnabo_INCLUDE_DIRS` -and `NABO_LIBRARY` by `libnabo_LIBRARIES` when configuring PointMatcher. +\attention On Windows, we only support version 1.3.1 of PointMatcher with version 3.3.7 of Eigen, with some changes to the recipe at +`https://github.com/ethz-asl/libpointmatcher/blob/master/doc/CompilationWindows.md`:`NABO_INCLUDE_DIR` becomes `libnabo_INCLUDE_DIRS` +and `NABO_LIBRARY` becomes `libnabo_LIBRARIES` in the "Build libpointmatcher" section. \subsection thirdpartyLeda LEDA From 4ed3b0129c3894758ac921cd7baa6468b521950c Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Mon, 25 Jan 2021 15:35:27 +0100 Subject: [PATCH 13/38] make the dependencies to boost general, not just for windows --- .../cmake/modules/CGAL_pointmatcher_support.cmake | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/Installation/cmake/modules/CGAL_pointmatcher_support.cmake b/Installation/cmake/modules/CGAL_pointmatcher_support.cmake index 7247f399f18..7323feae702 100644 --- a/Installation/cmake/modules/CGAL_pointmatcher_support.cmake +++ b/Installation/cmake/modules/CGAL_pointmatcher_support.cmake @@ -1,14 +1,11 @@ if(libpointmatcher_FOUND AND NOT TARGET CGAL::pointmatcher_support) - if(WIN32 OR CMAKE_SYSTEM_NAME STREQUAL Windows) - find_package(Boost COMPONENTS thread filesystem system program_options date_time chrono) - endif() - if(NOT (WIN32 OR CMAKE_SYSTEM_NAME STREQUAL Windows) - OR ( Boost_chrono_FOUND + find_package(Boost COMPONENTS thread filesystem system program_options date_time chrono) + if(Boost_chrono_FOUND AND Boost_thread_FOUND AND Boost_filesystem_FOUND AND Boost_system_FOUND AND Boost_program_options_FOUND - AND Boost_date_time_FOUND) ) + AND Boost_date_time_FOUND) add_library(CGAL::pointmatcher_support INTERFACE IMPORTED) target_compile_definitions(CGAL::pointmatcher_support INTERFACE "CGAL_LINKED_WITH_POINTMATCHER") target_include_directories(CGAL::pointmatcher_support INTERFACE "${libpointmatcher_INCLUDE_DIR}") From ce75d010e93c6cc0ca0ed3997d925b89d7eb161b Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Thu, 28 Jan 2021 14:19:22 +0100 Subject: [PATCH 14/38] First try: use nullptr as a criterion to detect a moved-from object --- Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h b/Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h index 0831c09c656..1fb2611c15d 100644 --- a/Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h +++ b/Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h @@ -114,6 +114,7 @@ public: , random(std::move(other.random)) { hierarchy[0] = this; + other.hierarchy[0] = nullptr; for(int i=1; i:: clear() { - for(int i=0;iclear(); } From b4256accb94cbecef9413b894a87d25589696f98 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Thu, 28 Jan 2021 16:04:27 +0100 Subject: [PATCH 15/38] WIP: Second try Something is off, because the test suite does not pass. --- .../include/CGAL/Triangulation_hierarchy_3.h | 62 ++++++++++--------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h b/Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h index 1fb2611c15d..da7ce4ff8d7 100644 --- a/Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h +++ b/Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h @@ -92,7 +92,26 @@ public: private: + template + constexpr std::array + make_array_of_triangulations(const Geom_traits& traits, std::index_sequence) + { + return {{(static_cast(Is), traits)...}}; + } + template + constexpr std::array create_array_of_triangulation(const Geom_traits& traits) + { + return make_array_of_triangulations(traits, std::make_index_sequence()); + } + + void init_hierarchy() { + hierarchy[0] = this; + for(int i=1; i hierarchy_triangulations; std::array hierarchy; boost::rand48 random; @@ -111,25 +130,20 @@ public: Triangulation_hierarchy_3(Triangulation_hierarchy_3&& other) noexcept( noexcept(Tr_Base(std::move(other))) ) : Tr_Base(std::move(other)) + , hierarchy_triangulations(std::move(other.hierarchy_triangulations)) , random(std::move(other.random)) { - hierarchy[0] = this; - other.hierarchy[0] = nullptr; - for(int i=1; i Triangulation_hierarchy_3(InputIterator first, InputIterator last, const Geom_traits& traits = Geom_traits()) : Tr_Base(traits) + , hierarchy_triangulations(create_array_of_triangulation(traits)) { - hierarchy[0] = this; - for(int i=1; i(*this) = std::move(other); - hierarchy[0] = this; - for(int i=1; i Triangulation_hierarchy_3:: Triangulation_hierarchy_3(const Geom_traits& traits) : Tr_Base(traits) + , hierarchy_triangulations(create_array_of_triangulation(traits)) { - hierarchy[0] = this; - for(int i=1;i Triangulation_hierarchy_3:: Triangulation_hierarchy_3(const Triangulation_hierarchy_3 &tr) : Tr_Base(tr) + , hierarchy_triangulations(tr.hierarchy_triangulations) { - hierarchy[0] = this; - for(int i=1; i:: clear() { - if(hierarchy[0] == nullptr) return; // moved-from object - for(int i=1;iclear(); } From 0199c5794c3efc1acd73b28fec5df003a7ff4a82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Mon, 1 Feb 2021 09:36:30 +0100 Subject: [PATCH 16/38] remove doc from concept that applies only to current models --- Kernel_23/doc/Kernel_23/Concepts/FunctionObjectConcepts.h | 8 -------- 1 file changed, 8 deletions(-) diff --git a/Kernel_23/doc/Kernel_23/Concepts/FunctionObjectConcepts.h b/Kernel_23/doc/Kernel_23/Concepts/FunctionObjectConcepts.h index 34c54c85e42..de17fd5beb0 100644 --- a/Kernel_23/doc/Kernel_23/Concepts/FunctionObjectConcepts.h +++ b/Kernel_23/doc/Kernel_23/Concepts/FunctionObjectConcepts.h @@ -3848,10 +3848,6 @@ public: If `l1` and `l2` are parallel, then the bisector is defined as the line which has the same direction as `l1`, and which is at the same distance from `l1` and `l2`. - If `Kernel::FT` is not a model of `FieldWithSqrt` - an approximation of the square root will be used in this function, - impacting the exactness of the result even with an (exact) multiprecision - number type. */ Kernel::Line_2 operator()(const Kernel::Line_2&l1, const Kernel::Line_2&l2); @@ -3893,10 +3889,6 @@ public: If `h1` and `h2` are parallel, then the bisector is defined as the plane which has the same oriented normal vector as `h1`, and which is at the same distance from `h1` and `h2`. - If `Kernel::FT` is not a model of `FieldWithSqrt` - an approximation of the square root will be used in this function, - impacting the exactness of the result even with an (exact) multiprecision - number type. */ Kernel::Plane_3 operator()(const Kernel::Plane_3&h1, const Kernel::Plane_3&h2); From d4b6a55a688a3ca79e602378a17496cd9b1dd979 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Mon, 1 Feb 2021 09:36:47 +0100 Subject: [PATCH 17/38] remove unused typedefs --- Kernel_23/test/Kernel_23/include/CGAL/_test_fct_line_2.h | 1 - Kernel_23/test/Kernel_23/include/CGAL/_test_fct_plane_3.h | 1 - 2 files changed, 2 deletions(-) diff --git a/Kernel_23/test/Kernel_23/include/CGAL/_test_fct_line_2.h b/Kernel_23/test/Kernel_23/include/CGAL/_test_fct_line_2.h index 28ec8a3d1ea..05df87c02f1 100644 --- a/Kernel_23/test/Kernel_23/include/CGAL/_test_fct_line_2.h +++ b/Kernel_23/test/Kernel_23/include/CGAL/_test_fct_line_2.h @@ -53,7 +53,6 @@ _test_fct_line_2(const R& ) std::cout << "Testing functions Line_2" ; typedef typename R::RT RT; - typedef typename R::FT FT; typedef typename R::Point_2 Point_2; typedef typename R::Line_2 Line_2; diff --git a/Kernel_23/test/Kernel_23/include/CGAL/_test_fct_plane_3.h b/Kernel_23/test/Kernel_23/include/CGAL/_test_fct_plane_3.h index a5502ad4c3b..17578d544ce 100644 --- a/Kernel_23/test/Kernel_23/include/CGAL/_test_fct_plane_3.h +++ b/Kernel_23/test/Kernel_23/include/CGAL/_test_fct_plane_3.h @@ -53,7 +53,6 @@ _test_fct_plane_3(const R& ) std::cout << "Testing functions Plane_3" ; typedef typename R::RT RT; - typedef typename R::FT FT; typedef typename R::Point_3 Point_3; typedef typename R::Plane_3 Plane_3; From f218fb8d3a64cb3c34340fe2b7de4467ca8a5e6a Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Mon, 1 Feb 2021 14:46:37 +0100 Subject: [PATCH 18/38] Fix the segfault in the previous commit (WIP: Second try) --- Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h b/Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h index da7ce4ff8d7..ad0f1e61fef 100644 --- a/Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h +++ b/Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h @@ -167,8 +167,8 @@ public: void swap(Triangulation_hierarchy_3 &tr) { Tr_Base::swap(tr); - for(int i=1; i Date: Mon, 1 Feb 2021 14:47:18 +0100 Subject: [PATCH 19/38] Add tests for move constructors --- .../include/CGAL/_test_cls_delaunay_3.h | 15 ++++++++++++++- .../include/CGAL/_test_cls_regular_3.h | 14 ++++++++++++-- .../include/CGAL/_test_cls_triangulation_3.h | 15 ++++++++++++++- 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h index f7d4579828c..471e3791fd4 100644 --- a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h +++ b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h @@ -421,7 +421,20 @@ _test_cls_delaunay_3(const Triangulation &) assert(T1.number_of_vertices() == 0); assert(T1.is_valid()); - + // move constructor + { + Cls T_copy(T0); + assert(T_copy.dimension() == 3); + assert(T_copy.number_of_vertices() == 4); + assert(T_copy.is_valid()); + Cls T_move_constructed(std::move(T_copy)); + assert(T_move_constructed.dimension() == 3); + assert(T_move_constructed.number_of_vertices() == 4); + assert(T_move_constructed.is_valid()); + assert(T_copy.dimension() == -2); + assert(T_copy.number_of_vertices() == -1); + assert(T_copy.is_valid()); + } // Affectation : T1=T0; diff --git a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h index fba2db023fd..1da16d061c7 100644 --- a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h +++ b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h @@ -205,6 +205,16 @@ _test_cls_regular_3(const Triangulation &) << T.number_of_vertices() << std::endl; assert(T.is_valid()); assert(T.dimension()==3); + + // move constructor + { + Triangulation T_copy(T); + assert(T_copy == T); + assert(T_copy == T); + Triangulation T_move_constructed(std::move(T_copy)); + assert(T_move_constructed == T); + assert(T_copy.dimension() == -2); + assert(T_copy.number_of_vertices() == -1); + assert(T_copy.is_valid()); + } } - - diff --git a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h index ded18a9175f..ca8b84ac193 100644 --- a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h +++ b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h @@ -286,7 +286,20 @@ _test_cls_triangulation_3(const Triangulation &) assert(T1.number_of_vertices() == 0); assert(T1.is_valid()); - + // move constructor + { + Cls T_copy(T0); + assert(T_copy.dimension() == 3); + assert(T_copy.number_of_vertices() == 4); + assert(T_copy.is_valid()); + Cls T_move_constructed(std::move(T_copy)); + assert(T_move_constructed.dimension() == 3); + assert(T_move_constructed.number_of_vertices() == 4); + assert(T_move_constructed.is_valid()); + assert(T_copy.dimension() == -2); + assert(T_copy.number_of_vertices() == -1); + assert(T_copy.is_valid()); + } // Assignment T1=T0; From 0e0c536c025dcd91977c2ee0cb64a1d4fdc0361c Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Mon, 1 Feb 2021 20:42:37 +0100 Subject: [PATCH 20/38] Do not test is_valid(): a moved-from triangulation is not valid A moved-from triangulation do not have the infinite vertex, and it is of dimension -2. That is a valid TDS, but not a valid CGAL triangulation. --- .../test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h | 3 +-- .../test/Triangulation_3/include/CGAL/_test_cls_regular_3.h | 3 +-- .../Triangulation_3/include/CGAL/_test_cls_triangulation_3.h | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h index 471e3791fd4..9f9bea50e27 100644 --- a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h +++ b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h @@ -432,8 +432,7 @@ _test_cls_delaunay_3(const Triangulation &) assert(T_move_constructed.number_of_vertices() == 4); assert(T_move_constructed.is_valid()); assert(T_copy.dimension() == -2); - assert(T_copy.number_of_vertices() == -1); - assert(T_copy.is_valid()); + assert(T_copy.number_of_vertices() + 1 == 0); } // Affectation : diff --git a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h index 1da16d061c7..f7f195ee29d 100644 --- a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h +++ b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h @@ -214,7 +214,6 @@ _test_cls_regular_3(const Triangulation &) Triangulation T_move_constructed(std::move(T_copy)); assert(T_move_constructed == T); assert(T_copy.dimension() == -2); - assert(T_copy.number_of_vertices() == -1); - assert(T_copy.is_valid()); + assert(T_copy.number_of_vertices() + 1 == 0); } } diff --git a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h index ca8b84ac193..1737aff8a4a 100644 --- a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h +++ b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h @@ -297,8 +297,7 @@ _test_cls_triangulation_3(const Triangulation &) assert(T_move_constructed.number_of_vertices() == 4); assert(T_move_constructed.is_valid()); assert(T_copy.dimension() == -2); - assert(T_copy.number_of_vertices() == -1); - assert(T_copy.is_valid()); + assert(T_copy.number_of_vertices() + 1 == 0); } // Assignment From 3b0cea9a47a81d8d1dc9637fa350a0426756c34f Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Mon, 1 Feb 2021 20:43:42 +0100 Subject: [PATCH 21/38] Check that clear() can be called on a moved-from triangulation --- .../test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h | 5 +++++ .../test/Triangulation_3/include/CGAL/_test_cls_regular_3.h | 5 +++++ .../Triangulation_3/include/CGAL/_test_cls_triangulation_3.h | 5 +++++ 3 files changed, 15 insertions(+) diff --git a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h index 9f9bea50e27..db27d607d10 100644 --- a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h +++ b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h @@ -433,6 +433,11 @@ _test_cls_delaunay_3(const Triangulation &) assert(T_move_constructed.is_valid()); assert(T_copy.dimension() == -2); assert(T_copy.number_of_vertices() + 1 == 0); + + Cls T_copy2(T0); + Cls T_move_constructed2(std::move(T_copy2)); + T_copy2.clear(); + assert(T_copy2 == Cls()); } // Affectation : diff --git a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h index f7f195ee29d..040e243e673 100644 --- a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h +++ b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h @@ -215,5 +215,10 @@ _test_cls_regular_3(const Triangulation &) assert(T_move_constructed == T); assert(T_copy.dimension() == -2); assert(T_copy.number_of_vertices() + 1 == 0); + + Cls T_copy2(T); + Cls T_move_constructed2(std::move(T_copy2)); + T_copy2.clear(); + assert(T_copy2 == Cls()); } } diff --git a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h index 1737aff8a4a..d24115cc7bc 100644 --- a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h +++ b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h @@ -298,6 +298,11 @@ _test_cls_triangulation_3(const Triangulation &) assert(T_move_constructed.is_valid()); assert(T_copy.dimension() == -2); assert(T_copy.number_of_vertices() + 1 == 0); + + Cls T_copy2(T0); + Cls T_move_constructed2(std::move(T_copy2)); + T_copy2.clear(); + assert(T_copy2 == Cls()); } // Assignment From fe99ad3a2fd310f50163522c2fb97e9708c9322f Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Mon, 1 Feb 2021 21:06:47 +0100 Subject: [PATCH 22/38] Check that a moved-from triangulation can be assigned --- .../test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h | 5 +++++ .../test/Triangulation_3/include/CGAL/_test_cls_regular_3.h | 5 +++++ .../Triangulation_3/include/CGAL/_test_cls_triangulation_3.h | 5 +++++ 3 files changed, 15 insertions(+) diff --git a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h index db27d607d10..0e5ded48030 100644 --- a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h +++ b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h @@ -438,6 +438,11 @@ _test_cls_delaunay_3(const Triangulation &) Cls T_move_constructed2(std::move(T_copy2)); T_copy2.clear(); assert(T_copy2 == Cls()); + + Cls T_copy3(T0); + Cls T_move_constructed3(std::move(T_copy3)); + T_copy3 = T0; + assert(T_copy3 == T0); } // Affectation : diff --git a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h index 040e243e673..2100ec9d03b 100644 --- a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h +++ b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h @@ -220,5 +220,10 @@ _test_cls_regular_3(const Triangulation &) Cls T_move_constructed2(std::move(T_copy2)); T_copy2.clear(); assert(T_copy2 == Cls()); + + Cls T_copy3(T); + Cls T_move_constructed3(std::move(T_copy3)); + T_copy3 = T; + assert(T_copy3 == T); } } diff --git a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h index d24115cc7bc..fcda1f921b0 100644 --- a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h +++ b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h @@ -303,6 +303,11 @@ _test_cls_triangulation_3(const Triangulation &) Cls T_move_constructed2(std::move(T_copy2)); T_copy2.clear(); assert(T_copy2 == Cls()); + + Cls T_copy3(T0); + Cls T_move_constructed3(std::move(T_copy3)); + T_copy3 = T0; + assert(T_copy3 == T0); } // Assignment From de8bf2fd87ed41346cc468c8204c734e005f2d44 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Mon, 1 Feb 2021 23:17:21 +0100 Subject: [PATCH 23/38] Fix Triangulation_hierarchy_3::operator=(Triangulation_hierarchy_3&&) --- Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h b/Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h index ad0f1e61fef..e55b8eab5b3 100644 --- a/Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h +++ b/Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h @@ -157,8 +157,7 @@ public: noexcept( noexcept(Triangulation_hierarchy_3(std::move(other))) ) { static_cast(*this) = std::move(other); - hierarchy_triangulations = std::move(hierarchy_triangulations); - init_hierarchy(); + hierarchy_triangulations = std::move(other.hierarchy_triangulations); return *this; } From f14ab371ae320732c1aad9b02436767fb4f4987a Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Mon, 1 Feb 2021 23:17:46 +0100 Subject: [PATCH 24/38] Test move-assignments --- .../include/CGAL/_test_cls_delaunay_3.h | 12 ++++++++++++ .../include/CGAL/_test_cls_regular_3.h | 11 +++++++++++ .../include/CGAL/_test_cls_triangulation_3.h | 12 ++++++++++++ 3 files changed, 35 insertions(+) diff --git a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h index 0e5ded48030..1dfd828dd04 100644 --- a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h +++ b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h @@ -444,6 +444,18 @@ _test_cls_delaunay_3(const Triangulation &) T_copy3 = T0; assert(T_copy3 == T0); } + // move-assignment + { + Cls T_copy4(T0); + Cls T_move_assigned; + T_move_assigned = std::move(T_copy4); + assert(T_copy4.dimension() == -2); + assert(T_copy4.number_of_vertices() + 1 == 0); + assert(T_move_assigned.dimension() == 3); + assert(T_move_assigned.number_of_vertices() == 4); + assert(T_move_assigned.is_valid()); + assert(T_move_assigned == T0); + } // Affectation : T1=T0; diff --git a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h index 2100ec9d03b..43d397d14fe 100644 --- a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h +++ b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h @@ -226,4 +226,15 @@ _test_cls_regular_3(const Triangulation &) T_copy3 = T; assert(T_copy3 == T); } + // move-assignment + { + Cls T_copy4(T); + Cls T_move_assigned; + T_move_assigned = std::move(T_copy4); + assert(T_copy4.dimension() == -2); + assert(T_copy4.number_of_vertices() + 1 == 0); + assert(T_move_assigned.dimension() == 3); + assert(T_move_assigned.is_valid()); + assert(T_move_assigned == T); + } } diff --git a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h index fcda1f921b0..c903a39ed16 100644 --- a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h +++ b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h @@ -309,6 +309,18 @@ _test_cls_triangulation_3(const Triangulation &) T_copy3 = T0; assert(T_copy3 == T0); } + // move-assignment + { + Cls T_copy4(T0); + Cls T_move_assigned; + T_move_assigned = std::move(T_copy4); + assert(T_copy4.dimension() == -2); + assert(T_copy4.number_of_vertices() + 1 == 0); + assert(T_move_assigned.dimension() == 3); + assert(T_move_assigned.number_of_vertices() == 4); + assert(T_move_assigned.is_valid()); + assert(T_move_assigned == T0); + } // Assignment T1=T0; From e6fe1c203191fe76c10acb6fda8dd881d880d1e9 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Tue, 2 Feb 2021 20:52:49 +0100 Subject: [PATCH 25/38] Add the testcase from issue #5396 --- .../include/CGAL/_test_cls_delaunay_3.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h index 1dfd828dd04..d8cf1d38e7e 100644 --- a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h +++ b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h @@ -1228,6 +1228,18 @@ _test_cls_delaunay_3(const Triangulation &) _test_remove_cluster(); } + // Test from issue https://github.com/CGAL/cgal/issues/5396 + { + auto Triangulate = []() -> Triangulation + { + Triangulation tri; + for (int i=0; i<10; i++) + tri.insert(Point(i+1, i+2, i+3)); + + return std::move(tri); // the move prevents the NRVO + }; + Triangulation t = Triangulate(); + } } #endif // CGAL_TEST_CLS_DELAUNAY_C From cbc73a8fc4abf1a80cbf3961f3f3791d61cb1f5a Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Wed, 3 Feb 2021 12:47:33 +0100 Subject: [PATCH 26/38] Add the testsuite for Triangulation_2, factorize it with T_3 I have added the file `Testsuite/include/CGAL/Testsuite/Triangulation_23/test_move_semantic.h` so that is can be reused in the tests for `Triangulation_2` and `Triangulation_3`. So far, the tests in `test_delaunay_hierarchy_2.cpp` fail... --- .../Triangulation_23/test_move_semantic.h | 69 +++++++++++++++++++ .../CGAL/_test_cls_triangulation_short_2.h | 10 +++ .../include/CGAL/_test_cls_delaunay_3.h | 43 +++--------- .../include/CGAL/_test_cls_regular_3.h | 35 ++-------- .../include/CGAL/_test_cls_triangulation_3.h | 44 +++--------- 5 files changed, 100 insertions(+), 101 deletions(-) create mode 100644 Testsuite/include/CGAL/Testsuite/Triangulation_23/test_move_semantic.h diff --git a/Testsuite/include/CGAL/Testsuite/Triangulation_23/test_move_semantic.h b/Testsuite/include/CGAL/Testsuite/Triangulation_23/test_move_semantic.h new file mode 100644 index 00000000000..3254fb6571f --- /dev/null +++ b/Testsuite/include/CGAL/Testsuite/Triangulation_23/test_move_semantic.h @@ -0,0 +1,69 @@ +// Copyright (c) 2021 GeometryFactory Sarl (France). +// All rights reserved. +// +// This file is part of CGAL (www.cgal.org). +// +// $URL$ +// $Id$ +// SPDX-License-Identifier: LGPL-3.0-or-later OR LicenseRef-Commercial +// +// +// Author(s) : Laurent Rineau +// + + +#include + +namespace CGAL { + namespace Testsuite { + namespace Triangulation_23 { + template + void test_move_semantic(Tr source_tr) { + const auto dimension = source_tr.dimension(); + const auto nb_of_vertices = source_tr.number_of_vertices(); + auto check_triangulation_validity = [&](const Tr& tr) { + assert(tr.is_valid()); + assert(tr.number_of_vertices() == nb_of_vertices); + assert(tr.dimension() == dimension); + }; + auto check_moved_from_triangulation = [](const Tr& tr_copy) { + assert(tr_copy.dimension() == -2); + assert(tr_copy.number_of_vertices() + 1 == 0); + }; + auto check_empty_triangulation = [](const Tr& tr_copy2) { + assert(tr_copy2.dimension() == -1); + assert(tr_copy2.number_of_vertices() == 0); + }; + // move constructor + { + Tr tr_copy(source_tr); + check_triangulation_validity(tr_copy); + + Tr tr_move_constructed(std::move(tr_copy)); + check_triangulation_validity(tr_move_constructed); + check_moved_from_triangulation(tr_copy); + + Tr tr_copy2(source_tr); + Tr tr_move_constructed2(std::move(tr_copy2)); + check_moved_from_triangulation(tr_copy2); + tr_copy2.clear(); + check_empty_triangulation(tr_copy2); + + Tr tr_copy3(source_tr); + Tr tr_move_constructed3(std::move(tr_copy3)); + check_moved_from_triangulation(tr_copy3); + tr_copy3 = source_tr; + check_triangulation_validity(tr_copy3); + } + // move-assignment + { + Tr tr_copy4(source_tr); + Tr tr_move_assigned; + tr_move_assigned = std::move(tr_copy4); + check_triangulation_validity(tr_move_assigned); + check_moved_from_triangulation(tr_copy4); + } + }; + } + } +} diff --git a/Triangulation_2/test/Triangulation_2/include/CGAL/_test_cls_triangulation_short_2.h b/Triangulation_2/test/Triangulation_2/include/CGAL/_test_cls_triangulation_short_2.h index d34268631cf..249b1d9e115 100644 --- a/Triangulation_2/test/Triangulation_2/include/CGAL/_test_cls_triangulation_short_2.h +++ b/Triangulation_2/test/Triangulation_2/include/CGAL/_test_cls_triangulation_short_2.h @@ -31,6 +31,7 @@ #include #include #include +#include template @@ -281,6 +282,15 @@ _test_cls_triangulation_short_2( const Triangul &) assert( T2_3_4.number_of_vertices() == 11 ); assert( T2_3_4.is_valid() ); + /****************************/ + /******* MOVE SEMANTIC*******/ + + std::cout << " move constructors and move assignment" << std::endl; + namespace test_tr_23 = CGAL::Testsuite::Triangulation_23; + test_tr_23::test_move_semantic(T0_1); + test_tr_23::test_move_semantic(T1_5); + test_tr_23::test_move_semantic(T2_8); + test_tr_23::test_move_semantic(T2_3); /*********************************************/ /****** FINITE/INFINITE VERTICES/FACES *******/ diff --git a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h index d8cf1d38e7e..307db51472a 100644 --- a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h +++ b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h @@ -31,6 +31,7 @@ #include #include #include +#include // Accessory set of functions to differentiate between // Delaunay::nearest_vertex[_in_cell] and @@ -421,41 +422,8 @@ _test_cls_delaunay_3(const Triangulation &) assert(T1.number_of_vertices() == 0); assert(T1.is_valid()); - // move constructor - { - Cls T_copy(T0); - assert(T_copy.dimension() == 3); - assert(T_copy.number_of_vertices() == 4); - assert(T_copy.is_valid()); - Cls T_move_constructed(std::move(T_copy)); - assert(T_move_constructed.dimension() == 3); - assert(T_move_constructed.number_of_vertices() == 4); - assert(T_move_constructed.is_valid()); - assert(T_copy.dimension() == -2); - assert(T_copy.number_of_vertices() + 1 == 0); - - Cls T_copy2(T0); - Cls T_move_constructed2(std::move(T_copy2)); - T_copy2.clear(); - assert(T_copy2 == Cls()); - - Cls T_copy3(T0); - Cls T_move_constructed3(std::move(T_copy3)); - T_copy3 = T0; - assert(T_copy3 == T0); - } - // move-assignment - { - Cls T_copy4(T0); - Cls T_move_assigned; - T_move_assigned = std::move(T_copy4); - assert(T_copy4.dimension() == -2); - assert(T_copy4.number_of_vertices() + 1 == 0); - assert(T_move_assigned.dimension() == 3); - assert(T_move_assigned.number_of_vertices() == 4); - assert(T_move_assigned.is_valid()); - assert(T_move_assigned == T0); - } + namespace test_tr_23 = CGAL::Testsuite::Triangulation_23; + test_tr_23::test_move_semantic(T0); // Affectation : T1=T0; @@ -488,6 +456,7 @@ _test_cls_delaunay_3(const Triangulation &) assert(T1_0.dimension()==1); assert(T1_0.number_of_vertices()==n); assert(T1_0.is_valid()); + test_tr_23::test_move_semantic(T1_0); std::cout << " Constructor7 " << std::endl; Cls T1_1; n = T1_1.insert(l2.begin(),l2.end()); @@ -548,6 +517,8 @@ _test_cls_delaunay_3(const Triangulation &) assert(T2_0.dimension()==2); assert(T2_0.number_of_vertices()==8); + test_tr_23::test_move_semantic(T2_0); + { Cls Tfromfile; std::cout << " I/O" << std::endl; @@ -596,6 +567,8 @@ _test_cls_delaunay_3(const Triangulation &) assert(T3_0.number_of_vertices()==125); assert(T3_0.dimension()==3); + test_tr_23::test_move_semantic(T3_0); + if (del) { std::cout << " deletion in Delaunay - grid case - (dim 3) " << std::endl; diff --git a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h index 43d397d14fe..ea40d75963e 100644 --- a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h +++ b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_regular_3.h @@ -17,6 +17,8 @@ #include #include #include +#include + template void _test_cls_regular_3(const Triangulation &) @@ -206,35 +208,6 @@ _test_cls_regular_3(const Triangulation &) assert(T.is_valid()); assert(T.dimension()==3); - // move constructor - { - Triangulation T_copy(T); - assert(T_copy == T); - assert(T_copy == T); - Triangulation T_move_constructed(std::move(T_copy)); - assert(T_move_constructed == T); - assert(T_copy.dimension() == -2); - assert(T_copy.number_of_vertices() + 1 == 0); - - Cls T_copy2(T); - Cls T_move_constructed2(std::move(T_copy2)); - T_copy2.clear(); - assert(T_copy2 == Cls()); - - Cls T_copy3(T); - Cls T_move_constructed3(std::move(T_copy3)); - T_copy3 = T; - assert(T_copy3 == T); - } - // move-assignment - { - Cls T_copy4(T); - Cls T_move_assigned; - T_move_assigned = std::move(T_copy4); - assert(T_copy4.dimension() == -2); - assert(T_copy4.number_of_vertices() + 1 == 0); - assert(T_move_assigned.dimension() == 3); - assert(T_move_assigned.is_valid()); - assert(T_move_assigned == T); - } + namespace test_tr_23 = CGAL::Testsuite::Triangulation_23; + test_tr_23::test_move_semantic(T); } diff --git a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h index c903a39ed16..a2afe749131 100644 --- a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h +++ b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_triangulation_3.h @@ -23,6 +23,7 @@ #include #include #include +#include template bool check_all_are_finite(Triangulation* tr, const Container& cont) @@ -286,41 +287,8 @@ _test_cls_triangulation_3(const Triangulation &) assert(T1.number_of_vertices() == 0); assert(T1.is_valid()); - // move constructor - { - Cls T_copy(T0); - assert(T_copy.dimension() == 3); - assert(T_copy.number_of_vertices() == 4); - assert(T_copy.is_valid()); - Cls T_move_constructed(std::move(T_copy)); - assert(T_move_constructed.dimension() == 3); - assert(T_move_constructed.number_of_vertices() == 4); - assert(T_move_constructed.is_valid()); - assert(T_copy.dimension() == -2); - assert(T_copy.number_of_vertices() + 1 == 0); - - Cls T_copy2(T0); - Cls T_move_constructed2(std::move(T_copy2)); - T_copy2.clear(); - assert(T_copy2 == Cls()); - - Cls T_copy3(T0); - Cls T_move_constructed3(std::move(T_copy3)); - T_copy3 = T0; - assert(T_copy3 == T0); - } - // move-assignment - { - Cls T_copy4(T0); - Cls T_move_assigned; - T_move_assigned = std::move(T_copy4); - assert(T_copy4.dimension() == -2); - assert(T_copy4.number_of_vertices() + 1 == 0); - assert(T_move_assigned.dimension() == 3); - assert(T_move_assigned.number_of_vertices() == 4); - assert(T_move_assigned.is_valid()); - assert(T_move_assigned == T0); - } + namespace test_tr_23 = CGAL::Testsuite::Triangulation_23; + test_tr_23::test_move_semantic(T0); // Assignment T1=T0; @@ -397,12 +365,14 @@ _test_cls_triangulation_3(const Triangulation &) assert(T2_0.dimension()==1); assert(T2_0.number_of_vertices()==3); + test_tr_23::test_move_semantic(T2_0); v0=T2_0.insert(p4); assert(T2_0.is_valid()); assert(T2_0.dimension()==2); assert(T2_0.number_of_vertices()==4); + test_tr_23::test_move_semantic(T2_0); v0=T2_0.insert(p5); v0=T2_0.insert(p6); @@ -414,6 +384,8 @@ _test_cls_triangulation_3(const Triangulation &) assert(T2_0.dimension()==2); assert(T2_0.number_of_vertices()==8); + test_tr_23::test_move_semantic(T2_0); + if (! del) // to avoid doing the following tests for both Delaunay // and non Delaunay triangulations { @@ -437,6 +409,8 @@ _test_cls_triangulation_3(const Triangulation &) assert( T2_1.dimension()==2 ); assert( T2_1.is_valid() ); + test_tr_23::test_move_semantic(T2_1); + std::cout << " Constructor11 " << std::endl; // 3-dimensional triangulations // This is a simple grid : From 8dbf50a94b0f2065521617c94403f2c6455f2584 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Wed, 3 Feb 2021 21:50:10 +0100 Subject: [PATCH 27/38] Create CGAL::make_filled_array That function template will replace my adhoc `create_array_of_triangulation`. --- STL_Extension/include/CGAL/array.h | 14 ++++++++++++++ .../include/CGAL/Triangulation_hierarchy_3.h | 17 +++-------------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/STL_Extension/include/CGAL/array.h b/STL_Extension/include/CGAL/array.h index 1900a8b1ba0..2f2cf3de44b 100644 --- a/STL_Extension/include/CGAL/array.h +++ b/STL_Extension/include/CGAL/array.h @@ -18,6 +18,7 @@ #else # include #endif +#include namespace CGAL { @@ -68,6 +69,19 @@ struct Construct_array } }; +template +constexpr std::array +make_filled_array_aux(const T& value, std::index_sequence) +{ + return {(static_cast(Is), value)...}; +} + +template +constexpr std::array make_filled_array(const T& value) +{ + return make_filled_array_aux(value, std::make_index_sequence()); +} + } //namespace CGAL #endif // CGAL_ARRAY_H diff --git a/Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h b/Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h index e55b8eab5b3..6fc656ca9eb 100644 --- a/Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h +++ b/Triangulation_3/include/CGAL/Triangulation_hierarchy_3.h @@ -51,6 +51,7 @@ #include #include +#include #endif //CGAL_TRIANGULATION_3_DONT_INSERT_RANGE_OF_POINTS_WITH_INFO @@ -92,18 +93,6 @@ public: private: - template - constexpr std::array - make_array_of_triangulations(const Geom_traits& traits, std::index_sequence) - { - return {{(static_cast(Is), traits)...}}; - } - template - constexpr std::array create_array_of_triangulation(const Geom_traits& traits) - { - return make_array_of_triangulations(traits, std::make_index_sequence()); - } - void init_hierarchy() { hierarchy[0] = this; for(int i=1; i(traits)) + , hierarchy_triangulations(make_filled_array(traits)) { init_hierarchy(); insert(first, last); @@ -465,7 +454,7 @@ template Triangulation_hierarchy_3:: Triangulation_hierarchy_3(const Geom_traits& traits) : Tr_Base(traits) - , hierarchy_triangulations(create_array_of_triangulation(traits)) + , hierarchy_triangulations(make_filled_array(traits)) { init_hierarchy(); } From 9799879eefdc538d5d4ea949116bcf1914ae2f35 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Wed, 3 Feb 2021 21:51:00 +0100 Subject: [PATCH 28/38] Modernize a bit assuming C++14 --- STL_Extension/include/CGAL/array.h | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/STL_Extension/include/CGAL/array.h b/STL_Extension/include/CGAL/array.h index 2f2cf3de44b..5a80d1335fe 100644 --- a/STL_Extension/include/CGAL/array.h +++ b/STL_Extension/include/CGAL/array.h @@ -13,11 +13,7 @@ #define CGAL_ARRAY_H #include -#ifndef CGAL_CFG_NO_CPP0X_ARRAY -# include -#else -# include -#endif +#include #include namespace CGAL { @@ -50,8 +46,7 @@ namespace CGAL { // It's also untrue that this is not documented... It is ! template< typename T, typename... Args > -inline -std::array< T, 1 + sizeof...(Args) > +constexpr std::array< T, 1 + sizeof...(Args) > make_array(const T & t, const Args & ... args) { std::array< T, 1 + sizeof...(Args) > a = { { t, static_cast(args)... } }; @@ -63,7 +58,9 @@ make_array(const T & t, const Args & ... args) struct Construct_array { template - std::array operator()(const T& t, const Args& ... args) + constexpr + std::array + operator()(const T& t, const Args& ... args) { return make_array (t, args...); } From b8bb3ffc09ebf6079cb6cf2e0f6afba736e4d8e1 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Thu, 4 Feb 2021 09:21:36 +0100 Subject: [PATCH 29/38] Apply on triangulation_hierarchy_2 the same fix as in 3D --- .../include/CGAL/Triangulation_hierarchy_2.h | 61 +++++++------------ 1 file changed, 21 insertions(+), 40 deletions(-) diff --git a/Triangulation_2/include/CGAL/Triangulation_hierarchy_2.h b/Triangulation_2/include/CGAL/Triangulation_hierarchy_2.h index 266c4964e03..679ace59a24 100644 --- a/Triangulation_2/include/CGAL/Triangulation_hierarchy_2.h +++ b/Triangulation_2/include/CGAL/Triangulation_hierarchy_2.h @@ -37,6 +37,7 @@ #include #include #include +#include namespace CGAL { @@ -82,7 +83,14 @@ public: #endif private: - // here is the stack of triangulations which form the hierarchy + void init_hierarchy() { + hierarchy[0] = this; + for(int i=1; i hierarchy_triangulations; std::array hierarchy; boost::rand48 random; @@ -93,13 +101,10 @@ public: Triangulation_hierarchy_2(Triangulation_hierarchy_2&& other) noexcept( noexcept(Tr_Base(std::move(other))) ) : Tr_Base(std::move(other)) + , hierarchy_triangulations(std::move(other.hierarchy_triangulations)) , random(std::move(other.random)) { - hierarchy[0] = this; - for(int i=1; i @@ -107,10 +112,7 @@ public: const Geom_traits& traits = Geom_traits()) : Tr_Base(traits) { - hierarchy[0] = this; - for(int i=1;i(*this) = std::move(other); - hierarchy[0] = this; - for(int i=1; i Triangulation_hierarchy_2:: Triangulation_hierarchy_2(const Geom_traits& traits) : Tr_Base(traits) + , hierarchy_triangulations( + make_filled_array(traits)) { - hierarchy[0] = this; - for(int i=1;i Triangulation_hierarchy_2:: Triangulation_hierarchy_2(const Triangulation_hierarchy_2 &tr) - : Tr_Base() + : Triangulation_hierarchy_2(tr.geom_traits()) { - // create an empty triangulation to be able to delete it ! - hierarchy[0] = this; - for(int i=1;i:: swap(Triangulation_hierarchy_2 &tr) { - Tr_Base* temp; Tr_Base::swap(tr); - for(int i= 1; i -Triangulation_hierarchy_2:: -~Triangulation_hierarchy_2() -{ - clear(); - for(int i= 1; i From 46cb451bde38d78fc85a4c8b14cef478d6182a46 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Thu, 4 Feb 2021 10:47:52 +0100 Subject: [PATCH 30/38] Force a move-construction in the test case for issue #5396 --- .../test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h index 307db51472a..21178f53eed 100644 --- a/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h +++ b/Triangulation_3/test/Triangulation_3/include/CGAL/_test_cls_delaunay_3.h @@ -1209,9 +1209,10 @@ _test_cls_delaunay_3(const Triangulation &) for (int i=0; i<10; i++) tri.insert(Point(i+1, i+2, i+3)); - return std::move(tri); // the move prevents the NRVO + return tri; }; - Triangulation t = Triangulate(); + auto t = Triangulate(); + auto t2 = std::move(t); } } From 0eae59b8cd650fa995b8c9905abb30e4a2c89625 Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Tue, 9 Feb 2021 09:58:12 +0100 Subject: [PATCH 31/38] Add tests for the demo --- .github/test.sh | 13 +++++++++++++ .github/workflows/demo.yml | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100755 .github/test.sh create mode 100644 .github/workflows/demo.yml diff --git a/.github/test.sh b/.github/test.sh new file mode 100755 index 00000000000..a744f6d5b08 --- /dev/null +++ b/.github/test.sh @@ -0,0 +1,13 @@ +#!/bin/bash +/usr/local/bin/cmake --version +FACTOR=$1 +set -ex +cd Polyhedron/demo +LIST_OF_PLUGINS=$(/usr/local/bin/cmake --build . -t help | egrep 'plugin$' |& cut -d\ -f2) +PLUGINS_ARRAY=(${LIST_OF_PLUGINS}); +NB_OF_PLUGINS=${#PLUGINS_ARRAY[@]} +DEL=$(($NB_OF_PLUGINS / 4)) +mkdir build +cd build +/usr/local/bin/cmake -DCGAL_DIR=$2 ../Polyhedron +make -j2 ${PLUGINS_ARRAY[@]:$(($FACTOR * $DEL)):$((($FACTOR + 1) * $DEL))} diff --git a/.github/workflows/demo.yml b/.github/workflows/demo.yml new file mode 100644 index 00000000000..7d3a31bfd49 --- /dev/null +++ b/.github/workflows/demo.yml @@ -0,0 +1,37 @@ +name: Test Polyhedron Demo + +on: [push, pull_request] + +jobs: + batch_1: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2.0.0 + - name: install dependencies + run: .github/install.sh + - name: run1 + run: ./.github/test.sh 0 ${{ github.workspace }} + batch_2: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2.0.0 + - name: install dependencies + run: .github/install.sh + - name: run2 + run: ./.github/test.sh 1 ${{ github.workspace }} + batch_3: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2.0.0 + - name: install dependencies + run: .github/install.sh + - name: run3 + run: ./.github/test.sh 2 ${{ github.workspace }} + batch_4: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2.0.0 + - name: install dependencies + run: .github/install.sh + - name: run4 + run: ./.github/test.sh 3 ${{ github.workspace }} From 519870c4cbf6d04ee6931f0f8b8680940bd61e33 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Tue, 9 Feb 2021 15:38:08 +0100 Subject: [PATCH 32/38] Add support for MSVC 2015 MSVC 2015 has a partial support for C++14, and in particular for C++14 `constexpr` functions. Since Boost-1.57 (that is the minimal requirement for CGAL since version 5.0), `` has a macro `BOOST_CXX14_CONSTEXPR` that can be either `constexpr` for fully-C++14 compilers, or empty for non-compliant compilers. --- STL_Extension/include/CGAL/array.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/STL_Extension/include/CGAL/array.h b/STL_Extension/include/CGAL/array.h index 5a80d1335fe..1693c6994e8 100644 --- a/STL_Extension/include/CGAL/array.h +++ b/STL_Extension/include/CGAL/array.h @@ -46,7 +46,8 @@ namespace CGAL { // It's also untrue that this is not documented... It is ! template< typename T, typename... Args > -constexpr std::array< T, 1 + sizeof...(Args) > +BOOST_CXX14_CONSTEXPR +std::array< T, 1 + sizeof...(Args) > make_array(const T & t, const Args & ... args) { std::array< T, 1 + sizeof...(Args) > a = { { t, static_cast(args)... } }; From 160118e7e9c3dd07b970b9014c563587199263b8 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Thu, 11 Feb 2021 14:36:26 +0100 Subject: [PATCH 33/38] Fix a warning from MSVC 2015 include\CGAL/array.h(67): warning C4814: 'CGAL::Construct_array::operator ()': in C++14 'constexpr' will not imply 'const'; consider explicitly specifying 'const' It cannot harm. --- STL_Extension/include/CGAL/array.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/STL_Extension/include/CGAL/array.h b/STL_Extension/include/CGAL/array.h index 1693c6994e8..cfaca0a0550 100644 --- a/STL_Extension/include/CGAL/array.h +++ b/STL_Extension/include/CGAL/array.h @@ -61,7 +61,7 @@ struct Construct_array template constexpr std::array - operator()(const T& t, const Args& ... args) + operator()(const T& t, const Args& ... args) const { return make_array (t, args...); } From a74914ecbd0afdc9fccd9175269c3742e0b0cf2a Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Mon, 15 Feb 2021 15:18:02 +0100 Subject: [PATCH 34/38] Fix warning null pointer --- GraphicsView/include/CGAL/Qt/camera_impl.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/GraphicsView/include/CGAL/Qt/camera_impl.h b/GraphicsView/include/CGAL/Qt/camera_impl.h index 6629dd99994..709409f1609 100644 --- a/GraphicsView/include/CGAL/Qt/camera_impl.h +++ b/GraphicsView/include/CGAL/Qt/camera_impl.h @@ -883,10 +883,13 @@ void Camera::interpolateTo(const Frame &fr, qreal duration) { imprecision along the viewing direction. */ CGAL_INLINE_FUNCTION Vec Camera::pointUnderPixel(const QPoint &pixel, bool &found) const { - float depth; + float depth = 2.0; // Qt uses upper corner for its origin while GL uses the lower corner. - dynamic_cast(parent())->glReadPixels(pixel.x(), screenHeight() - 1 - pixel.y(), 1, 1, - GL_DEPTH_COMPONENT, GL_FLOAT, &depth); + if(parent()) + { + dynamic_cast(parent())->glReadPixels(pixel.x(), screenHeight() - 1 - pixel.y(), 1, 1, + GL_DEPTH_COMPONENT, GL_FLOAT, &depth); + } found = depth < 1.0; Vec point(pixel.x(), pixel.y(), depth); point = unprojectedCoordinatesOf(point); From 4a6aa7e0247b9c595513562bf193b864c6717bb9 Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Mon, 15 Feb 2021 15:21:23 +0100 Subject: [PATCH 35/38] use Qt::MiddleButton instead of MidButton(deprecated since 5.6 or earlier) --- GraphicsView/include/CGAL/Qt/qglviewer_impl.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/GraphicsView/include/CGAL/Qt/qglviewer_impl.h b/GraphicsView/include/CGAL/Qt/qglviewer_impl.h index 5e6ab9dae8a..9d77b64eb57 100644 --- a/GraphicsView/include/CGAL/Qt/qglviewer_impl.h +++ b/GraphicsView/include/CGAL/Qt/qglviewer_impl.h @@ -715,7 +715,7 @@ void CGAL::QGLViewer::setDefaultMouseBindings() { (mh == qglviewer::FRAME) ? frameKeyboardModifiers : cameraKeyboardModifiers; setMouseBinding(modifiers, ::Qt::LeftButton, mh, qglviewer::ROTATE); - setMouseBinding(modifiers, ::Qt::MidButton, mh, qglviewer::ZOOM); + setMouseBinding(modifiers, ::Qt::MiddleButton, mh, qglviewer::ZOOM); setMouseBinding(modifiers, ::Qt::RightButton, mh, qglviewer::TRANSLATE); setMouseBinding(::Qt::Key_R, modifiers, ::Qt::LeftButton, mh, qglviewer::SCREEN_ROTATE); @@ -724,7 +724,7 @@ void CGAL::QGLViewer::setDefaultMouseBindings() { } // Z o o m o n r e g i o n - setMouseBinding(::Qt::ShiftModifier, ::Qt::MidButton, qglviewer::CAMERA, qglviewer::ZOOM_ON_REGION); + setMouseBinding(::Qt::ShiftModifier, ::Qt::MiddleButton, qglviewer::CAMERA, qglviewer::ZOOM_ON_REGION); // S e l e c t setMouseBinding(::Qt::ShiftModifier, ::Qt::LeftButton, qglviewer::SELECT); @@ -732,7 +732,7 @@ void CGAL::QGLViewer::setDefaultMouseBindings() { setMouseBinding(::Qt::ShiftModifier, ::Qt::RightButton, qglviewer::RAP_FROM_PIXEL); // D o u b l e c l i c k setMouseBinding(::Qt::NoModifier, ::Qt::LeftButton, qglviewer::ALIGN_CAMERA, true); - setMouseBinding(::Qt::NoModifier, ::Qt::MidButton, qglviewer::SHOW_ENTIRE_SCENE, true); + setMouseBinding(::Qt::NoModifier, ::Qt::MiddleButton, qglviewer::SHOW_ENTIRE_SCENE, true); setMouseBinding(::Qt::NoModifier, ::Qt::RightButton, qglviewer::CENTER_SCENE, true); setMouseBinding(frameKeyboardModifiers, ::Qt::LeftButton, qglviewer::ALIGN_FRAME, true); @@ -1198,7 +1198,7 @@ static QString mouseButtonsString(::Qt::MouseButtons b) { result += CGAL::QGLViewer::tr("Left", "left mouse button"); addAmpersand = true; } - if (b & ::Qt::MidButton) { + if (b & ::Qt::MiddleButton) { if (addAmpersand) result += " & "; result += CGAL::QGLViewer::tr("Middle", "middle mouse button"); @@ -1733,7 +1733,7 @@ Mouse tab. \c ::Qt::AltModifier, \c ::Qt::ShiftModifier, \c ::Qt::MetaModifier). Possibly combined using the \c "|" operator. -\p button is one of the ::Qt::MouseButtons (\c ::Qt::LeftButton, \c ::Qt::MidButton, +\p button is one of the ::Qt::MouseButtons (\c ::Qt::LeftButton, \c ::Qt::MiddleButton, \c ::Qt::RightButton...). \p doubleClick indicates whether or not the user has to double click this button @@ -3008,27 +3008,27 @@ void CGAL::QGLViewer::toggleCameraMode() { camera()->frame()->stopSpinning(); setMouseBinding(modifiers, ::Qt::LeftButton, qglviewer::CAMERA, qglviewer::MOVE_FORWARD); - setMouseBinding(modifiers, ::Qt::MidButton, qglviewer::CAMERA, qglviewer::LOOK_AROUND); + setMouseBinding(modifiers, ::Qt::MiddleButton, qglviewer::CAMERA, qglviewer::LOOK_AROUND); setMouseBinding(modifiers, ::Qt::RightButton, qglviewer::CAMERA, qglviewer::MOVE_BACKWARD); setMouseBinding(::Qt::Key_R, modifiers, ::Qt::LeftButton, qglviewer::CAMERA, qglviewer::ROLL); setMouseBinding(::Qt::NoModifier, ::Qt::LeftButton, qglviewer::NO_CLICK_ACTION, true); - setMouseBinding(::Qt::NoModifier, ::Qt::MidButton, qglviewer::NO_CLICK_ACTION, true); + setMouseBinding(::Qt::NoModifier, ::Qt::MiddleButton, qglviewer::NO_CLICK_ACTION, true); setMouseBinding(::Qt::NoModifier, ::Qt::RightButton, qglviewer::NO_CLICK_ACTION, true); setWheelBinding(modifiers, qglviewer::CAMERA, qglviewer::MOVE_FORWARD); } else { // Should stop flyTimer. But unlikely and not easy. setMouseBinding(modifiers, ::Qt::LeftButton, qglviewer::CAMERA, qglviewer::ROTATE); - setMouseBinding(modifiers, ::Qt::MidButton, qglviewer::CAMERA, qglviewer::ZOOM); + setMouseBinding(modifiers, ::Qt::MiddleButton, qglviewer::CAMERA, qglviewer::ZOOM); setMouseBinding(modifiers, ::Qt::RightButton, qglviewer::CAMERA, qglviewer::TRANSLATE); setMouseBinding(::Qt::Key_R, modifiers, ::Qt::LeftButton, qglviewer::CAMERA, qglviewer::SCREEN_ROTATE); setMouseBinding(::Qt::NoModifier, ::Qt::LeftButton, qglviewer::ALIGN_CAMERA, true); - setMouseBinding(::Qt::NoModifier, ::Qt::MidButton, qglviewer::SHOW_ENTIRE_SCENE, true); + setMouseBinding(::Qt::NoModifier, ::Qt::MiddleButton, qglviewer::SHOW_ENTIRE_SCENE, true); setMouseBinding(::Qt::NoModifier, ::Qt::RightButton, qglviewer::CENTER_SCENE, true); setWheelBinding(modifiers, qglviewer::CAMERA, qglviewer::ZOOM); From 0010df67e64bb45e621cdffe85e14569e6d54bb8 Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Tue, 16 Feb 2021 08:57:47 +0100 Subject: [PATCH 36/38] Remove useless if. --- GraphicsView/include/CGAL/Qt/camera_impl.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/GraphicsView/include/CGAL/Qt/camera_impl.h b/GraphicsView/include/CGAL/Qt/camera_impl.h index 709409f1609..971ae6b1de9 100644 --- a/GraphicsView/include/CGAL/Qt/camera_impl.h +++ b/GraphicsView/include/CGAL/Qt/camera_impl.h @@ -885,11 +885,8 @@ CGAL_INLINE_FUNCTION Vec Camera::pointUnderPixel(const QPoint &pixel, bool &found) const { float depth = 2.0; // Qt uses upper corner for its origin while GL uses the lower corner. - if(parent()) - { dynamic_cast(parent())->glReadPixels(pixel.x(), screenHeight() - 1 - pixel.y(), 1, 1, GL_DEPTH_COMPONENT, GL_FLOAT, &depth); - } found = depth < 1.0; Vec point(pixel.x(), pixel.y(), depth); point = unprojectedCoordinatesOf(point); From 38f1b179c09693d71fa80f65198d27dcf878a32f Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Tue, 16 Feb 2021 09:50:47 +0100 Subject: [PATCH 37/38] Test the dynamic_cast rsult --- GraphicsView/include/CGAL/Qt/camera_impl.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/GraphicsView/include/CGAL/Qt/camera_impl.h b/GraphicsView/include/CGAL/Qt/camera_impl.h index 971ae6b1de9..2b815b6fd65 100644 --- a/GraphicsView/include/CGAL/Qt/camera_impl.h +++ b/GraphicsView/include/CGAL/Qt/camera_impl.h @@ -885,8 +885,11 @@ CGAL_INLINE_FUNCTION Vec Camera::pointUnderPixel(const QPoint &pixel, bool &found) const { float depth = 2.0; // Qt uses upper corner for its origin while GL uses the lower corner. - dynamic_cast(parent())->glReadPixels(pixel.x(), screenHeight() - 1 - pixel.y(), 1, 1, - GL_DEPTH_COMPONENT, GL_FLOAT, &depth); + if(auto parent = dynamic_cast(parent())) + { + parent->glReadPixels(pixel.x(), screenHeight() - 1 - pixel.y(), 1, 1, + GL_DEPTH_COMPONENT, GL_FLOAT, &depth); + } found = depth < 1.0; Vec point(pixel.x(), pixel.y(), depth); point = unprojectedCoordinatesOf(point); From 0a085569bf200e6f5f27f076a1fd48d2ab8a2159 Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Wed, 17 Feb 2021 08:37:57 +0100 Subject: [PATCH 38/38] Fix parent error --- GraphicsView/include/CGAL/Qt/camera_impl.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/GraphicsView/include/CGAL/Qt/camera_impl.h b/GraphicsView/include/CGAL/Qt/camera_impl.h index 2b815b6fd65..30a56a72988 100644 --- a/GraphicsView/include/CGAL/Qt/camera_impl.h +++ b/GraphicsView/include/CGAL/Qt/camera_impl.h @@ -885,10 +885,10 @@ CGAL_INLINE_FUNCTION Vec Camera::pointUnderPixel(const QPoint &pixel, bool &found) const { float depth = 2.0; // Qt uses upper corner for its origin while GL uses the lower corner. - if(auto parent = dynamic_cast(parent())) + if(auto p = dynamic_cast(parent())) { - parent->glReadPixels(pixel.x(), screenHeight() - 1 - pixel.y(), 1, 1, - GL_DEPTH_COMPONENT, GL_FLOAT, &depth); + p->glReadPixels(pixel.x(), screenHeight() - 1 - pixel.y(), 1, 1, + GL_DEPTH_COMPONENT, GL_FLOAT, &depth); } found = depth < 1.0; Vec point(pixel.x(), pixel.y(), depth);