-
Notifications
You must be signed in to change notification settings - Fork 67
Iridescent fresnel, bxdfs #918
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
| // 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) | ||
| { | ||
| T cosTheta_2 = cosTheta * cosTheta; | ||
| T sinTheta2 = hlsl::promote<T>(1.0) - cosTheta_2; | ||
| const T eta = orientedEta; | ||
| const T eta2 = eta*eta; | ||
| const T etak = orientedEtak; | ||
| const T etak2 = etak*etak; | ||
|
|
||
| const T etaLen2 = eta2 + etak2; | ||
| assert(hlsl::all(etaLen2 > hlsl::promote<T>(hlsl::exp2<scalar_type>(-numeric_limits<scalar_type>::digits)))); | ||
| T t1 = etaLen2 * cosTheta_2; | ||
| const T etaCosTwice = eta * cosTheta * scalar_type(2.0); | ||
|
|
||
| const T rs_common = etaLen2 + cosTheta_2; | ||
| Rs = (rs_common - etaCosTwice) / (rs_common + etaCosTwice); | ||
| const T rp_common = t1 + hlsl::promote<T>(1.0); | ||
| Rp = (rp_common - etaCosTwice) / (rp_common + etaCosTwice); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make a new struct for this e.g. ThinFilmConductor, its separate functionality, same for the dielectric
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will note that it's the same calculation all the way until the end where the regular fresnel just returns (Rs + Rp) * 0.5 instead of inout Rs, Rp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
then return complex_t<T> and treat Rs as Real and Rp as imag, or make a struct to hold Polarized Reflectance and DRY the code
| template<class Config NBL_PRIMARY_REQUIRES(config_concepts::MicrofacetConfiguration<Config>) | ||
| struct SIridescent | ||
| { | ||
| using this_t = SIridescent<Config>; | ||
| NBL_BXDF_CONFIG_ALIAS(scalar_type, Config); | ||
| NBL_BXDF_CONFIG_ALIAS(vector2_type, Config); | ||
| NBL_BXDF_CONFIG_ALIAS(vector3_type, Config); | ||
| NBL_BXDF_CONFIG_ALIAS(ray_dir_info_type, Config); | ||
|
|
||
| NBL_BXDF_CONFIG_ALIAS(isotropic_interaction_type, Config); | ||
| NBL_BXDF_CONFIG_ALIAS(anisotropic_interaction_type, Config); | ||
| NBL_BXDF_CONFIG_ALIAS(sample_type, Config); | ||
| NBL_BXDF_CONFIG_ALIAS(spectral_type, Config); | ||
| NBL_BXDF_CONFIG_ALIAS(quotient_pdf_type, Config); | ||
| NBL_BXDF_CONFIG_ALIAS(isocache_type, Config); | ||
| NBL_BXDF_CONFIG_ALIAS(anisocache_type, Config); | ||
|
|
||
| using ndf_type = ndf::GGX<scalar_type, false>; | ||
| using fresnel_type = fresnel::Iridescent<spectral_type>; | ||
| using measure_transform_type = ndf::SDualMeasureQuant<scalar_type,true,ndf::MTT_REFLECT>; | ||
|
|
||
| NBL_CONSTEXPR_STATIC_INLINE BxDFClampMode _clamp = BxDFClampMode::BCM_MAX; | ||
|
|
||
| struct SCreationParams | ||
| { | ||
| scalar_type A; | ||
| scalar_type thickness; // thin-film thickness in nm | ||
| spectral_type ior0; | ||
| spectral_type ior1; | ||
| spectral_type ior2; | ||
| spectral_type iork2; | ||
| }; | ||
| using creation_type = SCreationParams; | ||
|
|
||
| struct SIridQuery | ||
| { | ||
| using scalar_type = scalar_type; | ||
|
|
||
| scalar_type getDevshV() NBL_CONST_MEMBER_FUNC { return devsh_v; } | ||
| scalar_type getDevshL() NBL_CONST_MEMBER_FUNC { return devsh_l; } | ||
|
|
||
| scalar_type devsh_v; | ||
| scalar_type devsh_l; | ||
| }; | ||
| using query_type = SIridQuery; | ||
|
|
||
| static this_t create(scalar_type A, scalar_type thickness, NBL_CONST_REF_ARG(spectral_type) ior0, NBL_CONST_REF_ARG(spectral_type) ior1, NBL_CONST_REF_ARG(spectral_type) ior2, NBL_CONST_REF_ARG(spectral_type) iork2) | ||
| { | ||
| this_t retval; | ||
| retval.__base.ndf.A = vector2_type(A, A); | ||
| retval.__base.ndf.a2 = A*A; | ||
| retval.__base.ndf.one_minus_a2 = scalar_type(1.0) - A*A; | ||
| retval.__base.fresnel.Dinc = thickness; | ||
| retval.__base.fresnel.ior1 = ior0; | ||
| retval.__base.fresnel.ior2 = ior1; | ||
| retval.__base.fresnel.ior3 = ior2; | ||
| retval.__base.fresnel.iork3 = iork2; | ||
| return retval; | ||
| } | ||
| static this_t create(NBL_CONST_REF_ARG(creation_type) params) | ||
| { | ||
| return create(params.A, params.thickness, params.ior0, params.ior1, params.ior2, params.iork2); | ||
| } | ||
|
|
||
| query_type createQuery(NBL_CONST_REF_ARG(sample_type) _sample, NBL_CONST_REF_ARG(isotropic_interaction_type) interaction) | ||
| { | ||
| query_type query; | ||
| ndf_type ggx_ndf = __base.getNDF(); | ||
| query.devsh_v = ggx_ndf.devsh_part(interaction.getNdotV2()); | ||
| query.devsh_l = ggx_ndf.devsh_part(_sample.getNdotL2()); | ||
| return query; | ||
| } | ||
|
|
||
| spectral_type eval(NBL_CONST_REF_ARG(query_type) query, NBL_CONST_REF_ARG(sample_type) _sample, NBL_CONST_REF_ARG(isotropic_interaction_type) interaction, NBL_CONST_REF_ARG(isocache_type) cache) | ||
| { | ||
| if (_sample.getNdotL() > numeric_limits<scalar_type>::min && interaction.getNdotV() > numeric_limits<scalar_type>::min) | ||
| { | ||
| struct SGGXG2XQuery | ||
| { | ||
| using scalar_type = scalar_type; | ||
|
|
||
| scalar_type getDevshV() NBL_CONST_MEMBER_FUNC { return devsh_v; } | ||
| scalar_type getDevshL() NBL_CONST_MEMBER_FUNC { return devsh_l; } | ||
| BxDFClampMode getClampMode() NBL_CONST_MEMBER_FUNC { return _clamp; } | ||
|
|
||
| scalar_type devsh_v; | ||
| scalar_type devsh_l; | ||
| BxDFClampMode _clamp; | ||
| }; | ||
|
|
||
| SGGXG2XQuery g2_query; | ||
| g2_query.devsh_v = query.getDevshV(); | ||
| g2_query.devsh_l = query.getDevshL(); | ||
| g2_query._clamp = _clamp; | ||
|
|
||
| measure_transform_type dualMeasure = __base.template __DG<SGGXG2XQuery>(g2_query, _sample, interaction, cache); | ||
| dualMeasure.maxNdotL = _sample.getNdotL(_clamp); | ||
| scalar_type DG = dualMeasure.getProjectedLightMeasure(); | ||
| fresnel_type f = __base.getFresnel(); | ||
| f.absCosTheta = cache.getLdotH(); | ||
| return f() * DG; | ||
| } | ||
| else | ||
| return hlsl::promote<spectral_type>(0.0); | ||
| } | ||
|
|
||
| sample_type generate(NBL_CONST_REF_ARG(isotropic_interaction_type) interaction, const vector2_type u, NBL_REF_ARG(isocache_type) cache) | ||
| { | ||
| SGGXAnisotropic<Config> ggx_aniso = SGGXAnisotropic<Config>::create(__base.ndf.A.x, __base.ndf.A.y, __base.fresnel.ior3/__base.fresnel.ior2, __base.fresnel.iork3/__base.fresnel.ior2); | ||
| anisocache_type anisocache; | ||
| sample_type s = ggx_aniso.generate(anisotropic_interaction_type::create(interaction), u, anisocache); | ||
| cache = anisocache.iso_cache; | ||
| return s; | ||
| } | ||
|
|
||
| scalar_type pdf(NBL_CONST_REF_ARG(query_type) query, NBL_CONST_REF_ARG(isotropic_interaction_type) interaction, NBL_CONST_REF_ARG(isocache_type) cache) | ||
| { | ||
| struct SGGXDG1Query | ||
| { | ||
| using scalar_type = scalar_type; | ||
|
|
||
| scalar_type getNdf() NBL_CONST_MEMBER_FUNC { return ndf; } | ||
| scalar_type getG1over2NdotV() NBL_CONST_MEMBER_FUNC { return G1_over_2NdotV; } | ||
|
|
||
| scalar_type ndf; | ||
| scalar_type G1_over_2NdotV; | ||
| }; | ||
|
|
||
| SGGXDG1Query dg1_query; | ||
| ndf_type ggx_ndf = __base.getNDF(); | ||
| dg1_query.ndf = __base.__D(cache); | ||
|
|
||
| const scalar_type devsh_v = query.getDevshV(); | ||
| dg1_query.G1_over_2NdotV = ggx_ndf.G1_wo_numerator_devsh_part(interaction.getNdotV(_clamp), devsh_v); | ||
|
|
||
| measure_transform_type dualMeasure = __base.template __DG1<SGGXDG1Query>(dg1_query); | ||
| return dualMeasure.getMicrofacetMeasure(); | ||
| } | ||
|
|
||
| quotient_pdf_type quotient_and_pdf(NBL_CONST_REF_ARG(query_type) query, NBL_CONST_REF_ARG(sample_type) _sample, NBL_CONST_REF_ARG(isotropic_interaction_type) interaction, NBL_CONST_REF_ARG(isocache_type) cache) | ||
| { | ||
| scalar_type _pdf = pdf(query, interaction, cache); | ||
|
|
||
| spectral_type quo = hlsl::promote<spectral_type>(0.0); | ||
| if (_sample.getNdotL() > numeric_limits<scalar_type>::min && interaction.getNdotV() > numeric_limits<scalar_type>::min) | ||
| { | ||
| struct SGGXG2XQuery | ||
| { | ||
| using scalar_type = scalar_type; | ||
|
|
||
| scalar_type getDevshV() NBL_CONST_MEMBER_FUNC { return devsh_v; } | ||
| scalar_type getDevshL() NBL_CONST_MEMBER_FUNC { return devsh_l; } | ||
| BxDFClampMode getClampMode() NBL_CONST_MEMBER_FUNC { return _clamp; } | ||
|
|
||
| scalar_type devsh_v; | ||
| scalar_type devsh_l; | ||
| BxDFClampMode _clamp; | ||
| }; | ||
|
|
||
| ndf_type ggx_ndf = __base.getNDF(); | ||
|
|
||
| SGGXG2XQuery g2_query; | ||
| g2_query.devsh_v = query.getDevshV(); | ||
| g2_query.devsh_l = query.getDevshL(); | ||
| g2_query._clamp = _clamp; | ||
| const scalar_type G2_over_G1 = ggx_ndf.template G2_over_G1<SGGXG2XQuery, sample_type, isotropic_interaction_type, isocache_type>(g2_query, _sample, interaction, cache); | ||
|
|
||
| fresnel_type f = __base.getFresnel(); | ||
| f.absCosTheta = cache.getLdotH(); | ||
| const spectral_type reflectance = f(); | ||
| quo = reflectance * G2_over_G1; | ||
| } | ||
|
|
||
| return quotient_pdf_type::create(quo, _pdf); | ||
| } | ||
|
|
||
| SCookTorrance<Config, ndf_type, fresnel_type, measure_transform_type> __base; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could have just added a Delta NDF, and then used a CookTorrance with Iridescent Fresnel
| vector_type iork3; | ||
| vector_type eta12; // outside (usually air 1.0) -> thin-film IOR | ||
| vector_type eta23; // thin-film -> base material IOR | ||
| vector_type etak23; // thin-film -> complex component, k==0 makes dielectric |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what are the pros and const of specializing and not having this member when you don't support Transmission
| using monochrome_type = vector<scalar_type, 1>; | ||
| using vector_type = T; // assert dim==3? | ||
|
|
||
| static this_t create(scalar_type Dinc, vector_type ior1, vector_type ior2, vector_type ior3, vector_type iork3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make a creation struct, too many parameters to a function with same type, easy to mess up
| struct colorspace_base | ||
| { | ||
| NBL_CONSTEXPR_STATIC_INLINE float32_t wavelength_R = 580.0f; | ||
| NBL_CONSTEXPR_STATIC_INLINE float32_t wavelength_G = 550.0f; | ||
| NBL_CONSTEXPR_STATIC_INLINE float32_t wavelength_B = 450.0f; | ||
| // default CIE RGB primaries wavelengths | ||
| NBL_CONSTEXPR_STATIC_INLINE float32_t wavelength_R = 700.0f; | ||
| NBL_CONSTEXPR_STATIC_INLINE float32_t wavelength_G = 546.1f; | ||
| NBL_CONSTEXPR_STATIC_INLINE float32_t wavelength_B = 435.8f; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't even, each colorspace just needs to provide them, end of
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the base inheritance makes no sense
|
In this PR also resolve all left over comments from #930 |
…on + takes Spectrum template type
…ixThroughputWeights
| ((NBL_CONCEPT_REQ_EXPR_RET_TYPE)((iso.getLuminosityContributionHint()), ::nbl::hlsl::is_same_v, typename T::spectral_type)) | ||
| ((NBL_CONCEPT_REQ_EXPR_RET_TYPE)((iso.getPrefixThroughputWeights()), ::nbl::hlsl::is_same_v, typename T::spectral_type)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd only have getLuminosityContributionHint
| spectral_type luminosityContributionHint; | ||
| spectral_type throughputWeights; // product of all quotients so far |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you only need luminosityContributionHint, so you're not doing the abs sum and div on every invocation
everything multiplied up to this point basically
| quant_type dmq; | ||
| return dmq; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're returning an uninitialized variable, the values will be bogus
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it enough that we check infinity bool and not use the return value? Or just for safety
| if (isInfinity) | ||
| return dmq; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again, undef var
| template<typename PH=fresnel_type NBL_FUNC_REQUIRES(!fresnel::TwoSidedFresnel<PH>) | ||
| static fresnel_type getOrientedFresnel(NBL_CONST_REF_ARG(fresnel_type) fresnel, scalar_type NdotV) | ||
| { | ||
| // expect conductor fresnel | ||
| return fresnel; | ||
| } | ||
| template<typename PH=fresnel_type NBL_FUNC_REQUIRES(fresnel::TwoSidedFresnel<PH>) | ||
| static fresnel_type getOrientedFresnel(NBL_CONST_REF_ARG(fresnel_type) fresnel, scalar_type NdotV) | ||
| { | ||
| return fresnel.getReorientedFresnel(NdotV); | ||
| } | ||
|
|
||
| template<class Interaction=conditional_t<IsAnisotropic,anisotropic_interaction_type,isotropic_interaction_type>, | ||
| class MicrofacetCache=conditional_t<IsAnisotropic,anisocache_type,isocache_type>, typename C=bool_constant<!IsBSDF> > | ||
| static enable_if_t<C::value && !IsBSDF, bool> checkValid(NBL_CONST_REF_ARG(fresnel_type) orientedFresnel, NBL_CONST_REF_ARG(sample_type) _sample, NBL_CONST_REF_ARG(Interaction) interaction, NBL_CONST_REF_ARG(MicrofacetCache) cache) | ||
| { | ||
| return _sample.getNdotL() > numeric_limits<scalar_type>::min && interaction.getNdotV() > numeric_limits<scalar_type>::min; | ||
| } | ||
| template<class Interaction=conditional_t<IsAnisotropic,anisotropic_interaction_type,isotropic_interaction_type>, | ||
| class MicrofacetCache=conditional_t<IsAnisotropic,anisocache_type,isocache_type>, typename C=bool_constant<IsBSDF> > | ||
| static enable_if_t<C::value && IsBSDF, bool> checkValid(NBL_CONST_REF_ARG(fresnel_type) orientedFresnel, NBL_CONST_REF_ARG(sample_type) _sample, NBL_CONST_REF_ARG(Interaction) interaction, NBL_CONST_REF_ARG(MicrofacetCache) cache) | ||
| { | ||
| fresnel::OrientedEtas<monochrome_type> orientedEta = fresnel::OrientedEtas<monochrome_type>::create(scalar_type(1.0), hlsl::promote<monochrome_type>(orientedFresnel.getRefractionOrientedEta())); | ||
| return cache.isValid(orientedEta); | ||
| } | ||
|
|
||
| bool dotIsUnity(const vector3_type a, const vector3_type b) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefix the methods with __ to indicate privateness
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also use NBL_FUNC_REQUIRES instead of enable_if_t on the return type
| if (isInfinity) | ||
| if (isInfinity) // after all calls setting DG, allows compiler to throw away calls to ndf.D if using overwriteDG |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better comment, you need to specifically call out why its not combined with the !isInfinity above the overwriteDG
| { | ||
| const scalar_type reflectance = _f(hlsl::abs(cache.getVdotH()))[0]; | ||
| spectral_type prefixThroughputWeights = interaction.getPrefixThroughputWeights(); | ||
| const scalar_type reflectance = hlsl::dot(impl::__implicit_promote<spectral_type, typename fresnel_type::vector_type>::__call(_f(hlsl::abs(cache.getVdotH()))), prefixThroughputWeights); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok you don't want to promote a 1D value to 3D to do a dot product which you know weights to 1.f
You want to do an
fresnel_type::vector_type fresnelValue = _f(hlsl::abs(cache.getVdotH()))[0];
NBL_IF_CONSTEXPR(fresnel_type::monochrome)
{
reflectance = fresnelValue;
}
else
{
reflectance = hlsl::dot(interaction.getPrefixThroughputWeights(),fresnelValue);
}Also something somewhere (the Config) needs to check that the Fresnel operator()'s return type is either monochrome or the same as Interaction's spectral_type
Finally you probably need this commonalized into some pseudo-private method for the BSDF cook torrance, since you need it for:
- generate to decide reflection vs refraction
- pdf to compute the pdf
- quotient_and_pdf to compute both
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw fun fact, you can do fresnelVal = mix(reflectance,hlsl::promote<fresnel_type::vector_type>(1.0)-reflectance,cache.isTransmission()) first, and then just compute fresnelVal/dot(lumaCoeffs,fresnelVal)
because of the way everything is a linear combo and weights add up to 1
P.S. Obivously don't do the division and dot product if Fresnel is monochrome #918 (comment)
| spectral_type prefixThroughputWeights = interaction.getPrefixThroughputWeights(); | ||
| const scalar_type reflectance = hlsl::dot(impl::__implicit_promote<spectral_type, typename fresnel_type::vector_type>::__call(_f(hlsl::abs(VdotH))), prefixThroughputWeights); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see the other comment about avoiding dot product when fresnel is monochrome and commonalizing
| runningSum.imag(hlsl::mix(bias, bias + numbers::pi<T>, reverse)); | ||
| runningSum.imag(ieee754::flipSign<T>(d, reverse)); | ||
|
|
||
| wraparound += hlsl::mix(0u, 1u, reverse); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think if (reverse) wraparound++; would work more efficiently, also why not rename reverse into overflow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually I have even better idea
The reverse thing just simply says if the result needs to be rotated by 180 degrees because the real answer was supposed to be mode than PI and it got PI subtracted from it.
I actually have some ideas whether we can skip the flipSign on every addAngle and just do it at the end in getSumOfArccos, we'll investiagate them later, can write down a TODO there
| } | ||
|
|
||
| complex_t<T> runningSum; | ||
| uint16_t wraparound; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment that it counts in pi (half revolutions)
| static this_t create(T cosA, T sinA) | ||
| { | ||
| this_t retval; | ||
| retval.runningSum = complex_t<T>::create(cosA, T(0)); | ||
| retval.runningSum = complex_t<T>::create(cosA, sinA); | ||
| // retval.runningSum.real(cosA); | ||
| // retval.runningSum.imag(sinA); | ||
| retval.wraparound = 0u; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fun fact, add this to your unit test
accumulator_t::create(cos(6),sin(6)).getSumofArccos() >5.99you should be setting wraparound = 1 whenever sine here is negative
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or assert that sinA>=0
| const T c = a * b - sqrt<T>((T(1.0) - a * a) * (T(1.0) - b * b)); | ||
| const T cosB = runningSum.real(); | ||
| const T sinB = runningSum.imag(); | ||
| const bool reverse = abs<T>(min<T>(a, cosB)) > max<T>(a, cosB); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
write down the logic for how this got derived, summarize https://www.linkedin.com/pulse/spherical-triangle-sampling-only-onearccos-call-matt-kielan/?trackingId=9vN%2F1ttORoKiriQfFkC0Ug%3D%3D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the whole logic here depends on the angle being added being less than PI, I'm not sure this identity holds for when this angle is [0,PI] but angle being added is [0,2*PI)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw let me break it down
bool overflow;
if (a<cosB) // angle B large enough to "fill the gap" in pi-A
overflow = cosB<abs(a);
else // angle A large enough to "fill the gap" in pi-B
overflow = a<abs(cosB);
yeah I think this expects both angles are [0,PI] (runningSum.y and sinA is always positive)
I think you could get away with checking the sign of d below to check for overflow (if negative you have overflow)
| // apply triple angle formula | ||
| const T absArccosSumABC = acos<T>(clamp<T>(cosSumAB * cosC - (cosA * sinB + sinA * cosB) * sinC, T(-1.0), T(1.0))); | ||
| return ((AltminusB ? ABltC : ABltminusC) ? (-absArccosSumABC) : absArccosSumABC) + ((AltminusB || ABltminusC) ? numbers::pi<T> : (-numbers::pi<T>)); | ||
| return acos<T>(runningSum.real()) + wraparound * numbers::pi<T>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cast wraparound to float, or even keep it as a float I guess
| void addCosine(T cosA, T biasA) | ||
| void addCosine(T cosA, T sinA) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be called addAngle cause its a sine and cosine
| const vector3_type localH = ndf.generateH(localV, u); | ||
| const scalar_type VdotH = hlsl::dot(localV, localH); | ||
| assert(VdotH >= scalar_type(0.0)); // VNDF sampling guarantees VdotH has same sign as NdotV (should be positive for BRDF) | ||
| cache = anisocache_type::createPartial(VdotH, LdotH, localH.z, transmitted, rcpEta); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be done in BSDF postlude, then you don't need to take a cache argument at all
…ncept, iridescent bxdf use cook torrance base
| retval.helper.thinFilmIor = ior2; | ||
| retval.helper.eta12 = ior2/ior1; | ||
| retval.helper.eta23 = ior3/ior2; | ||
| retval.helper.etak23 = hlsl::promote<vector_type>(0.0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you could get this #918 (comment) done via CRTP where you feed this class into the helper class and it has getEta12() etc getters
or the other way round, a base class with getters and it goes into helper class without Curious Recurrence
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually base class with just getters and mambers could also solve #918 (comment)
| template<typename C> | ||
| struct traits<bxdf::reflection::SIridescent<C> > | ||
| { | ||
| NBL_CONSTEXPR_STATIC_INLINE BxDFType type = BT_BRDF; | ||
| NBL_CONSTEXPR_STATIC_INLINE bool IsMicrofacet = true; | ||
| NBL_CONSTEXPR_STATIC_INLINE bool clampNdotV = true; | ||
| NBL_CONSTEXPR_STATIC_INLINE bool clampNdotL = true; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you not make a partial spec which catches SCookTorrance aliases?
No description provided.