Skip to content

camera::get_ray() function should perform all sample jittering #1042

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

Closed
hollasch opened this issue Aug 5, 2022 · 0 comments
Closed

camera::get_ray() function should perform all sample jittering #1042

hollasch opened this issue Aug 5, 2022 · 0 comments
Assignees
Milestone

Comments

@hollasch
Copy link
Collaborator

hollasch commented Aug 5, 2022

Currently the camera::get_ray() function performs depth-of-field lens jittering, and the scene::render() loops perform the pixel sample jittering. Both of these should be done in the camera::get_ray() function.

@hollasch hollasch added this to the v4.0.0 milestone Aug 5, 2022
@hollasch hollasch self-assigned this Aug 5, 2022
hollasch added a commit that referenced this issue May 31, 2023
This change does several things. Primarily, it's a refinement of the
division between the scene class and the camera class.

The scene class holds the world description (which includes all scene
geometry and lighting) and the camera. It is responsible for using the
camera to interrogate the world, using the main render loop (all samples
for all pixels).

The camera class is responsible for all 2D rendered image parameters,
and uses these to generate rays one at a time.

The net effect of these changes are:

1. The image_width and aspect_ratio member variables move from the scene
   class to the camera class.

2. camera::initialize() no longer needs to take the aspect ratio
   argument.

3. camera::get_image_height() is a new public function that returns the
   image height, computed from the image width and the desired aspect
   ratio.

4. `camera::aperture` is now `camera::defocus_diameter`.
   `camera::lens_radius` is now `camera::defocus_radius`.

5. The scene and camera parameter assignments have been reordered to
   logical groupings based on the updated mental model.

6. In TheNextWeek/main.cc, we assigned background color, camera up, and
   focus distance before handing the scene off to the scene generators.
   There's been at least one case where a reader was confused about the
   state of the scene object because values were set in two different
   locations. This saved repeating some lines of code, but the
   simplicity of assigning everything in one place for each scene is
   better.

These two changes solidify the responsibilities of the two classes in
preparation for future changes to the camera class, addressing the
following issues:

- #546 Confusion surrounding use of the word "aperture"
- #682 Camera UV coordinates - handle images with width or height of one
- #858 Improve camera.h naming, commentary
- #1042 camera::get_ray() function should perform all sample jittering
- #1076 Book 1 Chapter 13.2 dev-major: Generating Sample Rays

One question left is that now that everything's assigned in the scene
generator functions, why not just have them allocate and return the
generated scene? I've decided to leave the current approach of mutating
the scene -- passed by reference -- purely because it's simpler code.
In my opinion, there's not a whole lot of advantage to the returned
value approach.
@hollasch hollasch modified the milestones: v4.0.0, v4.0.0-book1 Jun 19, 2023
hollasch added a commit that referenced this issue Jun 25, 2023
hollasch added a commit that referenced this issue Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant