search

mmp / pbrt-v4

Source code to pbrt, the ray tracer described in the forthcoming 4th edition of the "Physically Based Rendering: From Theory to Implementation" book.
https://pbrt.org
Apache License 2.0
2.8k stars 429 forks source link

Coated Conductor 'g' parameter = 1.0 produce a Nan's and black dots render result #333

Closed pbrt4bounty closed 1 year ago

pbrt4bounty commented 1 year ago

It appears that the Coated Conductor material produces incorrect results when the value of 'g' is set to absolute values ​​such as 1.0 or -1.0. The render appears with black dots and many Nan errors on the console. This does not happen when the values ​​are 0.99 or -0.99. The guide says about 'g':

float texture 'g'  
  Henyey-Greenstein asymmetry parameter ranging from -1 to 1 that describes the distribution of scattered light in the medium

Is it perhaps a precision error? I think something similar happened with the 'g' parameter in the 'Subsurface' material.

imagen I can easily set these limits in my addon, but I'm wondering if it's possible to do this in the render engine itself. Thanks..!

pbrt4bounty commented 1 year ago

Seems that I found the place where this value are clamped.. https://github.com/mmp/pbrt-v4/blob/master/src/pbrt/materials.cpp#L385 Float gg = Clamp(texEval(g, ctx), -1, 1); Maybe we should adjust it to Float gg = Clamp(texEval(g, ctx), -0.99, 0.99); @mmp what do you think?

pbrt4bounty commented 1 year ago

Seems that 'thickness' is also involved. Clamp 'g' to 0.99 / -0.99 don't fix the issue. When g = 1.0, the render produce more dots & nan's what the higher the value of 'tickness' is. I need to review my implementation code here. The scene, in case anyone wants to do some tests. thickness_dots.zip

pbrt4bounty commented 1 year ago

Ok. Seems that my previous comment are incorrect.. This are my conclusions, after some tests with the shared scene: Up limits for g: When g = 1.0, any thickness value greater than 0.000004 produces rendering artifacts and NaN console messages When g = 0.999, the render is correct and there are no NaN messages whatever the thickness value.(e.g thickness = 10.0) Down limits for g: When g = -1.0, any thickness value greater than 0.000001 produces rendering artifacts and NaN console messages When g = -0.999, any thickness value greater than 0.15 produces rendering artifacts and NaN console messages When g = -0.99, the render is correct and there are no NaN messages whatever the thickness value.(e.g thickness = 10.0)

According to these tests, it IS possible to fix this error, by setting Float gg = Clamp(texEval(g, ctx), -0.99, 0.99); That being said, the result has been obtained by trial and error, using the reference scene. I don't have enough technical knowledge about the shading system implemented in this version of pbrt. Even, it is possible that I are not using these parameters correctly. I have ready the patch to fix this and I can create a pull request if you, @mmp have not objection. Cheers.. Edited: the issue affect also coateddiffuse material and is fixed using the same clamp values for g in the line 279 of materials.cpp

mmp commented 1 year ago

Yep, Henyey-Greenstein isn't great for |g| \approx 1 which is basically either fully forward scattering or fully backward scattering. It's like a BSDF where if you have a perfectly specular surface: you really want a Dirac delta rather than a very smooth microfacet surface.

Anyway, the origin of those NaNs seems to be in the SampleHenyeyGreenstein function; here is a graph of what it computes for cosTheta as a function of u[0] for a g close to 1. It turns out that the numerics are unstable for u[0] close to 1 in this case.

image

Anyway, I have added some clamping to both the sampling and evaluation code that should fix this. Let me know if you see any more issues.

pbrt4bounty commented 1 year ago

All right now.. Thanks Matt.