Eliminate non-index versions of split predicates & traversals

Orthtree/include/CGAL/Orthtree.h

@@ -155,7 +155,6 @@ public:
   typedef unspecified_type Node_range;
   typedef unspecified_type Node_index_range;
 #else
-  typedef boost::iterator_range<Traversal_iterator<Self>> Node_range;
   typedef boost::iterator_range<Index_traversal_iterator<Self>> Node_index_range;
 #endif
 
@@ -439,33 +438,18 @@ public:
   std::size_t depth() const { return m_side_per_depth.size() - 1; }
 
   /*!
-    \brief constructs a node range using a tree-traversal function.
+    \brief constructs a node index range using a tree-traversal function.
 
-    This method allows to iterate on the nodes of the tree with a
+    This method allows iteration over the nodes of the tree with a
     user-selected order (preorder, postorder, leaves-only, etc.).
 
-    todo: this should be removed, replaced with traverse_indices
-
     \tparam Traversal model of `OrthtreeTraversal` that provides functions
-    compatible with the type of the orthree
+    compatible with the type of the orthtree
 
     \param traversal the instance of `Traversal` used
 
-    \return a forward input iterator over the nodes of the tree
+    \return a forward input iterator over the node indices of the tree
    */
-  template <typename Traversal>
-  Node_range traverse(Traversal traversal) const {
-
-    auto first = traversal.first_index();
-
-    auto next = [=](const Self& tree, Node_index index) -> Maybe_node_index {
-      return traversal.next_index(index);
-    };
-
-    return boost::make_iterator_range(Traversal_iterator<Self>(*this, first, next),
-                                      Traversal_iterator<Self>());
-  }
-
   template <typename Traversal>
   Node_index_range traverse_indices(Traversal traversal) const {
 
@@ -479,12 +463,17 @@ public:
                                       Index_traversal_iterator<Self>());
   }
 
-  // todo: document this convenience function
-  template <typename Traversal, typename ...Args>
-  Node_range traverse(Args&& ...args) const {
-    return traverse<Traversal>({*this, std::forward<Args>(args)...});
-  }
 
+  /*!
+    \brief Convenience function for using a traversal without constructing it yourself
+
+    \tparam Traversal model of `OrthtreeTraversal` that provides functions
+    compatible with the type of the orthtree
+
+    \param args Arguments to to pass to the traversal's constructor, excluding the first (always an orthtree reference)
+
+    \return a forward input iterator over the node indices of the tree
+   */
   template <typename Traversal, typename ...Args>
   Node_index_range traverse_indices(Args&& ...args) const {
     return traverse_indices(Traversal{*this, std::forward<Args>(args)...});
@@ -1168,8 +1157,8 @@ public:
 
   /// \cond SKIP_IN_MANUAL
   void dump_to_polylines(std::ostream& os) const {
-    for (const Node& n: traverse<Orthtrees::Preorder_traversal>())
+    for (const auto n: traverse_indices<Orthtrees::Preorder_traversal>())
-      if (n.is_leaf()) {
+      if (is_leaf(n)) {
         Bbox box = bbox(n);
         dump_box_to_polylines(box, os);
       }

Orthtree/include/CGAL/Orthtree/Split_predicates.h

@@ -38,14 +38,6 @@ public:
   Maximum_number_of_inliers(std::size_t bucket_size) :
           m_bucket_size(bucket_size) {}
 
-  /*!
-    \brief returns `true` if `n` should be split, `false` otherwise.
-   */
-  template<typename Node>
-  bool operator()(const Node &n) const {
-    return (n.size() > m_bucket_size);
-  }
-
   /*!
     \brief returns `true` if `i` should be split, `false` otherwise.
    */
@@ -73,14 +65,6 @@ public:
    */
   Maximum_depth(std::size_t max_depth) : m_max_depth(max_depth) {}
 
-  /*!
-    \brief returns `true` if `n` should be split, `false` otherwise.
-   */
-  template<typename Node>
-  bool operator()(const Node &n) const {
-    return n.depth() < m_max_depth;
-  }
-
   /*!
     \brief returns `true` if `i` should be split, `false` otherwise.
    */
@@ -117,16 +101,6 @@ public:
   Maximum_depth_and_maximum_number_of_inliers(std::size_t max_depth, std::size_t bucket_size) :
           m_max_depth(max_depth), m_bucket_size(bucket_size) {}
 
-  /*!
-    \brief returns `true` if `n` should be split, `false` otherwise.
-   */
-  template<typename Node>
-  bool operator()(const Node &n) const {
-    std::size_t num_points = n.size();
-    std::size_t depth = n.depth();
-    return (num_points > m_bucket_size && depth < m_max_depth);
-  }
-
   /*!
     \brief returns `true` if `i` should be split, `false` otherwise.
    */

Orthtree/include/CGAL/Orthtree/Traversal_iterator.h

@@ -27,83 +27,12 @@ namespace CGAL {
 /*!
  * \ingroup PkgOrthtreeClasses
  *
- * \brief
+ * \brief Wraps a traversal definition to produce an iterator which traverses the tree when incremented.
  *
  * \todo
  *
- * \tparam Value
+ * \tparam Tree The orthtree type to iterate over
  */
-template <class Tree>
-class Traversal_iterator
-  : public boost::iterator_facade<Traversal_iterator<Tree>, const typename Tree::Node, boost::forward_traversal_tag> {
-public:
-
-  /// \name Types
-  /// @{
-
-  /*!
-   * \brief
-   *
-   * \todo
-   */
-  typedef std::function<boost::optional<std::size_t>(const Tree&, std::size_t)> Traversal_function;
-
-  typedef typename Tree::Node_index Node_index;
-
-  /// @}
-
-public:
-
-  /// \name Creation
-  /// @{
-
-  /*!
-   * \brief Default constructor, creates an end sentinel
-   *
-   * \todo
-   */
-  Traversal_iterator() : m_next() {}
-
-  /*!
-   * \brief
-   *
-   * \todo
-   *
-   * \param tree
-   * \param first
-   * \param next
-   */
-  Traversal_iterator(const Tree& tree, Node_index first, const Traversal_function& next) :
-    m_tree(&tree), m_index(first), m_next(next) {}
-
-  /// @}
-
-private:
-
-  friend class boost::iterator_core_access;
-
-  bool equal(Traversal_iterator<Tree> const& other) const {
-    return m_index == other.m_index;
-  }
-
-  void increment() {
-    // invoking increment on the sentinel is undefined behavior
-    m_index = m_next(*m_tree, m_index.get());
-  }
-
-  const typename Tree::Node& dereference() const {
-    return (*m_tree)[m_index.get()];
-  }
-
-private:
-
-  Traversal_function m_next;
-
-  boost::optional<std::size_t> m_index = {};
-  const Tree* m_tree = nullptr;
-
-};
-
 template <class Tree>
 class Index_traversal_iterator : public boost::iterator_facade<
   Index_traversal_iterator<Tree>,

Orthtree/include/CGAL/Orthtree/Traversals.h

@@ -43,27 +43,16 @@ template <typename Tree>
 struct Preorder_traversal {
 private:
 
-  using Node = typename Tree::Node;
-
   const Tree& m_orthtree;
 
 public:
 
   Preorder_traversal(const Tree& orthtree) : m_orthtree(orthtree) {}
 
-  const Node* first() const {
-    return &m_orthtree[m_orthtree.root()];
-  }
-
   typename Tree::Node_index first_index() const {
     return m_orthtree.root();
   }
 
-  const Node* next(const Node* n) const {
-    if (n == nullptr || !next_index(m_orthtree.index(*n))) return nullptr;
-    return &m_orthtree[*next_index(m_orthtree.index(*n))];
-  }
-
   std::optional<typename Tree::Node_index> next_index(typename Tree::Node_index n) const {
 
     if (m_orthtree.is_leaf(n)) {
@@ -94,30 +83,14 @@ template <typename Tree>
 struct Postorder_traversal {
 private:
 
-  using Node = typename Tree::Node;
-
   const Tree& m_orthtree;
 
 public:
 
   Postorder_traversal(const Tree& orthtree) : m_orthtree(orthtree) {}
 
-  const Node* first() const {
-    return deepest_first_child(m_orthtree, m_orthtree.root());
-  }
-
   typename Tree::Node_index first_index() const {
-    return *m_orthtree.index(first());
+    return m_orthtree.deepest_first_child(m_orthtree.root());
-  }
-
-  const Node* next(const Node* n) const {
-
-    auto next = deepest_first_child(m_orthtree, next_sibling(m_orthtree, n));
-
-    if (!next)
-      next = n->parent();
-
-    return next;
   }
 
   std::optional<typename Tree::Node_index> next_index(typename Tree::Node_index n) const {
@@ -137,30 +110,14 @@ template <typename Tree>
 struct Leaves_traversal {
 private:
 
-  using Node = typename Tree::Node;
-
   const Tree& m_orthtree;
 
 public:
 
   Leaves_traversal(const Tree& orthtree) : m_orthtree(orthtree) {}
 
-  const Node* first() const {
-    return m_orthtree.deepest_first_child(&m_orthtree.root());
-  }
-
   typename Tree::Node_index first_index() const {
-    return m_orthtree.deepest_first_child(0);
+    return m_orthtree.deepest_first_child(m_orthtree.root());
-  }
-
-  const Node* next(const Node* n) const {
-
-    auto next = m_orthtree.deepest_first_child(m_orthtree.next_sibling(n));
-
-    if (!next)
-      next = m_orthtree.deepest_first_child(m_orthtree.next_sibling_up(n));
-
-    return next;
   }
 
   std::optional<typename Tree::Node_index> next_index(typename Tree::Node_index n) const {
@@ -188,8 +145,6 @@ template <typename Tree>
 struct Level_traversal {
 private:
 
-  using Node = typename Tree::Node;
-
   const Tree& m_orthtree;
   const std::size_t m_depth;
 
@@ -200,34 +155,11 @@ public:
   */
   Level_traversal(const Tree& orthtree, std::size_t depth) : m_orthtree(orthtree), m_depth(depth) {}
 
-  template <typename Node>
-  const Node* first() const {
-    return m_orthtree.first_child_at_depth(m_orthtree.root(), m_depth);
-  }
-
   typename Tree::Node_index first_index() const {
     // assumes the tree has at least one child at m_depth
     return *m_orthtree.first_child_at_depth(m_orthtree.root(), m_depth);
   }
 
-  template <typename Node>
-  const Node* next(const Node* n) const {
-    const Node* next = m_orthtree.next_sibling(n);
-
-    if (!next) {
-      const Node* up = n;
-      do {
-        up = m_orthtree.next_sibling_up(up);
-        if (!up)
-          return nullptr;
-
-        next = m_orthtree.first_child_at_depth(up, m_depth);
-      } while (!next);
-    }
-
-    return next;
-  }
-
   std::optional<typename Tree::Node_index> next_index(typename Tree::Node_index n) const {
 
     auto next = m_orthtree.next_sibling(n);

Orthtree/test/Orthtree/test_node_adjacent.cpp

@@ -53,11 +53,11 @@ int main(void) {
   auto left_top_back = octree.node(Traits::LEFT_TOP_BACK);
 
   assert(octree.node(Traits::RIGHT_TOP_BACK) ==
-         octree.adjacent_node(left_top_back, Traits::RIGHT).get());
+         *octree.adjacent_node(left_top_back, Traits::RIGHT));
   assert(octree.node(Traits::LEFT_BOTTOM_BACK) ==
-         octree.adjacent_node(left_top_back, Traits::DOWN).get());
+         *octree.adjacent_node(left_top_back, Traits::DOWN));
   assert(octree.node(Traits::LEFT_TOP_FRONT) ==
-         octree.adjacent_node(left_top_back, Traits::FRONT));
+         *octree.adjacent_node(left_top_back, Traits::FRONT));
   assert(!octree.adjacent_node(left_top_back, Traits::LEFT));
   assert(!octree.adjacent_node(left_top_back, Traits::UP));
   assert(!octree.adjacent_node(left_top_back, Traits::BACK));

Orthtree/test/Orthtree/test_octree_grade.cpp

@@ -30,7 +30,7 @@ std::size_t count_jumps(Octree& octree) {
       if (!adjacent_node)
         continue;
 
-      if ((octree.depth(node) - octree.depth(adjacent_node.get())) > 1)
+      if ((octree.depth(node) - octree.depth(*adjacent_node)) > 1)
         jumps++;
     }
   }