Skip to content

Book 3, Section 6.1: Uses random_in_unit_sphere? #617

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
pgetto opened this issue May 23, 2020 · 7 comments
Closed

Book 3, Section 6.1: Uses random_in_unit_sphere? #617

pgetto opened this issue May 23, 2020 · 7 comments

Comments

@pgetto
Copy link

pgetto commented May 23, 2020

The "Cornell box, refactored" image seems to be based on an older version of the Lambertian scatter() function which uses the random_in_unit_sphere function to generate the direction for the scatter ray.

The version of scatter() which uses random_unit_vector (as in the code at the end of Book 2), generates the "same" image as the "Cornell box, with different sampling strategy" image.

So, the conclusion of section 6.2 in book 3 is a little confusing. I expect most will be using the random_unit_vector() function and will see the "same" image after the changes to prepare for importance sampling.

@hollasch
Copy link
Collaborator

We have a large task (almost completed for book 1) of updating every render with the latest changes. There are many differences. Book 1 images are targeted for v3.2.0, while books 2 & 3 may or may not make it into that release.

@pgetto
Copy link
Author

pgetto commented May 23, 2020

Thanks for your reply. This is about the text as much as the image (I think).

But, if you need help generating some of the images, I would be happy to help. I am mid-way through book 3 (though I have done my implementation in Java, so it is not exactly the same code).

@hollasch
Copy link
Collaborator

Ah, sorry, I thought you were talking about image differences. I understand now.

@trevordblack
Copy link
Collaborator

I was aware of the Cornell Box problem in section 6.2 and 6.3, where the Lambertian rewrites would confuse the Cornell Box commentary because we moved to the new random_unit_vector

I could have sworn I documented that somewhere, but I've lost the issue.

You're correct. Moving to the "correct" lambertian in book 1 means a lot of the text in book 3 will be confused. Section 6.2 and 6.3 is a journey that starts at "incorrect" lambertian and ends at "correct" lambertian. Now, starting with "correct" lambertian definitely muddies the text.

This will require some small (but VERY significant) rewrites. This likely isn't going to make into 3.2.0

@pgetto
Copy link
Author

pgetto commented May 28, 2020

Yes, exactly.

@trevordblack trevordblack modified the milestones: v3.2.0, Future May 31, 2020
@trevordblack trevordblack modified the milestones: Future, v3.3.0 Jun 25, 2020
@hollasch hollasch modified the milestones: v3.3.0, Backlog, v4.0.0 Oct 12, 2020
@trevordblack trevordblack self-assigned this Oct 27, 2020
@trevordblack
Copy link
Collaborator

I believe this was resolved in #832

@pgetto Do you agree?
These changes are already in the dev-major branch if you're interested or willing to checkout that branch and then open books/RayTracingTheRestOfYourLife.html in a browser

@trevordblack
Copy link
Collaborator

Closing this out.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

3 participants