From dea7367de277c188eb998d10e84b0d46c022358b Mon Sep 17 00:00:00 2001 From: Mike Marcacci Date: Tue, 30 Oct 2018 16:32:26 -0700 Subject: [PATCH 1/5] Fix #7517 - asymmetrical cameraForBounds --- src/ui/camera.js | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/src/ui/camera.js b/src/ui/camera.js index 3cde087716c..3a85c4a44b9 100644 --- a/src/ui/camera.js +++ b/src/ui/camera.js @@ -434,17 +434,9 @@ class Camera extends Evented { return; } - // we separate the passed padding option into two parts, the part that does not affect the map's center - // (lateral and vertical padding), and the part that does (paddingOffset). We add the padding offset - // to the options `offset` object where it can alter the map's center in the subsequent calls to - // `easeTo` and `flyTo`. - const paddingOffset = [(options.padding.left - options.padding.right) / 2, (options.padding.top - options.padding.bottom) / 2], - lateralPadding = Math.min(options.padding.right, options.padding.left), - verticalPadding = Math.min(options.padding.top, options.padding.bottom); - options.offset = [options.offset[0] + paddingOffset[0], options.offset[1] + paddingOffset[1]]; - const tr = this.transform; - // we want to calculate the upper right and lower left of the box defined by p0 and p1 + + // 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)); @@ -454,10 +446,10 @@ class Camera extends Evented { 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 offset = Point.convert(options.offset), - size = upperRight.sub(lowerLeft), - scaleX = (tr.width - lateralPadding * 2 - Math.abs(offset.x) * 2) / size.x, - scaleY = (tr.height - verticalPadding * 2 - Math.abs(offset.y) * 2) / size.y; + // Calculate zoom: consider the original bbox and padding. + const size = upperRight.sub(lowerLeft); + const scaleX = (tr.width - options.padding.left - options.padding.right) / size.x; + const scaleY = (tr.height - options.padding.top - options.padding.bottom) / size.y; if (scaleY < 0 || scaleX < 0) { warnOnce( @@ -465,11 +457,21 @@ class Camera extends Evented { ); return; } - options.center = tr.unproject(p0world.add(p1world).div(2)); - options.zoom = Math.min(tr.scaleZoom(tr.scale * Math.min(scaleX, scaleY)), options.maxZoom); - options.bearing = bearing; - return options; + const zoom = Math.min(tr.scaleZoom(tr.scale * Math.min(scaleX, scaleY)), options.maxZoom); + + // Calculate center: apply the zoom, the configured offset, as well as offset that exists as a result of padding. + const paddingOffset = [(options.padding.left - options.padding.right) / 2, (options.padding.top - options.padding.bottom) / 2]; + const offsetAtInitialZoom = Point.convert([options.offset[0] + paddingOffset[0], options.offset[1] + paddingOffset[1]]); + const offsetAtFinalZoom = offsetAtInitialZoom.mult(tr.scale / tr.zoomScale(zoom)); + + const center = tr.unproject(p0world.add(p1world).div(2).sub(offsetAtFinalZoom)); + + return { + center, + zoom, + bearing + }; } /** From 541265c19044dca6d8e03653db49cdd1aab9f5c5 Mon Sep 17 00:00:00 2001 From: Mike Marcacci Date: Tue, 30 Oct 2018 16:42:42 -0700 Subject: [PATCH 2/5] Add tests for asymmetrical cameraForBounds --- test/unit/ui/camera.test.js | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/test/unit/ui/camera.test.js b/test/unit/ui/camera.test.js index 3c6f60a8f07..117595d71c9 100644 --- a/test/unit/ui/camera.test.js +++ b/test/unit/ui/camera.test.js @@ -1714,11 +1714,38 @@ test('camera', (t) => { const camera = createCamera(); const bb = [[-133, 16], [-68, 50]]; - const transform = camera.cameraForBounds(bb, { padding: {top: 10, right: 75, bottom: 50, left: 25}, duration: 0 }); + const transform = camera.cameraForBounds(bb, { padding: {top: 15, right: 15, bottom: 15, left: 15}, duration: 0 }); t.deepEqual(fixedLngLat(transform.center, 4), { lng: -100.5, lat: 34.7171 }, 'correctly calculates coordinates for bounds with padding option as object applied'); t.end(); }); + t.test('asymetrical padding', (t) => { + const camera = createCamera(); + const bb = [[-133, 16], [-68, 50]]; + + const transform = camera.cameraForBounds(bb, { padding: {top: 10, right: 75, bottom: 50, left: 25}, duration: 0 }); + t.deepEqual(fixedLngLat(transform.center, 4), { lng: -96.5558, lat: 32.0833 }, 'correctly calculates coordinates for bounds with padding option as object applied'); + t.end(); + }); + + t.test('offset', (t) => { + const camera = createCamera(); + const bb = [[-133, 16], [-68, 50]]; + + const transform = camera.cameraForBounds(bb, { offset: [0, 100] }); + t.deepEqual(fixedLngLat(transform.center, 4), { lng: -100.5, lat: 44.4717 }, 'correctly calculates coordinates for bounds with padding option as object applied'); + t.end(); + }); + + t.test('offset and padding', (t) => { + const camera = createCamera(); + const bb = [[-133, 16], [-68, 50]]; + + const transform = camera.cameraForBounds(bb, { padding: {top: 10, right: 75, bottom: 50, left: 25}, offset: [0, 100] }); + t.deepEqual(fixedLngLat(transform.center, 4), { lng: -96.5558, lat: 44.4189 }, 'correctly calculates coordinates for bounds with padding option as object applied'); + t.end(); + }); + t.end(); }); From f7b3517f5374010d46ef5f1f6b26cc707105375e Mon Sep 17 00:00:00 2001 From: Mike Marcacci Date: Tue, 30 Oct 2018 17:08:50 -0700 Subject: [PATCH 3/5] Added debug file for #7517 --- debug/7517.html | 125 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 125 insertions(+) create mode 100644 debug/7517.html diff --git a/debug/7517.html b/debug/7517.html new file mode 100644 index 00000000000..71245d5a2d4 --- /dev/null +++ b/debug/7517.html @@ -0,0 +1,125 @@ + + + + Mapbox GL JS debug page + + + + + + + +
+ + + + + + From abaf876f4acfce893e445f6fa74a62e6f7113e99 Mon Sep 17 00:00:00 2001 From: Mike Marcacci Date: Tue, 30 Oct 2018 17:12:02 -0700 Subject: [PATCH 4/5] Fix lint errors in debug file --- debug/7517.html | 220 +++++++++++++++++++++++++----------------------- 1 file changed, 116 insertions(+), 104 deletions(-) diff --git a/debug/7517.html b/debug/7517.html index 71245d5a2d4..0fb7f135714 100644 --- a/debug/7517.html +++ b/debug/7517.html @@ -1,125 +1,137 @@ + Mapbox GL JS debug page -
- - - - + + + }); + - + + \ No newline at end of file From 044d3d4ddcad486829343eb178dfa6cc0722e865 Mon Sep 17 00:00:00 2001 From: Mike Marcacci Date: Wed, 31 Oct 2018 11:33:46 -0700 Subject: [PATCH 5/5] Make sure offset can be an object --- src/ui/camera.js | 6 ++++-- test/unit/ui/camera.test.js | 9 +++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/ui/camera.js b/src/ui/camera.js index 3a85c4a44b9..06135aea72d 100644 --- a/src/ui/camera.js +++ b/src/ui/camera.js @@ -461,8 +461,10 @@ class Camera extends Evented { const zoom = Math.min(tr.scaleZoom(tr.scale * Math.min(scaleX, scaleY)), options.maxZoom); // Calculate center: apply the zoom, the configured offset, as well as offset that exists as a result of padding. - const paddingOffset = [(options.padding.left - options.padding.right) / 2, (options.padding.top - options.padding.bottom) / 2]; - const offsetAtInitialZoom = Point.convert([options.offset[0] + paddingOffset[0], options.offset[1] + paddingOffset[1]]); + const offset = Point.convert(options.offset); + const paddingOffsetX = (options.padding.left - options.padding.right) / 2; + const paddingOffsetY = (options.padding.top - options.padding.bottom) / 2; + const offsetAtInitialZoom = new Point(offset.x + paddingOffsetX, offset.y + paddingOffsetY); const offsetAtFinalZoom = offsetAtInitialZoom.mult(tr.scale / tr.zoomScale(zoom)); const center = tr.unproject(p0world.add(p1world).div(2).sub(offsetAtFinalZoom)); diff --git a/test/unit/ui/camera.test.js b/test/unit/ui/camera.test.js index 117595d71c9..9aa8db43679 100644 --- a/test/unit/ui/camera.test.js +++ b/test/unit/ui/camera.test.js @@ -1737,6 +1737,15 @@ test('camera', (t) => { t.end(); }); + t.test('offset as object', (t) => { + const camera = createCamera(); + const bb = [[-133, 16], [-68, 50]]; + + const transform = camera.cameraForBounds(bb, { offset: { x: 0, y: 100 } }); + t.deepEqual(fixedLngLat(transform.center, 4), { lng: -100.5, lat: 44.4717 }, 'correctly calculates coordinates for bounds with padding option as object applied'); + t.end(); + }); + t.test('offset and padding', (t) => { const camera = createCamera(); const bb = [[-133, 16], [-68, 50]];