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

Document vector_layers extension #14

Open
pnorman opened this issue Aug 20, 2014 · 17 comments
Open

Document vector_layers extension #14

pnorman opened this issue Aug 20, 2014 · 17 comments
Assignees
Labels

Comments

@pnorman
Copy link
Contributor

pnorman commented Aug 20, 2014

We're starting to see a lot of tilejson with a vector_layers key. We should document this for standardization and so that the meaning the keys are documented.

Some examples

https://gist.github.com/springmeyer/bbf49c89180485f477b9
https://github.com/mapbox/mapbox-studio/blob/mb-pages/test/fixtures-mapboxapi/mapbox.mapbox-streets-v5.json

Edit: fixed link

@anandthakker
Copy link

Bumping this. Happy to put in a PR if yall agree/confirm that this should happen.

Assuming yes, my understanding is that the schema for vector_layers would look something like:

"vector_layers": {
  "type": "array",
  "items": {
    "type": "object",
    "properties": {
       "id": { "type": "string" }
       "description": { "type": "string" },
       "fields": { "type": "object" }
       "required": [ "id", "fields" ]
    }
  }
}

And the fields object maps field names to descriptions.

@amandasaurus
Copy link

I recently got stung by this when trying to use tessera which uses tilelive to serve up a tmstyle. If you have no vector_layers key in the TileJSON, then the resultant image is blank.

It would be helpful if the spec included this.

@pnorman
Copy link
Contributor Author

pnorman commented Jul 15, 2016

ping on this. I could also do a PR if there's agreement and understanding on what the schema is (though if @anandthakker already has something written, I'd prefer that)

@anandthakker
Copy link

@pnorman Nothing written, alas. Still willing to do it, but my queue is somewhat longer today than it was when I last commented here.

@gmaclennan
Copy link

gmaclennan commented Sep 19, 2016

It seems like the only property that mapbox-gl-js currently uses is the id. https://github.com/mapbox/mapbox-gl-js/blob/93c3b030726ff4d22dd9fbd24ce19c27c3f19093/js/style/style.js#L80-L112

Reviewing the source it looks like if that field is not present, and does not map to the layerId in the vector tile, then nothing will render.

@ARolek
Copy link

ARolek commented Sep 30, 2016

I'm currently implementing the TileJSON spec for an MVT server and would also like to see an agreed upon spec for the vector_layers key. I came up with the following proposal based on what is required by the MVT Layer Specification and some additional fields I see beneficial for displaying layers. A few things to note:

  • id and name are redundant. The MVT spec requires a layer have a name value, but as @gmaclennan pointed out, the id value is already being used by mapbox-gl. Ideally the spec would only use name as that's required for MVT Layers.
  • The fields key as proposed has been renamed to feature_tags. The MVT spec uses tags for feature metadata, and since the values are for "features" not the layer, I think the "feature_" prefix is more explicit.

Here's the spec I'm proposing:

{
    // REQUIRED. The MVT encoding version.
    "version": 2,

    // REQUIRED. The extent of the vector layer. 
    "extent": 4096,

    // REQUIRED. The name of the layer 
    // "name" and "id" are identical since mapbox has already implemented the id in mapbox-gl
    "id": "buildings",

    // REQUIRED. The name of the layer.
    // "name" and "id" are identical since mapbox has already implemented the id in mapbox-gl
    "name": "buildings",

    // OPTIONAL. Default: []
    // an array of feature tags that MAY be included on each feature
    "feature_tags": ["class", "name_en", "etc"]

    // OPTIONAL. Default: null
    // possible values include: "point", "line", "polygon", "unknown"
    "geometry_type": "polygon"

    // OPTIONAL. Default: 0. >= 0, <= 22.
    // An integer specifying the minimum zoom level.
    "minzoom": 0,

    //  OPTIONAL. Default: 22. >= 0, <= 22.
    // An integer specifying the maximum zoom level. MUST be >= minzoom.
    "maxzoom": 11
}

Looking forward to the discussion.

@pnorman
Copy link
Contributor Author

pnorman commented Oct 5, 2016

  • version and extent are specific to MVT, not vector tiles. If we were to specify format it should go a level higher in the json
  • I assume the json object you are describing is put in an array for the vector_layers property? This needs to be explicit
  • 👎 to having a duplicate name and id properties
  • null must be a possible geometry_type or leaving it undefined violates the possible values
  • what's the difference between null geometry_type and unknown?
  • what about other geometry types?

@ARolek
Copy link

ARolek commented Oct 5, 2016

version and extent are specific to MVT, not vector tiles. If we were to specify format it should go a level higher in the json

  • This brings up an interesting point. The spec needs to be polymorphic to support the various vector formats. I have drafted a new proposal at the end of the comment with this in mind.

I assume the json object you are describing is put in an array for the vector_layers property? This needs to be explicit

  • Correct. I was assuming context as the issue is about the vector_layers property, but it should be explicit.

👎 to having a duplicate name and id properties

  • I agree, which is why I suggested name but as @gmaclennan pointed out mapbox is already using the id field. We could always use id and then for MVT implementations put the name into the id field. It will probably be important to document how various formats should leverage this spec.

null must be a possible geometry_type or leaving it undefined violates the possible values

what's the difference between null geometry_type and unknown?

what about other geometry types?

  • I was referencing the MVT spec, but let's expand the available values. WKB has 18 geometry types, but I'm not sure what geometry types are supported across all vector formats. Any input on this?

Here's an additional proposal we can pick apart:

"vector_layers":[
    {
        // REQUIRED. The "id" or "name" of the layer 
        "id": "buildings",

        // REQUIRED. The vector tile type.
        "type": "mvt",

        // OPTIONAL. Default: null
        The details specific to the vector layer "type". The following example is for MVT.
        "details": {

            // REQUIRED. The MVT encoding version.
            "version": 2,

            // REQUIRED. The extent of the vector layer. 
            "extent": 4096,

            // OPTIONAL. Default: []
            // an array of feature tags that MAY be included on each feature
            "feature_tags": ["class", "name_en", "etc"]
        },

        // OPTIONAL. Default: null
        // possible values include: "point", "line", "polygon", "unknown", ... [additional values under review].
        "geometry_type": "polygon"

        // OPTIONAL. Default: 0. >= 0, <= 22.
        // An integer specifying the minimum zoom level.
        "minzoom": 0,

        //  OPTIONAL. Default: 22. >= 0, <= 22.
        // An integer specifying the maximum zoom level. MUST be >= minzoom.
        "maxzoom": 11
    }
]

@gmaclennan
Copy link

Some further info from experimenting: Mapbox Studio expects the fields property to be present, as an object with field names as keys and field descriptions as values. If this is not present the Tileset view of the webapp crashes. The style editor view works, but it is not possible to filter the layer.

@ARolek
Copy link

ARolek commented Oct 7, 2016

@gmaclennan can you provide an example? I wonder how set in stone the Mapbox implementation is? They might have just made some decisions expecting for the spec to drive some changes in the future.

@pnorman
Copy link
Contributor Author

pnorman commented Oct 9, 2016

I'm still 👎 on including the format as a property of the vector_layers object.

We should use name to be consistent with the rest of the spec, not id.

I'd like to see vector_layers contain a list of layers, each layer with a name, optional list of fields and min/max zoom where that layer MAY be found. I want to avoid requiring layers to follow a GeoJSON model of Point, LineString, Polygon or GeometryCollection, as it's not the only one that will get used.

@sfkeller
Copy link

sfkeller commented Jan 28, 2018

Sorry for insisting but e.g. #29 is closed just because it's referring to this overdue open issue.

Allow me to make an understatement: The lack of support from Mapbox of the TileJSON and the mbtiles specs. for vector tiles contrasts IMHO little bit to its PR activities and $164 million funding.

Can someone from Mapbox tell us who is the maintainer of this mentioned specs.?

IMHO many metadata discussions could be much more efficient if specs. would be updated. See currently openmaptiles/openmaptiles#379 (comment) , or geometalab/Vector-Tiles-Reader-QGIS-Plugin#170 , or systemed/tilemaker#102 .

Recently hope was raising since there's a pull request for mbtiles v1.3 mapbox/mbtiles-spec#46 (comment) .

This hope was put in question when @flippmoke wrote:

After some discussion with @ericfischer and @springmeyer on this issue
last week we have decided to hold off on this until after #47 is complete.

and @pnorman answered

I mentally gave up on the mbtiles and tilejson specs a few months ago

This frustration is the same reason for me hesitating to contribute. But I just don't want to give up yet!

@e-n-f
Copy link

e-n-f commented Jan 30, 2018

Sorry for the long delay on spec revisions @sfkeller. I think mapbox/mbtiles-spec#47 is ready to merge to describe the current reality. Please let me know if you have any significant objections to what is there.

@sfkeller
Copy link

@ericfischer : Thanks. We're reviewing the spec revision now. It's really not just me waiting for this update (also of TileJSON) it's for the sake of innovation based on established standards, see e.g. this from @tomchadwin https://twitter.com/tomchadwin/status/958646784439607296 .

@sfkeller
Copy link

sfkeller commented Feb 6, 2018

I'm glad to see mbtiles 1.3 released! https://github.com/mapbox/mbtiles-spec
Now I hope there will be soon a similar update of this TileJSON spec.!

@mapsam
Copy link
Member

mapsam commented Apr 3, 2018

Hey y'all, thanks for the back-and-forth here. Our plan is to include vector_layers in the next version of the specification (version 3). Since this key is clearly being used in Mapbox and other implementations, it needs to be documented. @GretaCB and myself will comment back here as we start drafting the next version.

@mapsam mapsam added the v3 label Apr 3, 2018
@mapsam
Copy link
Member

mapsam commented May 4, 2018

Per discussion with @ericfischer - the current plan in documenting the structure of vector_layers is to take the text from the mbtiles spec and move it into this specification as a source of truth, therefore the mbtiles-specification can point directly to version 3.0 here, rather than documenting the same thing in two places.

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

No branches or pull requests

9 participants