-
Notifications
You must be signed in to change notification settings - Fork 892
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
Add degrees_to_radians() utility function #231
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes #195 and should get us get closer to portable code across Windows/Linux/Mac. Only tested on Mac. Will test other platforms once all obvious portability issues have been fixed and will then take care of any remaining issues.
Added a constants.h file for all three books that defines both infinity (previously MAXFLOAT) and pi (previously M_PI) as portable constants. Changes are applied throughout all code and book contents. Tested on Mac only for now.
src/TheRestOfYourLife/main.cc no longer needs to include msc.h on Windows.
This file will include more than just constants in the future. For now, thinking about a switchable default floating-point precision type, like `fp` or `real`.
In the process of making our build warning-clean on Windows, I've created a wrapper header that disables warnings around the stb_image.h inclusion. In addition, this automatically defines the STB_IMAGE_IMPLEMENTATION macro before including. This change makes builds warning-free when using Microsoft Visual C++. Other compilers can now easily add specific pragmas to control their output.
- bucamera.h - coscubed.cc - cosine_density.cc - foomaterial.h - pi.cc - sphereimp.cc - sphereplot.cc - x2.cc - x2imp.cc None of these had comments explaining their purpose, and were likely created as an aid to development.
Old style was `HITTABLELISTH`, new is `HITTABLE_LIST_H`. Easier to scan. Resolves #220
We fixed the `constant_medium::hit()` function in TheNextWeek, but neglected to update the same file in TheRestOfYourLife.
In addition, this change removes the unnecessary inline keywords. On Windows + Visual C++, this yields a slight performance improvement, but really it was just a sanity check to ensure that this change didn't harm performance. Your mileage may vary.
- Use class initialization for constructors - Eliminate unused methods and functions - Move methods into class definition - Eliminate unnecessary `inline` keywords - Simplify implementations, using existing functions where possible - Binary operators have params u,v instead of v1,v2 - Drop unnecessary vec3:: qualifiers for method defs moved into class
There are still many places in the code where we're using `vec3` for color data, but this change fixes the lighting oversaturation problem, and also replaces colors with NaN components with a signalling color (pure cyan). Resolves #93
This change rolls back the prior commit that created a new `color` class. I ran across this section in the text, which explains the original rationale for using `vec3` for everything: We’ll use the same class `vec3` for colors, locations, directions, offsets, whatever. Some people don’t like this because it doesn’t prevent you from doing something silly, like adding a color to a location. They have a good point, but we’re going to always take the "less code" route when not obviously wrong. Seems good enough reason for me, so I removed the short-lived `color` class and instead added the `vec3::write_color()` method. In the end, this is quite a bit less code, and fullfills everything we needed in one simple function. (Ok, two, if you count the new `clamp()` utility.) This change includes source and all text changes. In addition, this change sneaks in a slight change in the output image parameters. On my setup, a release build now takes about 2-3 minutes for each of the three programs, which makes sanity perf testing easier.
Fix diffuse: use point on unit sphere, not inside
This is copied from PR #182, targeting the development branch. Original commentary: Ran the PNGs through pngquant+Zopfli to reduce file sizes, but as expected they are a bit larger. All the images add ~180 kilobytes to previous book size. Also the fix for "incorrect" diffuse (#181) is reflected in the screenshots. Resulting AO effect is slightly less intense than before, but more correct. Also in one place fixed source to compile and the color in the source to match the color of the sphere in the screenshot. Images in Book 2 and 3 are "not too bad"; they are larger and JPG artifacts aren't as visible there. Also they are much slower to render, and I'm lazy... :)
Update book1 images, plus fixed diffuse
Gah. Wrong target branch. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #217