-
Notifications
You must be signed in to change notification settings - Fork 99
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
1.3 - Attempt to document the de facto requirements for mbtiles #47
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You know better than I the intricacies of de defacto usage these days, so I'm mostly just reviewing grammar and formatting here.
README.md
Outdated
@@ -51,7 +54,8 @@ for presentational data, like rendered map tiles. | |||
|
|||
One MBTiles file represents a single tileset, optionally including grids | |||
of interactivity data. Multiple tilesets - layers, or maps in other terms, | |||
can be represented by multiple MBTiles files. | |||
can be represented by multiple MBTiles files. (But vector tiles within a single | |||
MBTiles file can contain multiple layers.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd lose the parens here since it's a freestanding sentence. Also the But
.
README.md
Outdated
## 1.3 | ||
|
||
* Information added about vector tiles | ||
* Metadata revised to reflect de facto usage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd add periods to follow the other versions.
README.md
Outdated
* **Development** - NOT USABLE: [1.2](https://github.com/mapbox/mbtiles-spec/blob/master/1.2/spec.md) | ||
* **Stable**: [1.1](https://github.com/mapbox/mbtiles-spec/blob/master/1.1/spec.md) | ||
* **Stable**: [1.3](https://github.com/mapbox/mbtiles-spec/blob/master/1.3/spec.md) | ||
* NOT USABLE: [1.2](https://github.com/mapbox/mbtiles-spec/blob/master/1.2/spec.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we elaborate on why 1.2 is not usable? I'm not sure myself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure it is usable
## Abstract | ||
|
||
MBTiles is a specification for storing tiled map data in | ||
[SQLite](http://sqlite.org/) databases for immediate usage and for transfer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for efficient transfer
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't a wording change - I prefer this existing wording because it describes what the spec does.
1.3/spec.md
Outdated
One other row is **required** if the `format` is `pbf` or `mvt`: | ||
|
||
* `json`: Lists the layers that appear in the vector tiles and the names and types of | ||
the attributes of features that appear in those layers. See below for more detail. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anchor link to "below"?
1.3/spec.md
Outdated
|
||
### Example | ||
|
||
A vector tileset that contains United States counties and primary roads from TIGER might |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Link TIGER?
1.3/spec.md
Outdated
* `type`: `overlay` | ||
* `version`: `2` | ||
* `scheme`: `tms` | ||
* `json`: `{"vector_layers": [ { "id": "tl_2016_us_county", "description": "Census counties", "minzoom": 0, "maxzoom": 5, "fields": {"ALAND": "Number", "AWATER": "Number", "COUNTYFP": "String", "GEOID": "String", "MTFCC": "String", "NAME": "String", "NAMELSAD": "String", "STATEFP": "String"} }, { "id": "tl_2016_us_primaryroads", "description": "Census primary roads", "minzoom": 0, "maxzoom": 5, "fields": {"FULLNAME": "String", "LINEARID": "String", "MTFCC": "String", "RTTYP": "String"} } ] }` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is a pseudo-table anyway, maybe pretty format the json
field for legibility?
Looks good. I will take a crack at the formatting, too. |
To pretty-print the JSON within a Markdown bulleted list, try:
|
Yeah, exactly. See here: https://gist.github.com/incanus/969d3cc9fd1f3a66263b33d90ff3ce68 |
Thanks. I guess that's clear enough that it's meant to be part of the bullet. |
I'd rather abandon this in favour of 2.0. |
@pnorman I agree that there's no need for this separate PR to exist if the 2.0 spec ends up containing all the necessary information to create working mbtiles files. |
@ericfischer could you please also provide a diff to the previous version? |
@kkaefer I don't know how to make github show a diff between two files with different names, but here it is as a gist: https://gist.github.com/ericfischer/121213249c8d87d632fb89579ba54672 |
It's best to do what I did and copy old_ver to new_ver in one commit then change new_ver so you can do a diff between the tip of the branch and the commit that copies. e.g. 14942c6...pnorman:2.0 |
Great idea @pnorman. I'll make a new branch that does that. |
8c04d1b
to
062347e
Compare
Thanks again @pnorman. @kkaefer, the diff is c689a6a...vector-revision. |
1.3/spec.md
Outdated
`pbf` and `mvt` both refer to zlib-deflated vector tile data in | ||
[Mapbox Vector Tile](https://github.com/mapbox/vector-tile-spec/) format. | ||
Historically, vector tilesets have been written with `pbf` as the `format`, | ||
but version 2 and beyond of the Mapbox Vector Tile spec prefer `mvt`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe explicitly mention that they should be treated as identical.
1.3/spec.md
Outdated
The `tile_data blob` column contains raw image data in binary. | ||
Note that the Y axis is reversed from the coordinate system commonly used in the URLs | ||
to request individual tiles, so the tile commonly referred to as 11/327/791 is inserted as | ||
`zoom_level` 11, `tile_column` 327, and `tile_row` 1256, since 1256 is 2^11 - 1 - 791. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This schema is known as "TMS"
1.3/spec.md
Outdated
The row contains a single JSON object with a `vector_layers` key, whose value is an array of layers. | ||
Each layer is a JSON object with the following keys: | ||
|
||
* `id`: The layer ID, used for a [CartoCSS selector](https://tilemill-project.github.io/tilemill/docs/guides/selectors/) or [Mapbox GL Style layer](https://www.mapbox.com/mapbox-gl-style-spec/#layers). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a string?
1.3/spec.md
Outdated
|
||
* `id`: The layer ID, used for a [CartoCSS selector](https://tilemill-project.github.io/tilemill/docs/guides/selectors/) or [Mapbox GL Style layer](https://www.mapbox.com/mapbox-gl-style-spec/#layers). | ||
* `description`: A human-readable description of the layer's contents (or empty string if not available). | ||
* `fields`: A JSON object whose keys and values are the names and types of attributes available in this layer. The type should be `Number`, `Boolean`, or `String`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it a string "Number"
?
Thanks @kkaefer. I hope these latest revisions address your comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel #46 is a much better approach, so haven't given this an in-depth edit, but I have a number of concerns
- The proposed 1.3 is incompatible with de-facto usage, because it
- has implementation-specific details like a json field which not all producers or consumers need or use
- Prohibits o5m, topojson, and geojson vector tiles, all of which have been or are likely to be seen in the wild
- Has implementation specific index details that are incompatible with some implementations
- It is incompatible with storing gzip compressed MapBox vector tiles, a common de-facto use
- Maintains confusion on what precisely is a requirement with no use of RFC 2119 keywords or similar
1.3/spec.md
Outdated
are tagged with a `format` of `pbf`, which should be treated as | ||
a synonym for `application/vnd.mapbox-vector-tile`. | ||
|
||
Four rows in `metadata` are **suggested** and, if provided, may enhance performance: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not clear how these keys relate to performance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Previous versions of the spec included this performance reference but I am happy to get rid of it.
1.3/spec.md
Outdated
* `type`: `overlay` or `baselayer` | ||
* `version`: The version of the tileset, as a plain number. | ||
This refers to a revision of the tileset itself, not of the MBTiles specification. | ||
* `scheme`: `tms`, to make the tiling scheme explicit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tms is the only allowed scheme, so what's the point of a metadata item which only has one allowed value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I mentioned this only because someone filed a ticket asking for it, but I should not be inventing fields when my only goal is to document what existing implementations do.
1.3/spec.md
Outdated
For new tilesets, the `format` should be a MIME type: for example, | ||
`image/png` or `image/jpeg` for PNG or JPEG image data, or | ||
`application/vnd.mapbox-vector-tile` for | ||
[Mapbox Vector Tile](https://github.com/mapbox/vector-tile-spec/) data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Listing formats feels like a shoutout to particular formats
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. This line can go.
1.3/spec.md
Outdated
in existing tilesets and should be treated as synonyms for | ||
`image/png` and `image/jpeg`. Many existing Mapbox Vector Tile tilesets | ||
are tagged with a `format` of `pbf`, which should be treated as | ||
a synonym for `application/vnd.mapbox-vector-tile`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👎 to adding more special cases where there isn't cause in the previous version of the spec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a lot of existing vector tilesets from Mapbox Studio Classic and Tippecanoe that have a metadata row saying format=pbf. My only goal with this PR is to document what existing implementations do.
1.3/spec.md
Outdated
|
||
It is common to create an index for this table: | ||
|
||
CREATE UNIQUE INDEX name on metadata (name); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Earlier the spec states "Note: the schemas outlined are meant to be followed as interfaces.", so I don't think implementation specific details like this should go in here. An index is also not needed for performance because the metadata table will be under a dozen rows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mentioned this index because node-mbtiles and mbutil do it. My only goal with this PR is to describe what existing implementations do.
1.3/spec.md
Outdated
Each layer is a JSON object with the following keys: | ||
|
||
* `id`: The layer ID, used for a [CartoCSS selector](https://tilemill-project.github.io/tilemill/docs/guides/selectors/) or [Mapbox GL Style layer](https://www.mapbox.com/mapbox-gl-style-spec/#layers). | ||
* `description`: A human-readable description of the layer's contents (or empty string if not available). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not omit if not available?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that the implementation requires it to be present even if empty. I should verify that this is actually true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Verified that the description is not actually required.
1.3/spec.md
Outdated
The layer object may also contain these keys: | ||
|
||
* `minzoom`: The lowest zoom level whose tiles this layer appears in. | ||
* `maxzoom`: The highest zoom level whose tiles this layer appears in. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How these interact with minzoom and maxzoom of the metadata should be documented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Thanks. Will clarify.
1.3/spec.md
Outdated
have the following metadata table: | ||
|
||
* `name`: `TIGER 2016` | ||
* `format`: `mvt` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be a mime type for the example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Thank you.
## Abstract | ||
|
||
MBTiles is a specification for storing tiled map data in | ||
[SQLite](http://sqlite.org/) databases for immediate usage and for transfer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't a wording change - I prefer this existing wording because it describes what the spec does.
1.3/spec.md
Outdated
|
||
CREATE TABLE tiles (zoom_level integer, tile_column integer, tile_row integer, tile_data blob); | ||
|
||
It is common to create an index for this table: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this table normally a view, making it rare for this to be the case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's a table in mbutil and tippecanoe and a view in node-mbtiles. I didn't realize it was often a view because I was following mbutils (and previous versions of this spec).
1.3/spec.md
Outdated
to summarize what layers are available in the tiles and what attributes are available for the | ||
features in those layers. | ||
|
||
The row contains the string representation of a JSON object with a `vector_layers` key, whose value is an array of layers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specify string encoding as UTF-8?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe UTF8 is the sole legal JSON encoding.
( language neutrality , multi-language support ) I added my opinion to #46 (comment) and probably also relevant for this spec. |
cf9c3ff
to
8539170
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're back to no longer documenting de-facto requirements and prohibiting o5m, topojson, and geojson vector tiles. Mbtiles of these exist, and the de-facto requirements need to reflect that.
…ded CONTRIBUTING.md process. Updated the wording of several statements in 1.3 of the spec. Clarified the use of RFC 2119.
1.3/spec.md
Outdated
|
||
The database MUST contain a table or view named `metadata`. | ||
|
||
This table or view MUST yield exactly two columns named `name` and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should specify the types of the columns here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @flippmoke. This and your other comments should be fixed in dc659f4
1.3/spec.md
Outdated
The metadata table is used as a key/value store for settings. It MUST contain these two rows: | ||
|
||
* `name`: The human-readable name of the tileset. | ||
* `format`: The file format of the tile data: `pbf`, `jpg`, `png`, or a MIME type for other formats. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is "MIME type" the correct thing here? If so should we specify MIME types RFC and/or link https://www.iana.org/assignments/media-types/media-types.xhtml ?
1.3/spec.md
Outdated
|
||
The database MAY have tables named `grids` and `grid_data`. | ||
|
||
The `grids` table MUST contain four columns, named `zoom_level`, `tile_column`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we specify the types of the columns here?
1.3/spec.md
Outdated
|
||
CREATE TABLE grids (zoom_level integer, tile_column integer, tile_row integer, grid blob); | ||
|
||
The `grid_data` table MUST contain five columns named `zoom_level`, `tile_column`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Column types specified here as well?
1.3/spec.md
Outdated
* `minzoom`: The lowest zoom level whose tiles this layer appears in. | ||
* `maxzoom`: The highest zoom level whose tiles this layer appears in. | ||
|
||
The `minzoom` SHOULD be greater than or equal to the tileset's `minzoom`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why SHOULD here instead of MUST?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, there's no good reason that a layer's zoom levels should go beyond the tileset's.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might also add wording such as "If minzoom
exists..."
area covered by all zoom levels. The bounds are represented as `WGS 84` | ||
latitude and longitude values, in the OpenLayers Bounds format | ||
(left, bottom, right, top). For example, the `bounds` of the full Earth, minus the poles, would be: | ||
`-180.0,-85,180,85`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume the -180.0,-85,180,85
is supposed to be web mercator format. However, should the bounds actually be -180.0,-85.06,180.0,85.06
which is ESPG:3857
?
From what I see, vector_layers is mandatory but bounds (extent), minzoom and maxzoom are optional. I'm aware of reasons to make them optional, but on the other hand, that's exactly the raison-d'etre having this metadata. Clients now have to scan the data to extract the required metadata by themselves which implies delays in order of seconds to minutes. There are even situations where the extraction e.g. of bounds is impossible for a client: Look at the In addition when comparing with OGC WMS/WMTS, bounds are mandatory there too. I therefore suggest to make at least bounds mandatory, if possible also minzoom and maxzoom. |
@sfkeller I think I agree that it should be required in That said, I do want to key in on something as we write this though, we need to be careful how we term optional and required in I think making this ideal should be shifted to |
I have checked the OSM QA TILES metadata sqlite> select * from metadata;
scheme|tms
basename|hungary.mbtiles
id|31012018194303.planet
filesize|34391547904
name|hungary.mbtiles
description|hungary.mbtiles
version|2
minzoom|12
maxzoom|12
center|19.2041015625,47.171156436539384,12
bounds|16.171875,45.767522962149904,22.236328125,48.574789910928864
type|overlay
format|pbf
json|{"vector_layers":[{"id":"osm","description":"","minzoom":12,"maxzoom":12,"fields":{}}]} and as I see - it has a following
imho: the |
Isn't that exactly the idea of a specification? The client then acts accordingly to |
@mnboos frankly, that is the way it should be. However, development outpaced the specification and existing data in a format exists. I do not want to invalidate a large amount of existing users files if possible suddenly. This is Mapbox's fault. We should have updated the spec as needed. |
@flippmoke Can you enumerate which existing (Mapbox) software and data would not be aligned with mandatory/required vector_layers, bounds (extent), minzoom and maxzoom? |
@sfkeller -- just to clarify, I am all for making it required in this version (interested in @ericfischer 's view however). I just wanted to make sure it was clear about the intention of this release, so a flood of other review items would not be brought up as to how it should be written. This difference is why we decided to go with |
So you say that there exist large amounts of mbtiles user files that have no bounds, minzoom nor maxzoom? I'm astonished about this and somehow left in the dark which (Mapbox) software does not produce these metadata items. Anyhow. How about this:
I hope that this processes won't be delayed again. And I suggest that a future spec.s won't be too ambitious since I fear, it could be then blocked by discussions about other far more fundamental additions. |
@sfkeller to be clear, I have no idea if it does or does not. If it does not we should fix this immediately I would think. I agree that we should make the language here to be required. I do not interact with this spec or its software almost at all. I am simply working to help facilitate this document to completion.
I hope not as well. I know that @ericfischer is on travel currently but will look into this on Monday when he is back. My goal is that Monday/Tuesday we can have this completed and merged.
I have no idea how ambitious the future spec should be here. |
ead2a4e
to
1970e38
Compare
Thanks @ericfischer and @flippmoke and all for the release 1.3! I've helped out for quite some time to standardize - actually since OGC & ISO 19000 started. But I've never seen such an immediate response like this after this release (see e.g. https://twitter.com/richardf/status/960802937860747264). Now let's keep the pace and move 1. to sync TileJSON spec. (c.f. mapbox/tilejson-spec#14 (comment)) and 2. to MBTiles spec. 2.0. |
@tmcw @willwhite @kkaefer @incanus
cc @migurski