Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow raster source tile updates to re-trigger tile request that failed previously #4890

Merged
8 changes: 5 additions & 3 deletions src/source/raster_tile_source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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;
Expand Down Expand Up @@ -133,7 +133,9 @@ export class RasterTileSource extends Evented implements Source {

callback();

this.load();
const sourceDataChanged = true;
wagewarbler marked this conversation as resolved.
Show resolved Hide resolved

this.load(sourceDataChanged);
}

/**
Expand Down
27 changes: 27 additions & 0 deletions src/source/source_cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -528,10 +528,37 @@
sourceCache.onAdd(undefined);
// we expect the source cache to have five tiles, but only to have reloaded one
expect(Object.keys(sourceCache._tiles)).toHaveLength(5);
expect(reloadTileSpy).toHaveBeenCalledTimes(1);

Check failure on line 531 in src/source/source_cache.test.ts

View workflow job for this annotation

GitHub Actions / Code Hygiene

Cannot find name 'Transform'. Did you mean 'transform'?

Check failure on line 531 in src/source/source_cache.test.ts

View workflow job for this annotation

GitHub Actions / Unit tests and Coverage

SourceCache / Source lifecycle › does reload errored tiles

ReferenceError: Transform is not defined at Object.<anonymous> (src/source/source_cache.test.ts:531:27)

});

test('does reload errored tiles, if event is source data change', () => {
const transform = new Transform();
wagewarbler marked this conversation as resolved.
Show resolved Hide resolved
transform.resize(511, 511);
transform.zoom = 1;

const sourceCache = createSourceCache();
sourceCache._source.loadTile = async (tile) => {
// 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';
};

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);

});

});

describe('SourceCache#update', () => {
Expand Down
11 changes: 7 additions & 4 deletions src/source/source_cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@

this.on('data', (e: MapSourceDataEvent) => this._dataHandler(e));

this.on('dataloading', () => {
this.on('dataloading', (e) => {
this._sourceErrored = false;
});

Expand All @@ -94,7 +94,7 @@
this._sourceErrored = this._source.loaded();
});

this._source = createSource(id, options, dispatcher, this);

Check warning on line 97 in src/source/source_cache.ts

View workflow job for this annotation

GitHub Actions / Code Hygiene

'e' is defined but never used. Allowed unused args must match /^_/u

this._tiles = {};
this._cache = new TileCache(0, (tile) => this._unloadTile(tile));
Expand Down Expand Up @@ -244,7 +244,7 @@
!this._coveredTiles[id] && (symbolLayer || !this._tiles[id].holdingForFade());
}

reload() {
reload(sourceDataChanged?: boolean) {
if (this._paused) {
this._shouldReloadOnResume = true;
return;
Expand All @@ -253,7 +253,9 @@
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') {
HarelM marked this conversation as resolved.
Show resolved Hide resolved
this._reloadTile(i, 'reloading');
}
}
}

Expand Down Expand Up @@ -919,7 +921,8 @@
// 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);
wagewarbler marked this conversation as resolved.
Show resolved Hide resolved
this.reload(sourceDataChanged);
if (this.transform) {
this.update(this.transform, this.terrain);
}
Expand Down
1 change: 1 addition & 0 deletions src/ui/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading