Globe white flash fix (#4845)

* Fix white frame when transitioning to globe projection * Refactor globeness animation * Update changelog * Add unit test that checks that globe transform and globe projection have their globe state synchronized * Shorter sleeps in globe transform tests
maplibre · Oct 17, 2024 · 9c62b8b · 9c62b8b
1 parent 76edc77
commit 9c62b8b
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 36 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 
 ### 🐞 Bug fixes
 - Fix text not being hidden behind the globe when overlap mode was set to `always` ([#4802](https://github.com/maplibre/maplibre-gl-js/issues/4802))
+- Fix a single white frame being displayed when the map internally transitions from mercator to globe projection ([#4816](https://github.com/maplibre/maplibre-gl-js/issues/4816))
 - _...Add new stuff here..._
 
 ## 5.0.0-pre.2

diff --git a/src/geo/projection/globe.ts b/src/geo/projection/globe.ts
@@ -17,7 +17,6 @@ export const globeConstants = {
      * Used for globe rendering.
      */
     globeTransitionTimeSeconds: 0.5,
-    zoomTransitionTimeSeconds: 0.5,
     maxGlobeZoom: 12.0,
     errorTransitionTimeSeconds: 0.5
 };

diff --git a/src/geo/projection/globe_transform.test.ts b/src/geo/projection/globe_transform.test.ts
@@ -1,4 +1,4 @@
-import {GlobeProjection} from './globe';
+import {globeConstants, GlobeProjection} from './globe';
 import {EXTENT} from '../../data/extent';
 import Point from '@mapbox/point-geometry';
 import {LngLat} from '../lng_lat';
@@ -34,6 +34,9 @@ function createGlobeTransform(globeProjection: GlobeProjection) {
 
 describe('GlobeTransform', () => {
     const globeProjectionMock = getGlobeProjectionMock();
+    // Force faster animations so we can use shorter sleeps when testing them
+    globeConstants.globeTransitionTimeSeconds = 0.1;
+    globeConstants.errorTransitionTimeSeconds = 0.1;
 
     describe('getProjectionData', () => {
         const globeTransform = createGlobeTransform(globeProjectionMock);
@@ -447,12 +450,12 @@ describe('GlobeTransform', () => {
             globeTransform.newFrameUpdate();
             globeTransform.setGlobeViewAllowed(false);
 
-            await sleep(20);
+            await sleep(10);
             globeTransform.newFrameUpdate();
             expect(globeTransform.getGlobeViewAllowed()).toBe(false);
             expect(globeTransform.isGlobeRendering).toBe(true);
 
-            await sleep(1000);
+            await sleep(150);
             globeTransform.newFrameUpdate();
             expect(globeTransform.getGlobeViewAllowed()).toBe(false);
             expect(globeTransform.isGlobeRendering).toBe(false);
@@ -463,7 +466,7 @@ describe('GlobeTransform', () => {
             globeTransform.newFrameUpdate();
             globeTransform.setGlobeViewAllowed(false, false);
 
-            await sleep(20);
+            await sleep(10);
             globeTransform.newFrameUpdate();
             expect(globeTransform.getGlobeViewAllowed()).toBe(false);
             expect(globeTransform.isGlobeRendering).toBe(false);
@@ -731,4 +734,26 @@ describe('GlobeTransform', () => {
             ]);
         });
     });
+
+    test('transform and projection instance are synchronized properly', async () => {
+        const projectionMock = getGlobeProjectionMock();
+        const globeTransform = createGlobeTransform(projectionMock);
+        // projectionMock.useGlobeRendering and globeTransform.isGlobeRendering must have the same value
+        expect(projectionMock.useGlobeRendering).toBe(true);
+        expect(globeTransform.isGlobeRendering).toBe(projectionMock.useGlobeRendering);
+        globeTransform.setGlobeViewAllowed(false);
+        globeTransform.newFrameUpdate();
+        expect(projectionMock.useGlobeRendering).toBe(false);
+        expect(globeTransform.isGlobeRendering).toBe(projectionMock.useGlobeRendering);
+
+        await sleep(150);
+        globeTransform.setGlobeViewAllowed(true);
+        globeTransform.newFrameUpdate();
+        expect(projectionMock.useGlobeRendering).toBe(false);
+        expect(globeTransform.isGlobeRendering).toBe(projectionMock.useGlobeRendering);
+        await sleep(10);
+        globeTransform.newFrameUpdate();
+        expect(projectionMock.useGlobeRendering).toBe(true);
+        expect(globeTransform.isGlobeRendering).toBe(projectionMock.useGlobeRendering);
+    });
 });
diff --git a/src/geo/projection/globe_transform.ts b/src/geo/projection/globe_transform.ts
@@ -213,20 +213,17 @@ export class GlobeTransform implements ITransform {
     // Transition handling
     private _lastGlobeStateEnabled: boolean = true;
 
-    private _lastLargeZoomStateChange: number = -1000.0;
-    private _lastLargeZoomState: boolean = false;
-
     /**
      * Stores when {@link newFrameUpdate} was last called.
      * Serves as a unified clock for globe (instead of each function using a slightly different value from `browser.now()`).
      */
-    private _lastUpdateTime = browser.now();
+    private _lastUpdateTimeSeconds = browser.now() / 1000.0;
     /**
      * Stores when switch from globe to mercator or back last occurred, for animation purposes.
      * This switch can be caused either by the map passing the threshold zoom level,
      * or by {@link setGlobeViewAllowed} being called.
      */
-    private _lastGlobeChangeTime: number = browser.now() - 10_000; // Ten seconds before transform creation
+    private _lastGlobeChangeTimeSeconds: number = browser.now() / 1000 - 10; // Ten seconds before transform creation
 
     private _skipNextAnimation: boolean = true;
 
@@ -351,19 +348,18 @@ export class GlobeTransform implements ITransform {
             this._skipNextAnimation = true;
         }
         this._globeProjectionAllowed = allow;
-        this._lastGlobeChangeTime = this._lastUpdateTime;
+        this._lastGlobeChangeTimeSeconds = this._lastUpdateTimeSeconds;
     }
 
     /**
      * Should be called at the beginning of every frame to synchronize the transform with the underlying projection.
      */
     newFrameUpdate(): TransformUpdateResult {
-        this._updateErrorCorrectionValue();
-
-        this._lastUpdateTime = browser.now();
+        this._lastUpdateTimeSeconds = browser.now() / 1000.0;
         const oldGlobeRendering = this.isGlobeRendering;
         this._globeness = this._computeGlobenessAnimation();
-
+        // Everything below this comment must happen AFTER globeness update
+        this._updateErrorCorrectionValue();
         this._calcMatrices();
 
         if (oldGlobeRendering === this.isGlobeRendering) {
@@ -400,34 +396,25 @@ export class GlobeTransform implements ITransform {
      */
     private _computeGlobenessAnimation(): number {
         // Update globe transition animation
-        const globeState = this._globeProjectionAllowed;
-        const currentTime = this._lastUpdateTime;
+        const globeState = this._globeProjectionAllowed && this.zoom < globeConstants.maxGlobeZoom;
+        const currentTimeSeconds = this._lastUpdateTimeSeconds;
         if (globeState !== this._lastGlobeStateEnabled) {
-            this._lastGlobeChangeTime = currentTime;
+            this._lastGlobeChangeTimeSeconds = currentTimeSeconds;
             this._lastGlobeStateEnabled = globeState;
         }
 
         const oldGlobeness = this._globeness;
 
         // Transition parameter, where 0 is the start and 1 is end.
-        const globeTransition = Math.min(Math.max((currentTime - this._lastGlobeChangeTime) / 1000.0 / globeConstants.globeTransitionTimeSeconds, 0.0), 1.0);
+        const globeTransition = Math.min(Math.max((currentTimeSeconds - this._lastGlobeChangeTimeSeconds) / globeConstants.globeTransitionTimeSeconds, 0.0), 1.0);
         let newGlobeness = globeState ? globeTransition : (1.0 - globeTransition);
 
         if (this._skipNextAnimation) {
             newGlobeness = globeState ? 1.0 : 0.0;
-            this._lastGlobeChangeTime = currentTime - globeConstants.globeTransitionTimeSeconds * 1000.0 * 2.0;
+            this._lastGlobeChangeTimeSeconds = currentTimeSeconds - globeConstants.globeTransitionTimeSeconds * 2.0;
             this._skipNextAnimation = false;
         }
 
-        // Update globe zoom transition
-        const currentZoomState = this.zoom >= globeConstants.maxGlobeZoom;
-        if (currentZoomState !== this._lastLargeZoomState) {
-            this._lastLargeZoomState = currentZoomState;
-            this._lastLargeZoomStateChange = currentTime;
-        }
-        const zoomTransition = Math.min(Math.max((currentTime - this._lastLargeZoomStateChange) / 1000.0 / globeConstants.zoomTransitionTimeSeconds, 0.0), 1.0);
-        const zoomGlobenessBound = currentZoomState ? (1.0 - zoomTransition) : zoomTransition;
-        newGlobeness = Math.min(newGlobeness, zoomGlobenessBound);
         newGlobeness = easeCubicInOut(newGlobeness); // Smooth animation
 
         if (oldGlobeness !== newGlobeness) {
@@ -443,7 +430,7 @@ export class GlobeTransform implements ITransform {
 
     isRenderingDirty(): boolean {
         // Globe transition
-        return (this._lastUpdateTime - this._lastGlobeChangeTime) / 1000.0 < (Math.max(globeConstants.globeTransitionTimeSeconds, globeConstants.zoomTransitionTimeSeconds));
+        return (this._lastUpdateTimeSeconds - this._lastGlobeChangeTimeSeconds) < globeConstants.globeTransitionTimeSeconds;
     }
 
     getProjectionData(overscaledTileID: OverscaledTileID, aligned?: boolean, ignoreTerrainMatrix?: boolean): ProjectionData {

diff --git a/src/util/test/util.ts b/src/util/test/util.ts
@@ -230,12 +230,7 @@ export function getGlobeProjectionMock(): GlobeProjection {
         get useGlobeControls(): boolean {
             return true;
         },
-        get useGlobeRendering(): boolean {
-            return true;
-        },
-        set useGlobeRendering(_value: boolean) {
-            // do not set
-        },
+        useGlobeRendering: true,
         latitudeErrorCorrectionRadians: 0,
         errorQueryLatitudeDegrees: 0,
     } as GlobeProjection;