[2.1+] Implementing Clear Coat material

Design / architecture / roadmap discussions related to future of Ogre3D (version 2.0 and above)
Post Reply
rujialiu
Greenskin
Posts: 141
Joined: Mon May 09, 2016 8:21 am
x 11

[2.1+] Implementing Clear Coat material

Post by rujialiu » Thu Aug 16, 2018 2:01 pm

Hi!

I'm trying to implement clear coat material, referencing Filament:

https://github.com/google/filament/tree ... haders/src

There are three things to do:

1. add the two parameters (clearCoat and clearCoatRoughness) to the datablock AND send it to the material buffer on GPU.
That's easy. I've already done it.

2. Direct lighting.
I thought it's easy because only BRDF needs to be modified. In shading_lit.fs, filament changed f0 according to clearCoat:

pixel.f0 = mix(pixel.f0, f0ClearCoatToSurface(pixel.f0), pixel.clearCoat);

However, I find if difficult to change F0. There is no such thing as Filament's "PixelParams" structure, and many shading parameters (including F0) are pieces rather than variables. Pieces are best suited for small expressions (and fragments of expressions), but when it's better complex, my brain stops working.
And there can be multiple places to define a piece, so I have to modify them all or introduce another piece...

It'll be nice if we have a similar structure like PixelParams, and remove many small pieces. I understand that using pieces instead of functions/variables can improve shader's speed, but still, I hope it'll be easier to modify HLMS's shader code. Apart from Filament's shaders, I've also read some other PBR shader codes. I had to say that HLMS's one
is the hardest to modify. It's not hard to UNDERSTAND, but when it comes to actually change it.... I've no confidence. I even can't guarantee the shader will always compile because many pieces are "part of expression", rather than full expressions. It's possible that my modified code can compile for my software, but other peope, when using another permutation, gets compilation error.

3. IBL (environment mapping / local light probes)
Since currently Ogre doesn't do split-sum approximation and prefiltered cubemap, I think we can simply modify envColourS with filament's evaluateClearCoatIBL function (in light_indirect.fs) before calling BRDF_EnvMap (correct me if I'm wrong).

However, Filament's code has a comment "We want to use the geometric normal for the clear coat layer". It's reasonable because the clear coat layer is usally flat. However, I don't think when IBL is evaluated, there is only nNormal which already considered normal map (if it exists). The original geometric normal is lost. Should I save one for later use?

Any comments are welcome. Thanks!
3 x

User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 3844
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 150
Contact:

Re: [2.1+] Implementing Clear Coat material

Post by dark_sylinc » Sat Aug 18, 2018 4:53 pm

Hi!

Yes! Clear Coat is mostly BRDF stuff and quite isolated, which makes it an ideal feature that can be submitted via PR.

Now regarding your questions and doubts:

1. Yes, F0 handling is a mess. Ideally F0 would be handled via functions e.g.

Code: Select all

float_fresnel getF0( material, textures )
{
    return ...;
}
However several driver/API combinations are terribly bad at optimizing that pattern.

Unfortunately I went to the use of insertpiece() in the templates, which is too verbose for trivial stuff, e.g. "@insertpiece( FresnelType )" is more distracting and longer to type than float3 or float.
I was also worried about OpenGL drivers being bad at parsing macros, but I did not know at the time that macros are parsed by Ogre (exactly for this reason! drivers were bad at it). Had I known that I would've gone for macros instead:

Code: Select all

//Hlms style....
@insertpiece( FresnelType ) fresnelS

//Macro style
float_fresnel fresnelS
Eventually we'll clean this up. You can clean it up if you want to.

But this does not solve your questions, so:
is the hardest to modify. It's not hard to UNDERSTAND, but when it comes to actually change it.... I've no confidence. I even can't guarantee the shader will always compile because many pieces are "part of expression", rather than full expressions. It's possible that my modified code can compile for my software, but other peope, when using another permutation, gets compilation error.
I think pointing out and enumerating the cases can help you boost your confidence. F0 code cases can be divided into the following:
  1. fresnelS is a float. Comes from material.F0. Read only.
  2. fresnelS is a float. The value F0 comes from (material.F0 * alpha). Only when using Transparent mode. Read only.
  3. Same as 1., but comes from the read-write variable F0. F0 can come from:
    1. A texture (Fresnel workflow when a specular map is set)
    2. Derivation from diffuse texture and metalness (Metallic workflow)
    3. Additionally F0 may be multiplied against the alpha in transparent mode (just like in 2.)
  4. Repeat all the above steps, but fresnelS is now a float3. instead of a float.
So you've got 5x2 = 10 total cases, but you have mostly about 3x2 = 6 cases to worry about because many of them just sum up to "the value is stored into F0".

Likewise you have fresneD. On most cases fresnelD = 1.0 - fresnelS; except for the lesser-used BRDFs with the FLAG_SPERATE_DIFFUSE_FRESNEL flag set; where fresnelD is still sourced from F0, but is calculated with a different formula, instead of being just 1.0 - fresnelS; it's the fresnel formula but using NdotL instead of VdotH.
Anyway, since they're used infrequently and may not even work well with cloth, you can just ignore this.

If you're having trouble in that the code will work in all cases, I have two suggestions:
  1. Publish your changes into a public fork of Ogre, and request for help in the forums. More pair of eyes are better than one. We may detect that your code may not work with a particular setting combination and suggest a fix up.
  2. Sometimes just forbid that setting combo. If you feel something is blatantly incompatible (or requires very convoluted changes), detect that case in HlmsPbs::calculateHashForPreCreate and raise an exception explaining the incompatibility (or that the setting is not yet supported).
It's reasonable because the clear coat layer is usally flat. However, I don't think when IBL is evaluated, there is only nNormal which already considered normal map (if it exists). The original geometric normal is lost. Should I save one for later use?
When normal_map property is defined, the geometric normal is stored in geomNormal.
You can either use @property( normal_map ) to select between the two; or define a macro when normal_map is not defined i.e.

Code: Select all

@property( !normal_map )#define geomNormal nNormal
Cheers
1 x

rujialiu
Greenskin
Posts: 141
Joined: Mon May 09, 2016 8:21 am
x 11

Re: [2.1+] Implementing Clear Coat material

Post by rujialiu » Sun Aug 19, 2018 3:10 am

dark_sylinc wrote:
Sat Aug 18, 2018 4:53 pm
1. Yes, F0 handling is a mess. Ideally F0 would be handled via functions e.g.

Code: Select all

float_fresnel getF0( material, textures )
{
    return ...;
}
What about computing the final F0 and store it somewhere? e.g. Filament's "PixelParam" is a central storage that contains "processed material parameters" for shading.
dark_sylinc wrote:
Sat Aug 18, 2018 4:53 pm
However several driver/API combinations are terribly bad at optimizing that pattern.
What kind of pattern? Any reference (article etc)? I'm interested.
dark_sylinc wrote:
Sat Aug 18, 2018 4:53 pm
Eventually we'll clean this up. You can clean it up if you want to.
Yeah, I'd like to clean it up, but I prefer the "preprocess the parameters" approach (something like a "preprocessShadingParameters" function) instead of providing a getF0() function. Is that ok?
dark_sylinc wrote:
Sat Aug 18, 2018 4:53 pm
I think pointing out and enumerating the cases can help you boost your confidence. F0 code cases can be divided into the following:
Thanks for the info!
dark_sylinc wrote:
Sat Aug 18, 2018 4:53 pm
If you're having trouble in that the code will work in all cases, I have two suggestions:
  1. Publish your changes into a public fork of Ogre, and request for help in the forums. More pair of eyes are better than one. We may detect that your code may not work with a particular setting combination and suggest a fix up.
That's exactly what I'm planning 8-)

And also, to make sure I didn't screw up other things (since I'm essentially doing a refactor) not related to clearcoat, I'd like to do some testing to make sure the shader output doesn't change. It can be done by comparing the rendering images pixel-by-pixel, but is there any more technical way like traditional unit testing? I've found this https://github.com/cezbloch/shaderator but it's still in early development.

