Cycles: Clean up the Principled Hair BSDF implementation #104669

Lukas Stockner · 2023-02-12T22:11:00+01:00

Lukas Stockner commented

2023-02-12 22:11:00 +01:00

Rename roughness variables for more clarity - before, the SVM/OSL code would set s and v to the linear roughness values, and the setup function would overwrite them with the distribution parameters. This actually caused a bug in the albedo code, since it intended to use the linear roughness value, but ended up getting the remapped value.
Deduplicate the evaluation and sample functions. Most of their code is the same, only the middle part is different.
Changed albedo computation to return the sum of the intensities of the four BSDF lobes. Previously, the code applied the inverse of the color->sigma mapping from the paper - this returns the color specified in the node, but for very dark hair (e.g. when using the Melanin controls) the result is extremely low (e.g. 0.000001) despite the hair still reflecting a significant amount of light (since the R lobe is independent of sigma). This causes issues with the light component passes, so this change fixes #104586.
There's quite a few computations at the start of the evaluation function that are needed for sampling, evaluation and albedo computation, but only depend on the view direction. Therefore, just precompute them - we still have space in PrincipledHairExtra after all.
Fix a tiny bug - the direction sampling code did not account for the R lobe roughness modifier.

- Rename roughness variables for more clarity - before, the SVM/OSL code would set s and v to the linear roughness values, and the setup function would overwrite them with the distribution parameters. This actually caused a bug in the albedo code, since it intended to use the linear roughness value, but ended up getting the remapped value. - Deduplicate the evaluation and sample functions. Most of their code is the same, only the middle part is different. - Changed albedo computation to return the sum of the intensities of the four BSDF lobes. Previously, the code applied the inverse of the color->sigma mapping from the paper - this returns the color specified in the node, but for very dark hair (e.g. when using the Melanin controls) the result is extremely low (e.g. 0.000001) despite the hair still reflecting a significant amount of light (since the R lobe is independent of sigma). This causes issues with the light component passes, so this change fixes #104586. - There's quite a few computations at the start of the evaluation function that are needed for sampling, evaluation and albedo computation, but only depend on the view direction. Therefore, just precompute them - we still have space in PrincipledHairExtra after all. - Fix a tiny bug - the direction sampling code did not account for the R lobe roughness modifier.

👍 1

Lukas Stockner added the

Module

Render & Cycles

label 2023-02-12 22:11:00 +01:00

Lukas Stockner added 1 commit 2023-02-12 22:11:02 +01:00

f806033977 Cycles: Clean up the Principled Hair BSDF implementation

- Rename roughness variables for more clarity - before, the SVM/OSL code would
  set s and v to the linear roughness values, and the setup function would over-
  write them with the distribution parameters. This actually caused a bug in the
  albedo code, since it intended to use the linear roughness value, but ended up
  getting the remapped value.
- Deduplicate the evaluation and sample functions. Most of their code is the
  same, only the middle part is different.
- Changed albedo computation to return the sum of the intensities of the four
  BSDF lobes. Previously, the code applied the inverse of the color->sigma
  mapping from the paper - this returns the color specified in the node, but
  for very dark hair (e.g. when using the Melanin controls) the result is
  extremely low (e.g. 0.000001) despite the hair still reflecting a significant
  amount of light (since the R lobe is independent of sigma). This causes issues
  with the light component passes, so this change fixes #104586.
- There's quite a few computations at the start of the evaluation function that
  are needed for sampling, evaluation and albedo computation, but only depend on
  the view direction. Therefore, just precompute them - we still have space in
  PrincipledHairExtra after all.
- Fix a tiny bug - the direction sampling code did not account for the R lobe
  roughness modifier.

Lukas Stockner requested review from Weizhen Huang 2023-02-12 22:11:08 +01:00

Brecht Van Lommel approved these changes 2023-02-13 08:24:15 +01:00

Brecht Van Lommel added this to the Render & Cycles project 2023-02-13 08:24:46 +01:00

Lukas Stockner merged commit cb77865c21 into main

2023-02-13 22:49:12 +01:00

Lukas Stockner deleted branch principled-hair-cleanup

2023-02-13 22:49:12 +01:00

Lukas Stockner referenced this issue from a commit

2023-02-13 22:49:14 +01:00

Cycles: Clean up the Principled Hair BSDF implementation

Brecht Van Lommel commented

2023-02-13 23:30:30 +01:00

It looks like this broke compilation on HIP.
https://builder.blender.org/admin/#/builders/10/builds/8787/steps/7/logs/stdio

Can you revert this commit, and then we check how to re-commit this for 3.6 in a few days.

I might disable HIP entirely in 3.6 until a solution is found, because we can not be working around compiler bugs continuously while refactoring code for principled v2.

It looks like this broke compilation on HIP. https://builder.blender.org/admin/#/builders/10/builds/8787/steps/7/logs/stdio Can you revert this commit, and then we check how to re-commit this for 3.6 in a few days. I might disable HIP entirely in 3.6 until a solution is found, because we can not be working around compiler bugs continuously while refactoring code for principled v2.

Lukas Stockner commented

2023-02-13 23:35:37 +01:00

Done: bb9eb262d4

I'll put together a quick workaround for #104586 that we can commit in the meantime.

Done: https://projects.blender.org/blender/blender/commit/bb9eb262d4d35065f477681a8ea9a6261486ea91 I'll put together a quick workaround for #104586 that we can commit in the meantime.

Brecht Van Lommel commented

2023-02-13 23:37:19 +01:00

Thanks! If you make a PR with a simpler workaround you can test that it compiles on the GPU with the @blender-bot package command.

Thanks! If you make a PR with a simpler workaround you can test that it compiles on the GPU with the `@blender-bot package` command.

Brecht Van Lommel referenced this pull request

2023-02-15 17:14:27 +01:00

Cycles HIP binaries disabled due to compiler bugs #104786

Brecht Van Lommel commented

2023-02-15 17:34:44 +01:00

I've disabled HIP binaries in 3.6, you can commit this cleanup to main now.

Brecht Van Lommel removed this from the Render & Cycles project 2023-02-17 14:17:32 +01:00

Sign in to join this conversation.