PBR Lighting issue

[zzuegg]

To continue the discussion from:
https://hub.jmonkeyengine.org/t/zombiegirl-gltfloader-vs-monkeywrench/48007/15

Summary:

According to gltf specs, and on the reference viewers, the default value for metallic factor is 1. Jme's importers are setting the metallic factor to 0 to pass the test cases. The problematic line in my opinion is:

//223 of PBRLighting
vec4 diffuseColor = albedo - albedo * Metallic;

I have checked, a few other repositories, and i have not yet found that piece of math again. Additionally, the whole block:

float specular = 0.5;
float nonMetalSpec = 0.08 * specular;
vec4 specularColor = (nonMetalSpec - nonMetalSpec * Metallic) + albedo * Metallic;
vec4 diffuseColor = albedo - albedo * Metallic;
vec3 fZero = vec3(specular);

is quite unique. F0 is in all other shaders defined as:

vec3 F0 = vec3(0.04);
F0 = mix(F0, albedo, metallic);

[zzuegg]

When testing the MetalRoughSpheres jme shows also different results than the khronos sample viewer.

The reflexivity is lost completely in the second column in jme while in the khronos renderer it slowly fades

[stephengold]

Thanks for investigating.

When it comes time to solve this, instead of modifying the PBRLighting shaders (and potentially breaking many apps) I suggest defining a new PBR material with a new name.

[yaRnMcDonuts]

This looks like it is quite a big bug in our PBR implementation.

I've always speculated that something looks slightly off with some of my PBR textures in jme compared to the original source, especially under certain lighting conditions... and I think this might have been it.

I just tested the results with the old (broken) fZero calculation compared to the new code you suggested, and it appears to look much better with your code.

The difference is even more noticeable first-hand, but you can still see a difference in these 2 screenshots (both taken with identical lighting in same place and the same time of day in-game) :

Here is with broken fZero calculation (you can notice a very slight annoying glare to parts of the scene)

And here is the same time/place with the fixed fZero calculation. It looks much better in my opinion, especially the grass:

[zzuegg]

Unfortunately the sphere tests renders wrong when i replace the fZero calculation. There is more to it. My guess is that jme counteracts the wrongness at some point. I am not yet at that point. Currently i am convertig the khronos reference shader to jme so it can be used as a direct replacement.

Are the samples rendering correct on your end?

[yaRnMcDonuts]

Are the samples rendering correct on your end?

Do you mean the varying shiny spheres example you posted the screenshot from?

I tried finding that test to run and check, but I can't seem to find it.

[zzuegg]

https://github.com/KhronosGroup/glTF-Sample-Models/tree/main/2.0

The models are MetalRoughSperes and the no texture variant

[yaRnMcDonuts]

I applied the fixed PBRLighting shader from my PR to those MetalRoughSpheres and loaded them up in my editor with defaultProbe.j3o, and it looks okay as far as I can tell:

What did it look like when you tried rendering them?

[yaRnMcDonuts]

Hi @zzuegg I'm curious if you managed to test this in the latest alpha release to see if you think the fixed fZero calculation is okay?

Or do you think there still may be a chance that the strange fZero calcualtion was necessary to account for something else that is unique in jme's pbr implementation?

I also just realized I didn't apply the new fZero calculation to the specGloss pipeilne, and it still uses the old calculation here:

jmonkeyengine/jme3-core/src/main/resources/Common/ShaderLib/module/pbrlighting/PBRLightingUtils.glsllib

Line 477 in b846c6a

surface.fZero = surface.specularColor.xyz;

Should I also apply these changes there, or do you think specGloss needs a different fZero calculation? I could try to test and see the difference, but unfortunately I only have 1 specGloss model in my game so its not as easy for me to spot the rendering differences with specGloss like I was able to when I changed things for the metallicRoughness pipeline.

Thanks!