maplibre-gl-js
maplibre-gl-js copied to clipboard
fix incorrect bounds with fitBounds when crossing antimeridian
If you add bounds that cross the 180th meridian (antimeridian), it causes the view to zoom out so that the northeast point of the bounds is to the left and the southeast point of the bounds is to the right. See below for two examples using map.fitBounds() for the islands of Fiji - current and expected:
Current (not correct): https://output.jsbin.com/yitecurala
Expected (my fix): https://output.jsbin.com/janaferezi
Launch Checklist
- [x] Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
- [ ] Briefly describe the changes in this PR.
- [ ] Link to related issues.
- [ ] Include before/after visuals or gifs if this PR includes visual changes.
- [x] Write tests for all new functionality.
- [ ] Document any changes to public APIs.
- [ ] Post benchmark scores.
- [ ] Add an entry to
CHANGELOG.mdunder the## mainsection.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 87.77%. Comparing base (
34dd6b6) to head (06dd907). Report is 43 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #4620 +/- ##
==========================================
- Coverage 87.97% 87.77% -0.21%
==========================================
Files 247 247
Lines 33622 33633 +11
Branches 2157 2173 +16
==========================================
- Hits 29580 29521 -59
- Misses 3076 3133 +57
- Partials 966 979 +13
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Thanks for taking the time to contribute!
@HarelM I've come across an interesting case... What should happen if the sw and ne points are flipped? In the current version, fitBounds() will act as if nothing happened. However, with adjustAntiMeridian(), it puts the ne point to the east and the sw point to the west. Is this undesirable? I feel like LngLatBounds is supposed to take sw and ne in order so its normal that it assumes they have been input in correctly, otherwise the bounds could just be defined as any two points, e.g p0 and p1.
That's exactly what I aimed for in terms of writing tests 😃. You can have two points on the globe and you need to decide if you always take the "shotest" arc, or you use the arc based on the "direction" (vector) between the points. I think the shortest is problematic in terms of cases it supports (it can't show a wide map), so the only possible solution (without adding a new flag) is to assume order and have that define the "direction". What do you think?
Yes I totally agree. I think this is the most logical way of going about things. I just hope there isn't someone who has been using fitBounds() the wrong way all this time only to have everything messed up after the change!
It's probably important to document this as part of the method documentation, just to be on the safe side.
Version 5 is the next breaking change version, I believe I'll start looking at it in the next month or so, we can consider adding this change only as part of this breaking change version. But I don't know if this is really a behavior change and not simply a bug fix...
Even a bug fix breaks someones workflow! ;) https://xkcd.com/1172/
Thanks! Can you add a changelog entry please? Also worth mentioning something in fit bounds docs (public API).