Skip to content
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.

Sign up for GitHub

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

now nonskippdf materials can be added to lights hittable list #1674

Open
wants to merge 1 commit into
base: dev-patch
Choose a base branch
from
Open

now nonskippdf materials can be added to lights hittable list #1674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@

build/
/*.ppm

.vscode/
Comment on lines +5 to +6
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an artifact of your own personal environment, so it belongs in your personal global .gitconfig file (along with other things like editor, tool, tagging, OS and other artifacts that come from your general environment and toolsets).

11 changes: 8 additions & 3 deletions src/TheRestOfYourLife/hittable_list.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,14 @@ class hittable_list : public hittable {
return sum;
}

vec3 random(const point3& origin) const override {
auto int_size = int(objects.size());
return objects[random_int(0, int_size-1)]->random(origin);
vec3 random(const point3& origin) const override {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation error.

int int_size = int(objects.size());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the change to make this a double declaration?

auto vec = objects[random_int(0, int_size-1)]->random(origin);
while (vec.is_nan()) {
vec = objects[random_int(0, int_size-1)]->random(origin);
}
Comment on lines +64 to +67
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pattern calls for a do-while loop to eliminate the code duplication.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More generally, a vector that contains a NaN value has issues before this code executes, so we still need to find the root cause for why this happens and fix it at the source. Addressing it here is too late.


return vec;
}

private:
Expand Down
9 changes: 5 additions & 4 deletions src/TheRestOfYourLife/main.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ int main() {

// Glass Sphere
auto glass = make_shared<dielectric>(1.5);
world.add(make_shared<sphere>(point3(190,90,190), 90, glass));
auto nonglass = make_shared<lambertian>(color(0.8, 0.2, 0.4));
world.add(make_shared<sphere>(point3(190,90,190), 90, nonglass));
Comment on lines +47 to +48
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be a testing artifact that made it into your PR.


// Light Sources
auto empty_material = shared_ptr<material>();
Expand All @@ -56,9 +57,9 @@ int main() {
camera cam;

cam.aspect_ratio = 1.0;
cam.image_width = 600;
cam.samples_per_pixel = 100;
cam.max_depth = 50;
cam.image_width = 400;
cam.samples_per_pixel = 1000;
cam.max_depth = 10;
Comment on lines +60 to +62
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More testing artifacts.

cam.background = color(0,0,0);

cam.vfov = 40;
Expand Down
9 changes: 6 additions & 3 deletions src/TheRestOfYourLife/sphere.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,14 @@ class sphere : public hittable {
if (!this->hit(ray(origin, direction), interval(0.001, infinity), rec))
return 0;

auto dist_squared = (center.at(0) - origin).length_squared();
auto cos_theta_max = std::sqrt(1 - radius*radius/dist_squared);
auto cos_theta_max = sqrt(1 - radius*radius/(center.at(0) - origin).length_squared());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the motivation for this change. It doesn't seem related to the PR purpose.

auto solid_angle = 2*pi*(1-cos_theta_max);

return 1 / solid_angle;
if (std::isnan(solid_angle) || std::fabs(solid_angle) < 1e-3) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reasoning for such a large minimum value of solid_angle? 1e-3 eliminates over half of all representable finite floating-point numbers.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would solid_angle be NaN here? The only way I can is if cos_theta_max is NaN. Why would that be?

→ sqrt(1 - radius*radius/(center.at(0) - origin).length_squared()) must be NaN
→ 1 - radius*radius/(center.at(0) - origin).length_squared() must be negative
→ radius*radius/(center.at(0) - origin).length_squared() must be greater than 1
→ radius*radius/(center.at(0) - origin) must be greater than 1
→ radius*radius must be less than center.at(0) - origin

Seems like this needs further digging to figure out why this is an issue.

return 0;
} else {
return 1 / solid_angle;
}
}

vec3 random(const point3& origin) const override {
Expand Down
4 changes: 4 additions & 0 deletions src/TheRestOfYourLife/vec3.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ class vec3 {
return (std::fabs(e[0]) < s) && (std::fabs(e[1]) < s) && (std::fabs(e[2]) < s);
}

bool is_nan() const {
return std::isnan(e[0]) || std::isnan(e[1]) || std::isnan(e[2]);
}

static vec3 random() {
return vec3(random_double(), random_double(), random_double());
}
Expand Down