Cycles: Rework Principled BSDF Clearcoat #110993

Merged
Lukas Stockner merged 21 commits from LukasStockner/blender:clearcoat-principled into main 2023-09-13 00:03:19 +02:00
Member
  • Adds tint control, which simulates volumetric absorption inside the coating layer. This results in angle-dependent saturation and affects all underlying layers (diffuse, subsurface, metallic, transmission). It provides a physically-based alternative to ad-hoc effects such as tinted specular highlights or transmission interfaces.
  • Renames the component from "Clearcoat" to "Coat", since it's no longer necessarily clear now. This matches naming in e.g. other renderers or OpenPBR.
  • Removes hardcoded 0.25 weight multiplier, and adds versioning code to update existing files accordingly.
  • Replaces the GTR1 microfacet component with regular GGX. This removes a corner case in the Microfacet code, solves #53038, and makes us more consistent with other standard surface shaders. The original Disney BSDF used GTR1, but it doesn't appear that it caught on in the industry.
- Adds tint control, which simulates volumetric absorption inside the coating layer. This results in angle-dependent saturation and affects all underlying layers (diffuse, subsurface, metallic, transmission). It provides a physically-based alternative to ad-hoc effects such as tinted specular highlights or transmission interfaces. - Renames the component from "Clearcoat" to "Coat", since it's no longer necessarily clear now. This matches naming in e.g. other renderers or OpenPBR. - Removes hardcoded 0.25 weight multiplier, and adds versioning code to update existing files accordingly. - Replaces the GTR1 microfacet component with regular GGX. This removes a corner case in the Microfacet code, solves #53038, and makes us more consistent with other standard surface shaders. The original Disney BSDF used GTR1, but it doesn't appear that it caught on in the industry.
Lukas Stockner added the
Interest
Render & Cycles
label 2023-08-10 10:23:31 +02:00
Lukas Stockner added this to the Render & Cycles project 2023-08-10 10:23:49 +02:00
Lukas Stockner requested review from Brecht Van Lommel 2023-08-10 10:23:59 +02:00
Member

This is probably unintentional so that's why I'm reporting it.

Changing the Coat Tint colour while the coat parameter is set to 0 still has an impact on the material.
Clear coat colour.png

