From 1836d6e7cad4345bb6f209ef941b3ea869e8b2e0 Mon Sep 17 00:00:00 2001 From: Aaron Caldwell Date: Thu, 24 Oct 2024 12:38:31 -0600 Subject: [PATCH 1/8] Clear errored tile on initial data load event from source --- src/source/source_cache.ts | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/source/source_cache.ts b/src/source/source_cache.ts index 646361999a..5e58b9dcbc 100644 --- a/src/source/source_cache.ts +++ b/src/source/source_cache.ts @@ -85,8 +85,12 @@ export class SourceCache extends Evented { this.on('data', (e: MapSourceDataEvent) => this._dataHandler(e)); - this.on('dataloading', () => { + this.on('dataloading', (e) => { this._sourceErrored = false; + // Clear errored tiles on init data load event, not on subsequent tile updates + if (!e.tile) { + this.clearErroredTiles(); + } }); this.on('error', () => { @@ -583,6 +587,15 @@ export class SourceCache extends Evented { } } + // Remove previously errored tiles from the retain list + clearErroredTiles() { + Object.values(this._tiles).forEach((tile: Tile) => { + if (tile.state === 'errored') { + this._removeTile(tile.tileID.key); + } + }); + } + /** * Removes tiles that are outside the viewport and adds new tiles that * are inside the viewport. From baf958da52875a80ddb9ce57c85cf876666c0b36 Mon Sep 17 00:00:00 2001 From: Aaron Caldwell Date: Thu, 24 Oct 2024 12:39:06 -0600 Subject: [PATCH 2/8] Add supporting unit test --- src/source/source_cache.test.ts | 35 +++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/source/source_cache.test.ts b/src/source/source_cache.test.ts index 1cfdf61e19..e5b19a9484 100644 --- a/src/source/source_cache.test.ts +++ b/src/source/source_cache.test.ts @@ -532,6 +532,41 @@ describe('SourceCache / Source lifecycle', () => { }); + test('clears errored tiles on initial data load', () => { + const preserveTiles = [ + new OverscaledTileID(2, 0, 2, 2, 2), + new OverscaledTileID(2, 0, 2, 1, 2), + ]; + const errorTiles = [ + new OverscaledTileID(2, 0, 2, 2, 1), + new OverscaledTileID(2, 0, 2, 1, 1) + ]; + + const sourceCache = createSourceCache(); + + sourceCache._source.loadTile = async (tile) => { + tile.state = 'loaded'; + }; + preserveTiles.forEach((id) => sourceCache._addTile(id)); + expect(Object.keys(sourceCache._tiles)).toHaveLength(preserveTiles.length); + + // Reset load tile to apply error state + sourceCache._source.loadTile = async (tile) => { + tile.state = 'errored'; + }; + errorTiles.forEach((id) => sourceCache._addTile(id)); + expect(Object.keys(sourceCache._tiles)).toHaveLength(preserveTiles.length + errorTiles.length); + + // Expect no change on a tile dataloading event + sourceCache.getSource().fire(new Event('dataloading', {tile: {}})); + expect(Object.keys(sourceCache._tiles)).toHaveLength(preserveTiles.length + errorTiles.length); + + // Expect errored tiles to be removed on a generic dataloading event + sourceCache.getSource().fire(new Event('dataloading')); + expect(Object.keys(sourceCache._tiles)).toHaveLength(preserveTiles.length); + + }); + }); describe('SourceCache#update', () => { From 3fe30eb53da6ac66068fc5a3732b978dcf4c2795 Mon Sep 17 00:00:00 2001 From: Aaron Caldwell Date: Thu, 24 Oct 2024 16:16:48 -0600 Subject: [PATCH 3/8] Revise approach to attach to original event broadcast and catch at reload call --- src/source/raster_tile_source.ts | 8 +++--- src/source/source_cache.test.ts | 46 +++++++++++++------------------- src/source/source_cache.ts | 22 +++++---------- src/ui/events.ts | 1 + 4 files changed, 31 insertions(+), 46 deletions(-) diff --git a/src/source/raster_tile_source.ts b/src/source/raster_tile_source.ts index 6300e5b183..b98acc3a45 100644 --- a/src/source/raster_tile_source.ts +++ b/src/source/raster_tile_source.ts @@ -85,7 +85,7 @@ export class RasterTileSource extends Evented implements Source { extend(this, pick(options, ['url', 'scheme', 'tileSize'])); } - async load() { + async load(sourceDataChanged: boolean = false) { this._loaded = false; this.fire(new Event('dataloading', {dataType: 'source'})); this._tileJSONRequest = new AbortController(); @@ -101,7 +101,7 @@ export class RasterTileSource extends Evented implements Source { // before the TileJSON arrives. this makes sure the tiles needed are loaded once TileJSON arrives // ref: https://github.com/mapbox/mapbox-gl-js/pull/4347#discussion_r104418088 this.fire(new Event('data', {dataType: 'source', sourceDataType: 'metadata'})); - this.fire(new Event('data', {dataType: 'source', sourceDataType: 'content'})); + this.fire(new Event('data', {dataType: 'source', sourceDataType: 'content', sourceDataChanged})); } } catch (err) { this._tileJSONRequest = null; @@ -133,7 +133,9 @@ export class RasterTileSource extends Evented implements Source { callback(); - this.load(); + const sourceDataChanged = true; + + this.load(sourceDataChanged); } /** diff --git a/src/source/source_cache.test.ts b/src/source/source_cache.test.ts index e5b19a9484..adab167382 100644 --- a/src/source/source_cache.test.ts +++ b/src/source/source_cache.test.ts @@ -532,38 +532,30 @@ describe('SourceCache / Source lifecycle', () => { }); - test('clears errored tiles on initial data load', () => { - const preserveTiles = [ - new OverscaledTileID(2, 0, 2, 2, 2), - new OverscaledTileID(2, 0, 2, 1, 2), - ]; - const errorTiles = [ - new OverscaledTileID(2, 0, 2, 2, 1), - new OverscaledTileID(2, 0, 2, 1, 1) - ]; + test('does reload errored tiles, if event is source data change', () => { + const transform = new Transform(); + transform.resize(511, 511); + transform.zoom = 1; const sourceCache = createSourceCache(); - sourceCache._source.loadTile = async (tile) => { - tile.state = 'loaded'; - }; - preserveTiles.forEach((id) => sourceCache._addTile(id)); - expect(Object.keys(sourceCache._tiles)).toHaveLength(preserveTiles.length); - - // Reset load tile to apply error state - sourceCache._source.loadTile = async (tile) => { - tile.state = 'errored'; + // this transform will try to load the four tiles at z1 and a single z0 tile + // we only expect _reloadTile to be called with the 'loaded' z0 tile + tile.state = tile.tileID.canonical.z === 1 ? 'errored' : 'loaded'; }; - errorTiles.forEach((id) => sourceCache._addTile(id)); - expect(Object.keys(sourceCache._tiles)).toHaveLength(preserveTiles.length + errorTiles.length); - - // Expect no change on a tile dataloading event - sourceCache.getSource().fire(new Event('dataloading', {tile: {}})); - expect(Object.keys(sourceCache._tiles)).toHaveLength(preserveTiles.length + errorTiles.length); - // Expect errored tiles to be removed on a generic dataloading event - sourceCache.getSource().fire(new Event('dataloading')); - expect(Object.keys(sourceCache._tiles)).toHaveLength(preserveTiles.length); + const reloadTileSpy = jest.spyOn(sourceCache, '_reloadTile'); + sourceCache.on('data', (e) => { + if (e.dataType === 'source' && e.sourceDataType === 'metadata') { + sourceCache.update(transform); + sourceCache.getSource().fire(new Event('data', {dataType: 'source', sourceDataType: 'content', sourceDataChanged: true})); + } + }); + sourceCache.onAdd(undefined); + // We expect the source cache to have five tiles, and for all of them + // to be reloaded + expect(Object.keys(sourceCache._tiles)).toHaveLength(5); + expect(reloadTileSpy).toHaveBeenCalledTimes(5); }); diff --git a/src/source/source_cache.ts b/src/source/source_cache.ts index 5e58b9dcbc..c46947fe8f 100644 --- a/src/source/source_cache.ts +++ b/src/source/source_cache.ts @@ -87,10 +87,6 @@ export class SourceCache extends Evented { this.on('dataloading', (e) => { this._sourceErrored = false; - // Clear errored tiles on init data load event, not on subsequent tile updates - if (!e.tile) { - this.clearErroredTiles(); - } }); this.on('error', () => { @@ -248,7 +244,7 @@ export class SourceCache extends Evented { !this._coveredTiles[id] && (symbolLayer || !this._tiles[id].holdingForFade()); } - reload() { + reload(sourceDataChanged?: boolean) { if (this._paused) { this._shouldReloadOnResume = true; return; @@ -257,7 +253,9 @@ export class SourceCache extends Evented { this._cache.reset(); for (const i in this._tiles) { - if (this._tiles[i].state !== 'errored') this._reloadTile(i, 'reloading'); + if (sourceDataChanged || this._tiles[i].state !== 'errored') { + this._reloadTile(i, 'reloading'); + } } } @@ -587,15 +585,6 @@ export class SourceCache extends Evented { } } - // Remove previously errored tiles from the retain list - clearErroredTiles() { - Object.values(this._tiles).forEach((tile: Tile) => { - if (tile.state === 'errored') { - this._removeTile(tile.tileID.key); - } - }); - } - /** * Removes tiles that are outside the viewport and adds new tiles that * are inside the viewport. @@ -932,7 +921,8 @@ export class SourceCache extends Evented { // for sources with mutable data, this event fires when the underlying data // to a source is changed. (i.e. GeoJSONSource#setData and ImageSource#serCoordinates) if (this._sourceLoaded && !this._paused && e.dataType === 'source' && eventSourceDataType === 'content') { - this.reload(); + const sourceDataChanged = Boolean(e.sourceDataChanged); + this.reload(sourceDataChanged); if (this.transform) { this.update(this.transform, this.terrain); } diff --git a/src/ui/events.ts b/src/ui/events.ts index 3d4d97edb6..e34fbc6c0c 100644 --- a/src/ui/events.ts +++ b/src/ui/events.ts @@ -450,6 +450,7 @@ export type MapSourceDataEvent = MapLibreEvent & { source: SourceSpecification; sourceId: string; sourceDataType: MapSourceDataType; + sourceDataChanged?: boolean; /** * The tile being loaded or changed, if the event has a `dataType` of `source` and * the event is related to loading of a tile. From f85b3931df1de70098b8f6679c7071642d775633 Mon Sep 17 00:00:00 2001 From: Aaron Caldwell Date: Thu, 24 Oct 2024 16:45:05 -0600 Subject: [PATCH 4/8] Remove unused 'e' --- src/source/source_cache.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/source/source_cache.ts b/src/source/source_cache.ts index c46947fe8f..014331e79c 100644 --- a/src/source/source_cache.ts +++ b/src/source/source_cache.ts @@ -85,7 +85,7 @@ export class SourceCache extends Evented { this.on('data', (e: MapSourceDataEvent) => this._dataHandler(e)); - this.on('dataloading', (e) => { + this.on('dataloading', () => { this._sourceErrored = false; }); From afe8eafa49c10c481d3addf9b6bf1ac3791e69f7 Mon Sep 17 00:00:00 2001 From: Aaron Caldwell Date: Fri, 25 Oct 2024 13:53:59 -0600 Subject: [PATCH 5/8] Update test to reflect updated main branch --- src/source/source_cache.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/source/source_cache.test.ts b/src/source/source_cache.test.ts index 06487e1bee..2810efbd69 100644 --- a/src/source/source_cache.test.ts +++ b/src/source/source_cache.test.ts @@ -528,9 +528,9 @@ describe('SourceCache / Source lifecycle', () => { }); test('does reload errored tiles, if event is source data change', () => { - const transform = new Transform(); + const transform = new MercatorTransform(); transform.resize(511, 511); - transform.zoom = 1; + transform.setZoom(1); const sourceCache = createSourceCache(); sourceCache._source.loadTile = async (tile) => { From ee2be1a3475665a8a850d228f16115d9741a1bf7 Mon Sep 17 00:00:00 2001 From: Aaron Caldwell Date: Fri, 25 Oct 2024 13:54:47 -0600 Subject: [PATCH 6/8] Feedback updates. Remove unneeded boolean var. Remove unused Boolean cast --- src/source/raster_tile_source.ts | 4 +--- src/source/source_cache.ts | 3 +-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/source/raster_tile_source.ts b/src/source/raster_tile_source.ts index 768b87e4ac..7e4193e4d5 100644 --- a/src/source/raster_tile_source.ts +++ b/src/source/raster_tile_source.ts @@ -133,9 +133,7 @@ export class RasterTileSource extends Evented implements Source { callback(); - const sourceDataChanged = true; - - this.load(sourceDataChanged); + this.load(true); } /** diff --git a/src/source/source_cache.ts b/src/source/source_cache.ts index b30744ac15..edeb458021 100644 --- a/src/source/source_cache.ts +++ b/src/source/source_cache.ts @@ -930,8 +930,7 @@ export class SourceCache extends Evented { // for sources with mutable data, this event fires when the underlying data // to a source is changed. (i.e. GeoJSONSource#setData and ImageSource#serCoordinates) if (this._sourceLoaded && !this._paused && e.dataType === 'source' && eventSourceDataType === 'content') { - const sourceDataChanged = Boolean(e.sourceDataChanged); - this.reload(sourceDataChanged); + this.reload(e.sourceDataChanged); if (this.transform) { this.update(this.transform, this.terrain); } From 2caa799d48c3244ce14360c44d53321a5b0d6724 Mon Sep 17 00:00:00 2001 From: Aaron Caldwell Date: Sat, 26 Oct 2024 14:54:06 -0600 Subject: [PATCH 7/8] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4ca8d322e..ba86ff9386 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ ### 🐞 Bug fixes - ⚠️ Fix order of normalizeSpriteURL and transformRequest in loadSprite ([#3897](https://github.com/maplibre/maplibre-gl-js/issues/3897)) +- Fix issue where raster tile source won't fetch updates following request error ([#4890](https://github.com/maplibre/maplibre-gl-js/pull/4890)) - _...Add new stuff here..._ ## v5.0.0-pre.3 From b5550482ffb19b4dc6a11f925c6698b6b01113c0 Mon Sep 17 00:00:00 2001 From: Aaron Caldwell Date: Sat, 26 Oct 2024 14:55:43 -0600 Subject: [PATCH 8/8] Spacing --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba86ff9386..b6a0002cbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ ### 🐞 Bug fixes - ⚠️ Fix order of normalizeSpriteURL and transformRequest in loadSprite ([#3897](https://github.com/maplibre/maplibre-gl-js/issues/3897)) -- Fix issue where raster tile source won't fetch updates following request error ([#4890](https://github.com/maplibre/maplibre-gl-js/pull/4890)) +- Fix issue where raster tile source won't fetch updates following request error ([#4890](https://github.com/maplibre/maplibre-gl-js/pull/4890)) - _...Add new stuff here..._ ## v5.0.0-pre.3