-
Notifications
You must be signed in to change notification settings - Fork 909
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
Progression 3 updates #1462
Progression 3 updates #1462
Conversation
This keeps the call in the same location across multiple versions of pi.cc.
- Add missing includes - Add precise decimal value of expected result
Resolves #1018
This fixes the lack of `material::scatter()` signature change in the `material` base class, but still lacks the corresponding changes required in metal, dielectric, diffuse_light and isotropic materials. In addition, it lacks the change required to the `camera::ray_color()` function call to `material::scatter()`. Resolves #1312
Here's a reminder when going through these potato chip changes that on GitHub while reviewing a commit, |
Oh, by the way, the corresponding book 3 progression for this batch is from |
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.
A couple questions, but overall changes make sense and look good.
@@ -457,27 +473,19 @@ | |||
|
|||
<div class='together'> | |||
Our work above is equally valid as a means to solve for $pi$ as it is a means to solve for the area |
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.
Shouldn't this be $\pi$
instead of $pi$
?
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.
Yup!
Book 2 already does this.
A slew of changes from the book 3 progression up to listing 28. Listing 28 leaves us in a broken state, covered in issues #1302, #1321.