mapbox / tile-cover

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

Optimize mergeTiles #49

Closed mourner closed 9 years ago

mourner commented 9 years ago

Using tilebelt.hasSiblings is woefully inefficient, because to check if a tile has siblings, you need to loop through all tiles (and there can be a million of them). In addition, manipulating arrays in this check (with all the concats etc.) is slowing things down even more.

It should be MUCH faster to merge tiles by processing the tile hash instead of an array.

mourner commented 9 years ago

OK, here's another reason it's so slow. After the first pass of merges on the current zoom level (z), the next pass to higher zoom level happens on ALL tiles (both z and z-1) again. But the only tiles that can be merged on the second pass are z-1 ones (and the amount of z-1 tiles is usually much much smaller than remaining z tiles).

@morganherlocker Now if we rewrite the algorithm to fix both issues, theoretically there will be pretty much no performance overhead of say z0-16 compared to z15-16. If my theory proves to be true, we can simplify the API and remove min_zoom option, making it always 0, so that API just accepts zoom. Are there use cases where we want to specify min_zoom higher than 0 provided there's no performance difference?