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.

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

Book 3 / ch 12: Problem in sphere pdf value when origin is inside the sphere #470

Open
Dalamar42 opened this issue Apr 12, 2020 · 7 comments
Assignees
Milestone

Comments

@Dalamar42
Copy link

In chapter 12 of book 3 where sphere::pdf_value and sphere::random are introduced the code only handles the case where the origin is outside the sphere.

If the origin is inside the sphere then the ratio of (radius^2 / ||centre - origin||^2) will be greater than 1 so the square root will return a NaN and kill the pixel.

I noticed this when I was rendering the book 1 cover scene. I don't know for sure, but I suspect it was caused by a lambertian sphere intersecting a dielectric and having a ray scattered off of the lambertian be sent towards the enclosing dielectric.

My solution to this in my code was to say any ray has equal probability to hit the enclosing sphere and so to generate a uniformly random direction vector and return a pdf value of (1 / 4π) for it.

There is one more problematic case where the origin is exactly on the surface of the sphere and the random direction picked by sphere::random is such that the second intersection with the sphere is closer to origin than the tmin passed to hit. This causes sphere::pdf_value to return 0.0 since hit doesn't return anything which causes a NaN due to a division by zero in colour. Same thing can happen if origin is very close to the surface of the sphere.

This case seems to happen extremely rarely though so I handled it by acting as if no scattering had occurred and having colour just return emitted in this case.

@trevordblack
Copy link
Collaborator

NaNs are in the book. They happen. We have a pretty good idea of where they all come from. But we've been waffling on if we want to more directly tackle them.

Generally, in book 1, the NaNs come from the trigonometry in dielectric. cos(x) can be negative for values near pi/2, and can be greater than 1 for values near 0.

The general solution to dealing with NaNs is

  1. At the source, clamp things between 0 and 1 (or whatever the expectation is)
  2. At the end, turn NaNs into (0,0,0) so they just appear as a dip in energy (rather than a black pixel)

@trevordblack
Copy link
Collaborator

Good catch, though, most people don't get this far into the book

@trevordblack trevordblack self-assigned this Apr 12, 2020
@hollasch
Copy link
Collaborator

Thomas, can you elaborate on this?

This causes sphere::pdf_value to return 0.0 since hit doesn't return anything which causes a NaN due to a division by zero in colour.

I'm not seeing how a zero return from pdf_value leads to a NaN.

@hollasch hollasch added this to the Future milestone Apr 19, 2020
@Dalamar42
Copy link
Author

Dalamar42 commented Apr 19, 2020

To be exact, I made a mistake when I originally wrote the issue. I think in this case you will get inf, not NaN.

This needs verifying, but I think what can happen is the following.

  • we have a single sphere light source in hittable_pdf
  • ray hits lambertian surface S
  • the mixture PDF's generate method randomly chooses to call hittable_pdf's generate getting vector v
  • due to whichever of the conditions I described above, hittable_pdf's pdf_value incorrectly returns 0.0 for this v
  • now if this v happens to not be in the top hemisphere of S then the cosine PDF's pdf_value will also return 0.0 so the sum of the two pdf values will be 0.0
  • in colour when we compute emitted + attenuation * scattering_pdf * ray_colour / pdf_value we are going to get a division by zero

To avoid this you need the guarantee that for every geometry, any vector returned by that geometry's random will return a non-zero value if given to the same geometry's pdf_value. This way in the mixture PDF you know you are not going to get a pdf_value of 0.0 regardless of the PDF chosen in generate.

I run some tests in my code for all other geometries included in the books and I believe this condition holds for them. It's only spheres where I found this issue.

@hollasch
Copy link
Collaborator

I'm not sure this is the correct fix. What's wrong with infinite components?

If instead of zero you got a small value, then you can end up with components of, say, a million. The result is the same — light intensity is clamped to 1.0 for each component, and then mapped to 255 on output.

@Dalamar42
Copy link
Author

I agree. For this issue clamping to [0.0, 1.0] is sufficient to solve the problem and the occurrence of these cases is infrequent enough that it doesn't have a visible impact on the image. I have done the same fix on my implementation.

The issue I reported above has to do more with rays originating inside the sphere which is a different problem and which in my code produced a bad image even with clamping.

I added the last two paragraphs of the original issue description as something I noticed on the same code, but they're not directly related with the issue. I don't think I made that very clear. Sorry about that.

@hollasch
Copy link
Collaborator

Sounds good. I agree that stamping out NaNs is worthwhile.

@trevordblack trevordblack modified the milestones: Future, v3.3.0 Jun 25, 2020
@hollasch hollasch modified the milestones: v3.3.0, Backlog Oct 12, 2020
@hollasch hollasch modified the milestones: Backlog, v4.0.1 Jul 26, 2024
@hollasch hollasch modified the milestones: v4.0.1, Backlog Aug 21, 2024
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

3 participants