mapbox / tile-cover

Generate the minimum number of tiles to cover a geojson geometry
MIT License
189 stars 40 forks source link

Tilecover with the same min and max zoom creates unpredictable results #73

Closed tcql closed 7 years ago

tcql commented 7 years ago

screen shot 2016-11-02 at 11 01 03 trying to cover the US with min_zoom:7, max_zoom: 7 - no coastal tiles, and lots of scattered random ones

screen shot 2016-11-02 at 11 28 09 if I generate tiles with min_zoom: 6, max_zoom: 7 then use tilebelt to get the child-tiles for z6 tiles, I get what I was expecting

Interestingly, both runs result in the same number of tiles, which suggests there's just some data flow issue where the coastal tiles get their coordinates scrambled up

bkowshik commented 7 years ago

I came across a similar issue today with the following code snippet:

// File: index.js
var feature = {"type":"Feature","geometry":{"type":"Polygon","coordinates":[[[77.3440933227539,13.239277104266526],[77.3440933227539,13.239611302005722],[77.34443664550781,13.239611302005722],[77.34443664550781,13.239277104266526],[77.3440933227539,13.239277104266526]]]},"properties":{}};
var tiles = tileCover.geojson(feature.geometry, {min_zoom: 25, max_zoom: 25});
console.log(JSON.stringify(tiles));

I ran the script and piped to tippecanoe and mbview.

node index.js | tippecanoe -pf -pk -fo tileset.mbtiles && mbview tileset.mbtiles -q
screen shot 2016-11-16 at 3 50 22 pm

Staggered tiles viewed with mbview

screen shot 2016-11-16 at 4 01 09 pm

Close up view of staggered tiles

bkowshik commented 7 years ago

Also, I have seen the following behaviour on a couple of occasions. Basically, things are great except a few extra projection tiles somewhere around the corners of the feature that is tile-covered.

screen shot 2016-11-16 at 4 51 14 pm

A z7 tile covered by z15 tiles

screen shot 2016-11-16 at 4 51 37 pm

An extra tile on the top-left corner

bkowshik commented 7 years ago

Added a fixture and a failing test case with: https://github.com/mapbox/tile-cover/commit/a8fdfe57ca73383579b3aa5af8d9e577637e2980. The idea behind the fixture is:

  1. When you take a z20 tile as a feature
  2. And tile-cover it with min_zoom and max_zoom limits of 20
  3. You should get the same feature as the tile

I am not sure if this fixture is helpful in creating a fix, but I am documenting it for reference.

screen shot 2016-11-23 at 12 10 00 am

Input fixture on the left and output tiles on the right


cc: @tcql @mourner

mourner commented 7 years ago

@bkowshik I think this can be an expected output — it may return adjacent tiles because the tile coordinate in mercator is unprojected to latitude/longitude bbox, which is subject to floating point precision errors, and then tiles that had a border exactly matching a tile side can start overlapping it slightly.

However, the cases in the first two comments look like serious bugs.

mourner commented 7 years ago

@bkowshik It looks like https://github.com/mapbox/tile-cover/issues/73#issuecomment-260911228 is different from the original issue, and tile-cover generates the tiles properly but tippecanoe somehow rounds the coordinates because they're so tiny (cc @ericfischer could this be the case?). However, I don't think you should ever use a zoom as high as z25 — it translates to about 30-50cm width for a single tile.

mourner commented 7 years ago

@tcql I can't reproduce the original issue with the following script — everything looks good:

var tileCover = require('./');
var geojson = require('./test/fixtures/world/USA.geo.json');
var tiles = tileCover.geojson(geojson.features[0].geometry, {min_zoom: 7, max_zoom: 7});
console.log(JSON.stringify(tiles));
node debug.js | geojsonio

Can you set up a broken test case?

e-n-f commented 7 years ago

@mourner Yes, it looks like the misaligned grid is probably from Tippecanoe tiny polygon reduction and could be eliminated with --no-tiny-polygon-reduction, at the cost of probably making the low zooms not work at all. I don't think this could explain the case of the one extra stray tile at the top left, but I'll try some tests and find out.

mourner commented 7 years ago

@tcql I have to close this as not reproducible on the tile-cover side. Please feel free to reopen after we have a reproducible test case.