don't reset bearing in fitbounds

[kelvinabrokwa]

#1225

[kelvinabrokwa]

@jfirebaugh 👀

[1ec5]

This maintains the current zoom but doesn’t account for rotation. For example, this call:

map.fitBounds([[37, -109+2./60+48./(60*60)], [41, -102.05]], {bearing: 60})

shows most of Colorado but not all of it. (#1338 has another example.)

In mapbox/mapbox-gl-native@bcbb56c for mapbox/mapbox-gl-native#1783, I corrected for this issue by fitting to a “visual bounding box” parallel to the viewport that encompasses the passed-in bounds.

[1ec5]

Specifically for the box zoom gesture that you’re testing, try this:

1. Rotate the map such that the District of Columbia’s northeast border is parallel to the top of the page.
2. Box-zoom the District of Columbia.

There’s a lot of padding on the top and even more on the bottom:

[kelvinabrokwa]

@1ec5 woops totally missed that. But doesn't box zoom always have a bunch of padding?

[EDIT]: it doesn't

[1ec5]

It appears to have no padding: if you box-zoom on Colorado, for example, it fits perfectly when there’s no rotation but zooms in too far otherwise.

[jfirebaugh]

@kelvinabrokwa Nice! Can you please add some tests for fitBounds to camera.test.js?

[kelvinabrokwa]

@jfirebaugh will do. It's not quite ready yet though. I jumped the gun on those googly eyes...

[1ec5]

These bounds are incorrect: if you rotate the map by 45° clockwise and box-zoom the District of Columbia, bounds.ne is the northern corner of D.C. and bounds.sw is the southern corner. But Camera.prototype.fitBounds() is expecting an unrotated bounding box. So the camera ends up trying to fit to a bogus bounding box.

There are two solutions: the simpler solution is to extend bounds by the other two corners of the zoom box. The center point would be correct, but the zoom level would be suboptimal in degenerate cases like D.C. The more complex but more correct solution is to port over the changes in mapbox/mapbox-gl-native#1795, then pass in the four corners of the zoom box rather than a LatLngBounds.

[1ec5]

Better to use forEach() or a plain ol’ for loop in this case. In C++, the for…in syntax was cleaner than the alternative, but there isn’t much to be gained here.

[mourner]

Always iterate over arrays with a plain for loop unless there's a strong reason not to.

[1ec5]

These variables only start out at ±∞; nePixel and swPixel are fine.

[1ec5]

The calls to project() need to take place inside the for loop below. As it is, you’re assuming that the northwest point remains the northwest point after rotation.

[1ec5]

Something’s up with Camera.prototype.easeTo(): it works when fitBounds() calls it to set the center point or the zoom level, but not both. Meanwhile, if linear is false, flyTo() appears to work better, albeit with a suboptimal zoom level.