Bxdf fixes cook torrance #930

devshgraphicsprogramming · 2025-09-16T14:03:10Z

Description

Continues #899 , #916 and #919

Testing

TODO list:

…eturn quants

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

include/nbl/builtin/hlsl/math/functions.hlsl

include/nbl/builtin/hlsl/cpp_compat/vector.hlsl

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

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

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

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

devshgraphicsprogramming · 2025-10-08T10:19:36Z

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

+    template<typename C=bool_constant<!IsAnisotropic>, typename T=conditional_t<IsBSDF, vector3_type, vector2_type> >
+    enable_if_t<C::value && !IsAnisotropic, sample_type> generate(NBL_CONST_REF_ARG(isotropic_interaction_type) interaction, const T u, NBL_REF_ARG(isocache_type) cache)


you can use conditional_t directly in a function signature, there's no need to add another template parameter T

P.S. also try to use NBL_FUNC_REQUIRES instead of the enable_if_t

devshgraphicsprogramming · 2025-10-08T10:21:35Z

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

+        fresnel_type _f = fresnel;
+        NBL_IF_CONSTEXPR(IsBSDF)
+            _f = impl::getOrientedFresnel<fresnel_type, IsBSDF>::__call(fresnel, interaction.getNdotV());


either don't passIsBSDF to impl::getOrientedFresnel or skip the NBL_IF_CONSTEXPR

devshgraphicsprogramming · 2025-10-08T10:23:25Z

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

-        spectral_type quo = hlsl::promote<spectral_type>(0.0);
        const bool notTIR = impl::check_TIR_helper<fresnel_type, IsBSDF>::template __call<MicrofacetCache>(_f, cache);
-        if (notTIR)
+        assert(notTIR);


add a comment why

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

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

devshgraphicsprogramming · 2025-10-08T10:55:24Z

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

+        dmq.microfacetMeasure = dg1;
+        dmq.projectedLightMeasure = dg1;// TODO: figure this out * _sample.getNdotL(BxDFClampMode::BCM_MAX);
+        return dmq;
+    }
+    template<class LS, class Interaction, typename C=bool_constant<IsBSDF> NBL_FUNC_REQUIRES(LightSample<LS> && RequiredInteraction<Interaction>)
+    enable_if_t<C::value && IsBSDF, quant_type> DG1(NBL_CONST_REF_ARG(dg1_query_type) query, NBL_CONST_REF_ARG(quant_query_type) quant_query, NBL_CONST_REF_ARG(LS) _sample, NBL_CONST_REF_ARG(Interaction) interaction)
+    {
+        scalar_type dg1 = base_type::template DG1<dg1_query_type>(query);
+        quant_type dmq;
+        dmq.microfacetMeasure = dg1;  // note: microfacetMeasure/2NdotV


if your base DG1 returns something else than actual DG1 like DG1_over_4NdotV, then it should be called DG1_over_4NdotV

then you assign straight to projectedLightMeasure in the reflective case and properly multiply the 4*NdotV back in for the microfacetMeasure

ideally you should really just return DG1_over_2NdotV cause extra 0.5 mul is kinda pointless inside

devshgraphicsprogramming · 2025-10-08T11:00:41Z

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

+        const scalar_type VdotHLdotH = quant_query.getVdotHLdotH();
+        const scalar_type VdotH_etaLdotH = quant_query.getVdotH_etaLdotH();
+        const bool transmitted = reflect_refract==MTT_REFRACT || (reflect_refract!=MTT_REFLECT && VdotHLdotH < scalar_type(0.0));
+        scalar_type NdotL_over_denominator = _sample.getNdotL(BxDFClampMode::BCM_ABS);
+        if (transmitted)
+                NdotL_over_denominator *= -scalar_type(4.0) * VdotHLdotH / (VdotH_etaLdotH * VdotH_etaLdotH);
+        dmq.projectedLightMeasure = dg1;// TODO: figure this out * NdotL_over_denominator;


for the projectedLightMeasure transmittance you'd just do mix(0.5,2.0,transmitted)*DG1_over_2NdotV and then

if (transmitted) dmq.projectedLightMeasure *= VdotHLdotH*neg_rcp2_VdotH_etaLdotH; dmq.microfacetMeasure *= 2.0*clampedNdotV;

devshgraphicsprogramming · 2025-10-08T11:01:57Z

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

+    template<class LS, class Interaction, class MicrofacetCache, typename C=bool_constant<!IsBSDF> NBL_FUNC_REQUIRES(LightSample<LS> && RequiredInteraction<Interaction> && RequiredMicrofacetCache<MicrofacetCache>)
+    enable_if_t<C::value && !IsBSDF, quant_type> D(NBL_CONST_REF_ARG(quant_query_type) quant_query, NBL_CONST_REF_ARG(LS) _sample, NBL_CONST_REF_ARG(Interaction) interaction, NBL_CONST_REF_ARG(MicrofacetCache) cache)
+    {
+        scalar_type d = __ndf_base.template D<MicrofacetCache>(cache);
+        quant_type dmq;
+        dmq.microfacetMeasure = d;
+        dmq.projectedLightMeasure = d * _sample.getNdotL(BxDFClampMode::BCM_MAX);
+        return dmq;
+    }
+    template<class LS, class Interaction, class MicrofacetCache, typename C=bool_constant<IsBSDF> NBL_FUNC_REQUIRES(LightSample<LS> && RequiredInteraction<Interaction> && RequiredMicrofacetCache<MicrofacetCache>)
+    enable_if_t<C::value && IsBSDF, quant_type> D(NBL_CONST_REF_ARG(quant_query_type) quant_query, NBL_CONST_REF_ARG(LS) _sample, NBL_CONST_REF_ARG(Interaction) interaction, NBL_CONST_REF_ARG(MicrofacetCache) cache)
+    {
+        scalar_type d = __ndf_base.template D<MicrofacetCache>(cache);
+        quant_type dmq;
+        dmq.microfacetMeasure = d;  // note: microfacetMeasure/2NdotV
+
+        const scalar_type VdotHLdotH = quant_query.getVdotHLdotH();
+        const scalar_type VdotH_etaLdotH = quant_query.getVdotH_etaLdotH();
+        const bool transmitted = reflect_refract==MTT_REFRACT || (reflect_refract!=MTT_REFLECT && VdotHLdotH < scalar_type(0.0));
+        scalar_type NdotL_over_denominator = _sample.getNdotL(BxDFClampMode::BCM_ABS);
+        if (transmitted)
+                NdotL_over_denominator *= -scalar_type(4.0) * VdotHLdotH / (VdotH_etaLdotH * VdotH_etaLdotH);
+        dmq.projectedLightMeasure = d * NdotL_over_denominator;
+        return dmq;
+    }
+
+    template<class LS, class Interaction, typename C=bool_constant<!IsBSDF> NBL_FUNC_REQUIRES(LightSample<LS> && RequiredInteraction<Interaction>)
+    enable_if_t<C::value && !IsBSDF, quant_type> DG1(NBL_CONST_REF_ARG(dg1_query_type) query, NBL_CONST_REF_ARG(quant_query_type) quant_query, NBL_CONST_REF_ARG(LS) _sample, NBL_CONST_REF_ARG(Interaction) interaction)
+    {
+        scalar_type dg1 = base_type::template DG1<dg1_query_type>(query);
+        quant_type dmq;
+        dmq.microfacetMeasure = dg1;
+        dmq.projectedLightMeasure = dg1;// TODO: figure this out * _sample.getNdotL(BxDFClampMode::BCM_MAX);
+        return dmq;
+    }
+    template<class LS, class Interaction, typename C=bool_constant<IsBSDF> NBL_FUNC_REQUIRES(LightSample<LS> && RequiredInteraction<Interaction>)
+    enable_if_t<C::value && IsBSDF, quant_type> DG1(NBL_CONST_REF_ARG(dg1_query_type) query, NBL_CONST_REF_ARG(quant_query_type) quant_query, NBL_CONST_REF_ARG(LS) _sample, NBL_CONST_REF_ARG(Interaction) interaction)
+    {
+        scalar_type dg1 = base_type::template DG1<dg1_query_type>(query);
+        quant_type dmq;
+        dmq.microfacetMeasure = dg1;  // note: microfacetMeasure/2NdotV
+
+        const scalar_type VdotHLdotH = quant_query.getVdotHLdotH();
+        const scalar_type VdotH_etaLdotH = quant_query.getVdotH_etaLdotH();
+        const bool transmitted = reflect_refract==MTT_REFRACT || (reflect_refract!=MTT_REFLECT && VdotHLdotH < scalar_type(0.0));
+        scalar_type NdotL_over_denominator = _sample.getNdotL(BxDFClampMode::BCM_ABS);
+        if (transmitted)
+                NdotL_over_denominator *= -scalar_type(4.0) * VdotHLdotH / (VdotH_etaLdotH * VdotH_etaLdotH);
+        dmq.projectedLightMeasure = dg1;// TODO: figure this out * NdotL_over_denominator;
+        return dmq;
+    }


This stuff maybe we can rethink, I'd be tempted to allow an NDF "skip" providing D and correlated if it has both the DG1 and Dcorrelated methods (so the cook torrance can leverage that and skip multiplying D with correlated by itself for eval)

devshgraphicsprogramming · 2025-10-08T11:03:19Z

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

    NBL_CONSTEXPR_STATIC_INLINE bool IsBSDF = ndf_type::NDFSurfaceType != ndf::MTT_REFLECT;

+    template<class Interaction, class MicrofacetCache>
+    static bool __checkValid(NBL_CONST_REF_ARG(fresnel_type) f, NBL_CONST_REF_ARG(sample_type) _sample, NBL_CONST_REF_ARG(Interaction) interaction, NBL_CONST_REF_ARG(MicrofacetCache) cache)


why make static and pass the f if its your member?

is it because the fresnel is already properly oriented? then give f a clear name like reorientedFresnel

and since its static you may as well make the __checkValid into a standalone struct functor with a partial spec on IsBSDF, that way you don't call impl::check_TIR_helper but you can call cache.isValid(reorientedFresnel.getRefractionOrientedEta()) directly

actually if you use impl::checkValid<IsBSDF>::template __call() here

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

Lines 350 to 351 in dcc2464

const bool notTIR = impl::check_TIR_helper<fresnel_type, IsBSDF>::template __call<MicrofacetCache>(_f, cache);

assert(notTIR);

then there's no need to have an impl::check_TIR_helper at all

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

devshgraphicsprogramming · 2025-10-09T09:08:25Z

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

+        else
+            dmq.projectedLightMeasure = d * _sample.getNdotL(BxDFClampMode::BCM_MAX);


btw you can always use ABS, because it should be the callee's job to not call you on backfacing BRDF if it chooses to

this is just the NDF

…cache with defaults

devshgraphicsprogramming · 2025-10-20T09:52:57Z

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

+    bool isValid(const scalar_type eta) NBL_CONST_MEMBER_FUNC
    {
+        fresnel::OrientedEtas<monochrome_type> orientedEta = fresnel::OrientedEtas<monochrome_type>::create(scalar_type(1.0), hlsl::promote<monochrome_type>(eta));


I don't get what you're doing here

devshgraphicsprogramming · 2025-10-20T10:01:08Z

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

+    bool isValid(const scalar_type eta) NBL_CONST_MEMBER_FUNC
    {
-        return iso_cache.isValid(orientedEta);
+        return iso_cache.isValid(eta);


I don't get whats going on here

devshgraphicsprogramming · 2025-10-20T10:15:55Z

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

+        const scalar_type viewShortenFactor = hlsl::mix(scalar_type(1.0), rcpOrientedEta.value[0], transmitted);
+        retval.iso_cache.VdotL = VdotH * (VdotH * viewShortenFactor + LdotH) - viewShortenFactor;


even though I gave you this, it might be invalid.

refl -> VdotH*(VdotH+LdotH) - 1 = 2*VdotH^2 -1 trans -> VdotH*(VdotH/eta+LdotH) - 1/eta != VdotH^2/eta - VdotH/eta + VdotH*LdotH

yeh its messed up.

Need to bring back the original with one slight opt

retval.iso_cache.VdotL = VdotH * hlsl::mix( scalar_type(2.0) * VdotH - scalar_type(1.0), VdotH * rcpOrientedEta.value[0] + LdotH - rcpOrientedEta.value[0], transmitted );

keptsecret added 23 commits September 4, 2025 10:46

changes to dual measure quant, old stuff commented out for now

5218545

create query structs in beckmann ndf, removed query concepts, funcs r…

f7525af

…eturn quants

did the same for ggx ndf, some fixes to beckmann

c50db68

Merge branch 'master' into bxdf_fixes_cook_torrance

c49bedb

fixes to ndf concept

319c954

moved eval, quotient/pdf into cook torrance base

4eeacf1

moved generate H into ndfs, generate impl in cook torrance base

e7bc784

use new cook torrance base in microfacet bxdfs

72226bb

Merge branch 'master' into bxdf_fixes_cook_torrance

f34b348

numerous typo bug fixes

ad13044

fixes to ggx ndf

d8d2116

fixed conductor fresnel naming ior as eta

238b08e

minor fixes to beckmann ndf, removed obsolete comments

8bc707f

added luminosity contribution hint to cook torrance bsdfs

9fb28cf

added aniso overloads to isotropic bxdf methods

4cb8a0a

checks for invalid generate sample

392dc31

removed obsolete commented out stuff

02de86e

added cartesian-polar conversions from unit tests

4a7f532

Merge branch 'fix_rotation_mat' into bxdf_fixes_cook_torrance

80c4f67

Merge branch 'master' into bxdf_fixes_cook_torrance

6233bd1

vector hashes

ce5fbac

Merge branch 'fix_rotation_mat' into bxdf_fixes_cook_torrance

47e814b

Merge branch 'master' into bxdf_fixes_cook_torrance

c983975

devshgraphicsprogramming commented Sep 16, 2025

View reviewed changes

include/nbl/builtin/hlsl/bxdf/common.hlsl Outdated Show resolved Hide resolved

devshgraphicsprogramming commented Sep 16, 2025

View reviewed changes

include/nbl/builtin/hlsl/math/functions.hlsl Outdated Show resolved Hide resolved

devshgraphicsprogramming commented Sep 16, 2025

View reviewed changes

include/nbl/builtin/hlsl/cpp_compat/vector.hlsl Outdated Show resolved Hide resolved

keptsecret added 2 commits September 17, 2025 15:26

fix fresnel usage in bxdf pdf

0f2ee0b

fix F calc in generate

b5f02e6

devshgraphicsprogramming commented Sep 17, 2025

View reviewed changes

include/nbl/builtin/hlsl/bxdf/fresnel.hlsl Outdated Show resolved Hide resolved

devshgraphicsprogramming commented Sep 17, 2025

View reviewed changes

include/nbl/builtin/hlsl/bxdf/cook_torrance_base.hlsl Outdated Show resolved Hide resolved