Skip to content

Commit 309e216

Browse files
keptsecretkeptsecret
committed
fixes to VdotL calculation, contigency for beckmann vndf sampling being invalid
1 parent a77d59b commit 309e216

File tree

6 files changed

+99
-30
lines changed

6 files changed

+99
-30
lines changed

‎include/nbl/builtin/hlsl/bxdf/base/cook_torrance_base.hlsl‎

Lines changed: 89 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -252,44 +252,107 @@ struct SCookTorrance
252252
const vector3_type localH = ndf.generateH(upperHemisphereV, u.xy);
253253
const scalar_type VdotH = hlsl::dot(localV, localH);
254254

255-
assert(NdotV * VdotH > scalar_type(0.0));
255+
NBL_IF_CONSTEXPR(!ndf_type::GuaranteedVNDF)
256+
{
257+
// allow for rejection sampling, theoretically NdotV=0 or VdotH=0 is valid, but leads to 0 value contribution anyway
258+
if ((IsBSDF ? (NdotV*VdotH) : VdotH) <= scalar_type(0.0))
259+
return sample_type::createInvalid();
260+
assert(!hlsl::isnan(NdotV*VdotH));
261+
}
262+
else
263+
{
264+
assert(NdotV*VdotH >= scalar_type(0.0));
265+
}
256266
const scalar_type reflectance = _f(hlsl::abs(VdotH))[0];
257267

258268
scalar_type rcpChoiceProb;
259269
scalar_type z = u.z;
260270
bool transmitted = math::partitionRandVariable(reflectance, z, rcpChoiceProb);
261271

