From 5e9801839ded781213147c8d47792b2d1eab1e01 Mon Sep 17 00:00:00 2001 From: Ahmed Essam Date: Sun, 1 Mar 2020 02:44:34 +0200 Subject: [PATCH 1/7] Add move constructor to AABB tree and use smart pointers --- AABB_tree/include/CGAL/AABB_tree.h | 139 ++++++++++++------ .../CGAL/internal/AABB_tree/AABB_node.h | 77 +++------- .../internal/AABB_tree/AABB_search_tree.h | 18 +-- 3 files changed, 122 insertions(+), 112 deletions(-) diff --git a/AABB_tree/include/CGAL/AABB_tree.h b/AABB_tree/include/CGAL/AABB_tree.h index 99dfd89f7f9..52150f28352 100644 --- a/AABB_tree/include/CGAL/AABB_tree.h +++ b/AABB_tree/include/CGAL/AABB_tree.h @@ -13,6 +13,8 @@ #ifndef CGAL_AABB_TREE_H #define CGAL_AABB_TREE_H +#include + #include #include @@ -61,6 +63,8 @@ namespace CGAL { // type of the primitives container typedef std::vector Primitives; + typedef AABB_tree Self; + public: typedef AABBTraits AABB_traits; @@ -111,6 +115,14 @@ namespace CGAL { /// class using `traits`. AABB_tree(const AABBTraits& traits = AABBTraits()); + // move constructor and assignment operator + AABB_tree(Self&&) noexcept; + Self& operator=(Self&&) noexcept; + + // Disabled copy constructor & assignment operator + AABB_tree(const Self&) = delete; + Self& operator=(const Self&) = delete; + /** * @brief Builds the datastructure from a sequence of primitives. * @param first iterator over first primitive to insert @@ -477,10 +489,7 @@ public: // clear nodes void clear_nodes() { - if( size() > 1 ) { - delete [] m_p_root_node; - } - m_p_root_node = nullptr; + m_p_nodes.clear(); } // clears internal KD tree @@ -489,8 +498,7 @@ public: if ( m_search_tree_constructed ) { CGAL_assertion( m_p_search_tree!=nullptr ); - delete m_p_search_tree; - m_p_search_tree = nullptr; + m_p_search_tree.reset(); m_search_tree_constructed = false; } } @@ -516,6 +524,20 @@ public: private: typedef AABB_node Node; + /** + * @brief Builds the tree by recursive expansion. + * @param first the first primitive to insert + * @param last the last primitive to insert + * @param range the number of primitive of the range + * + * [first,last[ is the range of primitives to be added to the tree. + */ + template + void expand(Node& node, + ConstPrimitiveIterator first, + ConstPrimitiveIterator beyond, + const std::size_t range, + const AABBTraits&); public: // returns a point which must be on one primitive @@ -555,8 +577,8 @@ public: AABBTraits m_traits; // set of input primitives Primitives m_primitives; - // single root node - Node* m_p_root_node; + // tree nodes. first node is the root node + std::vector m_p_nodes; #ifdef CGAL_HAS_THREADS mutable CGAL_MUTEX internal_tree_mutex;//mutex used to protect const calls inducing build() mutable CGAL_MUTEX kd_tree_mutex;//mutex used to protect calls to accelerate_distance_queries @@ -572,7 +594,13 @@ public: #endif const_cast< AABB_tree* >(this)->build(); } - return m_p_root_node; + return std::addressof(m_p_nodes[0]); + } + + Node& new_node() + { + m_p_nodes.emplace_back(); + return m_p_nodes.back(); } const Primitive& singleton_data() const { @@ -581,17 +609,11 @@ public: } // search KD-tree - mutable const Search_tree* m_p_search_tree; + mutable std::unique_ptr m_p_search_tree; mutable bool m_search_tree_constructed; mutable bool m_default_search_tree_constructed; // indicates whether the internal kd-tree should be built bool m_need_build; - private: - // Disabled copy constructor & assignment operator - typedef AABB_tree Self; - AABB_tree(const Self& src); - Self& operator=(const Self& src); - }; // end class AABB_tree /// @} @@ -600,13 +622,31 @@ public: AABB_tree::AABB_tree(const Tr& traits) : m_traits(traits) , m_primitives() - , m_p_root_node(nullptr) + , m_p_nodes() , m_p_search_tree(nullptr) , m_search_tree_constructed(false) , m_default_search_tree_constructed(true) , m_need_build(false) {} + template + typename AABB_tree::Self& AABB_tree::operator=(Self&& tree) noexcept + { + m_traits = std::move(tree.traits); + m_primitives = std::exchange(tree.m_primitives, {}); + m_p_nodes = std::exchange(tree.m_p_nodes, {}); + m_p_search_tree = std::exchange(tree.m_p_search_tree, nullptr); + m_search_tree_constructed = std::exchange(tree.m_search_tree_constructed, false); + m_default_search_tree_constructed = std::exchange(tree.m_default_search_tree_constructed, true); + m_need_build = std::exchange(tree.m_need_build, false); + } + + template + AABB_tree::AABB_tree(Self&& tree) noexcept + { + *this = std::move(tree); + } + template template AABB_tree::AABB_tree(ConstPrimitiveIterator first, @@ -614,7 +654,7 @@ public: T&& ... t) : m_traits() , m_primitives() - , m_p_root_node(nullptr) + , m_p_nodes() , m_p_search_tree(nullptr) , m_search_tree_constructed(false) , m_default_search_tree_constructed(true) @@ -670,25 +710,51 @@ public: m_need_build = true; } + template + template + void + AABB_tree::expand(Node& node, + ConstPrimitiveIterator first, + ConstPrimitiveIterator beyond, + const std::size_t range, + const Tr& traits) + { + node.set_bbox(traits.compute_bbox_object()(first, beyond)); + + // sort primitives along longest axis aabb + traits.split_primitives_object()(first, beyond, node.bbox()); + + switch(range) + { + case 2: + node.set_children(*first, *(first+1)); + break; + case 3: + node.set_children(*first, new_node()); + expand(node.right_child(), first+1, beyond, 2, traits); + break; + default: + const std::size_t new_range = range/2; + node.set_children(new_node(), new_node()); + expand(node.left_child(), first, first + new_range, new_range, traits); + expand(node.right_child(), first + new_range, beyond, range - new_range, traits); + } + } + + // Build the data structure, after calls to insert(..) template void AABB_tree::build() { clear_nodes(); + if(m_primitives.size() > 1) { // allocates tree nodes - m_p_root_node = new Node[m_primitives.size()-1](); - if(m_p_root_node == nullptr) - { - std::cerr << "Unable to allocate memory for AABB tree" << std::endl; - CGAL_assertion(m_p_root_node != nullptr); - m_primitives.clear(); - clear(); - } + m_p_nodes.reserve(m_primitives.size()-1); // constructs the tree - m_p_root_node->expand(m_primitives.begin(), m_primitives.end(), + expand(new_node(), m_primitives.begin(), m_primitives.end(), m_primitives.size(), m_traits); } @@ -729,28 +795,19 @@ public: bool AABB_tree::build_kd_tree(ConstPointIterator first, ConstPointIterator beyond) const { - m_p_search_tree = new Search_tree(first, beyond); + m_p_search_tree = std::make_unique(first, beyond); m_default_search_tree_constructed = true; - if(m_p_search_tree != nullptr) - { - m_search_tree_constructed = true; - return true; - } - else - { - std::cerr << "Unable to allocate memory for accelerating distance queries" << std::endl; - return false; - } + m_search_tree_constructed = true; + return true; } template void AABB_tree::do_not_accelerate_distance_queries()const { - clear_search_tree(); - m_default_search_tree_constructed = false; + clear_search_tree(); + m_default_search_tree_constructed = false; } - // constructs the search KD tree from internal primitives template bool AABB_tree::accelerate_distance_queries() const diff --git a/AABB_tree/include/CGAL/internal/AABB_tree/AABB_node.h b/AABB_tree/include/CGAL/internal/AABB_tree/AABB_node.h index 3844166ed50..4e41cec195c 100644 --- a/AABB_tree/include/CGAL/internal/AABB_tree/AABB_node.h +++ b/AABB_tree/include/CGAL/internal/AABB_tree/AABB_node.h @@ -32,6 +32,9 @@ namespace CGAL { template class AABB_node { +private: + typedef AABB_node Self; + public: typedef typename AABBTraits::Bounding_box Bounding_box; @@ -41,27 +44,15 @@ public: , m_p_left_child(nullptr) , m_p_right_child(nullptr) { }; - /// Non virtual Destructor - /// Do not delete children because the tree hosts and delete them - ~AABB_node() { }; + AABB_node(Self&& node) = default; + + // Disabled copy constructor & assignment operator + AABB_node(const Self& src) = delete; + Self& operator=(const Self& src) = delete; /// Returns the bounding box of the node const Bounding_box& bbox() const { return m_bbox; } - /** - * @brief Builds the tree by recursive expansion. - * @param first the first primitive to insert - * @param last the last primitive to insert - * @param range the number of primitive of the range - * - * [first,last[ is the range of primitives to be added to the tree. - */ - template - void expand(ConstPrimitiveIterator first, - ConstPrimitiveIterator beyond, - const std::size_t range, - const AABBTraits&); - /** * @brief General traversal query * @param query the query @@ -93,8 +84,17 @@ public: { return *static_cast(m_p_left_child); } const Primitive& right_data() const { return *static_cast(m_p_right_child); } + template + void set_children(Left&& l, Right&& r) + { + m_p_left_child = static_cast(std::addressof(l)); + m_p_right_child = static_cast(std::addressof(r)); + } + void set_bbox(const Bounding_box& bbox) + { + m_bbox = bbox; + } -private: Node& left_child() { return *static_cast(m_p_left_child); } Node& right_child() { return *static_cast(m_p_right_child); } Primitive& left_data() { return *static_cast(m_p_left_child); } @@ -109,49 +109,8 @@ private: void *m_p_left_child; void *m_p_right_child; -private: - // Disabled copy constructor & assignment operator - typedef AABB_node Self; - AABB_node(const Self& src); - Self& operator=(const Self& src); - }; // end class AABB_node - -template -template -void -AABB_node::expand(ConstPrimitiveIterator first, - ConstPrimitiveIterator beyond, - const std::size_t range, - const Tr& traits) -{ - m_bbox = traits.compute_bbox_object()(first, beyond); - - // sort primitives along longest axis aabb - traits.split_primitives_object()(first, beyond, m_bbox); - - switch(range) - { - case 2: - m_p_left_child = &(*first); - m_p_right_child = &(*(++first)); - break; - case 3: - m_p_left_child = &(*first); - m_p_right_child = static_cast(this)+1; - right_child().expand(first+1, beyond, 2,traits); - break; - default: - const std::size_t new_range = range/2; - m_p_left_child = static_cast(this) + 1; - m_p_right_child = static_cast(this) + new_range; - left_child().expand(first, first + new_range, new_range,traits); - right_child().expand(first + new_range, beyond, range - new_range,traits); - } -} - - template template void diff --git a/AABB_tree/include/CGAL/internal/AABB_tree/AABB_search_tree.h b/AABB_tree/include/CGAL/internal/AABB_tree/AABB_search_tree.h index 96f08168671..f50afaf4f03 100644 --- a/AABB_tree/include/CGAL/internal/AABB_tree/AABB_search_tree.h +++ b/AABB_tree/include/CGAL/internal/AABB_tree/AABB_search_tree.h @@ -13,6 +13,8 @@ #ifndef CGAL_AABB_SEARCH_TREE_H #define CGAL_AABB_SEARCH_TREE_H +#include + #include @@ -83,7 +85,7 @@ namespace CGAL typedef typename CGAL::Orthogonal_k_neighbor_search Neighbor_search; typedef typename Neighbor_search::Tree Tree; private: - Tree* m_p_tree; + std::unique_ptr m_p_tree; Point_and_primitive_id get_p_and_p(const Point_and_primitive_id& p) @@ -104,21 +106,13 @@ namespace CGAL std::vector points; while(begin != beyond) { Point_and_primitive_id pp = get_p_and_p(*begin); - points.push_back(Decorated_point(pp.first,pp.second)); + points.emplace_back(pp.first,pp.second); ++begin; } - m_p_tree = new Tree(points.begin(), points.end()); - if(m_p_tree != nullptr) - m_p_tree->build(); - else - std::cerr << "unable to build the search tree!" << std::endl; + m_p_tree = std::make_unique(points.begin(), points.end()); + m_p_tree->build(); } - ~AABB_search_tree() { - delete m_p_tree; - } - - Point_and_primitive_id closest_point(const Point& query) const { Neighbor_search search(*m_p_tree, query, 1); From 5f8e3bc3cf10b59956a62b6fb65049cc5c7a1209 Mon Sep 17 00:00:00 2001 From: Ahmed Essam Date: Tue, 3 Mar 2020 15:00:42 +0200 Subject: [PATCH 2/7] Use lval ref instead of perfect forwarding in set_children Co-Authored-By: Sebastien Loriot --- AABB_tree/include/CGAL/internal/AABB_tree/AABB_node.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AABB_tree/include/CGAL/internal/AABB_tree/AABB_node.h b/AABB_tree/include/CGAL/internal/AABB_tree/AABB_node.h index 4e41cec195c..e1a9974ded6 100644 --- a/AABB_tree/include/CGAL/internal/AABB_tree/AABB_node.h +++ b/AABB_tree/include/CGAL/internal/AABB_tree/AABB_node.h @@ -85,7 +85,7 @@ public: const Primitive& right_data() const { return *static_cast(m_p_right_child); } template - void set_children(Left&& l, Right&& r) + void set_children(Left& l, Right& r) { m_p_left_child = static_cast(std::addressof(l)); m_p_right_child = static_cast(std::addressof(r)); From 95c1af02248e1167f551130504e4670be4fb649f Mon Sep 17 00:00:00 2001 From: Ahmed Essam Date: Fri, 6 Mar 2020 16:06:52 +0200 Subject: [PATCH 3/7] Add tree creation benchmark --- AABB_tree/benchmark/AABB_tree/CMakeLists.txt | 13 +++- AABB_tree/benchmark/AABB_tree/test.cpp | 4 +- .../benchmark/AABB_tree/tree_creation.cpp | 71 +++++++++++++++++++ 3 files changed, 84 insertions(+), 4 deletions(-) create mode 100644 AABB_tree/benchmark/AABB_tree/tree_creation.cpp diff --git a/AABB_tree/benchmark/AABB_tree/CMakeLists.txt b/AABB_tree/benchmark/AABB_tree/CMakeLists.txt index 0716a91ec94..35a5c15ac52 100644 --- a/AABB_tree/benchmark/AABB_tree/CMakeLists.txt +++ b/AABB_tree/benchmark/AABB_tree/CMakeLists.txt @@ -23,9 +23,18 @@ if ( NOT Boost_FOUND ) return() endif() +# google benchmark +find_package(benchmark REQUIRED) +if ( NOT benchmark_FOUND ) + message(STATUS "This project requires Google benchmarks, and will not be compiled.") +endif() + +message(${CMAKE_CXX_FLAGS_RELEASE}) + # include for local package include_directories( BEFORE ../../include ) -add_executable (test_ test.cpp) - +# add_executable (test_ test.cpp) // TODO: fix this benchmark +add_executable(tree_creation tree_creation.cpp) +target_link_libraries(tree_creation benchmark::benchmark) diff --git a/AABB_tree/benchmark/AABB_tree/test.cpp b/AABB_tree/benchmark/AABB_tree/test.cpp index 22ebbbb08e5..5c6dbd927e6 100644 --- a/AABB_tree/benchmark/AABB_tree/test.cpp +++ b/AABB_tree/benchmark/AABB_tree/test.cpp @@ -7,7 +7,7 @@ #include #include #include -#include +#include #include #include @@ -18,7 +18,7 @@ typedef CGAL::Exact_predicates_inexact_constructions_kernel K; typedef CGAL::Surface_mesh Surface_mesh; typedef CGAL::AABB_face_graph_triangle_primitive Primitive; -typedef CGAL::AABB_do_intersect_transform_traits Traits; +typedef CGAL::Do_intersect_traversal_traits_with_transformation Traits; typedef CGAL::AABB_tree Tree; namespace PMP = CGAL::Polygon_mesh_processing; diff --git a/AABB_tree/benchmark/AABB_tree/tree_creation.cpp b/AABB_tree/benchmark/AABB_tree/tree_creation.cpp new file mode 100644 index 00000000000..deb999c6b27 --- /dev/null +++ b/AABB_tree/benchmark/AABB_tree/tree_creation.cpp @@ -0,0 +1,71 @@ +#include +#include +#include +#include +#include +#include +#include + +typedef CGAL::Simple_cartesian K; +typedef CGAL::Surface_mesh Surface_mesh; +typedef CGAL::AABB_face_graph_triangle_primitive Primitive; +typedef CGAL::AABB_traits Traits; +typedef CGAL::AABB_tree Tree; +typedef K::Segment_3 Segment; +typedef K::Point_3 Point_3; + + +Surface_mesh mesh; + +static void BM_TreeCreation(benchmark::State& state) +{ + for (auto _ : state) + { + benchmark::DoNotOptimize([]() { + Tree tree{mesh.faces_begin(), mesh.faces_end(), mesh}; + tree.build(); + return 0; + }()); + } +} +BENCHMARK(BM_TreeCreation); + +static void BM_Intersections(benchmark::State& state) +{ + Point_3 p(-0.5, 0.03, 0.04); + Point_3 q(-0.5, 0.04, 0.06); + + Tree tree{mesh.faces_begin(), mesh.faces_end(), mesh}; + tree.build(); + + Segment segment_query(p, q); + + for (auto _ : state) + { + benchmark::DoNotOptimize([&]() { + tree.accelerate_distance_queries(); + tree.number_of_intersected_primitives(segment_query); + Point_3 point_query(2.0, 2.0, 2.0); + Point_3 closest = tree.closest_point(point_query); + return 0; + }()); + } +} +BENCHMARK(BM_Intersections); + + +int main(int argc, char** argv) +{ + const char* default_file = "data/handle.off"; + const char* filename = argc > 2? argv[2] : default_file; + + { + std::ifstream input(filename); + input >> mesh; + } + + ::benchmark::Initialize(&argc, argv); + ::benchmark::RunSpecifiedBenchmarks(); + + return EXIT_SUCCESS; +} From 49465bbcb62d35ed56efca4b36fb4d9d28fa9dcd Mon Sep 17 00:00:00 2001 From: Ahmed Essam Date: Fri, 6 Mar 2020 16:28:21 +0200 Subject: [PATCH 4/7] Use stack variable instead of pointer --- AABB_tree/include/CGAL/AABB_tree.h | 6 +++--- .../CGAL/internal/AABB_tree/AABB_search_tree.h | 16 +++++++--------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/AABB_tree/include/CGAL/AABB_tree.h b/AABB_tree/include/CGAL/AABB_tree.h index 52150f28352..95933a614c3 100644 --- a/AABB_tree/include/CGAL/AABB_tree.h +++ b/AABB_tree/include/CGAL/AABB_tree.h @@ -633,9 +633,9 @@ public: typename AABB_tree::Self& AABB_tree::operator=(Self&& tree) noexcept { m_traits = std::move(tree.traits); - m_primitives = std::exchange(tree.m_primitives, {}); - m_p_nodes = std::exchange(tree.m_p_nodes, {}); - m_p_search_tree = std::exchange(tree.m_p_search_tree, nullptr); + m_primitives = std::move(tree.m_primitives); + m_p_nodes = std::move(tree.m_p_nodes); + m_p_search_tree = std::move(tree.m_p_search_tree); m_search_tree_constructed = std::exchange(tree.m_search_tree_constructed, false); m_default_search_tree_constructed = std::exchange(tree.m_default_search_tree_constructed, true); m_need_build = std::exchange(tree.m_need_build, false); diff --git a/AABB_tree/include/CGAL/internal/AABB_tree/AABB_search_tree.h b/AABB_tree/include/CGAL/internal/AABB_tree/AABB_search_tree.h index f50afaf4f03..9ba3cf2481b 100644 --- a/AABB_tree/include/CGAL/internal/AABB_tree/AABB_search_tree.h +++ b/AABB_tree/include/CGAL/internal/AABB_tree/AABB_search_tree.h @@ -13,8 +13,6 @@ #ifndef CGAL_AABB_SEARCH_TREE_H #define CGAL_AABB_SEARCH_TREE_H -#include - #include @@ -85,7 +83,7 @@ namespace CGAL typedef typename CGAL::Orthogonal_k_neighbor_search Neighbor_search; typedef typename Neighbor_search::Tree Tree; private: - std::unique_ptr m_p_tree; + Tree m_tree; Point_and_primitive_id get_p_and_p(const Point_and_primitive_id& p) @@ -100,22 +98,22 @@ namespace CGAL public: template AABB_search_tree(ConstPointIterator begin, ConstPointIterator beyond) - : m_p_tree(nullptr) + : m_tree{} { - typedef typename Add_decorated_point::Point_3 Decorated_point; + typedef typename Add_decorated_point::Point_3 Decorated_point; std::vector points; while(begin != beyond) { Point_and_primitive_id pp = get_p_and_p(*begin); - points.emplace_back(pp.first,pp.second); + points.emplace_back(pp.first, pp.second); ++begin; } - m_p_tree = std::make_unique(points.begin(), points.end()); - m_p_tree->build(); + m_tree.insert(points.begin(), points.end()); + m_tree.build(); } Point_and_primitive_id closest_point(const Point& query) const { - Neighbor_search search(*m_p_tree, query, 1); + Neighbor_search search(m_tree, query, 1); return Point_and_primitive_id(static_cast(search.begin()->first), search.begin()->first.id()); } }; From 587c77774c19805d437aa1af0996bf3413b4bd40 Mon Sep 17 00:00:00 2001 From: Ahmed Essam Date: Fri, 6 Mar 2020 16:34:40 +0200 Subject: [PATCH 5/7] No need to check existance of package if REQUIRED is used --- AABB_tree/benchmark/AABB_tree/CMakeLists.txt | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/AABB_tree/benchmark/AABB_tree/CMakeLists.txt b/AABB_tree/benchmark/AABB_tree/CMakeLists.txt index 35a5c15ac52..4db5f8e86e5 100644 --- a/AABB_tree/benchmark/AABB_tree/CMakeLists.txt +++ b/AABB_tree/benchmark/AABB_tree/CMakeLists.txt @@ -5,31 +5,15 @@ cmake_minimum_required(VERSION 3.1...3.15) project( AABB_traits_benchmark) # CGAL and its components -find_package( CGAL QUIET) -if ( CGAL_FOUND ) - include( ${CGAL_USE_FILE} ) -else () - message(STATUS "This project requires the CGAL library, and will not be compiled.") - return() - -endif() - +find_package( CGAL REQUIRED ) +include( ${CGAL_USE_FILE} ) # Boost and its components find_package( Boost REQUIRED ) # include for local directory -if ( NOT Boost_FOUND ) - message(STATUS "This project requires the Boost library, and will not be compiled.") - return() -endif() # google benchmark find_package(benchmark REQUIRED) -if ( NOT benchmark_FOUND ) - message(STATUS "This project requires Google benchmarks, and will not be compiled.") -endif() - -message(${CMAKE_CXX_FLAGS_RELEASE}) # include for local package include_directories( BEFORE ../../include ) From 2b9c2c163449077099f3cf0c89cebb5acdf41e64 Mon Sep 17 00:00:00 2001 From: Ahmed Essam Date: Sat, 7 Mar 2020 04:28:13 +0200 Subject: [PATCH 6/7] Test AABBTree move semantics --- .../benchmark/AABB_tree/tree_creation.cpp | 4 +- AABB_tree/include/CGAL/AABB_tree.h | 3 +- .../AABB_tree/aabb_test_move_constructor.cpp | 193 ++++++++++++++++++ 3 files changed, 196 insertions(+), 4 deletions(-) create mode 100644 AABB_tree/test/AABB_tree/aabb_test_move_constructor.cpp diff --git a/AABB_tree/benchmark/AABB_tree/tree_creation.cpp b/AABB_tree/benchmark/AABB_tree/tree_creation.cpp index deb999c6b27..aa58d5545f2 100644 --- a/AABB_tree/benchmark/AABB_tree/tree_creation.cpp +++ b/AABB_tree/benchmark/AABB_tree/tree_creation.cpp @@ -36,14 +36,12 @@ static void BM_Intersections(benchmark::State& state) Point_3 q(-0.5, 0.04, 0.06); Tree tree{mesh.faces_begin(), mesh.faces_end(), mesh}; - tree.build(); + tree.accelerate_distance_queries(); Segment segment_query(p, q); - for (auto _ : state) { benchmark::DoNotOptimize([&]() { - tree.accelerate_distance_queries(); tree.number_of_intersected_primitives(segment_query); Point_3 point_query(2.0, 2.0, 2.0); Point_3 closest = tree.closest_point(point_query); diff --git a/AABB_tree/include/CGAL/AABB_tree.h b/AABB_tree/include/CGAL/AABB_tree.h index 95933a614c3..6ac023ecbb6 100644 --- a/AABB_tree/include/CGAL/AABB_tree.h +++ b/AABB_tree/include/CGAL/AABB_tree.h @@ -632,13 +632,14 @@ public: template typename AABB_tree::Self& AABB_tree::operator=(Self&& tree) noexcept { - m_traits = std::move(tree.traits); + m_traits = std::move(tree.m_traits); m_primitives = std::move(tree.m_primitives); m_p_nodes = std::move(tree.m_p_nodes); m_p_search_tree = std::move(tree.m_p_search_tree); m_search_tree_constructed = std::exchange(tree.m_search_tree_constructed, false); m_default_search_tree_constructed = std::exchange(tree.m_default_search_tree_constructed, true); m_need_build = std::exchange(tree.m_need_build, false); + return *this; } template diff --git a/AABB_tree/test/AABB_tree/aabb_test_move_constructor.cpp b/AABB_tree/test/AABB_tree/aabb_test_move_constructor.cpp new file mode 100644 index 00000000000..da175f7bb63 --- /dev/null +++ b/AABB_tree/test/AABB_tree/aabb_test_move_constructor.cpp @@ -0,0 +1,193 @@ + +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +template auto create_tree(); +template void init_tree(T &tree) {} +template bool test_tree(T &tree); +template class TestUtils; + +// test 0 is from "aabb_test_singleton_tree" +template <> struct TestUtils<0> { + typedef CGAL::Simple_cartesian K; + typedef K::FT FT; + typedef K::Point_3 Point; + typedef K::Plane_3 Plane; + typedef K::Segment_3 Segment; + typedef K::Triangle_3 Triangle; + typedef std::vector::iterator Iterator; + typedef CGAL::AABB_segment_primitive Primitive; + typedef CGAL::AABB_traits Traits; + typedef CGAL::AABB_tree Tree; + + Point a = {1.0, 0.0, 0.0}; + Point b = {0.0, 1.0, 0.0}; + Point c = {0.0, 0.0, 1.0}; + Point d = {0.0, 0.0, 0.0}; + std::vector segments = {Segment(Point(0, 0, 0), Point(2, 2, 2))}; +}; + +template <> auto create_tree<0>() { + using T = TestUtils<0>; + T utils; + return T::Tree(utils.segments.begin(), utils.segments.end()); +} +using Test0Param = decltype(create_tree<0>()); +template <> bool test_tree<0, Test0Param>(Test0Param &tree) { + using T = TestUtils<0>; + T utils; + + T::Plane plane_query(utils.a, utils.b, utils.d); + T::Triangle triangle_query(utils.a, utils.b, utils.c); + + // Test calls to all functions + CGAL::Emptyset_iterator devnull; + tree.all_intersections(triangle_query, devnull); + tree.all_intersected_primitives(triangle_query, devnull); + assert(tree.any_intersected_primitive(triangle_query)); + assert(tree.any_intersection(triangle_query)); + const CGAL::Bbox_3 bbox = tree.bbox(); + assert(bbox == CGAL::Bbox_3(0, 0, 0, 2, 2, 2)); + tree.clear(); + tree.insert(utils.segments.begin(), utils.segments.end()); + tree.build(); + assert(tree.closest_point(T::Point(-0.1, -0.1, -0.1)) == T::Point(0, 0, 0)); + assert(tree.closest_point(T::Point(-0.1, -0.1, -0.1), T::Point(0, 0, 0)) == + T::Point(0, 0, 0)); + assert(tree.closest_point_and_primitive(T::Point(-0.1, -0.1, -0.1)).second == + utils.segments.begin()); + assert(tree.do_intersect(plane_query) == true); + assert(tree.do_intersect(triangle_query) == true); + assert(!tree.empty()); + assert(tree.size() == 1); + tree.clear(); + assert(tree.size() == 0); + tree.insert(utils.segments.begin(), utils.segments.end()); + assert(tree.size() == 1); + assert(tree.number_of_intersected_primitives(plane_query) == 1); + tree.rebuild(utils.segments.begin(), utils.segments.end()); + assert(tree.size() == 1); + assert(tree.number_of_intersected_primitives(triangle_query) == 1); + assert(tree.squared_distance(T::Point(0, 0, 0)) == 0); + return true; +} + +// test 1 is from "aabb_test_all_intersected_primitives" +template <> struct TestUtils<1> { + typedef CGAL::Epick K; + typedef K::FT FT; + typedef K::Point_3 Point; + typedef K::Vector_3 Vector; + typedef K::Segment_3 Segment; + typedef K::Ray_3 Ray; + typedef CGAL::Surface_mesh> Mesh; + typedef CGAL::AABB_halfedge_graph_segment_primitive + S_Primitive; + typedef CGAL::AABB_face_graph_triangle_primitive + T_Primitive; + typedef CGAL::AABB_traits T_Traits; + typedef CGAL::AABB_traits S_Traits; + typedef CGAL::AABB_tree T_Tree; + typedef CGAL::AABB_tree S_Tree; + typedef T_Tree::Primitive_id T_Primitive_id; + typedef S_Tree::Primitive_id S_Primitive_id; +}; + +template <> auto create_tree<1>() { + using T = TestUtils<1>; + + static CGAL::Surface_mesh> m1 = {}; + static CGAL::Surface_mesh> m2 = {}; + static bool mesh_loaded = false; + if (!mesh_loaded) { + std::ifstream in("data/cube.off"); + assert(in); + in >> m1; + in.close(); + in.open("data/tetrahedron.off"); + assert(in); + in >> m2; + in.close(); + mesh_loaded = true; + } + return std::make_pair(T::T_Tree{faces(m1).first, faces(m1).second, m1}, + T::S_Tree{edges(m2).first, edges(m2).second, m2}); +} +using Test1Param = decltype(create_tree<1>()); +template <> void init_tree<1, Test1Param>(Test1Param &trees) { + trees.first.build(); + trees.second.build(); +} +template <> bool test_tree<1, Test1Param>(Test1Param &trees) { + using T = TestUtils<1>; + + auto &cube_tree = trees.first; + auto &tet_tree = trees.second; + + std::list t_primitives; + std::list s_primitives; + cube_tree.all_intersected_primitives(tet_tree, + std::back_inserter(t_primitives)); + CGAL_assertion(t_primitives.size() == 6); + tet_tree.all_intersected_primitives(cube_tree, + std::back_inserter(s_primitives)); + CGAL_assertion(s_primitives.size() == 6); + CGAL_assertion(tet_tree.do_intersect(cube_tree)); + CGAL_assertion(cube_tree.do_intersect(tet_tree)); + + std::vector all_primitives; + cube_tree.all_intersected_primitives(tet_tree, + std::back_inserter(all_primitives)); + bool found_f5 = false; + for (auto prim : all_primitives) { + if ((int)prim.first == 5) + found_f5 = true; + } + CGAL_assertion(found_f5); + CGAL_USE(found_f5); + return true; +} + +template bool run_test() { + // create_tree should return prvalue for guaranteed copy elision + auto tree_1 = create_tree(); + init_tree(tree_1); + auto tree_2 = create_tree(); + init_tree(tree_2); + auto tree_3 = create_tree(); + init_tree(tree_3); + + decltype(tree_1) tree_ctor{std::move(tree_2)}; + decltype(tree_1) tree_assig{}; + tree_assig = std::move(tree_3); + + bool normal = test_tree(tree_1); + bool move_ctor = test_tree(tree_ctor); + bool move_ass = + test_tree(tree_assig); // test move assignment operator + + if (!normal) + std::cout << "Test " << test_number << "failed on the original tree\n"; + if (!move_ctor) + std::cout << "Test " << test_number << "failed on move constructed tree\n"; + if (!move_ass) + std::cout << "Test " << test_number << "failed on move assigned tree\n"; + return normal && move_ctor && move_ass; +} + +int main() { return (run_test<0>() && run_test<1>()) ? 0 : 1; } From a3f04527715d5fe2212ca9a78e69958a1bad0165 Mon Sep 17 00:00:00 2001 From: Ahmed Essam Date: Sat, 7 Mar 2020 04:28:41 +0200 Subject: [PATCH 7/7] Update CHANGES.md --- AABB_tree/benchmark/AABB_tree/test.cpp | 4 ++-- Installation/CHANGES.md | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/AABB_tree/benchmark/AABB_tree/test.cpp b/AABB_tree/benchmark/AABB_tree/test.cpp index 5c6dbd927e6..22ebbbb08e5 100644 --- a/AABB_tree/benchmark/AABB_tree/test.cpp +++ b/AABB_tree/benchmark/AABB_tree/test.cpp @@ -7,7 +7,7 @@ #include #include #include -#include +#include #include #include @@ -18,7 +18,7 @@ typedef CGAL::Exact_predicates_inexact_constructions_kernel K; typedef CGAL::Surface_mesh Surface_mesh; typedef CGAL::AABB_face_graph_triangle_primitive Primitive; -typedef CGAL::Do_intersect_traversal_traits_with_transformation Traits; +typedef CGAL::AABB_do_intersect_transform_traits Traits; typedef CGAL::AABB_tree Tree; namespace PMP = CGAL::Polygon_mesh_processing; diff --git a/Installation/CHANGES.md b/Installation/CHANGES.md index ad9ad327281..6459ae143b4 100644 --- a/Installation/CHANGES.md +++ b/Installation/CHANGES.md @@ -6,6 +6,9 @@ Release History Release date: June 2020 +### Add Move Semantics to AABB Trees + - Added move constructor and assignment operator to AABB Tree class. + ### Surface Mesh Topology (new package) - This package allows to compute some topological invariants of