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

fitBounds with offset results in incorrect map center #6736

Closed
hyperknot opened this issue May 24, 2018 · 3 comments
Closed

fitBounds with offset results in incorrect map center #6736

hyperknot opened this issue May 24, 2018 · 3 comments

Comments

@hyperknot
Copy link

hyperknot commented May 24, 2018

mapbox-gl-js version: 0.45.0
browser: all

Steps to Trigger Behavior

  1. Use fitBounds without offset
  2. Use fitBounds with offset

Link to Demonstration

https://jsbin.com/qeqabic

Expected Behavior

fitBounds's offset handling should be the same as flyTo/easeTo/jumpTo's offset handling. That is the camera center should be moved by offset in pixels after the animation, like a panBy. This is what is written in the documentation as well.

Actual Behavior

fitBounds offset handling is weird / broken. It is moving the camera center into some weird zoom multiplied scenario. This means that using the offset option actually changes the zoom, quite a lot.

In the submitted example I believe the correct behaviour is the easeTo implementation which is the same as fitBounds + panBy (which need inverted coordinates, to make life simple 🤪).

Now, this has been like this since forever, although I'd be surprised if people would be using it like this.

  1. Can you recommend a workaround which makes it possible to use fitBounds with proper offset handling?

  2. Is it possible to introduce a parameter or something which makes it handle the proper offset, like newOffset: true or something?

ref: #4846

@mollymerp
Copy link
Contributor

Hi @hyperknot – thanks for using Mapbox and sorry to hear you're running into an issue.

I believe the correct behaviour is the easeTo implementation which is the same as fitBounds + panBy

I disagree because this results in a portion of the bounds being off the screen, effectively breaking the promise of fitBounds.

This means that using the offset option actually changes the zoom, quite a lot.

A change in final zoom level with offset is expected, because you're adding to the total area that needs to fit into the viewport, so the camera must zoom out to accommodate

I do agree that the current behavior is slightly incorrect – the zoom level is being correctly calculated, but the center point is slightly off.

actual camera
image

expected camera
image

@mollymerp mollymerp changed the title fitBounds offset handling inconsistent/broken fitBounds with offset results in incorrect map center Jun 2, 2018
@hyperknot
Copy link
Author

hyperknot commented Jun 2, 2018

Hi @mollymerp,

I think now I understand how is fit bound's offset trying to work, it's trying to cram the whole bbox into the small area between the padding and the offset-ed area.

But that means that:

  • features can end up unreadably small
  • the whole operation might break depending on browser size with Map cannot fit within canvas with the given bounds, padding, and/or offset. This means that fitBounds with offset can only be used with fixed size map windows reliably.

My use case is the super classic "show a nice popup and a feature together on the map" case.

That is, I'd like to have the zoom level of the fitBounds + adjusted pan so that the popup is just visible. If the very bottom of the bbox is missing it's not a problem.

New example: https://jsbin.com/neremam

What I'm trying to achieve is this (fitBounds + panBy in the example):
fitpan3

and not this (fitBounds offset and fitBounds o pad in the example):
fitoff2

Is there any way to get to that end state via animation?

@hyperknot
Copy link
Author

With the new cameraForBounds, calculating camera parameters and adjusting them for custom display is super nice now, I'm closing this issue.

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

2 participants