Skip to content

Book 1 Chapter 13.2 dev-major: Generating Sample Rays #1076

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 Sep 28, 2022 Discussed in #1075 · 1 comment
Closed

Book 1 Chapter 13.2 dev-major: Generating Sample Rays #1076

hollasch opened this issue Sep 28, 2022 Discussed in #1075 · 1 comment

Comments

@hollasch
Copy link
Collaborator

Discussed in #1075

Originally posted by LollipopFt September 25, 2022

return ray(origin, lower_left_corner + s*horizontal + t*vertical - origin);

was changed to this:
return ray(
origin + offset,
lower_left_corner + s*horizontal + t*vertical - origin - offset
);

this flipped the image on the vertical axis such that it is upside down. changing the code to

            return ray(
                origin + offset,
-                lower_left_corner + s*horizontal + t*vertical - origin - offset
+                lower_left_corner + s*horizontal + (1-t)*vertical - origin - offset
            );

fixed it.
is it just me?

@hollasch hollasch self-assigned this Sep 28, 2022
@hollasch hollasch added this to the v4.0.0 milestone Sep 28, 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
Copy link
Collaborator Author

Fixed in #1144

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

No branches or pull requests

1 participant