HLMS PBR BRDF

Discussion area about developing with Ogre-Next (2.1, 2.2 and beyond)


rujialiu
Goblin
Posts: 296
Joined: Mon May 09, 2016 8:21 am
x 35

Re: HLMS PBR BRDF

Post by rujialiu »

dark_sylinc wrote: Sun Jan 12, 2020 11:41 pm Fixed! Thanks!
Thanks! Now D3D11 works fine. However, iOS is not working :(
1. RSC_UAV is also never set for iOS. Since my iPad can even do compute shader based voxelization, it should support UAV, right?
2. Shader compile error:

Code: Select all

Metal SL Compiler Error in 0SpecularIblIntegrator_cs:
Compilation failed:

program_source:331:31: error: no matching member function for call to 'read'
                                                        float4 lastResultVal = OGRE_imageLoad2D( lastResult, loadCoords.xy );
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
program_source:108:50: note: expanded from macro 'OGRE_imageLoad2D'
#define OGRE_imageLoad2D( inImage, iuv ) inImage.read( ushort2( iuv ) )
                                        ~~~~~~~~^~~~
/System/Library/PrivateFrameworks/GPUCompiler.framework/Libraries/lib/clang/902.9/include/metal/metal_texture:3071:24: note: candidate function not viable: requires at least 2 arguments, but 1 was provided
  METAL_FUNC vec<T, 4> read(ushort2 coord, ushort array, ushort lod = 0) const thread
                       ^
/System/Library/PrivateFrameworks/GPUCompiler.framework/Libraries/lib/clang/902.9/include/metal/metal_texture:3090:24: note: candidate function not viable: requires at least 2 arguments, but 1 was provided
  METAL_FUNC vec<T, 4> read(uint2 coord, uint array, uint lod = 0) const thread
                       ^
/System/Library/PrivateFrameworks/GPUCompiler.framework/Libraries/lib/clang/902.9/include/metal/metal_texture:3110:24: note: candidate function not viable: requires at least 2 arguments, but 1 was provided
  METAL_FUNC vec<T, 4> read(ushort2 coord, ushort array, ushort lod = 0) const device
                       ^
/System/Library/PrivateFrameworks/GPUCompiler.framework/Libraries/lib/clang/902.9/include/metal/metal_texture:3129:24: note: candidate function not viable: requires at least 2 arguments, but 1 was provided
  METAL_FUNC vec<T, 4> read(uint2 coord, uint array, uint lod = 0) const device
                       ^
/System/Library/PrivateFrameworks/GPUCompiler.framework/Libraries/lib/clang/902.9/include/metal/metal_texture:3150:24: note: candidate function not viable: requires at least 2 arguments, but 1 was provided
  METAL_FUNC vec<T, 4> read(ushort2 coord, ushort array, ushort lod = 0) const constant
                       ^
/System/Library/PrivateFrameworks/GPUCompiler.framework/Libraries/lib/clang/902.9/include/metal/metal_texture:3169:24: note: candidate function not viable: requires at least 2 arguments, but 1 was provided
  METAL_FUNC vec<T, 4> read(uint2 coord, uint array, uint lod = 0) const constant
                       ^
program_source:360:14: error: use of undeclared identifier 'mod'
                normal.x = mod( uv0.x, 0.5 ) * 4.0 - 1.0;
             ^
program_source:365:23: error: no matching function for call to 'runIntegrator'
        float4 outputValue = runIntegrator( normal, gl_GlobalInvocationID.xyz PARAMS_ARG );
                      ^~~~~~~~~~~~~
program_source:321:16: note: candidate function not viable: no known conversion from 'texture2d<float, access::read_write>' to 'texture2d_array<float, access::read_write>' for 6th argument
        INLINE float4 runIntegrator( float3 normal, ushort3 loadCoords PARAMS_ARG_DECL )
               ^
Error retriving entry point 'main_metal' in shader 0SpecularIblIntegrator_cs

But on the bright side, we're quite satisfied with the DPM version with D3D11! Cubemap array version also works, but the mipmaps are a bit too blurred (when src size is 512x512, I can hardly recognize any object in mip 1!). Considering the fact that we had to support older iPads that don't support cubemap arrays, we decided that we'll use DPM on both render systems.

Like we discussed, I've added a variable perceptualRoughness to PixelData struct because I'm using both roughness AND perceptualRoughness (see below). I haven't tested as carefully as xrgo, but I also did some experiments and believe this change is needed to match other engines.

Here is the diff that we're currently satisfied with:

Code: Select all

--- a/Hlms/Pbs/Any/Main/200.BRDFs_piece_ps.any
+++ b/Hlms/Pbs/Any/Main/200.BRDFs_piece_ps.any
@@ -217,7 +217,7 @@ float3 BRDF_IR( float3 lightDir, float3 lightDiffuse, PixelData pixelData )
     @property( ltc_texture_available )
         #define brdfLUT ltcMatrix
         float2 envBRDF = OGRE_SampleArray2D( brdfLUT, ltcSampler,
-                                             float2( pixelData.NdotV, 1.0 - pixelData.roughness ), 2 ).xy;
+                                             float2( pixelData.NdotV, 1.0 - pixelData.perceptualRoughness ), 2 ).xy;
     @else
         float2 envBRDF = float2( 1.0f, 0.0f );
     @end
