Fix highp samplers issues

[dennisadams]

Fixes #6735
Fixes #4752
Fixes #7273
#7176 is partly fixed

@likangning93 observed in #8311 that "devices are free to select the precision of sampler2D when it isn't explicitly set". Following,
#8805 and #9060 explicitly set highp for samplers in batch tables and clipping planes, respectively, to solve some issues.

As mentioned in my comment in #9023, I tried adding more highps across the code and had some success:

1. Adding highp in

   cesium/Source/Shaders/Builtin/Functions/shadowDepthCompare.glsl

   Line 7 in 67a36c7

   float czm_sampleShadowMap(sampler2D shadowMap, vec2 uv)

   
   solves black stripes on texture when shadow enabled on mobile devices #7176.
   Edit: black stripes on texture when shadow enabled on mobile devices #7176 seems fixed on Android devices, but some iPads still have the issue. See comments below.

• 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 likely more places across the shadow map code needing a highp.

2. Adding highp in

   cesium/Source/Shaders/PostProcessStages/PassThroughDepth.glsl

   Line 1 in 67a36c7

   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.
3. Adding highp in

   cesium/Source/Scene/PickDepth.js

   Line 119 in 67a36c7

   "uniform sampler2D u_texture;\n" +

   
   has some precision benefits. I haven't noticed any related open issue, but can elaborate if needed.
4. Added highp also in executeDebugPickDepth and executeDebugGlobeDepth.

TODO:

• Check which other samplers can do with highp.
• Check if other open issues are solved by this PR. Strong candidates are KML clampToGround disappears on mobile #7273, Shadows have bad artifacts on iOS #4752 and a number of other iOS issues.
• Profile
• Check highp support as noted by @lilleyse. Replace by mediump if not supported.
• Update CHANGES.md

@likangning93, @lilleyse your feedback will be appreciated.

[cesium-concierge]

Thanks for the pull request @dennisadams!

• ✔️ Signed CLA found.
• ❔ CHANGES.md was not updated.
  • If this change updates the public API in any way, please add a bullet point to CHANGES.md.
• ❔ Unit tests were not updated.
  • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

• Cesium Viewer works.
• Works in 2D/CV.
• Works (or fails gracefully) in IE11.

[dennisadams]

Some thoughts and updates:

I played a bit with some post process stages and with classification and didn't notice any relevant precision problem.

@likangning93 can you confirm this solves the mentioned issues?
Do you have iOS devices for testing? I don't, but it would be great to know if some open iOS issues are solved.

I tried to see what's the right way to check highp support. It seems that using gl.getShaderPrecisionFormat for HIGH_FLOAT and HIGH_INT isn't really reliable on iOS. (@likangning93 is that the reason for the manual checkFloatTexturePrecision in the reverted #7421?)
Anyway, I thought a simple solution can be to add a #define highp or #define highp mediump inside the #else here:

cesium/Source/Renderer/ShaderSource.js

Lines 240 to 244 in 3e8a6cc

	#ifdef GL_FRAGMENT_PRECISION_HIGH\n\
	precision highp float;\n\
	#else\n\
	precision mediump float;\n\
	#endif\n\n";

Any thoughts or better ideas?

[lilleyse]

@dennisadams I can confirm this fixes #7176 #6735 #4752 and probably #7273 on a Lenovo IdeaPad Duet Chromebook.

We should be able to test iPad Pro and iPad 4 soon.

[lilleyse]

Anyway, I thought a simple solution can be to add a #define highp or #define highp mediump inside the #else here:

Yeah that could work. Otherwise just the simple approach works too:

#ifdef GL_FRAGMENT_PRECISION_HIGH
float czm_sampleShadowMap(highp sampler2D shadowMap, vec2 uv)
#else
float czm_sampleShadowMap(sampler2D shadowMap, vec2 uv)
#endif
{
#ifdef USE_SHADOW_DEPTH_TEXTURE
    return texture2D(shadowMap, uv).r;
#else
    return czm_unpackDepth(texture2D(shadowMap, uv));
#endif
}

[dennisadams]

@dennisadams I can confirm this fixes #7176 #6735 #4752 and probably #7273 on a Lenovo IdeaPad Duet Chromebook.

We should be able to test iPad Pro and iPad 4 soon.

Great. I've added #4752 to the 'fixes' list meanwhile.

Yeah that could work. Otherwise just the simple approach works too:

Yes, that's simple and clear as long as it doesn't become too messy.
In any case, should we fallback to a mediump or leave it unspecified?

[lilleyse]

Everything looks good on the iPad Pro and iPad 4... except #7176 which is interesting because the other shadow bug #4752 looks fine

[lilleyse]

In any case, should we fallback to a mediump or leave it unspecified?

Does mediump vs. lowp make a difference visually? Or are they both broken looking?

[dennisadams]

Does mediump vs. lowp make a difference visually? Or are they both broken looking?

On my device they are identically broken, but that's probably hardware-dependent.

[dennisadams]

Everything looks good on the iPad Pro and iPad 4... except #7176 which is interesting because the other shadow bug #4752 looks fine

That's weird.
A bit of a long shot, but can you try adding highps also in the other functions in shadowDepthCompare.glsl and shadowVisibility.glsl?

Another thing I just noticed and might be related is that in

float czm_sampleShadowMap(sampler2D shadowMap, vec2 uv)
{
#ifdef USE_SHADOW_DEPTH_TEXTURE
    return texture2D(shadowMap, uv).r;
#else
    return czm_unpackDepth(texture2D(shadowMap, uv));
#endif
}

if I force the second option (even though USE_SHADOW_DEPTH_TEXTURE is defined) the shadows look good, even without highp. Can you try that as well?

[lilleyse]

A bit of a long shot, but can you try adding highps also in the other functions in shadowDepthCompare.glsl and shadowVisibility.glsl?

Unfortunately no luck with this

[lilleyse]

@dennisadams so I think what's left here is:

• Provide fallback if highp isn't supported. The #define highp mediump approach in Fix highp samplers issues #9064 (comment) is good with me.
• Update CHANGES.md
• Add KML clampToGround disappears on mobile #7273 to the "Fixes" list
• Keep black stripes on texture when shadow enabled on mobile devices #7176 open and investigate later
• Keeping looking into Artifacts in classification example in 1.70? #8953 (@IanLilleyT has been looking into this but the more help the better)

[dennisadams]

@lilleyse I've addressed your comments.
I would've liked to help with the debugging if only I had an iPad.

By the way, do we need to add anything to modernizeShader.js for dealing with the precision annotations?

[lilleyse]

I fixed a small bug in 274542b which revealed a larger issue in #817 that's out of scope for this PR.

This is a long-awaited fix and I'm sure @likangning93 will be very happy. Thanks @dennisadams!

[lilleyse]

By the way, do we need to add anything to modernizeShader.js for dealing with the precision annotations?

Nope, works fine in both WebGL1 and WebGL2.

[dennisadams]

Great, thanks @lilleyse!