From 0747b09e23d967b241ed281355fa497ecf3952b7 Mon Sep 17 00:00:00 2001 From: Simon Giraudot Date: Mon, 26 Oct 2020 15:56:42 +0100 Subject: [PATCH] Make node trivially copiable and simplify APIs --- .../Orthtree/Octree_traversal_manual.cpp | 6 +- Orthtree/include/CGAL/Orthtree.h | 86 ++--- Orthtree/include/CGAL/Orthtree/Node.h | 356 ++++++++---------- Orthtree/include/CGAL/Orthtree/Traversal.h | 79 ++-- .../CGAL/Orthtree/Traversal_iterator.h | 10 +- Orthtree/include/CGAL/Orthtree_traits_d.h | 5 +- 6 files changed, 252 insertions(+), 290 deletions(-) diff --git a/Orthtree/examples/Orthtree/Octree_traversal_manual.cpp b/Orthtree/examples/Orthtree/Octree_traversal_manual.cpp index 574793b0f26..df6ebfc92b7 100644 --- a/Orthtree/examples/Orthtree/Octree_traversal_manual.cpp +++ b/Orthtree/examples/Orthtree/Octree_traversal_manual.cpp @@ -53,15 +53,15 @@ int main(int argc, char **argv) { std::cout << std::endl; // Retrieve one of the deeper children - const Octree::Node &cur = octree[3][2]; + Octree::Node cur = octree[3][2]; std::cout << "Navigation relative to a child node" << std::endl; std::cout << "~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~" << std::endl; std::cout << "the third child of the fourth child: " << std::endl; std::cout << cur << std::endl; std::cout << "the third child: " << std::endl; - std::cout << *cur.parent() << std::endl; + std::cout << cur.parent() << std::endl; std::cout << "the next sibling of the third child of the fourth child: " << std::endl; - std::cout << (*cur.parent())[cur.index().to_ulong() + 1] << std::endl; + std::cout << cur.parent()[cur.index().to_ulong() + 1] << std::endl; return EXIT_SUCCESS; } diff --git a/Orthtree/include/CGAL/Orthtree.h b/Orthtree/include/CGAL/Orthtree.h index 87da0b78016..de37466cb55 100644 --- a/Orthtree/include/CGAL/Orthtree.h +++ b/Orthtree/include/CGAL/Orthtree.h @@ -18,6 +18,7 @@ #include #include #include +#include #include @@ -107,7 +108,7 @@ public: /*! * \brief A predicate that determines whether a node needs to be split when refining a tree */ - typedef std::function Split_predicate; + typedef std::function Split_predicate; /*! * \brief A model of `ConstRange` whose value type is `Node`. @@ -115,7 +116,7 @@ public: #ifdef DOXYGEN_RUNNING typedef unspecified_type Node_range; #else - typedef boost::iterator_range> Node_range; + typedef boost::iterator_range > Node_range; #endif /// \cond SKIP_IN_MANUAL @@ -123,7 +124,7 @@ public: /*! * \brief A function that determines the next node in a traversal given the current one */ - typedef std::function Node_traversal_method_const; + typedef std::function Node_traversal_method_const; /// \endcond @@ -176,6 +177,7 @@ public: : m_traits (traits) , m_range (point_range) , m_point_map (point_map) + , m_root(Node(), 0) { Array bbox_min; for (FT& f : bbox_min) @@ -207,7 +209,7 @@ public: for (std::size_t i = 0 ; i < Dimension::value; ++ i) { bbox_min[i] = bbox_centroid[i] - max_length; - bbox_max[i] = bbox_centroid[i] - max_length; + bbox_max[i] = bbox_centroid[i] + max_length; } Construct_point_d_from_array construct_point_d_from_array @@ -245,32 +247,32 @@ public: m_side_per_depth.resize(1); // Initialize a queue of nodes that need to be refined - std::queue todo; - todo.push(&m_root); + std::queue todo; + todo.push(m_root); // Process items in the queue until it's consumed fully while (!todo.empty()) { // Get the next element - auto current = todo.front(); + Node current = todo.front(); todo.pop(); // Check if this node needs to be processed - if (split_predicate(*current)) { + if (split_predicate(current)) { // Check if we've reached a new max depth - if (current->depth() == max_depth_reached()) { + if (current.depth() == max_depth_reached()) { // Update the side length map m_side_per_depth.push_back(*(m_side_per_depth.end() - 1) / 2); } // Split the node, redistributing its points to its children - split((*current)); + split(current); // Process each of its children for (int i = 0; i < Degree::value; ++i) - todo.push(&(*current)[i]); + todo.push(current[i]); } } @@ -299,52 +301,52 @@ public: void grade() { // Collect all the leaf nodes - std::queue leaf_nodes; - for (auto &leaf : traverse(Orthtrees::Traversal::Leaves())) { + std::queue leaf_nodes; + for (Node leaf : traverse(Orthtrees::Traversal::Leaves())) { // TODO: I'd like to find a better (safer) way of doing this - leaf_nodes.push(const_cast(&leaf)); + leaf_nodes.push(leaf); } // Iterate over the nodes while (!leaf_nodes.empty()) { // Get the next node - Node *node = leaf_nodes.front(); + Node node = leaf_nodes.front(); leaf_nodes.pop(); // Skip this node if it isn't a leaf anymore - if (!node->is_leaf()) + if (!node.is_leaf()) continue; // Iterate over each of the neighbors for (int direction = 0; direction < 6; ++direction) { // Get the neighbor - auto *neighbor = node->adjacent_node(direction); + Node neighbor = node.adjacent_node(direction); // If it doesn't exist, skip it - if (!neighbor) + if (neighbor.is_null()) continue; // Skip if this neighbor is a direct sibling (it's guaranteed to be the same depth) // TODO: This check might be redundant, if it doesn't affect performance maybe I could remove it - if (neighbor->parent() == node->parent()) + if (neighbor.parent() == node.parent()) continue; // If it's already been split, skip it - if (!neighbor->is_leaf()) + if (!neighbor.is_leaf()) continue; // Check if the neighbor breaks our grading rule // TODO: could the rule be parametrized? - if ((node->depth() - neighbor->depth()) > 1) { + if ((node.depth() - neighbor.depth()) > 1) { // Split the neighbor - split(*neighbor); + split(neighbor); // Add newly created children to the queue for (int i = 0; i < Degree::value; ++i) { - leaf_nodes.push(&(*neighbor)[i]); + leaf_nodes.push(neighbor[i]); } } } @@ -357,12 +359,9 @@ public: /// @{ /*! - \brief provides read-only access to the root node, and by - extension the rest of the tree. - - \return a const reference to the root node of the tree. + \brief returns the root node. */ - const Node &root() const { return m_root; } + Node root() const { return m_root; } /*! \brief convenience function to access the child nodes of the root @@ -373,7 +372,7 @@ public: \param index the index of the child node. \return a reference to the node. */ - const Node &operator[](std::size_t index) const { return m_root[index]; } + Node operator[](std::size_t index) const { return m_root[index]; } /*! \brief Finds the deepest level reached by a leaf node in this tree. @@ -394,13 +393,13 @@ public: template Node_range traverse(const Traversal &traversal = Traversal()) const { - const Node *first = traversal.first(&m_root); + Node first = traversal.first(m_root); Node_traversal_method_const next = std::bind(&Traversal::template next, traversal, _1); - return boost::make_iterator_range(Traversal_iterator(first, next), - Traversal_iterator()); + return boost::make_iterator_range(Traversal_iterator(first, next), + Traversal_iterator()); } /*! @@ -444,19 +443,19 @@ public: \param point query point. \return the node which contains the point. */ - const Node& locate(const Point &point) const { + Node locate(const Point &point) const { // Make sure the point is enclosed by the orthtree CGAL_precondition (CGAL::do_intersect(point, bbox(m_root))); // Start at the root node - auto *node_for_point = &m_root; + auto node_for_point = m_root; // Descend the tree until reaching a leaf node while (!node_for_point->is_leaf()) { // Find the point to split around - Point center = barycenter(*node_for_point); + Point center = barycenter(node_for_point); // Find the index of the correct sub-node typename Node::Index index; @@ -465,11 +464,11 @@ public: index[dimension ++] = (get<0>(r) < get<1>(r)); // Find the correct sub-node of the current node - node_for_point = &(*node_for_point)[index.to_ulong()]; + node_for_point = node_for_point[index.to_ulong()]; } // Return the result - return *node_for_point; + return node_for_point; } /*! @@ -560,7 +559,7 @@ public: // TODO: Document this // TODO: Could this method name be reduced to just "center" ? - Point barycenter(const Node &node) const { + Point barycenter(const Node& node) const { // Determine the side length of this node FT size = m_side_per_depth[node.depth()]; @@ -570,9 +569,11 @@ public: std::size_t i = 0; for (const FT& f : cartesian_range(m_bbox_min)) { + std::cerr << node.location()[i] << " "; bary[i] = node.location()[i] * size + (size / 2.0) + f; ++ i; } + std::cerr << std::endl; // Convert that location into a point Construct_point_d_from_array construct_point_d_from_array @@ -615,7 +616,7 @@ private: // functions : } - void split(Node &node) { + void split(Node& node) { // Make sure the node hasn't already been split assert(node.is_leaf()); @@ -625,6 +626,7 @@ private: // functions : // Find the point to around which the node is split Point center = barycenter(node); + std::cerr << center << std::endl; // Add the node's points to its children reassign_points(node, node.points().begin(), node.points().end(), center); @@ -709,7 +711,7 @@ private: // functions : // Fill the list with child nodes for (int index = 0; index < Degree::value; ++index) { - auto &child_node = node[index]; + Node child_node = node[index]; // Add a child to the list, with its distance children_with_distances.push_back( @@ -725,7 +727,7 @@ private: // functions : // Loop over the children for (auto child_with_distance : children_with_distances) { - auto &child_node = node[child_with_distance.index.to_ulong()]; + Node child_node = node[child_with_distance.index.to_ulong()]; // Check whether the bounding box of the child intersects with the search bounds if (do_intersect(child_node, search_bounds)) { @@ -746,7 +748,7 @@ private: // functions : // if this node is a leaf, than it's considered an intersecting node if (node.is_leaf()) { - *output++ = &node; + *output++ = node; return output; } diff --git a/Orthtree/include/CGAL/Orthtree/Node.h b/Orthtree/include/CGAL/Orthtree/Node.h index e45fbf1379b..0da00c3913b 100644 --- a/Orthtree/include/CGAL/Orthtree/Node.h +++ b/Orthtree/include/CGAL/Orthtree/Node.h @@ -14,7 +14,6 @@ #include -#include #include @@ -27,14 +26,15 @@ namespace CGAL { /*! - * \brief represents a single node of the tree. Alternatively referred to as a cell, orthant, or subtree - * - * \details The role of the node isn't fully stable yet - * - * \tparam Point_index is the datatype the node will contain + \brief represents a single node of the tree. Alternatively referred to as a cell, orthant, or subtree + + \details The role of the node isn't fully stable yet + + \tparam Point_index is the datatype the node will contain */ -template -class Orthtree::Node { +template +class Orthtree::Node +{ public: @@ -79,69 +79,26 @@ public: */ typedef boost::iterator_range Point_range; - // TODO: Should I use enum classes? - - /*! - * \brief the index of a node relative to its parent (a position defined by the corners of a cube) - * - * Corners are mapped to numbers as 3-bit integers, in "zyx" order. - * - * For example: - * > right-top-back --> x=1, y=1, z=0 --> zyx = 011 --> 3 - * - * The following diagram may be a useful reference: - * - * 6 7 - * +--------+ - * /| /| y+ - * / | / | * z+ - * 2 +--------+ 3| | * - * | | | | |/ - * | +-----|--+ +-----* x+ - * | / 4 | / 5 - * |/ |/ - * +--------+ - * 0 1 - * - * This lookup table may also be helpful: - * - * | Child | bitset | number | Enum | - * | --------------------- | ------ | ------ | --------------------- | - * | left, bottom, back | 000 | 0 | LEFT_BOTTOM_BACK | - * | right, bottom, back | 001 | 1 | RIGHT_BOTTOM_BACK | - * | left, top, back | 010 | 2 | LEFT_TOP_BACK | - * | right, top, back | 011 | 3 | RIGHT_TOP_BACK | - * | left, bottom, front | 100 | 4 | LEFT_BOTTOM_FRONT | - * | right, bottom, front | 101 | 5 | RIGHT_BOTTOM_FRONT | - * | left, top, front | 110 | 6 | LEFT_TOP_FRONT | - * | right, top, front | 111 | 7 | RIGHT_TOP_FRONT | - */ - enum Child { - LEFT_BOTTOM_BACK, - RIGHT_BOTTOM_BACK, - LEFT_TOP_BACK, - RIGHT_TOP_BACK, - LEFT_BOTTOM_FRONT, - RIGHT_BOTTOM_FRONT, - LEFT_TOP_FRONT, - RIGHT_TOP_FRONT - }; - typedef typename Traits::Adjacency Adjacency; /// @} private: - Point_range m_points; + // make Node trivially copiabled + struct Data + { + Point_range points; + Self parent; + std::uint8_t depth; + Int_location location; + std::unique_ptr children; - const Self *m_parent; + Data (Self parent) + : parent (parent), depth (0) { } + }; - std::uint8_t m_depth; - - Int_location m_location; - - std::unique_ptr m_children; + std::shared_ptr m_data; public: @@ -150,7 +107,10 @@ public: /// \name Construction /// @{ - /*! + // Default creates null node + Node() { } + + /*! \brief creates a new node, optionally as the child of a parent If no parent is provided, the node created is assumed to be the root of a tree. @@ -162,20 +122,21 @@ public: \param parent A reference to the node containing this one \param index This node's relationship to its parent - */ - explicit Node(Self *parent = nullptr, Index index = 0) : m_parent(parent), m_depth(0) { + */ + explicit Node(Self parent, Index index) + : m_data (new Data(parent)) { - if (parent) { + if (!parent.is_null()) { - m_depth = parent->m_depth + 1; + m_data->depth = parent.m_data->depth + 1; for (int i = 0; i < Dimension::value; i++) - m_location[i] = (2 * parent->m_location[i]) + index[i]; + m_data->location[i] = (2 * parent.m_data->location[i]) + index[i]; } else for (int i = 0; i < Dimension::value; i++) - m_location[i] = 0; + m_data->location[i] = 0; } /// @} @@ -196,10 +157,11 @@ public: assert(is_leaf()); - m_children = std::make_unique(); + m_data->children = std::make_unique(); for (int index = 0; index < Degree::value; index++) { - (*m_children)[index] = std::move(Self(this, {Index(index)})); + (*m_data->children)[index] = std::move(Self(*this, {Index(index)})); + } } @@ -211,7 +173,7 @@ public: */ void unsplit() { - m_children.reset(); + m_data->children.reset(); } /// @} @@ -223,46 +185,31 @@ public: /*! \brief returns this node's parent. - \pre `!is_root()` - */ - const Self* parent() const + \pre `!is_null()` + */ + Self parent() const { - CGAL_precondition (!is_root()); - return m_parent; + CGAL_precondition (!is_null()); + return m_data->parent; } /*! \brief returns the nth child fo this node. + \pre `!is_null()` \pre `!is_leaf()` \pre `0 <= index && index < Degree::value` The operator can be chained. For example, `n[5][2][3]` returns the third child of the second child of the fifth child of a node `n`. - */ - Self& operator[] (std::size_t index) { + */ + Self operator[](std::size_t index) const { + CGAL_precondition (!is_null()); CGAL_precondition (!is_leaf()); CGAL_precondition (0 <= index && index < Degree::value); - return (*m_children)[index]; - } - - /*! - \brief returns the nth child fo this node. - - \pre `!is_leaf()` - \pre `0 <= index && index < Degree::value` - - The operator can be chained. For example, `n[5][2][3]` returns the - third child of the second child of the fifth child of a node `n`. - */ - const Self& operator[](std::size_t index) const { - - CGAL_precondition (!is_leaf()); - CGAL_precondition (0 <= index && index < Degree::value); - - return (*m_children)[index]; + return (*m_data->children)[index]; } /// @} @@ -272,22 +219,31 @@ public: /*! \brief returns this node's depth. - */ - std::uint8_t depth() const { return m_depth; } + \pre `!is_null()` + */ + std::uint8_t depth() const + { + CGAL_precondition (!is_null()); + return m_data->depth; + } /*! \brief returns this node's location. + \pre `!is_null()` */ - Int_location location() const { - - return m_location; + Int_location location() const + { + CGAL_precondition (!is_null()); + return m_data->location; } /*! \brief returns this node's index in relation to its parent. + \pre `!is_null()` */ Index index() const { + CGAL_precondition (!is_null()); // TODO: There must be a better way of doing this! Index result; @@ -298,73 +254,92 @@ public: return result; } + /*! + \brief returns `true` if the node is null, `false` otherwise. + */ + bool is_null() const { return (m_data == nullptr); } + /*! \brief returns `true` if the node has no parent, `false` otherwise. - */ - bool is_root() const { return (!m_parent); } + \pre `!is_null()` + */ + bool is_root() const + { + CGAL_precondition(!is_null()); + return m_data->parent.is_null(); + } /*! \brief returns `true` if the node has no children, `false` otherwise. - */ - bool is_leaf() const { return (!m_children); } + \pre `!is_null()` + */ + bool is_leaf() const + { + CGAL_precondition(!is_null()); + return (!m_data->children); + } /*! - * \brief find the directly adjacent node in a specific direction - * - * Adjacent nodes are found according to several properties: - * - Adjacent nodes may be larger than the seek node, but never smaller - * - A node can have no more than 6 different adjacent nodes (left, right, up, down, front, back) - * - A node is free to have fewer than 6 adjacent nodes - * (e.g. edge nodes have no neighbors in some directions, the root node has none at all). - * - Adjacent nodes are not required to be leaf nodes - * - * - * Here's a diagram demonstrating the concept for a quadtree. - * Because it's in 2d space, the seek node has only four neighbors (up, down, left, right) - * - * +---------------+---------------+ - * | | | - * | | | - * | | | - * | A | | - * | | | - * | | | - * | | | - * +-------+-------+---+---+-------+ - * | | | | | | - * | A | (S) +---A---+ | - * | | | | | | - * +---+---+-------+---+---+-------+ - * | | | | | | - * +---+---+ A | | | - * | | | | | | - * +---+---+-------+-------+-------+ - * - * (S) : Seek node - * A : Adjacent node - * - * Note how the top adjacent node is larger than the seek node. - * The right adjacent node is the same size, even though it contains further subdivisions. - * - * This implementation returns a pointer to the adjacent node if it's found. - * If there is no adjacent node in that direction, it returns nullptr. - * - * \todo explain how direction is encoded - * - * \param direction which way to find the adjacent node relative to this one - * \return a pointer to the adjacent node if it exists - */ - const Self *adjacent_node(std::bitset direction) const { + \brief find the directly adjacent node in a specific direction + + \pre `!is_null()` + \pre `direction.to_ulong < 2 * Dimension::value` + + Adjacent nodes are found according to several properties: + - Adjacent nodes may be larger than the seek node, but never smaller + - A node can have no more than 6 different adjacent nodes (left, right, up, down, front, back) + - A node is free to have fewer than 6 adjacent nodes + (e.g. edge nodes have no neighbors in some directions, the root node has none at all). + - Adjacent nodes are not required to be leaf nodes + + Here's a diagram demonstrating the concept for a quadtree. + Because it's in 2d space, the seek node has only four neighbors (up, down, left, right) + + +---------------+---------------+ + | | | + | | | + | | | + | A | | + | | | + | | | + | | | + +-------+-------+---+---+-------+ + | | | | | | + | A | (S) +---A---+ | + | | | | | | + +---+---+-------+---+---+-------+ + | | | | | | + +---+---+ A | | | + | | | | | | + +---+---+-------+-------+-------+ + + (S) : Seek node + A : Adjacent node + + Note how the top adjacent node is larger than the seek node. + The right adjacent node is the same size, even though it contains further subdivisions. + + This implementation returns a pointer to the adjacent node if it's found. + If there is no adjacent node in that direction, it returns nullptr. + + \todo explain how direction is encoded + + \param direction which way to find the adjacent node relative to this one + \return a pointer to the adjacent node if it exists + */ + Self adjacent_node (std::bitset direction) const + { + CGAL_precondition(!is_null()); // Direction: LEFT RIGHT DOWN UP BACK FRONT // direction: 000 001 010 011 100 101 - // Nodes only have up to 6 different adjacent nodes (since cubes have 6 sides) - assert(direction.to_ulong() < 6); + // Nodes only have up to 2*dim different adjacent nodes (since cubes have 6 sides) + CGAL_precondition(direction.to_ulong() < Dimension::value * 2); // The root node has no adjacent nodes! if (is_root()) - return nullptr; + return Self(); // The least significant bit indicates the sign (which side of the node) bool sign = direction[0]; @@ -382,43 +357,29 @@ public: if (index()[dimension] != sign) { // This means the adjacent node is a direct sibling, the offset can be applied easily! - return &(*parent())[index().to_ulong() + offset]; + return parent()[index().to_ulong() + offset]; } // Find the parent's neighbor in that direction if it exists - auto *adjacent_node_of_parent = parent()->adjacent_node(direction); + Self adjacent_node_of_parent = parent().adjacent_node(direction); // If the parent has no neighbor, then this node doesn't have one - if (!adjacent_node_of_parent) - return nullptr; + if (adjacent_node_of_parent.is_null()) + return Node(); // If the parent's adjacent node has no children, then it's this node's adjacent node - if (adjacent_node_of_parent->is_leaf()) + if (adjacent_node_of_parent.is_leaf()) return adjacent_node_of_parent; // Return the nearest node of the parent by subtracting the offset instead of adding - return &(*adjacent_node_of_parent)[index().to_ulong() - offset]; + return adjacent_node_of_parent[index().to_ulong() - offset]; } /*! * \brief equivalent to adjacent_node, with a Direction rather than a bitset */ - const Self *adjacent_node(Adjacency adjacency) const { - return adjacent_node(std::bitset(static_cast(adjacency))); - } - - /*! - * \brief equivalent to adjacent_node, except non-const - */ - Self *adjacent_node(std::bitset direction) { - return const_cast(const_cast(this)->adjacent_node(direction)); - } - - /*! - * \brief equivalent to adjacent_node, with a Direction rather than a bitset and non-const - */ - Self *adjacent_node(Adjacency adjacency) { + Self adjacent_node(Adjacency adjacency) const { return adjacent_node(std::bitset(static_cast(adjacency))); } @@ -431,44 +392,44 @@ public: * \brief access to the content held by this node * \return a reference to the collection of point indices */ - Point_range &points() { return m_points; } + Point_range &points() { return m_data->points; } /*! * \brief access to the points via iterator * \return the iterator at the start of the collection of point indices held by this node */ - typename Point_range::iterator begin() { return m_points.begin(); } + typename Point_range::iterator begin() { return m_data->points.begin(); } /*! - * \brief access to the points via iterator - * \return the iterator at the end of the collection of point indices held by this node - */ - typename Point_range::iterator end() { return m_points.end(); } + * \brief access to the points via iterator + * \return the iterator at the end of the collection of point indices held by this node + */ + typename Point_range::iterator end() { return m_data->points.end(); } /*! * \brief read-only access to the content held by this node * \return a read-only reference to the collection of point indices */ - const Point_range &points() const { return m_points; } + const Point_range &points() const { return m_data->points; } /*! - * \brief read-only access to the points via iterator - * \return the iterator at the start of the collection of point indices held by this node - */ - typename Point_range::iterator begin() const { return m_points.begin(); } + * \brief read-only access to the points via iterator + * \return the iterator at the start of the collection of point indices held by this node + */ + typename Point_range::iterator begin() const { return m_data->points.begin(); } /*! - * \brief read-only access to the points via iterator - * \return the iterator at the end of the collection of point indices held by this node - */ - typename Point_range::iterator end() const { return m_points.end(); } + * \brief read-only access to the points via iterator + * \return the iterator at the end of the collection of point indices held by this node + */ + typename Point_range::iterator end() const { return m_data->points.end(); } /*! * \brief check whether this node contains any points * \return if this node contains no points */ bool empty() const { - return m_points.empty(); + return m_data->points.empty(); } /*! @@ -476,7 +437,7 @@ public: * \return the number of points this node owns */ std::size_t size() const { - return std::distance(m_points.begin(), m_points.end()); + return std::distance(m_data->points.begin(), m_data->points.end()); } /// @} @@ -495,9 +456,12 @@ public: bool operator==(const Self &rhs) const { // TODO: Should I compare the values they contain -// if (m_points != rhs.m_points) +// if (m_data->points != rhs.m_data->points) // return false; + if (is_null() || rhs.is_null()) + return (is_null() == rhs.is_null()); + // If one node is a leaf, and the other isn't, they're not the same if (is_leaf() != rhs.is_leaf()) return false; @@ -509,7 +473,7 @@ public: for (int i = 0; i < Degree::value; ++i) { // If any child cell is different, they're not the same - if ((*m_children)[i] != rhs[i]) + if ((*m_data->children)[i] != rhs[i]) return false; } } diff --git a/Orthtree/include/CGAL/Orthtree/Traversal.h b/Orthtree/include/CGAL/Orthtree/Traversal.h index c45dc491748..d22d44625ba 100644 --- a/Orthtree/include/CGAL/Orthtree/Traversal.h +++ b/Orthtree/include/CGAL/Orthtree/Traversal.h @@ -25,57 +25,57 @@ namespace Orthtrees { /// \cond SKIP_IN_MANUAL template -const Node* next_sibling(const Node* n) { +Node next_sibling(Node n) { // Passing null returns the first node - if (nullptr == n) - return nullptr; + if (n.is_null()) + return Node(); // If this node has no parent, it has no siblings - if (nullptr == n->parent()) - return nullptr; + if (n.parent().is_null()) + return Node(); // Find out which child this is - std::size_t index = n->index().to_ulong(); + std::size_t index = n.index().to_ulong(); constexpr static int degree = Node::Degree::value; // Return null if this is the last child if (index == degree - 1) - return nullptr; + return Node(); // Otherwise, return the next child - return &((*n->parent())[index + 1]); + return n.parent()[index + 1]; } template -const Node* next_sibling_up(const Node* n) { +Node next_sibling_up(Node n) { - if (!n) - return nullptr; + if (n.is_null()) + return Node(); - auto up = n->parent(); + Node up = n.parent(); - while (nullptr != up) { + while (!up.is_null()) { - if (nullptr != next_sibling(up)) + if (!next_sibling(up).is_null()) return next_sibling(up); - up = up->parent(); + up = up.parent(); } - return nullptr; + return Node(); } template -const Node* deepest_first_child(const Node* n) { +Node deepest_first_child(Node n) { - if (!n) - return nullptr; + if (n.is_null()) + return Node(); // Find the deepest child on the left - auto first = n; - while (!first->is_leaf()) - first = &(*first)[0]; + Node first = n; + while (!first.is_leaf()) + first = first[0]; return first; } @@ -97,7 +97,7 @@ struct Preorder { * \return */ template - const Node* first(const Node* root) const { + Node first(Node root) const { return root; } @@ -109,25 +109,20 @@ struct Preorder { * \return */ template - const Node* next(const Node* n) const { + Node next(Node n) const { - if (n->is_leaf()) { + if (n.is_leaf()) { - auto next = next_sibling(n); - - if (nullptr == next) { + Node next = next_sibling(n); + if (next.is_null()) return next_sibling_up(n); - } return next; - } else { - - // Return the first child of this node - return &(*n)[0]; } - + else // Return the first child of this node + return n[0]; } }; @@ -145,7 +140,7 @@ struct Postorder { * \return */ template - const Node* first(const Node* root) const { + Node first(Node root) const { return deepest_first_child(root); } @@ -158,12 +153,12 @@ struct Postorder { * \return */ template - const Node* next(const Node* n) const { + Node next(Node n) const { - auto next = deepest_first_child(next_sibling(n)); + Node next = deepest_first_child(next_sibling(n)); if (!next) - next = n->parent(); + next = n.parent(); return next; } @@ -183,7 +178,7 @@ struct Leaves { * \return */ template - const Node* first(const Node* root) const { + Node first(Node root) const { return deepest_first_child(root); } @@ -196,11 +191,11 @@ struct Leaves { * \return */ template - const Node* next(const Node* n) const { + Node next(Node n) const { - auto next = deepest_first_child(next_sibling(n)); + Node next = deepest_first_child(next_sibling(n)); - if (!next) + if (next.is_null()) next = deepest_first_child(next_sibling_up(n)); return next; diff --git a/Orthtree/include/CGAL/Orthtree/Traversal_iterator.h b/Orthtree/include/CGAL/Orthtree/Traversal_iterator.h index 6670b480cd6..d8a594e4ef0 100644 --- a/Orthtree/include/CGAL/Orthtree/Traversal_iterator.h +++ b/Orthtree/include/CGAL/Orthtree/Traversal_iterator.h @@ -44,7 +44,7 @@ public: * * \todo */ - typedef std::function Traversal_function; + typedef std::function Traversal_function; /// @} @@ -58,7 +58,7 @@ public: * * \todo */ - Traversal_iterator() : m_value(nullptr), m_next() {} + Traversal_iterator() : m_value(), m_next() {} /*! * \brief @@ -68,7 +68,7 @@ public: * \param first * \param next */ - Traversal_iterator(Value *first, const Traversal_function &next) : m_value(first), m_next(next) {} + Traversal_iterator(Value first, const Traversal_function &next) : m_value(first), m_next(next) {} /// @} @@ -84,12 +84,12 @@ private: } Value &dereference() const { - return *m_value; + return const_cast(m_value); } private: - Value *m_value; + Value m_value; Traversal_function m_next; }; } diff --git a/Orthtree/include/CGAL/Orthtree_traits_d.h b/Orthtree/include/CGAL/Orthtree_traits_d.h index 6a979164fc4..542f7b92ab7 100644 --- a/Orthtree/include/CGAL/Orthtree_traits_d.h +++ b/Orthtree/include/CGAL/Orthtree_traits_d.h @@ -85,7 +85,7 @@ public: { Point_d operator() (const Array& array) const { - return Point_d ( /* todo */ ); + return Point_d (array.begin(), array.end()); } }; #endif @@ -102,7 +102,8 @@ public: Bbox_d operator() (const Array& min, const Array& max) const { - return Bbox_d ( /* todo */ ); + return Bbox_d (Point_d (min.begin(), min.end()), + Point_d (max.begin(), max.end())); } }; #endif