Skip to content

Commit 9bc0066

Browse files
committed
aabb: fix performance puzzle with aabb::hit()
It turns out that the slow version, preserved in the original code, had a bug in that it failed to tighten the `ray_t` interval with each successive iteration through the X, Y and Z axes. Instead, it was modifying temporary variables. This change cleans up the two versions of the `hit()` method, and aligns the text's version with our codebase. Resolves #817
1 parent 642d6f0 commit 9bc0066

File tree

2 files changed

+30
-43
lines changed

2 files changed

+30
-43
lines changed

books/RayTracingTheNextWeek.html

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -682,14 +682,19 @@
682682
...
683683
bool hit(const ray& r, interval ray_t) const {
684684
for (int a = 0; a < 3; a++) {
685-
auto invD = 1.0f / r.direction()[a];
686-
auto t0 = (min()[a] - r.origin()[a]) * invD;
687-
auto t1 = (max()[a] - r.origin()[a]) * invD;
688-
if (invD < 0.0f)
685+
const auto invD = 1 / r.direction()[a];
686+
const auto orig = r.origin()[a];
687+
688+
auto t0 = (axis(a).min - orig) * invD;
689+
auto t1 = (axis(a).max - orig) * invD;
690+
691+
if (invD < 0)
689692
std::swap(t0, t1);
690-
auto ray_tmin = t0 > ray_t.min ? t0 : ray_t.min;
691-
auto ray_tmax = t1 < ray_t.max ? t1 : ray_t.max;
692-
if (ray_tmax <= ray_tmin)
693+
694+
if (t0 > ray_t.min) ray_t.min = t0;
695+
if (t1 < ray_t.max) ray_t.max = t1;
696+
697+
if (ray_t.max <= ray_t.min)
693698
return false;
694699
}
695700
return true;

src/common/aabb.h

Lines changed: 18 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -51,43 +51,25 @@ class aabb {
5151
return x;
5252
}
5353

54-
#if 1
55-
// GitHub Issue #817
56-
// For some reason I haven't figured out yet, this version is 10x faster than the
57-
// version below. I'll come back and figure out why (and in the process, probably figure
58-
// out how to configure CMake to create a profile build). Parking this here for now, to
59-
// be removed before the v4 release.
60-
61-
bool hit(const ray& r, interval ray_t) const {
62-
for (int a = 0; a < 3; a++) {
63-
auto t0 = fmin((axis(a).min - r.origin()[a]) / r.direction()[a],
64-
(axis(a).max - r.origin()[a]) / r.direction()[a]);
65-
auto t1 = fmax((axis(a).min - r.origin()[a]) / r.direction()[a],
66-
(axis(a).max - r.origin()[a]) / r.direction()[a]);
67-
ray_t.min = fmax(t0, ray_t.min);
68-
ray_t.max = fmin(t1, ray_t.max);
69-
if (ray_t.max <= ray_t.min)
70-
return false;
71-
}
72-
return true;
73-
}
74-
#else
75-
bool hit(const ray& r, interval ray_t) const {
76-
auto r_origin = r.origin();
77-
auto r_dir = r.direction();
78-
for (int a = 0; a < 3; a++) {
79-
auto invD = 1.0f / r_dir[a];
80-
auto orig = r_origin[a];
81-
auto t0 = (axis(a).min - orig) * invD;
82-
auto t1 = (axis(a).max - orig) * invD;
83-
if (invD < 0)
84-
std::swap(t0, t1);
85-
if (fmin(t1, ray_t.max) <= fmax(t0, ray_t.min))
86-
return false;
87-
}
88-
return true;
54+
bool hit(const ray& r, interval ray_t) const {
55+
for (int a = 0; a < 3; a++) {
56+
const auto invD = 1 / r.direction()[a];
57+
const auto orig = r.origin()[a];
58+
59+
auto t0 = (axis(a).min - orig) * invD;
60+
auto t1 = (axis(a).max - orig) * invD;
61+
62+
if (invD < 0)
63+
std::swap(t0, t1);
64+
65+
if (t0 > ray_t.min) ray_t.min = t0;
66+
if (t1 < ray_t.max) ray_t.max = t1;
67+
68+
if (ray_t.max <= ray_t.min)
69+
return false;
8970
}
90-
#endif
71+
return true;
72+
}
9173

9274
public:
9375
interval x, y, z;

0 commit comments

Comments
 (0)