Fix for #3872 - unwanted pan-jumps at end of pan (#3873)

* Fix for #3872 - unwanted pan-jumps at end of pan

Always recompute coords framebuffer if matrix has changed -
any time that it is not mat4.exactEquals compared to how it was previously.
Don't wait for it to be not mat4.equals compared to how it was previously.

* New fix for #3872 - unwanted pan-jumps at end of pan

Recompute coords frameBuffer at the end of a pan, before
using it in the transform.recalculateZoom method.
Relatively inexpensive: only does one extra re-render
per pan operation.

* New-new fix for #3872 - unwanted pan-jumps at end of pan

Recompute coords frameBuffer immediately before using it in
the function terrain.pointCoordinate.

* Fix for #3872: address review comments

* Fix for #3872: address review comments 2

* Fix for #3872: fix failing tests

* Fix for #3872: address review comments 3

* Fix for #3872: Update tests

* Fix for #3872: update CHANGELOG

CHANGELOG.md

@@ -5,7 +5,9 @@
 - _...Add new stuff here..._
 
 ### 🐞 Bug fixes
-- fix type defnition on `localIdeographFontFamily` ([#3896](https://github.com/maplibre/maplibre-gl-js/pull/3896))
+- Fix type definition on `localIdeographFontFamily` ([#3896](https://github.com/maplibre/maplibre-gl-js/pull/3896))
+- Fix unwanted panning changes at the end of a panning motion ([#3872](https://github.com/maplibre/maplibre-gl-js/issues/3872))
+- _...Add new stuff here..._
 
 ## 4.1.1
 

src/render/painter.ts

@@ -116,7 +116,7 @@ export class Painter {
         this.context = new Context(gl);
         this.transform = transform;
         this._tileTextures = {};
-        this.terrainFacilitator = {dirty: true, matrix: mat4.create(), renderTime: 0};
+        this.terrainFacilitator = {dirty: true, matrix: mat4.identity(new Float64Array(16) as any), renderTime: 0};
 
         this.setup();
 
@@ -376,20 +376,12 @@ export class Painter {
             }
         }
 
+        this.maybeDrawDepthAndCoords(false);
+
         if (this.renderToTexture) {
             this.renderToTexture.prepareForRender(this.style, this.transform.zoom);
             // this is disabled, because render-to-texture is rendering all layers from bottom to top.
             this.opaquePassCutoff = 0;
-
-            // update coords/depth-framebuffer on camera movement, or tile reloading
-            const newTiles = this.style.map.terrain.sourceCache.tilesAfterTime(this.terrainFacilitator.renderTime);
-            if (this.terrainFacilitator.dirty || !mat4.equals(this.terrainFacilitator.matrix, this.transform.projMatrix) || newTiles.length) {
-                mat4.copy(this.terrainFacilitator.matrix, this.transform.projMatrix);
-                this.terrainFacilitator.renderTime = Date.now();
-                this.terrainFacilitator.dirty = false;
-                drawDepth(this, this.style.map.terrain);
-                drawCoords(this, this.style.map.terrain);
-            }
         }
 
         // Offscreen pass ===============================================
@@ -468,6 +460,34 @@ export class Painter {
         this.context.setDefault();
     }
 
+    /**
+     * Update the depth and coords framebuffers, if the contents of those frame buffers is out of date.
+     * If requireExact is false, then the contents of those frame buffers is not updated if it is close
+     * to accurate (that is, the camera has not moved much since it was updated last).
+     */
+    maybeDrawDepthAndCoords(requireExact: boolean) {
+        if (!this.style || !this.style.map || !this.style.map.terrain) {
+            return;
+        }
+        const prevMatrix = this.terrainFacilitator.matrix;
+        const currMatrix = this.transform.projMatrix;
+
+        // Update coords/depth-framebuffer on camera movement, or tile reloading
+        let doUpdate = this.terrainFacilitator.dirty;
+        doUpdate ||= requireExact ? !mat4.exactEquals(prevMatrix, currMatrix) : !mat4.equals(prevMatrix, currMatrix);
+        doUpdate ||= this.style.map.terrain.sourceCache.tilesAfterTime(this.terrainFacilitator.renderTime).length > 0;
+
+        if (!doUpdate) {
+            return;
+        }
+
+        mat4.copy(prevMatrix, currMatrix);
+        this.terrainFacilitator.renderTime = Date.now();
+        this.terrainFacilitator.dirty = false;
+        drawDepth(this, this.style.map.terrain);
+        drawCoords(this, this.style.map.terrain);
+    }
+
     renderLayer(painter: Painter, sourceCache: SourceCache, layer: StyleLayer, coords: Array<OverscaledTileID>) {
         if (layer.isHidden(this.transform.zoom)) return;
         if (layer.type !== 'background' && layer.type !== 'custom' && !(coords || []).length) return;

src/render/terrain.test.ts

@@ -30,12 +30,13 @@ describe('Terrain', () => {
     });
 
     test('pointCoordinate should not return null', () => {
-        expect.assertions(1);
+        expect.assertions(2);
         const painter = {
             context: new Context(gl),
             width: 1,
             height: 1,
-            transform: {center: {lng: 0}}
+            transform: {center: {lng: 0}},
+            maybeDrawDepthAndCoords: jest.fn(),
         } as any as Painter;
         const sourceCache = {} as SourceCache;
         const getTileByID = (tileID) : Tile => {
@@ -59,6 +60,8 @@ describe('Terrain', () => {
         const coordinate = terrain.pointCoordinate(new Point(0, 0));
 
         expect(coordinate).not.toBeNull();
+        expect(painter.maybeDrawDepthAndCoords).toHaveBeenCalled();
+
     });
 
     const setupMercatorOverflow = () => {
@@ -67,6 +70,7 @@ describe('Terrain', () => {
             context: new Context(gl),
             width: WORLD_WIDTH,
             height: 1,
+            maybeDrawDepthAndCoords: jest.fn(),
         } as any as Painter;
         const sourceCache = {} as SourceCache;
         const terrain = new Terrain(painter, sourceCache, {} as any as TerrainSpecification);
@@ -94,24 +98,26 @@ describe('Terrain', () => {
         `pointCoordinate should return negative mercator x
         if the point is on the LEFT outside the central globe`,
         () => {
-            expect.assertions(1);
+            expect.assertions(2);
             const pointX = 0;
             const terrain = setupMercatorOverflow();
             const coordinate = terrain.pointCoordinate(new Point(pointX, 0));
 
             expect(coordinate.x).toBe(-1);
+            expect(terrain.painter.maybeDrawDepthAndCoords).toHaveBeenCalled();
         });
 
     test(
         `pointCoordinate should return mercator x greater than 1
         if the point is on the RIGHT outside the central globe`,
         () => {
-            expect.assertions(1);
+            expect.assertions(2);
             const pointX = 3;
             const terrain = setupMercatorOverflow();
             const coordinate = terrain.pointCoordinate(new Point(pointX, 0));
 
             expect(coordinate.x).toBe(2);
+            expect(terrain.painter.maybeDrawDepthAndCoords).toHaveBeenCalled();
         });
 
     test('Calculate tile minimum and maximum elevation', () => {

src/render/terrain.ts

@@ -327,6 +327,9 @@ export class Terrain {
      * @returns mercator coordinate for a screen pixel
      */
     pointCoordinate(p: Point): MercatorCoordinate {
+        // First, ensure the coords framebuffer is up to date.
+        this.painter.maybeDrawDepthAndCoords(true);
+
         const rgba = new Uint8Array(4);
         const context = this.painter.context, gl = context.gl;
         // grab coordinate pixel from coordinates framebuffer