Cycles/EEVEE: Change Specular input on Principled BSDF to affect IOR #112552

Lukas Stockner · 2023-09-19T05:03:40+02:00

Lukas Stockner commented

2023-09-19 05:03:40 +02:00

In the original 3.6 code, the specular reflection component completely ignored the IOR input, and instead computed its own IOR based on the Specular input.

In the current 4.0 code, it uses the IOR input, but tweaks F0 based on the Specular input. The downside of this is that unlike 3.6, it's no longer possible to e.g. completely disable specular reflections through the Specular input, since F90 is unaffected.

This PR changes to the behavior of e.g. OpenPBR: Compute an adjusted IOR based on both the IOR and the Specular input, and then use that for both specular and transmission. That naturally results in the shader using IOR 1.0 if Specular=0, it is compliant with OpenPBR, and it will integrate nicely with e.g. the Nested Dielectrics in the future.

Note that this also affects refractions, but that should be fine - materials should be either opaque or transparent, and transparent materials have no reason to use the Specular input anyways.

In the original 3.6 code, the specular reflection component completely ignored the IOR input, and instead computed its own IOR based on the Specular input. In the current 4.0 code, it uses the IOR input, but tweaks F0 based on the Specular input. The downside of this is that unlike 3.6, it's no longer possible to e.g. completely disable specular reflections through the Specular input, since F90 is unaffected. This PR changes to the behavior of e.g. OpenPBR: Compute an adjusted IOR based on both the IOR and the Specular input, and then use that for both specular and transmission. That naturally results in the shader using IOR 1.0 if Specular=0, it is compliant with OpenPBR, and it will integrate nicely with e.g. the Nested Dielectrics in the future. Note that this also affects refractions, but that should be fine - materials should be either opaque or transparent, and transparent materials have no reason to use the Specular input anyways.

Lukas Stockner requested review from Brecht Van Lommel 2023-09-19 05:03:57 +02:00

Lukas Stockner requested review from Weizhen Huang 2023-09-19 05:03:57 +02:00

Lukas Stockner added this to the Render & Cycles project 2023-09-19 05:04:00 +02:00

Alaska commented

2023-09-19 06:57:02 +02:00

Note that this also affects refractions, but that should be fine - materials should be either opaque or transparent, and transparent materials have no reason to use the Specular input anyways.

This is just personal preference, but I really don't like this for a few reasons:

What if a users has been playing around with the specular slider on a diffuse material then switches to a transmissive materiel? They may not know that the IOR isn't what the UI says because the specular slider isn't set to the default value.
When a user discovers that the specular slider adjusts the IOR of refractive materials, they may become confused, what value should the Specular slider be set too to get the correct IOR? Many people would assume 1.0, but it's actually 0.5 (if I read the code correctly). This will probably lead to many people setting the wrong value for things. The tool tips do explain this. But many people don't read tool tips.

> Note that this also affects refractions, but that should be fine - materials should be either opaque or transparent, and transparent materials have no reason to use the Specular input anyways. This is just personal preference, but I really don't like this for a few reasons: 1. What if a users has been playing around with the specular slider on a diffuse material then switches to a transmissive materiel? They may not know that the IOR isn't what the UI says because the specular slider isn't set to the default value. 2. When a user discovers that the specular slider adjusts the IOR of refractive materials, they may become confused, what value should the Specular slider be set too to get the correct IOR? Many people would assume 1.0, but it's actually 0.5 (if I read the code correctly). This will probably lead to many people setting the wrong value for things. The tool tips do explain this. But many people don't read tool tips.

Weizhen Huang requested changes 2023-09-19 10:44:00 +02:00

Weizhen Huang left a comment

A specular value of 0 giving a diffuse material does sound more intuitive to me than the current approach. But I agree with Alaska that if transparent materials should not make use of the specular input, then we also shouldn't let the specular input affect the transparent IOR. We can adjust the specular IOR locally in its own block.

Also, I do not agree that materials should be either opaque or transparent, a Transmission value between 0 and 1 can model translucent/semi-transparent materials.

A specular value of 0 giving a diffuse material does sound more intuitive to me than the current approach. But I agree with Alaska that if transparent materials should not make use of the specular input, then we also shouldn't let the specular input affect the transparent IOR. We can adjust the specular IOR locally in its own block. Also, I do not agree that materials should be either opaque or transparent, a Transmission value between 0 and 1 can model translucent/semi-transparent materials.

source/blender/draw/intern/shaders/common_math_lib.glsl Outdated

						
				@ -337,0 +341,4 @@

				}

				/* Simplified form of F_eta(eta, 1.0). */

				float F0_from_ior(float eta)

Weizhen Huang commented

2023-09-19 10:33:15 +02:00

Don't move F0_from_ior() to here because

EEVEE Next implements this function in eevee_nodetree_lib.glsl which would be a redeclaration
This common_math_lib.glsl will be replaced by gpu_shader_math_base_lib.glsl.

ior_from_f0() could probably stay here for now, otherwise you need to implement it in eevee_nodetree_lib.glsl and add a forward declaration in closure_type_lib.glsl.
BTW should there be a consistent naming of either f0 or F0? I personally prefer F0 because lower letters usually indicate the amplitude/coefficients, and upper letters are the squared results/reflectance.

Don't move `F0_from_ior()` to here because 1. EEVEE Next implements this function in `eevee_nodetree_lib.glsl` which would be a redeclaration 2. This `common_math_lib.glsl` will be replaced by `gpu_shader_math_base_lib.glsl`. `ior_from_f0()` could probably stay here for now, otherwise you need to implement it in `eevee_nodetree_lib.glsl` and add a forward declaration in `closure_type_lib.glsl`. BTW should there be a consistent naming of either f0 or F0? I personally prefer F0 because lower letters usually indicate the amplitude/coefficients, and upper letters are the squared results/reflectance.

Lukas Stockner commented

2023-09-25 01:42:59 +02:00

Ah, yeah, I wasn't sure what the ideal place for it is here.

With the updated version, this is irrelevant anyways since we don't need ior_from_f0 anymore.

Also, I agree that we should standardize on F0.

Ah, yeah, I wasn't sure what the ideal place for it is here. With the updated version, this is irrelevant anyways since we don't need `ior_from_f0` anymore. Also, I agree that we should standardize on `F0`.

brecht marked this conversation as resolved

source/blender/nodes/shader/nodes/node_shader_bsdf_principled.cc Outdated

						
				@ -102,0 +101,4 @@

				      .subtype(PROP_FACTOR)

				      .description(

				          "Adjustment to the IOR to increase or decrease specular intensity "

				          "(0.5 means no adjustment, 0 removes all reflections, 1 doubles them)");

Weizhen Huang commented

2023-09-19 09:23:55 +02:00

This description is incorrect. A value of 1 doubles the reflectance only at normal incidence

Lukas Stockner commented

2023-09-25 01:41:40 +02:00

True, but I'm not really sure how to phrase it better without making the description overly verbose.

Weizhen Huang commented

2023-09-25 02:25:02 +02:00

Maybe “Adjust the specular reflectivity by modulating the IOR (0.5 means no adjustment, 0 removes all reflections, 1 doubles the reflectivity at normal incidence)”?

brecht marked this conversation as resolved

Weizhen Huang changed title from ~~Cycles: Change Specular input on Principled BSDF to affect IOR~~ to Cycles/EEVEE: Change Specular input on Principled BSDF to affect IOR

2023-09-19 10:49:00 +02:00

Brecht Van Lommel commented

2023-09-19 18:17:52 +02:00

I think not applying it to the transmission component makes sense. As in not have it affect the glass BSDF at all, both reflection and refraction. It's actually not clear to me what OpenPBR does here.

We should probably rename this input, since "Specular" makes it seem like something you set to 1 to enable full specularity, while the neutral value is actually 0.5. I don't really have a good naming suggestion besides "IOR Level" as in OpenPBR.

I think not applying it to the transmission component makes sense. As in not have it affect the glass BSDF at all, both reflection and refraction. It's actually not clear to me what OpenPBR does here. We should probably rename this input, since "Specular" makes it seem like something you set to 1 to enable full specularity, while the neutral value is actually 0.5. I don't really have a good naming suggestion besides "IOR Level" as in OpenPBR.

Brecht Van Lommel referenced this pull request

2023-09-19 19:27:44 +02:00

Principled v2 BSDF #99447

Lukas Stockner commented

2023-09-25 01:44:06 +02:00

Good point about the transmission. OpenPBR does combine them (see the section about dispersion), but for now I think we can keep them separate.

As for naming - maybe "Specular Adjust"?

Good point about the transmission. OpenPBR does combine them (see the section about dispersion), but for now I think we can keep them separate. As for naming - maybe "Specular Adjust"?

Lukas Stockner force-pushed principled-specular-ior-adjust from ccb1c973ed to 7bfad432bf

2023-09-25 01:58:31 +02:00

Compare

Weizhen Huang commented

2023-09-25 02:21:26 +02:00

OpenPBR does combine them

Indeed, they say “We assume that specular_ior (including any modulation via specular_ior_level as in equation ) defines n”

I don’t really like this approach and I’m not sure how practical it is to have two inputs doing the same thing, if users want the material to be a bit more specular they could just tune the IOR a bit higher.. Maybe ask for some feedbacks in the forum?

As for naming - maybe "Specular Adjust"?

If this is really just modulating the IOR, better put that in the name just as they do, Specular IOR level.

> OpenPBR does combine them Indeed, they say “We assume that specular_ior (including any modulation via specular_ior_level as in equation ) defines n” I don’t really like this approach and I’m not sure how practical it is to have two inputs doing the same thing, if users want the material to be a bit more specular they could just tune the IOR a bit higher.. Maybe ask for some feedbacks in the forum? > As for naming - maybe "Specular Adjust"? If this is really just modulating the IOR, better put that in the name just as they do, Specular IOR level.

Brecht Van Lommel referenced this issue from a commit

2023-09-25 12:42:42 +02:00

Shader: change Specular input on Principled BSDF to affect IOR

Brecht Van Lommel referenced this issue from a commit

2023-09-25 12:44:56 +02:00

Shader: change Specular input on Principled BSDF to affect IOR

Brecht Van Lommel referenced this issue from a commit

2023-09-25 13:26:44 +02:00

Shader: change Specular input on Principled BSDF to affect IOR

Brecht Van Lommel referenced this issue from a commit

2023-09-25 13:58:37 +02:00

Shader: change Specular input on Principled BSDF to affect IOR