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

coordsFromBbox enlarge width and height twice #27

Open jingsam opened 8 years ago

jingsam commented 8 years ago

The width calculated by coordsFromBbox() is base_width * scale scale, which should be base_width \ scale. Let check out:

console.log(abaculus.coordsFromBbox(0, 2, [-180, -85.0511, 180, 85.0511], 19000, 256))
console.log(abaculus.coordsFromCenter(0, 2, {x: 0, y: 0, w: 256, h: 256}, 19000, 256))

What I got is :

{ w: 1024, h: 1024, x: 256, y: 256 } 
{ w: 512, h: 512, x: 256, y: 256 } 

We have already get the right size from here: https://github.com/mapbox/abaculus/blob/master/index.js#L42-L43

But we enlarged the size again at here: https://github.com/mapbox/abaculus/blob/master/index.js#L50-L51

And x: 256, y: 256 is definitely not the center of w: 1024, h: 1024, which proves the results calculated by coordsFromBbox is wrong.

If this problem is confirmed. I will send a PR.

jingsam commented 8 years ago

Note that the test of coordsFromBbox and coordsFromCenter are not specify tileSize, such as https://github.com/mapbox/abaculus/blob/master/test/test.js#L44 https://github.com/mapbox/abaculus/blob/master/test/test.js#L73

Let's have a test:

console.log(abaculus.coordsFromBbox(0, 2, [-180, -85.0511, 180, 85.0511], 19000))
console.log(abaculus.coordsFromBbox(0, 2, [-180, -85.0511, 180, 85.0511], 19000, 256))
console.log(abaculus.coordsFromCenter(0, 2, {x: 0, y: 0, w: 256, h: 256}, 19000))
console.log(abaculus.coordsFromCenter(0, 2, {x: 0, y: 0, w: 256, h: 256}, 19000, 256))

The result is:

{ w: 512, h: 512, x: 128, y: 128 }
{ w: 1024, h: 1024, x: 256, y: 256 }
{ x: 128, y: 128, w: 512, h: 512 }
{ x: 256, y: 256, w: 512, h: 512 }

Because the tileSize is required by var sm = new SphericalMercator({ size: tileSize * s }); If s = 1, tileSize * 1= NaN = 256 everything is OK. However, when s != 1, for example 2, tileSize * 2= NaN = 256, which should be 512.

Conclusion: The tileSize can not be omited.

jingsam commented 8 years ago

PR has sent. see https://github.com/mapbox/abaculus/pull/28