Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KHR_materials_clearcoat and KHR_materials_specular #1677

Closed

Conversation

UX3D-nopper
Copy link
Contributor

As requested, the pull request for clearcoat. Please note, not final yet.
Also, the "specular" for F0 extension is added as well.

@sebavan
Copy link
Contributor

sebavan commented Oct 17, 2019

About clear coat, I am starting the implementation in Babylon but I wonder if we could enforce that the intensity factor would be stored in the texture R channel and roughness in the G ?

This could help packing the clear coat information together and so we would require only one sampler ?

Here is how we currently do it in Babylon: https://github.com/BabylonJS/Babylon.js/blob/05c912cc8aa1984c861eeb580e760da0f37a58f1/src/Shaders/pbr.fragment.fx#L621

@sebavan
Copy link
Contributor

sebavan commented Oct 17, 2019

Also I guess the default clear coat factor and roughness should be 1 and not 0 to ensure it multiplies correctly with the texture ?

@sebavan
Copy link
Contributor

sebavan commented Oct 17, 2019

Finally shouldn't the clear coat normal texture has a factor as well as it is the case for the core normal texture ? Basically being a normalTextureInfo instead of TextureInfo.

Scratch that it is already a normalTextureInfo so all good.

@UX3D-nopper
Copy link
Contributor Author

@sebavan The default factor is by purpose 0, that by default the effect is off. If one wants to have the effect enabled, one has to set the required value. Otherwise, all materials would have per default clear coat on, which is not wanted (e.g. compare with emissive).
In glTF, a parameter is stored in a texture and as a scalar/vector. So the factors are per purpose not in the texture, as these channels in the texture already do exist.
Finally, this normal texture is "derived" from the normal texture schema, so the mentioned factor is implicitly defined.

@sebavan
Copy link
Contributor

sebavan commented Oct 17, 2019

But as long as you have the extension the intent is probably to have it on, hence the proposed value of 1 and if you want it off, why simply not putting the extension ?

Yup agreed for the normal, I put out the comment too fast.

Now what about using only one texture packing the both clear coat related info ?

@UX3D-nopper
Copy link
Contributor Author

It's already packed in R and G: https://github.com/ux3d/glTF/blob/KHR_materials_pbrClearcoat/extensions/2.0/Khronos/KHR_materials_clearcoat/schema/glTF.KHR_materials_clearcoat.schema.json
If a shader supports clear coat - and you do not want to change the exisitng materials - it is more convenient to turon off the effect by default 0.0. It's just the glTF philosophy.

@sebavan
Copy link
Contributor

sebavan commented Oct 17, 2019

Ok for R and G I was only looking at the table, I wonder if we could precise it in it or if it is redundant info ?

@sebavan
Copy link
Contributor

sebavan commented Oct 17, 2019

Oh and by the way @UX3D-nopper do you happen to have a test asset ?

@UX3D-nopper
Copy link
Contributor Author

The schema "rules", but yes, could be explictly mentioned in the markdown document as well.

@UX3D-nopper
Copy link
Contributor Author

UX3D-nopper commented Nov 29, 2019

My team was getting back to me and they have raised a concern regarding the blending formula in clear coat. The formula is quite nice, if we only have one light. But if we have multiple lights e.g. IBL plus two punctual, the blending formula is not usable.

Actually, NdotL and HdotV are the problem.

If You have a look at other engines using clear coat, they do not make such effort and basically - more or less - add the clear coat value.

@UX3D-nopper
Copy link
Contributor Author

Suggest to simplify to:

blendFactor = clearcoatFactor * fresnel(0.04, NdotV)
f = (f_emissive + f_diffuse) * (1.0 - blendFactor) + mix(f_specular, f_clearcoat, blendFactor)

e.g. Substance Painter from Allegorithmic is doing it like this.

Taken from Substance Painter Docs.
@UX3D-nopper UX3D-nopper added the needs discussion Issue or PR requires working group discussion to resolve. label Nov 29, 2019
@bghgary
Copy link
Contributor

bghgary commented Dec 2, 2019

Babylon follows what Filament does, though we are out-of-date right now. We will update. @romainguy Do you mind reviewing?

@sebavan
Copy link
Contributor

sebavan commented Dec 2, 2019

We were still using the squared 1-Fc for specular :-) I ll update ASAP.

I think the main diff now is about the Fresnel term where we are relying on VdotH vs the proposed NdotV.

@UX3D-nopper I wonder which one it should be ?

@romainguy
Copy link

We got rid of the squared Fresnel when blending in clear coat, a while ago. We now do what @UX3D-nopper shows in his last comment (except we don't use NdotV but LdotH). @UX3D-nopper can you expand on what the issue is with multiple lights?

@sebavan
Copy link
Contributor

sebavan commented Dec 2, 2019

Agree, cause I always thought the fresnel of the BRDF should be computed from the half vector and not the normal to match with the density probability we are computing in the rest of the equation.

@UX3D-nopper
Copy link
Contributor Author

Let's say you have an image based light plus two punctual lights. When blending together, which L are u using? Obviously L per light, but with the old formula u sum up emssive every time as well.
Without the light dependency, the blending can be done once at the end.
I did not reinvent the wheel and the above formula is the one from Allegorithmic/Adobe Substance Painter.

@romainguy
Copy link

romainguy commented Dec 3, 2019

Ah so you do this as an optimization. We compute the clear coat BRDF per light and use NdotV for the IBL (and we don't attenuate the emissive bit, it's always added at the end).

@UX3D-nopper
Copy link
Contributor Author

But technically the emissive should be blended as well.

@romainguy
Copy link

Depends which layer is emissive :) TBH I don't care that much about an emissive surface with a clear coat layer being off at grazing angles. It seems like it's just one trade-off vs another one.