262272
ray_dir_info_type V = interaction.getV();
263-
const vector3_type H = hlsl::mul(interaction.getFromTangentSpace(), localH);
264-
Refract<scalar_type> r = Refract<scalar_type>::create(V.getDirection(), H);
265-
const scalar_type LdotH = hlsl::mix(VdotH, r.getNdotT(rcpEta.value2[0]), transmitted);
266-
267-
// fail if samples have invalid paths
268-
const scalar_type viewShortenFactor = hlsl::mix(scalar_type(1.0), rcpEta.value[0], transmitted);
269-
const scalar_type NdotL = localH.z * (VdotH * viewShortenFactor + LdotH) - NdotV * viewShortenFactor;
270-
// VNDF sampling guarantees that `VdotH` has same sign as `NdotV`
271-
// and `transmitted` controls the sign of `LdotH` relative to `VdotH` by construction (reflect -> same sign, or refract -> opposite sign)
273+
const vector3_type H = hlsl::normalize(hlsl::mul(interaction.getFromTangentSpace(), localH));
274+
275+
// TODO: UNDER CONSTRUCTION, will uncomment when sure basic stuff passes tests
276+
// Refract<scalar_type> r = Refract<scalar_type>::create(V.getDirection(), H);
277+
// const scalar_type LdotH = hlsl::mix(VdotH, r.getNdotT(rcpEta.value2[0]), transmitted);
278+
279+
// // fail if samples have invalid paths
280+
// const scalar_type viewShortenFactor = hlsl::mix(scalar_type(1.0), rcpEta.value[0], transmitted);
281+
// const scalar_type NdotL = localH.z * (VdotH * viewShortenFactor + hlsl::abs(LdotH)) - NdotV * viewShortenFactor;
282+
// // VNDF sampling guarantees that `VdotH` has same sign as `NdotV`
283+
// // and `transmitted` controls the sign of `LdotH` relative to `VdotH` by construction (reflect -> same sign, or refract -> opposite sign)
284+
// if (ComputeMicrofacetNormal<scalar_type>::isTransmissionPath(NdotV, NdotL) != transmitted)
285+
// return sample_type::createInvalid(); // should check if sample direction is invalid
286+
287+
// cache = anisocache_type::createPartial(VdotH, LdotH, localH.z, transmitted, rcpEta);
288+
// assert(cache.isValid(_f.getRefractionOrientedEta()));
289+
290+
// struct reflect_refract_wrapper // so we don't recalculate LdotH
291+
// {
292+
// vector3_type operator()(const bool doRefract, const scalar_type rcpOrientedEta) NBL_CONST_MEMBER_FUNC
293+
// {
294+
// return rr(NdotTorR, rcpOrientedEta);
295+
// }
296+
// bxdf::ReflectRefract<scalar_type> rr;
297+
// scalar_type NdotTorR;
298+
// };
299+
// bxdf::ReflectRefract<scalar_type> rr; // rr.getNdotTorR() and calls to mix as well as a good part of the computations should CSE with our computation of NdotL above
300+
// rr.refract = r;
301+
// reflect_refract_wrapper rrw;
302+
// rrw.rr = rr;
303+
// rrw.NdotTorR = LdotH;
304+
// ray_dir_info_type L = V.template reflectRefract<reflect_refract_wrapper>(rrw, transmitted, rcpEta.value[0]);
305+
306+
ray_dir_info_type L;
307+
if (transmitted)
308+
{
309+
// scalar_type eta = rcpEta.value[0]; // refraction takes eta as ior_incoming/ior_transmitted due to snell's law
310+
// vector3_type orientedH = ieee754::flipSignIfRHSNegative<vector3_type>(H, hlsl::promote<vector3_type>(NdotV));
311+
// scalar_type cosThetaI = hlsl::dot(V.getDirection(), orientedH);
312+
// scalar_type sin2ThetaI = hlsl::max(scalar_type(0), scalar_type(1) - cosThetaI * cosThetaI);
313+
// scalar_type sin2ThetaT = eta * eta * sin2ThetaI;
314+
315+
// if (sin2ThetaT >= 1) return sample_type::createInvalid();
316+
// scalar_type cosThetaT = hlsl::sqrt(scalar_type(1) - sin2ThetaT);
317+
// L.direction = eta * -V.getDirection() + (eta * cosThetaI - cosThetaT) * orientedH;
318+
319+
320+
Refract<scalar_type> r = Refract<scalar_type>::create(V.getDirection(), H);
321+
bxdf::ReflectRefract<scalar_type> rr;
322+
rr.refract = r;
323+
L = V.reflectRefract(rr, transmitted, rcpEta.value[0]);
324+
L.direction = hlsl::normalize(L.direction);
325+
}
326+
else
327+
{
328+
bxdf::Reflect<scalar_type> r = bxdf::Reflect<scalar_type>::create(V.getDirection(), H);
329+
L = V.reflect(r);
330+
}
331+
332+
vector3_type _N = interaction.getN();
333+
scalar_type NdotL = hlsl::dot(_N, L.getDirection());
272334
if (ComputeMicrofacetNormal<scalar_type>::isTransmissionPath(NdotV, NdotL) != transmitted)
273335
return sample_type::createInvalid(); // should check if sample direction is invalid
274336

275-
cache = anisocache_type::createPartial(VdotH, LdotH, localH.z, transmitted, rcpEta);
337+
cache.iso_cache.VdotH = VdotH;
338+
cache.iso_cache.LdotH = hlsl::dot(L.getDirection(), H);
339+
// cache.iso_cache.VdotL = hlsl::dot(V.getDirection(), L.getDirection());
340+
cache.iso_cache.VdotL = hlsl::mix(scalar_type(2.0) * VdotH * VdotH - scalar_type(1.0),
341+
VdotH * (VdotH * rcpEta.value[0] + cache.iso_cache.LdotH) - rcpEta.value[0], transmitted);
342+
// const scalar_type viewShortenFactor = hlsl::mix(scalar_type(1.0), rcpEta.value[0], transmitted);
343+
// scalar_type _VdotL = VdotH * (VdotH * viewShortenFactor + cache.iso_cache.LdotH) - viewShortenFactor;
344+
// scalar_type VdotL = hlsl::dot(V.getDirection(), L.getDirection());
345+
// assert(hlsl::abs(VdotL - cache.iso_cache.VdotL) < 1e-4);
346+
assert(localH.z > scalar_type(0.0));
347+
cache.iso_cache.absNdotH = hlsl::abs(localH.z);
348+
cache.iso_cache.NdotH2 = localH.z * localH.z;
349+
276350
assert(cache.isValid(_f.getRefractionOrientedEta()));
277351

278-
struct reflect_refract_wrapper // so we don't recalculate LdotH
279-
{
280-
vector3_type operator()(const bool doRefract, const scalar_type rcpOrientedEta) NBL_CONST_MEMBER_FUNC
281-
{
282-
return rr(NdotTorR, rcpOrientedEta);
283-
}
284-
bxdf::ReflectRefract<scalar_type> rr;
285-
scalar_type NdotTorR;
286-
};
287-
bxdf::ReflectRefract<scalar_type> rr; // rr.getNdotTorR() and calls to mix as well as a good part of the computations should CSE with our computation of NdotL above
288-
rr.refract = r;
289-
reflect_refract_wrapper rrw;
290-
rrw.rr = rr;
291-
rrw.NdotTorR = LdotH;
292-
ray_dir_info_type L = V.template reflectRefract<reflect_refract_wrapper>(rrw, transmitted, rcpEta.value[0]);
352+
// const scalar_type _viewShortenFactor = hlsl::mix(scalar_type(1.0), rcpEta.value[0], transmitted);
353+
// const scalar_type _NdotL = localH.z * (VdotH * _viewShortenFactor + cache.iso_cache.LdotH) - NdotV * _viewShortenFactor;
354+
scalar_type _NdotL = hlsl::dot(interaction.getN(), L.getDirection());
355+
assert(hlsl::abs(_NdotL - NdotL) < 1e-4);
293356

294357
const vector3_type T = interaction.getT();
295358
const vector3_type B = interaction.getB();

‎include/nbl/builtin/hlsl/bxdf/common.hlsl‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -828,8 +828,8 @@ struct SAnisotropicMicrofacetCache
828828
this_t retval;
829829
retval.iso_cache.VdotH = VdotH;
830830
retval.iso_cache.LdotH = LdotH;
831-
const scalar_type viewShortenFactor = hlsl::mix(scalar_type(1.0), rcpOrientedEta.value[0], transmitted);
832-
retval.iso_cache.VdotL = VdotH * (VdotH * viewShortenFactor + LdotH) - viewShortenFactor;
831+
retval.iso_cache.VdotL = hlsl::mix(scalar_type(2.0) * VdotH * VdotH - scalar_type(1.0),
832+
VdotH * (VdotH * rcpOrientedEta.value[0] + LdotH) - rcpOrientedEta.value[0], transmitted);
833833
assert(NdotH > scalar_type(0.0));
834834
retval.iso_cache.absNdotH = hlsl::abs(NdotH);
835835
retval.iso_cache.NdotH2 = NdotH * NdotH;

‎include/nbl/builtin/hlsl/bxdf/fresnel.hlsl‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ struct ComputeMicrofacetNormal
142142
assert(hlsl::dot(V, L) <= -hlsl::min(orientedEta, scalar_type(1.0) / orientedEta));
143143
const scalar_type etaFactor = hlsl::mix(scalar_type(1.0), orientedEta.value, _refract);
144144
vector_type tmpH = V + L * etaFactor;
145-
tmpH = ieee754::flipSign<vector_type>(tmpH, _refract);
145+
tmpH = ieee754::flipSign<vector_type>(tmpH, _refract && orientedEta > scalar_type(1.0));
146146
return tmpH;
147147
}
148148

@@ -232,6 +232,7 @@ struct Refract
232232
return ieee754::copySign(absNdotT, -NdotI); // TODO: make a ieee754::copySignIntoPositive, see https://github.com/Devsh-Graphics-Programming/Nabla/pull/899#discussion_r2197473145
233233
}
234234

235+
// refraction takes eta as ior_incoming/ior_transmitted due to snell's law
235236
vector_type operator()(const scalar_type rcpOrientedEta) NBL_CONST_MEMBER_FUNC
236237
{
237238
return N * (getNdotI() * rcpOrientedEta + getNdotT(rcpOrientedEta*rcpOrientedEta)) - rcpOrientedEta * I;
@@ -455,7 +456,9 @@ struct Dielectric
455456
return __call(orientedEta2, clampedCosTheta);
456457
}
457458

