Fix #130389: Cycles: Numerical issues in GGX_D with associative math flag

Turns out that with `-fassociative-math`, GCC turns
`(1.0f - cos_NH2) + alpha2 * cos_NH2` into
`cos_NH2 * (alpha2 - 1.0f) + 1.0f`.

Not sure why since the operation count is the same, but if alpha2 is very
small, `alpha2 - 1.0f` will be exactly -1.0f, which then causes issues.

Luckily, having one_minus_cos_NH2 as its own variable appears to be enough to
make GCC keep the original formulation.
Just to be safe, I've also used one_minus_cos_NH2 in the other branch to
hopefully reduce the chance of it being folded in again. Also turns a
division into a reciprocal, which is in theory slightly faster.

Pull Request: https://projects.blender.org/blender/blender/pulls/130469
This commit is contained in:
Lukas Stockner
2024-11-18 19:12:22 +01:00
committed by Thomas Dinges
parent 9e82331ae6
commit 1a40efbded

View File

@@ -509,13 +509,14 @@ ccl_device_inline float bsdf_G(float alpha2, float cos_NI, float cos_NO)
template<MicrofacetType m_type> ccl_device_inline float bsdf_D(float alpha2, float cos_NH)
{
const float cos_NH2 = min(sqr(cos_NH), 1.0f);
const float one_minus_cos_NH2 = 1.0f - cos_NH2;
if (m_type == MicrofacetType::BECKMANN) {
return expf((cos_NH2 - 1.0f) / (cos_NH2 * alpha2)) / (M_PI_F * alpha2 * sqr(cos_NH2));
return 1.0f / (expf(one_minus_cos_NH2 / (cos_NH2 * alpha2)) * M_PI_F * alpha2 * sqr(cos_NH2));
}
else {
kernel_assert(m_type == MicrofacetType::GGX);
return alpha2 / (M_PI_F * sqr((1.0f - cos_NH2) + alpha2 * cos_NH2));
return alpha2 / (M_PI_F * sqr(one_minus_cos_NH2 + alpha2 * cos_NH2));
}
}