search

tangrams / tangram

WebGL map rendering engine for creative cartography
https://tangram.city
MIT License
2.22k stars 290 forks source link

Meetar/overzoom #685

Closed meetar closed 5 years ago

meetar commented 5 years ago

This PR adds an zoom_offset parameter to data source specifications, to allow manual adjustment of the internal zoom_bias property.

In circumstances when the default tiles are either too large or detailed for the current application, this parameter allows the user to request lower zooms for each zoom level, reducing tile size and visual detail.

Examples:

sources:
  tilezen:
    type: TopoJSON
    url: https://tile.nextzen.org/tilezen/vector/v1/512/all/{z}/{x}/{y}.topojson
    tile_size: 512
    zoom_offset: 3
    url_params:
        api_key: global.api_key
sources:
  stamen:
    type: Raster
    url: http://a.tile.stamen.com/terrain-background/{z}/{x}/{y}.jpg
    zoom_offset: 2
nvkelso commented 5 years ago

Neat!

We already use max_zoom in the sources – is overzoom or over_zoom more consistent? Or should we use another term like zoom_offset or?

What would the new documentation read like for overzoom? I think of it more as a zoom_offset where I'm at zoom X but I'm going to request zoom X-y tiles to display at X zoom (as overzoomed). Is that how you're thinking about it?

Here's the reference for existing max_zoom:

max_zoom
Optional integer. Default is 18.

Sets the highest zoom level which will be requested from the datasource.
At higher zoom levels, the data from this zoom level will continue to be displayed,
a condition called "overzoom".

There is no corresponding min_zoom parameter, for reasons of performance.
tallytalwar commented 5 years ago

Is this the same zoom offset we were discussing @nvkelso? If so then yes, I prefer zoom_offset.

meetar commented 5 years ago

Yep, agreed re: zoom_offset 👍

On Dec 6, 2018, at 12:14 PM, Varun notifications@github.com wrote:

Is this the same zoom offset we were discussing @nvkelso? If so then yes, I prefer zoom_offset.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

meetar commented 5 years ago

If the map is zoomed out such that the current zoom - zoom_offset < the source's min_zoom, the min_zoom tiles are reloaded and rebuilt every time the zoom level changes – still looking into this.

bcamper commented 5 years ago

@meetar @nvkelso This is an interesting feature for reducing feature/raster resolution. If the primary use case is reducing network bandwidth for terrain shading, here are some interesting findings that suggest we could also just down-sample to use 256px normal tiles (while keeping the tile size 512px, meaning it is loading tiles at half-pixel resolution), which does not need any additional functionality.

Some Walkabout numbers, 1300x650 viewport:

Pacific NW at 6.666666666666669/46.608586122368365/-117.93289320122385

Washington state at 9.166666666666666/48.545903372270175/-120.71586691652305

The second two options are pixel identical. They load the same normal data but in two different ways. The 256px option doesn't save any tile requests, but loads less (in some cases considerably less) data overall, because it's keeping closer to the size of the viewport.

That technique could be extended by creating smaller tiles, e.g. 128px or 64px normal tiles, and loading those to cover the 512px tile space (the actual texture loaded can be any size, it just gets scaled to fit the expected raster tile size). This is more intensive compared to just changing a parameter of course, but also can achieve impressive results with current functionality.

It's also not mutually exclusive with adding this feature though, which is more flexible, if we want that control or have other use cases in mind (down-sampling vector data).

nvkelso commented 5 years ago

Clever to mix and match the 512 and 256 raster tile endpoints in this way!

I think it lessens the need to add support for zoom_offset, but zoom_offset is still the more generic way of solving the problem (as not all raster sources will support this server side trick).

bcamper commented 5 years ago

That was my sense too. The implementation is compact here.

nvkelso commented 5 years ago

Nice!

@bcamper is there anything more to review / change with this PR then?

bcamper commented 5 years ago

Not really. I may rename a couple related things to reflect the new usage of Tile. normalizedCoordinate() here. Otherwise I will tag this for a 0.17 release. My work on untiled rasters is the other current candidate for that.

meetar commented 5 years ago

Still seeing unnecessary tile unloading and rebuilding when the current zoom is less than the zoom_offset – I assume this has something to do with the overzoom proxy tile capability not recognizing the currently-displayed tile as the requested zoom.

bcamper commented 5 years ago

Still seeing unnecessary tile unloading and rebuilding when the current zoom is less than the zoom_offset – I assume this has something to do with the overzoom proxy tile capability not recognizing the currently-displayed tile as the requested zoom.

K, can you post an example?

meetar commented 5 years ago 
sources:
    stamen-terrain:
        type: Raster
        url: http://a.tile.stamen.com/terrain-background/{z}/{x}/{y}.jpg
    nextzen-osm:
        type: TopoJSON
        url: https://tile.nextzen.org/tilezen/vector/v1/256/all/{z}/{x}/{y}.topojson
        rasters: [stamen-terrain] # attach stamen terrain
        zoom_offset: 3

layers:
    terrain:
        data: { source: nextzen-osm, layer: earth } # render earth layer from vector data source
        draw:
            raster:
                order: 0 # draw on bottom

Result:

offset-proxy

[Edit:] This occurs even without the attached raster.

bcamper commented 5 years ago

Yep, good catch. Looks like this is actually an existing bug which can be seen at z0 when using 512px tiles (since that's the same as a zoom_offset of 1 with 256px tiles). But the higher zoom offsets are making it more obvious. I know the area that needs to be adjusted in the proxy finding logic (in TilePyramid). It won't look past the z0 coordinate tile, but in these cases, there are multiple tiles at the z0 coord level (one per style zoom, e.g. a z0 data tile styled for display at zoom 1). There is similar logic on the other end to handle style zooms past max_zoom. I'll take a look.

bcamper commented 5 years ago

This has ended up being significantly more complicated, to ensure that the relative zoom "downsampling" (effected by both tile_size and zoom_offset parameters) is applied correctly when a raster source is attached to another source. (The prior solution was downsampling the raster zoom_offset 2x, because it had already been applied once upstream when the geometry was created... but when the raster is attached to another source, it is the attaching source's zoom params that are applied.) This should be addressed here, but it's a fair amount of new logic so should be tested carefully (and increases the weight of this feature, though also improves logging/warning for existing cases where raster sources would be silently downsampled to match a vector source, e.g. if they had different tile_size values): https://github.com/tangrams/tangram/pull/685/commits/d0b352d8a6c0dab425afdd6b5914298da219624e

bcamper commented 5 years ago

I should clarify , my last comment is unrelated to the proxy tile bug mentioned in https://github.com/tangrams/tangram/pull/685#issuecomment-446839813, that was resolved in https://github.com/tangrams/tangram/pull/685/commits/b8a77e90b415969c22115d0563b1ef3ad76c4dbe.

But I discovered another bug with raster zoom_offset being applied twice when addressing that issue.