maplibre / maplibre-gl-js

MapLibre GL JS - Interactive vector tile maps in the browser
https://maplibre.org/maplibre-gl-js/docs/
Other
6.73k stars 719 forks source link

flyTo() with padding is somewhat broken with Globe #4891

Open pjamessteven opened 1 month ago

pjamessteven commented 1 month ago

maplibre-gl-js version: 5.0.0-pre.3

browser: Firefox/Chrome/Safari

Steps to Trigger Behavior

Use flyTo with set padding. The first time, padding will be applied properly. Now use flyTo again, with a different padding value. This time, the newly specified padding is not applied and flyTo uses the original padding value.

In other words, above zoom level 0 and when the globe is active, you have to flyTo the same location twice to get the padding to be applied correctly. This isn't the case when globe is not active.

I can use map.setPadding, but then I don't get a smooth transition.

Link to Demonstration

Without Globe (expected behavior):

https://jsbin.com/rejijay/edit?html,output

With Globe (borked):

https://jsbin.com/wihekiq/edit?html,output

HarelM commented 1 month ago

Is this related to the following discussion by any chance? https://github.com/maplibre/maplibre-gl-js/discussions/2984

From the above description it sounds like a regression big, but just to be on the safe side...

HarelM commented 1 month ago

You would use the jsbin like you would use other versions, pre-releases versions are uploaded to npm.

pjamessteven commented 1 month ago

Hey HareIM,

Thanks for that, didn't realise the prereleases were on npm. I don't believe it's related to that issue you mentioned. I've created a simple example that demonstrates the problem here. Above zoom level 0, when the globe is active, you have to flyTo the same location twice to get the padding to be applied correctly. This isn't the case when globe is not active.

Without Globe:

https://jsbin.com/rejijay/edit?html,output

With Globe:

https://jsbin.com/wihekiq/edit?html,output

HarelM commented 1 month ago

As far as I can tell, the issue is that globe does the padding differently than mercator. From what I saw in the code, mercator uses pointAtOffset to update the padding while calling flyTo, while globe is trying to do it prior to returning the ease function, which is not the same and probably what causes the bug, it basically ignores the pointAtOffset... https://github.com/maplibre/maplibre-gl-js/blob/dd17405c8995085c5c5132006a2a179076df7efc/src/geo/projection/globe_camera_helper.ts#L411-L418 vs https://github.com/maplibre/maplibre-gl-js/blob/dd17405c8995085c5c5132006a2a179076df7efc/src/geo/projection/mercator_camera_helper.ts#L219-L222

Cc: @kubapelc