Hlsl bxdfs 3 #899
Hlsl bxdfs 3 #899
Conversation
|
|
||
| scalar_type operator()(SIsotropicParams<scalar_type> params) | ||
| { | ||
| scalar_type nom = exp<scalar_type>( (params.NdotH2 - 1.0) / (params.n_or_a2 * params.NdotH2) ); // exp(x) == exp2(x/log(2)) ? |
There was a problem hiding this comment.
you can slap the log<scalar_type>(2.f) into the a2*NdotH2 expression here
| template<class T, class U> | ||
| struct is_ggx : bool_constant< | ||
| is_same<T, GGX<U> >::value | ||
| > {}; | ||
| } | ||
|
|
||
| template<class T> | ||
| struct is_ggx : impl::is_ggx<T, typename T::scalar_type> {}; | ||
|
|
||
| template<typename T> | ||
| NBL_CONSTEXPR bool is_ggx_v = is_ggx<T>::value; |
There was a problem hiding this comment.
could be a general trait, not hidden in impl::
| // Copyright (C) 2018-2023 - DevSH Graphics Programming Sp. z O.O. | ||
| // This file is part of the "Nabla Engine". | ||
| // For conditions of distribution and use, see copyright notice in nabla.h | ||
| #ifndef _NBL_BUILTIN_HLSL_BXDF_NDF_INCLUDED_ | ||
| #define _NBL_BUILTIN_HLSL_BXDF_NDF_INCLUDED_ | ||
|
|
||
| #include "nbl/builtin/hlsl/limits.hlsl" | ||
| #include "nbl/builtin/hlsl/bxdf/common.hlsl" | ||
|
|
||
| namespace nbl | ||
| { | ||
| namespace hlsl | ||
| { | ||
| namespace bxdf | ||
| { | ||
| namespace ndf | ||
| { |
There was a problem hiding this comment.
the NDF "fixes" the Lambda and Beta functions so it makes sense to have an ndf(this_t::query_type) (instead of just operator(this_t::query_type)) and lambda() methods instead of having the lambda buried somewhere else in anothre header
There was a problem hiding this comment.
actualy ndf should be called D to be consistent with that PBR papers (Cook Torrance)
then vndf is DG1
There was a problem hiding this comment.
So to recap:
- D (the ndf)
- Lambda
- Beta
- DG1
| // iso | ||
| Scalar getNdotV() NBL_CONST_MEMBER_FUNC { return hlsl::mix(math::conditionalAbsOrMax<Scalar>(_clamp == BxDFClampMode::BCM_ABS, interaction.getNdotV(), 0.0), interaction.getNdotV(), _clamp == BxDFClampMode::BCM_NONE); } | ||
| Scalar getNdotVUnclamped() NBL_CONST_MEMBER_FUNC { return interaction.getNdotV(); } | ||
| Scalar getNdotV2() NBL_CONST_MEMBER_FUNC { return interaction.getNdotV2(); } | ||
| Scalar getNdotL() NBL_CONST_MEMBER_FUNC { return hlsl::mix(math::conditionalAbsOrMax<Scalar>(_clamp == BxDFClampMode::BCM_ABS, _sample.getNdotL(), 0.0), _sample.getNdotL(), _clamp == BxDFClampMode::BCM_NONE); } | ||
| Scalar getNdotLUnclamped() NBL_CONST_MEMBER_FUNC { return _sample.getNdotL(); } | ||
| Scalar getNdotL2() NBL_CONST_MEMBER_FUNC { return _sample.getNdotL2(); } | ||
| Scalar getVdotL() NBL_CONST_MEMBER_FUNC { return _sample.getVdotL(); } | ||
| Scalar getNdotH() NBL_CONST_MEMBER_FUNC { return cache.getNdotH(); } | ||
| Scalar getNdotH2() NBL_CONST_MEMBER_FUNC { return cache.getNdotH2(); } | ||
| Scalar getVdotH() NBL_CONST_MEMBER_FUNC { return cache.getVdotH(); } | ||
| Scalar getLdotH() NBL_CONST_MEMBER_FUNC { return cache.getLdotH(); } | ||
|
|
||
| LS _sample; | ||
| SI interaction; | ||
| MC cache; | ||
| BxDFClampMode _clamp; | ||
| }; | ||
| template<class LS, class SI, class MC, typename Scalar> | ||
| NBL_PARTIAL_REQ_TOP(surface_interactions::Anisotropic<SI> && AnisotropicMicrofacetCache<MC>) | ||
| struct GGXParams<LS, SI, MC, Scalar NBL_PARTIAL_REQ_BOT(surface_interactions::Anisotropic<SI> && AnisotropicMicrofacetCache<MC>) > | ||
| { | ||
| using this_t = GGXParams<LS, SI, MC, Scalar>; | ||
|
|
||
| static this_t create(NBL_CONST_REF_ARG(LS) _sample, NBL_CONST_REF_ARG(SI) interaction, NBL_CONST_REF_ARG(MC) cache, BxDFClampMode _clamp) | ||
| { | ||
| this_t retval; | ||
| retval._sample = _sample; | ||
| retval.interaction = interaction; | ||
| retval.cache = cache; | ||
| retval._clamp = _clamp; | ||
| return retval; | ||
| } | ||
|
|
||
| // iso | ||
| Scalar getNdotV() NBL_CONST_MEMBER_FUNC { return hlsl::mix(math::conditionalAbsOrMax<Scalar>(_clamp == BxDFClampMode::BCM_ABS, interaction.getNdotV(), 0.0), interaction.getNdotV(), _clamp == BxDFClampMode::BCM_NONE); } | ||
| Scalar getNdotVUnclamped() NBL_CONST_MEMBER_FUNC { return interaction.getNdotV(); } | ||
| Scalar getNdotV2() NBL_CONST_MEMBER_FUNC { return interaction.getNdotV2(); } | ||
| Scalar getNdotL() NBL_CONST_MEMBER_FUNC { return hlsl::mix(math::conditionalAbsOrMax<Scalar>(_clamp == BxDFClampMode::BCM_ABS, _sample.getNdotL(), 0.0), _sample.getNdotL(), _clamp == BxDFClampMode::BCM_NONE); } | ||
| Scalar getNdotLUnclamped() NBL_CONST_MEMBER_FUNC { return _sample.getNdotL(); } | ||
| Scalar getNdotL2() NBL_CONST_MEMBER_FUNC { return _sample.getNdotL2(); } | ||
| Scalar getVdotL() NBL_CONST_MEMBER_FUNC { return _sample.getVdotL(); } | ||
| Scalar getNdotH() NBL_CONST_MEMBER_FUNC { return cache.getNdotH(); } | ||
| Scalar getNdotH2() NBL_CONST_MEMBER_FUNC { return cache.getNdotH2(); } | ||
| Scalar getVdotH() NBL_CONST_MEMBER_FUNC { return cache.getVdotH(); } | ||
| Scalar getLdotH() NBL_CONST_MEMBER_FUNC { return cache.getLdotH(); } | ||
|
|
||
| // aniso | ||
| Scalar getTdotL2() NBL_CONST_MEMBER_FUNC { return _sample.getTdotL() * _sample.getTdotL(); } | ||
| Scalar getBdotL2() NBL_CONST_MEMBER_FUNC { return _sample.getBdotL() * _sample.getBdotL(); } | ||
| Scalar getTdotV2() NBL_CONST_MEMBER_FUNC { return interaction.getTdotV() * interaction.getTdotV(); } | ||
| Scalar getBdotV2() NBL_CONST_MEMBER_FUNC { return interaction.getBdotV() * interaction.getBdotV(); } | ||
| Scalar getTdotH2() NBL_CONST_MEMBER_FUNC {return cache.getTdotH() * cache.getTdotH(); } | ||
| Scalar getBdotH2() NBL_CONST_MEMBER_FUNC {return cache.getBdotH() * cache.getBdotH(); } |
There was a problem hiding this comment.
we definitely need getXdotY(const BxDFClampMode) on our interaction, sample and cache concepts and skip this insanity
| vector2_type A; | ||
| spectral_type ior0, ior1; |
There was a problem hiding this comment.
you need different BxDF or specializations for anisotropic and isotropic, different amounts of variables to precompute.
| static this_t create(scalar_type NDFcos, scalar_type maxNdotV) | ||
| { | ||
| this_t retval; | ||
| retval.NDFcos = NDFcos; | ||
| if (is_ggx_v<NDF>) | ||
| retval.maxNdotL = maxNdotV; | ||
| else | ||
| retval.maxNdotV = maxNdotV; |
There was a problem hiding this comment.
this makes no sense to me when I'm reading it, why do we have NdotV thats then being assigned to NdotL when we have GGX
There was a problem hiding this comment.
I'm pretty sure this is wrong for GGX, you can't just take the dot product of the view vector and geometrical normal and use it as the dot product of view and geometrical
| scalar_type operator()() | ||
| { | ||
| if (is_ggx_v<NDF>) | ||
| return NDFcos * maxNdotL; | ||
| else | ||
| return 0.25 * NDFcos / maxNdotV; | ||
| } |
There was a problem hiding this comment.
write a comment that this computes the max(NdotL,0)/(4*max(NdotV,0)*max(NdotL,0)) factor which transforms PDFs in the f in projected microfacet f * NdotH measure to projected light measure f * NdotL
| template<typename NDF> | ||
| struct microfacet_to_light_measure_transform<NDF,REFLECT_BIT> | ||
| { | ||
| using this_t = microfacet_to_light_measure_transform<NDF,REFLECT_BIT>; | ||
| using scalar_type = typename NDF::scalar_type; | ||
|
|
||
| static this_t create(scalar_type NDFcos, scalar_type maxNdotV) |
There was a problem hiding this comment.
btw this doesn't need to be a struct, can be a plain old function (if you need partial specs, can be like the tgmath functions calling an impl::helper)
There was a problem hiding this comment.
because right now you're constructing the structs immediately with create and calling operator() on them immediately, also there's literally no input argument you could cache for multiple calls
| using isocache_type = IsoCache; | ||
| using anisocache_type = AnisoCache; |
There was a problem hiding this comment.
why does a smooth BxDF care about the microfacet cache ?
| template<typename T NBL_PRIMARY_REQUIRES(concepts::FloatingPointScalar<T> || concepts::FloatingPointLikeVectorial<T>) | ||
| struct DielectricFrontFaceOnly | ||
| { | ||
| using scalar_type = typename vector_traits<T>::scalar_type; |
There was a problem hiding this comment.
look at your tpedef, your code still wont compile with a scalar T, just give up and require vectorial for Schlick, Conductor, Dielectric and so on
There was a problem hiding this comment.
There was a problem hiding this comment.
It does compile and run with vector_traits<scalar_t> btw. But I've made the change.
Nabla/include/nbl/builtin/hlsl/vector_utils/vector_traits.hlsl
Lines 13 to 16 in 5a1575f
There was a problem hiding this comment.
vector_traits shall not be instantiable for a T=scalar please revert
| @@ -91,6 +86,11 @@ struct ComputeMicrofacetNormal | |||
| return bool((hlsl::bit_cast<unsigned_integer_type>(NdotV) ^ hlsl::bit_cast<unsigned_integer_type>(NdotL)) & unsigned_integer_type(1u)<<(sizeof(scalar_type)*8u-1u)); | |||
| } | |||
|
|
|||
| static bool isValidMicrofacet(const bool transmitted, const scalar_type VdotL, const scalar_type NdotH, const scalar_type eta, const scalar_type rcp_eta) | |||
There was a problem hiding this comment.
why not take orientedEta instead of two separate scalars?
| Refract<scalar_type> refract; | ||
| Refract<scalar_type> _refract; | ||
| vector_type I; | ||
| vector_type N; | ||
| scalar_type NdotI; | ||
| scalar_type NdotTorR; |
There was a problem hiding this comment.
bump #899 (comment)
| vector_type reflect() | ||
| { | ||
| return N * NdotI * 2.0f - I; | ||
| } | ||
|
|
||
| vector_type refract() | ||
| { | ||
| return N * (NdotI * rcpOrientedEta + _refract.NdotT) - I * rcpOrientedEta; | ||
| } |
There was a problem hiding this comment.
not sure why you need those, its literally the same as calling operator()(false) and operator()(true) ( the compiler will optimize out the mix and expressions you wrote out by hand will drop out)
| // when you know you'll reflect | ||
| void recomputeNdotR() | ||
| { | ||
| refract.recomputeNdotI(); | ||
| _refract.recomputeNdotI(); | ||
| NdotI = _refract.NdotI; | ||
| } | ||
|
|
||
| // when you know you'll refract | ||
| void recomputeNdotT(bool backside, scalar_type _NdotI2, scalar_type rcpOrientedEta2) | ||
| { | ||
| refract.recomputeNdotT(backside, _NdotI2, rcpOrientedEta2); | ||
| _refract.recomputeNdotT(backside, _NdotI2, rcpOrientedEta2); | ||
| } |
There was a problem hiding this comment.
see #899 (comment)
There was a problem hiding this comment.
and #899 (comment)
| static this_t create(bool r, NBL_CONST_REF_ARG(vector_type) I, NBL_CONST_REF_ARG(vector_type) N, scalar_type NdotI, scalar_type NdotTorR, scalar_type rcpOrientedEta) | ||
| static this_t create(NBL_CONST_REF_ARG(vector_type) I, NBL_CONST_REF_ARG(vector_type) N, scalar_type NdotI, scalar_type rcpOrientedEta) | ||
| { | ||
| this_t retval; | ||
| retval.I = I; | ||
| retval.N = N; | ||
| retval.NdotI = NdotI; | ||
| retval.NdotTorR = NdotTorR; | ||
| retval.rcpOrientedEta = rcpOrientedEta; | ||
| return retval; | ||
| } | ||
|
|
||
| static this_t create(bool r, NBL_CONST_REF_ARG(Refract<scalar_type>) refract, scalar_type rcpOrientedEta) | ||
| static this_t create(NBL_CONST_REF_ARG(Refract<scalar_type>) refract, scalar_type rcpOrientedEta) | ||
| { | ||
| this_t retval; | ||
| retval.I = refract.I; | ||
| retval.N = refract.N; | ||
| retval.NdotI = refract.NdotI; | ||
| retval.NdotTorR = hlsl::mix(refract.NdotI, refract.NdotT, r); | ||
| retval.rcpOrientedEta = rcpOrientedEta; | ||
| return retval; | ||
| } |
There was a problem hiding this comment.
both create are useless, one can fill out the struct members without much issue by themselves
| vector_type operator()(const bool doRefract) | ||
| { | ||
| return N * (NdotI * (hlsl::mix<scalar_type>(1.0f, rcpOrientedEta, doRefract)) + NdotTorR) - I * (hlsl::mix<scalar_type>(1.0f, rcpOrientedEta, doRefract)); | ||
| return N * (NdotI * (hlsl::mix<scalar_type>(1.0f, rcpOrientedEta, doRefract)) + hlsl::mix(NdotI, _refract.NdotT, doRefract)) - I * (hlsl::mix<scalar_type>(1.0f, rcpOrientedEta, doRefract)); |
There was a problem hiding this comment.
the NdotTorR should come from outside and not be computed as a member
having it as a member, getting recomputed, storing it gives you a really bad choice between:
- no optimization (can't provide your own dot product value)
- user-error prone (someone forgets to call
recomputebeforeoperator()
Description
Continuing #811 due to GH UI messing up diffs again
Testing
TODO list: