Skip to content

Commit 0a0c519

Browse files
hollaschhollasch
committed
Refactor sphere hit method to use common code
The sphere::hit() function calls get_sphere_uv() with the intersection point transformed to its position on a unit sphere centered at the origin. But this calculation was already performed for the normal vector, so we should just reuse that result instead of calculating it twice. Also, the prior sphere::hit() methods duplicated the main update block with a single change in variable value. This change solves and selects the intersection root first, then updates the hit record for a good hit, simplifying the code and the text.
1 parent 7a396c0 commit 0a0c519

File tree

7 files changed

+122
-231
lines changed

7 files changed

+122
-231
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ Change Log -- Ray Tracing in One Weekend
99
- Fix: `random_unit_vector()` was incorrect (#697)
1010
- Fix: Synchronize text and copies of `hittable.h`
1111
- Fix: Synchronize copies of `hittable_list.h`, `material.h`, `sphere.h`
12+
- Change: refactor `sphere::hit()` method to reuse common blocks of code.
1213

1314
### In One Weekend
1415
- Change: Wrote brief explanation waving away negative t values in initial normal sphere

‎books/RayTracingInOneWeekend.html‎

Lines changed: 30 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -902,31 +902,25 @@
902902
auto a = r.direction().length_squared();
903903
auto half_b = dot(oc, r.direction());
904904
auto c = oc.length_squared() - radius*radius;
905-
auto discriminant = half_b*half_b - a*c;
906-
907-
if (discriminant > 0) {
908-
auto root = sqrt(discriminant);
909-
910-
auto temp = (-half_b - root) / a;
911-
if (temp < t_max && temp > t_min) {
912-
rec.t = temp;
913-
rec.p = r.at(rec.t);
914-
rec.normal = (rec.p - center) / radius;
915-
return true;
916-
}
917905

918-
temp = (-half_b + root) / a;
919-
if (temp < t_max && temp > t_min) {
920-
rec.t = temp;
921-
rec.p = r.at(rec.t);
922-
rec.normal = (rec.p - center) / radius;
923-
return true;
924-
}
906+
auto discriminant = half_b*half_b - a*c;
907+
if (discriminant < 0) return false;
908+
auto sqrtd = sqrt(discriminant);
909+
910+
// Find the nearest root that lies in the acceptable range.
911+
auto root = (-half_b - sqrtd) / a;
912+
if (root < t_min || t_max < root) {
913+
root = (-half_b + sqrtd) / a;
914+
if (root < t_min || t_max < root)
915+
return false;
925916
}
926917

927-
return false;
928-
}
918+
rec.t = root;
919+
rec.p = r.at(rec.t);
920+
rec.normal = (rec.p - center) / radius;
929921

922+
return true;
923+
}
930924

931925
#endif
932926
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -1026,36 +1020,16 @@
10261020

10271021
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
10281022
bool sphere::hit(const ray& r, double t_min, double t_max, hit_record& rec) const {
1029-
vec3 oc = r.origin() - center;
1030-
auto a = r.direction().length_squared();
1031-
auto half_b = dot(oc, r.direction());
1032-
auto c = oc.length_squared() - radius*radius;
1033-
auto discriminant = half_b*half_b - a*c;
1023+
...
10341024

1035-
if (discriminant > 0) {
1036-
auto root = sqrt(discriminant);
1037-
auto temp = (-half_b - root) / a;
1038-
if (temp < t_max && temp > t_min) {
1039-
rec.t = temp;
1040-
rec.p = r.at(rec.t);
1041-
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
1042-
vec3 outward_normal = (rec.p - center) / radius;
1043-
rec.set_face_normal(r, outward_normal);
1044-
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
1045-
return true;
1046-
}
1047-
temp = (-half_b + root) / a;
1048-
if (temp < t_max && temp > t_min) {
1049-
rec.t = temp;
1050-
rec.p = r.at(rec.t);
1025+
rec.t = root;
1026+
rec.p = r.at(rec.t);
10511027
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
1052-
vec3 outward_normal = (rec.p - center) / radius;
1053-
rec.set_face_normal(r, outward_normal);
1028+
vec3 outward_normal = (rec.p - center) / radius;
1029+
rec.set_face_normal(r, outward_normal);
10541030
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
1055-
return true;
1056-
}
1057-
}
1058-
return false;
1031+
1032+
return true;
10591033
}
10601034
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
10611035
[Listing [sphere-final]: <kbd>[sphere.h]</kbd> The sphere class with normal determination]
@@ -2013,38 +1987,17 @@
20131987
};
20141988

20151989
bool sphere::hit(const ray& r, double t_min, double t_max, hit_record& rec) const {
2016-
vec3 oc = r.origin() - center;
2017-
auto a = r.direction().length_squared();
2018-
auto half_b = dot(oc, r.direction());
2019-
auto c = oc.length_squared() - radius*radius;
2020-
auto discriminant = half_b*half_b - a*c;
1990+
...
20211991

2022-
if (discriminant > 0) {
2023-
auto root = sqrt(discriminant);
2024-
auto temp = (-half_b - root) / a;
2025-
if (temp < t_max && temp > t_min) {
2026-
rec.t = temp;
2027-
rec.p = r.at(rec.t);
2028-
vec3 outward_normal = (rec.p - center) / radius;
2029-
rec.set_face_normal(r, outward_normal);
2030-
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
2031-
rec.mat_ptr = mat_ptr;
2032-
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
2033-
return true;
2034-
}
2035-
temp = (-half_b + root) / a;
2036-
if (temp < t_max && temp > t_min) {
2037-
rec.t = temp;
2038-
rec.p = r.at(rec.t);
2039-
vec3 outward_normal = (rec.p - center) / radius;
2040-
rec.set_face_normal(r, outward_normal);
1992+
rec.t = root;
1993+
rec.p = r.at(rec.t);
1994+
vec3 outward_normal = (rec.p - center) / radius;
1995+
rec.set_face_normal(r, outward_normal);
20411996
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
2042-
rec.mat_ptr = mat_ptr;
1997+
rec.mat_ptr = mat_ptr;
20431998
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
2044-
return true;
2045-
}
2046-
}
2047-
return false;
1999+
2000+
return true;
20482001
}
20492002
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
20502003
[Listing [sphere-material]: <kbd>[sphere.h]</kbd> Ray-sphere intersection with added material information]

‎books/RayTracingTheNextWeek.html‎

Lines changed: 25 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -222,9 +222,7 @@
222222
just needs to become a function `center(time)`:
223223