I've also heard that when compiled to SPIR-V, we can use SPIRV-Cross to generate debuggable C++, but it's also tagged as "Experimental". Anyway, they are better than nothing. It'll be great if there are better tools.
1 x

User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 3844
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 150
Contact:

Re: [2.1+] Implementing Clear Coat material

Post by dark_sylinc » Sun Aug 19, 2018 5:35 am

rujialiu wrote:
Sun Aug 19, 2018 3:10 am
What about computing the final F0 and store it somewhere? e.g. Filament's "PixelParam" is a central storage that contains "processed material parameters" for shading.
I just saw the code from Filament.

Storing it into a temporary struct sounds ideal, but then I remember we deal with 3 shading languages and some dodgy drivers. The simpler we stay in terms of language features (i.e. stick to primitive datatypes) the lower the risk.
It may work, I would have to see what you come up with :)
dark_sylinc wrote:
Sat Aug 18, 2018 4:53 pm
However several driver/API combinations are terribly bad at optimizing that pattern.
What kind of pattern? Any reference (article etc)? I'm interested.
Well... anything that isn't simple. Actually Metal is very good at optimization, and HLSL is also good (though it takes forever...). Most of the problems come from the GLSL/Vulkan side.

Sadly this article from Aras on why he wrote GLSL Optimizer is still relevant today.
Even with Vulkan and SPIR-V (and SPIR-V was supposed to solve these problems by moving the frontend compiler out of the driver) there are still driver bugs (particularly Android drivers have lots of bugs, for example some Android drivers go kapputt if samplers are passed as arguments to functions).
GDC18's Vulkan on Android at minute 22:20 lists all current shader driver bugs. Watchout there's a bug named "Local struct variables don't work (sometimes)".

Granted, most of the problems come from Android. But there's the every now and then problem with a desktop driver.
Yeah, I'd like to clean it up, but I prefer the "preprocess the parameters" approach (something like a "preprocessShadingParameters" function) instead of providing a getF0() function. Is that ok?
In theory sounds good, I would have to see some code to judge better.
And also, to make sure I didn't screw up other things (since I'm essentially doing a refactor) not related to clearcoat, I'd like to do some testing to make sure the shader output doesn't change. It can be done by comparing the rendering images pixel-by-pixel, but is there any more technical way like traditional unit testing? I've found this https://github.com/cezbloch/shaderator but it's still in early development.

I've also heard that when compiled to SPIR-V, we can use SPIRV-Cross to generate debuggable C++, but it's also tagged as "Experimental". Anyway, they are better than nothing. It'll be great if there are better tools.
Ogre 1.x has the Visual Unit Testing Framework. We never ported it to Ogre 2.1 though. It generates HTML output with visual comparison.
1 x

rujialiu
Greenskin
Posts: 141
Joined: Mon May 09, 2016 8:21 am
x 11

Re: [2.1+] Implementing Clear Coat material

Post by rujialiu » Mon Aug 20, 2018 6:58 am

dark_sylinc wrote:
Sun Aug 19, 2018 5:35 am
Storing it into a temporary struct sounds ideal, but then I remember we deal with 3 shading languages and some dodgy drivers. The simpler we stay in terms of language features (i.e. stick to primitive datatypes) the lower the risk.
It may work, I would have to see what you come up with :)
ah... I hate OpenGL :)
Luckily, we only use D3D11 and Metal RS, and our minimal requirement for video card is rather high, so I can concentrate on HLSL and Metal.

Here is the simplest thing I came up with. The changes are not short, but quite clear. I've only test it several hours, but it can be viewed as a proof-of-concept thing, and I'm quite happy about it.

Code: Select all

 ogre2.1/Hlms/Pbs/Any/AreaLights_piece_ps.any       |  4 +--
 ogre2.1/Hlms/Pbs/HLSL/BRDFs_piece_ps.hlsl          | 29 +++++++---------------
 ogre2.1/Hlms/Pbs/HLSL/Forward3D_piece_ps.hlsl      |  4 +--
 ogre2.1/Hlms/Pbs/HLSL/PixelShader_ps.hlsl          | 25 ++++++++++++++-----
 .../Hlms/Pbs/HLSL/Structs_piece_vs_piece_ps.hlsl   | 12 +++++++++
 ogre2.1/Hlms/Pbs/HLSL/Textures_piece_ps.hlsl       | 14 ++++-------
 6 files changed, 49 insertions(+), 39 deletions(-)

diff --git a/ogre2.1/Hlms/Pbs/Any/AreaLights_piece_ps.any b/ogre2.1/Hlms/Pbs/Any/AreaLights_piece_ps.any
index f5bd34a..0bd5c7c 100644
--- a/ogre2.1/Hlms/Pbs/Any/AreaLights_piece_ps.any
+++ b/ogre2.1/Hlms/Pbs/Any/AreaLights_piece_ps.any
@@ -136,7 +136,7 @@
 										  passBuf.areaApproxLights[@n].diffuse.xyz * diffuseMask,
 										  passBuf.areaApproxLights[@n].specular.xyz * specCol
 										@property( syntax != glsl )
-											, material, nNormal @insertpiece( brdfExtraParams )
+											, material, pixel, nNormal @insertpiece( brdfExtraParams )
 										@end
 											) * ( globalDot * globalDot ) * booster;
 		float atten = 1.0 / (0.5 + (passBuf.areaApproxLights[@n].attenuation.y + passBuf.areaApproxLights[@n].attenuation.z * fDistance) * fDistance );
