mapbox / abaculus

Library for creating static maps from tiles based on center or corner lng,lat coordinates. Uses node-blend.
ISC License
127 stars 38 forks source link

allow user to specify base tilesize #12

Closed samanpwbb closed 8 years ago

samanpwbb commented 9 years ago

Looks like a base tilesize of 256 is hardcoded: https://github.com/mapbox/abaculus/blob/master/index.js#L76

User should be able to pass a custom base tilesize in.

samanpwbb commented 9 years ago

Using this branch for now: https://github.com/mapbox/abaculus/tree/tilesize-512

dprophet commented 8 years ago

@samanpwbb So why is this a fix? Arent you now hard coding all tiles to be 512? We have a mix of 256 and 512 being composited together.

samanpwbb commented 8 years ago

It's not a fix, issue is still open and wouldn't be too hard to implement.

javisantana commented 8 years ago

The actual problem is the image size, abaculus uses 256 to calculate tile positions and that's fine but when rendering them they should be reduced to 256px when rendered

mapnik has a way to resize images but that requires at least node-mapnik 3.15

dprophet commented 8 years ago

@mtirwin So. To be clear, this isnt a fix for the 512 problem.

@samanpwbb and @miccolis Read below.

https://www.mapbox.com/help/enable-retina-tiles/

Clearly specifies the industry standard, "@2x"

Your own documentation specifies

GET {{site.tileApi}}/styles/v1/<username>/<styleid>/tiles/<z>/<x>/<y>@2x?access_token=<access token>

Where I can optionally specify @2x for retina or not.

I am confused why your new API isnt following your own standards. Mapbox very documentation, everywhere, on every site, throughout stackexchange and mapbox.com/help and every example, shows @2x. If I omit @2x I should get 256. If I add @2x I get 512.

var url = 'https://api.mapbox.com/v4/mapbox.streets/{z}/{x}/{y}';
    url += (L.Browser.retina ? '@2x.png' : '.png');
    url += '?access_token=my-token';

This is an Industry standard, OpenLayers standard, Leaflet, OSM standard, Mapbox standard, CartoDB standard, etc.

Please edumacate me as to why this is my or @javisantana problem?

miccolis commented 8 years ago

@dprophet I'm not sure I follow you. The document you link to is a help page from the Mapbox site that links off to API documentation and itself doesn't mention any relationship between the @2x flag and tile dimensions.

The docs for retrieving a tile from the Mapbox Style API are https://www.mapbox.com/api-documentation/#retrieve-raster-tiles-from-styles and describe the tile dimensions and relationship with the @2x specifier. The API offers 512px tiles for standard screens and 1024px tiles for high-dpi screens. The 512px tiles are not Retina or high-dpi tiles. The 512px tiles we're providing cover four times the land area as 256px. They do so at a traditional pixel ratio. We support the @2x flag, and do the thing you'd expect - double the pixel ratio and provide 2x tiles, which in this case are 1024px.

I do see the older API documentation that link off to OSMs tile docs and talk about 256px tiles. However the documentation specifically states: "The {z}/{x}/{y} parameters are the tile coordinates as described in the Slippy Map Tilenames specification". It doesn't claim to completely adopt OSMs api.

Also it is generally possible to use tiles of sizes other than 256px in many mapping libraries including Openlayers, Leaflet, and all of the Mapbox libraries and SDKs.

This is not anyone's specific problem, or really a problem as such. We should be able to add support for various tile sizes to this project.

samanpwbb commented 8 years ago

Edit: sorry, I was just informed that cartoDB uses abaculus and that's where you're running into issues. Hopefully you can get things cleared up.

miccolis commented 8 years ago

@samanpwbb other projects do use abaculus still including some CartdoDB products that are totally worth supporting. @bsudekum is working on adding support for additional tilesizes and we should have that in merge-able shape shortly.

dprophet commented 8 years ago

@miccolis So, you are telling me that in the documentation below

http://mapbox.github.io/notes/#549cec183d2a5057cd00

Your @2x means you will return 1024 not 512?

What about 256?

Technically the standard by all tile servers I can find state the

http://{s}.somedomain.com/blabla/{z}/{x}/{y}{r}.png

Where {r} is substituted by the device.

I read your documents above. Loved that you supported the {r} '@2x'

I am glad we will get abaculus fixed. However I have a legacy .NET Bloomberg maps rendering system that also must change because you defaulted to 512. Kicking the ball back and forth because of a bad decision to default to 512.

@alinapaz Please move Bloomberg's style out of the BETA API you have given us and move to your legacy where 256 is the default and I can optionally specify @2x.

mtirwin commented 8 years ago

👋 @dprophet. Going to move responses back to our collab repo since you're getting into details of Bloomberg's implementation.

dprophet commented 8 years ago

@bsudekum and @miccolis

Will your tileSize changes work work https://github.com/mapnik/node-mapnik/releases/tag/3.0.1

I know you are specifically requiring "mapnik": "~3.5.0". Any particular reason? I dont see the dependencies between your 512 changes and the node-mapnik version.

Thoughts?

To tell the truth, the abaculus library is so low level and there are at least 10 layers of code above this that need to change. Including various cross-dependencies on node-mapnik versions.

Its never easy.

alinapaz commented 8 years ago

@dprophet the 3.5.0 release of node-mapnik was a big one which is why the version got bumped here back in February.

It doesn't really overlap with what we're doing in the latest work, however I'd certainly recommend looking at an update of node-mapnik before doing fork of abaculus as node-mapnik 3.0.1 release is from Oct 2014.

Does this help answer your question?

cc @bsudekum @miccolis

dprophet commented 8 years ago

@alinapaz Yes. You answered the question.

I have 1 major problem and this will not be solved in the near future.

Mapbox Base Maps = 512 Application Overlay Maps = 256

If I fix the basemap I break the application overlays. If I fix the application overlays the basemaps are broken.

I have been looking at this issue and the code for a couple of days now. Its very complex. The standard approach that has been used with node-mapnik and the the layers of code ontop of this was to use the '@2x' option. Our system supports the @2x option as a global scaling for all tileSets (not 1 setting for an application layer and a different setting for a the Mapbox basemap)

@bsudekum @miccolis Guys, I really do thank you for your abaculus changes. Unfortunately its at the lowest stack. I need to change the entire product stack.

I need to release or production next week for our ALPHA candidates and testing. I am removing the Mapbox basemaps until this problem has a real fix.

We have been going back and forth for 2 months now with no resolution.

I need this feature https://www.mapbox.com/api-documentation/#retina

Clearly specified, @2x = 512

@alinapaz Since I own the technology stack and I make the final decisions, since your new BETA api doesnt support '@1x' '@2x' '@4x' options, please move Bloomberg basemaps to your old raster api system.

dprophet commented 8 years ago

Additionally you have already created basemaps for Bloomberg

Dark: http://{s}.tiles.mapbox.com/v3/bloombergbnef.blahblah/{z}/{x}/{y}.png Satellite: http://{s}.tiles.mapbox.com/v3/bloombergbnef.blahblah/{z}/{x}/{y}.png Terrain: http://{s}.tiles.mapbox.com/v3/bloombergbnef.blahblah/{z}/{x}/{y}.png Streets: http://{s}.tiles.mapbox.com/v3/bloombergbnef.blahblah/{z}/{x}/{y}.png

These work fine. Why am I stuck with a new BETA API that doesnt work?