Cesium: Clipping planes weird behavior on certain Android devices

[dennisadams]

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: https://github.com/CesiumGS/cesium/blob/952ad67d8899b8a8da161efb02ca0a88d20c616b/Source/Shaders/Builtin/Functions/unpackFloat.glsl#L21
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]

Thanks for the detailed write up @dennisadams

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

[likangning93]

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: https://github.com/CesiumGS/cesium/commit/dacc45254559237ed68945c69a3b66a423dadc28#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 https://github.com/CesiumGS/cesium/pull/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]

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|
|---|---|
|||

[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]

@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]

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

[dennisadams]

@likangning93
I've tried your solution in a few other places and got some nice results:
1. Adding highp in https://github.com/CesiumGS/cesium/blob/67a36c7cf55e2219c9053ed278b1bbb0c4d1feb3/Source/Shaders/Builtin/Functions/shadowDepthCompare.glsl#L7
fixes #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.
2. Adding highp in https://github.com/CesiumGS/cesium/blob/67a36c7cf55e2219c9053ed278b1bbb0c4d1feb3/Source/Shaders/PostProcessStages/PassThroughDepth.glsl#L1
fixes #6735, or at least some of the issues mentioned over there. PassThroughDepth is used for the globe depth texture.
3. Adding highp in https://github.com/CesiumGS/cesium/blob/67a36c7cf55e2219c9053ed278b1bbb0c4d1feb3/Source/Scene/PickDepth.js#L119
has some precision benefits. I haven't noticed any related open issue, but can elaborate if needed.
- Consider adding highp also in executeDebugPickDepth.
4. Other issues probably fixed by the above, although I haven't checked: #7273, #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]

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]

@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.