@@ -154,7 +154,7 @@ INLINE float3 BRDF_AreaLightApprox
 (
 	float3 lightDir, float3 viewDir, float NdotV, float3 lightDiffuse, float3 lightSpecular
 	@property( syntax != glsl )
-		, Material material, float3 nNormal @insertpiece( brdfExtraParamDefs )
+		, Material material, PixelParams pixel, float3 nNormal @insertpiece( brdfExtraParamDefs )
 	@end
 )
 {
diff --git a/ogre2.1/Hlms/Pbs/HLSL/BRDFs_piece_ps.hlsl b/ogre2.1/Hlms/Pbs/HLSL/BRDFs_piece_ps.hlsl
index 70dcd72..1b4e5fb 100644
--- a/ogre2.1/Hlms/Pbs/HLSL/BRDFs_piece_ps.hlsl
+++ b/ogre2.1/Hlms/Pbs/HLSL/BRDFs_piece_ps.hlsl
@@ -1,18 +1,7 @@
-@property( !metallic_workflow && (!specular_map || !fresnel_workflow) )
-	@property( !transparent_mode )
-		@piece( F0 )material.F0@end
-	@end @property( transparent_mode )
-		//Premultiply F0.xyz with the alpha from the texture, but only in transparent mode.
-		@piece( F0 )(material.F0.@insertpiece( FresnelSwizzle ) * diffuseCol.w)@end
-	@end
-@end @property( metallic_workflow || (specular_map && fresnel_workflow) )
-	@piece( F0 )F0@end
-@end
-
 @property( !fresnel_scalar )
-	@piece( maxR1F0 )max( 1.0 - ROUGHNESS, @insertpiece( F0 ).x )@end
+	@piece( maxR1F0 )max( 1.0 - ROUGHNESS, pixel.f0.x )@end
 @end @property( fresnel_scalar )
-	@piece( maxR1F0 )max( (1.0 - ROUGHNESS).xxx, @insertpiece( F0 ).xyz )@end
+	@piece( maxR1F0 )max( (1.0 - ROUGHNESS).xxx, pixel.f0.xyz )@end
 @end
 
 //For mortals:
@@ -20,11 +9,11 @@
 //	getDiffuseFresnel	= 1.0 - F0 + pow( 1.0 - NdotL, 5.0 ) * F0
 //	getSpecularFresnelWithRoughness = F0 + pow( 1.0 - VdotH, 5.0 ) * (max(ROUGHNESS, (1.0 - F0)) - F0)
 //	getDiffuseFresnelWithRoughness = max(ROUGHNESS, (1.0 - F0) - F0 + pow( 1.0 - NdotL, 5.0 ) * F0
-@piece( getSpecularFresnel )@insertpiece( F0 ).@insertpiece( FresnelSwizzle ) + pow( 1.0 - VdotH, 5.0 ) * (1.0 - @insertpiece( F0 ).@insertpiece( FresnelSwizzle ))@end
-@piece( getDiffuseFresnel )1.0 - @insertpiece( F0 ).@insertpiece( FresnelSwizzle ) + pow( 1.0 - NdotL, 5.0 ) * @insertpiece( F0 ).@insertpiece( FresnelSwizzle )@end
+@piece( getSpecularFresnel )pixel.f0.@insertpiece( FresnelSwizzle ) + pow( 1.0 - VdotH, 5.0 ) * (1.0 - pixel.f0.@insertpiece( FresnelSwizzle ))@end
+@piece( getDiffuseFresnel )1.0 - pixel.f0.@insertpiece( FresnelSwizzle ) + pow( 1.0 - NdotL, 5.0 ) * pixel.f0.@insertpiece( FresnelSwizzle )@end
 
-@piece( getSpecularFresnelWithRoughness )@insertpiece( F0 ).@insertpiece( FresnelSwizzle ) + pow( 1.0 - VdotH, 5.0 ) * (@insertpiece( maxR1F0 ) - @insertpiece( F0 ).@insertpiece( FresnelSwizzle ))@end
-@piece( getDiffuseFresnelWithRoughness )@insertpiece( maxR1F0 ) - @insertpiece( F0 ).@insertpiece( FresnelSwizzle ) + pow( 1.0 - NdotL, 5.0 ) * @insertpiece( F0 ).@insertpiece( FresnelSwizzle )@end
+@piece( getSpecularFresnelWithRoughness )pixel.f0.@insertpiece( FresnelSwizzle ) + pow( 1.0 - VdotH, 5.0 ) * (@insertpiece( maxR1F0 ) - pixel.f0.@insertpiece( FresnelSwizzle ))@end
+@piece( getDiffuseFresnelWithRoughness )@insertpiece( maxR1F0 ) - pixel.f0.@insertpiece( FresnelSwizzle ) + pow( 1.0 - NdotL, 5.0 ) * pixel.f0.@insertpiece( FresnelSwizzle )@end
 
 @property( !fresnel_scalar )
 	@piece( getMaxFresnelS )fresnelS@end
@@ -36,7 +25,7 @@
 @piece( DeclareBRDF )
 //Blinn-Phong
 float3 BRDF( float3 lightDir, float3 viewDir, float NdotV, float3 lightDiffuse,
-			 float3 lightSpecular, Material material, float3 nNormal @insertpiece( brdfExtraParamDefs ) )
+			 float3 lightSpecular, Material material, PixelParams pixel, float3 nNormal @insertpiece( brdfExtraParamDefs ) )
 {
 	float3 halfWay= normalize( lightDir + viewDir );
 	float NdotL = saturate( dot( nNormal, lightDir ) );			//Diffuse (Lambert)
@@ -86,7 +75,7 @@ float3 BRDF( float3 lightDir, float3 viewDir, float NdotV, float3 lightDiffuse,
 @property( BRDF_CookTorrance )
 @piece( DeclareBRDF )
 //Cook-Torrance
-float3 BRDF( float3 lightDir, float3 viewDir, float NdotV, float3 lightDiffuse, float3 lightSpecular, Material material, float3 nNormal @insertpiece( brdfExtraParamDefs ) )
+float3 BRDF( float3 lightDir, float3 viewDir, float NdotV, float3 lightDiffuse, float3 lightSpecular, Material material, PixelParams pixel, float3 nNormal @insertpiece( brdfExtraParamDefs ) )
 {
 	float3 halfWay= normalize( lightDir + viewDir );
 	float NdotL = saturate( dot( nNormal, lightDir ) );
@@ -136,7 +125,7 @@ float3 BRDF( float3 lightDir, float3 viewDir, float NdotV, float3 lightDiffuse,
 @property( BRDF_Default )
 @piece( DeclareBRDF )
 //Default BRDF
-float3 BRDF( float3 lightDir, float3 viewDir, float NdotV, float3 lightDiffuse, float3 lightSpecular, Material material, float3 nNormal @insertpiece( brdfExtraParamDefs ) )
+float3 BRDF( float3 lightDir, float3 viewDir, float NdotV, float3 lightDiffuse, float3 lightSpecular, Material material, PixelParams pixel, float3 nNormal @insertpiece( brdfExtraParamDefs ) )
 {
 	float3 halfWay= normalize( lightDir + viewDir );
 	float NdotL = saturate( dot( nNormal, lightDir ) );
diff --git a/ogre2.1/Hlms/Pbs/HLSL/Forward3D_piece_ps.hlsl b/ogre2.1/Hlms/Pbs/HLSL/Forward3D_piece_ps.hlsl
index 78c8458..a9d9a80 100644
--- a/ogre2.1/Hlms/Pbs/HLSL/Forward3D_piece_ps.hlsl
+++ b/ogre2.1/Hlms/Pbs/HLSL/Forward3D_piece_ps.hlsl
@@ -111,7 +111,7 @@
 			}
 
 			//Point light
-			float3 tmpColour = BRDF( lightDir, viewDir, NdotV, lightDiffuse.xyz, lightSpecular, material, nNormal @insertpiece( brdfExtraParams ) );
+			float3 tmpColour = BRDF( lightDir, viewDir, NdotV, lightDiffuse.xyz, lightSpecular, material, pixel, nNormal @insertpiece( brdfExtraParams ) );
 			finalColour += tmpColour * atten;
 		}
 	}
@@ -164,7 +164,7 @@
 
 			if( spotCosAngle >= spotParams.y )
 			{
-				float3 tmpColour = BRDF( lightDir, viewDir, NdotV, lightDiffuse.xyz, lightSpecular, material, nNormal @insertpiece( brdfExtraParams ) );
+				float3 tmpColour = BRDF( lightDir, viewDir, NdotV, lightDiffuse.xyz, lightSpecular, material, pixel, nNormal @insertpiece( brdfExtraParams ) );
 				finalColour += tmpColour * atten;
 			}
 		}
diff --git a/ogre2.1/Hlms/Pbs/HLSL/PixelShader_ps.hlsl b/ogre2.1/Hlms/Pbs/HLSL/PixelShader_ps.hlsl
index 186760f..e42ba6f 100644
--- a/ogre2.1/Hlms/Pbs/HLSL/PixelShader_ps.hlsl
+++ b/ogre2.1/Hlms/Pbs/HLSL/PixelShader_ps.hlsl
@@ -7,6 +7,7 @@
 		@insertpiece( PassDecl )
 	@end
 	@insertpiece( MaterialDecl )
+	@insertpiece( PixelParamsDecl )
 	@insertpiece( InstanceDecl )
 	@insertpiece( PccManualProbeDecl )
 @end
@@ -135,6 +136,7 @@ float3 qmul( float4 q, float3 v )
 	@insertpiece( custom_ps_preExecution )
 
 	Material material;
+	PixelParams pixel;
 
 @property( diffuse_map )	uint diffuseIdx;@end
 @property( normal_map_tex )	uint normalIdx;@end
@@ -150,7 +152,6 @@ float3 qmul( float4 q, float3 v )
 
 float4 diffuseCol;
 @property( specular_map && !metallic_workflow && !fresnel_workflow )float3 specularCol;@end
-@property( metallic_workflow || (specular_map && fresnel_workflow) )@insertpiece( FresnelType ) F0;@end
 @property( roughness_map )float ROUGHNESS;@end
 
 @property( hlms_normal || hlms_qtangent )	float3 nNormal;@end
@@ -345,17 +346,29 @@ float4 diffuseCol;
 
 	@insertpiece( custom_ps_preLights )
 
+// moved from BRDFs_piece_ps.hlsl
+@property( !metallic_workflow && (!specular_map || !fresnel_workflow) )
+	@property( !transparent_mode )
+		pixel.f0 = material.F0;
+	@end @property( transparent_mode )
+		//Premultiply F0.xyz with the alpha from the texture, but only in transparent mode.
+		pixel.f0 = (material.F0.@insertpiece( FresnelSwizzle ) * diffuseCol.w;
+	@end
+@end @property( metallic_workflow || (specular_map && fresnel_workflow) )
+	// (already calculated)
+@end
+
 @property( !custom_disable_directional_lights )
 @property( hlms_lights_directional )
 	@insertpiece( ObjLightMaskCmp )
-		finalColour += BRDF( passBuf.lights[0].position.xyz, viewDir, NdotV, passBuf.lights[0].diffuse, passBuf.lights[0].specular, material, nNormal @insertpiece( brdfExtraParams ) ) @insertpiece( DarkenWithShadowFirstLight );
+		finalColour += BRDF( passBuf.lights[0].position.xyz, viewDir, NdotV, passBuf.lights[0].diffuse, passBuf.lights[0].specular, material, pixel, nNormal @insertpiece( brdfExtraParams ) ) @insertpiece( DarkenWithShadowFirstLight );
 @end
 @foreach( hlms_lights_directional, n, 1 )
 	@insertpiece( ObjLightMaskCmp )
-		finalColour += BRDF( passBuf.lights[@n].position.xyz, viewDir, NdotV, passBuf.lights[@n].diffuse, passBuf.lights[@n].specular, material, nNormal @insertpiece( brdfExtraParams ) )@insertpiece( DarkenWithShadow );@end
+		finalColour += BRDF( passBuf.lights[@n].position.xyz, viewDir, NdotV, passBuf.lights[@n].diffuse, passBuf.lights[@n].specular, material, pixel, nNormal @insertpiece( brdfExtraParams ) )@insertpiece( DarkenWithShadow );@end
 @foreach( hlms_lights_directional_non_caster, n, hlms_lights_directional )
 	@insertpiece( ObjLightMaskCmp )
-		finalColour += BRDF( passBuf.lights[@n].position.xyz, viewDir, NdotV, passBuf.lights[@n].diffuse, passBuf.lights[@n].specular, material, nNormal @insertpiece( brdfExtraParams ) );@end
+		finalColour += BRDF( passBuf.lights[@n].position.xyz, viewDir, NdotV, passBuf.lights[@n].diffuse, passBuf.lights[@n].specular, material, pixel, nNormal @insertpiece( brdfExtraParams ) );@end
 @end
 
 @property( hlms_lights_point || hlms_lights_spot || hlms_lights_area_approx )	float3 lightDir;
@@ -370,7 +383,7 @@ float4 diffuseCol;
 	if( fDistance <= passBuf.lights[@n].attenuation.x @insertpiece( andObjLightMaskCmp ) )
 	{
 		lightDir *= 1.0 / fDistance;
-		tmpColour = BRDF( lightDir, viewDir, NdotV, passBuf.lights[@n].diffuse, passBuf.lights[@n].specular, material, nNormal @insertpiece( brdfExtraParams ) )@insertpiece( DarkenWithShadowPoint );
+		tmpColour = BRDF( lightDir, viewDir, NdotV, passBuf.lights[@n].diffuse, passBuf.lights[@n].specular, material, pixel, nNormal @insertpiece( brdfExtraParams ) )@insertpiece( DarkenWithShadowPoint );
 		float atten = 1.0 / (0.5 + (passBuf.lights[@n].attenuation.y + passBuf.lights[@n].attenuation.z * fDistance) * fDistance );
 		finalColour += tmpColour * atten;
 	}@end
@@ -395,7 +408,7 @@ float4 diffuseCol;
 		float spotAtten = saturate( (spotCosAngle - passBuf.lights[@n].spotParams.y) * passBuf.lights[@n].spotParams.x );
 		spotAtten = pow( spotAtten, passBuf.lights[@n].spotParams.z );
 	@end
-		tmpColour = BRDF( lightDir, viewDir, NdotV, passBuf.lights[@n].diffuse, passBuf.lights[@n].specular, material, nNormal @insertpiece( brdfExtraParams ) )@insertpiece( DarkenWithShadow );
+		tmpColour = BRDF( lightDir, viewDir, NdotV, passBuf.lights[@n].diffuse, passBuf.lights[@n].specular, material, pixel, nNormal @insertpiece( brdfExtraParams ) )@insertpiece( DarkenWithShadow );
 		float atten = 1.0 / (0.5 + (passBuf.lights[@n].attenuation.y + passBuf.lights[@n].attenuation.z * fDistance) * fDistance );
 		finalColour += tmpColour * (atten * spotAtten);
 	}@end
diff --git a/ogre2.1/Hlms/Pbs/HLSL/Structs_piece_vs_piece_ps.hlsl b/ogre2.1/Hlms/Pbs/HLSL/Structs_piece_vs_piece_ps.hlsl
index 50296f1..ba2cab1 100644
--- a/ogre2.1/Hlms/Pbs/HLSL/Structs_piece_vs_piece_ps.hlsl
+++ b/ogre2.1/Hlms/Pbs/HLSL/Structs_piece_vs_piece_ps.hlsl
@@ -167,6 +167,18 @@ cbuffer MaterialBuf : register(b1)
 };
 @end
 
+@piece( PixelParamsDecl )
+// parameters for pixel shading
+struct PixelParams
+{
+	float3 f0;
+@property( pbs_extra_params )
+    float clearCoat;
+    float clearCoatRoughness;
+    float clearCoatLinearRoughness;
+@end
+};
+@end
 
 @piece( InstanceDecl )
 //Uniforms that change per Item/Entity
diff --git a/ogre2.1/Hlms/Pbs/HLSL/Textures_piece_ps.hlsl b/ogre2.1/Hlms/Pbs/HLSL/Textures_piece_ps.hlsl
index 132c598..ca2cec1 100644
--- a/ogre2.1/Hlms/Pbs/HLSL/Textures_piece_ps.hlsl
+++ b/ogre2.1/Hlms/Pbs/HLSL/Textures_piece_ps.hlsl
@@ -106,12 +106,10 @@
 		@piece( kS )specularCol@end
 	@end
 	@property( specular_map && fresnel_workflow )
-		@piece( SampleSpecularMap )	F0 = textureMaps[@value( specular_map_idx )].Sample(
+		@piece( SampleSpecularMap )	pixel.f0 = textureMaps[@value( specular_map_idx )].Sample(
 													samplerState@value(specular_map_idx),
 													float3( UV_SPECULAR( inPs.uv@value(uv_specular).xy ),
 															specularIdx) ).@insertpiece( FresnelSwizzle ) * material.F0.@insertpiece( FresnelSwizzle );@end
-		@piece( specularExtraParamDef ), @insertpiece( FresnelType ) F0@end
-		@piece( specularExtraParam ), F0@end
 	@end
 	@property( !specular_map || fresnel_workflow )
 		@piece( kS )material.kS@end
@@ -123,19 +121,17 @@
 													samplerState@value(specular_map_idx),
 													float3( UV_SPECULAR( inPs.uv@value(uv_specular).xy ),
 															specularIdx) ).x * material.F0.x;
-		F0 = lerp( float( 0.03f ).xxx, @insertpiece( kD ).xyz * 3.14159f, metalness );
+		pixel.f0 = lerp( float( 0.03f ).xxx, @insertpiece( kD ).xyz * 3.14159f, metalness );
 		@insertpiece( kD ).xyz = @insertpiece( kD ).xyz - @insertpiece( kD ).xyz * metalness;
 	@end @property( !specular_map )
-		F0 = lerp( float( 0.03f ).xxx, @insertpiece( kD ).xyz * 3.14159f, material.F0.x );
+		pixel.f0 = lerp( float( 0.03f ).xxx, @insertpiece( kD ).xyz * 3.14159f, material.F0.x );
 		@insertpiece( kD ).xyz = @insertpiece( kD ).xyz - @insertpiece( kD ).xyz * material.F0.x;
 	@end
-	@property( hlms_alphablend )F0 *= material.F0.w;@end
-	@property( transparent_mode )F0 *= diffuseCol.w;@end
+	@property( hlms_alphablend )pixel.f0 *= material.F0.w;@end
+	@property( transparent_mode )pixel.f0 *= diffuseCol.w;@end
 @end /// SampleSpecularMap
 
 	@piece( kS )material.kS.xyz@end
