Skip to content

Commit

Permalink
Fix bounds/bearing calculation bug (mapbox#10064) in the following me…
Browse files Browse the repository at this point in the history
…thods:

* `map.fitBounds()`
* `map.fitScreenCoordinates()`
* `map.cameraForBounds()`

Consider 45 and 135 rotations and how all four corners need to be taken into account.

Changes in tests:

* Fix tests for camera => #cameraForBounds (they were incorrect)
* Add new test for camera => #fitBounds with non-zero bearing
* Changed input coordinates in tests camera => #fitScreenCoordinates to make the input region not square
  • Loading branch information
TannerPerrien committed Mar 6, 2022
1 parent 3753b6d commit 3f151d8
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 18 deletions.
27 changes: 20 additions & 7 deletions src/ui/camera.js
Original file line number Diff line number Diff line change
Expand Up @@ -621,15 +621,28 @@ class Camera extends Evented {
const tr = this.transform;
const edgePadding = tr.padding;

// We want to calculate the upper right and lower left of the box defined by p0 and p1
// in a coordinate system rotate to match the destination bearing.
const p0world = tr.project(LngLat.convert(p0));
const p1world = tr.project(LngLat.convert(p1));
// We want to calculate the corners of the box defined by p0 and p1 in a coordinate system
// rotated to match the destination bearing. All four corners of the box must be taken
// into account because of camera rotation.
const p0LatLng = LngLat.convert(p0);
const p1LatLng = LngLat.convert(p1);
const p0world = tr.project(p0LatLng);
const p1world = tr.project(p1LatLng);
const p2world = tr.project(new LngLat(p0LatLng.lng, p1LatLng.lat));
const p3world = tr.project(new LngLat(p1LatLng.lng, p0LatLng.lat));
const p0rotated = p0world.rotate(-degToRad(bearing));
const p1rotated = p1world.rotate(-degToRad(bearing));

const upperRight = new Point(Math.max(p0rotated.x, p1rotated.x), Math.max(p0rotated.y, p1rotated.y));
const lowerLeft = new Point(Math.min(p0rotated.x, p1rotated.x), Math.min(p0rotated.y, p1rotated.y));
const p2rotated = p2world.rotate(-bearing * Math.PI / 180);
const p3rotated = p3world.rotate(-bearing * Math.PI / 180);

const upperRight = new Point(
Math.max(p0rotated.x, p1rotated.x, p2rotated.x, p3rotated.x),
Math.max(p0rotated.y, p1rotated.y, p2rotated.y, p3rotated.y)
);
const lowerLeft = new Point(
Math.min(p0rotated.x, p1rotated.x, p2rotated.x, p3rotated.x),
Math.min(p0rotated.y, p1rotated.y, p2rotated.y, p3rotated.y)
);

// Calculate zoom: consider the original bbox and padding.
const size = upperRight.sub(lowerLeft);
Expand Down
32 changes: 21 additions & 11 deletions test/unit/ui/camera.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1832,7 +1832,7 @@ test('camera', (t) => {

const transform = camera.cameraForBounds(bb, {bearing: 175});
t.deepEqual(fixedLngLat(transform.center, 4), {lng: -100.5, lat: 34.7171}, 'correctly calculates coordinates for new bounds');
t.equal(fixedNum(transform.zoom, 3), 2.558);
t.equal(fixedNum(transform.zoom, 3), 2.396);
t.equal(transform.bearing, 175);
t.end();
});
Expand All @@ -1843,7 +1843,7 @@ test('camera', (t) => {

const transform = camera.cameraForBounds(bb, {bearing: -30});
t.deepEqual(fixedLngLat(transform.center, 4), {lng: -100.5, lat: 34.7171}, 'correctly calculates coordinates for new bounds');
t.equal(fixedNum(transform.zoom, 3), 2.392);
t.equal(fixedNum(transform.zoom, 3), 2.222);
t.equal(transform.bearing, -30);
t.end();
});
Expand Down Expand Up @@ -1945,6 +1945,16 @@ test('camera', (t) => {
t.end();
});

t.test('padding is calculated with bearing', (t) => {
const camera = createCamera();
const bb = [[-133, 16], [-68, 50]];

camera.fitBounds(bb, {bearing: 45, duration:0});
t.deepEqual(fixedLngLat(camera.getCenter(), 4), {lng: -100.5, lat: 34.7171}, 'pans to coordinates based on fitBounds with bearing applied');
t.equal(fixedNum(camera.getZoom(), 3), 2.254);
t.end();
});

t.test('padding object', (t) => {
const camera = createCamera();
const bb = [[-133, 16], [-68, 50]];
Expand Down Expand Up @@ -1976,12 +1986,12 @@ test('camera', (t) => {
t.test('bearing 225', (t) => {
const camera = createCamera();
const p0 = [128, 128];
const p1 = [256, 256];
const p1 = [256, 384];
const bearing = 225;

camera.fitScreenCoordinates(p0, p1, bearing, {duration:0});
t.deepEqual(fixedLngLat(camera.getCenter(), 4), {lng: -45, lat: 40.9799}, 'centers, rotates 225 degrees, and zooms based on screen coordinates');
t.equal(fixedNum(camera.getZoom(), 3), 1.5);
t.deepEqual(fixedLngLat(camera.getCenter(), 4), {lng: -45, lat: 0}, 'centers, rotates 225 degrees, and zooms based on screen coordinates');
t.equal(fixedNum(camera.getZoom(), 3), 0.915); // 0.915 ~= log2(4*sqrt(2)/3)
t.equal(camera.getBearing(), -135);
t.equal(camera.getPitch(), 0);
t.end();
Expand Down Expand Up @@ -2022,25 +2032,25 @@ test('camera', (t) => {
const camera = createCamera();

const p0 = [128, 128];
const p1 = [256, 256];
const p1 = [256, 384];
const bearing = 0;

camera.fitScreenCoordinates(p0, p1, bearing, {duration:0});
t.deepEqual(fixedLngLat(camera.getCenter(), 4), {lng: -45, lat: 40.9799}, 'centers and zooms in based on screen coordinates');
t.equal(fixedNum(camera.getZoom(), 3), 2);
t.deepEqual(fixedLngLat(camera.getCenter(), 4), {lng: -45, lat: 0}, 'centers and zooms in based on screen coordinates');
t.equal(fixedNum(camera.getZoom(), 3), 1);
t.equal(camera.getBearing(), 0);
t.end();
});

t.test('inverted points', (t) => {
const camera = createCamera();
const p1 = [128, 128];
const p0 = [256, 256];
const p0 = [256, 384];
const bearing = 0;

camera.fitScreenCoordinates(p0, p1, bearing, {duration:0});
t.deepEqual(fixedLngLat(camera.getCenter(), 4), {lng: -45, lat: 40.9799}, 'centers and zooms based on screen coordinates in opposite order');
t.equal(fixedNum(camera.getZoom(), 3), 2);
t.deepEqual(fixedLngLat(camera.getCenter(), 4), {lng: -45, lat: 0}, 'centers and zooms based on screen coordinates in opposite order');
t.equal(fixedNum(camera.getZoom(), 3), 1);
t.equal(camera.getBearing(), 0);
t.end();
});
Expand Down

0 comments on commit 3f151d8

Please sign in to comment.