Address feedback

Signed-off-by: Dan Bailey <[email protected]>

openvdb/openvdb/tree/InternalNode.h

@@ -472,66 +472,66 @@ class InternalNode
     // Enabling OpenVDB asserts will catch where assumptions are incorrectly invalidated.
 
     /// @brief Return the tile value at offset.
-    /// @note Use getValue() for a safer alternative.
+    /// @note Use getValue(const Coord&) for a safer alternative.
     /// @warning This method should only be used by experts seeking low-level optimizations.
     const ValueType& getValueUnsafe(Index offset) const;
     /// @brief Return the tile value and active state at offset.
-    /// @note Use getValue() for a safer alternative.
+    /// @note Use probeValue(const Coord&, ValueType&) for a safer alternative.
     /// @warning This method should only be used by experts seeking low-level optimizations.
     bool getValueUnsafe(Index offset, ValueType& value) const;
 
     /// @brief Return the child node at offset.
-    /// @note Use probeChild() for a safer alternative.
+    /// @note Use probeChild(const Coord&) for a safer alternative.
     /// @warning This method should only be used by experts seeking low-level optimizations.
     ChildNodeType* getChildUnsafe(Index offset);
     /// @brief Return the child node at offset.
-    /// @note Use probeConstChild() for a safer alternative.
+    /// @note Use probeConstChild(const Coord&) for a safer alternative.
     /// @warning This method should only be used by experts seeking low-level optimizations.
     const ChildNodeType* getConstChildUnsafe(Index offset) const;
     /// @brief Return the child node at offset.
-    /// @note Use probeChild() for a safer alternative.
+    /// @note Use probeChild(const Coord&) for a safer alternative.
     /// @warning This method should only be used by experts seeking low-level optimizations.
     const ChildNodeType* getChildUnsafe(Index offset) const;
 
     /// @brief Set the tile active state at offset but don't change its value.
-    /// @note Use setActiveState() for a safer alternative.
+    /// @note Use setActiveState(const Coord&, bool) for a safer alternative.
     /// @warning This method should only be used by experts seeking low-level optimizations.
     void setActiveStateUnsafe(Index offset, bool on);
     /// @brief Set the tile value at offset but don't change its value.
-    /// @note Use setValueOnly() for a safer alternative.
+    /// @note Use setValueOnly(const Coord&, const ValueType&) for a safer alternative.
     /// @warning This method should only be used by experts seeking low-level optimizations.
     void setValueOnlyUnsafe(Index offset, const ValueType& value);
     /// @brief Mark the tile active at offset but don't change its value.
-    /// @note Use setValueOn() for a safer alternative.
+    /// @note Use setValueOn(const Coord&) for a safer alternative.
     /// @warning This method should only be used by experts seeking low-level optimizations.
     void setValueOnUnsafe(Index offset);
     /// @brief Set the tile value at offset and mark the voxel as active.
-    /// @note Use setValueOn() for a safer alternative.
+    /// @note Use setValueOn(const Coord&, const ValueType&) for a safer alternative.
     /// @warning This method should only be used by experts seeking low-level optimizations.
     void setValueOnUnsafe(Index offset, const ValueType& value);
     /// @brief Mark the tile inactive at offset but don't change its value.
-    /// @note Use setValueOff() for a safer alternative.
+    /// @note Use setValueOff(const Coord&) for a safer alternative.
     /// @warning This method should only be used by experts seeking low-level optimizations.
     void setValueOffUnsafe(Index offset);
     /// @brief Set the tile value at offset and mark the voxel as inactive.
-    /// @note Use setValueOff() for a safer alternative.
+    /// @note Use setValueOff(const Coord&, const ValueType&) for a safer alternative.
     /// @warning This method should only be used by experts seeking low-level optimizations.
     void setValueOffUnsafe(Index offset, const ValueType& value);
 
     /// @brief Replace a tile at offset with the given child node.
-    /// @note Use addChild() for a safer alternative.
+    /// @note Use addChild(ChildNodeType*) for a safer alternative.
     /// @warning This method should only be used by experts seeking low-level optimizations.
     void setChildUnsafe(Index offset, ChildNodeType* child);
     /// @brief Replace a child node at offset with the given child node.
-    /// @note Use addChild() for a safer alternative.
+    /// @note Use addChild(ChildNodeType*) for a safer alternative.
     /// @warning This method should only be used by experts seeking low-level optimizations.
     void resetChildUnsafe(Index offset, ChildNodeType* child);
     /// @brief Replace a child node at offset with the given value and active state.
-    /// @note Use addChild() for a safer alternative.
+    /// @note Use addChild(ChildNodeType*) for a safer alternative.
     /// @warning This method should only be used by experts seeking low-level optimizations.
     ChildNodeType* stealChildUnsafe(Index offset, const ValueType& value, bool active);
     /// @brief Delete a child node at offset and replace with the given value and active state.
-    /// @note Use addTile() for a safer alternative.
+    /// @note Use addTile(Index, const ValueType&, bool) for a safer alternative.
     /// @warning This method should only be used by experts seeking low-level optimizations.
     void deleteChildUnsafe(Index offset, const ValueType& value, bool active);
 
@@ -1627,17 +1627,17 @@ inline bool
 InternalNode<ChildT, Log2Dim>::isValueOn(const Coord& xyz) const
 {
     const Index n = this->coordToOffset(xyz);
-    if (this->isChildMaskOff(n)) return this->isValueMaskOn(n);
-    return mNodes[n].getChild()->isValueOn(xyz);
+    return this->isChildMaskOff(n) ? this->isValueMaskOn(n)
+        : mNodes[n].getChild()->isValueOn(xyz);
 }
 
 template<typename ChildT, Index Log2Dim>
 inline bool
 InternalNode<ChildT, Log2Dim>::isValueOff(const Coord& xyz) const
 {
     const Index n = this->coordToOffset(xyz);
-    if (this->isChildMaskOff(n)) return this->isValueMaskOn(n);
-    return mNodes[n].getChild()->isValueOff(xyz);
+    return this->isChildMaskOff(n) ? this->isValueMaskOn(n)
+        : mNodes[n].getChild()->isValueOff(xyz);
 }
 
 template<typename ChildT, Index Log2Dim>
@@ -2501,12 +2501,7 @@ inline void
 InternalNode<ChildT, Log2Dim>::deleteChildUnsafe(Index n, const ValueType& value, bool active)
 {
     // replace child with tile (and delete child)
-    OPENVDB_ASSERT(n < NUM_VALUES);
-    OPENVDB_ASSERT(mChildMask.isOn(n));
-    delete mNodes[n].getChild();
-    mChildMask.setOff(n);
-    mValueMask.set(n, active);
-    mNodes[n].setValue(value);
+    delete this->stealChildUnsafe(n, value, active);
 }
 
 

openvdb/openvdb/tree/LeafNode.h

@@ -473,11 +473,11 @@ class LeafNode
     void setValuesOff() { mValueMask.setOff(); }
 
     /// Return @c true if the voxel at the given coordinates is active.
-    bool isValueOn(const Coord& xyz) const {return this->isValueOn(LeafNode::coordToOffset(xyz));}
+    bool isValueOn(const Coord& xyz) const { return this->isValueOn(LeafNode::coordToOffset(xyz)); }
     /// Return @c true if the voxel at the given offset is active.
     bool isValueOn(Index offset) const { OPENVDB_ASSERT(offset < SIZE); return mValueMask.isOn(offset); }
     /// Return @c true if the voxel at the given coordinates is inactive.
-    bool isValueOff(const Coord& xyz) const {return this->isValueOff(LeafNode::coordToOffset(xyz));}
+    bool isValueOff(const Coord& xyz) const { OPENVDB_ASSERT(offset < SIZE); return this->isValueOff(LeafNode::coordToOffset(xyz)); }
     /// Return @c true if the voxel at the given offset is inactive.
     bool isValueOff(Index offset) const { return mValueMask.isOff(offset); }
 

openvdb/openvdb/tree/RootNode.h

@@ -766,11 +766,11 @@ class RootNode
     /// @brief Return the tile value at the given coordinate.
     /// @note Use cbeginValueAll() for a safer alternative.
     /// @warning This method should only be used by experts seeking low-level optimizations.
-    const ValueType& getValueUnsafe(const Coord& xyz) const;
+    const ValueType& getTileValueUnsafe(const Coord& xyz) const;
     /// @brief Return the tile value and active state at the given coordinate.
     /// @note Use cbeginValueAll() for a safer alternative.
     /// @warning This method should only be used by experts seeking low-level optimizations.
-    bool getValueUnsafe(const Coord& xyz, ValueType& value) const;
+    bool getTileValueUnsafe(const Coord& xyz, ValueType& value) const;
     /// @brief Return the child node at the given coordinate.
     /// @note Use beginChildAll() for a safer alternative.
     /// @warning This method should only be used by experts seeking low-level optimizations.
@@ -2919,7 +2919,7 @@ RootNode<ChildT>::probeConstNodeAndCache(const Coord& xyz, AccessorT& acc) const
 
 template<typename ChildT>
 inline const typename ChildT::ValueType&
-RootNode<ChildT>::getValueUnsafe(const Coord& xyz) const
+RootNode<ChildT>::getTileValueUnsafe(const Coord& xyz) const
 {
     MapCIter iter = this->findCoord(xyz);
     OPENVDB_ASSERT(iter != mTable.end());
@@ -2930,7 +2930,7 @@ RootNode<ChildT>::getValueUnsafe(const Coord& xyz) const
 
 template<typename ChildT>
 inline bool
-RootNode<ChildT>::getValueUnsafe(const Coord& xyz, ValueType& value) const
+RootNode<ChildT>::getTileValueUnsafe(const Coord& xyz, ValueType& value) const
 {
     MapCIter iter = this->findCoord(xyz);
     OPENVDB_ASSERT(iter != mTable.end());

openvdb/openvdb/unittest/TestRootNode.cc

@@ -124,12 +124,12 @@ TEST_F(TestRoot, testUnsafe)
     EXPECT_TRUE(root.addChild(child)); // always returns true
 
     { // get value
-        EXPECT_EQ(root.getValueUnsafe(Coord(1, 2, 3)), 2.0f);
-        EXPECT_EQ(root.getValueUnsafe(Coord(4096, 2, 3)), 3.0f);
+        EXPECT_EQ(root.getTileValueUnsafe(Coord(1, 2, 3)), 2.0f);
+        EXPECT_EQ(root.getTileValueUnsafe(Coord(4096, 2, 3)), 3.0f);
         float value = -1.0f;
-        EXPECT_TRUE(root.getValueUnsafe(Coord(1, 2, 3), value));
+        EXPECT_TRUE(root.getTileValueUnsafe(Coord(1, 2, 3), value));
         EXPECT_EQ(value, 2.0f); value = -1.0f;
-        EXPECT_FALSE(root.getValueUnsafe(Coord(4096, 2, 3), value));
+        EXPECT_FALSE(root.getTileValueUnsafe(Coord(4096, 2, 3), value));
         EXPECT_EQ(value, 3.0f); value = -1.0f;
     }