Fix potential infinite loop in zoomFromMercatorZAdjusted #12450 (#1…

…2488)

- Prevent NaN by gating invalid values from the callee
- Prevent infinite loop by using a binary search instead of the previous method more prone to floating point comparison issues
- Add regression tests and new test for this function

src/geo/transform.js

@@ -17,7 +17,13 @@ import assert from 'assert';
 import getProjectionAdjustments, {getProjectionAdjustmentInverted, getScaleAdjustment, getProjectionInterpolationT} from './projection/adjustments.js';
 import {getPixelsToTileUnitsMatrix} from '../source/pixels_to_tile_units.js';
 import {UnwrappedTileID, OverscaledTileID, CanonicalTileID} from '../source/tile_id.js';
-import {calculateGlobeMatrix, polesInViewport, GLOBE_ZOOM_THRESHOLD_MIN, GLOBE_SCALE_MATCH_LATITUDE} from '../geo/projection/globe_util.js';
+import {
+    calculateGlobeMatrix,
+    polesInViewport,
+    GLOBE_ZOOM_THRESHOLD_MIN,
+    GLOBE_ZOOM_THRESHOLD_MAX,
+    GLOBE_SCALE_MATCH_LATITUDE
+} from '../geo/projection/globe_util.js';
 import {projectClamped} from '../symbol/projection.js';
 
 import type Projection from '../geo/projection/projection.js';
@@ -2059,19 +2065,36 @@ class Transform {
     // latitude and the center's latitude as you zoom in, camera to center distance varies dynamically.
     // As the cameraToCenterDistance is a function of zoom, we need to approximate the true zoom
     // given a mercator meter value in order to eliminate the zoom/cameraToCenterDistance dependency.
-    zoomFromMercatorZAdjusted(z: number): number {
-        const getZoom = (zoom) => {
-            const d = this.getCameraToCenterDistance(this.projection, zoom);
-            return this.scaleZoom(d / (z * this.tileSize));
-        };
+    zoomFromMercatorZAdjusted(mercatorZ: number): number {
+        assert(this.projection.name === 'globe');
+        assert(mercatorZ !== 0);
+
+        let zoomLow = 0;
+        let zoomHigh = GLOBE_ZOOM_THRESHOLD_MAX;
+        let zoom = 0;
+        let minZoomDiff = Infinity;
+
+        const epsilon = 1e-6;
+
+        while (zoomHigh - zoomLow > epsilon && zoomHigh > zoomLow) {
+            const zoomMid = zoomLow + (zoomHigh - zoomLow) * 0.5;
+
+            const worldSize = this.tileSize * Math.pow(2, zoomMid);
+            const d = this.getCameraToCenterDistance(this.projection, zoomMid, worldSize);
+            const newZoom = this.scaleZoom(d / (mercatorZ * this.tileSize));
 
-        let zoom = getZoom(this.zoom);
-        let diff = Math.abs(zoom - getZoom(zoom));
-        let lastdiff;
-        while (lastdiff !== diff) {
-            zoom = getZoom(zoom);
-            lastdiff = diff;
-            diff = Math.abs(zoom - getZoom(zoom));
+            const diff = Math.abs(zoomMid - newZoom);
+
+            if (diff < minZoomDiff) {
+                minZoomDiff = diff;
+                zoom = zoomMid;
+            }
+
+            if (zoomMid < newZoom) {
+                zoomLow = zoomMid;
+            } else {
+                zoomHigh = zoomMid;
+            }
         }
 
         return zoom;
@@ -2183,9 +2206,9 @@ class Transform {
         }
     }
 
-    getCameraToCenterDistance(projection: Projection, zoom: number = this.zoom): number {
+    getCameraToCenterDistance(projection: Projection, zoom: number = this.zoom, worldSize: number = this.worldSize): number {
         const t = getProjectionInterpolationT(projection, zoom, this.width, this.height, 1024);
-        const projectionScaler = projection.pixelSpaceConversion(this.center.lat, this.worldSize, t);
+        const projectionScaler = projection.pixelSpaceConversion(this.center.lat, worldSize, t);
         return 0.5 / Math.tan(this._fov * 0.5) * this.height * projectionScaler;
     }
 

src/ui/camera.js

@@ -729,7 +729,7 @@ class Camera extends Evented {
         const meterPerECEF = earthRadius / GLOBE_RADIUS;
         const altitudeECEF = vec3.length(cameraPosition);
         const altitudeMeter = altitudeECEF * meterPerECEF - earthRadius;
-        const mercatorZ = mercatorZfromAltitude(altitudeMeter, 0);
+        const mercatorZ = mercatorZfromAltitude(Math.max(altitudeMeter, Number.EPSILON), 0);
 
         const zoom = Math.min(tr.zoomFromMercatorZAdjusted(mercatorZ), eOptions.maxZoom);
 

test/unit/geo/transform.test.js

@@ -1726,6 +1726,25 @@ test('transform', (t) => {
         t.end();
     });
 
+    t.test("zoomFromMercatorZAdjusted", (t) => {
+        const transform = new Transform();
+        transform.resize(500, 500);
+        t.ok(transform.setProjection({name: 'globe'}));
+
+        for (let zoom = 0; zoom < 6; ++zoom) {
+            transform.zoom = zoom;
+
+            t.equal(fixedNum(transform.zoomFromMercatorZAdjusted(mercatorZfromAltitude(1e3, 0)), 3), 6);
+            t.equal(fixedNum(transform.zoomFromMercatorZAdjusted(mercatorZfromAltitude(1e6, 0)), 3), 5.874);
+            t.equal(fixedNum(transform.zoomFromMercatorZAdjusted(mercatorZfromAltitude(4e6, 0)), 3), 3.87);
+            t.equal(fixedNum(transform.zoomFromMercatorZAdjusted(mercatorZfromAltitude(1e7, 0)), 3), 2.054);
+            t.equal(fixedNum(transform.zoomFromMercatorZAdjusted(mercatorZfromAltitude(2e7, 0)), 3), 1.052);
+            t.equal(fixedNum(transform.zoomFromMercatorZAdjusted(mercatorZfromAltitude(5e7, 0)), 3), 0);
+        }
+
+        t.end();
+    });
+
     t.test("ZoomDeltaToMovement", (t) => {
         const transform = new Transform();
         transform.resize(100, 100);

test/unit/ui/camera.test.js

@@ -8,6 +8,7 @@ import {fixedLngLat, fixedNum, fixedVec3} from '../../util/fixed.js';
 import {equalWithPrecision} from '../../util/index.js';
 import MercatorCoordinate from '../../../src/geo/mercator_coordinate.js';
 import LngLat from '../../../src/geo/lng_lat.js';
+import LngLatBounds from '../../../src/geo/lng_lat_bounds.js';
 import {vec3, quat} from 'gl-matrix';
 
 test('camera', (t) => {
@@ -2305,6 +2306,21 @@ test('camera', (t) => {
             t.end();
         });
 
+        t.test('#12450', (t) => {
+            const camera = createCamera();
+
+            camera.setCenter([-115.6288447, 35.1509267]);
+            camera.setZoom(5);
+
+            const bounds = new LngLatBounds();
+            bounds.extend([-115.6288447, 35.1509267]);
+            camera.fitBounds(bounds, {padding: 75, duration: 0});
+
+            t.deepEqual(fixedLngLat(camera.getCenter(), 4), {lng: -115.6288, lat: 35.1509});
+            t.equal(camera.getZoom(), 20);
+            t.end();
+        });
+
         t.end();
     });