Skip to content

Commit d42e0da

Browse files
wagewarblerAaron Caldwell
and
Aaron Caldwell
authored
Allow raster source tile updates to re-trigger tile request that failed previously (#4890)
* Clear errored tile on initial data load event from source * Add supporting unit test * Revise approach to attach to original event broadcast and catch at reload call * Remove unused 'e' * Update test to reflect updated main branch * Feedback updates. Remove unneeded boolean var. Remove unused Boolean cast * Update changelog * Spacing --------- Co-authored-by: Aaron Caldwell <[email protected]>
1 parent 95cfbdb commit d42e0da

File tree

5 files changed

+37
-6
lines changed

5 files changed

+37
-6
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
### 🐞 Bug fixes
88
- ⚠️ Fix order of normalizeSpriteURL and transformRequest in loadSprite ([#3897](https://github.com/maplibre/maplibre-gl-js/issues/3897))
99
- ⚠️ Remove unminified prod build ([#4906](https://github.com/maplibre/maplibre-gl-js/pull/4906))
10+
- Fix issue where raster tile source won't fetch updates following request error ([#4890](https://github.com/maplibre/maplibre-gl-js/pull/4890))
1011
- _...Add new stuff here..._
1112

1213
## v5.0.0-pre.3

src/source/raster_tile_source.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ export class RasterTileSource extends Evented implements Source {
8585
extend(this, pick(options, ['url', 'scheme', 'tileSize']));
8686
}
8787

88-
async load() {
88+
async load(sourceDataChanged: boolean = false) {
8989
this._loaded = false;
9090
this.fire(new Event('dataloading', {dataType: 'source'}));
9191
this._tileJSONRequest = new AbortController();
@@ -101,7 +101,7 @@ export class RasterTileSource extends Evented implements Source {
101101
// before the TileJSON arrives. this makes sure the tiles needed are loaded once TileJSON arrives
102102
// ref: https://github.com/mapbox/mapbox-gl-js/pull/4347#discussion_r104418088
103103
this.fire(new Event('data', {dataType: 'source', sourceDataType: 'metadata'}));
104-
this.fire(new Event('data', {dataType: 'source', sourceDataType: 'content'}));
104+
this.fire(new Event('data', {dataType: 'source', sourceDataType: 'content', sourceDataChanged}));
105105
}
106106
} catch (err) {
107107
this._tileJSONRequest = null;
@@ -133,7 +133,7 @@ export class RasterTileSource extends Evented implements Source {
133133

134134
callback();
135135

136-
this.load();
136+
this.load(true);
137137
}
138138

139139
/**

src/source/source_cache.test.ts

+27
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,33 @@ describe('SourceCache / Source lifecycle', () => {
527527

528528
});
529529

530+
test('does reload errored tiles, if event is source data change', () => {
531+
const transform = new MercatorTransform();
532+
transform.resize(511, 511);
533+
transform.setZoom(1);
534+
535+
const sourceCache = createSourceCache();
536+
sourceCache._source.loadTile = async (tile) => {
537+
// this transform will try to load the four tiles at z1 and a single z0 tile
538+
// we only expect _reloadTile to be called with the 'loaded' z0 tile
539+
tile.state = tile.tileID.canonical.z === 1 ? 'errored' : 'loaded';
540+
};
541+
542+
const reloadTileSpy = jest.spyOn(sourceCache, '_reloadTile');
543+
sourceCache.on('data', (e) => {
544+
if (e.dataType === 'source' && e.sourceDataType === 'metadata') {
545+
sourceCache.update(transform);
546+
sourceCache.getSource().fire(new Event('data', {dataType: 'source', sourceDataType: 'content', sourceDataChanged: true}));
547+
}
548+
});
549+
sourceCache.onAdd(undefined);
550+
// We expect the source cache to have five tiles, and for all of them
551+
// to be reloaded
552+
expect(Object.keys(sourceCache._tiles)).toHaveLength(5);
553+
expect(reloadTileSpy).toHaveBeenCalledTimes(5);
554+
555+
});
556+
530557
});
531558

532559
describe('SourceCache#update', () => {

src/source/source_cache.ts

+5-3
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ export class SourceCache extends Evented {
253253
!this._coveredTiles[id] && (symbolLayer || !this._tiles[id].holdingForFade());
254254
}
255255

256-
reload() {
256+
reload(sourceDataChanged?: boolean) {
257257
if (this._paused) {
258258
this._shouldReloadOnResume = true;
259259
return;
@@ -262,7 +262,9 @@ export class SourceCache extends Evented {
262262
this._cache.reset();
263263

264264
for (const i in this._tiles) {
265-
if (this._tiles[i].state !== 'errored') this._reloadTile(i, 'reloading');
265+
if (sourceDataChanged || this._tiles[i].state !== 'errored') {
266+
this._reloadTile(i, 'reloading');
267+
}
266268
}
267269
}
268270

@@ -928,7 +930,7 @@ export class SourceCache extends Evented {
928930
// for sources with mutable data, this event fires when the underlying data
929931
// to a source is changed. (i.e. GeoJSONSource#setData and ImageSource#serCoordinates)
930932
if (this._sourceLoaded && !this._paused && e.dataType === 'source' && eventSourceDataType === 'content') {
931-
this.reload();
933+
this.reload(e.sourceDataChanged);
932934
if (this.transform) {
933935
this.update(this.transform, this.terrain);
934936
}

src/ui/events.ts

+1
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,7 @@ export type MapSourceDataEvent = MapLibreEvent & {
461461
source: SourceSpecification;
462462
sourceId: string;
463463
sourceDataType: MapSourceDataType;
464+
sourceDataChanged?: boolean;
464465
/**
465466
* The tile being loaded or changed, if the event has a `dataType` of `source` and
466467
* the event is related to loading of a tile.

0 commit comments

Comments
 (0)