Fix inflated map.getBounds due to horizonShift and edge of map

.flowconfig

@@ -47,7 +47,7 @@
 .*/_site/.*
 
 [version]
-0.100.0
+0.103.0
 
 [options]
 server.max_workers=4

package.json

@@ -69,7 +69,7 @@
     "eslint-plugin-html": "^6.1.1",
     "eslint-plugin-import": "^2.22.1",
     "eslint-plugin-jsdoc": "^32.3.4",
-    "flow-bin": "^0.100.0",
+    "flow-bin": "0.103.0",
     "gl": "^4.9.0",
     "glob": "^7.1.6",
     "is-builtin-module": "^3.0.0",

src/geo/transform.js

@@ -687,7 +687,7 @@ class Transform {
 
         // Do a depth-first traversal to find visible tiles and proper levels of detail
         const stack = [];
-        const result = [];
+        let result = [];
         const maxZoom = z;
         const overscaledZ = options.reparseOverscaled ? actualZ : z;
 
@@ -820,7 +820,8 @@ class Transform {
 
         if (this.fogCullDistSq) {
             const fogCullDistSq = this.fogCullDistSq;
-            result.splice(0, result.length, ...result.filter(entry => {
+            const horizonLineFromTop = this.horizonLineFromTop();
+            result = result.filter(entry => {
                 const min = [0, 0, 0, 1];
                 const max = [EXTENT, EXTENT, 0, 1];
 
@@ -834,8 +835,14 @@ class Transform {
                 if (sqDist === 0) { return true; }
 
                 let overHorizonLine = false;
-                const horizonLineFromTop = this.horizonLineFromTop();
-                if (sqDist > fogCullDistSq && horizonLineFromTop !== 0) {
+
+                // Terrain loads at one zoom level lower than the raster data,
+                // so the following checks whether the terrain sits above the horizon and ensures that
+                // when mountains stick out above the fog (due to horizon-blend),
+                // we haven’t accidentally culled some of the raster tiles we need to draw on them.
+                // If we don’t do this, the terrain is default black color and may flash in and out as we move toward it.
+
+                if (this.elevation && sqDist > fogCullDistSq && horizonLineFromTop !== 0) {
                     const projMatrix = this.calculateProjMatrix(entry.tileID.toUnwrapped());
 
                     let minmax;
@@ -858,13 +865,13 @@ class Transform {
                     // NDC to Screen
                     const screenCoordY = (1 - worldFar[1]) * this.height * 0.5;
 
-                    // Prevent cutting tiles crossing over the horizon lines to
+                    // Prevent cutting tiles crossing over the horizon line to
                     // prevent pop-in and out within the fog culling range
                     overHorizonLine = screenCoordY < horizonLineFromTop;
                 }
 
                 return sqDist < fogCullDistSq || overHorizonLine;
-            }));
+            });
         }
 
         const cover = result.sort((a, b) => a.distanceSq - b.distanceSq).map(a => a.tileID);
@@ -888,13 +895,15 @@ class Transform {
     zoomScale(zoom: number) { return Math.pow(2, zoom); }
     scaleZoom(scale: number) { return Math.log(scale) / Math.LN2; }
 
+    // LngLat to MercatorCoordinate
     project(lnglat: LngLat) {
         const lat = clamp(lnglat.lat, -this.maxValidLatitude, this.maxValidLatitude);
         return new Point(
                 mercatorXfromLng(lnglat.lng) * this.worldSize,
                 mercatorYfromLat(lat) * this.worldSize);
     }
 
+    // MercatorCoordinate to LngLat
     unproject(point: Point): LngLat {
         return new MercatorCoordinate(point.x / this.worldSize, point.y / this.worldSize).toLngLat();
     }
@@ -1068,11 +1077,10 @@ class Transform {
      * @param {Point} p top left origin screen point, in pixels.
      * @private
      */
-    pointCoordinate(p: Point): MercatorCoordinate {
+    pointCoordinate(p: Point, z?: number = this.centerAltitude): MercatorCoordinate {
         const horizonOffset = this.horizonLineFromTop(false);
         const clamped = new Point(p.x, Math.max(horizonOffset, p.y));
-
-        return this.rayIntersectionCoordinate(this.pointRayIntersection(clamped));
+        return this.rayIntersectionCoordinate(this.pointRayIntersection(clamped, z));
     }
 
     /**
@@ -1149,11 +1157,26 @@ class Transform {
      */
     getBounds(): LngLatBounds {
         if (this._terrainEnabled()) return this._getBounds3D();
+
+        let tl = this.pointCoordinate(new Point(this._edgeInsets.left, this._edgeInsets.top));
+        let tr = this.pointCoordinate(new Point(this.width - this._edgeInsets.right, this._edgeInsets.top));
+        const br = this.pointCoordinate(new Point(this.width - this._edgeInsets.right, this.height - this._edgeInsets.bottom));
+        const bl = this.pointCoordinate(new Point(this._edgeInsets.left, this.height - this._edgeInsets.bottom));
+
+        // Snap points if off the edge of the map.
+        const slope = (p1, p2) => (p2.y - p1.y) / (p2.x - p1.x);
+
+        if (tl.y > 1 && tr.y >= 0) tl = new MercatorCoordinate((1 - bl.y) / slope(bl, tl) + bl.x, 1);
+        else if (tl.y < 0 && tr.y <= 1) tl = new MercatorCoordinate(-bl.y / slope(bl, tl) + bl.x, 0);
+
+        if (tr.y > 1 && tl.y >= 0) tr = new MercatorCoordinate((1 - br.y) / slope(br, tr) + br.x, 1);
+        else if (tr.y < 0 && tl.y <= 1) tr = new MercatorCoordinate(-br.y / slope(br, tr) + br.x, 0);
+
         return new LngLatBounds()
-            .extend(this.pointLocation(new Point(this._edgeInsets.left, this._edgeInsets.top)))
-            .extend(this.pointLocation(new Point(this.width - this._edgeInsets.right, this._edgeInsets.top)))
-            .extend(this.pointLocation(new Point(this.width - this._edgeInsets.right, this.height - this._edgeInsets.bottom)))
-            .extend(this.pointLocation(new Point(this._edgeInsets.left, this.height - this._edgeInsets.bottom)));
+            .extend(this.coordinateLocation(tl))
+            .extend(this.coordinateLocation(tr))
+            .extend(this.coordinateLocation(bl))
+            .extend(this.coordinateLocation(br));
     }
 
     _getBounds3D(): LngLatBounds {
@@ -1169,29 +1192,46 @@ class Transform {
         }, {min: Number.MAX_VALUE, max: 0});
         minmax.min *= elevation.exaggeration();
         minmax.max *= elevation.exaggeration();
-        const top = this.horizonLineFromTop();
-        return [
-            new Point(0, top),
-            new Point(this.width, top),
-            new Point(this.width, this.height),
-            new Point(0, this.height)
-        ].reduce((acc, p) => {
-            return acc
-                .extend(this.coordinateLocation(this.rayIntersectionCoordinate(this.pointRayIntersection(p, minmax.min))))
-                .extend(this.coordinateLocation(this.rayIntersectionCoordinate(this.pointRayIntersection(p, minmax.max))));
-        }, new LngLatBounds());
+        // const top = this.horizonLineFromTop();
+
+        const topLeft = new Point(this._edgeInsets.left, this._edgeInsets.top);
+        const topRight = new Point(this.width - this._edgeInsets.right, this._edgeInsets.top);
+        const bottomRight = new Point(this.width - this._edgeInsets.right, this.height - this._edgeInsets.bottom);
+        const bottomLeft = new Point(this._edgeInsets.left, this.height - this._edgeInsets.bottom);
+
+        // Consider far points at the maximum possible elevation
+        // and near points at the minimum to ensure full coverage.
+        let tl = this.pointCoordinate(topLeft, minmax.min);
+        let tr = this.pointCoordinate(topRight, minmax.min);
+        const br = this.pointCoordinate(bottomRight, minmax.max);
+        const bl = this.pointCoordinate(bottomLeft, minmax.max);
+
+        // Snap points if off the edge of the map.
+        const slope = (p1, p2) => (p2.y - p1.y) / (p2.x - p1.x);
+
+        if (tl.y > 1 && tr.y >= 0) tl = new MercatorCoordinate((1 - bl.y) / slope(bl, tl) + bl.x, 1);
+        else if (tl.y < 0 && tr.y <= 1) tl = new MercatorCoordinate(-bl.y / slope(bl, tl) + bl.x, 0);
+
+        if (tr.y > 1 && tl.y >= 0) tr = new MercatorCoordinate((1 - br.y) / slope(br, tr) + br.x, 1);
+        else if (tr.y < 0 && tl.y <= 1) tr = new MercatorCoordinate(-br.y / slope(br, tr) + br.x, 0);
+
+        return new LngLatBounds()
+            .extend(this.coordinateLocation(tl))
+            .extend(this.coordinateLocation(tr))
+            .extend(this.coordinateLocation(bl))
+            .extend(this.coordinateLocation(br));
+
     }
 
     /**
-     * Returns position of horizon line from the top of the map in pixels. If horizon is not visible, returns 0.
+     * Returns position of horizon line from the top of the map in pixels.
+     * If horizon is not visible, returns 0 by default or a negative value if called with clampToTop = false.
      * @private
      */
     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);
+        const offset = this.height / 2 - h * (1 - this._horizonShift);
         return clampToTop ? Math.max(0, offset) : offset;
     }
 
@@ -1704,12 +1744,16 @@ class Transform {
         return !!this._elevation;
     }
 
-    isHorizonVisibleForPoints(p0: Point, p1: Point): boolean {
+    // Check if any of the four corners are off the edge of the rendered map
+    isCornerOffEdge(p0: Point, p1: Point): boolean {
         const minX = Math.min(p0.x, p1.x);
         const maxX = Math.max(p0.x, p1.x);
         const minY = Math.min(p0.y, p1.y);
         const maxY = Math.max(p0.y, p1.y);
 
+        const horizon = this.horizonLineFromTop(false);
+        if (minY < horizon) return true;
+
         const min = new Point(minX, minY);
         const max = new Point(maxX, maxY);
 
@@ -1726,9 +1770,11 @@ class Transform {
 
         for (const corner of corners) {
             const rayIntersection = this.pointRayIntersection(corner);
+            // Point is above the horizon
             if (rayIntersection.t < 0) {
                 return true;
             }
+            // Point is off the bondaries of the map
             const coordinate = this.rayIntersectionCoordinate(rayIntersection);
             if (coordinate.x < minWX || coordinate.y < minWY ||
                 coordinate.x > maxWX || coordinate.y > maxWY) {
@@ -1740,6 +1786,7 @@ class Transform {
     }
 
     // Checks the four corners of the frustum to see if they lie in the map's quad.
+    //
     isHorizonVisible(): boolean {
         // we consider the horizon as visible if the angle between
         // a the top plane of the frustum and the map plane is smaller than this threshold.
@@ -1748,7 +1795,7 @@ class Transform {
             return true;
         }
 
-        return this.isHorizonVisibleForPoints(new Point(0, 0), new Point(this.width, this.height));
+        return this.isCornerOffEdge(new Point(0, 0), new Point(this.width, this.height));
     }
 
     /**

src/ui/camera.js

@@ -848,7 +848,7 @@ class Camera extends Evented {
         const raycast = this._raycastElevationBox(point0, point1);
 
         if (!raycast) {
-            if (this.transform.isHorizonVisibleForPoints(point0, point1)) {
+            if (this.transform.isCornerOffEdge(point0, point1)) {
                 return this;
             }
 

test/unit/geo/transform.test.js

@@ -231,6 +231,66 @@ test('transform', (t) => {
         t.end();
     });
 
+    t.test('getBounds (#10261)', (t) => {
+        const transform = new Transform();
+        transform.resize(500, 500);
+        transform.zoom = 2;
+        transform.pitch = 80;
+
+        t.test('Looking at North Pole', (t) => {
+            transform.center = {lng: 0, lat: 90};
+            t.deepEqual(transform.center, {lng: 0, lat: 79.3677012485858});
+            const bounds = transform.getBounds();
+
+            // Bounds stops at the edge of the map
+            t.same(bounds.getNorth().toFixed(6), transform.maxValidLatitude);
+            // Top corners of bounds line up with side of view
+            t.same(transform.locationPoint(bounds.getNorthWest()).x.toFixed(10), 0);
+            t.same(transform.locationPoint(bounds.getNorthEast()).x.toFixed(10), transform.width);
+            // Bottom of bounds lines up with bottom of view
+            t.same(transform.locationPoint(bounds.getSouthEast()).y.toFixed(10), transform.height);
+            t.same(transform.locationPoint(bounds.getSouthWest()).y.toFixed(10), transform.height);
+
+            t.same(toFixed(bounds.toArray()), toFixed([[ -56.6312307639145, 62.350646608460806 ], [ 56.63123076391412, 85.0511287798 ]]));
+
+            t.end();
+        });
+        t.test('Looking at South Pole', (t) => {
+            transform.bearing = 180;
+            transform.center = {lng: 0, lat: -90};
+
+            t.deepEqual(transform.center, {lng: 0, lat: -79.3677012485858});
+            const bounds = transform.getBounds();
+
+            // Bounds stops at the edge of the map
+            t.same(bounds.getSouth().toFixed(6), -transform.maxValidLatitude);
+            // Top corners of bounds line up with side of view
+            t.same(transform.locationPoint(bounds.getSouthEast()).x.toFixed(10), 0);
+            t.same(transform.locationPoint(bounds.getSouthWest()).x.toFixed(10), transform.width);
+            // Bottom of bounds lines up with bottom of view
+            t.same(transform.locationPoint(bounds.getNorthEast()).y.toFixed(10), transform.height);
+            t.same(transform.locationPoint(bounds.getNorthWest()).y.toFixed(10), transform.height);
+
+            t.same(toFixed(bounds.toArray()), toFixed([[ -56.6312307639145, -85.0511287798], [ 56.63123076391412, -62.350646608460806]]));
+
+            t.end();
+        });
+        t.end();
+
+        function toFixed(bounds) {
+            const n = 10;
+            return [
+                [normalizeFixed(bounds[0][0], n), normalizeFixed(bounds[0][1], n)],
+                [normalizeFixed(bounds[1][0], n), normalizeFixed(bounds[1][1], n)]
+            ];
+        }
+
+        function normalizeFixed(num, n) {
+            // workaround for "-0.0000000000" ≠ "0.0000000000"
+            return parseFloat(num.toFixed(n)).toFixed(n);
+        }
+    });
+
     test('coveringTiles', (t) => {
         const options = {
             minzoom: 1,
@@ -940,7 +1000,7 @@ test('transform', (t) => {
 
     t.test('isHorizonVisible', (t) => {
 
-        t.test('isHorizonVisibleForPoints', (t) => {
+        t.test('isCornerOffEdge', (t) => {
             const transform = new Transform();
             transform.maxPitch = 85;
             transform.resize(800, 800);
@@ -952,21 +1012,21 @@ test('transform', (t) => {
             t.true(transform.isHorizonVisible());
 
             p0 = new Point(0, 0); p1 = new Point(10, 10);
-            t.true(transform.isHorizonVisibleForPoints(p0, p1));
+            t.true(transform.isCornerOffEdge(p0, p1));
 
             p0 = new Point(0, 250); p1 = new Point(10, 350);
-            t.true(transform.isHorizonVisibleForPoints(p0, p1));
+            t.true(transform.isCornerOffEdge(p0, p1));
 
             p0 = new Point(0, transform.horizonLineFromTop() - 10);
             p1 = new Point(10, transform.horizonLineFromTop() + 10);
-            t.true(transform.isHorizonVisibleForPoints(p0, p1));
+            t.true(transform.isCornerOffEdge(p0, p1));
 
             p0 = new Point(0, 700); p1 = new Point(10, 710);
-            t.false(transform.isHorizonVisibleForPoints(p0, p1));
+            t.false(transform.isCornerOffEdge(p0, p1));
 
             p0 = new Point(0, transform.horizonLineFromTop());
             p1 = new Point(10, transform.horizonLineFromTop() + 10);
-            t.false(transform.isHorizonVisibleForPoints(p0, p1));
+            t.false(transform.isCornerOffEdge(p0, p1));
 
             t.end();
         });