mapbox / mapbox-gl-js

Interactive, thoroughly customizable maps in the browser, powered by vector tiles and WebGL
https://docs.mapbox.com/mapbox-gl-js/
Other
11.19k stars 2.22k forks source link

Max bounds not respected when global padding is present #11006

Open qmartouzet opened 3 years ago

qmartouzet commented 3 years ago

mapbox-gl-js version: 2.4.1

browser: Chrome 93

Steps to Trigger Behavior

  1. Create a map with a global padding not symetric (left is wider than right, top is wider than bottom
  2. Set maxBounds on this map.
  3. Draw maxBounds on this map

Link to Demonstration

https://jsfiddle.net/qmartouzet/Lg2zmr6y/

Expected Behavior

Drawn rectangle representing maxBounds should be barely visible at the edge of the map.

Actual Behavior

The whole map seems shifted by some amount to the north and to the west. Cf screenshot below (red is top padding, blue line is left padding, orange corner is north-west corner of maxBounds) :

maxbounds-not-accurate

Comments

rreusser commented 3 years ago

Let's see, it looks to me like what it's doing is using the padding to compute the center of the map, then aligning the center of the max bounds box to the center of the padding box. What it's not doing, however, is adjusting the zoom to account for the reduced visible area. In that sense I'm not sure it's the asymmetry that's affecting the result but instead the reduced visible area.

In the example below, I've set symmetric padding to cover almost the entire map. I'd expect the bounds to fit the little box in the center, but instead it's mostly unaffected:

Screen Shot 2021-09-14 at 2 26 55 PM copy

I tested out an additional adjustment by adding to EdgeInsets:

    visibleFraction (width: number, height: number): number {
        return Math.max(0.01, Math.min(
            1,  
            (width - this.left - this.right) / width,
            (height - this.top - this.bottom) / height
        )); 
    }

and then adding to this zoom adjustment in _constrain:

https://github.com/mapbox/mapbox-gl-js/blob/main/src/geo/transform.js#L1418

this.zoom += this.scaleZoom(s) + Math.log2(this._edgeInsets.visibleFraction());

The result is a map that looks like this (with various asymmetric insets):

Screen Shot 2021-09-14 at 2 35 05 PM

It doesn't currently handle interactions correctly, but I wonder if this is indeed the correct way to think about how this feature should work.

rreusser commented 3 years ago

my attempt a short summary would be: when padding is present, applying maxBounds affects the center of the map but not the zoom.

qmartouzet commented 3 years ago

Hi,

Thanks for investigating on this subject.

From my perspective, I think that maxBounds should refer to the visible part of the map, regardless of padding.

My use case is to let my users select the visible area of their map, without knowing of any control there will be on the map (mapbox standard controls and custom controls such as legend, title, etc). If the rendered visible area is outside what they set up, it could lead to some misunderstanding. To add up to this, I host for them some raster tiles (hi-def / gigapixel pictures for example). If maxBounds is padding related, it'll be a nightmare to set up to avoid some empty zones at the edges of the picture.

(I hope what I'm writing makes sense as I'm not native english speaker...).

rreusser commented 3 years ago

Ah, now I see the issue, @qmartouzet. Thanks for elaborating. I've changed the title of the ticket to reflect the underlying problem, that padding overrides max bounds and causes out-of-bounds data to be displayed. I believe the cause is definitely in the transform._constrain() function, but I'm not yet certain exactly where the fix would fit since there are a number of interconnected constraints which it applies.

https://github.com/mapbox/mapbox-gl-js/blob/ce8b9ef8ca456ee3cc6bda67294cd5894fde611e/src/geo/transform.js#L1384-L1460