-	@piece( metallicExtraParamDef ), float3 F0@end
-	@piece( metallicExtraParam ), F0@end
 @end
 @end
The main changes are:
1. declare a "PixelParams" struct, and a local variable just after the local variable "Material material".
2. all non-trivial BRDFs that takes a Material struct parameter, now has a PixelParams struct parameter following it.
3. @piece( F0 ) is moved to pixel shader
4. the use of @insertpiece( F0 ) is changed to use pixel.f0 directly
5. In texture sampling code, changes to the global variable F0 is now to pixel.f0
For HLSL/Metal, since we're already using a struct local variable, adding another one seems ok :)
And the use of PixelParams removes a global variable F0 which may or may not be passed to BRDFs according to several properties. If we continue the refactor and place diffuseColor, ROUGHNESS and other things into PixelParams, we will have even less global variables and no BrdfExtraParams! I think it will make the code much clearer.

I know OpenGL will still be a mess, but...even grouping these variables together (with "pixel_" prefix?) should increase the readability.

Yeah, I'd like to clean it up, but I prefer the "preprocess the parameters" approach (something like a "preprocessShadingParameters" function) instead of providing a getF0() function. Is that ok?
In theory sounds good, I would have to see some code to judge better.
hmmm... finally I didn't take that route. One small step at a time :) Now I only moved @piece( F0 ) code to pixel shader's main(), and all @insertpiece( F0 ) is using pixel.f0 directly.
Ogre 1.x has the Visual Unit Testing Framework. We never ported it to Ogre 2.1 though. It generates HTML output with visual comparison.
Wow, that is very nice! When I finish clearcoat I would like to use it. Is it easy to port it to Ogre 2.1? It would be great if the community can help.
1 x

User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 3844
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 150
Contact:

Re: [2.1+] Implementing Clear Coat material

Post by dark_sylinc » Tue Aug 21, 2018 3:29 am

I have yet to review the patch (is it available on a public Bitbucket repo fork?)
rujialiu wrote:
Mon Aug 20, 2018 6:58 am
I know OpenGL will still be a mess, but...even grouping these variables together (with "pixel_" prefix?) should increase the readability.
That is... actually a great idea!

Instead of writing pixel.variable we write pixel_variable. And struct params can be passed via macros or insertpiece blocks:

Code: Select all

#define PixelParamDecl float3 pixel_nNormal, float pixel_F0, etc...
#define pixelParamDecl //Nothing, just to make syntax highlighting happy
#define pixelParamArg pixel_nNormal, pixel_F0, etc...

void myFunc( PixelParamDecl pixelParamDecl )
{
    //Use pixel_F0 pixel_nNormal etc
}

myFunc( pixelParamArg );
That way readability improves as well as maintenance, keeps us everybody sane; and avoids most bugs.

If you want to use real structs on HLSL & Metal, we can still be cross-compatible with GLSL:

Code: Select all

#define PixelParamDecl PixelParams
#define pixelParamDecl pixelParams
#define pixelParamArg pixelParams

#define pixel_nNormal pixelParamDecl.nNormal
#define pixel_F0 pixelParamDecl.F0

//This translates to: myFunc( PixelParams pixelParams )
void myFunc( PixelParamDecl pixelParamDecl )
{
//Use of pixel_nNormal translates to pixelParamDecl.nNormal
}

myFunc( pixelParamArg ); //Same as myFunc( pixelParams );
I like your idea... a lot!

That's all for now. This is going to be a very busy week for me so I don't know when I'll review the pasted patch.
If I see it on Bitbucket or something like that (i.e. not in patch form) I may be able to take a look it my tablet while drinking coffee.

Cheers
Matias
2 x

rujialiu
Greenskin
Posts: 141
Joined: Mon May 09, 2016 8:21 am
x 11

Re: [2.1+] Implementing Clear Coat material

Post by rujialiu » Tue Aug 21, 2018 11:35 am

dark_sylinc wrote:
Tue Aug 21, 2018 3:29 am
I have yet to review the patch (is it available on a public Bitbucket repo fork?)
Not yet...
That is... actually a great idea!

Instead of writing pixel.variable we write pixel_variable. And struct params can be passed via macros or insertpiece blocks:

Code: Select all

#define PixelParamDecl float3 pixel_nNormal, float pixel_F0, etc...
#define pixelParamDecl //Nothing, just to make syntax highlighting happy
#define pixelParamArg pixel_nNormal, pixel_F0, etc...

void myFunc( PixelParamDecl pixelParamDecl )
{
    //Use pixel_F0 pixel_nNormal etc
}

myFunc( pixelParamArg );
That way readability improves as well as maintenance, keeps us everybody sane; and avoids most bugs.
Yeah! That was what I thought, but suddenly I realized that you're already using a global struct variable "Material material" in GLSL :-S You just didn't pass it into functions. So if struct sometimes doesn't work, "Material material" already breaks; if it works, we can also add another global variable "PixelParams pixel" near "Material material" and it won't be worse than before???
That's all for now. This is going to be a very busy week for me so I don't know when I'll review the pasted patch.
If I see it on Bitbucket or something like that (i.e. not in patch form) I may be able to take a look it my tablet while drinking coffee.
Never mind, it's not urgent. You can review it when it's more polished.

Now that I've implemented a version of clear coat, which looks good. I'm not sure whether my IBL implementation is actually correct, though.

Here is the patch for clear-coat direct lighting:

Code: Select all

 ogre2.1/Hlms/Pbs/HLSL/BRDFs_piece_ps.hlsl          | 73 +++++++++++++++++++++-
 ogre2.1/Hlms/Pbs/HLSL/PixelShader_ps.hlsl          | 26 ++++++--
 .../Hlms/Pbs/HLSL/Structs_piece_vs_piece_ps.hlsl   |  1 +
 3 files changed, 95 insertions(+), 5 deletions(-)

