From 3000ba109488c5503ce7719066ae0bd8a8100960 Mon Sep 17 00:00:00 2001
From: Andrew Olsen <andrew.olsen@koordinates.com>
Date: Thu, 16 May 2024 18:22:43 +1000
Subject: [PATCH] Remove finalize elevation when updating elevation on the fly
 (#3944)

* Fix for #3878

Don't _finalizeElevation if freezeElevation has not been set.

This means that elevation logic has two distinct operating modes.

Mode one - freezeElevation is false - default:
- updateElevation is called continuously during the movement
- finalizeElevation is *not* called at the end.

Mode two - freezeElevation is true:
- updateElevation is *not* called continuoulsy during the movement
- finalizeElevation is called at the end.

Previously, both methods were called in mode one, which lead to
issue #3878 if freezeElevation was not set.

* Fix for #3878 - address review comments (done -> async)

* Update src/ui/camera.test.ts

* Fix for #3878 - address review comments 2

---------

Co-authored-by: Harel M <harel.mazor@gmail.com>
---
 CHANGELOG.md          |  2 +-
 src/ui/camera.test.ts | 51 ++++++++++++++++++++++++++++---------------
 src/ui/camera.ts      |  4 ++--
 3 files changed, 36 insertions(+), 21 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b74869bc5d..4ab0567270 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,7 +5,7 @@
 
 ### 🐞 Bug fixes
 
-- _...Add new stuff here..._
+- Fix drift in zoom that may happen during flyTo and easeTo due to freezeElevation logic. ([#3878](https://github.com/maplibre/maplibre-gl-js/issues/3878))
 
 ## 4.3.0
 
diff --git a/src/ui/camera.test.ts b/src/ui/camera.test.ts
index 4d86e61dc2..3114b48f4f 100644
--- a/src/ui/camera.test.ts
+++ b/src/ui/camera.test.ts
@@ -1728,7 +1728,7 @@ describe('#flyTo', () => {
         camera.flyTo({center: [100, 0], bearing: 90, animate: true});
     });
 
-    test('check freezeElevation events', done => {
+    test('check elevation events freezeElevation=false', async () => {
         const camera = createCamera();
         const stub = jest.spyOn(browser, 'now');
 
@@ -1737,28 +1737,43 @@ describe('#flyTo', () => {
         camera._prepareElevation = () => { terrainCallbacks.prepare++; };
         camera._updateElevation = () => { terrainCallbacks.update++; };
         camera._finalizeElevation = () => { terrainCallbacks.finalize++; };
-
         camera.setCenter([-10, 0]);
-
-        camera.on('moveend', () => {
-            expect(terrainCallbacks.prepare).toBe(1);
-            expect(terrainCallbacks.update).toBe(0);
-            expect(terrainCallbacks.finalize).toBe(1);
-            done();
-        });
+        const moveEnded = camera.once('moveend');
 
         stub.mockImplementation(() => 0);
-        camera.flyTo({center: [10, 0], duration: 20, freezeElevation: true});
+        camera.flyTo({center: [10, 0], duration: 20, freezeElevation: false});
+        stub.mockImplementation(() => 1);
+        camera.simulateFrame();
+        stub.mockImplementation(() => 20);
+        camera.simulateFrame();
+        await moveEnded;
+        expect(terrainCallbacks.prepare).toBe(1);
+        expect(terrainCallbacks.update).toBe(2);
+        expect(terrainCallbacks.finalize).toBe(0);
+    });
 
-        setTimeout(() => {
-            stub.mockImplementation(() => 1);
-            camera.simulateFrame();
+    test('check elevation events freezeElevation=true', async() => {
+        const camera = createCamera();
+        const stub = jest.spyOn(browser, 'now');
 
-            setTimeout(() => {
-                stub.mockImplementation(() => 20);
-                camera.simulateFrame();
-            }, 0);
-        }, 0);
+        const terrainCallbacks = {prepare: 0, update: 0, finalize: 0} as any;
+        camera.terrain = {} as Terrain;
+        camera._prepareElevation = () => { terrainCallbacks.prepare++; };
+        camera._updateElevation = () => { terrainCallbacks.update++; };
+        camera._finalizeElevation = () => { terrainCallbacks.finalize++; };
+        camera.setCenter([-10, 0]);
+        const moveEnded = camera.once('moveend');
+
+        stub.mockImplementation(() => 0);
+        camera.flyTo({center: [10, 0], duration: 20, freezeElevation: true});
+        stub.mockImplementation(() => 1);
+        camera.simulateFrame();
+        stub.mockImplementation(() => 20);
+        camera.simulateFrame();
+        await moveEnded;
+        expect(terrainCallbacks.prepare).toBe(1);
+        expect(terrainCallbacks.update).toBe(0);
+        expect(terrainCallbacks.finalize).toBe(1);
     });
 
     test('check elevation callbacks', done => {
diff --git a/src/ui/camera.ts b/src/ui/camera.ts
index cd9859682d..8640950b36 100644
--- a/src/ui/camera.ts
+++ b/src/ui/camera.ts
@@ -1038,7 +1038,7 @@ export abstract class Camera extends Evented {
             this._fireMoveEvents(eventData);
 
         }, (interruptingEaseId?: string) => {
-            if (this.terrain) this._finalizeElevation();
+            if (this.terrain && options.freezeElevation) this._finalizeElevation();
             this._afterEase(eventData, interruptingEaseId);
         }, options as any);
 
@@ -1366,7 +1366,7 @@ export abstract class Camera extends Evented {
             this._fireMoveEvents(eventData);
 
         }, () => {
-            if (this.terrain) this._finalizeElevation();
+            if (this.terrain && options.freezeElevation) this._finalizeElevation();
             this._afterEase(eventData);
         }, options);