Iridescent fresnel, bxdfs #918
Conversation
|
In this PR also resolve all left over comments from #930 |
| static scalar_type __getScaledReflectance(NBL_CONST_REF_ARG(fresnel_type) orientedFresnel, NBL_CONST_REF_ARG(Interaction) interaction, scalar_type clampedVdotH) | ||
| { | ||
| spectral_type throughputWeights = interaction.getLuminosityContributionHint(); | ||
| return hlsl::dot<spectral_type>(impl::__implicit_promote<spectral_type, typename fresnel_type::vector_type>::__call(orientedFresnel(clampedVdotH)), throughputWeights); |
There was a problem hiding this comment.
there shouldn't be an impl::__implicit_promote here at all, the spectral_type and fresnel_type::vector_type should match 100%
| dmq.microfacetMeasure = scalar_type(0.0); | ||
| dmq.projectedLightMeasure = scalar_type(0.0); |
There was a problem hiding this comment.
both should be set to bit_cast<scale_type>(numeric_limits<scale_type>::inf) not 0s
| dmq.microfacetMeasure = scalar_type(0.0); | ||
| dmq.projectedLightMeasure = scalar_type(0.0); |
There was a problem hiding this comment.
both should be set to bit_cast<scale_type>(numeric_limits<scale_type>::inf) not 0s
| // TODO: will probably merge with __call at some point | ||
| static void __polarized(const T orientedEta, const T orientedEtak, const T cosTheta, NBL_REF_ARG(T) Rp, NBL_REF_ARG(T) Rs) | ||
| static void __polarized(const T orientedEta, const T etaLen2, const T clampedCosTheta, NBL_REF_ARG(T) Rp, NBL_REF_ARG(T) Rs) |
There was a problem hiding this comment.
cool but this method/function should not be part of Conductor or Dielectric class, can make it free floating in fresnel namespace if you want
Also flesh out future optimizations for faster angle adding
| vector_type R12p, R23p, R12s, R23s; | ||
| const vector_type scale = scalar_type(1.0)/eta12; | ||
| const vector_type cosTheta2_2 = hlsl::promote<vector_type>(1.0) - hlsl::promote<vector_type>(1-cosTheta_1*cosTheta_1) * scale * scale; | ||
| const vector_type cosTheta2_2 = hlsl::promote<vector_type>(1.0) - hlsl::promote<vector_type>(1.0-cosTheta_1*cosTheta_1) * scale * scale; |
There was a problem hiding this comment.
this 1.0 should be wrapped in scalar_type(1.0) before - with cos theta
you can declare a const scalar_type unity = 1.0; somwhere in the function to save yourself a headache
|
|
||
| // Optical Path Difference | ||
| const vector_type D = hlsl::promote<vector_type>(2.0 * Dinc) * thinFilmIor * cosTheta_2; | ||
| const vector_type D = hlsl::promote<vector_type>(2.0 * params.getDinc()) * params.getThinFilmIor() * cosTheta_2; |
There was a problem hiding this comment.
hlsl::promote<vector_type>(2.0 * params.getDinc()) * params.getThinFilmIor() is a single vector which could be precomputed instead of getThinFilmIor()
| R12s = hlsl::mix(R12s, hlsl::promote<vector_type>(1.0), cosTheta2_2 <= hlsl::promote<vector_type>(0.0)); | ||
| R12p = hlsl::mix(R12p, hlsl::promote<vector_type>(1.0), cosTheta2_2 <= hlsl::promote<vector_type>(0.0)); | ||
|
|
||
| R23s = hlsl::mix(R23s, hlsl::promote<vector_type>(0.0), cosTheta2_2 <= hlsl::promote<vector_type>(0.0)); | ||
| R23p = hlsl::mix(R23p, hlsl::promote<vector_type>(0.0), cosTheta2_2 <= hlsl::promote<vector_type>(0.0)); |
There was a problem hiding this comment.
you can do const vector<bool,> tir = cosTheta2_2 <= hlsl::promote<vector_type>(0.0);
btw since you compute it as cosTheta_2 = hlsl::sqrt(hlsl::max(cosTheta2_2, hlsl::promote<vector_type>(0.0))); you can actually compute this condition as cosTheta2_2<=0.0
which in turns is hlsl::promote<vector_type>(1.0-cosTheta_1*cosTheta_1) * scale * scale >= 1.0
and in turn is hlsl::promote<>(1.0-cosTheta_1*cosTheta_1) >= eta12
So you can know the TIR waaay in advance
| template<typename T, bool SupportsTransmission NBL_PRIMARY_REQUIRES(concepts::FloatingPointLikeVectorial<T>) | ||
| struct iridescent_base | ||
| { | ||
| using scalar_type = typename vector_traits<T>::scalar_type; | ||
| using vector_type = T; | ||
|
|
||
| scalar_type getDinc() NBL_CONST_MEMBER_FUNC { return Dinc; } | ||
| vector_type getThinFilmIor() NBL_CONST_MEMBER_FUNC { return thinFilmIor; } | ||
| vector_type getEta12() NBL_CONST_MEMBER_FUNC { return eta12; } | ||
| vector_type getEta23() NBL_CONST_MEMBER_FUNC { return eta23; } | ||
| vector_type getEtak23() NBL_CONST_MEMBER_FUNC | ||
| { | ||
| NBL_IF_CONSTEXPR(SupportsTransmission) | ||
| return hlsl::promote<vector_type>(0.0); | ||
| else | ||
| return etak23; | ||
| } | ||
|
|
||
| scalar_type Dinc; // thickness of thin film in nanometers, rec. 100-25000nm | ||
| vector_type thinFilmIor; |
There was a problem hiding this comment.
still you are storking etak23 for dielectric iridescent.
don't take SupportsTransmission have etak23 and getEtak23() in your iridescent_base, define them in your Iridescent specializations and actually make them inherit instead of compose
There was a problem hiding this comment.
Dinc and thinFilmIoR can be combined into a single member and getter
if you want more semantically clear initialization make creation parameter structs for Iridescent::create
| static this_t create(scalar_type Dinc, vector_type ior1, vector_type ior2, vector_type ior3, vector_type iork3) | ||
| { | ||
| this_t retval; | ||
| retval.helper.Dinc = Dinc; | ||
| retval.helper.thinFilmIor = ior2; | ||
| retval.helper.eta12 = ior2/ior1; | ||
| retval.helper.eta23 = ior3/ior2; | ||
| retval.helper.etak23 = iork3/ior2; | ||
| return retval; | ||
| } |
There was a problem hiding this comment.
where's our create from a creation parameters struct ?
There was a problem hiding this comment.
| // Check for total internal reflection | ||
| R12s = hlsl::mix(R12s, hlsl::promote<vector_type>(1.0), cosTheta2_2 <= hlsl::promote<vector_type>(0.0)); | ||
| R12p = hlsl::mix(R12p, hlsl::promote<vector_type>(1.0), cosTheta2_2 <= hlsl::promote<vector_type>(0.0)); | ||
|
|
||
| R23s = hlsl::mix(R23s, hlsl::promote<vector_type>(0.0), cosTheta2_2 <= hlsl::promote<vector_type>(0.0)); | ||
| R23p = hlsl::mix(R23p, hlsl::promote<vector_type>(0.0), cosTheta2_2 <= hlsl::promote<vector_type>(0.0)); | ||
|
|
||
| // Compute the transmission coefficients | ||
| vector_type T121p = hlsl::promote<vector_type>(1.0) - R12p; | ||
| vector_type T121s = hlsl::promote<vector_type>(1.0) - R12s; |
There was a problem hiding this comment.
actually it would make sense to compute a vector<bool,> tir then do if (all(tir)) path that skips a lot of the calculations (since T121p, T121s, R23s, R23p and cosTheta2 are all zero then)
There was a problem hiding this comment.
Also it would make sense to have a notTIR instead of tir boolean (and if (!any(notTIR)) )
because you can make a const vector_type killFactor = vector_type(notTIR); // 0 when TIR, 1 otherwise
and then you can compute R23p, R23s, T121p and T121s as originalMixLHS*killFactor
then obviously you compute R12 from 1.0-T121
| const vector_type cosTheta2_2 = hlsl::promote<vector_type>(1.0) - hlsl::promote<vector_type>(1-cosTheta_1*cosTheta_1) * scale * scale; | ||
| const vector_type cosTheta2_2 = hlsl::promote<vector_type>(1.0) - hlsl::promote<vector_type>(1.0-cosTheta_1*cosTheta_1) * scale * scale; | ||
|
|
||
| cosTheta_2 = hlsl::sqrt(cosTheta2_2); | ||
| cosTheta_2 = hlsl::sqrt(hlsl::max(cosTheta2_2, hlsl::promote<vector_type>(0.0))); |
There was a problem hiding this comment.
a little code-style correctness note, after you clamp cosTheta_2 to not have imaginary numbers due to sqrt you loose the property that cosTheta2_2 == cosTheta_2*cosTheta_2 so you should "hide" cosTheta_2_2 in its own {} scope so its not accessible after the cosTheta_2 assignment
| template<typename Params> | ||
| static T __call(NBL_CONST_REF_ARG(Params) params, const scalar_type clampedCosTheta) | ||
| { | ||
| const vector_type wavelengths = vector_type(colorspace::scRGB::wavelength_R, colorspace::scRGB::wavelength_G, colorspace::scRGB::wavelength_B); |
There was a problem hiding this comment.
suppy colorspace from iridescent_base last template param (you can default to scSRGB)
| quo = hlsl::mix(reflectance / scaled_reflectance, | ||
| (hlsl::promote<spectral_type>(1.0) - reflectance) / (scalar_type(1.0) - scaled_reflectance), cache.isTransmission()) * G2_over_G1; |
There was a problem hiding this comment.
see #918 (comment)
you can simplify thismix to mix(reflectance,hlsl::promote<fresnel_type::vector_type>(1.0)-reflectance,cache.isTransmission())/scaled_reflectance
No description provided.