-
Notifications
You must be signed in to change notification settings - Fork 944
Introduce interval class for hit() methods #786
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
books/RayTracingInOneWeekend.html
Outdated
// Find the nearest root that lies in the acceptable range. | ||
auto root = (-half_b - sqrtd) / a; | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight | ||
if (!t.contains(root)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will fix this in subsequent commit.
for (int a = 0; a < 3; a++) { | ||
auto t0 = fmin((minimum[a] - r.origin()[a]) / r.direction()[a], | ||
(maximum[a] - r.origin()[a]) / r.direction()[a]); | ||
auto t1 = fmax((minimum[a] - r.origin()[a]) / r.direction()[a], | ||
(maximum[a] - r.origin()[a]) / r.direction()[a]); | ||
ray_tmin = fmax(t0, ray_tmin); | ||
ray_tmax = fmin(t1, ray_tmax); | ||
auto ray_tmin = fmax(t0, ray_t.min); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Future change using interval intersection.
CHANGELOG needs addition |
Oif. |
What am I looking at? this seems like a gargantuan omni-bus of a PR |
Yup. Basically, introduce the |
I quickly identified at least eight different cases where this would help, and I'd rather develop/PR them one-by-one than submit one gigantic hairball. |
Ref: #777 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
//============================================================================================== | ||
// To the extent possible under law, the author(s) have dedicated all copyright and related and | ||
// neighboring rights to this software to the public domain worldwide. This software is | ||
// distributed without any warranty. | ||
// | ||
// You should have received a copy (see file COPYING.txt) of the CC0 Public Domain Dedication | ||
// along with this software. If not, see <http://creativecommons.org/publicdomain/zero/1.0/>. | ||
//============================================================================================== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New copyright template
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review. What about the copyright? It's the same as the others, except for the "Originally written" lines some have.
Lots of additional use cases for interval pending, but just tackling the
hit
methods for now.