224224
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
225-
bool moving_sphere::hit(
226-
const ray& r, double t_min, double t_max, hit_record& rec) const {
227-
225+
bool moving_sphere::hit(const ray& r, double t_min, double t_max, hit_record& rec) const {
228226
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
229227
vec3 oc = r.origin() - center(r.time());
230228
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
@@ -233,35 +231,26 @@
233231
auto c = oc.length_squared() - radius*radius;
234232

235233
auto discriminant = half_b*half_b - a*c;
234+
if (discriminant < 0) return false;
235+
auto sqrtd = sqrt(discriminant);
236+
237+
// Find the nearest root that lies in the acceptable range.
238+
auto root = (-half_b - sqrtd) / a;
239+
if (root < t_min || t_max < root) {
240+
root = (-half_b + sqrtd) / a;
241+
if (root < t_min || t_max < root)
242+
return false;
243+
}
236244

237-
if (discriminant > 0) {
238-
auto root = sqrt(discriminant);
239-
240-
auto temp = (-half_b - root) / a;
241-
if (temp < t_max && temp > t_min) {
242-
rec.t = temp;
243-
rec.p = r.at(rec.t);
245+
rec.t = root;
246+
rec.p = r.at(rec.t);
244247
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
245-
auto outward_normal = (rec.p - center(r.time())) / radius;
248+
auto outward_normal = (rec.p - center(r.time())) / radius;
246249
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
247-
rec.set_face_normal(r, outward_normal);
248-
rec.mat_ptr = mat_ptr;
249-
return true;
250-
}
250+
rec.set_face_normal(r, outward_normal);
251+
rec.mat_ptr = mat_ptr;
251252

252-
temp = (-half_b + root) / a;
253-
if (temp < t_max && temp > t_min) {
254-
rec.t = temp;
255-
rec.p = r.at(rec.t);
256-
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
257-
auto outward_normal = (rec.p - center(r.time())) / radius;
258-
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
259-
rec.set_face_normal(r, outward_normal);
260-
rec.mat_ptr = mat_ptr;
261-
return true;
262-
}
263-
}
264-
return false;
253+
return true;
265254
}
266255
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
267256
[Listing [moving-sphere-hit]: <kbd>[moving_sphere.h]</kbd> Moving sphere hit function]
@@ -1202,38 +1191,20 @@
12021191
Update the `sphere::hit()` function to use this function to update the hit record UV coordinates.
12031192

12041193
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
1205-
...
12061194
bool sphere::hit(...) {
12071195
...
1208-
if (discriminant > 0) {
1209-
...
1210-
temp = (-half_b - root) / a;
1211-
if (temp < t_max && temp > t_min) {
1212-
rec.t = temp;
1213-
rec.p = r.at(rec.t);
1214-
vec3 outward_normal = (rec.p - center) / radius;
1215-
rec.set_face_normal(r, outward_normal);
1216-
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
1217-
get_sphere_uv((rec.p-center)/radius, rec.u, rec.v);
1218-
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
1219-
rec.mat_ptr = mat_ptr;
1220-
return true;
1221-
}
12221196

1223-
temp = (-half_b + root) / a;
1224-
if (temp < t_max && temp > t_min) {
1225-
rec.t = temp;
1226-
rec.p = r.at(rec.t);
1227-
vec3 outward_normal = (rec.p - center) / radius;
1228-
rec.set_face_normal(r, outward_normal);
1197+
rec.t = root;
1198+
rec.p = r.at(rec.t);
1199+
vec3 outward_normal = (rec.p - center) / radius;
1200+
rec.set_face_normal(r, outward_normal);
12291201
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
1230-
get_sphere_uv((rec.p-center)/radius, rec.u, rec.v);
1202+
get_sphere_uv(outward_normal, rec.u, rec.v);
12311203
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
1232-
rec.mat_ptr = mat_ptr;
1233-
return true;
1234-
}
1235-
...
1204+
rec.mat_ptr = mat_ptr;
12361205

1206+
return true;
1207+
}
12371208
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
12381209
[Listing [get-sphere-uv-call]: <kbd>[sphere.h]</kbd> Sphere UV coordinates from hit]
12391210
</div>

‎src/InOneWeekend/sphere.h‎

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -38,33 +38,26 @@ bool sphere::hit(const ray& r, double t_min, double t_max, hit_record& rec) cons
3838
auto a = r.direction().length_squared();
3939
auto half_b = dot(oc, r.direction());
4040
auto c = oc.length_squared() - radius*radius;
41-
auto discriminant = half_b*half_b - a*c;
42-
43-
if (discriminant > 0) {
44-
auto root = sqrt(discriminant);
4541

46-
auto temp = (-half_b - root) / a;
47-
if (temp < t_max && temp > t_min) {
48-
rec.t = temp;
49-
rec.p = r.at(rec.t);
50-
vec3 outward_normal = (rec.p - center) / radius;
51-
rec.set_face_normal(r, outward_normal);
52-
rec.mat_ptr = mat_ptr;
53-
return true;
54-
}
42+
auto discriminant = half_b*half_b - a*c;
43+
if (discriminant < 0) return false;
44+
auto sqrtd = sqrt(discriminant);
5545

56-
temp = (-half_b + root) / a;
57-
if (temp < t_max && temp > t_min) {
58-
rec.t = temp;
59-
rec.p = r.at(rec.t);
60-
vec3 outward_normal = (rec.p - center) / radius;
61-
rec.set_face_normal(r, outward_normal);
62-
rec.mat_ptr = mat_ptr;
63-
return true;
64-
}
46+
// Find the nearest root that lies in the acceptable range.
47+
auto root = (-half_b - sqrtd) / a;
48+
if (root < t_min || t_max < root) {
49+
root = (-half_b + sqrtd) / a;
50+
if (root < t_min || t_max < root)
51+
return false;
6552
}
6653

67-
return false;
54+
rec.t = root;
55+
rec.p = r.at(rec.t);
56+
vec3 outward_normal = (rec.p - center) / radius;
57+
rec.set_face_normal(r, outward_normal);
58+
rec.mat_ptr = mat_ptr;
59+
60+
return true;
6861
}
6962

7063

‎src/TheNextWeek/moving_sphere.h‎

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -56,40 +56,31 @@ bool moving_sphere::bounding_box(double _time0, double _time1, aabb& output_box)
5656
}
5757

5858

59-
// replace "center" with "center(r.time())"
6059
bool moving_sphere::hit(const ray& r, double t_min, double t_max, hit_record& rec) const {
6160
vec3 oc = r.origin() - center(r.time());
6261
auto a = r.direction().length_squared();
6362
auto half_b = dot(oc, r.direction());
6463
auto c = oc.length_squared() - radius*radius;
6564

6665
auto discriminant = half_b*half_b - a*c;
67-
68-
if (discriminant > 0) {
69-
auto root = sqrt(discriminant);
70-
71-
auto temp = (-half_b - root)/a;
72-
if (temp < t_max && temp > t_min) {
73-
rec.t = temp;
74-
rec.p = r.at(rec.t);
75-
vec3 outward_normal = (rec.p - center(r.time())) / radius;
76-
rec.set_face_normal(r, outward_normal);
77-
rec.mat_ptr = mat_ptr;
78-
return true;
79-
}
80-
81-
temp = (-half_b + root)/a;
82-
if (temp < t_max && temp > t_min) {
83-
rec.t = temp;
84-
rec.p = r.at(rec.t);
85-
vec3 outward_normal = (rec.p - center(r.time())) / radius;
86-
rec.set_face_normal(r, outward_normal);
87-
rec.mat_ptr = mat_ptr;
88-
return true;
89-
}
66+
if (discriminant < 0) return false;
67+
auto sqrtd = sqrt(discriminant);
68+
69+
// Find the nearest root that lies in the acceptable range.
70+
auto root = (-half_b - sqrtd) / a;
71+
if (root < t_min || t_max < root) {
72+
root = (-half_b + sqrtd) / a;
73+
if (root < t_min || t_max < root)
74+
return false;
9075
}
9176

92-
return false;
77+
rec.t = root;
78+
rec.p = r.at(rec.t);
79+
vec3 outward_normal = (rec.p - center(r.time())) / radius;
80+
rec.set_face_normal(r, outward_normal);
81+
rec.mat_ptr = mat_ptr;
82+
83+
return true;
9384
}
9485

9586
#endif

0 commit comments

Comments
 (0)