nvpro-samples / nvpro_core

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

possible unseted behavour #57

Closed tigrazone closed 2 months ago

tigrazone commented 4 months ago

https://github.com/nvpro-samples/nvpro_core/blob/c98df794436fed19c17d3eb12f6ff8e5002ce2c4/nvvkhl/shaders/bsdf_functions.h#L75-L81 as I understand, if refract() return false used refractedDir but on branch where

     // Total internal reflection 
     return false; 

There is not seted refractedDir

https://github.com/nvpro-samples/nvpro_core/blob/c98df794436fed19c17d3eb12f6ff8e5002ce2c4/nvvkhl/shaders/ggx.glsl#L116-L132

On branch where return true seted needed value but value not used when returned true. Looks like an issue

mklefrancois commented 4 months ago

Thanks for pointing out the inconsistency, the issue will be fix shortly and an update to GitHub will be done as soon as possible. Note that, in case of total refraction, it is often the case that transmitted vector is set to the incident vector.

  if(k < 0.0F)
  {
    // Total internal reflection
    transmitted = incident;
    return false;
  }
tigrazone commented 4 months ago

Is commented https://github.com/nvpro-samples/nvpro_core/blob/c98df794436fed19c17d3eb12f6ff8e5002ce2c4/nvvkhl/shaders/bsdf_functions.h#L80 not issue?

Also: Why here https://github.com/nvpro-samples/nvpro_core/blob/c98df794436fed19c17d3eb12f6ff8e5002ce2c4/nvvkhl/shaders/bsdf_functions.h#L225 // * mat.transmissionFactor; is commented?