Chapter 7, 8, 9 floating point intersection error

[vchizhov]

There is a small issue with the materials introduced in chapters 7, 8 and 9 (lambertian, metal, dielectric). Due to floating point error, it may happen that the intersection of a ray with some geometry (in this case a sphere) may be "on the wrong side" (let the ideal intersection be at t0, however due to precision error let one get t1: t1>t0, then r_in.point_at_parameter(t1) would be "on the wrong side"). That means that when we try to scatter a ray, it will start inside of the sphere resulting in self-intersection and a black pixel contribution in the final image (akin to shadow acne). This is valid for both lambertian and metal materials, it is especially obvious for perfect mirror materials (fuzz==0.0) when they are introduced in chapter 8 since we get black pixels where we're not supposed to. To mitigate this issue, one can define epsilon = 0.01 (depending on the scale of your geometry this value may have to be smaller or larger) and when returning the scattered ray, make it have origin not rec.p, but rather rec.p + epsilon*normal. Which leads me to another issue - if an object is intersected "from inside", the wrong normal is used for lambertian scattering and reflection (fortunately the right normal is used for refraction). To fix this, use this normal for lambertian scattering, reflection and refraction: normal = dot(r_in,rec.normal)<0.0f? rec.normal : -rec.normal. For refraction the scattered ray should rather start from rec.p - epsilon*normal (we want to be on the opposite side, to avoid self-intersection).
Another thing that may look like a mistake is: schlick(cosine, ref_idx) since based on the definition it should be schlick(cosine, ni_over_nt). However, in this case it is lucky that schlick(cosine, 1.0/ni_over_nt) == schlick(cosine, ni_over_nt) since ((1-ni_over_nt)/(1+ni_over_nt))^2 == ((1-1/ni_over_nt)/(1+1/ni_over_nt))^2.

[hollasch]

There seem to be multiple issues here. These need to be separate issues.

[trevordblack]

For clarity, this issue is just that the scattering ray should be moved form the surface epsilon*+-Normal?

[vchizhov]

@trevordblack Not only. There's the issue of incorrect normals being used for metals and lambertian materials, especially for intersection on the inside. I have fixed those in one of the commits to my fork.

[trevordblack]

Finally looped back around on this.

1. If you believe it is a good idea to add epsilon offsets, create a separate issue.
2. We have "lazy" determination of what the "local" normal is (in the material). I think it makes more sense to move this code into the sphere::hit function.
3. If we don't move the local normal determination into the sphere::hit. Then I'm not sure of the utility of including this determination in the Metal material. Light paths in Metal don't make a lot of sense from a naive perspective.

On investigation, It appears as if PBRT defaults to the shading normal always pointing outward.
http://www.pbr-book.org/3ed-2018/Geometry_and_Transformations/Interactions.html
Although that representation has multiple gotchas for when the shading normal should be flipped.

Regardless, I think it's a good idea to qualify the name of the normal in hit_record. Whether it is surface_normal, local_normal, outward_normal. Something.

[hollasch]

Epsilon offsets are evil when used with floating point. They're fine with fixed-point arithmetic, but not with floating point.

[hollasch]

Naming in general could be improved in multiple places in the code.

Related: #248

[trevordblack]

Unless I'm mistaken, the line:

if (world->hit(r, 0.001, infinity, rec)) {

Solves the problem of self-intersection to an acceptable degree.
Is there any other win from using epsilon offsets?

[hollasch]

It's a very brittle "fix", depending on an unstated convention of scale and viewing parameters, and for many cases is either way too small or way too big. There are much more robust ways to handle this that use floating-point representation properly, but there's enough to do for now.