458-
scalar_type getRefractionOrientedEta() NBL_CONST_MEMBER_FUNC { return orientedEta.value[0]; } // expect T to be monochrome?
459+
// default to monochrome, but it is possible to have RGB fresnel without dispersion fixing the refraction Eta
460+
// to be something else than the etas used to compute RGB reflectance or some sort of interpolation of them
461+
scalar_type getRefractionOrientedEta() NBL_CONST_MEMBER_FUNC { return orientedEta.value[0]; }
459462
OrientedEtaRcps<T> getOrientedEtaRcps() NBL_CONST_MEMBER_FUNC { return orientedEta.getReciprocals(); }
460463

461464
Dielectric<T> getReorientedFresnel(const scalar_type NdotI) NBL_CONST_MEMBER_FUNC

‎include/nbl/builtin/hlsl/bxdf/ndf.hlsl‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ NBL_CONCEPT_END(
4848
((NBL_CONCEPT_REQ_TYPE)(T::dg1_query_type))
4949
((NBL_CONCEPT_REQ_TYPE)(T::g2g1_query_type))
5050
((NBL_CONCEPT_REQ_TYPE)(T::quant_query_type))
51+
((NBL_CONCEPT_REQ_EXPR_RET_TYPE)((T::GuaranteedVNDF), ::nbl::hlsl::is_same_v, bool))
5152
((NBL_CONCEPT_REQ_EXPR_RET_TYPE)((ndf.template D<dummy_impl::sample_t, dummy_impl::interaction_t, dummy_impl::cache_t>(quant_query, _sample, interaction, cache)), ::nbl::hlsl::is_same_v, typename T::quant_type))
5253
((NBL_CONCEPT_REQ_EXPR_RET_TYPE)((ndf.template DG1<dummy_impl::sample_t, dummy_impl::interaction_t>(dg1_query, quant_query, _sample, interaction)), ::nbl::hlsl::is_same_v, typename T::quant_type))
5354
((NBL_CONCEPT_REQ_EXPR_RET_TYPE)((ndf.template correlated<dummy_impl::sample_t, dummy_impl::interaction_t>(g2_query, _sample, interaction)), ::nbl::hlsl::is_same_v, typename T::scalar_type))

‎include/nbl/builtin/hlsl/bxdf/ndf/beckmann.hlsl‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,7 @@ struct Beckmann
232232
{
233233
NBL_HLSL_NDF_CONSTEXPR_DECLS(_IsAnisotropic,reflect_refract);
234234
NBL_HLSL_NDF_TYPE_ALIASES(((Beckmann<T,IsAnisotropic,SupportedPaths>))((impl::BeckmannCommon<T,IsAnisotropic>))((impl::SBeckmannDG1Query<scalar_type>))((impl::SBeckmannG2overG1Query<scalar_type>))((DualMeasureQuantQuery<scalar_type>)));
235+
NBL_CONSTEXPR_STATIC_INLINE bool GuaranteedVNDF = false;
235236

236237
template<typename C=bool_constant<!IsAnisotropic> >
237238
static enable_if_t<C::value && !IsAnisotropic, this_t> create(scalar_type A)

‎include/nbl/builtin/hlsl/bxdf/ndf/ggx.hlsl‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@ struct GGX
176176
{
177177
NBL_HLSL_NDF_CONSTEXPR_DECLS(_IsAnisotropic,reflect_refract);
178178
NBL_HLSL_NDF_TYPE_ALIASES(((GGX<T,IsAnisotropic,SupportedPaths>))((impl::GGXCommon<T,SupportsTransmission,IsAnisotropic>))((impl::SGGXDG1Query<scalar_type>))((impl::SGGXG2XQuery<scalar_type>))((DualMeasureQuantQuery<scalar_type>)));
179+
NBL_CONSTEXPR_STATIC_INLINE bool GuaranteedVNDF = true;
179180

180181
template<typename C=bool_constant<!IsAnisotropic> >
181182
static enable_if_t<C::value && !IsAnisotropic, this_t> create(scalar_type A)

0 commit comments

Comments
 (0)