diff --git a/ogre2.1/Hlms/Pbs/HLSL/BRDFs_piece_ps.hlsl b/ogre2.1/Hlms/Pbs/HLSL/BRDFs_piece_ps.hlsl
index 1b4e5fb..fbc51bc 100644
--- a/ogre2.1/Hlms/Pbs/HLSL/BRDFs_piece_ps.hlsl
+++ b/ogre2.1/Hlms/Pbs/HLSL/BRDFs_piece_ps.hlsl
@@ -123,7 +123,63 @@ float3 BRDF( float3 lightDir, float3 viewDir, float NdotV, float3 lightDiffuse,
 @end
 
 @property( BRDF_Default )
+
 @piece( DeclareBRDF )
+
+@property( pbs_extra_params )
+
+float D_GGX(float linearRoughness, float NoH, float3 h) {
+    float oneMinusNoHSquared = 1.0 - NoH * NoH;
+    float a = NoH * linearRoughness;
+    float k = linearRoughness / (oneMinusNoHSquared + a * a);
+    float d = k * k * (1.0 / 3.141592654);
+    return d;
+}
+
+float distributionClearCoat(float linearRoughness, float NoH, const float3 h) {
+    return D_GGX(linearRoughness, NoH, h);
+}
+
+float V_Kelemen(float LoH) {
+    // Kelemen 2001, "A Microfacet Based Coupled Specular-Matte BRDF Model with Importance Sampling"
+    return 0.25 / (LoH * LoH);
+}
+
+float visibilityClearCoat(float roughness, float linearRoughness, float LoH) {
+    return V_Kelemen(LoH);
+}
+
+float pow5(float x) {
+    float x2 = x * x;
+    return x2 * x2 * x;
+}
+
+float F_Schlick(float f0, float f90, float VoH) {
+    return f0 + (f90 - f0) * pow5(1.0 - VoH);
+}
+
+float2 clearCoatLobe(PixelParams pixel, float3 h, float NoH, float LoH) {
+@property( normal_map )
+    // If the material has a normal map, we want to use the geometric normal
+    // instead to avoid applying the normal map details to the clear coat layer
+    float clearCoatNoH = saturate(dot(pixel.geomNormal, h));
+@end @property( !normal_map )
+    float clearCoatNoH = NoH;
+@end
+    // clear coat specular lobe
+    float D = distributionClearCoat(pixel.clearCoatLinearRoughness, clearCoatNoH, h);
+    float V = visibilityClearCoat(pixel.clearCoatRoughness, pixel.clearCoatLinearRoughness, LoH);
+    float F = F_Schlick(0.04, 1.0, LoH) * pixel.clearCoat; // fix IOR to 1.5
+    return float2(D * V * F, F); // x: clearCoat, y: Fc
+}
+
+float3 f0ClearCoatToSurface(float3 f0) {
+    // Approximation of iorTof0(f0ToIor(f0), 1.5)
+    // This assumes that the clear coat layer has an IOR of 1.5
+    return saturate(f0 * (f0 * (0.941892 - 0.263008 * f0) + 0.346479) - 0.0285998);
+}
+@end
+
 //Default BRDF
 float3 BRDF( float3 lightDir, float3 viewDir, float NdotV, float3 lightDiffuse, float3 lightSpecular, Material material, PixelParams pixel, float3 nNormal @insertpiece( brdfExtraParamDefs ) )
 {
@@ -131,6 +187,7 @@ float3 BRDF( float3 lightDir, float3 viewDir, float NdotV, float3 lightDiffuse,
 	float NdotL = saturate( dot( nNormal, lightDir ) );
 	float NdotH = saturate( dot( nNormal, halfWay ) );
 	float VdotH = saturate( dot( viewDir, halfWay ) );
+    float LdotH = saturate( dot( lightDir, halfWay ) );
 
 	float sqR = ROUGHNESS * ROUGHNESS;
 
@@ -176,7 +233,21 @@ float3 BRDF( float3 lightDir, float3 viewDir, float NdotV, float3 lightDiffuse,
 	//We should divide Rd by PI, but it is already included in kD
 	float3 Rd = (lightScatter * viewScatter * energyFactor * fresnelD) * @insertpiece( kD ).xyz * lightDiffuse;
 
-	return NdotL * (Rs + Rd);
+@property( pbs_extra_params )
+    float2 ret = clearCoatLobe(pixel, halfWay, NdotH, LdotH);
+	float clearCoat = ret.x;
+	float Fc = ret.y;
+    // Energy compensation and absorption; the clear coat Fresnel term is
+    // squared to take into account both entering through and exiting through
+    // the clear coat layer
+    float attenuation = 1.0 - Fc;
+    float3 color = (Rd + Rs * attenuation) * attenuation + clearCoat;
+@end @property( !pbs_extra_params )
+    // The energy compensation term is used to counteract the darkening effect
+    // at high roughness
+    float3 color = Rd + Rs;
+@end
+	return NdotL * color;
 }
 @end
 @end
diff --git a/ogre2.1/Hlms/Pbs/HLSL/PixelShader_ps.hlsl b/ogre2.1/Hlms/Pbs/HLSL/PixelShader_ps.hlsl
index e42ba6f..f7bff41 100644
--- a/ogre2.1/Hlms/Pbs/HLSL/PixelShader_ps.hlsl
+++ b/ogre2.1/Hlms/Pbs/HLSL/PixelShader_ps.hlsl
@@ -236,12 +236,12 @@ float4 diffuseCol;
 		nNormal = normalize( inPs.normal ) @insertpiece( two_sided_flip_normal );
 	@end @property( normal_map )
 		//Normal mapping.
-		float3 geomNormal = normalize( inPs.normal ) @insertpiece( two_sided_flip_normal );
+		pixel.geomNormal = normalize( inPs.normal ) @insertpiece( two_sided_flip_normal );
 		float3 vTangent = normalize( inPs.tangent );
 
 		//Get the TBN matrix
-		float3 vBinormal	= normalize( cross( geomNormal, vTangent )@insertpiece( tbnApplyReflection ) );
-		float3x3 TBN		= float3x3( vTangent, vBinormal, geomNormal );
+		float3 vBinormal	= normalize( cross( pixel.geomNormal, vTangent )@insertpiece( tbnApplyReflection ) );
+		float3x3 TBN		= float3x3( vTangent, vBinormal, pixel.geomNormal );
 
 		@property( normal_map_tex )nNormal = getTSNormal( float3( UV_NORMAL( inPs.uv@value(uv_normal).xy ),
 																  normalIdx ) );@end
@@ -346,18 +346,36 @@ float4 diffuseCol;
 
 	@insertpiece( custom_ps_preLights )
 
+// compute pixel parameters
+
 // moved from BRDFs_piece_ps.hlsl
 @property( !metallic_workflow && (!specular_map || !fresnel_workflow) )
 	@property( !transparent_mode )
 		pixel.f0 = material.F0;
 	@end @property( transparent_mode )
 		//Premultiply F0.xyz with the alpha from the texture, but only in transparent mode.
-		pixel.f0 = (material.F0.@insertpiece( FresnelSwizzle ) * diffuseCol.w;
+		pixel.f0 = material.F0.@insertpiece( FresnelSwizzle ) * diffuseCol.w;
 	@end
 @end @property( metallic_workflow || (specular_map && fresnel_workflow) )
 	// (already calculated)
 @end
 
+@property( pbs_extra_params )
+    pixel.clearCoat = material.rawClearCoat;
+    // Clamp the clear coat roughness to avoid divisions by 0
+    float clearCoatRoughness = material.rawClearCoatRoughness;
+    clearCoatRoughness = lerp(0.045, 0.6, clearCoatRoughness);
+    // Remap the roughness to perceptually linear roughness
+    pixel.clearCoatRoughness = clearCoatRoughness;
+    pixel.clearCoatLinearRoughness = clearCoatRoughness * clearCoatRoughness;
+    // The base layer's f0 is computed assuming an interface from air to an IOR
+    // of 1.5, but the clear coat layer forms an interface from IOR 1.5 to IOR
+    // 1.5. We recompute f0 by first computing its IOR, then reconverting to f0
+    // by using the correct interface
+    pixel.f0 = lerp(pixel.f0, f0ClearCoatToSurface(pixel.f0), pixel.clearCoat);
+    // TODO: the base layer should be rougher
+@end
+
 @property( !custom_disable_directional_lights )
 @property( hlms_lights_directional )
 	@insertpiece( ObjLightMaskCmp )
diff --git a/ogre2.1/Hlms/Pbs/HLSL/Structs_piece_vs_piece_ps.hlsl b/ogre2.1/Hlms/Pbs/HLSL/Structs_piece_vs_piece_ps.hlsl
index ba2cab1..8ed255a 100644
--- a/ogre2.1/Hlms/Pbs/HLSL/Structs_piece_vs_piece_ps.hlsl
+++ b/ogre2.1/Hlms/Pbs/HLSL/Structs_piece_vs_piece_ps.hlsl
@@ -171,6 +171,7 @@ cbuffer MaterialBuf : register(b1)
 // parameters for pixel shading
 struct PixelParams
 {
+	float3 geomNormal;
 	float3 f0;
 @property( pbs_extra_params )
     float clearCoat;
comments:
0. I fixed a small bug in previous patch (an extra left parathesis when in transparent_mode)
1. I moved geomNormal to PixelParams, otherwise it's difficult to read it from auxliary functions (have to add a parameter to quite a few functions)
2. There are duplicated codes in BRDF functions (e.g. F_Schlick). However, the current HLMS code inlined all auxiliary functions, so I had to copy all auxiliary functions from Filament.
3. I don't know whether there are out/inout function parameteres in HLSL, so I returned a float2 from clearCoatLobe, while in filament's code Fc is an out parameter.
4. In our Material struct, we have two additional parameters: rawClearCoat and rawClearCoatRoughness

So far so good. But after that, implementing IBL took me longer than expected. Here are the changes:

Code: Select all

 ogre2.1/Hlms/Pbs/HLSL/BRDFs_piece_ps.hlsl | 38 +++++++++++++++++++++++++++++--
 ogre2.1/Hlms/Pbs/HLSL/PixelShader_ps.hlsl |  2 ++
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/ogre2.1/Hlms/Pbs/HLSL/BRDFs_piece_ps.hlsl b/ogre2.1/Hlms/Pbs/HLSL/BRDFs_piece_ps.hlsl
index fbc51bc..9d9296c 100644
--- a/ogre2.1/Hlms/Pbs/HLSL/BRDFs_piece_ps.hlsl
+++ b/ogre2.1/Hlms/Pbs/HLSL/BRDFs_piece_ps.hlsl
@@ -149,6 +149,10 @@ float visibilityClearCoat(float roughness, float linearRoughness, float LoH) {
     return V_Kelemen(LoH);
 }
 
+float sq(float x) {
+    return x * x;
+}
+
 float pow5(float x) {
     float x2 = x * x;
     return x2 * x2 * x;
@@ -287,8 +291,38 @@ float3 BRDF_IR( float3 lightDir, float3 lightDiffuse,
 	@end @property( !fresnel_separate_diffuse )
 		float fresnelD = 1.0f - @insertpiece( getMaxFresnelS );@end
 
-	finalColour += envColourD * @insertpiece( kD ).xyz * fresnelD +
-					envColourS * @insertpiece( kS ).xyz * fresnelS;
+	float3 Fr = envColourS * @insertpiece( kS ).xyz * fresnelS;
+	float3 Fd = envColourD * @insertpiece( kD ).xyz * fresnelD;
+	@property( pbs_extra_params )
+		if(use_pcc > 0) {
+			// adapted from filament's evaluateClearCoatIBL
+		@property( normal_map )
+    		// We want to use the geometric normal for the clear coat layer
+			float3 normal = pixel.geomNormal;
+		@end @property( !normal_map )
+			float3 normal = nNormal;
+		@end
+			float3 reflDir = 2.0 * dot( viewDir, normal ) * normal - viewDir;
+    		float clearCoatNoV = saturate( dot( normal, viewDir ) );
+    		float3 clearCoatR = localCorrect( reflDir, posInProbSpace, @insertpiece( pccProbeSource ) );
+
+    		// The clear coat layer assumes an IOR of 1.5 (4% reflectance)
+    		float Fc = F_Schlick(0.04, 1.0, clearCoatNoV) * pixel.clearCoat;
+    		float attenuation = 1.0 - Fc;
+			float specularAO = 1.0;
+    		Fr *= sq(attenuation);
+			float specularIrradiance = texEnvProbeMap.SampleLevel( envMapSamplerState,
+														 clearCoatR, pixel.clearCoatRoughness * 12.0 ).xyz @insertpiece( ApplyEnvMapScale ) * material.IBLSpecularPowerScale;
+    		Fr += specularIrradiance * (specularAO * Fc);
+    		Fd *= attenuation;
+
+			// should probeFade be applied here?
+			Fr = Fr * saturate( probeFade * 200.0 );
+			Fd = Fd * saturate( probeFade * 200.0 );
+		}
+	@end
+
+	finalColour += Fd + Fr;
 @end
 
 @property( hlms_fine_light_mask )
diff --git a/ogre2.1/Hlms/Pbs/HLSL/PixelShader_ps.hlsl b/ogre2.1/Hlms/Pbs/HLSL/PixelShader_ps.hlsl
index f7bff41..abfc852 100644
--- a/ogre2.1/Hlms/Pbs/HLSL/PixelShader_ps.hlsl
+++ b/ogre2.1/Hlms/Pbs/HLSL/PixelShader_ps.hlsl
@@ -443,10 +443,12 @@ float4 diffuseCol;
 @end
 
 @property( use_envprobe_map || hlms_use_ssr || use_planar_reflections || ambient_hemisphere )
+	float use_pcc = -1.0; // we cannot rely on use_parallax_correct_cubemaps in BRDFs_piece_ps!
 	float3 reflDir = 2.0 * dot( viewDir, nNormal ) * nNormal - viewDir;
 
 	@property( use_envprobe_map )
 		@property( use_parallax_correct_cubemaps )
+			use_pcc = 1.0;
 			float3 envColourS;
 			float3 envColourD;
 			float3 posInProbSpace = toProbeLocalSpace( inPs.pos, @insertpiece( pccProbeSource ) );

At first, I tried to add IBL code directly after PCC's envColourS's computation, but later I found that we should attenuate the final color of PCC (multipled by fresnel) rather than envColourS, so I moved the code to BRDF_EnvMap.
However, use_parallax_correct_cubemaps is set in Textures_piece_ps.hlsl, which is parsed AFTER BRDF_piece_ps.hlsl because 'B' < 'T', so we can't use property "use_parallax_correct_cubemaps" in BRDF_EnvMap's piece definition. That's why I used a float use_pcc variable.

There is one thing that made me confused: the current version used geomNormal when we have normal maps, but it is slightly (but definitely notice-able) different from single-layer material's PCC reflection (I compared the render of full-specular-no-clearcoat and no-specular-full-clearcoat). However, if I changed geomNormal to nNormal, the render matched almost perfectly. The normal map I tested is mostly flat, so I expected changing from geomNormal to nNormal should not changed too much.

It would be great if anyone can review/test my codes above. Though more expensive, I think clearcoat can make many materials more realistic.
1 x

al2950
OGRE Expert User
OGRE Expert User
Posts: 1149
Joined: Thu Dec 11, 2008 7:56 pm
Location: Bristol, UK
x 59

Re: [2.1+] Implementing Clear Coat material

Post by al2950 » Tue Aug 21, 2018 3:53 pm

I created a PR with your changes so your code can be easily discussed and shared :D. (I used your username for your patches, I hope thats ok). I have only made sure it runs, so have not done any serious comparison. Your patches did not include any C++ code, do you have any? Happy to add the relevant C++ code, and make some updates to PBS Material sample to show them off it you want.?

The only thing I have to say at this moment is 'pbs_extra_params' should probably be renamed to 'clear_coat_params'
1 x

rujialiu
Greenskin
Posts: 141
Joined: Mon May 09, 2016 8:21 am
x 11

Re: [2.1+] Implementing Clear Coat material

Post by rujialiu » Tue Aug 21, 2018 4:05 pm

al2950 wrote:
Tue Aug 21, 2018 3:53 pm
I created a PR with your changes so your code can be easily discussed and shared :D. (I used your username for your patches, I hope thats ok). I have only made sure it runs, so have not done any serious comparison. Your patches did not include any C++ code, do you have any? Happy to add the relevant C++ code, and make some updates to PBS Material sample to show them off it you want.?

The only thing I have to say at this moment is 'pbs_extra_params' should probably be renamed to 'clear_coat_params'
Wow, thank you!!! I don't have C++ codes now, it's in my office. It's quite trivial but I'll try to clean it up (it's mixed with other our own codes inside a HlmsPbs's subclass) and post here. The pbs_extra_param name is because.... I was planning to add anisotropy parameters, cloth shading model and subsurface shading model to it later (adapt from filament of course). It's more of an experimental stuff. I'm not good at polishing shaders :P
0 x

romainguy
Gnoblar
Posts: 2
Joined: Mon Aug 27, 2018 9:49 pm
x 2

Re: [2.1+] Implementing Clear Coat material

Post by romainguy » Mon Aug 27, 2018 9:51 pm

Hi, one of the authors of Filament here :)

If you want to make your life easier you don't have to modify f0. The modification is done in Filament because f0 is normally computed assuming an air/material interface, but when light goes through a clear coat layer the interface becomes varnish/material. It has the side effect of darkening the base layer and while it's technically more correct, it's also not entirely necessary (I don't think Unreal Engine 4 does this for instance).
1 x

romainguy
Gnoblar
Posts: 2
Joined: Mon Aug 27, 2018 9:49 pm
x 2

Re: [2.1+] Implementing Clear Coat material

Post by romainguy » Mon Aug 27, 2018 9:56 pm

Another quick note about clear coat: last week I merged in a change that allows a material to define a normal map for the clear coat layer, independent of the surface. This means we use the geometric normal when the surface has a normal map *and* the clear coat layer does _not_ have a normal map.
1 x

rujialiu
Greenskin
Posts: 141
Joined: Mon May 09, 2016 8:21 am
x 11

Re: [2.1+] Implementing Clear Coat material

Post by rujialiu » Tue Aug 28, 2018 6:40 am

romainguy wrote:
Mon Aug 27, 2018 9:51 pm
Hi, one of the authors of Filament here :)

If you want to make your life easier you don't have to modify f0. The modification is done in Filament because f0 is normally computed assuming an air/material interface, but when light goes through a clear coat layer the interface becomes varnish/material. It has the side effect of darkening the base layer and while it's technically more correct, it's also not entirely necessary (I don't think Unreal Engine 4 does this for instance).
Wow, nice to see you here! Looks like this is your first post on Ogre forum, so welcome to Ogre forum!!!
Thanks for your comment. I'm still learning PBR and wants to improve Ogre 2.1+'s PBR (especially IBL because GI is very important for us). Filament is a very good learning material for me. Thanks for the awesome project :)
0 x

User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 3844
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 150
Contact:

Re: [2.1+] Implementing Clear Coat material

Post by dark_sylinc » Sun Sep 23, 2018 8:31 pm

I've been thinking about a shader refactor, and namely about your proposals.

There are a few things in our shader codebase that need addressing:
1. Like 90% of the codebase is exactly the same or has minor differences. There is no reason to have near 3 copies of each shader for each platform.

2. I started Samples/Media/Hlms/Common/GLSL/CrossPlatformSettings_piece_all.glsl (and same for HLSL & Metal) to abstract the minor differences via macros.

3. @insertpiece( FresnelType ) and co. just clutter everything. This was done because I did not trust GLSL's macro preprocessor. But we use our own! Had I known that, I wouldn't have done this. This would be MUCH clearer:

Code: Select all

@property( !fresnel_scalar )
#define float_fresnel float
#define fresnelValue fresnel
#define fresnelValue fresnel.x
@end

@property( fresnel_scalar )
#define float_fresnel float3
#define fresnelValue fresnel.xyz

float_fresnel myTempComputation = fresnelValue + blah;
@end
4. GLSL uses global variables, HLSL uses some global variables, and Metal cannot use global variables. This means HLSL & Metal need to handle parameter passing while GLSL needs to deal with name clashing.
Parameter passing for everyone solves the discrepances. This brings us to our next point...

5. Using structs to handle per-pixel params.
Currently we have a few "loose" around the codebase such as:

Code: Select all

diffuseCol
geomNormal
nNormal
finalColour
And then a few "special" ones:

Code: Select all

specularCol
metalness
F0
First, it would be nice to have all of them together in one place.
Second, as for the use of structures and my concern about Android compatibility, I found out others (UE4, Roblox) are resorting to use SPIRV-Cross to generate "more compatible" GLSL. i.e. their pipeline is GLSL -> SPIRV -> SPIRV-Cross -> GLSL -> GLES 2/3 driver.

Oryol engine has a nice example of how to get SPIRV-Cross to spit out code.
So whenever Android comes into play, we'll dig into these solutions which, as crazy as they sound, they seem like the best solution (not to mention it is a joint effort).

Third, the "special" params are currently optimized to be accessed directly if they're not modified by the shader code. i.e. read directly from the UBO.
I was thinking "nah, the shader compiler will optimize this" then I read state of the art compiler may struggle with it.

If that's the case, then we may still need direct accessors as an optimization, and handle the values individually. My idea of using macros a few posts above may just work out.
But still, it would be nice if these variables are all together. And if direct access is controlled by a single property, and macros handle the rest, so that "normal code" doesn't become a @property()@end mess.
1 x

rujialiu
Greenskin
Posts: 141
Joined: Mon May 09, 2016 8:21 am
x 11

Re: [2.1+] Implementing Clear Coat material

Post by rujialiu » Mon Sep 24, 2018 6:54 am

dark_sylinc wrote:
Sun Sep 23, 2018 8:31 pm
I've been thinking about a shader refactor, and namely about your proposals.
Thanks!!!
First, it would be nice to have all of them together in one place.
Yeah, that is the ultimate goal :)
Second, as for the use of structures and my concern about Android compatibility, I found out others (UE4, Roblox) are resorting to use SPIRV-Cross to generate "more compatible" GLSL. i.e. their pipeline is GLSL -> SPIRV -> SPIRV-Cross -> GLSL -> GLES 2/3 driver.

Oryol engine has a nice example of how to get SPIRV-Cross to spit out code.
So whenever Android comes into play, we'll dig into these solutions which, as crazy as they sound, they seem like the best solution (not to mention it is a joint effort).
So, this is for GLSL only or also for HLSL/Metal? In Oryol's code above I found the following:

Code: Select all

void to_hlsl_sm5(const vector<uint32_t>& spirv, const string& out_path) {
...

Does that mean we can have HLSL SM 5 *bytecode* from spirv? If so, this can be a huge improvement for our use case because we have soooo many shader permutations that it's hard to pre-compile everything into the microcode cache.

It's a big problem for us because we can add/remove lights at runtime, for example we can have 1~3 shadow casting point lights, 0~1 directional lights, 0~5 fake area lights, 0~2 LTC area lights, that is a factor 3*2*6*3=108 on top of material combination! We also allow enabling/disabling certain features like PCC, Irradiance Volume, (hemisphere) ambient light etc, so the total number of possible combination is huge :-S

BUT, if we can compile shaders as GLSL->SPIRV->HLSL SM5 byte code, it'll be faster than compiling directly with D3D11 compiler. While it doesn't solve shader permutation problem, it will definitely reduce customer complaint 8-)


Edit: After reading Oryol's whole article, I know that they're producing HLSL SM5's source code, not bytecode. They still need Microsoft's HLSL compiler :(

Third, the "special" params are currently optimized to be accessed directly if they're not modified by the shader code. i.e. read directly from the UBO.
I was thinking "nah, the shader compiler will optimize this" then I read state of the art compiler may struggle with it.

If that's the case, then we may still need direct accessors as an optimization, and handle the values individually. My idea of using macros a few posts above may just work out.
But still, it would be nice if these variables are all together. And if direct access is controlled by a single property, and macros handle the rest, so that "normal code" doesn't become a @property()@end mess.
If we use glslang to compile GLSL to SPIRV, does this problem go away? If the answer to my previous question (can we get HLSL SM5 bytecode from SPIRV without invoking D3D11's compiler) is also "yes", everything boils down to "developing a working SPIRV-based shader pipeline"?
0 x

paroj
OGRE Team Member
OGRE Team Member
Posts: 680
Joined: Sun Mar 30, 2014 2:51 pm
x 100
Contact:

Re: [2.1+] Implementing Clear Coat material

Post by paroj » Mon Sep 24, 2018 10:46 am

SPIRV->HLSL SM5 byte code
there is a project for SPIRV > DXIL, but thats D3D12 only.

also note that I drafted an evolution proposal for SPIRV here:
https://github.com/OGRECave/evolution/b ... Support.md
0 x

rujialiu
Greenskin
Posts: 141
Joined: Mon May 09, 2016 8:21 am
x 11

Re: [2.1+] Implementing Clear Coat material

Post by rujialiu » Mon Sep 24, 2018 4:03 pm

paroj wrote:
Mon Sep 24, 2018 10:46 am
SPIRV->HLSL SM5 byte code
there is a project for SPIRV > DXIL, but thats D3D12 only.

also note that I drafted an evolution proposal for SPIRV here:
https://github.com/OGRECave/evolution/b ... Support.md
er... and that project is still in early age.... Also, when Ogre 2.x supports DX12, we can also use Microsoft's own open source shader compiler? That may give another pipeline: HLSL -> SPIRV -> GLSL/GLES/MSL.

BTW: If you implement your proposal for Ogre 1.x, would it be easy to port to Ogre 2.x? I'm not familiar with glslang but I do find a useful sentence in your proposal: "The Plugin will need to build glslang which is a beast" :D
0 x

Post Reply