Experimental way to deal with artifacts in GGX, inspired by Frostbite 3

[bhouston]

See discussion here:

#8349 (comment)

[erichlof]

Thanks Ben, this should do the trick! :-)

[bhouston]

Just to be clear @erichlof I've never confirmed that this actually gets rid of the artifacts, so please confirm and if @WestLangley approves as well we can merge.

[WestLangley]

@erichlof Also, please describe the artifacts you are seeing -- and under what parameter settings -- so we understand what we are supposed to be fixing. See #8349 (comment)

[erichlof]

@WestLangley ,
I have created a nearly identical html source file in which the only change is removing our recent epsilon value added to the denominator.
Here is the original correctly working three.js pathtracing renderer:
http://erichlof.github.io/THREE.js-PathTracing-Renderer/ThreeJS_PathTracing_Renderer.html

And here is the same program, with the epsilon omitted on line 403. This is literally the only difference between the 2 source files.
http://erichlof.github.io/THREE.js-PathTracing-Renderer/PathTracing_Renderer_DivideByZeroArtifact.html

When one of the dot products is zero, the GGX brdf returns a black color, possibly due to a NaN.

Unfortunately this is a personal project at the moment that is not in the three.js normal usage scope (my vertex shader is 2 lines and my fragment shader is near 1000, which does all the work). I don't know if this can be easily demonstrated with a current three.js PBR materials example that uses the library in the normal way. Maybe the PBR IBL (which kind-of does ray tracing to the environment map) would demonstrate the issue? @bhouston any thoughts on how to make a quick test-case using the normal three.js examples?

[erichlof]

@bhouston
Yes I can confirm that the EPSILONs you added do in fact fix the problem! :)

[WestLangley]

So far, there is no evidence this PR fixes any three.js bug.

If it does, I would love to see evidence of it, and an explanation as to the cause.

[erichlof]

Hi @WestLangley
I located the artifact in a current three.js example - http://threejs.org/examples/#webgl_materials_standard

On my desktop, everything looks great, no artifacts. On my Samsung Galaxy S6 however, there are tiny black patches on the side of the gun (the barrel in particular) as well as large black patches all over when zooming the camera through the model. When zooming through the inside of the gun model on my desktop, the artifacts do not occur and the model's triangles are correctly clipped - everything looks normal. The small black patches come and go while rotating the object, so there is a black shimmering effect.

I never noticed this before because I checked out these demos on my desktop when they were introduced.

In this demo, http://threejs.org/examples/#webgl_lights_pointlights2 the standard material torus shapes are mostly unlit - is this what was intended? They are somewhat illuminated by the pinkish point light, but when the other colored point lights fly by, there is no change to their specular reflectance or brightness - they remain black even when right next to the point light. This occurs severely on mobile and somewhat less on desktop.

I'm sorry but I don't know how to take a snapshot of my mobile screen - I'm searching for an app right now on my device - I'll report back soon.

[MasterJames]

I think to get the screen grab you hold both power and home at the same time.
I had no problem with the gun but also see black toruses.
At first I thought with half the textures loaded the gun was doing like you say so maybe that one was a network glitch.

Samsung Galaxy Note 3 Chrome

[erichlof]

@MasterJames
Thanks for the screen-capture tip - haha! learn something new every day! :) I'm uploading soon...

[erichlof]

Here's the screenshots from the Galaxy S6:

[erichlof]

As you can see from my phone's time-stamp, it's past my bedtime (darn daylight savings time!). I'll check back in tomorrow morning. Thanks all :)

[mrdoob]

Oh! I saw that on a Nexus 5 too.

I thought it was because the phone doesn't support EXT_shader_texture_lod and was not able to handle properly my textureCube(sampler, coord, MIP_LEVEL) hacky replacement.

Have you confirmed that the glitches are gone after this patch?

[bhouston]

@mrdoob wrote:

I thought it was because the phone doesn't support EXT_shader_texture_lod and was not able to handle properly my textureCube(sampler, coord, MIP_LEVEL) hacky replacement.

To see if it is caused by that hack or something else, I've added PMREM generation with CubeUV to the materials_standard example in this PR (which of course will avoid using that hack):

#8360

[bhouston]

Interstingly, the materials_standard example has no direct lighting, just a hemisphere light (indirect diffuse) and envmap (indirect specular, indirect diffuse). This means that G_GGX_Smith and D_GGX are not the culprits in this example as they are not even called.

I did just an experiment an isolated the issue to the envMap and indirect specular. I've also just commented out BRDF_Specular_GGX_Environment and the issue is still there. Thus the issue appears to be related to the incomign indirect specular irradiance. Checking further...

[bhouston]

Yeah, in the materials_standard example, the issue seems to arise from "getLightProbeIndirectRadiance" at least before I upgraded it to use the PMREM/CubeUV stuff. I'm having trouble tracing exactly why the issue arises though.

[bhouston]

When I upgrade materials_standard to CubeUV with PMREM there are still artifacts but they are greatly reduced.... This confuses me a bit to be honest.

[erichlof]

I didn't think about the fact that there is no direct lighting in that example. I'll look as well for other culprits and report back. Although the direct point light torus example doesn't seem to be working properly either, but that might be a light falloff or precision issue.

Ben, could you check if the examples force highp precision in the fragment shader? When I view www.glslsandbox.com examples and the line "precision mediump float;" is at the top, my Android glitches all over the place. When I simply change it to "highp", it works perfectly. Just a thought...

[erichlof]

I was looking at the GGX environment calculations and could this be the culprit?
https://github.com/mrdoob/three.js/blob/dev/src/renderers/shaders/ShaderChunk/bsdfs.glsl#L111

Maybe clamping that to the Epsilon also might avoid raising a 0 dotNV to a exp2(0) later in the function?

Also, I know that the PI's and RECIPROCAL_PI's were added back in until validation, but should there be a RECIPROCAL_PI in the environment function? - it's hard for me to follow the math and make sure everything is correctly cancelling out when it's supposed to be.

[bhouston]

I was looking at the GGX environment calculations and could this be the culprit?
https://github.com/mrdoob/three.js/blob/dev/src/renderers/shaders/ShaderChunk/bsdfs.glsl#L111

It may be a culprit but I was able to still reproduce the issue with BRDF_Specular_GGX_Environment() commented out. So part of the issue is actually within getLightProbeIndirectRadiance().

[bhouston]

When I simply change it to "highp", it works perfectly. Just a thought...

I would love that scenario, but I understand a number of mobile devices simply do not support highp, see here:

https://developers.google.com/web/updates/2011/12/Use-mediump-precision-in-WebGL-when-possible?hl=en

[erichlof]

Oh ok thanks - I will look into that function ( although I don't know much about its inner workings), but maybe just to have an extra pair of eyes on this issue :)

Yes, the 'highp' thing is annoying to say the least - everyone copies and pastes 'mediump' on their fragment shader code on glslsandbox and I'm not able to correctly view any of the examples on my latest higher-end smartphone. Thanks for that link, I might try to file a report or feature request (or stand in line, ha)

[bhouston]

@erichlof "precision highp float;" tends to solve all issues for me.

[erichlof]

Do you mean for this particular issue, or just in general?

If we made three.js enforce highp precision everywhere, how many devices would we leave behind out there in the wild I wonder?

Be back in a couple of hours...

[antont]

this doc is at least somehow updated in 2015 and says, in https://developer.mozilla.org/en-US/docs/Web/API/WebGL_API/WebGL_best_practices

Using highp precision in fragment shaders will prevent your content from working on some older mobile hardware. You can use mediump instead, but be aware that this often results in corrupted rendering due to lack of precision on most mobile devices, and the corruption is not going to be visible on a typical desktop computer. In general, only using highp in both vertex and fragment shaders is safer unless shaders are thoroughly tested on a variety of platforms. Starting in Firefox 11, the WebGL getShaderPrecisionFormat() function is implemented, allowing you to check if highp precision is supported, and more generally letting you query the actual precision of all supported precision qualifiers.

[erichlof]

@antont Thanks for that link and info. I vote for having three.js enforce 'highp' precision as the default everywhere. Ben's link about 'mediump' is from 2011, maybe the majority of cell phones out there now in 2016 have no problem with 'highp'.

Personally speaking, I have a contract with my mobile provider so whenever the new phones come out, I get to upgrade, which means three.js examples run even faster (most at 60fps) and raw Shadertoy.com and glslsandbox.com fragment shaders also perform better on every new generation.

I guess we need to decide where the cuttoff is - which models not to worry about because of age, and which models to target so the above issues I posted don't happen with the majority of three.js users who are on modern mobile devices.

[antont]

yah would be nice to know what the reality is with the devices, didn't have time to check more yet.