diff --git a/Hlms/Pbs/Any/Main/800.PixelShader_piece_ps.any b/Hlms/Pbs/Any/Main/800.PixelShader_piece_ps.any
index 166fa79..aa57e7d 100644
--- a/Hlms/Pbs/Any/Main/800.PixelShader_piece_ps.any
+++ b/Hlms/Pbs/Any/Main/800.PixelShader_piece_ps.any
@@ -1,4 +1,4 @@
-@piece( envSpecularRoughness ) pixelData.roughness * passBuf.envMapNumMipmaps @end
+@piece( envSpecularRoughness ) pixelData.perceptualRoughness * passBuf.envMapNumMipmaps @end
 
 @piece( DefaultHeaderPS )
     @property( !fresnel_scalar )
@@ -34,6 +34,7 @@
             @end
             float4    diffuse;
             float3    specular;
+            float    perceptualRoughness;
             float    roughness;
             float_fresnel    F0;
 
@@ -300,15 +301,15 @@
 
 @piece( SampleRoughnessMap )
     /// ROUGHNESS MAP
-    pixelData.roughness = material.kS.w;
+    pixelData.perceptualRoughness = material.kS.w;
     @property( roughness_map )
-        pixelData.roughness = SampleRoughness( textureMaps@value( roughness_map_idx ),
+        pixelData.perceptualRoughness = SampleRoughness( textureMaps@value( roughness_map_idx ),
                                                 samplerState@value( roughness_map_sampler ),
                                                 UV_ROUGHNESS( inPs.uv@value(uv_roughness).xy ),
                                                 texIndex_roughnessIdx ).x;
     @end
     // convert from perceptual linear roughness to internal roughness
-    pixelData.roughness = max( 0.001f, pixelData.roughness * pixelData.roughness );
+    pixelData.roughness = max( 0.001f, pixelData.perceptualRoughness * pixelData.perceptualRoughness );
 @end
User avatar
TaaTT4
OGRE Contributor
OGRE Contributor
Posts: 267
Joined: Wed Apr 23, 2014 3:49 pm
Location: Bologna, Italy
x 75
Contact:

Re: HLMS PBR BRDF

Post by TaaTT4 »

There's also a memory leak in CompositorPassIblSpecular::setupComputeShaders.

I believe that the following line (220):

Code: Select all

const String newId = StringConverter::toString( Id::generateNewId<CompositorPassIblSpecular>() );
should stay inside the loop:

Code: Select all

for( uint8 mip = 0u; mip < outNumMips; ++mip )
Otherwise, outNumMips copies of iblSpecular shader are created with the same name (and then their deallocation fails).

Senior programmer at 505 Games; former senior engine programmer at Sandbox Games
Worked on: Racecraft EsportRacecraft Coin-Op, Victory: The Age of Racing

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

Re: HLMS PBR BRDF

Post by dark_sylinc »

TaaTT4 wrote: Mon Jan 13, 2020 3:09 pm There's also a memory leak in CompositorPassIblSpecular::setupComputeShaders.
Otherwise, outNumMips copies of iblSpecular shader are created with the same name (and then their deallocation fails).
Oopss! Thanks. Pushed a fix. I added an assert so this doesn't happen again. I can't test every sample right now, so it's possible this assert triggers somewhere else too.
rujialiu wrote: Mon Jan 13, 2020 2:10 pm 1. RSC_UAV is also never set for iOS. Since my iPad can even do compute shader based voxelization, it should support UAV, right?
Yes. I just remembered that RSC_UAV was meant to mean 'supports UAV in pixel shaders'. This was a mistake from mine in not being much more explicit on the naming of the flag.
I should've named RSC_UAV_CS and RSC_UAV_PS.

The confusion is that:
  1. DX 10 & 10.1 hardware supports DirectCompute 4.0 (only if driver update added support for it), but no UAV other than extremely basic ones (and we're not interested in supporting this, ever). Hence supporting Compute Shaders meant UAV support was guaranteed. I was not clear about this (it was in my head and nowhere else...).
  2. iOS HW does not support UAV in pixel shaders
So I now changed the meaning of RSC_UAV to mean 'supports UAV in at least one stage' which is what the name intuitively suggests.

Code: Select all

2. Shader compile error:

Metal SL Compiler Error in 0SpecularIblIntegrator_cs:
Compilation failed:
I've fixed this blindly (I have no access to Mac right now). I'll be able to verify it later, but if you get to see the commits before I get to double check, it should work with the commits I pushed.
But on the bright side, we're quite satisfied with the DPM version with D3D11! Cubemap array version also works, but the mipmaps are a bit too blurred (when src size is 512x512, I can hardly recognize any object in mip 1!). Considering the fact that we had to support older iPads that don't support cubemap arrays, we decided that we'll use DPM on both render systems.
I'm glad to hear this :)
Like we discussed, I've added a variable perceptualRoughness to PixelData struct because I'm using both roughness AND perceptualRoughness (see below). I haven't tested as carefully as xrgo, but I also did some experiments and believe this change is needed to match other engines.

Here is the diff that we're currently satisfied with:
Thanks for the diff!!! This helps me test the change quicker.
rujialiu
Goblin
Posts: 296
Joined: Mon May 09, 2016 8:21 am
x 35

Re: HLMS PBR BRDF

Post by rujialiu »

dark_sylinc wrote: Mon Jan 13, 2020 3:58 pm So I now changed the meaning of RSC_UAV to mean 'supports UAV in at least one stage' which is what the name intuitively suggests.
Thanks! It's reasonable.
I've fixed this blindly (I have no access to Mac right now). I'll be able to verify it later, but if you get to see the commits before I get to double check, it should work with the commits I pushed.
Thanks! It compiles. However, I got another error:

Code: Select all

Shader 0SpecularIblIntegrator_cs compiled successfully.

validateComputeFunctionArguments:834: failed assertion `Compute Function(main_metal): Shader uses texture(lastResult[8]) as read-write, but hardware does not support read-write texture of this pixel format.'
Actually I've met this error before: the voxelization code also suffers from this error after I upgraded XCode and/or iOS. I didn't care because we're not using voxelization on iOS anyway, but now I realize that it may related to ALL texture-writing codes, not just voxelization.
Thanks for the diff!!! This helps me test the change quicker.
Nice to hear it's useful, but actually I haven't finished yet. For example, I haven't confirmed ltc area light codes etc.
rujialiu
Goblin
Posts: 296
Joined: Mon May 09, 2016 8:21 am
x 35

Re: HLMS PBR BRDF

Post by rujialiu »

rujialiu wrote: Tue Jan 14, 2020 4:11 am Actually I've met this error before: the voxelization code also suffers from this error after I upgraded XCode and/or iOS. I didn't care because we're not using voxelization on iOS anyway, but now I realize that it may related to ALL texture-writing codes, not just voxelization.
It looks like this:
https://forums.developer.apple.com/thread/123657
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 5296
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 1278
Contact:

Re: HLMS PBR BRDF

Post by dark_sylinc »

Grrrr... Apple can be so annoying. Read-Write UAVs was added to iOS in A11 chip.
However if you read and write in a careful manner (read once, then write once, only one thread touches each pixel) it just works (official Apple statement is "undefined behavior" though).

It can also be hacked away by setting the same UAV in two slots, one for read one for write (again official Apple statement is "undefined behavior")

Fortunately this is for a feature for ibl_specular (spread IBL generation over multiple frames) that we're not currently using, so I'll force disable that feature instead of trying to hack it.
rujialiu
Goblin
Posts: 296
Joined: Mon May 09, 2016 8:21 am
x 35

Re: HLMS PBR BRDF

Post by rujialiu »

dark_sylinc wrote: Tue Jan 14, 2020 4:24 am Grrrr... Apple can be so annoying. Read-Write UAVs was added to iOS in A11 chip.
However if you read and write in a careful manner (read once, then write once, only one thread touches each pixel) it just works (official Apple statement is "undefined behavior" though).

It can also be hacked away by setting the same UAV in two slots, one for read one for write (again official Apple statement is "undefined behavior")

Fortunately this is for a feature for ibl_specular (spread IBL generation over multiple frames) that we're not currently using, so I'll force disable that feature instead of trying to hack it.
Haha! I just spent some time on it (reading feature set table and ibl_specular pass code) and found the same thing (read-write is only needed when we spread IBL generation over multiple frames).

If you're going to force disable it officially soon, I'll just wait 8-)
rujialiu
Goblin
Posts: 296
Joined: Mon May 09, 2016 8:21 am
x 35

Re: HLMS PBR BRDF

Post by rujialiu »

dark_sylinc wrote: Tue Jan 14, 2020 4:24 am Fortunately this is for a feature for ibl_specular (spread IBL generation over multiple frames) that we're not currently using, so I'll force disable that feature instead of trying to hack it.
You're so quick! I've just test it. It works!!! And there is no noticable visual difference from our D3D11 version, so I guess it really works 8-)
Thanks!!!
rujialiu
Goblin
Posts: 296
Joined: Mon May 09, 2016 8:21 am
x 35

Re: HLMS PBR BRDF

Post by rujialiu »

dark_sylinc wrote: Tue Jan 14, 2020 4:24 am Fortunately this is for a feature for ibl_specular (spread IBL generation over multiple frames) that we're not currently using, so I'll force disable that feature instead of trying to hack it.
Two of our beta testing machines failed:

Code: Select all

17:46:47: High-level program 0SpecularIblIntegrator_cs encountered an error during loading and is thus not supported.
OGRE EXCEPTION(-2147024809:RenderingAPIException): Cannot create D3D11 Compute shader 0SpecularIblIntegrator_cs from microcode.
Error Description:invalid parameters were passed.
 in D3D11HLSLProgram::CreateComputeShader at C:\OGRE\RenderSystems\Direct3D11\src\OgreD3D11HLSLProgram.cpp (line 2000)
Both machines is running at windows 7 with A NVidia GTX 750 Ti card. There was no compile errors (I used fxc -T cs_50 xx.hlsl to compile it on the same machine and it succeeded)

I was quite desparate at first, but after some random tries, I found if I force-disable the same thing as iOS, both machines work again!
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 5296
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 1278
Contact:

Re: HLMS PBR BRDF

Post by dark_sylinc »

rujialiu wrote: Tue Jan 14, 2020 2:08 pm Both machines is running at windows 7 with A NVidia GTX 750 Ti card. There was no compile errors (I used fxc -T cs_50 xx.hlsl to compile it on the same machine and it succeeded)

I was quite desparate at first, but after some random tries, I found if I force-disable the same thing as iOS, both machines work again!
That is definitely a driver bug. Try updating the drivers, or moving to Windows 10.
Or just use the same iOS disable thing :)
rujialiu
Goblin
Posts: 296
Joined: Mon May 09, 2016 8:21 am
x 35

Re: HLMS PBR BRDF

Post by rujialiu »

dark_sylinc wrote: Tue Jan 14, 2020 3:26 pm That is definitely a driver bug. Try updating the drivers, or moving to Windows 10.
Or just use the same iOS disable thing :)
Ok. We disabled it :)
rujialiu
Goblin
Posts: 296
Joined: Mon May 09, 2016 8:21 am
x 35

Re: HLMS PBR BRDF

Post by rujialiu »

Just now our beta tester found a shader compilation error:

Code: Select all

@property( needs_env_brdf && ltc_texture_available )
    @piece( DeclAreaLtcTextures )
        @property( syntax == glsl || syntax == glsles )
            uniform sampler2DArray ltcMatrix;
        @end
        ...
In some case needs_env_brdf=0 but we still have ltc area lights...
Currently I changed it to @property(ltc_texture_available) but I don't know whether it's correct.
rujialiu
Goblin
Posts: 296
Joined: Mon May 09, 2016 8:21 am
x 35

Re: HLMS PBR BRDF

Post by rujialiu »

xrgo wrote: Sun Jan 12, 2020 3:40 am what about the diffuse part, as solarportal mentioned:
SolarPortal wrote:then we rendered an irradiance texture which is a small 32x32 cubemap with no mips for the diffuse IBL and ran a separate compute shader that computes Diffuse Irradiance
apparently is very important...
Maybe it's time to discuss the diffuse part?

By separating light probes (for diffuse GI only) from PCCs, we can have a relatively dense grid of light probes, each of which have a small 32x32 cubemap. It's like Unity's LPPV, but it looks like Unity's light probes only got data from baked lightmap, not rendered in realtime, so LPPV can't be used for realtime diffuse GI.

We already have irradiance field with depth, but currently we can only create it from voxel cone tracing, and voxelization can be slow and memory heavy. Can we use the same way as PCC probes to render irradiance problems? It's slower than raytracing or voxel cone tracing, but at least we don't need voxelization and the grid can be at least denser than PCC probes, so it should look nicer than using PCC probes for diffuse GI?
rujialiu
Goblin
Posts: 296
Joined: Mon May 09, 2016 8:21 am
x 35

Re: HLMS PBR BRDF

Post by rujialiu »

xrgo wrote: Wed Dec 25, 2019 12:39 am ...can't test substance since it doesn't allow adding lights
I managed to test substance player (It's free!) with custom lights. Just edit fs.glsl of the shader you want to modify:

Code: Select all

uniform vec3 Lamp0Pos = vec3(0.0,0.0,70.0);
uniform vec3 Lamp0Color = vec3(1.0,1.0,1.0);
uniform float Lamp0Intensity = 1.0;
Those uniforms will be reset by the app so modify them directly won't take effect. However, you can make some new uniforms:

Code: Select all

uniform vec3 Lamp0Pos2 = vec3(0.0,50.0,60.0);
uniform vec3 Lamp0Color2 = vec3(1.0,1.0,1.0);
uniform float Lamp0Intensity2 = 10.0;
And replace every occurrence with the new one (there are very few). That's it. The value of Lamp0Pos2 above is IN FRONT of the preset objects (cube/rounded cubes etc. If you open RoundedCube.FBX in blender you will see the rounded boxes have minx=-50, maxx=50 etc).

This way, I'm able to make exactly the same scene with Ogre and with some modifications the result is now EXTREMELY similar (I'd say 99% similar but I don't have a picture now)
* Disable substance player's IBL lighting by dragging environment's exposure to -10
* apply perceptualRoughness modification, as I posted earlier
* Substance Player's light attenuation is slightly different:

Code: Select all

float lampAttenuation(float distance)
{
    return 1.0/(1.0+DISTANCE_ATTENUATION_MULT*distance*distance);
}
I don't mean Ogre's is wrong, but to compare result, we need to change it to match Substance Player's:

Code: Select all

--- a/media/Hlms/Pbs/Any/Main/200.ForwardPlus_piece_ps.any
+++ b/media/Hlms/Pbs/Any/Main/200.ForwardPlus_piece_ps.any
@@ -134,6 +134,9 @@
         if( fDistance <= attenuation.x @insertpiece( andObjLightMaskFwdPlusCmp ) )
         {
             lightDir *= 1.0 / fDistance;

+@property( micmic_substance_player_type )
+            float atten = 1.0 / (1.0 + attenuation.z * fDistance * fDistance );
+@else
             float atten = 1.0 / (0.5 + (attenuation.y + attenuation.z * fDistance) * fDistance );
             @property( hlms_forward_fade_attenuation_range )
                 atten *= max( (attenuation.x - fDistance) * attenuation.w, 0.0f );
...
+@end
* Substance Player is using lambertian diffuse, so

Code: Select all

--- a/media/Hlms/Pbs/Any/Main/200.BRDFs_piece_ps.any
+++ b/media/Hlms/Pbs/Any/Main/200.BRDFs_piece_ps.any
@@ -156,7 +156,9 @@ INLINE float3 BRDF( float3 lightDir, float3 lightDiffuse, float3 lightSpecular,
 
     //We should divide Rs by PI, but it was done inside G for performance
     float3 Rs = ( fresnelS * (R * G) ) * pixelData.specular.xyz * lightSpecular;
-
+@property( micmic_substance_player_type == 2 )
+    float3 Rd = pixelData.diffuse.xyz * (1.0 - pixelData.F0) * lightDiffuse;
+@else
     //Diffuse BRDF (*Normalized* Disney, see course_notes_moving_frostbite_to_pbr.pdf
     //"Moving Frostbite to Physically Based Rendering" Sebastien Lagarde & Charles de Rousiers)
     float energyBias    = pixelData.roughness * 0.5;
@@ -173,7 +175,7 @@ INLINE float3 BRDF( float3 lightDir, float3 lightDiffuse, float3 lightSpecular,

     //We should divide Rd by PI, but it is already included in kD
     float3 Rd = (lightScatter * viewScatter * energyFactor * fresnelD) * pixelData.diffuse.xyz * lightDiffuse;
-
+@end
     return NdotL * (Rs + Rd);
 }
 @end
In my code above, micmic_substance_player_type > 0 means change light attenuation and micmic_substance_player_type = 2 means change brdf (use lambertian diffuse). Note that we're still using Ogre's setAttenuation function to set quadratic coefficient:

Code: Select all

#define DISTANCE_ATTENUATION_MULT 0.001
...
light->setAttenuation(1000.0f, 0.0f, 0.0f, DISTANCE_ATTENUATION_MULT);
The next step is to compare IBL lighting but I'm afraid I won't have time in near future. Anyone wants to do it?
rujialiu
Goblin
Posts: 296
Joined: Mon May 09, 2016 8:21 am
x 35

Re: HLMS PBR BRDF

Post by rujialiu »

Anyone had time to look at IBL?

Currently the shader used this (which is discussed here: viewtopic.php?f=25&p=523550#p523544)
https://seblagarde.wordpress.com/2011/0 ... llo-world/

However, that article is quite old... it even pradates Disney's principled PBR in 2012! I believe the new cubemap filtering/pre-integration also solves this problem in another way, so that old trick doesn't need to be applied anymore? I haven't carefully checked the result, but after removing fresnelS and fresnelD, it looks much closer to Substance Player and VRay at glazing angles, for rougher surfaces (roughness > 0.5).

BTW: It looks like Substance Player is using importance sampling for IBL specular and SH for IBL diffuse, it should be a good reference for checking Ogre's IBL.
User avatar
TaaTT4
OGRE Contributor
OGRE Contributor
Posts: 267
Joined: Wed Apr 23, 2014 3:49 pm
Location: Bologna, Italy
x 75
Contact:

Re: HLMS PBR BRDF

Post by TaaTT4 »

Unfortunately, I don't have the time to make test&compare at the moment. My focus is to port my engine to OGRE 2.2 and this takes highest priority over everything else. I'm not even sure to own the skills to be a reliable in this field.
Anyway, I'm super-interested in IBL. Apart the lack of tools (exporters, material editor and so on), the biggest issue that the artists who work with me have raised is the fact that OGRE rendering is a bit too much different from Marmoset and Substance Player. This creates difficulties to them since, as soon as they see their work in OGRE, it's not what they expect (and then they have to re-tune textures and materials). The same thing happens when you buy/use assets made by a 3d party.
I'm not arguing about who's better and it's not even fair to compare "them" to "us" (commercial vs open-source development model; many developers vs an almost solo programmer). But since UE4 and his friends have a huge user-base and they have created a de-facto standard, trying to adhere to it, in my opinion, should be a priority.

Senior programmer at 505 Games; former senior engine programmer at Sandbox Games
Worked on: Racecraft EsportRacecraft Coin-Op, Victory: The Age of Racing

zxz
Gremlin
Posts: 184
Joined: Sat Apr 16, 2016 9:25 pm
x 19

Re: HLMS PBR BRDF

Post by zxz »

The IBL code and other features (like IES profiles) seem to exist only in the v2-2-irradiance-field branch. However, I can't find much information regarding the status of this branch. Is it stable? Unstable? How far are these things from being merged into master? Is there a writeup of new features somewhere?

Also, have any changes resulting from the discussion in this thread landed in the repo? I'm also interested in getting results more comparable with artists' tools.

Thanks!
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 5296
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 1278
Contact:

Re: HLMS PBR BRDF

Post by dark_sylinc »

zxz wrote: Wed Apr 22, 2020 12:13 am The IBL code and other features (like IES profiles) seem to exist only in the v2-2-irradiance-field branch. However, I can't find much information regarding the status of this branch. Is it stable? Unstable? How far are these things from being merged into master? Is there a writeup of new features somewhere?
It's very stable. I haven't been very verbal about this but this is the plan:
  • 2.1 branch is mature, pending to be released as SDK
  • Current master will be released as 2.2 SDK
  • Current v2-2-irradiance-field branch will be released as 2.2.1 SDK and then become master
The reason none of the SDKs have been released yet is because I have been too lazy to do it. Perhaps I should gather strength to do it this weekend.
zxz wrote: Wed Apr 22, 2020 12:13 am Also, have any changes resulting from the discussion in this thread landed in the repo? I'm also interested in getting results more comparable with artists' tools.
If I recall it's:
  • Add perceptualRoughness
  • Add rujia's micmic_substance_player_type code
Am I missing anything?
xrgo
OGRE Expert User
OGRE Expert User
Posts: 1148
Joined: Sat Jul 06, 2013 10:59 pm
Location: Chile
x 168

Re: HLMS PBR BRDF

Post by xrgo »

dark_sylinc wrote: Wed Apr 22, 2020 12:50 am Am I missing anything?
this:

Code: Select all

float sqR = pixelData.roughness * pixelData.roughness;
sqR = sqR * sqR;
I have made the comparison and its matching filament's brdf viewtopic.php?p=546901#p546901
and also looks very similar to eevee and ue viewtopic.php?p=546833#p546833
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 5296
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 1278
Contact:

Re: HLMS PBR BRDF

Post by dark_sylinc »

xrgo wrote: Wed Apr 22, 2020 5:38 am
dark_sylinc wrote: Wed Apr 22, 2020 12:50 am Am I missing anything?
this:

Code: Select all

float sqR = pixelData.roughness * pixelData.roughness;
sqR = sqR * sqR;
I have made the comparison and its matching filament's brdf viewtopic.php?p=546901#p546901
and also looks very similar to eevee and ue viewtopic.php?p=546833#p546833
Isn't that perceptualRoughness? (I believe so) Or am I confusing it with something else?
User avatar
TaaTT4
OGRE Contributor
OGRE Contributor
Posts: 267
Joined: Wed Apr 23, 2014 3:49 pm
Location: Bologna, Italy
x 75
Contact:

Re: HLMS PBR BRDF

Post by TaaTT4 »

dark_sylinc wrote: Wed Apr 22, 2020 6:09 am Isn't that perceptualRoughness? (I believe so) Or am I confusing it with something else?
Yes, I guess it's the same thing that rujialiu calls perceptualRoughness.
At this point, can't we add this directly in OGRE code (controllable by an optional flag)? I mean, rujialiu is using it, xrgo is using it, I am using it... Do I have to continue?! :wink: :lol:

Senior programmer at 505 Games; former senior engine programmer at Sandbox Games
Worked on: Racecraft EsportRacecraft Coin-Op, Victory: The Age of Racing

xrgo
OGRE Expert User
OGRE Expert User
Posts: 1148
Joined: Sat Jul 06, 2013 10:59 pm
Location: Chile
x 168

Re: HLMS PBR BRDF

Post by xrgo »

oohhh yes, sorry, didn't noticed... apparently this:

Code: Select all

pixelData.roughness = max( 0.001f, pixelData.perceptualRoughness * pixelData.perceptualRoughness );
makes it have the same effect later int brdf
so that would be enough
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 5296
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 1278
Contact:

Re: HLMS PBR BRDF

Post by dark_sylinc »

Pushed and enabled by default.

I'm in doubt between 2 ways on how to approach Rujia's method:
  1. New BRDF mode. Sounds conceptually correct, since Substance Player's BRDF is different (and less correct) than ours; and this setting would be set at material level.
  2. Global toggle "substance player mode": More practical. Users would rarely want to mix Substance Player's BRDFs with Ogre's default; and setting this value per material is too cumbersome. A global toggle sounds much more user friendly
Btw lighting attenuation must be done as global toggle, since it's something that should affect all BRDFs the same way, otherwise we'd get very inconsistent results.

Thoughts?
xrgo
OGRE Expert User
OGRE Expert User
Posts: 1148
Joined: Sat Jul 06, 2013 10:59 pm
Location: Chile
x 168

Re: HLMS PBR BRDF

Post by xrgo »

my vote is for a global toggle, simpler, and our artists uses substance for every asset, if it looks closer I'll definitely use it for every material, so a per datablock setting is something I wouldn't use
User avatar
TaaTT4
OGRE Contributor
OGRE Contributor
Posts: 267
Joined: Wed Apr 23, 2014 3:49 pm
Location: Bologna, Italy
x 75
Contact:

Re: HLMS PBR BRDF

Post by TaaTT4 »

I have no preference at all. At the moment, I use the same BRDF algorithm (Default) for all the materials. I felt the need to use a different BRDF just in specific edge cases (eg. cloth like objects, grass and tree billboards), but I haven't ever had time to try and experiment.

Senior programmer at 505 Games; former senior engine programmer at Sandbox Games
Worked on: Racecraft EsportRacecraft Coin-Op, Victory: The Age of Racing

Post Reply