New MeshStandardMaterial in r112 has banding artifacts on some GPUs

[toji]

As of r112 the default MeshStandardMaterial (in this case specifically as it's used when loading glTF files, but probably in other scenarios as well) is exhibiting banding artifacts on some GPUs. Specifically, I have see this issue on a Pixelbook and every generation of Pixel phones. The issue does not occur on the Nvidia desktop GPUs I have tried, nor on an Oculus Go or Oculus Quest.

It should also be noted that I've observed the material's new shader incurred a noticable performance hit relative to r111 for my particular application on various mobile devices, including ones that don't exhibit the rendering artifacts.

The artifacts look like this (Rendered with Three.js r112 on a Pixelbook)

The expected output looks like this (Rendered with Three.js r111 on the same Pixelbook)

Live link, currently using r112: https://xrdinosaurs.com

All of the dinosaurs on that page exhibit the issue, but ones with large areas of flat or smooth colors (like the TRex's belly) tend to stand out more.

The material in the screenshot (pasted in full below) uses the glTF extension "KHR_materials_pbrSpecularGlossiness", and has a diffuse, normal, and specular/glossiness textures.

    {
      "doubleSided": true,
      "emissiveFactor": [
        0,
        0,
        0
      ],
      "extensions": {
        "KHR_materials_pbrSpecularGlossiness": {
          "diffuseFactor": [
            0.46512957319999998,
            0.46512957319999998,
            0.46512957319999998,
            1
          ],
          "diffuseTexture": {
            "index": 4,
            "texCoord": 0
          },
          "glossinessFactor": 0.27610518290000002,
          "specularFactor": [
            0.92244664629999995,
            0.92244664629999995,
            0.92244664629999995
          ],
          "specularGlossinessTexture": {
            "index": 6,
            "texCoord": 0
          }
        }
      },
      "name": "TRex",
      "normalTexture": {
        "index": 5,
        "scale": 1,
        "texCoord": 0
      }
    }

[Mugen87]

I can confirm the glich on my Pixel (1). However, the artifacts only seems to occur when KHR_materials_pbrSpecularGlossiness is used.

#18042 introduced geometric anti-aliasing. However, GLTFLoader replaces the code of the lights_physical_fragment shader chunk with the following:

three.js/examples/js/loaders/GLTFLoader.js

Lines 683 to 688 in 3ba0553

	var lightPhysicalFragmentChunk = [
	'PhysicalMaterial material;',
	'material.diffuseColor = diffuseColor.rgb;',
	'material.specularRoughness = clamp( 1.0 - glossinessFactor, 0.04, 1.0 );',
	'material.specularColor = specularFactor.rgb;',
	].join( '\n' );

@toji Can you please modify the GLTFLoader copy of your app by replacing the above code with:

var lightPhysicalFragmentChunk = [
	'PhysicalMaterial material;',
	'material.diffuseColor = diffuseColor.rgb;',
	'vec3 dxy = max( abs( dFdx( geometryNormal ) ), abs( dFdy( geometryNormal ) ) );',
	'float geometryRoughness = max( max( dxy.x, dxy.y ), dxy.z );',

	'material.specularRoughness = max( 1.0 - glossinessFactor, 0.0525 );// 0.0525 corresponds to the base mip of a 256 cubemap.',
	'material.specularRoughness += geometryRoughness;',
	'material.specularRoughness = min( material.specularRoughness, 1.0 );',
	'material.specularColor = specularFactor.rgb;',
].join( '\n' );

@elalish Even if this patch does not solve the issue, we should add this to GLTFLoader in any case, right?

[elalish]

@Mugen87 Yes, for sure. I'll see if I can repro on my Pixel 3 as well.

[elalish]

@toji I do not repro on my Pixel 3 with Android 10, so at least it's not all Pixels...

Driver bug? I'm not sure which line is triggering it though. The presence/absence of geometric anti-aliasing wouldn't make that type of artifact anyway, I'm pretty sure.

[elalish]

@toji I also just tried my dedicated crappy tester phone (an Alcatel that's about $30 from Target). I don't get the banding artifacts, though I noticed the sky is black. As for perf: it's not exactly buttery, but it's not bad. Honestly that 3D view responds better than the 2D keyboard UI on this phone. Go figure.

[elalish]

Ugh, the black sky actually means the PMREM generation failed entirely, and it's due to this weird bug and workaround we found: google/model-viewer#920. I'd meant to get this into r112, but it slipped by me. Likely unrelated to the OP's bug though.

[plut0nist]

I reproduced OP's error on my OnePlus 5T in both Chrome and Firefox.

[toji]

Thanks! Applied the patch to GLTFLoader and observed that the banding artifact was reduced, but not eliminated.

Before patch:

After patch:

Basically, it looks like it fixed one of the bands. :) (Screenshots taken on a Pixelbook, but also observed on a Pixel 4 XL. Don't have other test devices on me at the moment.)

Happy to patch in any other changes you want to test, or you could check out https://github.com/toji/xr-dinosaurs if you want to test it locally. (Doesn't require any server component.)

As for perf: it's not exactly buttery, but it's not bad.

Thank you. 😁 Been working hard of performance. In this case during the update to r112 I noticed the perf regression and asked @mrdoob about it. He suggested it may be the new, more accurate standard shader and suggested swapping some materials out with MeshLambertMaterial to verify. Since the environment model didn't have properly configured PBR materials anyway I force it to Lambert and left it there, which gained back any perf regressions and then some (since it takes up a large chunk of the viewport.) Left the dinosaurs with the standard material since they're the "hero" asset.

[Mugen87]

Sidenote: Scene.environment does currently not work when GLTFLoader loads meshes using specular/glossiness materials since the renderer checks for MeshStandardMaterial e.g.

three.js/src/renderers/WebGLRenderer.js

Line 1633 in dd81aad

var environment = material.isMeshStandardMaterial ? scene.environment : null;

However, GLTFLoader creates a ShaderMaterial for each specular/glossiness material. Realized this today when testing with the T-Rex model^^. Well, one more reason to finally complete #14099.

[Mugen87]

I cannot reproduce the banding artifacts when no environment map is used. Everything looks good on a Pixel (1) with a simple lighting setup like an ambient and a point light.

[Mugen87]

Um, there seems to be more issues with physical materials using an environment map. Check out how the following examples look on a Pixel 1, Android 10.

https://threejs.org/examples/webgl_materials_envmaps_exr

https://threejs.org/examples/webgl_materials_physical_clearcoat

https://threejs.org/examples/webgl_materials_envmaps_hdr

I can't reproduce these artifacts on my iMac.

[sciecode]

Something pretty weird is happening on my machine, I'm able to reproduce these artifacts on all examples that utilize the new PMREMGenerator. But only when running them locally; when viewing them online ( from threejs.org page ) they all appear correct.

I've never experienced such issue and I'm completely lost as to what causes it.

I'm running a Windows 10 with a GeForce GTX 750 TI, so I don't think this is a device-specific issue.

[Mugen87]

But only when running them locally; when viewing them online ( from threejs.org page ) they all appear correct.

Be aware that the dev version has changes which are not present in prod yet. Can you please checkout the current master branch and then perform a local test?

[sciecode]

Can you please checkout the current master branch and then perform a local test?

First thing I've tried after noticing this behavior, same results. Really curious.

[Mugen87]

Can you please test with incognito windows? Sometimes stuff might be cached or browser extensions interfere which can be really confusing

[sciecode]

Tried with incognito, same results :(

I believe I know why this banding is occurring tho.
During mip generation, my local (incorrect) version is creating a black band that doesn't exist on live version.

This is replicated to lower mip levels as well.

This is likely happening due to some incorrect coordinate sampling, I've experimented with manually disabling anisotropic filtering, but the problem remains. It's hard to pin-point exactly what is happening, especially because I can't wrap my head around why it presents two different behaviors on the same machine.

[sciecode]

@elalish after running some tests on multiple examples, I was able to pinpoint the actual cause of these artifacts. Contrary to what I said on my previous post, the root cause of the problem is not related with incorrect coordinates sampling, but with the way PMREMGenerator is handling devicePixelRatio.

On devices with nominal floating point pixelRatio we are getting "bad" mipmap coordinates on the generated texture. So there are small gaps and overlapping regions. I also discovered that my machine has different devicePixelRatios depending if I'm viewing the example locally (0.899..) or online (1) and that's why I was only experiencing these artifacts locally.

I've setup a simple test by disabling setPixelRatio and it seems to solve the problem, if that's the case for others, then it would be best to refactor the way PMREMGenerator is handling it.

@toji @plut0nist mind checking the following examples on the devices that presented these artifacts?

DEV Example
TEST Example

[elalish]

@sciecode Thank you so much for figuring out the root cause! I'll see if I can work up a solution.

[toji]

@sciecode: Verified on my devices. The DEV Example test exhibits a very obvious black line in the environment map while the TEST Example does not. Awesome debugging, thanks!

I'm was extremely curious why PMREMGenerator needed to account for devicePixelRatio at all, so I went digging. It turns out it's because renderer.setViewport() automatically factors the DPR into any values you send it, which feels like it's clearly the wrong behavior for render targets other than the default framebuffer, which are allocated with exact pixel values that don't account for the pixel ratio at all. I'd suggest the "right" behavior here is that internally rendere.setViewport() should be using getTargetPixelRatio(), which returns 1 when the active render target is anything other than the default. I can definitely see how that could introduce backwards compat issues, though.