[Soc-2018-dev] Weekly report #08 - Implementing a Hair Shader for Cycles

[Leonardo E. Segovia]

Hey Brecht,

Thanks for the review! Comments on your thoughts are below.

El dom., 8 de jul. de 2018 a la(s) 19:31, Brecht Van Lommel
(brechtvanlommel at gmail.com) escribió:
>
> Regarding the reference render charts, it's best to not include any text in them, so that it can be translated to other languages. Any credits can then also be put at the bottom of the page instead of repeated in each image.
>
I've just uploaded charts for the remaining properties. These are all
done in Inkscape now, so translations should not be an issue
(obviously, I'll upload the sources once you approve their look). As
regards the credits, I'll clean them up ASAP.

>
> It also would be good if the image were not square, it's better if they have dimensions like 1024x256 so that more fits on a page, the entire height of the curl is not really needed.
>
I have tried to fit the renders so that they leave space for the text.
That's why charts like the ex-Roughness Randomization, etc. (those
with only one parameter) are exactly these dimensions (and besides,
CUDA tile size is a perfect match). The only one excessively large is
the Roughness v. Radial Roughness one, for obvious reasons.

>
> It did a deeper review of the principled hair shader code, here are my notes.
>
> We named undercoat incorrectly, it is the primary reflection roughness which is not the same. I got confused because there two different effects described in section 4.3 of the paper, and we only implement the first part. I still don't like the Primary Reflection Roughness though. The purpose of this is to simulate a shiny coat, so perhaps we can name it just "Coat" that is (1 - primary_reflection_roughness) and defaults to 0.0.
>
The parameter explained in the first part of 4.3 is named
"Glossiness" in V-Ray, which is appropriate since that's the intended
use. Wouldn't that be a better name? I agree with the remapping, and
will update that.
The second part (i.e. how to actually simulate undercoat) is a
roughness modifier applied to those hairs in certain parts of the
surface. This may be achieved by manipulating the relevant sockets
from an UV node, so I don't think we need to do further work there.

>
> I also would still like to see the Melanin become a slider in the 0..1 range, as we try to do for parameters in shaders whenever possible. I did some tests and found this mapping to work pretty well, basically using a log() to invert the exp() that will be applied to the absorption, and scaling the melanin coefficients. Note this means Melanin becomes a PROP_FACTOR with max 1.0.
>
> /* Map melanin 0..inf to more perceptually linear 0..1. */
> melanin_qty = -logf(fmaxf(1.0f - melanin_qty, 0.0001f));
> ...
> /* Normalized absorption coefficients to 1.0. */
> float3 melanin_sigma = eumelanin*make_float3(0.506f, 0.841f, 1.653f) + pheomelanin*make_float3(0.343f, 0.733f, 1.924f);
>
Good find! I had an idea that a logarithmic approach would do the job,
but I did not know the specifics (nor I wanted to mess with a known
working approach).

> And some smaller notes:
> * Bug: the OSL shader is using the longitudinal roughness instead of the azimuthal roughness for the reflectance to absorption mapping.
> * Can this mapping be made into a separate function? Since it's used both for tint and color would be nice not to duplicate code.
> * The Random socket should use SOCK_HIDE_VALUE to make it clear the value is ignored if nothing is connected.
> * In the OSL shader, use lower_case_names for local variables, only the parameters should use CamelCase.
> * Before merging, any debug code like printf's, curve_center(), and other commented out code should be removed.
> * No need to store a random value in the PrincipledHairBSDF and have it as a parameter in the OSL closure if we are not going to use it.
> * As an optimization, variables needed for only one of the 3 color parameterizations should be computed in the appropriate if/case instead of before. So factor_random_color, pheomelanin, etc. should only be computed for the Melanin parameterization.
> * For the Color parametrization, can we set the default to a brown color similar to the melanin one? Doesn't have to be an exact match, just something better than white.
> * Suggest to rename "Color Randomization" to "Random Color", sounds a little less technical and to the point. Same for "Roughness Randomization".
> * In nodes.h, rename "float melanin_concentration" to "float melanin" and "float melanin_redness_ratio" to "float melanin_redness", these should match the socket names.
>
All these have been noted, I'll push the necessary fixes during the week.

I'd still like your word on the new charts I pushed (Roughness,
Roughness Randomization and Undercoat Roughness). Please let me know
if there are any improvements to make, as I'll standardize the Melanin
charts with them.

Thanks again for the review!

Best regards,
Leonardo


--
Lic. Leonardo E. Segovia
Departamento de Ciencias e Ingeniería de la Computación
Universidad Nacional del Sur
San Andrés 800 - Campus Palihue, B8000 Bahía Blanca, Argentina