Skip to content

Commit

Permalink
Remove finalize elevation when updating elevation on the fly (#3944)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
olsen232 and HarelM committed May 16, 2024
1 parent 962b385 commit 3000ba1
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 21 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
51 changes: 33 additions & 18 deletions src/ui/camera.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand All @@ -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 => {
Expand Down
4 changes: 2 additions & 2 deletions src/ui/camera.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down

0 comments on commit 3000ba1

Please sign in to comment.