At first I thought that this needs to return un-squared coefficients, so I used average(sqr(fresnel_dielectric_polarized(...)))
in fresnel_dielectric
. But with the code as it is now, I don't…
LGTM now. The only open question from my side is the differentials topic, but I'd be fine with keeping it as-is.
The code here wasn't accounting for the thin film yet, this is fixed now.
Good point. If we're already handling units, might as well do it properly: I've created #120900 for this, depending on what PR gets merged first I'll just update the other to tag this input correctly.
Not really sure who to add as reviewers here, please feel free to add yourself/others.
Yes, I've noticed that and updated the code accordingly. What I meant is that the current code does
const float r_p = (cos_theta_t + eta * cos_theta_i) / (cos_theta_t - eta * cos_theta_i);
…
Good point, thanks, those are indeed very similar. I'll merge them together.
Comparing the two, I think I found a sign error in the current code. So far it wouldn't have mattered because we only…
Mostly LGTM now, just two nitpicks and one detail that I'm not certain about.