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

3d fill-extrusion-heights on iOS Safari are flat when height is over 65,536 meters #7247

Closed
levelsio opened this issue Sep 6, 2018 · 5 comments

Comments

@levelsio
Copy link

levelsio commented Sep 6, 2018

  • versionGlJs: 0.47.0

Platform: Safari iOS (and also Android 3rd-party reports)

Example Link: https://nomadlist.com/3d

Expected Behavior

Fill extrusion height is set from a features geoJSON dataset:
'fill-extrusion-height': {type: 'identity', property: 'height'},

Actual Behavior

fill-extrusion-heights in 3d ignore height on Safari iOS regardless of height value and all show as flat.

I think it's because I am showing fill-extrusion-heights on a world level, so they're quite big. And it's making some kind of integer overflow (65,536?) on iOS. I've seen Safari iOS handle integers differently before.

On any desktop browser like Safari MacOS and Chrome MacOS they show up properly as large bars.

How it looks on iOS:
photo_2018-09-07_03-52-13

How it should look (in Chrome desktop responsive mode): screen shot 2018-09-07 at 03 51 41

And how it looks on Chrome desktop:
screen shot 2018-09-07 at 03 51 50

@levelsio
Copy link
Author

levelsio commented Sep 8, 2018

It seems to be reported before:
#3954

"yes, fill-extrusion-height uses a Uint16 type, so the maximum possible value is 65535. At the moment we don't have plans to use bigger numbers as the increase in memory usage would be substantial for what seems to be a pretty limited use case — from what I've seen so far most use cases don't come near 65535 meters." by @lbud on 2017-01-13

It's stated the limit is removed:
#5338

Is it though?

@jfirebaugh
Copy link
Contributor

Duplicate of #7104.

@levelsio
Copy link
Author

Duplicate of #7104.

Sorry but this is wrong. It's a different issue. The examples in #7104 work fine on my iPhone 8 Plus in Safari iOS, but the fill-extrusion-height in my example works on no mobile devices at all, only desktop.

Please re-open the issue, thanks!

@levelsio
Copy link
Author

levelsio commented Sep 12, 2018

I've confirmed it's 65,536 height limit on mobile. So I think fill-extrusion-height is STILL defined as an Uint16, as seen in #3954. That issue was reportedly fixed and closed, but it's still Int16 on mobile somehow.

screen shot 2018-09-12 at 15 12 18

@mourner mourner reopened this Sep 18, 2018
@ChrisLoer ChrisLoer self-assigned this Sep 18, 2018
ChrisLoer added a commit that referenced this issue Sep 18, 2018
Fixes issue #7247: heights over 65,536 meters don't render on some devices.
See discussion in issue #2096.
ChrisLoer added a commit that referenced this issue Sep 18, 2018
Fixes issue #7247: heights over 65,536 meters don't render on some devices.
See discussion in issue #2096.
@ChrisLoer
Copy link
Contributor

The issue was actually an (optionally) low-precision float rather than a Int16 vs Uint16 distinction, but this should be fixed with #7292 -- please give it a try! Although I don't see a way in which this is different from #7104, so maybe I'm missing something. At any rate, I was able to reproduce using an iPhone 8.

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

No branches or pull requests

5 participants