Skip to content

Commit 89bb7e8

Browse files
IdclipIdclip
committed
Additional improvements to probe methods, fixed addTile and added additional unit tests
Signed-off-by: Nick Avramoussis <[email protected]>
1 parent 54a66eb commit 89bb7e8

File tree

3 files changed

+260
-122
lines changed

3 files changed

+260
-122
lines changed

‎openvdb/openvdb/tree/ValueAccessor.h‎

Lines changed: 108 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -641,11 +641,10 @@ class CacheItem
641641
void addTile(Index level, const Coord& xyz, const ValueType& value, bool state)
642642
{
643643
static_assert(!TreeCacheT::IsConstTree, "can't add a tile to a const tree");
644-
if (NodeType::LEVEL < level) return;
645-
if (this->isHashed(xyz)) {
644+
if (NodeType::LEVEL >= level && this->isHashed(xyz)) {
646645
assert(mNode);
647-
return const_cast<NodeType*>(mNode)->addTileAndCache(
648-
level, xyz, value, state, *mParent);
646+
const_cast<NodeType*>(mNode)->addTileAndCache(level, xyz, value, state, *mParent);
647+
return;
649648
}
650649
mNext.addTile(level, xyz, value, state);
651650
}
@@ -683,31 +682,41 @@ class CacheItem
683682
NodeT* probeNode(const Coord& xyz)
684683
{
685684
static_assert(!TreeCacheT::IsConstTree, "can't get a non-const node from a const tree");
686-
OPENVDB_NO_UNREACHABLE_CODE_WARNING_BEGIN
687-
if (this->isHashed(xyz)) {
688-
if ((std::is_same<NodeT, NodeType>::value)) {
685+
if constexpr ((std::is_same<NodeT, NodeType>::value)) {
686+
if (this->isHashed(xyz)) {
689687
assert(mNode);
690688
return reinterpret_cast<NodeT*>(const_cast<NodeType*>(mNode));
691689
}
690+
}
691+
692+
if (NodeT::LEVEL < NodeType::LEVEL && mNode) {
693+
// don't need to ascend the chain, descend the tree
692694
return const_cast<NodeType*>(mNode)->template probeNodeAndCache<NodeT>(xyz, *mParent);
693695
}
694-
return mNext.template probeNode<NodeT>(xyz);
695-
OPENVDB_NO_UNREACHABLE_CODE_WARNING_END
696+
else {
697+
// ascend
698+
return mNext.template probeNode<NodeT>(xyz);
699+
}
696700
}
697701

698702
template<typename NodeT>
699703
const NodeT* probeConstNode(const Coord& xyz)
700704
{
701-
OPENVDB_NO_UNREACHABLE_CODE_WARNING_BEGIN
702-
if (this->isHashed(xyz)) {
703-
if ((std::is_same<NodeT, NodeType>::value)) {
705+
if constexpr ((std::is_same<NodeT, NodeType>::value)) {
706+
if (this->isHashed(xyz)) {
704707
assert(mNode);
705708
return reinterpret_cast<const NodeT*>(mNode);
706709
}
710+
}
711+
712+
if (NodeT::LEVEL < NodeType::LEVEL && mNode) {
713+
// don't need to ascend the chain, descend the tree
707714
return mNode->template probeConstNodeAndCache<NodeT>(xyz, *mParent);
708715
}
709-
return mNext.template probeConstNode<NodeT>(xyz);
710-
OPENVDB_NO_UNREACHABLE_CODE_WARNING_END
716+
else {
717+
// ascend
718+
return mNext.template probeConstNode<NodeT>(xyz);
719+
}
711720
}
712721

713722
/// Return the active state of the voxel at the given coordinates.
@@ -870,9 +879,7 @@ class CacheItem<TreeCacheT, NodeVecT, /*AtRoot=*/false, true>
870879
, mHash(CoordLimits::max())
871880
, mNode(nullptr)
872881
, mNext(parent)
873-
, mBuffer(nullptr)
874-
{
875-
}
882+
, mBuffer(nullptr) {}
876883

877884
//@{
878885
/// Copy another CacheItem's node pointers and hash keys, but not its parent pointer.
@@ -881,9 +888,7 @@ class CacheItem<TreeCacheT, NodeVecT, /*AtRoot=*/false, true>
881888
, mHash(other.mHash)
882889
, mNode(other.mNode)
883890
, mNext(parent, other.mNext)
884-
, mBuffer(other.mBuffer)
885-
{
886-
}
891+
, mBuffer(other.mBuffer) {}
887892

888893
CacheItem& copy(TreeCacheT& parent, const CacheItem& other)
889894
{
@@ -953,11 +958,10 @@ class CacheItem<TreeCacheT, NodeVecT, /*AtRoot=*/false, true>
953958
void addTile(Index level, const Coord& xyz, const ValueType& value, bool state)
954959
{
955960
static_assert(!TreeCacheT::IsConstTree, "can't add a tile to a const tree");
956-
if (NodeType::LEVEL < level) return;
957-
if (this->isHashed(xyz)) {
961+
if (NodeType::LEVEL >= level && this->isHashed(xyz)) {
958962
assert(mNode);
959-
return const_cast<NodeType*>(mNode)->addTileAndCache(
960-
level, xyz, value, state, *mParent);
963+
const_cast<NodeType*>(mNode)->addTileAndCache(level, xyz, value, state, *mParent);
964+
return;
961965
}
962966
mNext.addTile(level, xyz, value, state);
963967
}
@@ -995,27 +999,41 @@ class CacheItem<TreeCacheT, NodeVecT, /*AtRoot=*/false, true>
995999
NodeT* probeNode(const Coord& xyz)
9961000
{
9971001
static_assert(!TreeCacheT::IsConstTree, "can't get a non-const node from a const tree");
998-
if (this->isHashed(xyz)) {
999-
if ((std::is_same<NodeT, NodeType>::value)) {
1002+
if constexpr ((std::is_same<NodeT, NodeType>::value)) {
1003+
if (this->isHashed(xyz)) {
10001004
assert(mNode);
10011005
return reinterpret_cast<NodeT*>(const_cast<NodeType*>(mNode));
10021006
}
1007+
}
1008+
1009+
if (NodeT::LEVEL < NodeType::LEVEL && mNode) {
1010+
// don't need to ascend the chain, descend the tree
10031011
return const_cast<NodeType*>(mNode)->template probeNodeAndCache<NodeT>(xyz, *mParent);
10041012
}
1005-
return mNext.template probeNode<NodeT>(xyz);
1013+
else {
1014+
// ascend
1015+
return mNext.template probeNode<NodeT>(xyz);
1016+
}
10061017
}
10071018

10081019
template<typename NodeT>
10091020
const NodeT* probeConstNode(const Coord& xyz)
10101021
{
1011-
if (this->isHashed(xyz)) {
1012-
if ((std::is_same<NodeT, NodeType>::value)) {
1022+
if constexpr ((std::is_same<NodeT, NodeType>::value)) {
1023+
if (this->isHashed(xyz)) {
10131024
assert(mNode);
10141025
return reinterpret_cast<const NodeT*>(mNode);
10151026
}
1027+
}
1028+
1029+
if (NodeT::LEVEL < NodeType::LEVEL && mNode) {
1030+
// don't need to ascend the chain, descend the tree
10161031
return mNode->template probeConstNodeAndCache<NodeT>(xyz, *mParent);
10171032
}
1018-
return mNext.template probeConstNode<NodeT>(xyz);
1033+
else {
1034+
// ascend
1035+
return mNext.template probeConstNode<NodeT>(xyz);
1036+
}
10191037
}
10201038

10211039
/// Return the active state of the voxel at the given coordinates.
@@ -1895,15 +1913,21 @@ class ValueAccessor1 : public ValueAccessorBase<_TreeType, IsSafe>
18951913
{
18961914
assert(BaseT::mTree);
18971915
static_assert(!BaseT::IsConstTree, "can't get a non-const node from a const tree");
1898-
if ((std::is_same<NodeT, NodeT0>::value)) {
1916+
if constexpr ((std::is_same<NodeT, NodeT0>::value)) {
18991917
if (this->isHashed(xyz)) {
19001918
assert(mNode0);
19011919
return reinterpret_cast<NodeT*>(const_cast<NodeT0*>(mNode0));
19021920
}
19031921
return BaseT::mTree->root().template probeNodeAndCache<NodeT>(xyz, *this);
1922+
} else if constexpr (NodeT::LEVEL < NodeT0::LEVEL) {
1923+
// Might still be worth caching this path if a NodeT0
1924+
// is accessed in the future
1925+
return BaseT::mTree->root().template probeNodeAndCache<NodeT>(xyz, *this);
1926+
} else {
1927+
// If we're accessing a node above our top most cache level then
1928+
// there's no point trying to cache it
1929+
return BaseT::mTree->root().template probeNode<NodeT>(xyz);
19041930
}
1905-
// No point trying to cache as this NodeT isn't part of this accessors chain
1906-
return BaseT::mTree->root().template probeNode<NodeT>(xyz);
19071931
}
19081932
LeafNodeT* probeLeaf(const Coord& xyz)
19091933
{
@@ -1916,15 +1940,21 @@ class ValueAccessor1 : public ValueAccessorBase<_TreeType, IsSafe>
19161940
const NodeT* probeConstNode(const Coord& xyz) const
19171941
{
19181942
assert(BaseT::mTree);
1919-
if ((std::is_same<NodeT, NodeT0>::value)) {
1943+
if constexpr ((std::is_same<NodeT, NodeT0>::value)) {
19201944
if (this->isHashed(xyz)) {
19211945
assert(mNode0);
19221946
return reinterpret_cast<const NodeT*>(mNode0);
19231947
}
19241948
return BaseT::mTree->root().template probeConstNodeAndCache<NodeT>(xyz, this->self());
1949+
} else if constexpr (NodeT::LEVEL < NodeT0::LEVEL) {
1950+
// Might still be worth caching this path if a NodeT0
1951+
// is accessed in the future
1952+
return BaseT::mTree->root().template probeConstNodeAndCache<NodeT>(xyz, this->self());
1953+
} else {
1954+
// If we're accessing a node above our top most cache level then
1955+
// there's no point trying to cache it
1956+
return BaseT::mTree->root().template probeConstNode<NodeT>(xyz);
19251957
}
1926-
// No point trying to cache as this NodeT isn't part of this accessors chain
1927-
return BaseT::mTree->root().template probeConstNode<NodeT>(xyz);
19281958
}
19291959
const LeafNodeT* probeConstLeaf(const Coord& xyz) const
19301960
{
@@ -2403,7 +2433,7 @@ class ValueAccessor2 : public ValueAccessorBase<_TreeType, IsSafe>
24032433
{
24042434
assert(BaseT::mTree);
24052435
static_assert(!BaseT::IsConstTree, "can't get a non-const node from a const tree");
2406-
if ((std::is_same<NodeT, NodeT0>::value)) {
2436+
if constexpr ((std::is_same<NodeT, NodeT0>::value)) {
24072437
if (this->isHashed0(xyz)) {
24082438
assert(mNode0);
24092439
return reinterpret_cast<NodeT*>(const_cast<NodeT0*>(mNode0));
@@ -2412,15 +2442,21 @@ class ValueAccessor2 : public ValueAccessorBase<_TreeType, IsSafe>
24122442
return const_cast<NodeT1*>(mNode1)->template probeNodeAndCache<NodeT>(xyz, *this);
24132443
}
24142444
return BaseT::mTree->root().template probeNodeAndCache<NodeT>(xyz, *this);
2415-
} else if ((std::is_same<NodeT, NodeT1>::value)) {
2445+
} else if constexpr ((std::is_same<NodeT, NodeT1>::value)) {
24162446
if (this->isHashed1(xyz)) {
24172447
assert(mNode1);
24182448
return reinterpret_cast<NodeT*>(const_cast<NodeT1*>(mNode1));
24192449
}
24202450
return BaseT::mTree->root().template probeNodeAndCache<NodeT>(xyz, *this);
2451+
} else if constexpr (NodeT::LEVEL < NodeT1::LEVEL) {
2452+
// Might still be worth caching this path if a NodeT0 or NodeT1
2453+
// are accessed in the future
2454+
return BaseT::mTree->root().template probeNodeAndCache<NodeT>(xyz, *this);
2455+
} else {
2456+
// If we're accessing a node above our top most cache level then
2457+
// there's no point trying to cache it
2458+
return BaseT::mTree->root().template probeNode<NodeT>(xyz);
24212459
}
2422-
// No point trying to cache as this NodeT isn't part of this accessors chain
2423-
return BaseT::mTree->root().template probeNode<NodeT>(xyz);
24242460
}
24252461

24262462
/// @brief @return a const pointer to the node of the specified type that contains
@@ -2429,7 +2465,7 @@ class ValueAccessor2 : public ValueAccessorBase<_TreeType, IsSafe>
24292465
const NodeT* probeConstNode(const Coord& xyz) const
24302466
{
24312467
assert(BaseT::mTree);
2432-
if ((std::is_same<NodeT, NodeT0>::value)) {
2468+
if constexpr ((std::is_same<NodeT, NodeT0>::value)) {
24332469
if (this->isHashed0(xyz)) {
24342470
assert(mNode0);
24352471
return reinterpret_cast<const NodeT*>(mNode0);
@@ -2438,15 +2474,21 @@ class ValueAccessor2 : public ValueAccessorBase<_TreeType, IsSafe>
24382474
return mNode1->template probeConstNodeAndCache<NodeT>(xyz, this->self());
24392475
}
24402476
return BaseT::mTree->root().template probeConstNodeAndCache<NodeT>(xyz, this->self());
2441-
} else if ((std::is_same<NodeT, NodeT1>::value)) {
2477+
} else if constexpr ((std::is_same<NodeT, NodeT1>::value)) {
24422478
if (this->isHashed1(xyz)) {
24432479
assert(mNode1);
24442480
return reinterpret_cast<const NodeT*>(mNode1);
24452481
}
24462482
return BaseT::mTree->root().template probeConstNodeAndCache<NodeT>(xyz, this->self());
2483+
} else if constexpr (NodeT::LEVEL < NodeT1::LEVEL) {
2484+
// Might still be worth caching this path if a NodeT0 or NodeT1
2485+
// are accessed in the future
2486+
return BaseT::mTree->root().template probeConstNodeAndCache<NodeT>(xyz, this->self());
2487+
} else {
2488+
// If we're accessing a node above our top most cache level then
2489+
// there's no point trying to cache it
2490+
return BaseT::mTree->root().template probeConstNode<NodeT>(xyz);
24472491
}
2448-
// No point trying to cache as this NodeT isn't part of this accessors chain
2449-
return BaseT::mTree->root().template probeConstNode<NodeT>(xyz);
24502492
}
24512493

24522494
/// @brief @return a pointer to the leaf node that contains
@@ -2997,7 +3039,7 @@ class ValueAccessor3 : public ValueAccessorBase<_TreeType, IsSafe>
29973039
{
29983040
assert(BaseT::mTree);
29993041
static_assert(!BaseT::IsConstTree, "can't get a non-const node from a const tree");
3000-
if ((std::is_same<NodeT, NodeT0>::value)) {
3042+
if constexpr ((std::is_same<NodeT, NodeT0>::value)) {
30013043
if (this->isHashed0(xyz)) {
30023044
assert(mNode0);
30033045
return reinterpret_cast<NodeT*>(const_cast<NodeT0*>(mNode0));
@@ -3009,7 +3051,7 @@ class ValueAccessor3 : public ValueAccessorBase<_TreeType, IsSafe>
30093051
return const_cast<NodeT2*>(mNode2)->template probeNodeAndCache<NodeT>(xyz, *this);
30103052
}
30113053
return BaseT::mTree->root().template probeNodeAndCache<NodeT>(xyz, *this);
3012-
} else if ((std::is_same<NodeT, NodeT1>::value)) {
3054+
} else if constexpr ((std::is_same<NodeT, NodeT1>::value)) {
30133055
if (this->isHashed1(xyz)) {
30143056
assert(mNode1);
30153057
return reinterpret_cast<NodeT*>(const_cast<NodeT1*>(mNode1));
@@ -3018,15 +3060,21 @@ class ValueAccessor3 : public ValueAccessorBase<_TreeType, IsSafe>
30183060
return const_cast<NodeT2*>(mNode2)->template probeNodeAndCache<NodeT>(xyz, *this);
30193061
}
30203062
return BaseT::mTree->root().template probeNodeAndCache<NodeT>(xyz, *this);
3021-
} else if ((std::is_same<NodeT, NodeT2>::value)) {
3063+
} else if constexpr ((std::is_same<NodeT, NodeT2>::value)) {
30223064
if (this->isHashed2(xyz)) {
30233065
assert(mNode2);
30243066
return reinterpret_cast<NodeT*>(const_cast<NodeT2*>(mNode2));
30253067
}
30263068
return BaseT::mTree->root().template probeNodeAndCache<NodeT>(xyz, *this);
3069+
} else if constexpr (NodeT::LEVEL < NodeT2::LEVEL) {
3070+
// Might still be worth caching this path if a NodeT0 or NodeT1
3071+
// are accessed in the future
3072+
return BaseT::mTree->root().template probeNodeAndCache<NodeT>(xyz, *this);
3073+
} else {
3074+
// If we're accessing a node above our top most cache level then
3075+
// there's no point trying to cache it
3076+
return BaseT::mTree->root().template probeNode<NodeT>(xyz);
30273077
}
3028-
// No point trying to cache as this NodeT isn't part of this accessors chain
3029-
return BaseT::mTree->root().template probeNode<NodeT>(xyz);
30303078
}
30313079
/// @brief @return a pointer to the leaf node that contains
30323080
/// voxel (x, y, z) and if it doesn't exist, return @c nullptr.
@@ -3038,7 +3086,7 @@ class ValueAccessor3 : public ValueAccessorBase<_TreeType, IsSafe>
30383086
const NodeT* probeConstNode(const Coord& xyz) const
30393087
{
30403088
assert(BaseT::mTree);
3041-
if ((std::is_same<NodeT, NodeT0>::value)) {
3089+
if constexpr ((std::is_same<NodeT, NodeT0>::value)) {
30423090
if (this->isHashed0(xyz)) {
30433091
assert(mNode0);
30443092
return reinterpret_cast<const NodeT*>(mNode0);
@@ -3050,7 +3098,7 @@ class ValueAccessor3 : public ValueAccessorBase<_TreeType, IsSafe>
30503098
return mNode2->template probeConstNodeAndCache<NodeT>(xyz, this->self());
30513099
}
30523100
return BaseT::mTree->root().template probeConstNodeAndCache<NodeT>(xyz, this->self());
3053-
} else if ((std::is_same<NodeT, NodeT1>::value)) {
3101+
} else if constexpr ((std::is_same<NodeT, NodeT1>::value)) {
30543102
if (this->isHashed1(xyz)) {
30553103
assert(mNode1);
30563104
return reinterpret_cast<const NodeT*>(mNode1);
@@ -3059,15 +3107,21 @@ class ValueAccessor3 : public ValueAccessorBase<_TreeType, IsSafe>
30593107
return mNode2->template probeConstNodeAndCache<NodeT>(xyz, this->self());
30603108
}
30613109
return BaseT::mTree->root().template probeConstNodeAndCache<NodeT>(xyz, this->self());
3062-
} else if ((std::is_same<NodeT, NodeT2>::value)) {
3110+
} else if constexpr ((std::is_same<NodeT, NodeT2>::value)) {
30633111
if (this->isHashed2(xyz)) {
30643112
assert(mNode2);
30653113
return reinterpret_cast<const NodeT*>(mNode2);
30663114
}
30673115
return BaseT::mTree->root().template probeConstNodeAndCache<NodeT>(xyz, this->self());
3116+
} else if constexpr (NodeT::LEVEL < NodeT2::LEVEL) {
3117+
// Might still be worth caching this path if a NodeT0 or NodeT1
3118+
// are accessed in the future
3119+
return BaseT::mTree->root().template probeConstNodeAndCache<NodeT>(xyz, this->self());
3120+
} else {
3121+
// If we're accessing a node above our top most cache level then
3122+
// there's no point trying to cache it
3123+
return BaseT::mTree->root().template probeConstNode<NodeT>(xyz);
30683124
}
3069-
// No point trying to cache as this NodeT isn't part of this accessors chain
3070-
return BaseT::mTree->root().template probeConstNode<NodeT>(xyz);
30713125
}
30723126
/// @brief @return a const pointer to the leaf node that contains
30733127
/// voxel (x, y, z) and if it doesn't exist, return @c nullptr.

0 commit comments

Comments
 (0)