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

Materials for Ground Primitives don't work for small areas on newer mobile devices #6739

Closed
likangning93 opened this issue Jun 27, 2018 · 13 comments · Fixed by #7421
Closed

Comments

@likangning93
Copy link
Contributor

likangning93 commented Jun 27, 2018

Sandcastle

Increasing offset to larger than 1.0 switches to spherical coordinates, which mostly works other than some depth precision issues.

Related: #6735

[EDIT] for anyone new to this issue, Cesium uses the materials-on-GroundPrimitives codepath when available for batching colored GroundPrimitives, which is why on some newer mobile platforms corridors on terrain won't work bu on earlier mobile platforms without depth texture support they will work.

@likangning93 likangning93 self-assigned this Jun 27, 2018
@likangning93
Copy link
Contributor Author

likangning93 commented Jun 27, 2018

I think it's a batch table problem.

Here's a Sandcastle with a stripped down version of the shader that just computes texture coordinates using planar distances.

Here's the same Sandcastle, but with all batch table reads replaced with hardcoded values.

The former computes texture coordinates very incorrectly on iPad.

@likangning93
Copy link
Contributor Author

likangning93 commented Jun 27, 2018

Pretty sure it's because of how high/low values are being encoded.

Sandcastle that just passes a diff between batch table read and expected from VS to FS.

Results on Desktop:
desktop

On iPad:
a7a96010-f6f6-4aab-9883-dfa69e55b54c

Haven't binary searched yet, but diff here is in the range of 980,000 to 990,000 in the R channel on our A9X iPad Pro.

@likangning93
Copy link
Contributor Author

likangning93 commented Jun 27, 2018

At @bagnell's suggestion, tried disabling floating point textures in the batch table which results in the same thing happening on Desktop. So that's interesting. The iPad should have float texture support though.

@likangning93
Copy link
Contributor Author

Maybe the iPad is actually doing the same packing thing under the hood, which might also explain the depth texture problems... that is very bad.

Regardless, I'll open a PR soon adding a check for float texture support.

@likangning93
Copy link
Contributor Author

likangning93 commented Jun 27, 2018

BTW on Desktop, using our float packing code, the diff doesn't seem to be as bad as it is on iPad.

@likangning93
Copy link
Contributor Author

^
notasbad

@likangning93
Copy link
Contributor Author

likangning93 commented Jun 27, 2018

^ that was on Windows Nvidia, it's less on Linux/Intel which is interesting.
Maybe instead of requiring float textures we can come up with something more clever to reduce the range of the floats getting packed.

screenshot_2018-06-27_19-36-57

[EDIT] that would also theoretically work for iPad, but we'd have to use feature detection to force something, which is never super satisfying. But it's better than just throwing in the towel!

@likangning93
Copy link
Contributor Author

likangning93 commented Jun 28, 2018

So I tried:

All of these seem plausible on desktop but don't produce useable results on the iPad, so we probably have to scrap the range reduction idea.

@likangning93
Copy link
Contributor Author

A good thing to have might be minimal proof of the iPad's float texture misbehavior, it'll at least help if we try to report this problem to Apple.

@likangning93
Copy link
Contributor Author

likangning93 commented Jun 28, 2018

I think I have to put this aside for a couple days at least, here's a few more notes for picking this back up:

last resort

  • spherical texture coordinates work fine, they just won't look good for small areas
  • we can add a fallback for this if nothing else works

Two more ideas to try

  • alternate packing method - go so far as to encode each float into many integer groups of sigfigs

    • -6123456.789 -> uint8: [194 or something, 23, 45, 67, 89].
    • probably lots of caveats on why you wouldn't want to do this for generic floats, but for us we know that they're distances in meters, equal or lower order of magnitude as Earth radius in meters, and only need accuracy to 2, maybe 3 decimal places
    • this is also just a fallback, Apple may yet fix this too
  • tweak Cartesian4.packFloat and czm_unpackFloat specifically for iOS

    • observe that there's a "bias" in here
    • these functions already behave differently on Intel/Linux and Nvidia/Windows
    • maybe we can tune for iOS

@likangning93
Copy link
Contributor Author

Pointed out by @mramato: https://www.khronos.org/webgl/public-mailing-list/public_webgl/1703/msg00036.php

It's nice to not be alone

@likangning93 likangning93 changed the title Materials for Ground Primitives don't work for small areas on iOS Materials for Ground Primitives don't work for small areas on mobile Dec 5, 2018
@likangning93 likangning93 changed the title Materials for Ground Primitives don't work for small areas on mobile Materials for Ground Primitives don't work for small areas on newer mobile devices Dec 5, 2018
@likangning93
Copy link
Contributor Author

A lot of users have been running into this and #6735 lately, because Cesium currently uses materials for GroundPrimitives to batch corridors when it detects the required extensions and many users consider corridors to be a legacy "fallback" for polylines on terrain.

For now we should disable both polylines on terrain and materials-on-GroundPrimitives on mobile, this way any legacy fallbacks will still work.

@Spec1alFx
Copy link

I'm one of this users;
just one detail: I found that clamped to ground polygons (corridors, rectangles and so on) are not displayed on modern devices BUT are working on my old Galaxy Note 3.

Question: polylines on terrain are a very useful (and wanted) feature and are working on mobile (although with some graphical glitches when camera is too near or too far). For my use cases it would be better to be free to choose between polylines on terrain and corridors than disable the first ones at all.

Thank you!

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.

2 participants