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

Clipping planes weird behavior on certain Android devices #9023

Closed
dennisadams opened this issue Jul 8, 2020 · 8 comments · Fixed by #9060
Closed

Clipping planes weird behavior on certain Android devices #9023

dennisadams opened this issue Jul 8, 2020 · 8 comments · Fixed by #9060

Comments

@dennisadams
Copy link
Contributor

Peek 2020-07-06 22-41

I tried the clipping planes sandcastle on an Android device and got this bug where at the top part of the model, the clipping plane is clipping in the wrong direction.
There already is an open issue - #8061 about clipping planes not working on certain Android devices, but in my case it does technically work and seems like a different issue.

This device doesn't support float textures, and so it uses getClippingPlaneUint8.
The glsl code from getClippingPlaneUint8 calls czm_unpackFloat in which the problem occurs, right here:

float exponent = floor(temp);

If I add a small epsilon value inside floor, e.g. float exponent = floor(temp + 0.01); everything works well.
From my understanding, before the "flooring" temp should either be an integer or an integer + 0.5.
My current guess is that because of precision issues on my device, temp's value turns out to be slightly less than the integer it's supposed to be, and this messes up the exponent and consequently the sign of the float.

The only other place using czm_unpackFloat (if float textures are not supported) is BatchTable.js. I wonder if this can cause other issues on some devices.

@lilleyse
Copy link
Contributor

lilleyse commented Jul 8, 2020

Thanks for the detailed write up @dennisadams

@likangning93 could you confirm that adding a small epsilon is an ok fix here?

@likangning93
Copy link
Contributor

likangning93 commented Jul 24, 2020

Sorry for the slow response! The packing/unpacking technique didn't look familiar enough to me despite what git blame would suggest, so I took a dive through some of the relevant PRs and looks like it originated here: dacc452#diff-a99111718fc832d03c79b3ec06c028a7R514

There's some reference to "discussions offline" but I wasn't able to find a link to anything explaining how this works.
I'm a little hesitant to change the implementation significantly without additional context around this technique, but @dennisadams's mention of device precision got me thinking about the highp sampler2D thing that was recently addressed in #8805, so first I'm going to try out a branch that uses highp to see if that does anything.

Fortunately I'm able to reproduce this combination (planes moving backwards + no float texture support) on an Acer Chromebook Tab 10, which uses a RK3399/Mali-T860 SoC. I'm also seeing weirdness on my Qualcomm 425/Adreno 308 Moto E4, which claims to support float textures, both when using float textures and when I override to force the uint8 codepath. I'd love if all this weirdness just turned out to be different default precisions for samplers on different devices/webgl implementations.

@dennisadams out of curiosity, what device are you testing on?

@likangning93
Copy link
Contributor

likangning93 commented Jul 24, 2020

I put together a smaller Sandcastle for testing this, it's here on the current deployment and here on my branch's deployment.

On the Acer Chromebook Tab 10 RK3399/Mali-T860 I'm seeing a significant improvement.

1.71 branch
Screenshot 2020-07-23 at 8 48 06 PM Screenshot 2020-07-23 at 8 47 29 PM

[EDIT] ignoring the alpha weirdness on the props, this is looking much closer to what I see when testing either link on desktop (well, Intel UHD 630/Linux).

Sadly no dice on the Qualcomm 425/Adreno 308 Moto E4, although that was different weirdness with the model not showing at all unless I viewed it from below. That must be something else entirely, I'm gonna say it's separate from this issue.

@dennisadams can you confirm?

@dennisadams
Copy link
Contributor Author

@dennisadams out of curiosity, what device are you testing on?

Galaxy A20 with ARM Mali-G71.

In my case the two screenshots look pretty much the same:

1.71 branch
   

But that's because they are both good. Probably my problems were with the clipping plane in a different position.
After applying your changes, I'm glad to confirm this solves the issues on my device.
FWIW adding the highp only in getClippingFunction.js seems to be good enough, but I don't understand in glsl best practices.
Thanks @likangning93!

@lilleyse
Copy link
Contributor

Awesome, nice quick fix @likangning93. Could you open a PR?

@dennisadams
Copy link
Contributor Author

@likangning93
I've tried your solution in a few other places and got some nice results:

  1. Adding highp in
    float czm_sampleShadowMap(sampler2D shadowMap, vec2 uv)

    fixes black stripes on texture when shadow enabled on mobile devices #7176.
  • In the shadows sandcastle, if you check the soft shadow option and decrease the shadow map size from 2048 to 256 you still get some artifacts. But I get them also on my linux so I suspect that's another issue.
  • There are more places needing a highp, such as the equivalent cubemap version czm_samplesShadowMap.
  1. Adding highp in
    uniform sampler2D u_depthTexture;

    fixes GroundPolylinePrimitive and Ground Primitive materials render poorly on newer mobile devices #6735, or at least some of the issues mentioned over there. PassThroughDepth is used for the globe depth texture.
  2. Adding highp in
    "uniform sampler2D u_texture;\n" +

    has some precision benefits. I haven't noticed any related open issue, but can elaborate if needed.
  • Consider adding highp also in executeDebugPickDepth.
  1. Other issues probably fixed by the above, although I haven't checked: KML clampToGround disappears on mobile #7273, Shadows have bad artifacts on iOS #4752.

This isn't the best place for this discussion, but I didn't want to split it across all issues.

A couple of general questions:

  1. Will all these highps hurt performance?
  2. Do we need to check if highp is supported?

I'd love if all this weirdness just turned out to be different default precisions for samplers on different devices/webgl implementations.

It may be your lucky day.

@lilleyse
Copy link
Contributor

  1. Will all these highps hurt performance?

We should profile to see, but performance is a lesser concern if highp ends up fixing these artifacts.

Do we need to check if highp is supported?

Yes, and that's something we're not checking everywhere. See https://developer.mozilla.org/en-US/docs/Web/API/WebGL_API/WebGL_best_practices#Be_precise_with_GLSL_variable_precision_annotations

@lilleyse
Copy link
Contributor

@dennisadams could you open a PR and copy your notes over there? I don't have a good device to test with but @likangning93 does.

Thanks for looking into this. I hope we can knock out as many mobile rendering bugs as possible.

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