Fix for #2094 - unwanted zoom changes (#3836)

* Fix for #2094 - unwanted zoom changes

As described in #2094
panning the map can result in a sudden jump in zoom-level as the
drag is released. This jump tends to be worse when more zoomed out,
and repeated short pans cause the zoom jumps to happen repeatedly
in the same direction, which will grow exponentially if each
one is zooming out - each zoom out causes the bug to get worse.

The problem lies in recalculateZoom, which tries to calculate a new
zoom level such that the previous camera altitude + terrain elevation
is the same as the current camera altitude + terrain elevation.
Camera altitude here refers specifically to the camera's height
above the terrain elevation, and terrain elevation is the terrain
height at the centre of the screen. (This doesn't really leave any
good nouns for describing the sum of these two variables, but
this sum should remain constant and is the camera's total height
above sea level).

A specific zoom maps - more-or-less - to a specific camera altitude.
Since the terrain elevation can be different (eg 100m higher) at the
end of a terrain drag, we need the camera altitude to be different
too (eg 100m lower to compensate). So, we need to recalculate the
zoom to one that maps to the required camera altitude. And so,
the function recalculateZoom exists to fulfil this purpose.

Unfortunately, the function miscalculates the required zoom, as
can be verified by comparing camera altitude + terrain elevation
before recalculateZoom is called vs after - very slight changes
in terrain elevation (10s of metres) lead to large and unrelated
changes in camera altitude (1000s of metres) when the camera is
very zoomed out.

Currently, the logic in recalculateZoom uses different math to
the math used to calculate the camera's altitude from the
current zoom. This different math may be close to correct, but,
it's impossible for me to debug. In this commit I've simply
replaced it with the inverse of the math used to calcualte the
camera's altitude from its zoom - ie:
if a zoom Z gives a camera altitude f(Z)

Then, to get the camera to required altitude A,
we need to set the zoom to Z = inverse-of-f(A)

* Tests for fix for #2094 (unwanted zoom changes)

Added asserts that camera altitude remains invariant throughout the
recalculateZoom test. These new asserts fail before the fix for 2094.

Moved the asserts that the zoom is a particular value to after the
altitude asserts - a particular zoom is correct if and only if it
results in the correct altitude, so whether the altitude assert fails
is more important information to have in a failing test.

Updated CHANGELOG.md

---------

Co-authored-by: Harel M <[email protected]>

CHANGELOG.md

@@ -5,6 +5,7 @@
 - _...Add new stuff here..._
 
 ### 🐞 Bug fixes
+- Fix unwanted zoom changes at the end of a panning motion ([#2094](https://github.com/maplibre/maplibre-gl-js/issues/2094))
 - _...Add new stuff here..._
 
 ## 4.1.0

src/geo/transform.test.ts

@@ -402,30 +402,39 @@ describe('transform', () => {
         transform.elevation = 200;
         transform.center = new LngLat(10.0, 50.0);
         transform.zoom = 14;
+        transform.pitch = 45;
         transform.resize(512, 512);
 
+        // This should be an invariant throughout - the zoom is greater when the camera is
+        // closer to the terrain (and therefore also when the terrain is closer to the camera),
+        // but that shouldn't change the camera's position in world space if that wasn't requested.
+        const expectedAltitude = 1865.7579397718;
+        expect(transform.getCameraPosition().altitude).toBeCloseTo(expectedAltitude, 10);
+
         // expect same values because of no elevation change
         const terrain = {
             getElevationForLngLatZoom: () => 200,
             pointCoordinate: () => null
         };
         transform.recalculateZoom(terrain as any);
+        expect(transform.getCameraPosition().altitude).toBeCloseTo(expectedAltitude, 10);
         expect(transform.zoom).toBe(14);
 
         // expect new zoom because of elevation change
         terrain.getElevationForLngLatZoom = () => 400;
         transform.recalculateZoom(terrain as any);
-        expect(transform.zoom).toBeCloseTo(14.127997275621933, 10);
         expect(transform.elevation).toBe(400);
-
         expect(transform._center.lng).toBeCloseTo(10, 10);
         expect(transform._center.lat).toBeCloseTo(50, 10);
+        expect(transform.getCameraPosition().altitude).toBeCloseTo(expectedAltitude, 10);
+        expect(transform.zoom).toBeCloseTo(14.1845318986, 10);
 
         // expect new zoom because of elevation change to point below sea level
         terrain.getElevationForLngLatZoom = () => -200;
         transform.recalculateZoom(terrain as any);
-        expect(transform.zoom).toBeCloseTo(13.773740316343467, 10);
         expect(transform.elevation).toBe(-200);
+        expect(transform.getCameraPosition().altitude).toBeCloseTo(expectedAltitude, 10);
+        expect(transform.zoom).toBeCloseTo(13.6895075574, 10);
     });
 
     test('pointCoordinate with terrain when returning null should fall back to 2D', () => {

src/geo/transform.ts

@@ -504,21 +504,25 @@ export class Transform {
      * @param terrain - the terrain
      */
     recalculateZoom(terrain: Terrain) {
+        const origElevation = this.elevation;
+        const origAltitude = Math.cos(this._pitch) * this.cameraToCenterDistance / this._pixelPerMeter;
+
         // find position the camera is looking on
         const center = this.pointLocation(this.centerPoint, terrain);
         const elevation = terrain.getElevationForLngLatZoom(center, this.tileZoom);
         const deltaElevation = this.elevation - elevation;
         if (!deltaElevation) return;
 
-        // calculate mercator distance between camera & target
-        const cameraPosition = this.getCameraPosition();
-        const camera = MercatorCoordinate.fromLngLat(cameraPosition.lngLat, cameraPosition.altitude);
-        const target = MercatorCoordinate.fromLngLat(center, elevation);
-        const dx = camera.x - target.x, dy = camera.y - target.y, dz = camera.z - target.z;
-        const distance = Math.sqrt(dx * dx + dy * dy + dz * dz);
-
-        // from this distance we calculate the new zoomlevel
-        const zoom = this.scaleZoom(this.cameraToCenterDistance / distance / this.tileSize);
+        // The camera's altitude off the ground + the ground's elevation = a constant:
+        // this means the camera stays at the same total height.
+        const requiredAltitude = origAltitude + origElevation - elevation;
+        // Since altitude = Math.cos(this._pitch) * this.cameraToCenterDistance / pixelPerMeter:
+        const requiredPixelPerMeter = Math.cos(this._pitch) * this.cameraToCenterDistance / requiredAltitude;
+        // Since pixelPerMeter = mercatorZfromAltitude(1, center.lat) * worldSize:
+        const requiredWorldSize = requiredPixelPerMeter / mercatorZfromAltitude(1, center.lat);
+        // Since worldSize = this.tileSize * scale:
+        const requiredScale = requiredWorldSize / this.tileSize;
+        const zoom = this.scaleZoom(requiredScale);
 
         // update matrices
         this._elevation = elevation;