search

jalberse / shimmer

Physically based rendering in Rust
Apache License 2.0
33 stars 0 forks source link

Fix Sphere lighting response #42

Closed jalberse closed 8 months ago

jalberse commented 9 months ago

two_spheres_scene() reveals clearly an issue with how we illuminate spheres. There's a sharp line apparent in the diffuse sphere being lit by the area light sphere; the top-right of both also seems to be relatively too bright.

I think the bug might be related to transposing values or something, since it's a weirdly directional bug, and the harsh falloff seems to occur on a 45.

jalberse commented 9 months ago

Possibly related to panic on sin_alpha calculation in Sphere::sample_with_context()

jalberse commented 9 months ago

This may also be related to incorrect lighting I'm observing on spheres that are oriented in certain directions (obfuscated previously because I was viewing meshes from a favorable direction).

I suspect that it's something with the BSDF shading frame, or perhaps the dpdu values being fed into it. The issue seems to present most strongly in 3 of 6 major directions. I'll need to check these.

jalberse commented 9 months ago

Possibly related to panic on sin_alpha calculation in Sphere::sample_with_context()

Somewhere along the way I fixed this (we not longer hit the panic), but the issue persists.

It's likely related to a similar issue we see on an equivalent triangulated mesh. So, likely not sphere or tri specific. It occurs in both a random walk and simple path integrator, so not specific to either of those.

I thought the issue might be BSDF shading frames - and I did fix a bug there, to be pushed - but it looks like that had not effect on this issue.

I'm still investigating.

jalberse commented 9 months ago

The issue was my Interval from_val_and_error() ctor using sub_next_up() instead of add_next_up() for the maximum value in the error bounds; just a silly error I must have made quite a while ago. It was causing the ray origin offset we use to prevent self-intersection to fail.

This is fixed now and will be pushed soon :)

jalberse commented 8 months ago

This is merged now