Add move constructor to AABB tree and use smart pointers

AABB_tree/include/CGAL/AABB_tree.h

@@ -13,6 +13,8 @@
 #ifndef CGAL_AABB_TREE_H
 #define CGAL_AABB_TREE_H
 
+#include <memory>
+
 #include <CGAL/license/AABB_tree.h>
 
 #include <CGAL/disable_warnings.h>
@@ -61,6 +63,8 @@ namespace CGAL {
 		// type of the primitives container
 		typedef std::vector<typename AABBTraits::Primitive> Primitives;
 
+		typedef AABB_tree<AABBTraits> 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 ) {
+      m_p_nodes.clear();
-				delete [] m_p_root_node;
-			}
-			m_p_root_node = nullptr;
     }
 
 		// 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.reset();
-				m_p_search_tree = nullptr;
 				m_search_tree_constructed = false;
                         }
 		}
@@ -516,6 +524,20 @@ public:
 	private:
 		typedef AABB_node<AABBTraits> 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<typename ConstPrimitiveIterator>
+    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
+		// tree nodes. first node is the root node
-		Node* m_p_root_node;
+		std::vector<Node> 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<AABBTraits>* >(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<const Search_tree> 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<AABBTraits> Self;
-		AABB_tree(const Self& src);
-		Self& operator=(const Self& src);
-
 	};  // end class AABB_tree
 
 /// @}
@@ -600,13 +622,31 @@ public:
   AABB_tree<Tr>::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 Tr>
+	typename AABB_tree<Tr>::Self& AABB_tree<Tr>::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<typename Tr>
+	AABB_tree<Tr>::AABB_tree(Self&& tree) noexcept
+	{
+		*this = std::move(tree);
+	}
+
  	template<typename Tr>
 	template<typename ConstPrimitiveIterator, typename ... T>
 	AABB_tree<Tr>::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<typename Tr>
+  template<typename ConstPrimitiveIterator>
+  void
+  AABB_tree<Tr>::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<typename Tr>
 	void AABB_tree<Tr>::build()
 	{
     clear_nodes();
+
     if(m_primitives.size() > 1) {
 
 			// allocates tree nodes
-			m_p_root_node = new Node[m_primitives.size()-1]();
+			m_p_nodes.reserve(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();
-			}
 
 			// 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<Tr>::build_kd_tree(ConstPointIterator first,
 		ConstPointIterator beyond) const
 	{
-		m_p_search_tree = new Search_tree(first, beyond);
+		m_p_search_tree = std::make_unique<const Search_tree>(first, beyond);
                 m_default_search_tree_constructed = true;
-		if(m_p_search_tree != nullptr)
+		m_search_tree_constructed = true;
-		{
+		return true;
-			m_search_tree_constructed = true;
-			return true;
-		}
-		else
-    {
-			std::cerr << "Unable to allocate memory for accelerating distance queries" << std::endl;
-			return false;
-    }
 	}
 
 	template<typename Tr>
 	void AABB_tree<Tr>::do_not_accelerate_distance_queries()const
 	{
-	  clear_search_tree();
+		clear_search_tree();
-	  m_default_search_tree_constructed = false;
+		m_default_search_tree_constructed = false;
 	}
 
-
 	// constructs the search KD tree from internal primitives
 	template<typename Tr>
 	bool AABB_tree<Tr>::accelerate_distance_queries() const

AABB_tree/include/CGAL/internal/AABB_tree/AABB_node.h

@@ -32,6 +32,9 @@ namespace CGAL {
 template<typename AABBTraits>
 class AABB_node
 {
+private:
+  typedef AABB_node<AABBTraits> 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
+  AABB_node(Self&& node) = default;
-  /// Do not delete children because the tree hosts and delete them
+
-  ~AABB_node() { };
+  // 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<typename ConstPrimitiveIterator>
-  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<Primitive*>(m_p_left_child); }
   const Primitive& right_data() const
                      { return *static_cast<Primitive*>(m_p_right_child); }
+  template <class Left, class Right>
+  void set_children(Left&& l, Right&& r)
+  {
+    m_p_left_child = static_cast<void*>(std::addressof(l));
+    m_p_right_child = static_cast<void*>(std::addressof(r));
+  }
+  void set_bbox(const Bounding_box& bbox)
+  {
+    m_bbox = bbox;
+  }
 
-private:
   Node& left_child() { return *static_cast<Node*>(m_p_left_child); }
   Node& right_child() { return *static_cast<Node*>(m_p_right_child); }
   Primitive& left_data() { return *static_cast<Primitive*>(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<AABBTraits> Self;
-  AABB_node(const Self& src);
-  Self& operator=(const Self& src);
-
 };  // end class AABB_node
 
-
-template<typename Tr>
-template<typename ConstPrimitiveIterator>
-void
-AABB_node<Tr>::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<Node*>(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<Node*>(this) + 1;
-    m_p_right_child = static_cast<Node*>(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<typename Tr>
 template<class Traversal_traits, class Query>
 void

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 <memory>
+
 #include <CGAL/license/AABB_tree.h>
 
 
@@ -83,7 +85,7 @@ namespace CGAL
                 typedef typename CGAL::Orthogonal_k_neighbor_search<TreeTraits> Neighbor_search;
                 typedef typename Neighbor_search::Tree Tree;
         private:
-                Tree* m_p_tree;
+                std::unique_ptr<Tree> 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<Decorated_point> 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());
+                        m_p_tree = std::make_unique<Tree>(points.begin(), points.end());
-                        if(m_p_tree != nullptr)
+                        m_p_tree->build();
-                                m_p_tree->build();
-                        else
-                                std::cerr << "unable to build the search tree!" << std::endl;
                 }
 
-                ~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);