HARP-15773 Fixes VisibleTileSet::markTilesDirty bug (#2217)

* HARP-15773 Fixes VisibleTileSet::markTilesDirty bug Basically, if we called this while the tile was still loading, then the execution of the tile.load method would use the existing donePromise in the BaseTileLoader We fix this by cancelling the TileLoader before requesting a new re-load Signed-off-by: Jonathan Stichbury <[email protected]>
heremaps · Jun 9, 2021 · 21f5d10 · 21f5d10
1 parent 8e867cb
commit 21f5d10
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 2 deletions.
diff --git a/@here/harp-mapview/lib/VisibleTileSet.ts b/@here/harp-mapview/lib/VisibleTileSet.ts
@@ -1318,6 +1318,11 @@ export class VisibleTileSet {
             const tileKey = DataSourceCache.getKeyForTile(tile);
             if (!retainedTiles.has(tileKey)) {
                 retainedTiles.add(tileKey);
+                // We need to cancel the loader first because if we don't then the call to
+                // tileLoader.loadAndDecode() inside Tile::load will return the existing promise (if
+                // the tile is still loading) and not re-request the tile data from the provider as
+                // required.
+                tile.tileLoader?.cancel();
                 this.addToTaskQueue(tile);
             }
         };

diff --git a/@here/harp-mapview/test/FakeOmvDataSource.ts b/@here/harp-mapview/test/FakeOmvDataSource.ts
@@ -26,18 +26,29 @@ export class FakeTileLoader implements ITileLoader {
         geometries: []
     };
 
+    reject?: (reason?: any) => void;
+
     isFinished: boolean = false;
 
     loadAndDecode(): Promise<TileLoaderState> {
-        return Promise.resolve(TileLoaderState.Ready);
+        this.state = TileLoaderState.Loading;
+        return new Promise((resolve, reject) => {
+            this.reject = reject;
+            // We use setTimeout to delay the resolve
+            setTimeout(() => {
+                this.state = TileLoaderState.Loaded;
+                resolve(TileLoaderState.Ready);
+            });
+        });
     }
 
     waitSettled(): Promise<TileLoaderState> {
         return Promise.resolve(TileLoaderState.Ready);
     }
 
     cancel(): void {
-        // Not covered with tests yet
+        this.state = TileLoaderState.Canceled;
+        this.reject!(this.state);
     }
 }
 export class FakeOmvDataSource extends DataSource {

diff --git a/@here/harp-mapview/test/VisibleTileSetTest.ts b/@here/harp-mapview/test/VisibleTileSetTest.ts
@@ -25,6 +25,7 @@ import { DataSource, DataSourceOptions } from "../lib/DataSource";
 import { FrustumIntersection, TileKeyEntry } from "../lib/FrustumIntersection";
 import { TileGeometryCreator } from "../lib/geometry/TileGeometryCreator";
 import { TileGeometryManager } from "../lib/geometry/TileGeometryManager";
+import { TileLoaderState } from "../lib/ITileLoader";
 import { MapView, TileTaskGroups } from "../lib/MapView";
 import { Tile } from "../lib/Tile";
 import { TileOffsetUtils } from "../lib/Utils";
@@ -430,6 +431,36 @@ describe("VisibleTileSet", function () {
         assert.equal(dataSourceTileList[0]?.visibleTiles[0].tileKey.mortonCode(), 1);
     });
 
+    it("#markTilesDirty cancels tileloader", async function () {
+        setupBerlinCenterCameraFromSamples();
+        const zoomLevel = 15;
+        const storageLevel = 14;
+        let dataSourceTileList = updateRenderList(zoomLevel, storageLevel).tileList;
+        let visibleTiles = dataSourceTileList[0].visibleTiles;
+
+        fixture.mapView.taskQueue.processNext(TileTaskGroups.FETCH_AND_DECODE, undefined, 2);
+        assert(visibleTiles.every(tile => tile.tileLoader?.state === TileLoaderState.Loading));
+
+        // See FakeTileLoader, this will take ~ 50ms.
+        await Promise.all(visibleTiles.map(tile => tile.tileLoader?.loadAndDecode()));
+
+        assert(visibleTiles.every(tile => tile.tileLoader?.state === TileLoaderState.Loaded));
+
+        // We need to reset the state.
+        fixture.vts.clearTileCache();
+        // Force the tiles to re-load.
+        dataSourceTileList = updateRenderList(zoomLevel, storageLevel).tileList;
+
+        visibleTiles = dataSourceTileList[0].visibleTiles;
+        assert(visibleTiles.every(tile => tile.tileLoader?.state === TileLoaderState.Loading));
+
+        // This is the crux of this test, we want to check that the loader state is cancelled.
+        fixture.vts.markTilesDirty();
+        assert(visibleTiles.every(tile => tile.tileLoader?.state === TileLoaderState.Canceled));
+        await Promise.all(visibleTiles.map(tile => tile.tileLoader?.loadAndDecode()));
+        assert(visibleTiles.every(tile => tile.tileLoader?.state === TileLoaderState.Loaded));
+    });
+
     it("#markTilesDirty properly handles cached & visible tiles", async function () {
         setupBerlinCenterCameraFromSamples();
         const zoomLevel = 15;