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

version 3.0.0 #36

Merged
merged 36 commits into from
Aug 18, 2021
Merged

version 3.0.0 #36

merged 36 commits into from
Aug 18, 2021

Conversation

mapsam
Copy link
Contributor

@mapsam mapsam commented May 1, 2018

This is the main branch for version 3.0.0 development of the tilejson specification. Descriptions of the scope of v3 can be found in #35. All open tickets that are under discussion for v3 are labelled as v3.

Lead authors are @mapsam and @GretaCB

We'll be making separate pull requests for each field into this branch so we can keep discussions about individual properties properly located. If you have comments about the general structure of the 3.0.0 specification, feel free to leave them here!

TODO - this list will be updated as we make pull requests and merge into this branch

Overall

Notes/additions/updates

  • add note about what "Default" means in Structure section attribution, name, tilejson, version, description #37 (comment)
  • consistify usage of "keys" or "fields" across the spec - currently the specification uses keys
  • consistify references to "set of tiles" - avoid using "tileset"
  • "key-value" vs. "key value"
  • clearly define the word "implementation" (per feedback from @pnorman)
  • add section note about how "interactivity" for GL-based maps is not represented by the TileJSON specification, and relates to older map formats grids #44 (comment)
  • double check usage of "integer" and examples using "10" strings.
  • consistify all usages of the word "tilejson"
  • clarify that tile URL must be absolute per tiles Property should clearly highlight if relative urls are supported or not.  #51
  • clarify if bounds left can equal bounds right (i.e. a single point in a tileset [0.0, 0.0, 0.0, 0.0])

Add content/review structure sections

Post release TODO

  • open v4 ticket suggesting renaming vector_layers.id -> vector_layers.name
  • revisit update to MBTiles specification Update vector_layers to match tilejson-spec 3.0  mbtiles-spec#53
  • open ticket about combining tilestats & tilejson into the same specification
  • open ticket about removing grids and interactivity fields from the specification - or generally open ticket about removing UTF-grid and interactivity fields like legend and template
  • open ticket about top-level field describing vector or raster, which helps consumers determine whether vector_layers is required or not.

cc @mapbox/core-tech

@mapsam mapsam mentioned this pull request May 1, 2018
3.0/README.md Outdated

# 3. Structure

Implementations MUST treat unknown keys as if they weren't present. However, implementations MUST expose unknown key/values in their API so that API users can optionally handle these keys. Implementations MUST treat invalid values for keys as if they weren't present. If the key is required, implementations MUST treat the entire TileJSON manifest file as invalid and refuse operation.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GretaCB Since vector_layers is the first field with two words, do we want to explicitly document that we prefer underscores _ as separators?

@mapsam
Copy link
Contributor Author

mapsam commented May 2, 2018

@GretaCB do you have an idea of what order we should put the fields in the Structure section in? Right now they are listed alphabetically, but they could also go in order of "REQUIRED" first and "OPTIONAL" second, or something else entirely.

cc @flippmoke who may have some thoughts here

@flippmoke
Copy link
Member

@mapsam I think required first might be best, but alphabetical works as well. I would just make sure its quickly clear what is required somehow so that it can be "checked" off if you were reading the spec and implementing it.

@GretaCB
Copy link
Contributor

GretaCB commented May 2, 2018

"REQUIRED" first and "OPTIONAL" second

👍 agreed, and alphabetical within those sections?

@mapsam
Copy link
Contributor Author

mapsam commented May 2, 2018

and alphabetical within those sections?

Sounds good! Thanks both of ya 🙏 Let's plan on re-ordering at the end so we can focus on writing now. Will add as a checkbox before we merge.

@GretaCB GretaCB mentioned this pull request May 2, 2018
@GretaCB GretaCB mentioned this pull request May 7, 2018
2 tasks
mapsam and others added 6 commits May 8, 2018 11:54
attribution, name, tilejson, version, description
* zooms, scheme, adds default value info, and adds minzoom MUST value
* Add tiles field, and add clarification around extensions used
@GretaCB GretaCB mentioned this pull request May 8, 2018
Copy link
Contributor

@pnorman pnorman left a 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 what an implementation is or does.

  • Content reviewed didn't include center, data, grids, legend, template, or vector_layers

3.0/README.md Outdated

# 3. Structure

Implementations MUST treat unknown keys as if they weren't present. However, implementations MUST expose unknown key/values in their API so that API users can optionally handle these keys. Implementations MUST treat invalid values for keys as if they weren’t present. If the the field is an optional field and the value is invalid, the default value MAY be applied. If the key is required, implementations MUST treat the entire TileJSON manifest file as invalid and refuse operation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is requiring implementations to have an API

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great point. What are your thoughts on a more generic description?

“… implementations MUST expose unknown key/values so users can optionally handle these keys.”

cc @mapsam

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take an example implementation stitches tiles into an image based on a tilejson (and bounding box, scale, zoom). It might not expose any key/values, including known ones, so could never be compliant.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pnorman I added the above quote to this PR based on your initial comment. Please provide example wording if you’d like further edits.

3.0/README.md Outdated

OPTIONAL. Default: null.

Contains an attribution to be displayed when the map is shown to a user. Implementations MAY decide to treat this as HTML or literal text. For security reasons, make absolutely sure that this field can't be abused as a vector for XSS or beacon tracking.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allowing HTML or literal text without saying how to pick which it is interpreted as is ambiguous.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree that this is unclear, since the goal of v3 is to document current usage and implementations are currently deciding between HTML and plain text, there isn’t much room for extra clarification. Let’s plan on addressing this by updating this field entirely in v4, for example #20.

3.0/README.md Outdated

## 3.2 `bounds`

OPTIONAL. Default: [-180, -90, 180, 90].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is inconsistent with the xyz naming schema for tiles or tms with global mercator.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying to clarify, are you referring to these coordinates…

    [ -180, -85.05112877980659, 180, 85.0511287798066 ]

and that they should be the default to avoid going beyond xyz-compliant tile bounds?

cc @mapsam

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to this PR.

@GretaCB GretaCB changed the title version 3.0 version 3.0.0 May 18, 2018
@mapsam mapsam mentioned this pull request May 30, 2018
@mapsam mapsam mentioned this pull request May 30, 2018
* Expand on empty fields object, provide example, and add subsections
@GretaCB GretaCB mentioned this pull request Jul 30, 2018
* Add note on grid interactivity

* Ensure integers are integers in the examples

* Consistify use of TileJSON and consistify fillzoom json example with the spec description

* Ensure tile endpoints are absolute urls

* Use set of tiles instead of tileset

* keys and values

* update section order & put required first

* add extra note about bounds of one dimension

* define implementation

* add caching, schema.json

* add osm.json example

* add examples link

* fix link
@mapsam
Copy link
Contributor Author

mapsam commented Aug 8, 2018

Update: Carol and I have added all planned updates and changes based on external and internal input. Next steps are to get more eyes and reviews on the final draft. Tagging @samanpwbb @ian29 @alliecrevier @flippmoke @stevage for starters, please feel free to review if you have time (but no pressure if you don't)!

Our plan is to have this merged into master next week.

@mapsam
Copy link
Contributor Author

mapsam commented Aug 8, 2018

Also cc @andrewharvey if you'd like to give this a review!

3.0.0/README.md Outdated

OPTIONAL. Default: 30. >= 0, <= 30.

An integer specifying the maximum zoom level. MUST be >= minzoom.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it worth mentioning overzooming in this description?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@samanpwbb what context do you think we should mention this? Similar to fillzoom above, I'm having a hard time determining how prescriptive or descriptive to be when it comes to how servers should act. I don't want to force tile servers to do anything they don't want to, but we can definitely provide some extra context/clarity for complex topics like overzooming. What about a descriptive helper sentence like:

Overzooming (link to a definition somewhere?) support is implemented by clients or servers. The specification does not enforce whether overzooming is available or not, merely provides information so servers and clients may better generate overzoomed tiles.

This feels like a statement that could exist outside fo the maxzoom field, too.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After thinking about this more (from prompt by @stevage), what I'd really like to see is an explicit description somewhere in tileJSON about the server-side over and under-zoom range (see #36 (comment)).

This matters because often client applications (like Studio) are responsible for communicating to users who may not understand how tiles work why their data layers are or are not visible on the map at any given time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting @samanpwbb - so you're thinking something like an overzoom property? Like:

{
  "minzoom": 0,
  "maxzoom": 15,
  "fillzoom": 8,
  "overzoom": 30
}

And this way the client knows it can safely make requests beyond the maxzoom (where data is available) but expect anything greater than maxzoom to be overzoomed.

Would you mind opening a ticket to discuss for v4?

@lukasmartinelli
Copy link

Why is this not merged?! @mapsam

@stevage
Copy link

stevage commented Sep 14, 2020

Has this been officially abandoned?

@dqunbp
Copy link

dqunbp commented Aug 9, 2021

Is it officially abandoned?

@mapsam
Copy link
Contributor Author

mapsam commented Aug 18, 2021

Thank you all for the reviews and feedback!

This was abandoned a couple years back due to a shift in work priorities. Apologies for not being clear about this. I've spent some time cleaning up the PR and think this is ready to be merged. For transparency, please note that the api.mapbox.com servers have not been updated to reflect "tilejson": "3.0.0" at this time.

@mapsam mapsam merged commit 622100a into master Aug 18, 2021
@mapsam mapsam deleted the 3.0 branch August 18, 2021 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants