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

map.fitBounds() with bearing does not fit the entire bounds #10064

Open
blahblahblah- opened this issue Nov 1, 2020 · 6 comments
Open

map.fitBounds() with bearing does not fit the entire bounds #10064

blahblahblah- opened this issue Nov 1, 2020 · 6 comments

Comments

@blahblahblah-
Copy link

When map.fitBounds(...) is called with a bearing option passed in, it seems like the bearing is not taken into the account on calculating the appropriate zoom, resulting in part of the bounded area to not be in view.

mapbox-gl-js version: 1.12.0

browser: Chrome

Steps to Trigger Behavior

  1. Create a bounded area with a list of coordinates
  2. Call map.fitBounds(...) with bearing option set

Link to Demonstration

https://jsfiddle.net/_blahblahblah/k4vL6s5r/

Expected Behavior

When clicking on the 'Fit to Line at 29º Bearing' button in the demo link, all 6 stations should be in view.

Actual Behavior

Only 4 of the 6 stations are in view.

@sgolbabaei sgolbabaei added the needs investigation 🔍 Issues that require further research (e.g. it's not clear whether it's GL JS or something else) label Nov 2, 2020
@karimnaaji
Copy link
Contributor

Hello @blahblahblah- , I think this was fixed by #9821, but is not yet released.

@blahblahblah-
Copy link
Author

Hi @karimnaaji, I also tested with the latest main and I can still reproduce the issue.

@karimnaaji
Copy link
Contributor

Ok thanks for confirming!

@karimnaaji karimnaaji added bug 🐞 good first issue and removed needs investigation 🔍 Issues that require further research (e.g. it's not clear whether it's GL JS or something else) labels Nov 4, 2020
aparshin added a commit to aparshin/mapbox-gl-js that referenced this issue Nov 30, 2020
Also influences on map.fitScreenCoordinates()  and map.cameraForBounds()
TannerPerrien added a commit to TannerPerrien/mapbox-gl-js that referenced this issue Mar 6, 2022
…thods:

* `map.fitBounds()`
* `map.fitScreenCoordinates()`
* `map.cameraForBounds()`

Consider 45 and 135 rotations and how all four corners need to be taken into account.

Changes in tests:

* Fix tests for camera => #cameraForBounds (they were incorrect)
* Add new test for camera => #fitBounds with non-zero bearing
* Changed input coordinates in tests camera => #fitScreenCoordinates to make the input region not square
TannerPerrien added a commit to TannerPerrien/mapbox-gl-js that referenced this issue Mar 8, 2022
…thods:

* `map.fitBounds()`
* `map.fitScreenCoordinates()`
* `map.cameraForBounds()`

Consider 45 and 135 rotations and how all four corners need to be taken into account.

Changes in tests:

* Fix tests for camera => #cameraForBounds (they were incorrect)
* Add new test for camera => #fitBounds with non-zero bearing
* Changed input coordinates in tests camera => #fitScreenCoordinates to make the input region not square

(cherry picked from commit 3f151d8)
SnailBones pushed a commit that referenced this issue Mar 9, 2022
* Fix bounds/bearing calculation bug (#10064) in the following methods:

* `map.fitBounds()`
* `map.fitScreenCoordinates()`
* `map.cameraForBounds()`
@pimhoogendoorn
Copy link

pimhoogendoorn commented May 11, 2023

Hi @TannerPerrien it seems that your fix has been merged however, I can still reproduce it even in the latest version 2.14.1. Is that correct and the reason why this issue is still open?

@pahuta
Copy link

pahuta commented Sep 26, 2023

Checked on v.2.15.0, bug is not reproduced.
Updated demo: https://jsfiddle.net/q4d50L9h/

@mapbox/gl-js , please consider to close the issue.

@chrisnok
Copy link

chrisnok commented Jan 3, 2024

If you change the demo to use bearing 135 instead of 29, you get an extra padding. Is there a workaround for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants