Add nodiscard attributes in various key places (issue #99)

Warning: This change is breaking the API, since the precomputed triangle
intersection function now no longer modifies the ray in place. The
traversal routine is expected to update the ray `tmax` value in order to
achieve good performance.

Author: madmann91

src/bvh/v2/binned_sah_builder.h

@@ -29,7 +29,7 @@ class BinnedSahBuilder : public TopDownSahBuilder<Node> {
 public:
     using typename TopDownSahBuilder<Node>::Config;
 
-    BVH_ALWAYS_INLINE static Bvh<Node> build(
+    [[nodiscard]] BVH_ALWAYS_INLINE static Bvh<Node> build(
         std::span<const BBox> bboxes,
         std::span<const Vec> centers,
         const Config& config = {})

src/bvh/v2/bvh.h

@@ -54,7 +54,7 @@ struct Bvh {
     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;
+    [[nodiscard]] inline Bvh extract_bvh(size_t root_id) const;
 
     /// Traverses the BVH from the given index in `start` using the provided stack. Every leaf
     /// encountered on the way is processed using the given `LeafFn` function, and every pair of
@@ -85,7 +85,7 @@ struct Bvh {
     inline void serialize(OutputStream&) const;
 
     template <typename IndexType = typename Index::Type>
-    static inline Bvh deserialize(InputStream&);
+    [[nodiscard]] static inline Bvh deserialize(InputStream&);
 };
 
 template <typename Node>

src/bvh/v2/default_builder.h

@@ -30,7 +30,7 @@ class DefaultBuilder {
     };
 
     /// Build a BVH in parallel using the given thread pool.
-    BVH_ALWAYS_INLINE static Bvh<Node> build(
+    [[nodiscard]] BVH_ALWAYS_INLINE static Bvh<Node> build(
         ThreadPool& thread_pool,
         std::span<const BBox> bboxes,
         std::span<const Vec> centers,
@@ -46,7 +46,7 @@ class DefaultBuilder {
     }
 
     /// Build a BVH in a single-thread.
-    BVH_ALWAYS_INLINE static Bvh<Node> build(
+    [[nodiscard]] BVH_ALWAYS_INLINE static Bvh<Node> build(
         std::span<const BBox>  bboxes,
         std::span<const Vec> centers,
         const Config& config = {})

src/bvh/v2/executor.h

@@ -13,25 +13,25 @@ namespace bvh::v2 {
 template <typename Derived>
 struct Executor {
     template <typename Loop>
-    inline void for_each(size_t begin, size_t end, const Loop& loop) {
+    BVH_ALWAYS_INLINE void for_each(size_t begin, size_t end, const Loop& loop) {
         return static_cast<Derived*>(this)->for_each(begin, end, loop);
     }
 
     template <typename T, typename Reduce, typename Join>
-    inline T reduce(size_t begin, size_t end, const T& init, const Reduce& reduce, const Join& join) {
+    BVH_ALWAYS_INLINE T reduce(size_t begin, size_t end, const T& init, const Reduce& reduce, const Join& join) {
         return static_cast<Derived*>(this)->reduce(begin, end, init, reduce, join);
     }
 };
 
 /// Executor that executes serially.
 struct SequentialExecutor : Executor<SequentialExecutor> {
     template <typename Loop>
-    void for_each(size_t begin, size_t end, const Loop& loop) {
+    BVH_ALWAYS_INLINE void for_each(size_t begin, size_t end, const Loop& loop) {
         loop(begin, end);
     }
 
     template <typename T, typename Reduce, typename Join>
-    T reduce(size_t begin, size_t end, const T& init, const Reduce& reduce, const Join&) {
+    BVH_ALWAYS_INLINE T reduce(size_t begin, size_t end, const T& init, const Reduce& reduce, const Join&) {
         T result(init);
         reduce(result, begin, end);
         return result;
@@ -48,7 +48,7 @@ struct ParallelExecutor : Executor<ParallelExecutor> {
     {}
 
     template <typename Loop>
-    void for_each(size_t begin, size_t end, const Loop& loop) {
+    BVH_ALWAYS_INLINE void for_each(size_t begin, size_t end, const Loop& loop) {
         if (end - begin < parallel_threshold)
             return loop(begin, end);
 
@@ -61,7 +61,7 @@ struct ParallelExecutor : Executor<ParallelExecutor> {
     }
 
     template <typename T, typename Reduce, typename Join>
-    T reduce(size_t begin, size_t end, const T& init, const Reduce& reduce, const Join& join) {
+    BVH_ALWAYS_INLINE T reduce(size_t begin, size_t end, const T& init, const Reduce& reduce, const Join& join) {
         if (end - begin < parallel_threshold) {
             T result(init);
             reduce(result, begin, end);

src/bvh/v2/mini_tree_builder.h

@@ -44,7 +44,7 @@ class MiniTreeBuilder {
 
     /// Starts building a BVH with the given primitive data. The build algorithm is multi-threaded,
     /// and runs on the given thread pool.
-    BVH_ALWAYS_INLINE static Bvh<Node> build(
+    [[nodiscard]] BVH_ALWAYS_INLINE static Bvh<Node> build(
         ThreadPool& thread_pool,
         std::span<const BBox> bboxes,
         std::span<const Vec> centers,

src/bvh/v2/node.h

@@ -65,7 +65,7 @@ struct Node {
     }
 
     /// Robust ray-node intersection routine. See "Robust BVH Ray Traversal", by T. Ize.
-    BVH_ALWAYS_INLINE std::pair<T, T> intersect_robust(
+    [[nodiscard]] BVH_ALWAYS_INLINE std::pair<T, T> intersect_robust(
         const Ray<T, Dim>& ray,
         const Vec<T, Dim>& inv_dir,
         const Vec<T, Dim>& inv_dir_pad,
@@ -76,7 +76,7 @@ struct Node {
         return make_intersection_result(ray, tmin, tmax);
     }
 
-    BVH_ALWAYS_INLINE std::pair<T, T> intersect_fast(
+    [[nodiscard]] BVH_ALWAYS_INLINE std::pair<T, T> intersect_fast(
         const Ray<T, Dim>& ray,
         const Vec<T, Dim>& inv_dir,
         const Vec<T, Dim>& inv_org,
@@ -93,7 +93,7 @@ struct Node {
         stream.write(index.value);
     }
 
-    static BVH_ALWAYS_INLINE Node deserialize(InputStream& stream) {
+    [[nodiscard]] static BVH_ALWAYS_INLINE Node deserialize(InputStream& stream) {
         Node node;
         for (auto& bound : node.bounds)
             bound = stream.read<T>();

src/bvh/v2/sphere.h

@@ -29,7 +29,7 @@ struct Sphere  {
     /// Intersects a ray with the sphere. If the ray is normalized, a dot product can be saved by
     /// setting `AssumeNormalized` to true.
     template <bool AssumeNormalized = false>
-    BVH_ALWAYS_INLINE std::optional<std::pair<T, T>> intersect(const Ray<T, N>& ray) const {
+    [[nodiscard]] BVH_ALWAYS_INLINE std::optional<std::pair<T, T>> intersect(const Ray<T, N>& ray) const {
         auto oc = ray.org - center;
         auto a = AssumeNormalized ? static_cast<T>(1.) : dot(ray.dir, ray.dir);
         auto b = static_cast<T>(2.) * dot(ray.dir, oc);

src/bvh/v2/sweep_sah_builder.h

@@ -27,7 +27,7 @@ class SweepSahBuilder : public TopDownSahBuilder<Node> {
 public:
     using typename TopDownSahBuilder<Node>::Config;
 
-    BVH_ALWAYS_INLINE static Bvh<Node> build(
+    [[nodiscard]] BVH_ALWAYS_INLINE static Bvh<Node> build(
         std::span<const BBox> bboxes,
         std::span<const Vec> centers,
         const Config& config = {})

src/bvh/v2/tri.h

@@ -6,7 +6,7 @@
 #include "bvh/v2/bbox.h"
 
 #include <limits>
-#include <utility>
+#include <tuple>
 #include <optional>
 
 namespace bvh::v2 {
@@ -44,17 +44,16 @@ struct PrecomputedTri {
     BVH_ALWAYS_INLINE BBox<T, 3> get_bbox() const { return convert_to_tri().get_bbox(); }
     BVH_ALWAYS_INLINE Vec<T, 3> get_center() const { return convert_to_tri().get_center(); }
 
-    /// Returns a pair containing the barycentric coordinates of the hit point if the given ray
-    /// intersects the triangle, otherwise returns nothing. The distance at which the ray intersects
-    /// the triangle is set in `ray.tmax`. The tolerance can be adjusted to account for numerical
-    /// precision issues.
-    BVH_ALWAYS_INLINE std::optional<std::pair<T, T>> intersect(
-        Ray<T, 3>& ray,
+    /// Returns a tuple containing the distance at which the ray intersects the triangle, and the
+    /// barycentric coordinates of the hit point if the given ray intersects the triangle, otherwise
+    /// returns nothing. The tolerance can be adjusted to account for numerical precision issues.
+    [[nodiscard]] BVH_ALWAYS_INLINE std::optional<std::tuple<T, T, T>> intersect(
+        const Ray<T, 3>& ray,
         T tolerance = -std::numeric_limits<T>::epsilon()) const;
 };
 
 template <typename T>
-std::optional<std::pair<T, T>> PrecomputedTri<T>::intersect(Ray<T, 3>& ray, T tolerance) const {
+std::optional<std::tuple<T, T, T>> PrecomputedTri<T>::intersect(const Ray<T, 3>& ray, T tolerance) const {
     auto c = p0 - ray.org;
     auto r = cross(ray.dir, c);
     auto inv_det = static_cast<T>(1.) / dot(n, ray.dir);
@@ -67,10 +66,8 @@ std::optional<std::pair<T, T>> PrecomputedTri<T>::intersect(Ray<T, 3>& ray, T to
     // when one of t, u, or v is a NaN
     if (u >= tolerance && v >= tolerance && w >= tolerance) {
         auto t = dot(n, c) * inv_det;
-        if (t >= ray.tmin && t <= ray.tmax) {
-            ray.tmax = t;
-            return std::make_optional(std::pair<T, T> { u, v });
-        }
+        if (t >= ray.tmin && t <= ray.tmax)
+            return std::make_optional(std::make_tuple(t, u, v));
     }
 
     return std::nullopt;

src/bvh/v2/vec.h

@@ -123,7 +123,7 @@ BVH_ALWAYS_INLINE T length(const Vec<T, N>& v) {
 }
 
 template <typename T, size_t N>
-BVH_ALWAYS_INLINE Vec<T, N> normalize(const Vec<T, N>& v) {
+[[nodiscard]] BVH_ALWAYS_INLINE Vec<T, N> normalize(const Vec<T, N>& v) {
     return v * (static_cast<T>(1.) / length(v));
 }