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

Setting maxzoom on a raster-dem source fails, preventing over-zoom #9159

Closed
JeffreyEarly opened this issue Jan 4, 2020 · 8 comments · Fixed by #9789
Closed

Setting maxzoom on a raster-dem source fails, preventing over-zoom #9159

JeffreyEarly opened this issue Jan 4, 2020 · 8 comments · Fixed by #9789
Labels
bug 🐞 needs investigation 🔍 Issues that require further research (e.g. it's not clear whether it's GL JS or something else)

Comments

@JeffreyEarly
Copy link

We created a custom raster-dem for sea-surface height topography up to zoom level 4. However, we found that setting a "maxzoom: 4" completely stops mapbox from loading the tiles at all.

Specifically, if you call

map.addSource("ssh_dem",{
     type: 'raster-dem',
     tiles: ['https://ssh...'],
     tileSize: 256
});

and add a hillshade layer, then the dem will load okay if you're at the appropriate zoom. It will also overzoom if the tile was previously loaded.

However, if you set a max zoom on the source,

map.addSource("ssh_dem",{
     type: 'raster-dem',
     tiles: ['https://ssh...'],
     tileSize: 256,
     maxzoom: 4
});

the whole thing will silently fail.

As a workaround, not setting the max zoom doesn't work for us because often the user will already be overzoomed, so the tiles simply won't load.

Here are live examples,

Any help on this would be greatly appreciated.

@andrewharvey
Copy link
Collaborator

Could you share a minimal reproducable example please? I just tested this with the example at https://docs.mapbox.com/mapbox-gl-js/example/hillshade/ by adding a maxzoom at the source and it's working as expected, loading up to the maxzoom and overzooming beyond that.

@JeffreyEarly
Copy link
Author

JeffreyEarly commented Jan 5, 2020

Strange. I tried that exact sample code, added maxzoom, and it reproduced the bug perfectly.

I've tried this on various Mac running 10.15 with both Safari and Chrome. What are you trying this on?

The only change from the sample code link you provided is this:

map.addSource('dem', {
    'type': 'raster-dem',
    'url': 'mapbox://mapbox.terrain-rgb',
    'maxzoom' : 4
    });

@andrewharvey
Copy link
Collaborator

I don't think it's a browser issue. This is what happens when I change the maxzoom on the source to around 18 then 9 then 7. I have no idea why setting maxzoom: 18 and maxzoom: 10 shows different results it should be the same, so looks like there is an issue but this is different to what you're describing. But when you set it less like zoom 7 I can see a more blurred hillshade which is what you'd expect since it's using lower resolution data.

Screenshot from 2020-01-05 16-14-15
Screenshot from 2020-01-05 16-13-25
Screenshot from 2020-01-05 16-13-06

What do you see?

@andrewharvey
Copy link
Collaborator

...on second though it seems the maxzoom is affecting the intensity/scaling, but it shouldn't it should be the zoom the tiles come from that affects it, not the maxzoom. This could be the issue. It could be that setting to 4 has such little intensity that you can't see it.

@JeffreyEarly
Copy link
Author

JeffreyEarly commented Jan 5, 2020

Yes, I think you're right. If you click on my original examples, you can actually see extremely faint artifacts in the maxzoom case. So maybe it's rendering something, it's just barely visible.

@andrewharvey
Copy link
Collaborator

I still think it's a bug, changing the maxzoom on the source shouldn't change how the layer renders for zooms under the maxzoom.

@JeffreyEarly
Copy link
Author

The relevant line of code appears to be line 63 of hillshade_prepare,

    vec2 deriv = vec2(
        (c + f + f + i) - (a + d + d + g),
        (g + h + h + i) - (a + b + b + c)
    ) /  pow(2.0, (u_zoom - u_maxzoom) * exaggeration + 19.2562 - u_zoom);

which comes with a bunch of good discussion around it.

If you simply fix u_maxzoom to something like 14, then the problem goes away,

    ) /  pow(2.0, (u_zoom - 14) * exaggeration + 19.2562 - u_zoom);

Looking at the original discussion, with @nickidlugash and @mollymerp, I think it makes sense to fix this value as a "constant" in the problem.

@JeffreyEarly
Copy link
Author

JeffreyEarly commented Jan 7, 2020

Any thoughts on how to do a long-term fix on this?

For our own website I ended up hard coding u_maxzoom=22.0 into hillshade_prepare because 22 matches the default assumed maxzoom level for a source. But it would be nice to get a long term fix.

It might be helpful for the discussion to know what our custom terrain tiles are. We have sea-surface height (SSH) topography encoded into terrain-rgb tiles. In our case though, because the topographic relief is typically a maximum of +/- 80 cm depending on the day, I ended up multiplying by 10000, so typical heights match the largest mountains in the world. The hillshade algorithm is then happy to treat SSH as mountains.

@mourner mourner added bug 🐞 needs investigation 🔍 Issues that require further research (e.g. it's not clear whether it's GL JS or something else) labels Jan 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 needs investigation 🔍 Issues that require further research (e.g. it's not clear whether it's GL JS or something else)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants