This is trivial, but I think writing hg->g = fast_expf(-0.0990567f / (d - 1.67154f));
inside the if (hg)
condition etc. makes it more clear to which phase functions these parameters are associated to.
Should add a comment that this function is for path guiding.
It's not obvious what sin2_theta_2
means. I would suggest to rename it sin_half_theta_sq
and add/or a comment saying it's sin^2(theta / 2)
.
This condition is incorrect, it should be the same as phase_draine()
.
The constraint of positive x in the comment is easy to miss. Better adding kernel_assert()
Mostly looks fine to me, have some suggestions about naming
Not sure if you want to split the commits when merging, if you do then in 3945ce06f9
it should be *wo = X * localO.Z + Y * localO.X + Z * localO.Y
(You wrote localO.X
twice).
The to_global()
/to_local()
and template change is gone?
@susman This change will be part of Blender 4.3, for now you can download the latest 4.3 Alpha build here https://builder.blender.org/download/daily/
This formulation has a singularity when s_theta == 1
, which causes many fireflies when looking again the light source, can you deal with this special case?
You can just write float v = -logf(2.0f * B * (s90 - 1.0f) + 1.0f) / logf(s90);
to save some computation.
The most important thing is to make it clear what the arguments are and what this function computes. I guess you can add a comment above the function:
/* Given a random number `rand`, sample…
For Volume Scatter the new mode is not necessary, the colors are allowed to go above 1 so you can just put your per channel density there. For Volume Absorption I admit the new parametrization is…