mapbox-gl-js
mapbox-gl-js copied to clipboard
mapbox
Map.flyTo padding not being cleared -> fitBounds is summing it up with it's own padding argument.
mapbox-gl-js version: 2.8.2 but also present in 2.7.1
browser: Google Chrome 100.0.4896.127 64-bit
Steps to Trigger Behavior
- Call function
map.FlyTowith padding ->
map.flyTo({
center: coordinates[0],
padding: {
top: 50,
bottom: 0,
left: 0,
right: 0
}
})
- Call function
map.fitBounds->
map.fitBounds(bounds, {
padding: 0
});
Link to Demonstration
https://codepen.io/sienki-jenki/pen/xxpvPza
https://user-images.githubusercontent.com/34219675/165626180-1b5e49a1-9430-4b07-82da-fd787679ad3f.mp4
When you open site, calling fitBounds will work correctly, however if you call flyTo, padding is getting saved in some state and is never cleared, meaning fitBounds is somehow summing up padding passed to flyTo with it's own passed paddings as argument. As you can see on video, 50px of top padding is added to fitBounds even when 0 is an argument.
Expected Behavior
Map fits linestring to view without padding, because padding: 0.
Actual Behavior
Map firts linestring to view with sum of paddings, meaning paddings passed to flyTo function are added to paddings of fitBounds.
Yes, i ran into same problem, and did a quick fix myself. Note that i assume that map is initialized with default fitBoundsOptions.padding and padding is alway given in form of { top, left, right, bottom }
class Map extends mapboxgl.Map {
_cameraForBoxAndBearing ( p0, p1, bearing, options ) {
if ( options && options.padding && this.options && this.options.fitBoundsOptions && this.options.fitBoundsOptions.padding && mapboxgl.version === "2.8.2" ) {
const defaultPadding = this.options.fitBoundsOptions.padding;
const edgePadding = this.transform.padding;
options.padding.top = defaultPadding.top - edgePadding.top + options.padding.top;
options.padding.left = defaultPadding.left - edgePadding.left + options.padding.left;
options.padding.right = defaultPadding.right - edgePadding.right + options.padding.right;
options.padding.bottom = defaultPadding.bottom - edgePadding.bottom + options.padding.bottom;
}
return super._cameraForBoxAndBearing( p0, p1, bearing, options );
}
}
We have also had this issue but in reverse. Using flyTo and then fitBounds also creates different issues related to the same bug.
-
On a landscape view where there is enough horizontal space on the right, the padding gets summed. (600px + 600px)
-
On a portrait view (our use case) where the 1200px width does not exist, MapBox refuses to fit bounds with the following error:
Map cannot fit within canvas with the given bounds, padding, and/or offset.
Our temporary solution is to wait for the camera to stop moving usingisMoving and then fitBounds
Both map.fitBounds and map.fitScreenCoordinates utilize map._cameraForBounds (or map.cameraForBounds) to account for padding via the zoom and center options and adjust the specified EasingOptions accordingly.
They then utilize map._fitInternal which removes the padding from the specified EasingOptions because "calculatedOptions already accounts for padding by setting zoom and center accordingly" and then executes either map.easeTo or map.flyTo. https://github.com/mapbox/mapbox-gl-js/blob/7bbf2e901b534e1feb44644d9456a3b038105f95/src/ui/camera.js#L999
However, map.easeTo and map.flyTo both set padding based on the padding specified in EasingOptions, which we know has been removed by map._fitInternal, and any existing padding. In this case, using map.fitBounds after map.flyTo will result in the padding set by map.flyTo to remain until map.flyTo, map.jumpTo, or map.easeTo are called.
A temporary solution would be to call map.setPadding and correct for the discrepancy yourself. I might try to tackle this myself and open a pull request, but I can't guarantee anything.
Both
map.fitBoundsandmap.fitScreenCoordinatesutilizemap._cameraForBounds(ormap.cameraForBounds) to account for padding via thezoomandcenteroptions and adjust the specifiedEasingOptionsaccordingly.They then utilize
map._fitInternalwhich removes the padding from the specifiedEasingOptionsbecause "calculatedOptions already accounts for padding by setting zoom and center accordingly" and then executes eithermap.easeToormap.flyTo.https://github.com/mapbox/mapbox-gl-js/blob/7bbf2e901b534e1feb44644d9456a3b038105f95/src/ui/camera.js#L999
However,
map.easeToandmap.flyToboth set padding based on the padding specified inEasingOptions, which we know has been removed bymap._fitInternal, and any existing padding. In this case, usingmap.fitBoundsaftermap.flyTowill result in the padding set bymap.flyToto remain untilmap.flyTo,map.jumpTo, ormap.easeToare called.A temporary solution would be to call
map.setPaddingand correct for the discrepancy yourself. I might try to tackle this myself and open a pull request, but I can't guarantee anything myself.
Although confusing, this seems to be the intended behavior according to a unit test. I think this should either be documented or the behavior should be normalized. https://github.com/mapbox/mapbox-gl-js/blob/184421e279d5522764699a149cfb3700bd3739e1/test/unit/ui/camera.test.js#L2294
Both
map.fitBoundsandmap.fitScreenCoordinatesutilizemap._cameraForBounds(ormap.cameraForBounds) to account for padding via thezoomandcenteroptions and adjust the specifiedEasingOptionsaccordingly.They then utilize
map._fitInternalwhich removes the padding from the specifiedEasingOptionsbecause "calculatedOptions already accounts for padding by setting zoom and center accordingly" and then executes eithermap.easeToormap.flyTo.https://github.com/mapbox/mapbox-gl-js/blob/7bbf2e901b534e1feb44644d9456a3b038105f95/src/ui/camera.js#L999
However,
map.easeToandmap.flyToboth set padding based on the padding specified inEasingOptions, which we know has been removed bymap._fitInternal, and any existing padding. In this case, usingmap.fitBoundsaftermap.flyTowill result in the padding set bymap.flyToto remain untilmap.flyTo,map.jumpTo, ormap.easeToare called.A temporary solution would be to call
map.setPaddingand correct for the discrepancy yourself. I might try to tackle this myself and open a pull request, but I can't guarantee anything.
Another solution: replace the line I linked with the following.
if (options.padding) extend(this.transform.padding, options.padding);
I tried to implement this myself, but I couldn't get testing to work and I don't have the time to spend troubleshooting.
I don't know if the question is related, but we have the problem that calling fitBounds on our map with new Mapbox.LngLatBounds does not adapt the map to the new bounds. The center of the map is not changed to the new center computed by map.cameraForBounds.
Instead the center of the map after the _fitInternal call is still the default center we set when setting up the map.
The map has a global padding which has get/set methods map.getPadding, map.setPadding and for debugging map.showPadding = true.
If you pass a padding option to easeTo and flyTo, they set the global padding.
If you call fitBounds without a padding option, it will respect the global bounds.
If you call fitBounds with a padding option, it will respect the global bounds and in addition apply your padding options.
Neither of these calls to fitBounds will set the global bounds though.
In my view, it's unexpected that easeTo/flyTo set the global bounds as a side affect. I feel the global bounds should only be set on map.setPadding which is respected by easeTo/flyTo/fitBounds and any padding options you pass to those methods is applied in addition to the global padding.
Given the current behaviour would have worked it's way into applications, any changes would be breaking changes imo, but would the confusion.
I am not able to get the padding to be respected with fitBounds after using flyTo and I have confirmed the global padding is still set.
On first load padding looks good.
Then, after using flyTo then resetting to the full map with fitBounds (exact same coordinates I loaded the map with), the padding is not respected.
You can see the padding is still set with the showPadding debug lines showing.
I have tried every version of setting and resetting the padding at different points in the process but have no luck with fitBounds on the subsequent runs.
@stepankuzmin Could you elaborate why this issue is closed? It got fixed in recent releases? Could you link PR/Release notes for that? 🙏
Hi @kamil-sienkiewicz-asi,
Yes, this was fixed, and GitHub automatically closed the issue. It will be available in the next v3.2.0 release.
Is this actually fixed? I can't get flyTo to work with padding properly at all anymore. The padding doesn't animate and doesn't seem to go to the right values.
Hi, @timcreatedit. Could you please make a repro? I've tried the original repo with v3.2.0, which seems to be working.
The changelog for v3.2.0 is available here https://github.com/mapbox/mapbox-gl-js/releases/tag/v3.2.0
@stepankuzmin @timcreatedit is right. Right now first invocation map.flyTo ignores padding.
See: https://codepen.io/kamil-sienkiewicz-asi/pen/BaEZmam
Click "Fly" wait for map till complete stop and then click Fly again. Second time padding is respected.
Actually it seems like map.easeTo is right now saving padding and then fitBounds is using that :| Same case as with flyTo
Yes, also here happening that the flyTo is not respecting the padding property anymore, this issue has been happening for more than a month