```
blendFactor = clearcoatFactor * fresnel(0.04, NdotV)
f = (f_emissive + f_diffuse) * (1.0 - blendFactor) + mix(f_specular, f_clearcoat, blendFactor)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this formula be simplified as follows? Is this the same formula?

f = mix(f_emissive + f_diffuse + f_specular, f_clearcoat, blendFactor)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The formula I provided is from Substance Painter and they are just blending the specular part, which does makes sense from my point of view.
Where do you have the formula from?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the same formula, just refactored. Recall that:

mix(x, y, a) = x * (1.0 - a) + y*a

Given the left side of mix is already multiplied by 1.0 - blendFactor, I don't think we need to show it being multiplied separately.

Looking at Substance Painter's code, I see why they did it that way: Their shader needed diffuse, emissive, and specular outputs as separate values, not added together. Perhaps we could show it as separate values too, but if we're showing it all added together then it may as well all go into the mix statement.

|**clearcoatRoughnessTexture** | [`textureInfo`](/specification/2.0/README.md#reference-textureInfo) | The clearcoat layer roughness texture. | No |
|**clearcoatNormalTexture** | [`normalTextureInfo`](/specification/2.0/README.md#reference-normaltextureinfo) | The clearcoat normal map texture. | No |

The clearcoat formula `f_clearcoat` is the same as the specular term from the Metallic-Roughness material, using F0 equal 0.04.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to specifying F0, perhaps also specify that metallic is zero and roughness comes from the product of clearcoatRoughnessFactor and clearcoatRoughnessTexture.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also that baseColor is vec3(1.0), full white

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For clear coat, no diffuse part is calculated. So baseColor does not have to be specified. The same with metallic.
But I agree, that how the specular part is calculated should be better described.

@koiava
Copy link

koiava commented Jan 9, 2020

I have a few comments regarding the proposed KHR_materials_specular extension. It is based on the Unreal engine specular parameter, right? Unreal engine probably inherited it from the original Disney BRDF from 2012. Blender also has an implementation of it called Principled BSDF. In this approach, specular defines the index of refraction (IOR) of the material with a value between 0 and 1, mapping to an IOR between 1.0 and 2.0 (and the corresponding F0 from 0 to 0.08). This also explains the strange default of 0.5, because it results in an IOR of 1.5, a good default for dielectric materials like plastic or glass.

However, I think this approach has some problems if it is combined with refractive materials. Due to this reason, Disney decided to get rid of the parameter when they introduced their refractive BSDF in 2015. Same for Unreal and Blender: when they added refraction, they introduced a new parameter called refraction (Unreal) and IOR (Blender) to control the IOR directly. They did not remove the old one (probably due to compatibility reasons), but they are now in the strange situation that there are two ways of defining the IOR: specular and refraction. In Unreal's guide to physically based materials, they mainly suggest to use this parameter for cavity, in which case its value will be between 0 and 0.5. They also explain how to use it as IOR, but admit that this is not needed for 99% of (non-refractive) materials. Therefore, I guess that there are not many materials out there that use this parameter to actually control the IOR. Note that Unity doesn't have this parameter at all.

I am writing this long introduction to motivate a different set of parameters for KHR_materials_specular which is better suited for refractive materials. It has three parameters:

  • specularFactor (number, min: 0, max: 1, default: 1), specularTexture (1-channel)
  • specularTintFactor, (color, min: (0,0,0), max: (1,1,1)), specularTintTexture (3-channel RGB)
  • ior (number, min: 1, max: inf, default: 1.5)

The parameters work like this (based on Appendix B: BRDF Implementation):

specular = specularFactor * specularTexture
specularTint = specularTintFactor * specularTintTexture

dielectricF0 = ((1 - ior) / (1+ior))^2
dielectricSpecular = specular * specularTint * dielectricF0
F0 = lerp(dielectricSpecular, baseColor.rgb, metallic)
F90 = lerp(specular, 1, metallic)

F = F0 + (F90 - F0) * (1 - V dot H)^5

Using these parameters, it is very easy to extend the model with refraction (via the IOR). In addition, it integrates the flexibility of the KHR_materials_pbrSpecularGlossiness into the default metallic-roughness material of glTF, so users don't have to choose between two incompatible material models.

Note that in this proposal it is impossible to set a texture for the ior value. This is on purpose, because in case of refraction it defines the IOR of the enclosed volume, which cannot vary across the surface without increasing the complexity of the model a lot (or breaking physics). The property of enclosed volumes is important in raytracers that support nested dielectrics.

There is one caveat: there is no generic, straightforward and lossless mapping from the Unreal specular to the proposed specular. However, there is hope, is works in most of the cases. If it is constant (not textured), it maps to ior, lossless. If it is textured, it is most likely used as cavity map. In this case, specular = 2.0 * unreal_specular, which is not lossless, but will most likely give good results.

Hi Tobias(@proog128 ),
On example you mentioned that
dielectricF0 = ((1 - ior) / (1+ior))^2
This will not be correct in case of nested dielectrics because it assumes that outside IOR is 1. Is it a limitation?

@proog128
Copy link
Contributor

proog128 commented Jan 9, 2020

@koiava You are right, for nested dielectrics the renderer has to set the outside IOR accordingly. I omitted it here to keep it simple. Nested dielectrics are mentioned in KHR_materials_volume, #1726.

@UX3D-nopper
Copy link
Contributor Author

Splitted up the three extensions into separate branches/pull requests:

KHR_materials_clearcoat #1740
KHR_materials_specular #1741
KHR_materials_thinfilm #1742

@UX3D-nopper UX3D-nopper deleted the KHR_materials_pbrClearcoat branch July 11, 2022 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension needs discussion Issue or PR requires working group discussion to resolve.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants