From b96ad6c664fbedf35bdc5b04da88959b43b8c96c Mon Sep 17 00:00:00 2001 From: Aidan Date: Fri, 30 Jul 2021 09:12:06 -0600 Subject: [PATCH 1/8] Added tests, should be failing --- test/unit/geo/transform.test.js | 36 +++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/unit/geo/transform.test.js b/test/unit/geo/transform.test.js index a5abf28c2e0..5bdb7c230b2 100644 --- a/test/unit/geo/transform.test.js +++ b/test/unit/geo/transform.test.js @@ -117,6 +117,42 @@ test('transform', (t) => { t.end(); }); + t.test('maxBounds snaps to correct side when crossing 180th meridian (#10447)', (t) => { + console.log("start snap test"); + const transform = new Transform(); + transform.zoom = 6; + transform.resize(500, 500); + + transform.lngRange = [160, 190]; + transform.latRange = [-55, -23]; + + transform.center = new LngLat(-170, -40); + + t.ok(transform.center.lng < 190); + t.ok(transform.center.lng > 175); + + t.end(); + }); + + t.test('maxBounds snaps to correct side when crossing 180th meridian on the West', (t) => { + console.log("start snap test"); + const transform = new Transform(); + transform.zoom = 6; + transform.resize(500, 500); + + transform.lngRange = [-190, -160]; + transform.latRange = [-55, -23]; + + transform.center = new LngLat(190, -40); + + console.log("transform center lng"); + console.log(transform.center); + t.ok(transform.center.lng > -190); + t.ok(transform.center.lng < -175); + + t.end(); + }); + t.test('_minZoomForBounds respects latRange and lngRange', (t) => { t.test('it returns 0 when latRange and lngRange are undefined', (t) => { const transform = new Transform(); From 794fd7e509b35771e746a886be7cb63dca92b8b7 Mon Sep 17 00:00:00 2001 From: Aidan Date: Fri, 30 Jul 2021 09:28:39 -0600 Subject: [PATCH 2/8] Tests passing. Added comments in transform.js --- src/geo/transform.js | 15 ++++++++++++--- test/unit/geo/transform.test.js | 2 +- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/geo/transform.js b/src/geo/transform.js index 692ea9760cb..702508b9e01 100644 --- a/src/geo/transform.js +++ b/src/geo/transform.js @@ -2,7 +2,7 @@ import LngLat from './lng_lat.js'; import LngLatBounds from './lng_lat_bounds.js'; -import MercatorCoordinate, {mercatorXfromLng, mercatorYfromLat, mercatorZfromAltitude, latFromMercatorY} from './mercator_coordinate.js'; +import MercatorCoordinate, {mercatorXfromLng, mercatorYfromLat, mercatorZfromAltitude, latFromMercatorY, lngFromMercatorX} from './mercator_coordinate.js'; import Point from '@mapbox/point-geometry'; import {wrap, clamp, radToDeg, degToRad, getAABBPointSquareDist, furthestTileCorner} from '../util/util.js'; import {number as interpolate} from '../style-spec/util/interpolate.js'; @@ -887,6 +887,7 @@ class Transform { zoomScale(zoom: number) { return Math.pow(2, zoom); } scaleZoom(scale: number) { return Math.log(scale) / Math.LN2; } + // World coordinates from LngLat project(lnglat: LngLat) { const lat = clamp(lnglat.lat, -this.maxValidLatitude, this.maxValidLatitude); return new Point( @@ -894,10 +895,12 @@ class Transform { mercatorYfromLat(lat) * this.worldSize); } + // LngLat from world coordinates unproject(point: Point): LngLat { return new MercatorCoordinate(point.x / this.worldSize, point.y / this.worldSize).toLngLat(); } + // Point at center in world coordinates. get point(): Point { return this.project(this.center); } setLocationAtPoint(lnglat: LngLat, point: Point) { @@ -1401,8 +1404,14 @@ class Transform { } if (this.lngRange) { - const x = point.x, - w2 = size.x / 2; + let x = point.x; + const w2 = size.x / 2; + + // If the left edge is more than 180 degrees below the minimum boundary, add 360 degrees to the value. + if (x - w2 - minX < -this.worldSize / 2) x += this.worldSize; + + // If the right edge is more than 180 degrees beyond the max boundary, add 360 degrees to the value. + if (x + w2 - maxX > this.worldSize / 2) x -= this.worldSize; if (x - w2 < minX) x2 = minX + w2; if (x + w2 > maxX) x2 = maxX - w2; diff --git a/test/unit/geo/transform.test.js b/test/unit/geo/transform.test.js index 5bdb7c230b2..5ef9ed33004 100644 --- a/test/unit/geo/transform.test.js +++ b/test/unit/geo/transform.test.js @@ -143,7 +143,7 @@ test('transform', (t) => { transform.lngRange = [-190, -160]; transform.latRange = [-55, -23]; - transform.center = new LngLat(190, -40); + transform.center = new LngLat(170, -40); console.log("transform center lng"); console.log(transform.center); From 528bd8da39793f0fed317074506742725623253b Mon Sep 17 00:00:00 2001 From: Aidan Date: Fri, 30 Jul 2021 18:08:55 -0600 Subject: [PATCH 3/8] Added wrapping to bounds check. More tests and debug page. --- debug/antimeridian.html | 41 ++++++++++++++++++ src/geo/transform.js | 21 +++++----- test/unit/geo/transform.test.js | 74 ++++++++++++++++++++++----------- 3 files changed, 101 insertions(+), 35 deletions(-) create mode 100644 debug/antimeridian.html diff --git a/debug/antimeridian.html b/debug/antimeridian.html new file mode 100644 index 00000000000..3675c21bc57 --- /dev/null +++ b/debug/antimeridian.html @@ -0,0 +1,41 @@ + + + + The Zealand conspiracy + + + + + + + +
+ + + + + + + diff --git a/src/geo/transform.js b/src/geo/transform.js index 702508b9e01..89ae5bc7a33 100644 --- a/src/geo/transform.js +++ b/src/geo/transform.js @@ -1362,7 +1362,7 @@ class Transform { let maxY = 90; let minX = -180; let maxX = 180; - let sy, sx, x2, y2; + let sy, sx, y2; const size = this.size, unmodified = this._unmodified; @@ -1403,24 +1403,23 @@ class Transform { if (y + h2 > maxY) y2 = maxY - h2; } + let x = point.x; + if (this.lngRange) { - let x = point.x; const w2 = size.x / 2; - // If the left edge is more than 180 degrees below the minimum boundary, add 360 degrees to the value. - if (x - w2 - minX < -this.worldSize / 2) x += this.worldSize; - - // If the right edge is more than 180 degrees beyond the max boundary, add 360 degrees to the value. - if (x + w2 - maxX > this.worldSize / 2) x -= this.worldSize; + // Wrapping. + if (x < minX) x += this.worldSize; + if (x > maxX) x -= this.worldSize; - if (x - w2 < minX) x2 = minX + w2; - if (x + w2 > maxX) x2 = maxX - w2; + if (x - w2 < minX) x = minX + w2; + if (x + w2 > maxX) x = maxX - w2; } // pan the map if the screen goes off the range - if (x2 !== undefined || y2 !== undefined) { + if (x !== point.x || y2 !== undefined) { this.center = this.unproject(new Point( - x2 !== undefined ? x2 : point.x, + x, y2 !== undefined ? y2 : point.y)); } diff --git a/test/unit/geo/transform.test.js b/test/unit/geo/transform.test.js index 5ef9ed33004..7fd499c4d82 100644 --- a/test/unit/geo/transform.test.js +++ b/test/unit/geo/transform.test.js @@ -117,40 +117,66 @@ test('transform', (t) => { t.end(); }); - t.test('maxBounds snaps to correct side when crossing 180th meridian (#10447)', (t) => { - console.log("start snap test"); - const transform = new Transform(); - transform.zoom = 6; - transform.resize(500, 500); + t.test('maxBounds should not jump to the wrong side when crossing 180th meridian (#10447)', (t) => { + t.test(' to the East', (t) => { + const transform = new Transform(); + transform.zoom = 6; + transform.resize(500, 500); + transform.lngRange = [160, 190]; + transform.latRange = [-55, -23]; - transform.lngRange = [160, 190]; - transform.latRange = [-55, -23]; + transform.center = new LngLat(-170, -40); - transform.center = new LngLat(-170, -40); + t.ok(transform.center.lng < 190); + t.ok(transform.center.lng > 175); - t.ok(transform.center.lng < 190); - t.ok(transform.center.lng > 175); + t.end(); + }); - t.end(); - }); + t.test('to the West', (t) => { + const transform = new Transform(); + transform.zoom = 6; + transform.resize(500, 500); + transform.lngRange = [-190, -160]; + transform.latRange = [-55, -23]; - t.test('maxBounds snaps to correct side when crossing 180th meridian on the West', (t) => { - console.log("start snap test"); - const transform = new Transform(); - transform.zoom = 6; - transform.resize(500, 500); + transform.center = new LngLat(170, -40); - transform.lngRange = [-190, -160]; - transform.latRange = [-55, -23]; + t.ok(transform.center.lng > -190); + t.ok(transform.center.lng < -175); - transform.center = new LngLat(170, -40); + t.end(); + }); - console.log("transform center lng"); - console.log(transform.center); - t.ok(transform.center.lng > -190); - t.ok(transform.center.lng < -175); + t.test('longitude 0 - 360', (t) => { + const transform = new Transform(); + transform.zoom = 6; + transform.resize(500, 500); + transform.lngRange = [0, 360]; + transform.latRange = [-90, 90]; + + transform.center = new LngLat(-155, 0); + + t.same(transform.center, new LngLat(205, 0)); + + t.end(); + }); + + t.test('longitude -360 - 0', (t) => { + const transform = new Transform(); + transform.zoom = 6; + transform.resize(500, 500); + transform.lngRange = [-360, 0]; + transform.latRange = [-90, 90]; + + transform.center = new LngLat(160, 0); + t.same(transform.center, new LngLat(-200, 0)); + + t.end(); + }); t.end(); + }); t.test('_minZoomForBounds respects latRange and lngRange', (t) => { From 4f8f121826585c163113224b234f65e776924c4e Mon Sep 17 00:00:00 2001 From: Aidan Date: Sun, 1 Aug 2021 14:55:46 -0600 Subject: [PATCH 4/8] Changed wrapping approach to fix bug where map could be forced to other side --- debug/antimeridian.html | 4 +-- src/geo/transform.js | 25 +++++++++++------- test/unit/geo/transform.test.js | 47 ++++++++++++++++++++++++++++++++- 3 files changed, 64 insertions(+), 12 deletions(-) diff --git a/debug/antimeridian.html b/debug/antimeridian.html index 3675c21bc57..f2bc9c56bd7 100644 --- a/debug/antimeridian.html +++ b/debug/antimeridian.html @@ -1,7 +1,7 @@ - The Zealand conspiracy + Mapbox GL JS debug page @@ -24,7 +24,7 @@ style: 'mapbox://styles/mapbox/streets-v11', zoom: 6, center: [179.012, -18.124], - maxBounds: [[175, -21.943], [360 - 175, -12.261]] + maxBounds: [[175, -25], [-175, -10]] }); map.transform.center = new mapboxgl.LngLat(190, -40); diff --git a/src/geo/transform.js b/src/geo/transform.js index 89ae5bc7a33..4ffd53726a5 100644 --- a/src/geo/transform.js +++ b/src/geo/transform.js @@ -2,7 +2,7 @@ import LngLat from './lng_lat.js'; import LngLatBounds from './lng_lat_bounds.js'; -import MercatorCoordinate, {mercatorXfromLng, mercatorYfromLat, mercatorZfromAltitude, latFromMercatorY, lngFromMercatorX} from './mercator_coordinate.js'; +import MercatorCoordinate, {mercatorXfromLng, mercatorYfromLat, mercatorZfromAltitude, latFromMercatorY} from './mercator_coordinate.js'; import Point from '@mapbox/point-geometry'; import {wrap, clamp, radToDeg, degToRad, getAABBPointSquareDist, furthestTileCorner} from '../util/util.js'; import {number as interpolate} from '../style-spec/util/interpolate.js'; @@ -1213,7 +1213,10 @@ class Transform { */ setMaxBounds(bounds?: LngLatBounds) { if (bounds) { - this.lngRange = [bounds.getWest(), bounds.getEast()]; + const eastBound = bounds.getEast(); + const westBound = bounds.getWest(); + // Unwrap bounds if they cross the 180th meridian + this.lngRange = [westBound, eastBound > westBound ? eastBound : eastBound + 360]; this.latRange = [bounds.getSouth(), bounds.getNorth()]; this._constrain(); } else { @@ -1358,8 +1361,8 @@ class Transform { this._constraining = true; - let minY = -90; - let maxY = 90; + let minY; + let maxY; let minX = -180; let maxX = 180; let sy, sx, y2; @@ -1406,14 +1409,18 @@ class Transform { let x = point.x; if (this.lngRange) { - const w2 = size.x / 2; - - // Wrapping. - if (x < minX) x += this.worldSize; - if (x > maxX) x -= this.worldSize; + // Translate to positive positions with the map center in the center position. + // This ensures that the map snaps to the correct edge. + const shift = this.worldSize / 2 - (minX + maxX) / 2; + x = (x + shift + this.worldSize) % this.worldSize; + minX += shift; + maxX += shift; + const w2 = size.x / 2; if (x - w2 < minX) x = minX + w2; if (x + w2 > maxX) x = maxX - w2; + + x -= shift; } // pan the map if the screen goes off the range diff --git a/test/unit/geo/transform.test.js b/test/unit/geo/transform.test.js index 7fd499c4d82..cc2a6534072 100644 --- a/test/unit/geo/transform.test.js +++ b/test/unit/geo/transform.test.js @@ -170,7 +170,7 @@ test('transform', (t) => { transform.latRange = [-90, 90]; transform.center = new LngLat(160, 0); - t.same(transform.center, new LngLat(-200, 0)); + t.same(transform.center.lng.toFixed(10), -200); t.end(); }); @@ -179,6 +179,51 @@ test('transform', (t) => { }); + t.test('maxBounds snaps in the correct direction (no forcing to other edge when width < 360)', (t) => { + const transform = new Transform(); + transform.zoom = 6; + transform.resize(500, 500); + transform.setMaxBounds(new LngLatBounds([-160, -20], [160, 20])); + + transform.center = new LngLat(170, 0); + t.ok(transform.center.lng > 150); + t.ok(transform.center.lng < 160); + + transform.center = new LngLat(-170, 0); + t.ok(transform.center.lng > -160); + t.ok(transform.center.lng < -150); + + t.end(); + }); + + t.test('maxBounds works with unwrapped values across the 180th meridian (#6985)', (t) => { + const transform = new Transform(); + transform.zoom = 6; + transform.resize(500, 500); + transform.setMaxBounds(new LngLatBounds([160, -20], [-160, 20])); //East bound is "smaller" + console.log(transform.getMaxBounds()); + + const wrap = val => ((val + 360) % 360); + + transform.center = new LngLat(170, 0); + t.same(wrap(transform.center.lng), 170); + + transform.center = new LngLat(-170, 0); + t.same(wrap(transform.center.lng), wrap(-170)); + + transform.center = new LngLat(150, 0); + let lng = wrap(transform.center.lng); + t.ok(lng > 160); + t.ok(lng < 180); + + transform.center = new LngLat(-150, 0); + lng = wrap(transform.center.lng); + t.ok(lng < 360 - 160); + t.ok(lng > 360 - 180); + + t.end(); + }); + t.test('_minZoomForBounds respects latRange and lngRange', (t) => { t.test('it returns 0 when latRange and lngRange are undefined', (t) => { const transform = new Transform(); From 72f3b837f350f95fab2a4146d96272538c5c3811 Mon Sep 17 00:00:00 2001 From: Aidan Date: Sun, 1 Aug 2021 15:11:12 -0600 Subject: [PATCH 5/8] Clean up misleading variables --- src/geo/transform.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/geo/transform.js b/src/geo/transform.js index 4ffd53726a5..f715c18d673 100644 --- a/src/geo/transform.js +++ b/src/geo/transform.js @@ -1361,11 +1361,9 @@ class Transform { this._constraining = true; - let minY; - let maxY; - let minX = -180; - let maxX = 180; - let sy, sx, y2; + let minY = 0; + let maxY = 0; + let minX, maxX, sy, sx, y2; const size = this.size, unmodified = this._unmodified; From bee55d7945c037fedbc3bc468a270f7227c2e676 Mon Sep 17 00:00:00 2001 From: Aidan Date: Sun, 1 Aug 2021 15:21:41 -0600 Subject: [PATCH 6/8] Removing log from test --- test/unit/geo/transform.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit/geo/transform.test.js b/test/unit/geo/transform.test.js index cc2a6534072..0744b3278f1 100644 --- a/test/unit/geo/transform.test.js +++ b/test/unit/geo/transform.test.js @@ -201,7 +201,6 @@ test('transform', (t) => { transform.zoom = 6; transform.resize(500, 500); transform.setMaxBounds(new LngLatBounds([160, -20], [-160, 20])); //East bound is "smaller" - console.log(transform.getMaxBounds()); const wrap = val => ((val + 360) % 360); From 69e8f8422e96e90fc2694d7d0aa550b96bcd9bfa Mon Sep 17 00:00:00 2001 From: Aidan H Date: Mon, 9 Aug 2021 14:22:06 -0600 Subject: [PATCH 7/8] Suggestions from rreusser --- src/geo/transform.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/geo/transform.js b/src/geo/transform.js index f715c18d673..d2b71fcb35f 100644 --- a/src/geo/transform.js +++ b/src/geo/transform.js @@ -887,7 +887,7 @@ class Transform { zoomScale(zoom: number) { return Math.pow(2, zoom); } scaleZoom(scale: number) { return Math.log(scale) / Math.LN2; } - // World coordinates from LngLat + // Transform from LngLat to Point in world coordinates [-180, 180] x [90, -90] --> [0, this.worldSize] x [0, this.worldSize] project(lnglat: LngLat) { const lat = clamp(lnglat.lat, -this.maxValidLatitude, this.maxValidLatitude); return new Point( @@ -895,7 +895,7 @@ class Transform { mercatorYfromLat(lat) * this.worldSize); } - // LngLat from world coordinates + // Transform from Point in world coordinates to LngLat [0, this.worldSize] x [0, this.worldSize] --> [-180, 180] x [90, -90] unproject(point: Point): LngLat { return new MercatorCoordinate(point.x / this.worldSize, point.y / this.worldSize).toLngLat(); } @@ -1361,8 +1361,8 @@ class Transform { this._constraining = true; - let minY = 0; - let maxY = 0; + let minY = Infinity; + let maxY = -Infinity; let minX, maxX, sy, sx, y2; const size = this.size, unmodified = this._unmodified; From f7ecfcc2af62daea7b01520ec0cb8afc5ad6904b Mon Sep 17 00:00:00 2001 From: Aidan H Date: Thu, 12 Aug 2021 11:40:49 -0600 Subject: [PATCH 8/8] Apply ricky's suggestions to debug Co-authored-by: Ricky Reusser --- debug/antimeridian.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/debug/antimeridian.html b/debug/antimeridian.html index f2bc9c56bd7..c0f8bfb4338 100644 --- a/debug/antimeridian.html +++ b/debug/antimeridian.html @@ -19,7 +19,7 @@