From d3cde8a3d03cbeb997f0e0135c8d6ce0b202788e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ars=C3=A8ne=20P=C3=A9rard-Gayot?= Date: Wed, 20 Mar 2024 14:38:10 +0100 Subject: [PATCH] Rework node index data type - Add separate header `index.h` to simplify code structure - Fix serialization of indices (issue #77) - Add comments to describe the meaning of BVH indices (issue #78) --- src/bvh/v2/bvh.h | 54 +++++++++++++------ src/bvh/v2/index.h | 83 ++++++++++++++++++++++++++++++ src/bvh/v2/mini_tree_builder.h | 12 ++--- src/bvh/v2/node.h | 59 +++++---------------- src/bvh/v2/reinsertion_optimizer.h | 32 ++++++------ src/bvh/v2/top_down_sah_builder.h | 4 +- 6 files changed, 158 insertions(+), 86 deletions(-) create mode 100644 src/bvh/v2/index.h diff --git a/src/bvh/v2/bvh.h b/src/bvh/v2/bvh.h index 56a51f25..03c98d09 100644 --- a/src/bvh/v2/bvh.h +++ b/src/bvh/v2/bvh.h @@ -8,6 +8,7 @@ #include #include #include +#include namespace bvh::v2 { @@ -28,8 +29,28 @@ struct Bvh { bool operator == (const Bvh& other) const = default; bool operator != (const Bvh& other) const = default; + /// Returns whether the node located at the given index is the left child of its parent. + static BVH_ALWAYS_INLINE bool is_left_sibling(size_t node_id) { return node_id % 2 == 1; } + + /// Returns the index of a sibling of a node. + static BVH_ALWAYS_INLINE size_t get_sibling_id(size_t node_id) { + return is_left_sibling(node_id) ? node_id + 1 : node_id - 1; + } + + /// Returns the index of the left sibling of the node. This effectively returns the given index + /// unchanged if the node is the left sibling, or the other sibling index otherwise. + static BVH_ALWAYS_INLINE size_t get_left_sibling_id(size_t node_id) { + return is_left_sibling(node_id) ? node_id : node_id - 1; + } + + /// Returns the index of the right sibling of the node. This effectively returns the given index + /// unchanged if the node is the right sibling, or the other sibling index otherwise. + static BVH_ALWAYS_INLINE size_t get_right_sibling_id(size_t node_id) { + return is_left_sibling(node_id) ? node_id + 1 : node_id; + } + /// Returns the root node of this BVH. - const Node& get_root() const { return nodes[0]; } + BVH_ALWAYS_INLINE const Node& get_root() const { return nodes[0]; } /// Extracts the BVH rooted at the given node index. inline Bvh extract_bvh(size_t root_id) const; @@ -79,18 +100,17 @@ Bvh Bvh::extract_bvh(size_t root_id) const { auto& dst_node = bvh.nodes[dst_id]; dst_node = src_node; if (src_node.is_leaf()) { - dst_node.index.first_id = static_cast(bvh.prim_ids.size()); + dst_node.index.set_first_id(bvh.prim_ids.size()); std::copy_n( - prim_ids.begin() + src_node.index.first_id, - src_node.index.prim_count, + prim_ids.begin() + src_node.index.first_id(), + src_node.index.prim_count(), std::back_inserter(bvh.prim_ids)); } else { - size_t first_id = bvh.nodes.size(); - dst_node.index.first_id = static_cast(first_id); + dst_node.index.set_first_id(bvh.nodes.size()); bvh.nodes.emplace_back(); bvh.nodes.emplace_back(); - stack.emplace(src_node.index.first_id + 0, first_id + 0); - stack.emplace(src_node.index.first_id + 1, first_id + 1); + stack.emplace(src_node.index.first_id() + 0, dst_node.index.first_id() + 0); + stack.emplace(src_node.index.first_id() + 1, dst_node.index.first_id() + 1); } } return bvh; @@ -104,9 +124,9 @@ void Bvh::traverse_top_down(Index start, Stack& stack, LeafFn&& leaf_fn, I restart: while (!stack.is_empty()) { auto top = stack.pop(); - while (top.prim_count == 0) { - auto& left = nodes[top.first_id]; - auto& right = nodes[top.first_id + 1]; + while (top.prim_count() == 0) { + auto& left = nodes[top.first_id()]; + auto& right = nodes[top.first_id() + 1]; auto [hit_left, hit_right, should_swap] = inner_fn(left, right); if (hit_left) { @@ -124,7 +144,7 @@ void Bvh::traverse_top_down(Index start, Stack& stack, LeafFn&& leaf_fn, I goto restart; } - [[maybe_unused]] auto was_hit = leaf_fn(top.first_id, top.first_id + top.prim_count); + [[maybe_unused]] auto was_hit = leaf_fn(top.first_id(), top.first_id() + top.prim_count()); if constexpr (IsAnyHit) { if (was_hit) return; } @@ -163,8 +183,8 @@ void Bvh::traverse_bottom_up(LeafFn&& leaf_fn, InnerFn&& inner_fn) { for (size_t i = 0; i < nodes.size(); ++i) { if (nodes[i].is_leaf()) continue; - parents[nodes[i].index.first_id] = i; - parents[nodes[i].index.first_id + 1] = i; + parents[nodes[i].index.first_id()] = i; + parents[nodes[i].index.first_id() + 1] = i; } std::vector seen(nodes.size(), false); for (size_t i = nodes.size(); i-- > 0;) { @@ -175,7 +195,7 @@ void Bvh::traverse_bottom_up(LeafFn&& leaf_fn, InnerFn&& inner_fn) { size_t j = parents[i]; while (j >= 0) { auto& node = nodes[j]; - if (seen[j] || !seen[node.index.first_id] || !seen[node.index.first_id + 1]) + if (seen[j] || !seen[node.index.first_id()] || !seen[node.index.first_id() + 1]) break; inner_fn(nodes[j]); seen[j] = true; @@ -188,8 +208,8 @@ template template void Bvh::refit(LeafFn&& leaf_fn) { traverse_bottom_up(leaf_fn, [&] (Node& node) { - const auto& left = nodes[node.index.first_id]; - const auto& right = nodes[node.index.first_id + 1]; + const auto& left = nodes[node.index.first_id()]; + const auto& right = nodes[node.index.first_id() + 1]; node.set_bbox(left.get_bbox().extend(right.get_bbox())); }); } diff --git a/src/bvh/v2/index.h b/src/bvh/v2/index.h new file mode 100644 index 00000000..beec4ae2 --- /dev/null +++ b/src/bvh/v2/index.h @@ -0,0 +1,83 @@ +#ifndef BVH_V2_INDEX_H +#define BVH_V2_INDEX_H + +#include "bvh/v2/utils.h" + +#include +#include + +namespace bvh::v2 { + +/// Packed index data structure. This index can either refer to a range of primitives for a BVH +/// leaf, or to the children of a BVH node. In either case, the index corresponds to a contiguous +/// range, which means that: +/// +/// - For leaves, primitives in a BVH node should be accessed via: +/// +/// size_t begin = index.first_id(); +/// size_t end = begin + index.prim_count(); +/// for (size_t i = begin; i < end; ++i) { +/// size_t prim_id = bvh.prim_ids[i]; +/// // ... +/// } +/// +/// Note that for efficiency, reordering the original data to avoid the indirection via +/// `bvh.prim_ids` is preferable. +/// +/// - For inner nodes, children should be accessed via: +/// +/// auto& left_child = bvh.nodes[index.first_id()]; +/// auto& right_child = bvh.nodes[index.first_id() + 1]; +/// +template +struct Index { + using Type = UnsignedIntType; + + static constexpr size_t bits = Bits; + static constexpr size_t prim_count_bits = PrimCountBits; + static constexpr Type max_prim_count = make_bitmask(prim_count_bits); + + static_assert(PrimCountBits < Bits); + + Type value; + + Index() = default; + explicit Index(Type value) : value(value) {} + + bool operator == (const Index&) const = default; + bool operator != (const Index&) const = default; + + BVH_ALWAYS_INLINE Type first_id() const { return value >> prim_count_bits; } + BVH_ALWAYS_INLINE Type prim_count() const { return value & max_prim_count; } + BVH_ALWAYS_INLINE bool is_leaf() const { return prim_count() != 0; } + BVH_ALWAYS_INLINE bool is_inner() const { return !is_leaf(); } + + BVH_ALWAYS_INLINE void set_first_id(Type first_id) { + *this = Index { first_id, prim_count() }; + } + + BVH_ALWAYS_INLINE void set_prim_count(Type prim_count) { + *this = Index { first_id(), prim_count }; + } + + static BVH_ALWAYS_INLINE Index make_leaf(Type first_prim, Type prim_count) { + assert(prim_count != 0); + return Index { first_prim, prim_count }; + } + + static BVH_ALWAYS_INLINE Index make_inner(Type first_child) { + return Index { first_child, 0 }; + } + +private: + explicit Index(Type first_id, Type prim_count) + : value((first_id << prim_count_bits) | (prim_count & max_prim_count)) + { + assert(first_id <= make_bitmask(Bits - PrimCountBits)); + assert(prim_count <= max_prim_count); + } +}; + +} // namespace bvh::v2 + +#endif diff --git a/src/bvh/v2/mini_tree_builder.h b/src/bvh/v2/mini_tree_builder.h index 1d45d329..4db478da 100644 --- a/src/bvh/v2/mini_tree_builder.h +++ b/src/bvh/v2/mini_tree_builder.h @@ -225,8 +225,8 @@ class MiniTreeBuilder { if (node.get_bbox().get_half_area() < threshold || node.is_leaf()) { pruned_roots.emplace_back(i, node_id); } else { - stack.push(node.index.first_id); - stack.push(node.index.first_id + 1); + stack.push(node.index.first_id()); + stack.push(node.index.first_id() + 1); } } } @@ -274,16 +274,16 @@ class MiniTreeBuilder { // Helper function to copy and fix the child/primitive index of a node auto copy_node = [&] (size_t i, Node& dst_node, const Node& src_node) { dst_node = src_node; - dst_node.index.first_id += static_cast( - src_node.is_leaf() ? prim_offsets[i] : node_offsets[i]); + dst_node.index.set_first_id(dst_node.index.first_id() + + (src_node.is_leaf() ? prim_offsets[i] : node_offsets[i])); }; // Make the leaves of the top BVH point to the right internal nodes for (auto& node : bvh.nodes) { if (!node.is_leaf()) continue; - assert(node.index.prim_count == 1); - size_t tree_id = bvh.prim_ids[node.index.first_id]; + assert(node.index.prim_count() == 1); + size_t tree_id = bvh.prim_ids[node.index.first_id()]; copy_node(tree_id, node, mini_trees[tree_id].get_root()); } diff --git a/src/bvh/v2/node.h b/src/bvh/v2/node.h index 253c8b37..cc320841 100644 --- a/src/bvh/v2/node.h +++ b/src/bvh/v2/node.h @@ -1,7 +1,7 @@ #ifndef BVH_V2_NODE_H #define BVH_V2_NODE_H -#include "bvh/v2/utils.h" +#include "bvh/v2/index.h" #include "bvh/v2/vec.h" #include "bvh/v2/bbox.h" #include "bvh/v2/ray.h" @@ -13,6 +13,8 @@ namespace bvh::v2 { +/// Binary BVH node, containing its bounds and an index into its children or the primitives it +/// contains. By definition, inner BVH nodes do not contain primitives; only leaves do. template < typename T, size_t Dim, @@ -20,57 +22,26 @@ template < size_t PrimCountBits = 4> struct Node { using Scalar = T; + using Index = bvh::v2::Index; + static constexpr size_t dimension = Dim; static constexpr size_t prim_count_bits = PrimCountBits; static constexpr size_t index_bits = IndexBits; - static constexpr size_t max_prim_count = make_bitmask(prim_count_bits); + /// Bounds of the node, laid out in memory as `[min_x, max_x, min_y, max_y, ...]`. Users should + /// not really depend on a specific order and instead use `get_bbox()` and extract the `min` + /// and/or `max` components accordingly. std::array bounds; - struct Index { - using Type = UnsignedIntType; - Type first_id : std::numeric_limits::digits - prim_count_bits; - Type prim_count : prim_count_bits; - - BVH_ALWAYS_INLINE bool operator == (const Index& other) const { - return first_id == other.first_id && prim_count == other.prim_count; - } - bool operator != (const Index&) const = default; - } index; - - static_assert(sizeof(Index) == sizeof(typename Index::Type)); + /// Index to the children of an inner node, or to the primitives for a leaf node. + Index index; Node() = default; bool operator == (const Node&) const = default; bool operator != (const Node&) const = default; - BVH_ALWAYS_INLINE bool is_leaf() const { return index.prim_count != 0; } - static BVH_ALWAYS_INLINE bool is_left_sibling(size_t node_id) { return node_id % 2 == 1; } - - static BVH_ALWAYS_INLINE size_t get_sibling_id(size_t node_id) { - return is_left_sibling(node_id) ? node_id + 1 : node_id - 1; - } - - static BVH_ALWAYS_INLINE size_t get_left_sibling_id(size_t node_id) { - return is_left_sibling(node_id) ? node_id : node_id - 1; - } - - static BVH_ALWAYS_INLINE size_t get_right_sibling_id(size_t node_id) { - return is_left_sibling(node_id) ? node_id + 1 : node_id; - } - - BVH_ALWAYS_INLINE void make_leaf(size_t first_prim, size_t prim_count) { - assert(prim_count != 0); - assert(prim_count <= max_prim_count); - index.prim_count = static_cast(prim_count); - index.first_id = static_cast(first_prim); - } - - BVH_ALWAYS_INLINE void make_inner(size_t first_child) { - index.prim_count = 0; - index.first_id = static_cast(first_child); - } + BVH_ALWAYS_INLINE bool is_leaf() const { return index.is_leaf(); } BVH_ALWAYS_INLINE BBox get_bbox() const { return BBox( @@ -119,16 +90,14 @@ struct Node { BVH_ALWAYS_INLINE void serialize(OutputStream& stream) const { for (auto&& bound : bounds) stream.write(bound); - stream.write(static_cast(index.first_id)); - stream.write(static_cast(index.prim_count)); + stream.write(index.value); } - static inline Node deserialize(InputStream& stream) { + static BVH_ALWAYS_INLINE Node deserialize(InputStream& stream) { Node node; for (auto& bound : node.bounds) bound = stream.read(); - node.index.first_id = stream.read(); - node.index.prim_count = stream.read(); + node.index = Index(stream.read()); return node; } diff --git a/src/bvh/v2/reinsertion_optimizer.h b/src/bvh/v2/reinsertion_optimizer.h index ed8af9a6..793657f2 100644 --- a/src/bvh/v2/reinsertion_optimizer.h +++ b/src/bvh/v2/reinsertion_optimizer.h @@ -77,8 +77,8 @@ class ReinsertionOptimizer { for (size_t i = begin; i < end; ++i) { auto& node = bvh.nodes[i]; if (!node.is_leaf()) { - parents[node.index.first_id + 0] = i; - parents[node.index.first_id + 1] = i; + parents[node.index.first_id() + 0] = i; + parents[node.index.first_id() + 1] = i; } } }); @@ -140,7 +140,7 @@ class ReinsertionOptimizer { auto node_area = bvh_.nodes[node_id].get_bbox().get_half_area(); auto parent_area = bvh_.nodes[parents_[node_id]].get_bbox().get_half_area(); auto area_diff = parent_area; - auto sibling_id = Node::get_sibling_id(node_id); + auto sibling_id = Bvh::get_sibling_id(node_id); auto pivot_bbox = bvh_.nodes[sibling_id].get_bbox(); auto parent_id = parents_[node_id]; auto pivot_id = parent_id; @@ -165,8 +165,8 @@ class ReinsertionOptimizer { if (!dst_node.is_leaf()) { auto child_area = reinsert_area + dst_node.get_bbox().get_half_area(); - stack.emplace_back(child_area, dst_node.index.first_id + 0); - stack.emplace_back(child_area, dst_node.index.first_id + 1); + stack.emplace_back(child_area, dst_node.index.first_id() + 0); + stack.emplace_back(child_area, dst_node.index.first_id() + 1); } } @@ -177,33 +177,33 @@ class ReinsertionOptimizer { area_diff += bvh_.nodes[pivot_id].get_bbox().get_half_area() - pivot_bbox.get_half_area(); } - sibling_id = Node::get_sibling_id(pivot_id); + sibling_id = Bvh::get_sibling_id(pivot_id); pivot_id = parents_[pivot_id]; } while (pivot_id != 0); - if (best_reinsertion.to == Node::get_sibling_id(best_reinsertion.from) || + if (best_reinsertion.to == Bvh::get_sibling_id(best_reinsertion.from) || best_reinsertion.to == parents_[best_reinsertion.from]) best_reinsertion = {}; return best_reinsertion; } BVH_ALWAYS_INLINE void reinsert_node(size_t from, size_t to) { - auto sibling_id = Node::get_sibling_id(from); + auto sibling_id = Bvh::get_sibling_id(from); auto parent_id = parents_[from]; auto sibling_node = bvh_.nodes[sibling_id]; auto dst_node = bvh_.nodes[to]; - bvh_.nodes[to].make_inner(Node::get_left_sibling_id(from)); + bvh_.nodes[to].index = Node::Index::make_inner(Bvh::get_left_sibling_id(from)); bvh_.nodes[sibling_id] = dst_node; bvh_.nodes[parent_id] = sibling_node; if (!dst_node.is_leaf()) { - parents_[dst_node.index.first_id + 0] = sibling_id; - parents_[dst_node.index.first_id + 1] = sibling_id; + parents_[dst_node.index.first_id() + 0] = sibling_id; + parents_[dst_node.index.first_id() + 1] = sibling_id; } if (!sibling_node.is_leaf()) { - parents_[sibling_node.index.first_id + 0] = parent_id; - parents_[sibling_node.index.first_id + 1] = parent_id; + parents_[sibling_node.index.first_id() + 0] = parent_id; + parents_[sibling_node.index.first_id() + 1] = parent_id; } parents_[sibling_id] = to; @@ -217,8 +217,8 @@ class ReinsertionOptimizer { auto& node = bvh_.nodes[index]; if (!node.is_leaf()) { node.set_bbox( - bvh_.nodes[node.index.first_id + 0].get_bbox().extend( - bvh_.nodes[node.index.first_id + 1].get_bbox())); + bvh_.nodes[node.index.first_id() + 0].get_bbox().extend( + bvh_.nodes[node.index.first_id() + 1].get_bbox())); } index = parents_[index]; } while (index != 0); @@ -227,7 +227,7 @@ class ReinsertionOptimizer { BVH_ALWAYS_INLINE auto get_conflicts(size_t from, size_t to) { return std::array { to, from, - Node::get_sibling_id(from), + Bvh::get_sibling_id(from), parents_[to], parents_[from] }; diff --git a/src/bvh/v2/top_down_sah_builder.h b/src/bvh/v2/top_down_sah_builder.h index fb2179cc..a5c9f1b1 100644 --- a/src/bvh/v2/top_down_sah_builder.h +++ b/src/bvh/v2/top_down_sah_builder.h @@ -89,7 +89,7 @@ class TopDownSahBuilder { if (item.size() > config_.min_leaf_size) { if (auto split_pos = try_split(node.get_bbox(), item.begin, item.end)) { auto first_child = bvh.nodes.size(); - node.make_inner(first_child); + node.index = Node::Index::make_inner(first_child); bvh.nodes.resize(first_child + 2); @@ -122,7 +122,7 @@ class TopDownSahBuilder { } } - node.make_leaf(item.begin, item.size()); + node.index = Node::Index::make_leaf(item.begin, item.size()); } bvh.prim_ids = std::move(get_prim_ids());