nvpro-samples / nvpro_core

shared source code and resources needed for the samples to run
Apache License 2.0
472 stars 113 forks source link

is possible to process roughness < MICROFACET_MIN_ROUGHNESS FASTER? #63

Closed tigrazone closed 1 week ago

tigrazone commented 3 weeks ago

I see in code is smallest roughness compared with MICROFACET_MIN_ROUGHNESS and set this value https://github.com/nvpro-samples/nvpro_core/blob/f855bba68c039f38cb57d38b21aa8289e73f8f59/nvvkhl/shaders/pbr_mat_eval.h#L98

but with this code we have 500009.59018393972796398234918146 as 1.0f / mat.roughness for every mat.roughness.x and mat.roughness.y https://github.com/nvpro-samples/nvpro_core/blob/f855bba68c039f38cb57d38b21aa8289e73f8f59/nvvkhl/shaders/bsdf_functions.h#L244

is it possible to rewrite code to support FASTER processing of near zero roughness without costly calculations? Like

if(abs(mat.roughness.x) > MICROFACET_MIN_ROUGHNESS || abs(mat.roughness.y) > MICROFACET_MIN_ROUGHNESS) { // full slow calculation
} else { //FAST result for ideal reflection/refraction without use of roughness
}

For much faster calculation the condition can be set to bool and in bsdf/btdf sample/evaluate code can be used as bool i.e.

pbrMat.emptyRoughness = abs(mat.roughness.x) < MICROFACET_MIN_ROUGHNESS && abs(mat.roughness.y) < MICROFACET_MIN_ROUGHNESS;

if(pbrMat.emptyRoughness)  //small fast code
else //big slower code
NBickford-NV commented 3 weeks ago

Hi @tigrazone! It's possible to do this; for instance, PBRT v4's TrowbridgeReitzDistribution (the same as GGX) has an EffectivelySmooth() function, which tests against the same 1e-3f threshold we do. If this returns true, then PBRT's dielectric and conductor BSDFs sample a perfect specular distribution, and set flags that make calling code handle the singularities in the PDF correctly.

There's two main reasons handling this fully is complex. Since we overhauled nvpro_core's BSDF implementation recently, we chose the pragmatic and simpler approach of clamping roughness > MICROFACET_MIN_ROUGHNESS, but this could change in the future.

Thanks for the question!

tigrazone commented 2 weeks ago

Processing case mat.roughness.x < MICROFACET_MIN_ROUGHNESS and mat.roughness.y < MICROFACET_MIN_ROUGHNESS is enough. Different > and < in one condition can be solved as is

NBickford-NV commented 2 weeks ago

Hi @tigrazone! I've just tested it out, and surprisingly, it turns out the complex BSDF routines do handle roughness == 0 cases okay, although it generates a lot of floating-point exceptions on the way. The simple routines break.

Here's a test scene with a material where roughness.x = 1.0f and roughness.y = 0.0f:

anisotropic_sphere_2.zip

In vk_gltf_renderer, it renders okay (and the blurrier reflections appear to be correct):

image

One might expect that the shader code would break as soon as it reaches this division by 0:

  data.pdf = hvd_ggx_eval(1.0f / mat.roughness, h0) * G1;

This sets data.pdf to NaN, since hvd_ggx_eval divides positive infinity by positive infinity. In the evaluation code, the BSDF also gets set to NaN, It turns out that in every sample, bsdfEvaluate() is followed by a check that the PDF is greater than 0; since comparison with NaN is 0, evaluation results are always discarded, as they should be.

The sampling code catches this and sets the PDF to DIRAC:

  if(isnan(data.pdf) || isinf(data.pdf))
  {
    data.pdf = DIRAC;
  }

Additionally, the code that computes bsdf_over_pdf in the sampling code doesn't have a singularity at roughness == 0.0f -- so as long as a sample checks for DIRAC, it gets the correct result.

Also, normally if there's a singularity at 0, then there's numerical instability around 0. But surprisingly, so far the images I've seen with low roughnesses look OK.

The bsdfSimple functions break because bsdfSimpleSample() calculates bsdf_over_pdf like this:

  if(data.pdf <= 0.00001F || any(isnan(bsdf_total)))
  {
    data.bsdf_over_pdf = vec3(0.0f);
    data.event_type    = BSDF_EVENT_ABSORB;
  }
  else
  {
    data.bsdf_over_pdf = bsdf_total / data.pdf;
  }

So samples that use these functions, like vk_mini_samples/ray_trace, break (note as well the stray blue pixels on the torus):

image

Therefore, I figure what I'd like to do ideally is to modify bsdfSimpleSample() so that it produces a DIRAC PDF and a stable bsdf_over_pdf in this case, and to modify the other BSDF functions so that they handle these cases without having to divide by 0.

tigrazone commented 2 weeks ago

maybe is better not to go into this code data.pdf = hvd_ggx_eval(1.0f / mat.roughness, h0) * G1; and precalculate case with NaN?

tigrazone commented 2 weeks ago

also I prefer not to compare like roughness == 0.0f I compare with precision fix like roughness < NEARzero where NEARzero is 1e-5f