Add CameraForBoxAndBearing function

[tadiseshan]

fixes #4221

Working with @ChrisLoer on adding CameraForBoxAndBearing() and fitScreenCoordinates() to have box zoom behavior without reset map bearing. We tested this manually on the debug page and ran the unit and integration tests. Still working on tests for new functionality and documentation for changes.

Launch Checklist

• briefly describe the changes in this PR
• write tests for all new functionality
• document any changes to public APIs
• post benchmark scores
• manually test the debug page

[ChrisLoer]

Thanks @taraadiseshan, this is awesome!

I followed up with some small stuff:

• Rebased onto the current version of master
• Added documentation and changelog entry
• Added unit tests to camera.js. I started down the path of making a render test, but the unit test ended up being easier to write and I think it offers pretty good coverage. I bootstrapped the expected values by running the test once, although at a glance they look like they match my expectations (e.g. the finished zoom is 1.5x when the destination is rotated 225 degrees, but 2x when kept at bearing 0).

🤔 At first I was thinking that we didn't need to document the change to BoxZoomHandler as a breaking change, because it seems to me the new behavior is obviously what people would expect...

@mourner you might be able to make suggestions about whether this is a good way to expand the API?

• The fitScreenCoordinates interface is the easiest fit for BoxZoomHandler, and we didn't want to touch the existing LatLngBounds-based fitBounds. I can imagine wanting something that's LatLng based but still allows you to control the target bearing (think "fit to kenya" but with bearing 180 because we want to use south-up map orientation). We weren't eager to add a third method just to explicitly support that, though.
• cameraForBoxAndBearing doesn't need to be public, it's just following the example of cameraForBounds, and it's nice that someone could use cameraForBoxAndBearing to implement their own bearing-supporting variant of fitBounds. BUT it's adding another method to the Map object and we probably want to be careful about how many methods we end up with.

[mourner]

This is great! One thing I'm not sure about though — why would zooming to a box and bearing require only two points? It's enough for the box zoom case because you're zooming to the same bearing as the current one, but seems not enough in the general case — I'd expect it two accept 4 points.

Also nit: box vs bounds terminology inconsistency.

[ChrisLoer]

Yeah, I think we can figure out better terminology, but I don't have a great idea yet. "bounds" is already taken to mean (aligned-to-north) "LatLngBounds", and I don't think we want to change that.

Two points and a bearing do define a specific box -- once you're axis-aligned to the bearing, any two points fit to a single box (right?). If you added more points, that would introduce the possibility of the points not fitting any box (or at least not to the corners). But maybe the right way to talk about this is something that sounds more like "fit these two points (to a box)".

cameraForTwoPointsAndBearing? 🤷‍♂️

[ChrisLoer]

@taraadiseshan I talked with @mourner about the terminology a bit this morning. His concern (which makes sense to me) is that while it's easy to imagine the box created by two onscreen points in the "box zoom" case, it's hard to imagine the "box" made by two arbitrary points and an arbitrary bearing. So for the "fit to Kenya at bearing X" case, it would be kind of hard to do a good job of choosing the right two points. He proposed something like "fit to a geojson feature" as a more natural API for that.

But we don't have to solve that problem now -- just fixing the "box zoom" case is a great improvement on its own. We can sidestep/defer the naming problem by just not making cameraForBoxAndBearing public. To do that, just append an "_" to the front of the method, and use the @private tag to make sure jsdoc excludes it from the public documentation. If you could push that change, I think this should be ready to go!

[asheemmamoowala]

proposed something like "fit to a geojson feature" as a more natural API for that.

#6208 introduces a fitToFeature method, but doesn't yet account for bearing and pitch.

[ChrisLoer]

🙌