Cycles: new Microfacet-based Hair BSDF with elliptical cross-section support #105600

Merged
Weizhen Huang merged 114 commits from weizhen/blender:microfacet_hair into main 2023-08-18 12:46:20 +02:00
Member

Implements the paper A Microfacet-based Hair Scattering Model by Weizhen Huang, Matthias B. Hullin and Johannes Hanika.

Features:

  • This is a far-field model, as opposed to the near-field Principled Hair BSDF model. The hair is expected to be less noisy, but lower roughness values takes longer to render due to numerical integration along the hair width. The hair also appears to be flat when viewed up-close.
  • The longitudinal width of the scattering lobe differs along the azimuth, providing a higher contrast compared to the evenly spread scattering in the near-field Principled Hair BSDF model. For a more detailed comparison, please refer to the original paper.
  • Supports elliptical cross-sections, adding more realism as human hairs are usually elliptical. The orientation of the cross-section is aligned with the curve normal, which can be adjusted using geometry nodes. Default is minimal twist. During sampling, light rays that hit outside the hair width will continue propogating as if the material is transparent.
  • Energy loss is expected especially with high roughness values and very light hair colors. There is non-physical modulation factors (Reflection, Transmission, Secondary Reflection) which might help to mitigate this issue.
    Hair close-up

Comparisons:

Using a modified version of the demo file from Daniel Bystedt, attached as hair_nodes-female_hair_styles.blend. Rendered with Mac M1 Ultra Metal, 256spp, original resolution 800x1200.

Model Principled Hair BSDF Microfacet Hair BSDF
Braids Hair Braids Hair Principled Braids Hair Microfacet
Render Time 12.23s 13.91s
Long Hair Long Hair Principled Long Hair Microfacet
Render Time 14.66s 13.63s
Curly Hair Curly Hair Principled Curly Hair Microfacet
Render Time 26.16s 28.12s
Curly Hair (denoised) Curly Hair Principled Curly Hair Microfacet
Render Time 26.7s 27.43s
Cyber Punk* Cyber Punk Principled Cyber Punk Microfacet
Render Time 16.15s 13.26s

* Cyber punk hair is rendered with non-physical scaling of Transmission = 2 for Microfacet Hair model

Missing:

  • A good default for cross-section orientation. There was an attempt (9039f76928) to default the orientation to align with the curve normal in the mathematical sense, but the stability (when animated) is unclear and it would be a hassle to generalise to all curve types. After the model is in main, we could experiment with the geometry nodes team to see what works the best as a default.
  • A better approximation of the sampling pdf than just 1.0.
  • A test file for regression test.
  • EEVEE support. The computation is too heavy for EEVEE, but maybe some approximation is possible.
Implements the paper [A Microfacet-based Hair Scattering Model](https://onlinelibrary.wiley.com/doi/full/10.1111/cgf.14588) by Weizhen Huang, Matthias B. Hullin and Johannes Hanika. ### Features: - This is a far-field model, as opposed to the near-field Principled Hair BSDF model. The hair is expected to be less noisy, but lower roughness values takes longer to render due to numerical integration along the hair width. The hair also appears to be flat when viewed up-close. - The longitudinal width of the scattering lobe differs along the azimuth, providing a higher contrast compared to the evenly spread scattering in the near-field Principled Hair BSDF model. For a more detailed comparison, please refer to the original paper. - Supports elliptical cross-sections, adding more realism as human hairs are usually elliptical. The orientation of the cross-section is aligned with the curve normal, which can be adjusted using geometry nodes. Default is minimal twist. During sampling, light rays that hit outside the hair width will continue propogating as if the material is transparent. - ~~Energy loss is expected especially with high roughness values and very light hair colors~~. There is non-physical modulation factors (Reflection, Transmission, Secondary Reflection) which might help to mitigate this issue. ![Hair close-up](attachments/996272c7-b33a-4f49-84ce-26679827c729) ### Comparisons: Using a modified version of the [demo file](https://www.blender.org/download/demo-files/#hair) from Daniel Bystedt, attached as `hair_nodes-female_hair_styles.blend`. Rendered with Mac M1 Ultra Metal, 256spp, original resolution 800x1200. | Model | Principled Hair BSDF | Microfacet Hair BSDF | |--|--|--| | Braids Hair | ![Braids Hair Principled](attachments/d06d3321-4bb9-4613-a260-2148b23afb8b) | ![Braids Hair Microfacet](attachments/a764604a-9403-448f-8850-38a824145686) | | Render Time |12.23s|13.91s| |Long Hair|![Long Hair Principled](attachments/7d4b23fe-2bfc-4a58-bedf-c7ed367d6556)|![Long Hair Microfacet](attachments/e7371fa2-73f0-4131-a0c6-4ee1ce59d7f1)| | Render Time |14.66s|13.63s| |Curly Hair|![Curly Hair Principled](attachments/5e485e32-f03f-4838-9983-f83816cdb9e6)|![Curly Hair Microfacet](attachments/6dd086df-9cde-423b-a526-999f92fe71da)| | Render Time |26.16s|28.12s| |Curly Hair (denoised)|![Curly Hair Principled](attachments/b97f0c41-e201-4a67-bccc-1b1b25b51ea9)|![Curly Hair Microfacet](attachments/d8e9c343-e725-4dda-82be-7276632bfc4e)| | Render Time |26.7s|27.43s| |Cyber Punk* |![Cyber Punk Principled](attachments/bdf1994d-d10c-415b-b70e-fbd3bf28ae81)|![Cyber Punk Microfacet](attachments/64239489-51aa-48a1-be72-115414fa1a10)| | Render Time |16.15s|13.26s| \* *Cyber punk hair is rendered with non-physical scaling of Transmission = 2 for Microfacet Hair model* ### Missing: - A good default for cross-section orientation. There was an attempt (9039f76928) to default the orientation to align with the curve normal in the mathematical sense, but the stability (when animated) is unclear and it would be a hassle to generalise to all curve types. After the model is in main, we could experiment with the geometry nodes team to see what works the best as a default. - A better approximation of the sampling `pdf` than just `1.0`. - A test file for regression test. - EEVEE support. The computation is too heavy for EEVEE, but maybe some approximation is possible.
Weizhen Huang added the
Module
Render & Cycles
label 2023-03-09 15:02:18 +01:00
Weizhen Huang added 58 commits 2023-03-09 15:02:33 +01:00
68573757bc Cycles: initial commit of microfacet hair bsdf
This is an implementation of the paper [A Microfacet-based Hair Scattering Model](https://onlinelibrary.wiley.com/doi/full/10.1111/cgf.14588) by Weizhen Huang, Matthias Hullin, and Johannes Hanika.
Original implementation in [Mitsuba 2](https://github.com/RiverIntheSky/roughhair) by Weizhen Huang.
Adapted to Cycles by Olivier Maury D16682 and Christophe Hery.
93e0559f54 WIP: calculate normals aligned with the curvature vector
Only works for Catmull-Rom and poly curve types for now
5015c8219b Compute hair normal with minimal total potential energy
Although computed in the geometry node, this normal is specific for
hairs is only implemented for Catmull-Rom splines. Questions remain
whether this should be generalized, also a few design ideas are obscure.
{F14115834}
f88b91fe61 Split `model_type` into `cross_section_type` and `distribution_type`
Also disable analytical GGX R because it seems confusing
Weizhen Huang added this to the Render & Cycles project 2023-03-09 15:03:56 +01:00
Weizhen Huang added 1 commit 2023-03-09 15:45:45 +01:00
Weizhen Huang added 1 commit 2023-03-09 16:18:33 +01:00
9039f76928 Revert changes regarding curve normal computation
This is irrelevant to the shader itself, should be put in a separate patch.
Weizhen Huang added 4 commits 2023-04-11 18:43:00 +02:00
Weizhen Huang added 1 commit 2023-04-12 15:00:21 +02:00
Weizhen Huang added 4 commits 2023-04-12 18:50:11 +02:00
Weizhen Huang added 3 commits 2023-04-13 14:28:27 +02:00
Weizhen Huang added 3 commits 2023-04-13 17:39:49 +02:00
Weizhen Huang added 1 commit 2023-04-14 19:46:57 +02:00
Weizhen Huang added 4 commits 2023-04-17 11:13:51 +02:00
Weizhen Huang added 1 commit 2023-04-17 19:33:18 +02:00
buildbot/vexp-code-patch-coordinator Build done. Details
1b08175c30
Add back `SD_BSDF_HAS_TRANSMISSION` flag
Weizhen Huang changed title from WIP: Cycles: new Microfacet-base Hair BSDF with elliptical cross-section support to WIP: Cycles: new Microfacet-based Hair BSDF with elliptical cross-section support 2023-04-18 13:00:29 +02:00
Weizhen Huang reviewed 2023-04-18 16:05:05 +02:00
@ -319,2 +323,4 @@
attr_intercept->add(time);
if (attr_normal) {
/* TODO: compute geometry normals. */
Author
Member

The curve normal (which is needed for orienting the cross-section) is not considered in the legacy hair curve. I don't want to put too much effort into this if the legacy curve is to be discarded at some point.

The curve normal (which is needed for orienting the cross-section) is not considered in the legacy hair curve. I don't want to put too much effort into this if the legacy curve is to be discarded at some point.
@ -469,3 +468,1 @@
ccl_device_inline float bsdf_principled_hair_albedo_roughness_scale(
const float azimuthal_roughness)
/* Hair Albedo. Also used by `bsdf_hair_microfacet.h` */
Author
Member

These are utitily functions for matching direct color with absorption coefficient. They are designed for Principled Hair BSDF, and don't seem to match very well with the Microfacet Hair BSDF. Currently I just "borrow" these functions so direct coloring is also available for the new model. I could move the three shared functions to bsdf_util.h, or duplicate them in bsdf_hair_microfacet.h, if this matters.

These are utitily functions for matching direct color with absorption coefficient. They are designed for Principled Hair BSDF, and don't seem to match very well with the Microfacet Hair BSDF. Currently I just "borrow" these functions so direct coloring is also available for the new model. I could move the three shared functions to `bsdf_util.h`, or duplicate them in `bsdf_hair_microfacet.h`, if this matters.
weizhen marked this conversation as resolved
@ -1142,1 +1141,3 @@
num_closures += 4;
else if (closure_type == CLOSURE_BSDF_HAIR_PRINCIPLED_ID ||
closure_type == CLOSURE_BSDF_HAIR_MICROFACET_ID) {
num_closures += 2;
Author
Member

Not sure why this was 4 before, there are only 2 closures, ...HairBSDF and ...HairExtra.

Not sure why this was 4 before, there are only 2 closures, `...HairBSDF` and `...HairExtra`.
weizhen marked this conversation as resolved
Weizhen Huang changed title from WIP: Cycles: new Microfacet-based Hair BSDF with elliptical cross-section support to Cycles: new Microfacet-based Hair BSDF with elliptical cross-section support 2023-04-18 16:05:32 +02:00
Weizhen Huang added 1 commit 2023-04-18 16:16:29 +02:00
buildbot/vexp-code-patch-coordinator Build done. Details
cab3b5eeed
Remove unused variable
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR105600) when ready.
Weizhen Huang requested review from Sergey Sharybin 2023-04-18 16:31:36 +02:00
Weizhen Huang requested review from Brecht Van Lommel 2023-04-18 16:31:51 +02:00

For the energy loss, do you think it would help a lot to add albedo scaling as we are planning to do for the regular microfacet BSDF, and would that be potentially straightforward or require more research? Or is it mainly a TRRT+ term like Principled Hair that would help?

From a user point of view, it would be great to have a far field model that both converges faster and avoids the energy loss somehow. I'm wondering what the best way to approach that is, make this model more energy conserving or add far field for Principled Hair.

For the energy loss, do you think it would help a lot to add albedo scaling as we are planning to do for the regular microfacet BSDF, and would that be potentially straightforward or require more research? Or is it mainly a TRRT+ term like Principled Hair that would help? From a user point of view, it would be great to have a far field model that both converges faster and avoids the energy loss somehow. I'm wondering what the best way to approach that is, make this model more energy conserving or add far field for Principled Hair.
Author
Member

I tried to isolate individual lobes. The TRRT+ term in Principled Hair doesn't seem to make a drastic difference:

With TRRT+ Without TRRT+ Only TRRT+
Principled_with_residual.png Principled_no_residual.png TRRT+.png

Here are comparisons of the individual lobes:

Principled Hair Microfacet Hair
R Principled_R.png Microfacet_R.png
TT Principled_TT.png Microfacet_TT.png
TRT Principled_TRT.png Microfacet_TRT.png

So the main source of energy loss seems to be the single-scattering microfacet BSDF, especially for the TT lobe.
An albedo scaling could make sense, but I don't know how we are approaching this in the planned new microfacet BSDF, is there a reference I could look into?

I tried to isolate individual lobes. The TRRT+ term in Principled Hair doesn't seem to make a drastic difference: | With TRRT+ | Without TRRT+ | Only TRRT+ | | -------- | -------- | -------- | | ![Principled_with_residual.png](/attachments/b0d1cf7a-84ba-4444-9d28-1f29f98e4232) | ![Principled_no_residual.png](/attachments/1758fe00-efbf-432a-b88b-cadc8e7dd2a7) | ![TRRT+.png](/attachments/c62c617d-7e1c-45e8-aecd-f8eaacf78a83) | Here are comparisons of the individual lobes: | | Principled Hair | Microfacet Hair | | -------- | -------- | -------- | | R | ![Principled_R.png](/attachments/eaac14d5-1edc-4435-a661-a8edbd3ae4bb) | ![Microfacet_R.png](/attachments/6b675241-36cd-4758-bdce-31d424d95a37) | | TT | ![Principled_TT.png](/attachments/39c566a8-98ec-436c-b277-c535cbaa5a84) | ![Microfacet_TT.png](/attachments/f11daccc-84d9-4f1f-a32f-b8dd10d2f9ae) | | TRT | ![Principled_TRT.png](/attachments/faef953b-05f0-4410-ac65-a4642f3fe119) | ![Microfacet_TRT.png](/attachments/b9f5f09e-316e-47c7-8164-3f64273f58e9) | So the main source of energy loss seems to be the single-scattering microfacet BSDF, especially for the TT lobe. An albedo scaling could make sense, but I don't know how we are approaching this in the planned new microfacet BSDF, is there a reference I could look into?

Thanks for the comparison, very interesting to see it broken down like that.

I'm not sure which exact approach @LukasStockner used, couldn't find a reference quickly in the principled-v2 branch, maybe he can clarify. But I think it's one of these two:
https://blog.selfshadow.com/publications/turquin/ms_comp_final.pdf
http://www.aconty.com/pdf/s2017_pbs_imageworks_slides.pdf

In principled-v2, see cycles_precompute.cpp for the code.

Thanks for the comparison, very interesting to see it broken down like that. I'm not sure which exact approach @LukasStockner used, couldn't find a reference quickly in the `principled-v2` branch, maybe he can clarify. But I think it's one of these two: https://blog.selfshadow.com/publications/turquin/ms_comp_final.pdf http://www.aconty.com/pdf/s2017_pbs_imageworks_slides.pdf In `principled-v2`, see `cycles_precompute.cpp` for the code.
Member

I'm not sure which exact approach @LukasStockner used, couldn't find a reference quickly in the principled-v2 branch, maybe he can clarify. But I think it's one of these two:

Yep, those are the two relevant approaches. For both, you compute the single-scattering albedo depending on angle and roughness. Then, you either add a second lobe based on that data so that the total albedo ends up being one (see Imageworks slides) or you just scale the single-scattering lobe up to normalize it to albedo one (see the Turquin paper).
The nontrivial part is accounting for Fresnel, since higher-order scattering leads to higher saturation, but the formula from the revised Imageworks slides works well.

I'm currently using the "just normalize it" approach for Principled v2 since it's much easier to implement and visually seems to be competitive with the secondary lobe approach (neither is perfect, but which one is better depends on the scene).

> I'm not sure which exact approach @LukasStockner used, couldn't find a reference quickly in the principled-v2 branch, maybe he can clarify. But I think it's one of these two: Yep, those are the two relevant approaches. For both, you compute the single-scattering albedo depending on angle and roughness. Then, you either add a second lobe based on that data so that the total albedo ends up being one (see Imageworks slides) or you just scale the single-scattering lobe up to normalize it to albedo one (see the Turquin paper). The nontrivial part is accounting for Fresnel, since higher-order scattering leads to higher saturation, but the formula from the revised Imageworks slides works well. I'm currently using the "just normalize it" approach for Principled v2 since it's much easier to implement and visually seems to be competitive with the secondary lobe approach (neither is perfect, but which one is better depends on the scene).
Weizhen Huang added 3 commits 2023-05-02 20:50:29 +02:00
Weizhen Huang force-pushed microfacet_hair from 67da18dc22 to 7d25982d7c 2023-05-02 20:54:51 +02:00 Compare
Weizhen Huang force-pushed microfacet_hair from 7d25982d7c to 9fd0c502d6 2023-05-04 20:01:12 +02:00 Compare
Author
Member

The last few commits aim to mitigate the energy loss issue.

Energy loss due to single-scattering microfacets

We refer to the paper Practical multiple scattering compensation for microfacet models, which proposes to simply scale up the reflectance, given that multi-scattering lobes have similar shapes as the single-scattering one. I find this approach very reasonable. Since our hair model already accounts for internal absorption, we also don't need to worry about the fresnel term. A possible optimization would be to use different scaling factors for reflection and transmission, but I'm going to follow whatever is used for the upcoming Multi-GGX for now.
Commit 91562e880e includes all the relevant changes, all changes except for those in bsdf_hair_microfacet.h are partially copied from the principled-v2 branch. For those, it's fine to wait until @LukasStockner push the Multi-GGX-related changes to main.

Energy loss due to omitted terms beyond TRT

According to Fig 16 in the paper A Microfacet-based Hair Scattering Model, The terms beyond TRT are mainly distributed around the specular cone direction, with a roughly diffuse azimuthal component. Therefore, I followed the approach in the paper A practical and controllable hair and fur model for production path tracing (also used in principled hair BSDF) to approximate the TRRT+ terms by summing up a geometric series.

Disclamer: The purpose of commit 9fd0c502d6 is to evaluate how much energy loss we can recover by adding the TRRT+ terms, the adopted approach is not strictly verified. It's probably better to use specular reflection and absorption to approximate the rest of the interactions instead of reusing the value from the previous interaction to save computation time, also the scaling 4 * bsdf->roughness I just took from the principled hair code, not sure if it's reasonable.

Comparison

Furnace test with

  1. zero absorption and maximal roughness, where no energy loss is expected, and
  2. a typical blond hair, where some energy absorption is expected.
No energy compensation With albedo scaling With albedo scaling and TRRT+
melanin=0,roughness=1 furnace_no_energy_compensation.png furnace_albedo_scaling.png furnace_trrt.png
Energy loss ~70% ~17% ~0%
melanin=0.2,roughness=0.3 furnace_no_energy_compensation_melanin_0.2_roughness_0.3.png furnace_albedo_scaling_melanin_0.2_roughness_0.3.png furnace_trrt_melanin_0.2_roughness_0.3.png
Energy "loss" ~53% ~45% ~43%

Therefore, albedo scaling recovers 8% of the energy with typical parameters for blond hair, and TRRT+ terms recovers 2% in addition.
It appears that we can achieve energy conservation with albedo scaling and TRRT+ terms, however, with only slightly positive absorption (melanin=0.0001), the energy loss is already ~12%:
furnace_trrt_melanin_0.0001.png
This is due to total internal reflection, similar as in optical fibers. The residual terms would be trapped inside the fiber, and with non-negative absorption, eventually all energy are absorbed. Such energy loss is "physical" and "expected", therefore, we can NOT expect the microfacet-based hair model to display the same saturation as the principled hair model.

Full blond hair

Principled Microfacet with energy compensation Microfacet without energy compensation
Curly_old_26_16s.png Curly_new_scaled_ggx_29_25s.png Curly_new_ggx.png

We can see that the hair appears slightly brighter with energy compensation, but still quite far from the energy level of principled hair BSDF. There is also a ~10% increase in rendering time.

GGX vs Beckmann

GGX Beckmann
Curly_new_scaled_ggx_29_25s.png Curly_new_scaled_beckmann_31_81s.png

The albedo scaling is computed for GGX, there is energy gain when combined with Beckmann distribution. Without proper measurement it's hard to tell which distribution matches the reality well, maybe we should only use GGX for simplicity.

The last few commits aim to mitigate the energy loss issue. ### Energy loss due to single-scattering microfacets We refer to the paper [Practical multiple scattering compensation for microfacet models](https://blog.selfshadow.com/publications/turquin/ms_comp_final.pdf), which proposes to simply scale up the reflectance, given that multi-scattering lobes have similar shapes as the single-scattering one. I find this approach very reasonable. Since our hair model already accounts for internal absorption, we also don't need to worry about the fresnel term. A possible optimization would be to use different scaling factors for reflection and transmission, but I'm going to follow whatever is used for the upcoming Multi-GGX for now. Commit 91562e880e includes all the relevant changes, all changes except for those in `bsdf_hair_microfacet.h` are partially copied from the principled-v2 branch. For those, it's fine to wait until @LukasStockner push the Multi-GGX-related changes to main. ### Energy loss due to omitted terms beyond TRT According to Fig 16 in the paper [A Microfacet-based Hair Scattering Model](https://onlinelibrary.wiley.com/doi/full/10.1111/cgf.14588), The terms beyond TRT are mainly distributed around the specular cone direction, with a roughly diffuse azimuthal component. Therefore, I followed the approach in the paper [A practical and controllable hair and fur model for production path tracing](http://benedikt-bitterli.me/pchfm/pchfm.pdf) (also used in principled hair BSDF) to approximate the TRRT+ terms by summing up a geometric series. **Disclamer**: The purpose of commit 9fd0c502d6 is to evaluate how much energy loss we can recover by adding the TRRT+ terms, the adopted approach is not strictly verified. It's probably better to use specular reflection and absorption to approximate the rest of the interactions instead of reusing the value from the previous interaction to save computation time, also the scaling `4 * bsdf->roughness` I just took from the principled hair code, not sure if it's reasonable. ### Comparison #### Furnace test with 1. zero absorption and maximal roughness, where no energy loss is expected, and 2. a typical blond hair, where some energy absorption is expected. || No energy compensation | With albedo scaling |With albedo scaling and TRRT+| |--| -------- | -------- | --| |`melanin=0`,`roughness=1`|![furnace_no_energy_compensation.png](/attachments/60cd4472-7e6b-4c35-9cb8-d62cea217968)| ![furnace_albedo_scaling.png](/attachments/30d790e8-cd3b-4558-89e6-abd6c6c36077) | ![furnace_trrt.png](/attachments/f2d5c284-6fb1-4bd6-b680-4b6f2be94744)| |Energy loss|~70%|~17%|~0%| |`melanin=0.2`,`roughness=0.3`|![furnace_no_energy_compensation_melanin_0.2_roughness_0.3.png](/attachments/1727defe-ee48-4cee-8aa5-6e3bc9ec5a4e)|![furnace_albedo_scaling_melanin_0.2_roughness_0.3.png](/attachments/5f09e1db-3609-40fd-b214-9a64e000ba3f)|![furnace_trrt_melanin_0.2_roughness_0.3.png](/attachments/2e0d61a4-6a44-4472-a686-465d95dee071)| |Energy "loss"|~53%|~45%|~43%| Therefore, albedo scaling recovers 8% of the energy with typical parameters for blond hair, and TRRT+ terms recovers 2% in addition. It appears that we can achieve energy conservation with albedo scaling and TRRT+ terms, however, with only slightly positive absorption (`melanin=0.0001`), the energy loss is already ~12%: ![furnace_trrt_melanin_0.0001.png](/attachments/86e37cd9-7242-4ccd-94a2-09a03717727e) This is due to total internal reflection, similar as in optical fibers. The residual terms would be trapped inside the fiber, and with non-negative absorption, eventually all energy are absorbed. Such energy loss is "physical" and "expected", therefore, we can **NOT** expect the microfacet-based hair model to display the same saturation as the principled hair model. #### Full blond hair | Principled | Microfacet with energy compensation | Microfacet without energy compensation | | -------- | -------- | -------- | | ![Curly_old_26_16s.png](/attachments/76900255-7e70-45df-a8f4-4309acaec8eb) | ![Curly_new_scaled_ggx_29_25s.png](/attachments/8e007049-f91a-4733-b974-8411916e3a06) |![Curly_new_ggx.png](/attachments/71480b45-e9f5-4634-a8d6-9ecc69eedccc) | We can see that the hair appears slightly brighter with energy compensation, but still quite far from the energy level of principled hair BSDF. There is also a ~10% increase in rendering time. ### GGX vs Beckmann |GGX|Beckmann| |--|--| |![Curly_new_scaled_ggx_29_25s.png](/attachments/8e007049-f91a-4733-b974-8411916e3a06)|![Curly_new_scaled_beckmann_31_81s.png](/attachments/95a03ddf-9d0a-447e-b3c4-c96c65d85eb2)| The albedo scaling is computed for GGX, there is energy gain when combined with Beckmann distribution. Without proper measurement it's hard to tell which distribution matches the reality well, maybe we should only use GGX for simplicity.
Weizhen Huang added 1 commit 2023-06-05 12:34:05 +02:00
Weizhen Huang added 1 commit 2023-06-05 12:38:14 +02:00
Weizhen Huang added 1 commit 2023-06-05 12:57:43 +02:00

@LukasStockner, is this something you could help reviewing?

@LukasStockner, is this something you could help reviewing?

On the user interface level, I'm wondering if we shouldn't unify this with the Principled Hair BSDF, as a single node.

The reason being that we have Principled BSDF, Principled Hair and Principled Volume that we recommend as the main shaders for users. But then if this is the best hair shaders, it should be the Principled Hair.

There could be an enum on the node that switching between the two modes, hiding sockets that are not relevant to one or the other implementation. As far as the Cycles SVM and OSL code is concerned these could remain two completely separate nodes though, it's only about the UI.

What do you think? Does that make sense, or does it seem like a poor fit?

On the user interface level, I'm wondering if we shouldn't unify this with the Principled Hair BSDF, as a single node. The reason being that we have Principled BSDF, Principled Hair and Principled Volume that we recommend as the main shaders for users. But then if this is the best hair shaders, it should be the Principled Hair. There could be an enum on the node that switching between the two modes, hiding sockets that are not relevant to one or the other implementation. As far as the Cycles SVM and OSL code is concerned these could remain two completely separate nodes though, it's only about the UI. What do you think? Does that make sense, or does it seem like a poor fit?
Weizhen Huang added 1 commit 2023-08-02 19:44:49 +02:00
Weizhen Huang added 1 commit 2023-08-04 12:57:47 +02:00
Weizhen Huang added 5 commits 2023-08-04 16:51:40 +02:00
Weizhen Huang added 2 commits 2023-08-04 18:24:33 +02:00
Lukas Stockner reviewed 2023-08-07 01:22:12 +02:00
Lukas Stockner left a comment
Member

Overall looks great, just a few notes from a first pass.

Overall looks great, just a few notes from a first pass.
@ -0,0 +16,4 @@
typedef struct MicrofacetHairExtra {
/* Optional modulation factors. */
float R, TT, TRT;
Member

Do you think it could make sense to support colors here? It would increase the space usage, so we should only do it if there's a realistic use case I guess.

Do you think it could make sense to support colors here? It would increase the space usage, so we should only do it if there's a realistic use case I guess.
Author
Member

Currently if the parametrization is set to Melanin Concentration, an extra Tint socket is enabled, which corresponds to the realistic case of dyed hair (there is melanin present in natural hair, extra pigments from the dye is added). The R, TT, TRT are non-physical modulation factors, I thinking adding colors there would make the hair appearance quite difficult to control, using Tint should be a more proper way.

Currently if the parametrization is set to Melanin Concentration, an extra Tint socket is enabled, which corresponds to the realistic case of dyed hair (there is melanin present in natural hair, extra pigments from the dye is added). The R, TT, TRT are non-physical modulation factors, I thinking adding colors there would make the hair appearance quite difficult to control, using Tint should be a more proper way.
weizhen marked this conversation as resolved
@ -0,0 +66,4 @@
* \{ */
/* Returns `sin(theta)` of the given direction. */
ccl_device_inline float sin_theta(const float3 w)
Member

This is probably fine, but one consideration here is that all functions in the kernel share a namespace, so e.g. sin_theta might easily conflict with something else in the future.
Then again, this is probably general enough that we could just move it to util/ if that happens.

This is probably fine, but one consideration here is that all functions in the kernel share a namespace, so e.g. `sin_theta` might easily conflict with something else in the future. Then again, this is probably general enough that we could just move it to `util/` if that happens.
@ -0,0 +479,4 @@
const float weight = (i == 0 || i == intervals) ? 0.5f : (i % 2 + 1);
const Spectrum A_t = exp(mu_a / cos_theta(wt) *
Member

This appears 4 times I think, can we split it into a helper function?

This appears 4 times I think, can we split it into a helper function?
@ -0,0 +608,4 @@
*eta = bsdf->eta;
/* Treat as transparent material if intersection lies outside of the projected radius. */
if (fabsf(bsdf->h) > bsdf->extra->radius) {
Member

Since both of these are already known in the setup function at shader evaluation time, we might want to implement the "default to transparent" logic there?

Since both of these are already known in the setup function at shader evaluation time, we might want to implement the "default to transparent" logic there?
weizhen marked this conversation as resolved
@ -0,0 +616,4 @@
return LABEL_TRANSMIT | LABEL_TRANSPARENT;
}
if (bsdf->extra->R <= 0.0f && bsdf->extra->TT <= 0.0f && bsdf->extra->TRT <= 0.0f) {
Member

Same here.

Same here.
weizhen marked this conversation as resolved
@ -0,0 +643,4 @@
const float h = sample_h * 2.0f - 1.0f;
const float gamma_mi = is_circular ?
asinf(h) :
Member

Nitpicking, but we should probably try to not make this span five lines (maybe an if (is_circular)?)

Nitpicking, but we should probably try to not make this span five lines (maybe an `if (is_circular)`?)
Author
Member

Indeed it doesn't look good, but adding if (is_circular) makes it span 8 lines, not sure of the benefits.

Indeed it doesn't look good, but adding `if (is_circular)` makes it span 8 lines, not sure of the benefits.
@ -0,0 +771,4 @@
fast_sincosf(phi_o, &sin_phi_o, &cos_phi_o);
/* Compute outgoing direction. */
wtrrt = make_float3(sin_phi_o * cos_theta_o, sin_theta_o, cos_phi_o * cos_theta_o);
Member

I think this is only used if we actually sample TRRT? The compiler should probably optimize that by itself though I guess.

I think this is only used if we actually sample TRRT? The compiler should probably optimize that by itself though I guess.
@ -0,0 +853,4 @@
const float3 local_I = bsdf->extra->wi;
const float3 local_O = make_float3(dot(wo, X), dot(wo, Y), dot(wo, Z));
/* TODO: better estimation of the pdf */
Member

Does this TODO refer to the PR, or it more general?
I think it's fine to keep it like this for now, just want to make sure it's not overlooked.

Does this TODO refer to the PR, or it more general? I think it's fine to keep it like this for now, just want to make sure it's not overlooked.
Author
Member

In the original implementation the pdf involves sampling microfacets, it is as costly as evaluating the BSDF itself, and it is also just an approximation, so not sure if it's worth it and how to do it better. As pdf is only used for MIS, it doesn't influence the correctness of the model, so left as a TODO as a future improvement.

In the original implementation the pdf involves sampling microfacets, it is as costly as evaluating the BSDF itself, and it is also just an approximation, so not sure if it's worth it and how to do it better. As pdf is only used for MIS, it doesn't influence the correctness of the model, so left as a TODO as a future improvement.
weizhen marked this conversation as resolved
@ -58,2 +58,3 @@
/* All closures contribute to the normal feature, but only diffuse-like ones to the albedo. */
normal += sc->N * sc->sample_weight;
/* If far-field hair, use fiber tangent as feature instead of normal. */
normal += (sc->type == CLOSURE_BSDF_HAIR_MICROFACET_ID ? safe_normalize(sd->dPdu) : sc->N) *
Member

Is this conditional just due to compatibility?
If yes, and the tangent works better, I think we should just change it for the old model as well.

Is this conditional just due to compatibility? If yes, and the tangent works better, I think we should just change it for the old model as well.
Author
Member

This piece of code is provided by @olivier.fx. I didn't came up with the logic myself, but for a far-field model the "normal" is always the incoming direction, not really a feature of the object itself, so tangent makes more sense here. For the previous near-field model the normal does influence the appearance and it makes sense to use normal as feature. Not sure if tangent works better, I'm not very knowledgeable about denoising.

This piece of code is provided by @olivier.fx. I didn't came up with the logic myself, but for a far-field model the "normal" is always the incoming direction, not really a feature of the object itself, so tangent makes more sense here. For the previous near-field model the normal does influence the appearance and it makes sense to use normal as feature. Not sure if tangent works better, I'm not very knowledgeable about denoising.
@ -90,3 +95,3 @@
}
BSDF = principled_hair(Normal, sigma, roughness, radial_roughness, m0_roughness, Offset, IOR);
normal major_axis = Normal;
Member

I think we can move this into the main if below?

I think we can move this into the main `if` below?
weizhen marked this conversation as resolved
@ -3434,2 +3445,4 @@
void PrincipledHairBsdfNode::attributes(Shader *shader, AttributeRequestSet *attributes)
{
/* Make sure we have the normal for elliptical cross section tracking. */
if (model == NODE_PRINCIPLED_HAIR_HUANG && aspect_ratio != 1.0f) {
Member

I think you also need to check whether the socket is connected here?

I think you also need to check whether the socket is connected here?
weizhen marked this conversation as resolved
@ -1282,0 +1282,4 @@
typedef struct NodeShaderHairPrincipled {
short model;
short parametrization;
short cross_section;
Member

Doesn't appear to be used?

Doesn't appear to be used?
weizhen marked this conversation as resolved
@ -1282,0 +1284,4 @@
short parametrization;
short cross_section;
char _pad[2];
} NodeShaderHairPrincipled;
Member

Not sure if we need this yet, custom1 and custom2 should be enough?

Not sure if we need this yet, `custom1` and `custom2` should be enough?
Author
Member

For the current two enums yes, but named struct seems easier understandable. On the other hand, it introduces additional storage and is seen as a compromise for not having enum-typed sockets. I'm actually not sure what the current recommendation is.

For the current two enums yes, but named struct seems easier understandable. On the other hand, it introduces additional storage and is seen as a compromise for not having enum-typed sockets. I'm actually not sure what the current recommendation is.

I think it's fine to have better names for storage, custom1 / custom2 are not that great.

I think it's fine to have better names for storage, custom1 / custom2 are not that great.
weizhen marked this conversation as resolved
@ -4546,0 +4554,4 @@
0,
"Far-field Model",
"Microfacet-based hair scattering model by Huang et. al 2022, suitable for viewing from a "
"distance, supports elliptical cross-sections and has preciser highlight in forward "
Member

"preciser"->"more precise"

"preciser"->"more precise"
weizhen marked this conversation as resolved
@ -27,0 +47,4 @@
.description(
"For elliptical hair cross-section, the aspect ratio is the ratio of the minor axis to "
"the major axis (the major axis is aligned with the curve normal). Recommended values "
"are 0.8~1 for Asian hair, 0.65~0.9 for Caucasian hair, 0.5~0.65 for African hair. Set "
Member

I'm not sure if the tooltips are the best place to put these values, might be better to keep it short and have them in the manual?

I'm not sure if the tooltips are the best place to put these values, might be better to keep it short and have them in the manual?
Author
Member

I don't see tooltips in any other shader nodes, so I'm not sure if there is a guideline here, and I also have no idea about UX. I thought it would be good to provide clear information here, so the users don't need to go to the manual to find it. Currently this spans three lines, if conciseness is desired I can shorten this of course.

I don't see tooltips in any other shader nodes, so I'm not sure if there is a guideline here, and I also have no idea about UX. I thought it would be good to provide clear information here, so the users don't need to go to the manual to find it. Currently this spans three lines, if conciseness is desired I can shorten this of course.

I think it's fine to have longer tooltips in general, so this is fine with me.

I think it's fine to have longer tooltips in general, so this is fine with me.
weizhen marked this conversation as resolved
Lukas Stockner added 1 commit 2023-08-08 02:12:12 +02:00
Lukas Stockner added 1 commit 2023-08-10 03:39:19 +02:00
Weizhen Huang added 2 commits 2023-08-14 15:14:13 +02:00
Weizhen Huang added 2 commits 2023-08-14 17:52:38 +02:00
1a6b4c6d0c Fix assertion error
`total_energy` could be zero even when the modulation factors are non-zero
Author
Member

@LukasStockner There is a problem with your change. Because num_closure is set to 2 in shader_graph.cpp, when it tries to allocate a transparent closure in bsdf_microfacet_hair_setup() it would fail and render black pixels. I basically replaced the hair closure with a transparent closure in 7d14b8da7e, but I'm not sure if this is the proper way to "de-allocate" a closure.

@LukasStockner There is a problem with your change. Because `num_closure` is set to 2 in `shader_graph.cpp`, when it tries to allocate a transparent closure in `bsdf_microfacet_hair_setup()` it would fail and render black pixels. I basically replaced the hair closure with a transparent closure in 7d14b8da7e75cec4804886d0f995a0fbfe0e517b, but I'm not sure if this is the proper way to "de-allocate" a closure.
Weizhen Huang added 4 commits 2023-08-15 16:50:28 +02:00
Member

@LukasStockner There is a problem with your change. Because num_closure is set to 2 in shader_graph.cpp, when it tries to allocate a transparent closure in bsdf_microfacet_hair_setup() it would fail and render black pixels. I basically replaced the hair closure with a transparent closure in 7d14b8da7e, but I'm not sure if this is the proper way to "de-allocate" a closure.

Ah, true, that's a problem. I had figured that the extra leftover closure doesn't hurt, but I forgot about the closure limit.

The linked commit seems like the correct way to do this. It only works if there hasn't been another allocation in between, but here that's the case.

One thing to note is that sd->num_closure_left += 2 is only correct as long as the Extra storage is not larger than a ShaderClosure. We might want to either add a static assert for that, or add general dealloc_extra and dealloc_closure to alloc.h, where dealloc_extra could take a size argument.

> @LukasStockner There is a problem with your change. Because num_closure is set to 2 in shader_graph.cpp, when it tries to allocate a transparent closure in bsdf_microfacet_hair_setup() it would fail and render black pixels. I basically replaced the hair closure with a transparent closure in 7d14b8da7e, but I'm not sure if this is the proper way to "de-allocate" a closure. Ah, true, that's a problem. I had figured that the extra leftover closure doesn't hurt, but I forgot about the closure limit. The linked commit seems like the correct way to do this. It only works if there hasn't been another allocation in between, but here that's the case. One thing to note is that `sd->num_closure_left += 2` is only correct as long as the Extra storage is not larger than a `ShaderClosure`. We might want to either add a static assert for that, or add general `dealloc_extra` and `dealloc_closure` to `alloc.h`, where `dealloc_extra` could take a size argument.
First-time contributor

maybe something using realtime raytracing can mimic this for EEVEE-Next?

maybe something using realtime raytracing can mimic this for EEVEE-Next?
Brecht Van Lommel approved these changes 2023-08-16 17:47:25 +02:00
Weizhen Huang added 3 commits 2023-08-18 12:37:44 +02:00
Weizhen Huang merged commit 6f8011edf7 into main 2023-08-18 12:46:20 +02:00
Sign in to join this conversation.
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No Assignees
5 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#105600
No description provided.