From b0cd6c11c3284081dead1bf90867ce6b3603fc68 Mon Sep 17 00:00:00 2001 From: Steve Hollasch Date: Fri, 11 Sep 2020 20:24:05 -0700 Subject: [PATCH] bvh_node: don't modify source object list Instead of reordering the vector of scene objects passed to the `bvh_node` constructor, it now creates a local copy instead. For some reason, this is actually slightly faster. Resolves #701 --- CHANGELOG.md | 10 ++++++++-- books/RayTracingTheNextWeek.html | 7 +++++-- src/TheNextWeek/bvh.h | 8 +++++--- src/TheRestOfYourLife/bvh.h | 9 ++++++--- 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d5d59c2f..d2d4c9dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ Change Log -- Ray Tracing in One Weekend - Fix: Listings 33, 39: Add consistent function signature for `trilinear_interp` (#722) ### _The Next Week_ + - Change: `bvh_node` no longer reorders the source vector of scene objects; uses local copy + instead (#701) - Delete: remove unused u,v,w variables in initial `perlin::noise()` function (#684) - Fix: Listing 15: In `bvh.h`, add missing `hittable_list.h` include (#690) - Fix: Listing 33, 34, 38: Change implicit casts to explicit ones (#692) @@ -19,6 +21,10 @@ Change Log -- Ray Tracing in One Weekend - Fix: Listing 70: Add missing `bvh.h` (#694) - Fix: Listing 70 and `main.cc`: Change a fuzz value of a metal sphere to 1.0 which is the maximum value (#694) +### _The Next Week_ + - Change: `bvh_node` no longer reorders the source vector of scene objects; uses local copy + instead (#701) + ---------------------------------------------------------------------------------------------------- # v3.2.0 (2020-07-18) @@ -59,8 +65,6 @@ significant change and improvement. We're hoping that changes to books one and t but that's never worked out for us before. Ah, dreams. ### Common - - Bug: Found a bug in book 3 source `isotropic::scatter()` method. Commented out, using default - (as it was previously). (#669) - Delete: vestigial `vec3::write_color()` method (now in color.h) - Change: All images and figures renamed to follow more logical convention, using the following pattern: `{fig,img}-.-.<filetype>` (#495) @@ -76,6 +80,8 @@ but that's never worked out for us before. Ah, dreams. because it already caught an existing bug in _The Rest of Your Life_ source. This change includes commenting out the book 3 `isotropic::scatter()` method, which was accidentally ignored anyway. (#639, #669) + - Fix: Found a bug in book 3 source `isotropic::scatter()` method. Commented out, using default + (as it was previously). (#669) - New: each book gets a section of recommended citation examples (#500) ### _In One Weekend_ diff --git a/books/RayTracingTheNextWeek.html b/books/RayTracingTheNextWeek.html index f3790efa..2333c3ff 100644 --- a/books/RayTracingTheNextWeek.html +++ b/books/RayTracingTheNextWeek.html @@ -861,6 +861,7 @@ #define BVH_H #include "rtweekend.h" + #include "hittable.h" #include "hittable_list.h" @@ -869,12 +870,12 @@ public: bvh_node(); - bvh_node(hittable_list& list, double time0, double time1) + bvh_node(const hittable_list& list, double time0, double time1) : bvh_node(list.objects, 0, list.objects.size(), time0, time1) {} bvh_node( - std::vector<shared_ptr<hittable>>& objects, + const std::vector<shared_ptr<hittable>>& src_objects, size_t start, size_t end, double time0, double time1); virtual bool hit( @@ -951,6 +952,8 @@ std::vector<shared_ptr<hittable>>& objects, size_t start, size_t end, double time0, double time1 ) { + auto objects = src_objects; // Create a modifiable array of the source scene objects + int axis = random_int(0,2); auto comparator = (axis == 0) ? box_x_compare : (axis == 1) ? box_y_compare diff --git a/src/TheNextWeek/bvh.h b/src/TheNextWeek/bvh.h index f266768d..e0269231 100644 --- a/src/TheNextWeek/bvh.h +++ b/src/TheNextWeek/bvh.h @@ -23,12 +23,12 @@ class bvh_node : public hittable { public: bvh_node(); - bvh_node(hittable_list& list, double time0, double time1) + bvh_node(const hittable_list& list, double time0, double time1) : bvh_node(list.objects, 0, list.objects.size(), time0, time1) {} bvh_node( - std::vector<shared_ptr<hittable>>& objects, + const std::vector<shared_ptr<hittable>>& src_objects, size_t start, size_t end, double time0, double time1); virtual bool hit( @@ -68,9 +68,11 @@ bool box_z_compare (const shared_ptr<hittable> a, const shared_ptr<hittable> b) bvh_node::bvh_node( - std::vector<shared_ptr<hittable>>& objects, + const std::vector<shared_ptr<hittable>>& src_objects, size_t start, size_t end, double time0, double time1 ) { + auto objects = src_objects; // Create a modifiable array of the source scene objects + int axis = random_int(0,2); auto comparator = (axis == 0) ? box_x_compare : (axis == 1) ? box_y_compare diff --git a/src/TheRestOfYourLife/bvh.h b/src/TheRestOfYourLife/bvh.h index 8c48b2f9..e0269231 100644 --- a/src/TheRestOfYourLife/bvh.h +++ b/src/TheRestOfYourLife/bvh.h @@ -14,6 +14,7 @@ #include "rtweekend.h" #include "hittable.h" +#include "hittable_list.h" #include <algorithm> @@ -22,12 +23,12 @@ class bvh_node : public hittable { public: bvh_node(); - bvh_node(hittable_list& list, double time0, double time1) + bvh_node(const hittable_list& list, double time0, double time1) : bvh_node(list.objects, 0, list.objects.size(), time0, time1) {} bvh_node( - std::vector<shared_ptr<hittable>>& objects, + const std::vector<shared_ptr<hittable>>& src_objects, size_t start, size_t end, double time0, double time1); virtual bool hit( @@ -67,9 +68,11 @@ bool box_z_compare (const shared_ptr<hittable> a, const shared_ptr<hittable> b) bvh_node::bvh_node( - std::vector<shared_ptr<hittable>>& objects, + const std::vector<shared_ptr<hittable>>& src_objects, size_t start, size_t end, double time0, double time1 ) { + auto objects = src_objects; // Create a modifiable array of the source scene objects + int axis = random_int(0,2); auto comparator = (axis == 0) ? box_x_compare : (axis == 1) ? box_y_compare