This is probably unintentional so that's why I'm reporting it. Changing the `Coat Tint` colour while the coat parameter is set to `0` still has an impact on the material. ![Clear coat colour.png](/attachments/32515dcf-83da-4435-88a4-4c5807054618)
Alaska reviewed 2023-08-10 15:02:57 +02:00
@ -209,3 +203,2 @@
ccl_private PrincipledDiffuseBsdf *bsdf = (ccl_private PrincipledDiffuseBsdf *)bsdf_alloc(
sd, sizeof(PrincipledDiffuseBsdf), diff_weight);
if (coat_tint != one_float3()) {
Member

Replacing if (coat_tint != one_float3()) with if (!isequal(coat_tint, one_float3())) fixes compilation issues with the Metal backend.

For reference, the compilation issue is this:

error: value of type 'bool __attribute__((ext_vector_type(3)))' (vector of 3 'bool' values) is not contextually convertible to 'bool'
      if (coat_tint != one_float3()) {
          ^~~~~~~~~~~~~~~~~~~~~~~~~
Replacing `if (coat_tint != one_float3())` with `if (!isequal(coat_tint, one_float3()))` fixes compilation issues with the Metal backend. For reference, the compilation issue is this: ``` error: value of type 'bool __attribute__((ext_vector_type(3)))' (vector of 3 'bool' values) is not contextually convertible to 'bool' if (coat_tint != one_float3()) { ^~~~~~~~~~~~~~~~~~~~~~~~~ ```
Alaska marked this conversation as resolved

The changes and implementation look fine besides the issues mentioned by @Alaska.

When this gets committed, add-ons will need to be updated too. I think it's just atomic and glTF that are affected. So I'll just ping @Blendphys @JulienDuroure here so they know this is coming.

The changes and implementation look fine besides the issues mentioned by @Alaska. When this gets committed, add-ons will need to be updated too. I think it's just atomic and glTF that are affected. So I'll just ping @Blendphys @JulienDuroure here so they know this is coming.
Author
Member

This is probably unintentional so that's why I'm reporting it.

Changing the Coat Tint colour while the coat parameter is set to 0 still has an impact on the material.
Clear coat colour.png

This is currently intentional. The intention is that the Coat input controls the strength of the specular reflection on the interface, while the Tint input controls the strength of the absorption in the volume of the layer.

In the future, we might want to introduce a Coat IOR control (as in OpenPBR) to control the specular intensity. In this case, the Coat input would act as a "is a Coat layer present?" input, and in that case it should indeed also control the tint.

I haven't done this yet since, to do it properly, the Coat IOR should affect the Fresnel or the "main" specular reflection.

However, now that I think about it we might want to:

  • Introduce Coat IOR now
  • Default to 1.2 on old files instead of adding the *0.25 term in versioning (IOR 1.2 has a reflectivity that is roughly one quarter of IOR 1.5)
  • Leave the Coat IOR <-> Main IOR interaction as TODO
  • Make the Coat input affect the Tint
> This is probably unintentional so that's why I'm reporting it. > > Changing the `Coat Tint` colour while the coat parameter is set to `0` still has an impact on the material. > ![Clear coat colour.png](/attachments/32515dcf-83da-4435-88a4-4c5807054618) This is currently intentional. The intention is that the Coat input controls the strength of the specular reflection on the interface, while the Tint input controls the strength of the absorption in the volume of the layer. In the future, we might want to introduce a Coat IOR control (as in OpenPBR) to control the specular intensity. In this case, the Coat input would act as a "is a Coat layer present?" input, and in that case it should indeed also control the tint. I haven't done this yet since, to do it properly, the Coat IOR should affect the Fresnel or the "main" specular reflection. However, now that I think about it we might want to: - Introduce Coat IOR now - Default to 1.2 on old files instead of adding the `*0.25` term in versioning (IOR 1.2 has a reflectivity that is roughly one quarter of IOR 1.5) - Leave the Coat IOR <-> Main IOR interaction as TODO - Make the Coat input affect the Tint
Lukas Stockner force-pushed clearcoat-principled from a42da79bc1 to ca6de1282d 2023-08-11 06:46:44 +02:00 Compare

However, now that I think about it we might want to:

  • Introduce Coat IOR now
  • Default to 1.2 on old files instead of adding the *0.25 term in versioning (IOR 1.2 has a reflectivity that is roughly one quarter of IOR 1.5)
  • Leave the Coat IOR <-> Main IOR interaction as TODO
  • Make the Coat input affect the Tint

That sounds good.

> However, now that I think about it we might want to: > - Introduce Coat IOR now > - Default to 1.2 on old files instead of adding the `*0.25` term in versioning (IOR 1.2 has a reflectivity that is roughly one quarter of IOR 1.5) > - Leave the Coat IOR <-> Main IOR interaction as TODO > - Make the Coat input affect the Tint That sounds good.
Lukas Stockner force-pushed clearcoat-principled from ca6de1282d to 2771d57ae2 2023-08-15 02:54:13 +02:00 Compare
Author
Member
  • Rebased
  • Added Coat IOR as discussed above
  • Added descriptions to Coat node sockets
  • Tint now reacts to the Coat input

One remaining detail: When creating a new file, it appears that the versioning code runs, since the initial Material ends up with an IOR of 1.2. Not sure how to avoid this, just bump the default file to a newer version?

Also, note for landing this: Two tests (principled coat and principled bsdf interior) fail and need to be updated. For the first one, we should to update the test file to turn on MIS on the lamp to get more than a few fireflies for the highlight.

- Rebased - Added Coat IOR as discussed above - Added descriptions to Coat node sockets - Tint now reacts to the Coat input One remaining detail: When creating a new file, it appears that the versioning code runs, since the initial `Material` ends up with an IOR of 1.2. Not sure how to avoid this, just bump the default file to a newer version? Also, note for landing this: Two tests (`principled coat` and `principled bsdf interior`) fail and need to be updated. For the first one, we should to update the test file to turn on MIS on the lamp to get more than a few fireflies for the highlight.
Lukas Stockner changed title from WIP: Cycles: Rework Principled BSDF Clearcoat to Cycles: Rework Principled BSDF Clearcoat 2023-08-15 03:06:31 +02:00
Member

Running a quick test, I can identify a few issues.

  1. Blender crashes when using EEVEE with this pull request on the Metal backend for Blender (Tested with a M1 Pro). I haven't done much debugging on the Metal EEVEE side so I might be wrong about this, but error messages suggests it is a similar issue as before. 'coat_tint.rgb != vec3(1.0)' vector of 3 'bool' values is not contextually convertible to 'bool'. Note: Cycles Metal does compile and work. EEVEE with OpenGL (like on Windows or Linux) doesn't crash.

  2. There is a mismatch between Cycles SVM and Cycles OSL when the coat IOR is below 1.0. Cycles SVM behaves like Coat IOR is 1.0. Cycles with OSL kind of behaves like the selected IOR and shows a Total Internal Reflection like effect, but is also a bit broken if I understand my physics correctly.

    Cycles SVM Cycles OSL Material Settings
    Cycles SVM.jpg Cycles OSL.jpg OSL Node settings.jpg
  3. The EEVEE material needs to be updated to match Cycles. At the moment it doesn't match (Tested on Windows with a Nvidia GPU). This is most noticeable when the coat tint has a strong colour, but coat is set low (or to 0). It's like the EEVEE implementation hasn't been updated with the recent changes?

    EEVEE Cycles Material Settings
    EEVEE.jpg Cycles.jpg EEVEE-Cycles Node settings.jpg
Running a quick test, I can identify a few issues. 1. Blender crashes when using EEVEE with this pull request on the Metal backend for Blender (Tested with a M1 Pro). I haven't done much debugging on the Metal EEVEE side so I might be wrong about this, but error messages suggests it is a similar issue as before. `'coat_tint.rgb != vec3(1.0)' vector of 3 'bool' values is not contextually convertible to 'bool'`. Note: Cycles Metal does compile and work. EEVEE with OpenGL (like on Windows or Linux) doesn't crash. 2. There is a mismatch between Cycles SVM and Cycles OSL when the coat IOR is below 1.0. Cycles SVM behaves like Coat IOR is 1.0. Cycles with OSL kind of behaves like the selected IOR and shows a Total Internal Reflection like effect, but is also a bit broken if I understand my physics correctly. | Cycles SVM | Cycles OSL | Material Settings | | - | - | - | | ![Cycles SVM.jpg](/attachments/1a8ece6f-399a-44cd-9017-ecde8042071c) | ![Cycles OSL.jpg](/attachments/96f50198-c6b2-4b16-9736-790d487e1a6a) | ![OSL Node settings.jpg](/attachments/7ecfd0aa-4c53-4598-9f5d-2df8cd758041) | 3. The EEVEE material needs to be updated to match Cycles. At the moment it doesn't match (Tested on Windows with a Nvidia GPU). This is most noticeable when the coat tint has a strong colour, but coat is set low (or to 0). It's like the EEVEE implementation hasn't been updated with the recent changes? | EEVEE | Cycles | Material Settings| | - | - | - | | ![EEVEE.jpg](/attachments/74c04865-331b-497d-b757-8a0cec982662) | ![Cycles.jpg](/attachments/3ada2e0a-a2dd-45da-854e-2182ca018d6d) | ![EEVEE-Cycles Node settings.jpg](/attachments/7ece2e85-2d9a-46c2-adac-c7de067ee046) |
Alaska reviewed 2023-08-15 06:55:12 +02:00
@ -67,6 +69,12 @@ void node_bsdf_principled(vec4 base_color,
vec3 base_color_tint = tint_from_color(base_color.rgb);
if (coat_tint.rgb != vec3(1.0)) {
Member

This results in Blender crashing on the Metal backend for Blender/EEVEE. You can find the error message below.

'coat_tint.rgb != vec3(1.0)' vector of 3 'bool' values is not contextually convertible to 'bool'

This results in Blender crashing on the Metal backend for Blender/EEVEE. You can find the error message below. `'coat_tint.rgb != vec3(1.0)' vector of 3 'bool' values is not contextually convertible to 'bool'`
Author
Member

Thanks, should be fixed now I think?

Thanks, should be fixed now I think?
Alaska marked this conversation as resolved

One remaining detail: When creating a new file, it appears that the versioning code runs, since the initial Material ends up with an IOR of 1.2. Not sure how to avoid this, just bump the default file to a newer version?

Defaults in the startup.blend can be changed in source/blender/blenloader/intern/versioning_defaults.cc. It's generally preferred to do that over updating the startup.blend for every template, since there is some risk in that.

> One remaining detail: When creating a new file, it appears that the versioning code runs, since the initial `Material` ends up with an IOR of 1.2. Not sure how to avoid this, just bump the default file to a newer version? Defaults in the startup.blend can be changed in `source/blender/blenloader/intern/versioning_defaults.cc`. It's generally preferred to do that over updating the `startup.blend` for every template, since there is some risk in that.
Member

I also noticed a weird behavior with certain coat colours and device configurations.

If the coat tint has a blue channel set to 0, and you're rendering with the CPU, then the coat will appear overly dark.
If you are rendering with a GPU or the blue channel is not set to 0, this issue does not occur. I have attached a video demonstrating the issue.

I tested this on and can reproduce this behavior on:

  • Windows with a Ryzen 9 5950X and RTX 4090
  • macOS with a M1 Pro

Steps to reproduce:

  1. Change the render engine to Cycles CPU.
  2. Change the coat tint to a colour that has a blue channel set to 0. E.G. RGB(1.0, 1.0, 0.0)
  3. Enable the coat (set coat to 1.0)
  4. Notice that the coat looks quite dark.
  5. Either switch to a GPU backend or change the coat tint to include some blue in it and everything will look "normal".

I also noticed a weird behavior with certain coat colours and device configurations. If the coat tint has a blue channel set to 0, and you're rendering with the CPU, then the coat will appear overly dark. If you are rendering with a GPU or the blue channel is not set to 0, this issue does not occur. I have attached a video demonstrating the issue. I tested this on and can reproduce this behavior on: - Windows with a Ryzen 9 5950X and RTX 4090 - macOS with a M1 Pro Steps to reproduce: 1. Change the render engine to Cycles CPU. 2. Change the coat tint to a colour that has a blue channel set to 0. E.G. `RGB(1.0, 1.0, 0.0)` 3. Enable the coat (set coat to 1.0) 4. Notice that the coat looks quite dark. 5. Either switch to a GPU backend or change the coat tint to include some blue in it and everything will look "normal". <video src="/attachments/3fcc1bdc-e03f-492e-8da3-1db3c8005ea0" title="Weird coat colour issue.mp4" controls></video>
Alaska reviewed 2023-08-15 13:12:32 +02:00
@ -101,0 +98,4 @@
.subtype(PROP_FACTOR)
.description(
"Controls the intensity of the coat layer, both the reflection and the tinting. "
"Typically should be zero or one for physically-based materials.");
Member

Remove the full stop from the end of "Typically should be zero or one for physically-based materials.". The Blender UI automatically adds a full stop to the end.

Remove the full stop from the end of `"Typically should be zero or one for physically-based materials."`. The Blender UI automatically adds a full stop to the end.
Alaska marked this conversation as resolved
Lukas Stockner added 1 commit 2023-08-15 13:58:18 +02:00
c8a53394c7 Adress review comments
- Fixed Metal EEVEE compilation
- Fixed EEVEE ignoring the Coat parameter
- Fixed IOR<1 in OSL
- Fixed extra . in the socket description
Author
Member

Thanks for the review!

Blender crashes when using EEVEE with this pull request on the Metal backend for Blender (Tested with a M1 Pro). I haven't done much debugging on the Metal EEVEE side so I might be wrong about this, but error messages suggests it is a similar issue as before. 'coat_tint.rgb != vec3(1.0)' vector of 3 'bool' values is not contextually convertible to 'bool'. Note: Cycles Metal does compile and work. EEVEE with OpenGL (like on Windows or Linux) doesn't crash.

This should work now I think, could you please test again?

There is a mismatch between Cycles SVM and Cycles OSL when the coat IOR is below 1.0. Cycles SVM behaves like Coat IOR is 1.0. Cycles with OSL kind of behaves like the selected IOR and shows a Total Internal Reflection like effect, but is also a bit broken if I understand my physics correctly.

Fixed, thanks. IOR is forced to be >1 here since it's a coating. IOR<1 in general is a hack for modeling nested dielectrics currently, I still hope to get proper support for this in the future.

The EEVEE material needs to be updated to match Cycles. At the moment it doesn't match (Tested on Windows with a Nvidia GPU). This is most noticeable when the coat tint has a strong colour, but coat is set low (or to 0). It's like the EEVEE implementation hasn't been updated with the recent changes?

Fixed, thanks.

I also noticed a weird behavior with certain coat colours and device configurations.
If the coat tint has a blue channel set to 0, and you're rendering with the CPU, then the coat will appear overly dark.

Can reproduce, this is weird. I'll try to find the issue.

Defaults in the startup.blend can be changed in source/blender/blenloader/intern/versioning_defaults.cc. It's generally preferred to do that over updating the startup.blend for every template, since there is some risk in that.

Perfect, thanks! I'll have a look.

Thanks for the review! > Blender crashes when using EEVEE with this pull request on the Metal backend for Blender (Tested with a M1 Pro). I haven't done much debugging on the Metal EEVEE side so I might be wrong about this, but error messages suggests it is a similar issue as before. 'coat_tint.rgb != vec3(1.0)' vector of 3 'bool' values is not contextually convertible to 'bool'. Note: Cycles Metal does compile and work. EEVEE with OpenGL (like on Windows or Linux) doesn't crash. This should work now I think, could you please test again? > There is a mismatch between Cycles SVM and Cycles OSL when the coat IOR is below 1.0. Cycles SVM behaves like Coat IOR is 1.0. Cycles with OSL kind of behaves like the selected IOR and shows a Total Internal Reflection like effect, but is also a bit broken if I understand my physics correctly. Fixed, thanks. IOR is forced to be >1 here since it's a coating. IOR<1 in general is a hack for modeling nested dielectrics currently, I still hope to get proper support for this in the future. > The EEVEE material needs to be updated to match Cycles. At the moment it doesn't match (Tested on Windows with a Nvidia GPU). This is most noticeable when the coat tint has a strong colour, but coat is set low (or to 0). It's like the EEVEE implementation hasn't been updated with the recent changes? Fixed, thanks. > I also noticed a weird behavior with certain coat colours and device configurations. If the coat tint has a blue channel set to 0, and you're rendering with the CPU, then the coat will appear overly dark. Can reproduce, this is weird. I'll try to find the issue. > Defaults in the startup.blend can be changed in source/blender/blenloader/intern/versioning_defaults.cc. It's generally preferred to do that over updating the startup.blend for every template, since there is some risk in that. Perfect, thanks! I'll have a look.
Lukas Stockner added 2 commits 2023-08-15 15:15:49 +02:00
Author
Member
  • Fixed the blue=0 issue
  • Adjusted versioning - now the IOR 1.2 trick is only used when the Coat input was dynamic (in this case, we avoid needing a Math node). If the input has a fixed value, just adjust it directly. This also solves the default file topic.
- Fixed the blue=0 issue - Adjusted versioning - now the IOR 1.2 trick is only used when the Coat input was dynamic (in this case, we avoid needing a Math node). If the input has a fixed value, just adjust it directly. This also solves the default file topic.

@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/PR110993) when ready.
Brecht Van Lommel requested changes 2023-08-15 15:59:59 +02:00
Brecht Van Lommel left a comment
Owner

Looks fine to me assuming GPU build passes on all platforms and the comment is addressed.

Looks fine to me assuming GPU build passes on all platforms and the comment is addressed.
@ -38,2 +38,2 @@
static const pxr::TfToken clearcoat("clearcoat", pxr::TfToken::Immortal);
static const pxr::TfToken clearcoatRoughness("clearcoatRoughness", pxr::TfToken::Immortal);
static const pxr::TfToken coat("coat", pxr::TfToken::Immortal);
static const pxr::TfToken coatRoughness("coatRoughness", pxr::TfToken::Immortal);

These should not be renamed.

These should not be renamed.
Member

There's still a mismatch between Cycles SVM, Cycles OSL, and EEVEE.

When the coat IOR is below or at 1.0, all three behave differently.

  • Cycles SVM behaves like IOR is 1.0 and there are no specular reflections (Based on the comments you've made, it seems this is the expected result)
  • Cycles OSL continues to show total internal reflection effects. (I have left a comment in the code on how to fix this)
  • EEVEE matches Cycles SVM, with the exception of it showing specular reflections when Coat IOR is 1.0 or lower. I don't know enough about this area to know if this is a oversight or a limitation.

Other than that, many of the previous issues have been addressed. Metal EEVEE and Cycles compiles and work just fine. EEVEE now more closely matches Cycles (with the exception of the IOR = 1.0 edge case). CPU and GPU now match in Cycles with the blue=0 issue being fixed.

There's still a mismatch between Cycles SVM, Cycles OSL, and EEVEE. When the coat IOR is below or at 1.0, all three behave differently. - Cycles SVM behaves like IOR is 1.0 and there are no specular reflections (Based on the comments you've made, it seems this is the expected result) - Cycles OSL continues to show total internal reflection effects. (I have left a comment in the code on how to fix this) - EEVEE matches Cycles SVM, with the exception of it showing specular reflections when Coat IOR is 1.0 or lower. I don't know enough about this area to know if this is a oversight or a limitation. --- Other than that, many of the previous issues have been addressed. Metal EEVEE and Cycles compiles and work just fine. EEVEE now more closely matches Cycles (with the exception of the IOR = 1.0 edge case). CPU and GPU now match in Cycles with the blue=0 issue being fixed.
Alaska reviewed 2023-08-16 02:27:49 +02:00
@ -104,0 +109,4 @@
if (Coat > 1e-5) {
float coat_r2 = CoatRoughness * CoatRoughness;
closure color CoatBSDF = dielectric_bsdf(
CoatNormal, vector(0.0), color(1.0), color(0.0), coat_r2, coat_r2, CoatIOR, "ggx");
Member

The CoatIOR in closure color CoatBSDF = dielectric_bsdf(CoatNormal, vector(0.0), color(1.0), color(0.0), coat_r2, coat_r2, CoatIOR, "ggx"); also needs to be set to max(CoatIOR, 1.0) to avoid inconsistencies when compared to Cycles SVM when the CoatIOR is below 1.0.

The `CoatIOR` in `closure color CoatBSDF = dielectric_bsdf(CoatNormal, vector(0.0), color(1.0), color(0.0), coat_r2, coat_r2, CoatIOR, "ggx");` also needs to be set to `max(CoatIOR, 1.0)` to avoid inconsistencies when compared to Cycles SVM when the `CoatIOR` is below 1.0.
LukasStockner marked this conversation as resolved
Alaska reviewed 2023-08-16 04:03:27 +02:00
@ -106,0 +109,4 @@
#define SOCK_COAT_ROUGHNESS_ID 16
b.add_input<decl::Float>("Coat IOR")
.default_value(1.5f)
.min(0.0f)
Member

If CoatIORs below 1.0 aren't supported at this time, then I would personally like see to min IOR set to 1.0.

If CoatIORs below 1.0 aren't supported at this time, then I would personally like see to min IOR set to 1.0.
LukasStockner marked this conversation as resolved
Weizhen Huang added 1 commit 2023-09-01 12:43:51 +02:00
Weizhen Huang added 1 commit 2023-09-01 13:50:55 +02:00
Lukas Stockner added 2 commits 2023-09-11 04:27:11 +02:00
Author
Member

There's still a mismatch between Cycles SVM, Cycles OSL, and EEVEE.

When the coat IOR is below or at 1.0, all three behave differently.

  • Cycles SVM behaves like IOR is 1.0 and there are no specular reflections (Based on the comments you've made, it seems this is the expected result)
  • Cycles OSL continues to show total internal reflection effects. (I have left a comment in the code on how to fix this)
  • EEVEE matches Cycles SVM, with the exception of it showing specular reflections when Coat IOR is 1.0 or lower. I don't know enough about this area to know if this is a oversight or a limitation.

Other than that, many of the previous issues have been addressed. Metal EEVEE and Cycles compiles and work just fine. EEVEE now more closely matches Cycles (with the exception of the IOR = 1.0 edge case). CPU and GPU now match in Cycles with the blue=0 issue being fixed.

Thanks for checking again - I've applied the OSL fix and limited the input to a minimum of 1.0 now. I'm not really sure what you mean with the EEVEE issue, though - I don't see anything obviously weird.

> There's still a mismatch between Cycles SVM, Cycles OSL, and EEVEE. > > When the coat IOR is below or at 1.0, all three behave differently. > - Cycles SVM behaves like IOR is 1.0 and there are no specular reflections (Based on the comments you've made, it seems this is the expected result) > - Cycles OSL continues to show total internal reflection effects. (I have left a comment in the code on how to fix this) > - EEVEE matches Cycles SVM, with the exception of it showing specular reflections when Coat IOR is 1.0 or lower. I don't know enough about this area to know if this is a oversight or a limitation. > > --- > > Other than that, many of the previous issues have been addressed. Metal EEVEE and Cycles compiles and work just fine. EEVEE now more closely matches Cycles (with the exception of the IOR = 1.0 edge case). CPU and GPU now match in Cycles with the blue=0 issue being fixed. Thanks for checking again - I've applied the OSL fix and limited the input to a minimum of 1.0 now. I'm not really sure what you mean with the EEVEE issue, though - I don't see anything obviously weird.
Alaska reviewed 2023-09-11 07:27:28 +02:00
@ -481,15 +481,12 @@ void USDMaterialReader::set_principled_node_inputs(bNode *principled,
set_node_input(roughness_input, principled, "Roughness", ntree, column, &context);
}
if (pxr::UsdShadeInput clearcoat_input = usd_shader.GetInput(usdtokens::clearcoat)) {
Member

I'm getting build errors here. Seems to be because you updated the names earlier in 96b362cac7 but didn't update them here.

/blender/source/blender/io/usd/intern/usd_reader_material.cc:484:70: error: no member named 'coat' in namespace 'usdtokens'
  if (pxr::UsdShadeInput coat_input = usd_shader.GetInput(usdtokens::coat)) {
                                                          ~~~~~~~~~~~^

/blender/source/blender/io/usd/intern/usd_reader_material.cc:488:80: error: no member named 'coatRoughness' in namespace 'usdtokens'
  if (pxr::UsdShadeInput coat_roughness_input = usd_shader.GetInput(usdtokens::coatRoughness)) {
                                                                    ~~~~~~~~~~~^~~~~~~~~~~~~
I'm getting build errors here. Seems to be because you updated the names earlier in https://projects.blender.org/blender/blender/commit/96b362cac7fb039f5717d5eb28a556d5dc75f0fb but didn't update them here. ``` /blender/source/blender/io/usd/intern/usd_reader_material.cc:484:70: error: no member named 'coat' in namespace 'usdtokens' if (pxr::UsdShadeInput coat_input = usd_shader.GetInput(usdtokens::coat)) { ~~~~~~~~~~~^ /blender/source/blender/io/usd/intern/usd_reader_material.cc:488:80: error: no member named 'coatRoughness' in namespace 'usdtokens' if (pxr::UsdShadeInput coat_roughness_input = usd_shader.GetInput(usdtokens::coatRoughness)) { ~~~~~~~~~~~^~~~~~~~~~~~~ ```
Author
Member

Ah, I was building with USD disabled. I'll fix it, thanks.

Ah, I was building with USD disabled. I'll fix it, thanks.
Member

I'm not really sure what you mean with the EEVEE issue, though - I don't see anything obviously weird.

Either I was testing it incorrectly, or it was fixed by recent changes/merges with main. I see no obvious issues with EEVEE now.

> I'm not really sure what you mean with the EEVEE issue, though - I don't see anything obviously weird. Either I was testing it incorrectly, or it was fixed by recent changes/merges with main. I see no obvious issues with EEVEE now.
Member

I rewrote EEVEE principled BSDF #111754, and adjusted the part in this pull request so that it uses coat IOR instead of the default 1.5 in the merge commit 445464e5f2

I rewrote EEVEE principled BSDF #111754, and adjusted the part in this pull request so that it uses coat IOR instead of the default 1.5 in the merge commit 445464e5f2999a727d0795b05e0519b73a75b090
Brecht Van Lommel approved these changes 2023-09-11 18:27:48 +02:00
Brecht Van Lommel left a comment
Owner

Looks good to me assuming those USD tokens giving build errors are updated to be clearcoat instead of coat.

Looks good to me assuming those USD tokens giving build errors are updated to be clearcoat instead of coat.
Author
Member

I rewrote EEVEE principled BSDF #111754, and adjusted the part in this pull request so that it uses coat IOR instead of the default 1.5 in the merge commit 445464e5f2

Fantastic, thanks!

> I rewrote EEVEE principled BSDF #111754, and adjusted the part in this pull request so that it uses coat IOR instead of the default 1.5 in the merge commit 445464e5f2999a727d0795b05e0519b73a75b090 Fantastic, thanks!
Lukas Stockner added 3 commits 2023-09-13 00:00:31 +02:00
Lukas Stockner merged commit 158dbc1b10 into main 2023-09-13 00:03:19 +02:00
Lukas Stockner deleted branch clearcoat-principled 2023-09-13 00:03:20 +02:00
First-time contributor

How is the tint change actually done. In other renderers they sometimes have a setting for weight or clear coat thickness. This will influence how much the tint effects the final color.

Today in blender today, someone pointed out. That in 4.1 branch, the material out has a attribute called thickness. There is no documentation yet. Is this attribute related to coat settings?

How is the tint change actually done. In other renderers they sometimes have a setting for weight or clear coat thickness. This will influence how much the tint effects the final color. Today in blender today, someone pointed out. That in 4.1 branch, the material out has a attribute called thickness. There is no documentation yet. Is this attribute related to coat settings?
Member

How is the tint change actually done. In other renderers they sometimes have a setting for weight or clear coat thickness.

At the moment, the coat layer has a thickness of "1 absorption distance" (This isn't a physical unit, it's a mathematical thing to apply the tint properly). And how much tint is applied is based on the "optical depth" (depending on how a ray enters a material, it might travel straight through the coat, or through the coat at a angle, if it travels through at an angle, it will travel further than straight through, increasing the "optical depth"). Along with the optical depth, the Coat Weight will increase or decrease the amount of tinting being applied.

Today in blender today, someone pointed out. That in 4.1 branch, the material out has a attribute called thickness. There is no documentation yet. Is this attribute related to coat settings?

The Thickness setting on the material output node is unrelated to Coat Tint. Based on what I understand, this is probably related to the Refraction Depth and other depth settings used by EEVEE.

> How is the tint change actually done. In other renderers they sometimes have a setting for weight or clear coat thickness. At the moment, the coat layer has a thickness of "1 absorption distance" (This isn't a physical unit, it's a mathematical thing to apply the tint properly). And how much tint is applied is based on the "optical depth" (depending on how a ray enters a material, it might travel straight through the coat, or through the coat at a angle, if it travels through at an angle, it will travel further than straight through, increasing the "optical depth"). Along with the optical depth, the `Coat Weight` will increase or decrease the amount of tinting being applied. > Today in blender today, someone pointed out. That in 4.1 branch, the material out has a attribute called thickness. There is no documentation yet. Is this attribute related to coat settings? The `Thickness` setting on the material output node is unrelated to Coat Tint. Based on what I understand, this is probably related to the `Refraction Depth` and other depth settings used by EEVEE.
First-time contributor

@Alaska thanks for the reply

Wonder why don't add coat thickness then. I mean they added color for it and use the thickness, but have added a hardcoded value as it seems. An old renderer I use did have custom weighting, was really nice feature. Specially on woods and car paints

@Alaska thanks for the reply Wonder why don't add coat thickness then. I mean they added color for it and use the thickness, but have added a hardcoded value as it seems. An old renderer I use did have custom weighting, was really nice feature. Specially on woods and car paints
Member

@RomboutVersluijs I don't know exactly why a thickness option wasn't added. Maybe it was to align with another material standard (OpenPBR/Standard surface)? Maybe it was for other reasons. You will need to wait for a response from Brecht and/or Lukas to get the exact reasoning.

Note: Discussion about features added to the Principled BSDF in 4.0+ should probably happen over on devtalk in the feedback thread for the Pricinipled BSDF: https://devtalk.blender.org/t/principled-v2-feedback-discussion-thread/24997

@RomboutVersluijs I don't know exactly why a thickness option wasn't added. Maybe it was to align with another material standard (OpenPBR/Standard surface)? Maybe it was for other reasons. You will need to wait for a response from Brecht and/or Lukas to get the exact reasoning. Note: Discussion about features added to the Principled BSDF in 4.0+ should probably happen over on devtalk in the feedback thread for the Pricinipled BSDF: https://devtalk.blender.org/t/principled-v2-feedback-discussion-thread/24997
First-time contributor

Im not sure how much he is involved with the adjustments. To my understanding it's Lucas stocker doing the work. He also did a bcon presentation on it last year. I'm very hyped for all the work he has and is doing

Im not sure how much he is involved with the adjustments. To my understanding it's Lucas stocker doing the work. He also did a bcon presentation on it last year. I'm very hyped for all the work he has and is doing
Author
Member

Coat Thickness is one of the many options that could theoretically be added, but in practice it's always a balancing act between supporting more use cases vs. keeping the node manageable.

In this case, thickness can easily be implemented manually with a Power node (as in, set the Tint input to pow(original tint, thickness)), so I don't think having it as an option directly in the Principled BSDF really adds much.

Side note: Coat Weight effectively was a thickness input before #113468.

Coat Thickness is one of the many options that could theoretically be added, but in practice it's always a balancing act between supporting more use cases vs. keeping the node manageable. In this case, thickness can easily be implemented manually with a Power node (as in, set the Tint input to `pow(original tint, thickness)`), so I don't think having it as an option directly in the Principled BSDF really adds much. Side note: Coat Weight effectively was a thickness input before #113468.
First-time contributor

@LukasStockner but how is the tint than controlled if it's occupied by the power input?

@LukasStockner but how is the tint than controlled if it's occupied by the power input?
Member

@RomboutVersluijs , the node setup Lukas is referring too likely looks like this:

Coat with thickness.png

Adjust the Coat Tint and Thickness values on the left to get the color and thickness you want.

Note: For ideal results, only use this node setup in combination with a Coat Weight of 1. And until #113468 is merged in Blender/Cycles, you may get "unexpected results" with this setup with a Coat Wight set to anything other than 0 or 1.

@RomboutVersluijs , the node setup Lukas is referring too likely looks like this: ![Coat with thickness.png](/attachments/07766acf-7fa9-44d2-8ad1-7900c851cd83) Adjust the Coat Tint and Thickness values on the left to get the color and thickness you want. Note: For ideal results, only use this node setup in combination with a `Coat Weight` of 1. And until #113468 is merged in Blender/Cycles, you may get "unexpected results" with this setup with a Coat Wight set to anything other than 0 or 1.
First-time contributor

@Alaska can I ask why the color is separated per component and then gets the power exponent. Is that because the method cannot handle the color at once, meaning a non separated color. Guess it's single value only, hence the color of the socket

@Alaska can I ask why the color is separated per component and then gets the power exponent. Is that because the method cannot handle the color at once, meaning a non separated color. Guess it's single value only, hence the color of the socket
Member

Is that because the method cannot handle the color at once, meaning a non separated color. Guess it's single value only, hence the color of the socket

Yes, the Math node only handles single number inputs. Which means that when you plug a Color input into it (Which is three numbers: Red, Green, Blue), it will be reduced to a single number, have the math applied to it, then output as a single number. This means we lose our color information if we pass a color input straight through a math node.

To get the desired result, we have to separate the individual color channels and apply the power to each color channel separately.

Note: You don't have to do this for all math operations you want to apply to the color input. The Mix RGB node supports various mathematical operations you can apply to color inputs. It just doesn't support power, which is required for this use case.

> Is that because the method cannot handle the color at once, meaning a non separated color. Guess it's single value only, hence the color of the socket Yes, the Math node only handles single number inputs. Which means that when you plug a Color input into it (Which is three numbers: Red, Green, Blue), it will be reduced to a single number, have the math applied to it, then output as a single number. This means we lose our color information if we pass a color input straight through a math node. To get the desired result, we have to separate the individual color channels and apply the power to each color channel separately. Note: You don't have to do this for all math operations you want to apply to the color input. The `Mix RGB` node supports various mathematical operations you can apply to color inputs. It just doesn't support `power`, which is required for this use case.
First-time contributor

@alaska sorry for the questions. But was does power actually do in this setup. How is it different vs boosting the color itself. I mean if the color component is multiplied by a certain number, is it the same as make the color values higher and thus more towards white?

@alaska sorry for the questions. But was does power actually do in this setup. How is it different vs boosting the color itself. I mean if the color component is multiplied by a certain number, is it the same as make the color values higher and thus more towards white?
Member

But was does power actually do in this setup.

The power node in this setup will increase the saturation in some situations and decrease the brightness of the coat as the "thickness" input is increased. This simulates the properties of a thicker coat (which absorbs more light as a result of light passing through more material).

I mean if the color component is multiplied by a certain number, is it the same as make the color values higher and thus more towards white?

Color values are typically in the range of [0..1]. As a result, it's likely your color will be something like this (Red: 0.3, Green: 0.8, Blue: 0.9). When you take a value less than 1, and raise it to the power of anything greater than 1, the output value will be lower than the input, which creates the "decreased brightness effect". The increased saturation effect comes from the non-linear relationship between input and output when working with powers, and the fact we're applying this operation to three different values (Red, Green, Blue). This is why it's different from just boosting the color until you reach white.

Example showing why using power can decrease the brightness:

Red, Green, Blue = 0.5, Thickness = 2
0.5^2 = 0.25 Because:
0.5^2 = 0.5 * 0.5 = 0.25

Note: If you want to use this method to add coat thickness to your materials, make sure your Red, Green, and Blue channels are all greater than 0, and less than 1, and avoid using thickness values less than 1. The RGB nature of Cycles and approximations in this thickness "simulation" make these two cases behave a bit weird. I should of mentioned this earlier.

> But was does power actually do in this setup. The power node in this setup will increase the saturation in some situations and decrease the brightness of the coat as the "thickness" input is increased. This simulates the properties of a thicker coat (which absorbs more light as a result of light passing through more material). > I mean if the color component is multiplied by a certain number, is it the same as make the color values higher and thus more towards white? Color values are typically in the range of [0..1]. As a result, it's likely your color will be something like this (Red: 0.3, Green: 0.8, Blue: 0.9). When you take a value less than 1, and raise it to the power of anything greater than 1, the output value will be lower than the input, which creates the "decreased brightness effect". The increased saturation effect comes from the non-linear relationship between input and output when working with powers, and the fact we're applying this operation to three different values (Red, Green, Blue). This is why it's different from just boosting the color until you reach white. Example showing why using power can decrease the brightness: ``` Red, Green, Blue = 0.5, Thickness = 2 0.5^2 = 0.25 Because: 0.5^2 = 0.5 * 0.5 = 0.25 ``` Note: If you want to use this method to add coat thickness to your materials, make sure your Red, Green, and Blue channels are all greater than 0, and less than 1, and avoid using thickness values less than 1. The RGB nature of Cycles and approximations in this thickness "simulation" make these two cases behave a bit weird. I should of mentioned this earlier.
First-time contributor

@Alaska thanks for the very clear and concise explanation. I understand now. I was thinking to simple of this. Now it makes sense

@Alaska thanks for the very clear and concise explanation. I understand now. I was thinking to simple of this. Now it makes sense
Sign in to join this conversation.
No reviewers
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
6 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#110993
No description provided.