Merge pull request #2332 from frozar/octree_depth_first_iterator_order

Reverse octree's depth first iterator order
PointCloudLibrary · Oct 8, 2018 · 1490739 · 1490739
2 parents 0fdb1c2 + c38f4af
commit 1490739
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 20 deletions.
diff --git a/octree/include/pcl/octree/impl/octree_iterator.hpp b/octree/include/pcl/octree/impl/octree_iterator.hpp
@@ -133,18 +133,16 @@ namespace pcl
         if ( (this->max_octree_depth_>=stack_entry.depth_) &&
              (stack_entry.node_->getNodeType () == BRANCH_NODE) )
         {
-          unsigned char child_idx;
-
           // current node is a branch node
           BranchNode* current_branch =
               static_cast<BranchNode*> (stack_entry.node_);
 
           // add all children to stack
-          for (child_idx = 0; child_idx < 8; ++child_idx)
+          for (int8_t i = 7; i >= 0; --i)
           {
+            const unsigned char child_idx = (unsigned char) i;
 
             // if child exist
-
             if (this->octree_->branchHasChild(*current_branch, child_idx))
             {
               // add child to stack

diff --git a/test/octree/test_octree_iterator.cpp b/test/octree/test_octree_iterator.cpp
@@ -884,25 +884,24 @@ struct OctreeBaseWalkThroughIteratorsTest : public testing::Test
 
 TEST_F (OctreeBaseWalkThroughIteratorsTest, LeafNodeDepthFirstIterator)
 {
-  // Depth first iterator seems to have a reverse order walk through: should be fixed
   OctreeT::LeafNodeDepthFirstIterator it = oct_a_.leaf_depth_begin ();
 
-  EXPECT_EQ (it.getCurrentOctreeKey (), OctreeKey (2u, 2u, 0u)); // depth: 2
-  EXPECT_EQ (it.getCurrentOctreeDepth (), 2u);
-  EXPECT_TRUE (it.isLeafNode ());
-  ++it;
-  EXPECT_EQ (it.getCurrentOctreeKey (), OctreeKey (1u, 0u, 0u)); // depth: 1
+  EXPECT_EQ (it.getCurrentOctreeKey (), OctreeKey (0u, 0u, 0u)); // depth: 1
   EXPECT_EQ (it.getCurrentOctreeDepth (), 1u);
   EXPECT_TRUE (it.isLeafNode ());
   ++it;
   EXPECT_EQ (it.getCurrentOctreeKey (), OctreeKey (0u, 2u, 0u)); // depth: 2
   EXPECT_EQ (it.getCurrentOctreeDepth (), 2u);
   EXPECT_TRUE (it.isLeafNode ());
   ++it;
-  EXPECT_EQ (it.getCurrentOctreeKey (), OctreeKey (0u, 0u, 0u)); // depth: 1
+  EXPECT_EQ (it.getCurrentOctreeKey (), OctreeKey (1u, 0u, 0u)); // depth: 1
   EXPECT_EQ (it.getCurrentOctreeDepth (), 1u);
   EXPECT_TRUE (it.isLeafNode ());
   ++it;
+  EXPECT_EQ (it.getCurrentOctreeKey (), OctreeKey (2u, 2u, 0u)); // depth: 2
+  EXPECT_EQ (it.getCurrentOctreeDepth (), 2u);
+  EXPECT_TRUE (it.isLeafNode ());
+  ++it;
   EXPECT_EQ (it, oct_a_.leaf_depth_end ());
 }
 
@@ -931,37 +930,36 @@ TEST_F (OctreeBaseWalkThroughIteratorsTest, LeafNodeBreadthFirstIterator)
 
 TEST_F (OctreeBaseWalkThroughIteratorsTest, DepthFirstIterator)
 {
-  // Depth first iterator seems to have a reverse order walk through: should be fixed
   OctreeT::DepthFirstIterator it = oct_a_.depth_begin ();
 
   EXPECT_EQ (it.getCurrentOctreeKey (), OctreeKey (0u, 0u, 0u)); // depth: 0
   EXPECT_EQ (it.getCurrentOctreeDepth (), 0u);
   EXPECT_TRUE (it.isBranchNode ());
   ++it;
-  EXPECT_EQ (it.getCurrentOctreeKey (), OctreeKey (1u, 1u, 0u)); // depth: 1
+  EXPECT_EQ (it.getCurrentOctreeKey (), OctreeKey (0u, 0u, 0u)); // depth: 1
+  EXPECT_EQ (it.getCurrentOctreeDepth (), 1u);
+  EXPECT_TRUE (it.isLeafNode ());
+  ++it;
+  EXPECT_EQ (it.getCurrentOctreeKey (), OctreeKey (0u, 1u, 0u)); // depth: 1
   EXPECT_EQ (it.getCurrentOctreeDepth (), 1u);
   EXPECT_TRUE (it.isBranchNode ());
   ++it;
-  EXPECT_EQ (it.getCurrentOctreeKey (), OctreeKey (2u, 2u, 0u)); // depth: 2
+  EXPECT_EQ (it.getCurrentOctreeKey (), OctreeKey (0u, 2u, 0u)); // depth: 2
   EXPECT_EQ (it.getCurrentOctreeDepth (), 2u);
   EXPECT_TRUE (it.isLeafNode ());
   ++it;
   EXPECT_EQ (it.getCurrentOctreeKey (), OctreeKey (1u, 0u, 0u)); // depth: 1
   EXPECT_EQ (it.getCurrentOctreeDepth (), 1u);
   EXPECT_TRUE (it.isLeafNode ());
   ++it;
-  EXPECT_EQ (it.getCurrentOctreeKey (), OctreeKey (0u, 1u, 0u)); // depth: 1
+  EXPECT_EQ (it.getCurrentOctreeKey (), OctreeKey (1u, 1u, 0u)); // depth: 1
   EXPECT_EQ (it.getCurrentOctreeDepth (), 1u);
   EXPECT_TRUE (it.isBranchNode ());
   ++it;
-  EXPECT_EQ (it.getCurrentOctreeKey (), OctreeKey (0u, 2u, 0u)); // depth: 2
+  EXPECT_EQ (it.getCurrentOctreeKey (), OctreeKey (2u, 2u, 0u)); // depth: 2
   EXPECT_EQ (it.getCurrentOctreeDepth (), 2u);
   EXPECT_TRUE (it.isLeafNode ());
   ++it;
-  EXPECT_EQ (it.getCurrentOctreeKey (), OctreeKey (0u, 0u, 0u)); // depth: 1
-  EXPECT_EQ (it.getCurrentOctreeDepth (), 1u);
-  EXPECT_TRUE (it.isLeafNode ());
-  ++it;
   EXPECT_EQ (it, oct_a_.depth_end ());
 }