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

coordsFromBbox enlarge width and height twice #27

Open
jingsam opened this issue Jul 19, 2016 · 2 comments
Open

coordsFromBbox enlarge width and height twice #27

jingsam opened this issue Jul 19, 2016 · 2 comments

Comments

@jingsam
Copy link

jingsam commented Jul 19, 2016

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
Copy link
Author

jingsam commented Jul 19, 2016

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
Copy link
Author

jingsam commented Jul 19, 2016

PR has sent. see #28

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

No branches or pull requests

1 participant