now nonskippdf materials can be added to lights hittable list #1674
Conversation
|
|
||
| .vscode/ |
There was a problem hiding this comment.
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).
| auto nonglass = make_shared<lambertian>(color(0.8, 0.2, 0.4)); | ||
| world.add(make_shared<sphere>(point3(190,90,190), 90, nonglass)); |
There was a problem hiding this comment.
This seems to be a testing artifact that made it into your PR.
| cam.image_width = 400; | ||
| cam.samples_per_pixel = 1000; | ||
| cam.max_depth = 10; |
| @@ -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()); | |||
There was a problem hiding this comment.
I don't understand the motivation for this change. It doesn't seem related to the PR purpose.
| auto int_size = int(objects.size()); | ||
| return objects[random_int(0, int_size-1)]->random(origin); | ||
| vec3 random(const point3& origin) const override { | ||
| int int_size = int(objects.size()); |
There was a problem hiding this comment.
Why the change to make this a double declaration?
| 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 { |
| 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); | ||
| } |
There was a problem hiding this comment.
This pattern calls for a do-while loop to eliminate the code duplication.
There was a problem hiding this comment.
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.
| auto solid_angle = 2*pi*(1-cos_theta_max); | ||
|
|
||
| return 1 / solid_angle; | ||
| if (std::isnan(solid_angle) || std::fabs(solid_angle) < 1e-3) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Hello, i' ve modified the pdf_value function for the sphere and the random hittable_list method to include controls for NaNs and zero division, now even materials like a lambertian can be added to the lights hittable list. To see the details and the changes refer to the relative issue (#1668), i hope this could be usefull.