maplibre / maplibre-style-spec

MapLibre Style Specification & Utilities
https://maplibre.org/maplibre-style-spec/
Other
71 stars 60 forks source link

Design Proposal: Expanding tileSources bounds #60

Open pramilk opened 1 year ago

pramilk commented 1 year ago

Design Proposal Template

Motivation

Bounds property in tile sources currently takes a bound and constraints the tiles from this bound. This is very constraining and does not provide flexibility that we need. Here are the current limitations we face.

  1. There is no way to specify a bound which should be excluded. For example in our case,

    • South Koria and Japan needs special handling and have a different tile source then the rest of the world. Since there is no support for exclusion, we add South Koria/Japan tile layers on top of rest of the world. This is causing un-necessary tile requests in this region
    • We have streetside/traffic coverage only in some portion of the world. Since exclusion is not supported, we have to download these tiles in regions which does not have any coverage.
  2. There is no way to specify multiple bounds. For example in our case

    • UK has a special imagery requirement. So inorder to only cover just UK, we have to add 3 tile sources to cover just UK (to exclude Iceland/France).
    • Also no way to provide multiple exclusions per tile source.

Proposed Change

Proposal is to expand bounds property to support inclusion and exclusion bound's along with min/max zoom for which these bounds will be applicable. Please look at below API modification for types.

API Modifications

Current TileJson type

type TileJSON = {
    tilejson: '2.2.0' | '2.1.0' | '2.0.1' | '2.0.0' | '1.0.0';
    name?: string;
    description?: string;
    version?: string;
    attribution?: string;
    template?: string;
    tiles: Array<string>;
    grids?: Array<string>;
    data?: Array<string>;
    minzoom?: number;
    maxzoom?: number;
    bounds?: [number, number, number, number];
    center?: [number, number, number];
};

Proposed TileJson with this change

type TileJSON = {
    tilejson: '2.2.0' | '2.1.0' | '2.0.1' | '2.0.0' | '1.0.0';
    name?: string;
    description?: string;
    version?: string;
    attribution?: string;
    template?: string;
    tiles: Array<string>;
    grids?: Array<string>;
    data?: Array<string>;
    minzoom?: number;
    maxzoom?: number;
    bounds?: TileJSONBounds;
    center?: [number, number, number];
};

type TileJSONBounds = [number, number, number, number] | {
    include?: Array<constrainedBounds>;
    exclude?: Array<constrainedBounds>;
};

type constrainedBounds = {
    bounds: [number, number, number, number];
    minzoom?: number;
    maxzoom?: number;
};

Alternative option with more flexibility is to support both bounds/polygon bounds.

type constrainedBounds = {
    type: "Bound" | "Polygon";
    coordinates: Array<[number, number]>;
    minzoom?: number;
    maxzoom?: number;
};

Migration Plan and Compatibility

It will be backward compatible.

wipfli commented 1 year ago

That is an interesting proposal, thanks @pramilk for writing it.

If I understand correctly, this design would allow you to make a tile pyramid where arbitrary sub-pyramids are missing. Could it not be a problem that this design is too generic?

Also if I remember correctly some people from Esri wanted to have exactly this functionality in MapLibre GL JS. Let me reach out to them...

ovivoda commented 1 year ago

@pramilk can you please add an example for the way you are using MapLibre now for case 1 and case 2? And how will you use it in the future if the proposal will be implemented. Maybe also give us a snippet of the tiles sources used now?

That way we will make sure we are on the same page, by fully understanding the use-case.

HarelM commented 1 year ago

I think this is a very interesting proposal, thanks! Two questions/comments come to my mind.

  1. Complicating bounds can lead to performance issues due to complex spatial calculations. Can we avoid it?
  2. Can't this be achieved using a tile server with the relevant tiles for the relevant area? (Maybe besides missing tiles that can be solved using simple bounds)
pramilk commented 1 year ago

If I understand correctly, this design would allow you to make a tile pyramid where arbitrary sub-pyramids are missing. Could it not be a problem that this design is too generic?

@wipfli, Yes it gives flexibility to uses who require this functionality without impacting any existing users. Please see my example /scenario in below example.

@pramilk can you please add an example for the way you are using MapLibre now for case 1 and case 2? And how will you use it in the future if the proposal will be implemented. Maybe also give us a snippet of the tiles sources used now?

That way we will make sure we are on the same page, by fully understanding the use-case.

@ovivoda, @wipfli Here is our style endpoint for reference used in below examples. Style endpoint used on Bing Maps. Here are 3 examples of where we will be able to use this API.

Use case for bounds exclusion:

Use case for multiple inclusions:

Use case for multiple inclusions:

Blue layer in this image represent coverage.

image

I think this is a very interesting proposal, thanks! Two questions/comments come to my mind.

  1. Complicating bounds can lead to performance issues due to complex spatial calculations. Can we avoid it?
  2. Can't this be achieved using a tile server with the relevant tiles for the relevant area? (Maybe besides missing tiles that can be solved using simple bounds)

@HarelM

1: There is no performance penalty (except for extra Js download/compile time) as its expands the current bounds functionality. So, if a user does not have any requirement to use this new functionality, they don't have to face perf penalty. Extra Js size should be minimal less then 50 lines.

2: Tile server can (does in our implementation) return empty tiles in these regions. But client still have to download them. For vector data, its downloaded, processed and then ignored. For Raster tiles they are downloaded, texturized and rendered.

HarelM commented 1 year ago

There was a comment in the shack channel about tilejson specification that will need to change as well, @pramilk can you please check this aspect?

wipfli commented 1 year ago

Could this design also be used with many bounds? For example say I want to show high details for train stations globally up to z18, while the rest is z14.

wipfli commented 1 year ago

Can includes and excludes be nested?

wipfli commented 1 year ago

As @HarelM said it might be that this needs an extension of the TileJSON specification, see https://github.com/mapbox/tilejson-spec. It is an open specification and I think we should aim to contribute to it. But let us first figure out how to do it here and then bring that idea to the TileJSON repo.

For completeness let me mention that MapLibre has forked the style specification and the rendering engines MapLibre GL JS and MapLibre GL Native from Mapbox. However, the TileJSON and MVT specifications have so far not been forked.

wipfli commented 1 year ago

Ah nice, @rbrundritt from Microsoft actually brought tiled tiles to the TileJSON repo back in 2019: https://github.com/mapbox/tilejson-spec/issues/58

wipfli commented 1 year ago

There are more issues in the TileJSON repo asking basically for this functionality:

https://github.com/mapbox/tilejson-spec/issues/60

https://github.com/mapbox/tilejson-spec/issues/49

wipfli commented 1 year ago

@nyurik @stepankuzmin Do you think Martin could support such a modified version of the TileJSON spec?

I am asking because I think if we change the way sources can be handled in the frontend, we should also give users the tools to create such sources with more detailed coverage bounds...

cc @msbarry from Planetiler, @bdon from Protomaps, @e-n-f from tippecanoe

bdon commented 1 year ago

A related issue recently added to felt/tippecanoe https://github.com/felt/tippecanoe/pull/82 - is bounds specifications that cross the antimeridian using the 0-360 degree range. This is an addition to existing bounds, not a replacement.

msbarry commented 1 year ago

Planetiler just writes the "bounds" field to mbtiles metadata, and tile servers like martin construct the tilejson from that, so we'd need to decide on a standard for how to encode this modified data in mbtiles for tile servers that understand this change to handle, as well as maintaining backwards compatibility for older tile servers.

Right now planetiler supports a single bounds/minzoom/maxzoom and/or a polygon file. It wouldn't be hard to extend this to support multiple regions/minzooms/maxzooms, since it the bound tests all get funneled through this one class - just need to know how to encode the metadata.

Complicating bounds can lead to performance issues due to complex spatial calculations. Can we avoid it?

Initially the polygon testing did incur a performance overhead in planetiler, but I mitigated this by adding a preprocessing step that calculates every tile that the polygon touches at each zoom level (using a RoaringBitmap) and just test against that at tile generation time.

wipfli commented 1 year ago

Thanks for pointing this out, @msbarry. Besides Martin, the other tile server that comes to mind is tileserver-gl. @acalcutt you are working a lot on tileserver-gl, do you think this proposal here could be integrated in tileserver-gl?

acalcutt commented 1 year ago

Right now, based on a quick look at the code, tileserver-gl generates it's TileJSON based on what is in the mbtiles metadata.

I think it depends how this changes the mbtiles file. I assume bounds/minzoom/maxzoom would still be in the mbtiles file, since that is part of the mbtiles-spec? Would include/exclude be new metadata values inside the mbtiles file?

as long as bounds/minzoom/maxzoom is still in the mbtiles file, I don't think this would affect tileserver. It would however need some work to support include/exclude...if that was added to the mbtiles metadata.

wipfli commented 1 year ago

Thanks for the update @acalcutt. I did not know there was also an mbtiles specification... But good to know now, so if we go forward with the tileSouces bounds proposed here we will also want to have them in TileJSON and in the MBTiles specification I think.

wipfli commented 1 year ago

Let me ping a few more people who appear to have been involved in the mbtiles issues in the past: @systemed, @pnorman, @sfkeller

pnorman commented 1 year ago

I don't see that there's any direct relationship with mbtiles here, as maplibre doesn't read mbtiles.

TileJSON is essentially unmaintained and I've wondered if it's necessary to look at forking it and transferring it to an org like OSGeo.

acalcutt commented 1 year ago

mbtiles and tilejson are related I think. all the information to generate tilejson from a mbtiles source comes from the mbtiles metadata.

