From b1f0effb204510464505f280c5990f543c2bb646 Mon Sep 17 00:00:00 2001 From: Vladimir Agafonkin Date: Wed, 16 Dec 2020 13:45:55 +0200 Subject: [PATCH 1/6] fix unprojecting points in 2D mode --- src/geo/transform.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/geo/transform.js b/src/geo/transform.js index 7bc0dd88ce1..27d2f9ef089 100644 --- a/src/geo/transform.js +++ b/src/geo/transform.js @@ -1012,8 +1012,7 @@ class Transform { } /** - * Returns position oh horizon line, from the top, in pixels. If horizon is not - * visible, returns 0. + * Returns position of horizon line from the top of the map in pixels. * @private */ horizonLineFromTop(): number { @@ -1021,8 +1020,7 @@ class Transform { const h = this.height / 2 / Math.tan(this._fov / 2) / Math.tan(Math.max(this._pitch, 0.1)) + this.centerOffset.y; // incorporate 3% of the area above center to account for reduced precision. const horizonEpsilon = 0.03; - const offset = this.height / 2 - h * (1 - horizonEpsilon); - return Math.max(0, offset); + return this.height / 2 - h * (1 - horizonEpsilon); } /** From 9f283102019419e52d7f586da479696adc0735a3 Mon Sep 17 00:00:00 2001 From: Vladimir Agafonkin Date: Thu, 17 Dec 2020 14:02:36 +0200 Subject: [PATCH 2/6] try a different fix (don't clamp if no horizon) --- src/geo/transform.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/geo/transform.js b/src/geo/transform.js index 27d2f9ef089..1c6afff5901 100644 --- a/src/geo/transform.js +++ b/src/geo/transform.js @@ -902,7 +902,7 @@ class Transform { pointCoordinate(p: Point): MercatorCoordinate { // For p above horizon, don't return point behind camera but clamp p.y at horizon line. const horizonOffset = this.horizonLineFromTop(); - const clamped = horizonOffset > p.y ? new Point(p.x, horizonOffset) : p; + const clamped = horizonOffset > 0 && horizonOffset > p.y ? new Point(p.x, horizonOffset) : p; return this.rayIntersectionCoordinate(this.pointRayIntersection(clamped)); } @@ -1012,7 +1012,7 @@ class Transform { } /** - * Returns position of horizon line from the top of the map in pixels. + * Returns position of horizon line from the top of the map in pixels. If horizon is not visible, returns 0. * @private */ horizonLineFromTop(): number { @@ -1020,7 +1020,8 @@ class Transform { const h = this.height / 2 / Math.tan(this._fov / 2) / Math.tan(Math.max(this._pitch, 0.1)) + this.centerOffset.y; // incorporate 3% of the area above center to account for reduced precision. const horizonEpsilon = 0.03; - return this.height / 2 - h * (1 - horizonEpsilon); + const offset = this.height / 2 - h * (1 - horizonEpsilon); + return Math.max(0, offset); } /** From 41901ffcbe9f76823b32e940defddd435f2b4c7a Mon Sep 17 00:00:00 2001 From: Vladimir Agafonkin Date: Thu, 17 Dec 2020 15:42:07 +0200 Subject: [PATCH 3/6] adjust source cache unit tests --- test/unit/source/source_cache.test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/unit/source/source_cache.test.js b/test/unit/source/source_cache.test.js index 1391a82982f..c0cd851c0ce 100644 --- a/test/unit/source/source_cache.test.js +++ b/test/unit/source/source_cache.test.js @@ -1435,11 +1435,11 @@ test('SourceCache#tilesIn', (t) => { t.equal(tiles[0].tile.tileID.key, 16); t.equal(tiles[0].tile.tileSize, 512); - t.deepEqual(round(tiles[0].bufferedTilespaceBounds), {min: {x: 4080, y: 4050}, max: {x:8192, y: 8162}}); + t.deepEqual(round(tiles[0].bufferedTilespaceBounds), {min: {x: 4080, y: 4034}, max: {x:8192, y: 8162}}); t.equal(tiles[1].tile.tileID.key, 528); t.equal(tiles[1].tile.tileSize, 512); - t.deepEqual(round(tiles[1].bufferedTilespaceBounds), {min: {x: 0, y: 4050}, max: {x: 4112, y: 8162}}); + t.deepEqual(round(tiles[1].bufferedTilespaceBounds), {min: {x: 0, y: 4034}, max: {x: 4112, y: 8162}}); t.end(); } @@ -1484,11 +1484,11 @@ test('SourceCache#tilesIn', (t) => { t.equal(tiles[0].tile.tileID.key, 17); t.equal(tiles[0].tile.tileSize, 1024); - t.deepEqual(round(tiles[0].bufferedTilespaceBounds), {min: {x: 4088, y: 4050}, max: {x:8192, y: 8154}}); + t.deepEqual(round(tiles[0].bufferedTilespaceBounds), {min: {x: 4088, y: 4042}, max: {x:8192, y: 8154}}); t.equal(tiles[1].tile.tileID.key, 529); t.equal(tiles[1].tile.tileSize, 1024); - t.deepEqual(round(tiles[1].bufferedTilespaceBounds), {min: {x: 0, y: 4050}, max: {x: 4104, y: 8154}}); + t.deepEqual(round(tiles[1].bufferedTilespaceBounds), {min: {x: 0, y: 4042}, max: {x: 4104, y: 8154}}); t.end(); } From b444d0b1300d49e5fdd1ee8f786c3242846e6fa6 Mon Sep 17 00:00:00 2001 From: Arindam Bose Date: Thu, 17 Dec 2020 13:46:29 -0800 Subject: [PATCH 4/6] Add option for unclamped horizon from top --- src/geo/transform.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/geo/transform.js b/src/geo/transform.js index 1c6afff5901..634294aa4cc 100644 --- a/src/geo/transform.js +++ b/src/geo/transform.js @@ -900,9 +900,8 @@ class Transform { * @private */ pointCoordinate(p: Point): MercatorCoordinate { - // For p above horizon, don't return point behind camera but clamp p.y at horizon line. - const horizonOffset = this.horizonLineFromTop(); - const clamped = horizonOffset > 0 && horizonOffset > p.y ? new Point(p.x, horizonOffset) : p; + const horizonOffset = this.horizonLineFromTop(false); + const clamped = new Point(p.x, Math.max(horizonOffset, p.y)); return this.rayIntersectionCoordinate(this.pointRayIntersection(clamped)); } @@ -1015,13 +1014,13 @@ class Transform { * Returns position of horizon line from the top of the map in pixels. If horizon is not visible, returns 0. * @private */ - horizonLineFromTop(): number { + horizonLineFromTop(clampToTop: boolean = true): number { // h is height of space above map center to horizon. const h = this.height / 2 / Math.tan(this._fov / 2) / Math.tan(Math.max(this._pitch, 0.1)) + this.centerOffset.y; // incorporate 3% of the area above center to account for reduced precision. const horizonEpsilon = 0.03; const offset = this.height / 2 - h * (1 - horizonEpsilon); - return Math.max(0, offset); + return clampToTop ? Math.max(0, offset) : offset; } /** From 79db2f28fee0f7b9202ed2fc13dd1a684a9716e4 Mon Sep 17 00:00:00 2001 From: Arindam Bose Date: Mon, 21 Dec 2020 15:56:54 -0800 Subject: [PATCH 5/6] Add unit tests --- test/unit/geo/transform.test.js | 54 +++++++++++++++++++++++++++++++++ test/unit/ui/camera.test.js | 11 +++++++ 2 files changed, 65 insertions(+) diff --git a/test/unit/geo/transform.test.js b/test/unit/geo/transform.test.js index 8612df08a08..5a2a5a69dd7 100644 --- a/test/unit/geo/transform.test.js +++ b/test/unit/geo/transform.test.js @@ -177,6 +177,60 @@ test('transform', (t) => { t.end(); }); + test('pointCoordinate retains direction when point is offscreen', (t) => { + + function assertDueNorth(t, m1, m2) { + const dx = m2.x - m1.x; + const dy = m2.y - m1.y; + const l = Math.sqrt(dx * dx + dy * dy); + const ndx = dx / l; + const ndy = dy / l; + t.ok(Math.abs(ndx) < 1e-10); + t.ok(dy < 0); + } + + t.test('no pitch', (t) => { + const transform = new Transform(); + transform.center = {lng: 0, lat: 0}; + transform.zoom = 16; + transform.pitch = 0; + transform.bearing = 0; + transform.resize(512, 512); + + const coord = transform.pointCoordinate(new Point(transform.width/2, -10000)); + assertDueNorth(t, { x: 0.5, y: 0.5, z : 0}, coord); + t.end(); + }); + + t.test('high pitch', (t) => { + const transform = new Transform(); + transform.center = {lng: 0, lat: 0}; + transform.zoom = 16; + transform.pitch = 80; + transform.bearing = 0; + transform.resize(512, 512); + + const coord = transform.pointCoordinate(new Point(transform.width/2, -10000)); + assertDueNorth(t, { x: 0.5, y: 0.5, z : 0}, coord); + t.end(); + }); + + t.test('medium pitch', (t) => { + const transform = new Transform(); + transform.center = {lng: 0, lat: 0}; + transform.zoom = 16; + transform.pitch = 70; + transform.bearing = 0; + transform.resize(512, 512); + + const coord = transform.pointCoordinate(new Point(transform.width/2, -10000)); + assertDueNorth(t, { x: 0.5, y: 0.5, z : 0}, coord); + t.end(); + }); + + t.end(); + }); + test('coveringTiles', (t) => { const options = { minzoom: 1, diff --git a/test/unit/ui/camera.test.js b/test/unit/ui/camera.test.js index 3d24c2373f9..63dae8290f5 100644 --- a/test/unit/ui/camera.test.js +++ b/test/unit/ui/camera.test.js @@ -374,6 +374,17 @@ test('camera', (t) => { t.end(); }); + t.test('pans equally in both directions', (t) => { + const camera = createCamera({bearing: 0}); + const c = camera.getCenter(); + camera.panBy([0, -10000], {duration: 0}); + const c1 = camera.getCenter(); + camera.panBy([0, 10000], {duration: 0}); + const c2 = camera.getCenter(); + t.ok(Math.abs(c1.lat - c.lat) - Math.abs(c2.lat - c.lat) < 1e-10); + t.end(); + }); + t.test('emits move events, preserving eventData', (t) => { const camera = createCamera(); let started, moved; From 5910899abc45f12843dfcdaef9653eefe43f1fda Mon Sep 17 00:00:00 2001 From: Arindam Bose Date: Mon, 21 Dec 2020 16:09:38 -0800 Subject: [PATCH 6/6] Fix lint errors --- test/unit/geo/transform.test.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/unit/geo/transform.test.js b/test/unit/geo/transform.test.js index 5a2a5a69dd7..84c92497aca 100644 --- a/test/unit/geo/transform.test.js +++ b/test/unit/geo/transform.test.js @@ -186,7 +186,7 @@ test('transform', (t) => { const ndx = dx / l; const ndy = dy / l; t.ok(Math.abs(ndx) < 1e-10); - t.ok(dy < 0); + t.ok(Math.abs(ndy + 1) < 1e-10); } t.test('no pitch', (t) => { @@ -197,8 +197,8 @@ test('transform', (t) => { transform.bearing = 0; transform.resize(512, 512); - const coord = transform.pointCoordinate(new Point(transform.width/2, -10000)); - assertDueNorth(t, { x: 0.5, y: 0.5, z : 0}, coord); + const coord = transform.pointCoordinate(new Point(transform.width / 2, -10000)); + assertDueNorth(t, {x: 0.5, y: 0.5, z : 0}, coord); t.end(); }); @@ -210,8 +210,8 @@ test('transform', (t) => { transform.bearing = 0; transform.resize(512, 512); - const coord = transform.pointCoordinate(new Point(transform.width/2, -10000)); - assertDueNorth(t, { x: 0.5, y: 0.5, z : 0}, coord); + const coord = transform.pointCoordinate(new Point(transform.width / 2, -10000)); + assertDueNorth(t, {x: 0.5, y: 0.5, z : 0}, coord); t.end(); }); @@ -223,8 +223,8 @@ test('transform', (t) => { transform.bearing = 0; transform.resize(512, 512); - const coord = transform.pointCoordinate(new Point(transform.width/2, -10000)); - assertDueNorth(t, { x: 0.5, y: 0.5, z : 0}, coord); + const coord = transform.pointCoordinate(new Point(transform.width / 2, -10000)); + assertDueNorth(t, {x: 0.5, y: 0.5, z : 0}, coord); t.end(); });