fnicollet / Leaflet

:leaves: JavaScript library for mobile-friendly interactive maps
http://leafletjs.com
BSD 2-Clause "Simplified" License
48 stars 20 forks source link

Start on fixing map bounds when rotating #22

Closed sisou closed 7 years ago

sisou commented 7 years ago

This PR starts work on the issue, highlighted in #7, that the map bounds are returned incorrectly when the map is rotated.

There is however an issue with bounds expressed as a single LatLng pair, when the map is rotated, as the visible-map borders are no longer going along latitude or longitude values, but go diagonal.

I added a new debug demo page ("rotate-bounds.html") to visualise the problem: https://rawgit.com/sisou/Leaflet/bugfix/7-fix-getbounds-when-rotated/debug/rotate/rotate-bounds.html

As you my know, the bounding box is defined by its NE and SW corner. But in my page (which is not necessarily doing it correctly) the NE and SW are not in fact those cardinal directions, but simply the lower left and top right corners of the visible map.

The general question is now: How should map bounds be represented in a rotated map, where it is not enough to only know the SW and NE corners?

fnicollet commented 7 years ago

Good question indeed :)

I had a look at openlayers and this example: http://openlayers.org/en/latest/examples/rotation.html If you type in the console:

map.getView().calculateExtent(map.getSize())

You always get the same value, no matter the rotation. See also this commit: https://github.com/openlayers/openlayers/issues/2195

Also, they had the same questions as couple years ago :) https://github.com/openlayers/openlayers/pull/2735#issuecomment-56259612

sisou commented 7 years ago

OK, so from that discussion (last of your links) I learned that it's a hot topic with two very different perspectives ;D

The commit you referenced and what it does is more interesting and probably what we are looking for: the getBounds function is used to retrieve the area, that is currently viewed. Since the returned object is limited to two pairs of coordinates (SW and NE corners coordinates), the resulting area is bound (haha) to be un-rotated, meaning it's edges are following latitude and longitude lines.

The getBounds function will most likely be used to render things that are currently within the viewport. Things like loading new map tiles, placing/loading new markers etc. Thus the returned area from that function needs to at least cover the viewed area.

Thus we need to make the the getBounds function return the un-rotated latLngBounds object, that contains the rotated view rectangle.

Any objections?

sisou commented 7 years ago

This would mean, however, that the returned bounds are changing when the view is rotated.

Basically we need this: http://stackoverflow.com/questions/622140/calculate-bounding-box-coordinates-from-a-rotated-rectangle

(maybe changing the names of the functions, to make them more clear of what they return, is actually a good idea...?)

sisou commented 7 years ago

Alright.

I adapted the getBounds function to return the bounding box that contains the current view.

This means, that when using fitBounds with the result from getBounds while rotated, the map zooms out (the bounding box from getBounds contains the view, while fitBounds makes the view contain the passed bounds).

Updated example page here: https://rawgit.com/sisou/Leaflet/bugfix/7-fix-getbounds-when-rotated/debug/rotate/rotate-bounds.html

What do you think?

fnicollet commented 7 years ago

I think that's all good, well done! :)

fnicollet commented 7 years ago

By the way, you should have push access to this repo now

Fabien

sisou commented 7 years ago

Yes, I received your invitation, thank you! =)

However, I want the second opinion from the current main maintainer before merging ;)