That doesn't mean Tilejson/Maplibre can't support both formats, but if someone wanted to support this from a mbtiles source, this new information would need to be in the mbtiles metadata, so the tileserver could use that info to generate it's tilejson endpoints.

Just one more side note on the mbtiles-spec, it mentions bounds/minzoom/maxzoom being a requirement at some point. In a future revision of this specification, the bounds, minzoom, and maxzoom rows of the metadata table will be mandatory. https://github.com/mapbox/mbtiles-spec/blob/master/1.3/spec.md#future-directions

sjg-wdw commented 1 year ago

I really like this idea. If you've ever cobbled together data sets with holes or weird constraints, this would help.

A couple things sprang to mind. First, would it make sense to use a different keyword than "bounds" for the new format? Similar to what's being contemplated for MBTiles, I think. The overall bounds is backward compatible, but then the new bounds is elsewhere and you only see it if your software looks for it.

Second, do you have a TileJSON example? Sometimes it's easier to understand if you can see a real world example. Your map is great here for that.

acalcutt commented 1 year ago

@sjg-wdw this is more what i was thinking with the mbtiles. If you kept old bounds/maxzoom/minzoom to keep it compliant, but added some new keywords for include/exclude, then something could be done to use them on the server side if they existed and this new tilejson format could be generated.

sjg-wdw commented 1 year ago

@sjg-wdw this is more what i was thinking with the mbtiles. If you kept old bounds/maxzoom/minzoom to keep it compliant, but added some new keywords for include/exclude, then something could be done to use them on the server side if they existed and this new tilejson format could be generated.

Yes, I like this idea. Thinking back to some of the janky TileJSON parsers I may have written in the past, it seems safer to add keys rather than change them.

pramilk commented 1 year ago

Another issue worth mentioning/considering.

Include/exclude bound ranges on tile source just means that the tiles will/will not be fetched for specific bounds. Layers will still try to render them (if they are in its zoom range). So might need a corresponding change in layers spec as well if user don't want layers to render data in include/exclude tile bounds.

One of the option can be to expand the visibility property to be 'none' | 'visible' | 'constrained'. Where 'constrained' will force the layer to only render within the tile source constrain.

wipfli commented 1 year ago

My intuition is that the performance penalty for rendering empty tiles is close to zero, but I have never measured it. Do you have some numbers on this @pramilk?

sjg-wdw commented 1 year ago

Empty tiles are probably fine in most cases. But if you're doing something weird, sometimes the absence of data can be meaningful. It's best to load things precisely if you can rather than rely on empty tiles to tell you something.

How that relates to layers, though.... I think you're saying that a layer should be turned off if its source is not currently visible. But how many layers does that relate to other than background?

pramilk commented 1 year ago

Regarding performance implication of rendering empty tiles: Client still have to download them. For vector data, its downloaded, processed and then ignored. For Raster tiles they are downloaded, texturized and rendered. It does not know if its empty or has some data.

bdon commented 1 year ago

+1 to forking the unmaintained TileJSON spec

In practice I do not think the overhead of empty tiles is that important for end-user-facing map applications, simply because human users do not spend a lot of time looking at repetitive or empty parts of the world. Is there strong counterexamples?

One issue with the constrained bounds approach is that it becomes possible to express inconsistent bounds information.

vincentsarago commented 1 year ago

👋 Hi, I'm not directly involved in Maplibre but I'm working on multiple tile servers (MVT / Raster) where we provide TileJSON documents so I'll be really interested to see where this discussion goes about Open Sourcing TileJSON.

Strictly talking about the bounds property, the OGC Features Specification for the spatial extent is defined as an array of bbox with the first one being the sum of the bbox and all other as regional bbox

ref: http://schemas.opengis.net/ogcapi/features/part1/1.0/openapi/schemas/extent.yaml see: https://github.com/developmentseed/tipg/blob/main/tipg/model.py#L32-L45

wipfli commented 1 year ago

Hi @vincentsarago. Nice, which tile servers are you involved in?

wipfli commented 1 year ago

@vincentsarago feel free to join the monthly meeting today at 8 PM CEST. We will discuss this design proposal today. Link to the zoom call is in the slack channel.

vincentsarago commented 1 year ago

@wipfli mostly two:

Thanks for the invite, I'll see if I can make it

ovivoda commented 1 year ago

Notes from TSC meeting on April 12:

Action points:

pnorman commented 1 year ago

I think the work should be done in a TileJSON fork, once the spec is changed, Maplibre can use those new attributes of the spec.

wipfli commented 1 year ago

https://github.com/wipfli/tilejson-spec/pull/1 prepares a TileJSON spec fork. Since the original repository a) does not have a license file and b) the license in the readme is not MIT/BSD, I would like to get some advise on licensing before we actually bring this into the MapLibre org.