Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EnvMap artifacting with 'mediump' precision on Quest 2 after upgrade from r132->r147 #25071

Closed
davehill00 opened this issue Dec 4, 2022 · 17 comments · Fixed by #25121
Closed

Comments

@davehill00
Copy link
Contributor

Describe the bug

I recently upgraded a project from r132 -> r147. In r147, applying envmap to a MeshStandardMaterial shader set to 'mediump' precision causes cubemap artifacting on Quest 2 hardware.

To Reproduce

Steps to reproduce the behavior:

  1. Go to this jsfiddle
  2. Observe reflections on the cylinder from the envmap.
  3. When the Precision option (in the GUI) is set to mediump or lowp, visible banding artifacts are visible.

When I dig in further, it appears that the vec3 bilinearCubeUV( sampler2D envMap, vec3 direction, float mipInt ) function in cube_uv_reflection_fragment.js is the cause of this artifacting. If I add a precision highp float; to the start of that function, which sets floating point precision to high just for the scope of that function IIUC, the issue goes away.

I'm not sure if this also reproduces on Android devices, as I don't have easy access to one to test.

Screenshots
no_artifacts_at_highp
banding_artifacts_at_mediump

Platform:

  • Device: [Quest 2]
  • OS: [Quest VR OS]
  • Browser: [Meta Quest Browser]
  • Three.js version: [r147]
@CodyJasonBennett
Copy link
Contributor

CodyJasonBennett commented Dec 4, 2022

I can reproduce this on a Pixel 3a, Chrome 107 Android 12.

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 4, 2022

Related: #14570

There is no real mediump support for built-in materials right now (so in some sense this issue is a regression but also enhancement). At least MeshStandardMaterial does not produce correct output on mediump mobile devices, even with no environment map applied.

However, it was always assumed that only single code sections in the shaders require updates to make them work on mediump. The mentioned vec3 bilinearCubeUV( sampler2D envMap, vec3 direction, float mipInt ) is obviously one of them.

@mrdoob
Copy link
Owner

mrdoob commented Dec 7, 2022

@davehill00 Any chance you can figure out the exact version of the library that produced this change?

@davehill00
Copy link
Contributor Author

@davehill00 Any chance you can figure out the exact version of the library that produced this change?

Yep, I may not get to it until the weekend but I'll see if I can pinpoint the version.

@davehill00
Copy link
Contributor Author

@mrdoob - took a quick look this evening. It works properly on r135, then EXR loading appears to be broken (at least for the format of EXR file I happen to have) through r140. In r141, EXR loading starts working again, and the mediump issue is present.

Was EXR loading actually broken all that time (r136->r140), or was it just certain formats of EXR? If I can figure out how to generate a working EXR, I can check those last few versions to nail down which version the mediump issue appeared.

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 8, 2022

Since r136, HDR textures and PMREM rely on half float textures, see #22998. We know that this change affected WebGL 1 devices with no half float texture support but could this be related to a Quest 2, too?

@davehill00
Copy link
Contributor Author

@Mugen87 FWIW, I'm seeing the failure to load in r136-r140 on Google Chrome on Mac.

The EXR I am trying to load is the one referenced in the jsFiddle referenced in the original issue I opened. It's one that I rendered out in Blender. When I open it in Photoshop, it looks like it's 32bpp, which I assume means 32-but float. I also rendered out a half-float version in Blender yesterday and saw the same errors in r136-r140.

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 8, 2022

I'm seeing the failure to load in r136-r140 on Google Chrome on Mac.

This particular EXR issue has been fixed via #24049 in r141. It was indeed an issue in the loader.

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 8, 2022

Can you please make a test and verify if the same banding artifacts occur with r136 and RGBELoader? Is it possible to render out a .hdr texture from Blender? I just want to make sure the banding is not related to EXRLoader.

@davehill00
Copy link
Contributor Author

Can you please make a test and verify if the same banding artifacts occur with r136 and RGBELoader? Is it possible to render out a .hdr texture from Blender? I just want to make sure the banding is not related to EXRLoader.

Okay -- created a .hdr of the same texture. On r135, there are no banding artifacts. On r136, the banding artifacts appear. Setting the shader precision to highp makes the artifacts go away. So, it looks like the change came in with r136.

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 9, 2022

Thanks for testing. So the issue is unrelated to EXRLoader.

@elalish I suspect #22998 has introduced the regression. Do you have any ideas why bilinearCubeUV() has problems with mediump after this change?

@elalish
Copy link
Contributor

elalish commented Dec 12, 2022

As best I can tell it's not that it has problems with mediump generally, but specifically on this device. I would guess it's some kind of GPU driver bug. Maybe too little precision during an intermediate computation?

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 12, 2022

I can reproduce the issue with Chrome on a Pixel 4a, too. It seems to happen on a 3a as well, see #25071 (comment).

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 12, 2022

If I add a precision highp float; to the start of that function, which sets floating point precision to high just for the scope of that function IIUC, the issue goes away.

I've added your patch to a modified MeshStandardMaterial here. I can confirm the issues goes away on the Pixel 4a, too. https://jsfiddle.net/nu2jc9kt/2/

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 12, 2022

I could narrow down the issue a bit. It seems changing the following line:

vec2 uv = getUV( direction, face ) * ( faceSize - 2.0 ) + 1.0;

to

highp vec2 uv = getUV( direction, face ) * ( faceSize - 2.0 ) + 1.0; 

fixes the issue, too. Demo: https://jsfiddle.net/6wxabnkq/4/

So it seems the (subsequent) uv computation breaks in some way with mediump.

@elalish
Copy link
Contributor

elalish commented Dec 12, 2022

Good find! That seems like a reasonable work-around.

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 12, 2022

Maybe we figure out at some point a new way of computation that is compatible for mediump. For now, the highp workaround should be sufficient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants