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

Async actor callback and promise #3374

Merged
merged 12 commits into from
Nov 22, 2023
42 changes: 24 additions & 18 deletions src/render/image_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,31 +18,37 @@ type Pattern = {
position: ImagePosition;
};

// When copied into the atlas texture, image data is padded by one pixel on each side. Icon
// images are padded with fully transparent pixels, while pattern images are padded with a
// copy of the image data wrapped from the opposite side. In both cases, this ensures the
// correct behavior of GL_LINEAR texture sampling mode.
/**
* When copied into the atlas texture, image data is padded by one pixel on each side. Icon
* images are padded with fully transparent pixels, while pattern images are padded with a
* copy of the image data wrapped from the opposite side. In both cases, this ensures the
* correct behavior of GL_LINEAR texture sampling mode.
*/
const padding = 1;

/*
ImageManager does three things:

1. Tracks requests for icon images from tile workers and sends responses when the requests are fulfilled.
2. Builds a texture atlas for pattern images.
3. Rerenders renderable images once per frame

These are disparate responsibilities and should eventually be handled by different classes. When we implement
data-driven support for `*-pattern`, we'll likely use per-bucket pattern atlases, and that would be a good time
to refactor this.
/**
* ImageManager does three things:
*
* 1. Tracks requests for icon images from tile workers and sends responses when the requests are fulfilled.
* 2. Builds a texture atlas for pattern images.
* 3. Rerenders renderable images once per frame
*
* These are disparate responsibilities and should eventually be handled by different classes. When we implement
* data-driven support for `*-pattern`, we'll likely use per-bucket pattern atlases, and that would be a good time
* to refactor this.
*/
export class ImageManager extends Evented {
images: {[_: string]: StyleImage};
updatedImages: {[_: string]: boolean};
callbackDispatchedThisFrame: {[_: string]: boolean};
loaded: boolean;
/**
* This is used to track requests for images that are not yet available. When the image is loaded,
* the requestors will be notified.
*/
requestors: Array<{
ids: Array<string>;
callback: (value: GetImagesResponse) => void;
promiseResolve: (value: GetImagesResponse) => void;
}>;

patterns: {[_: string]: Pattern};
Expand Down Expand Up @@ -75,8 +81,8 @@ export class ImageManager extends Evented {
this.loaded = loaded;

if (loaded) {
for (const {ids, callback} of this.requestors) {
callback(this._getImagesForIds(ids));
for (const {ids, promiseResolve} of this.requestors) {
promiseResolve(this._getImagesForIds(ids));
}
this.requestors = [];
}
Expand Down Expand Up @@ -193,7 +199,7 @@ export class ImageManager extends Evented {
if (this.isLoaded() || hasAllDependencies) {
resolve(this._getImagesForIds(ids));
} else {
this.requestors.push({ids, callback: resolve});
this.requestors.push({ids, promiseResolve: resolve});
}
});
}
Expand Down
12 changes: 6 additions & 6 deletions src/source/canvas_source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export class CanvasSource extends ImageSource {
this.animate = options.animate !== undefined ? options.animate : true;
}

load = () => {
async load() {
HarelM marked this conversation as resolved.
Show resolved Hide resolved
this._loaded = true;
if (!this.canvas) {
this.canvas = (this.options.canvas instanceof HTMLCanvasElement) ?
Expand Down Expand Up @@ -137,7 +137,7 @@ export class CanvasSource extends ImageSource {
};

this._finishLoading();
};
}

/**
* Returns the HTML `canvas` element.
Expand All @@ -160,7 +160,7 @@ export class CanvasSource extends ImageSource {
this.pause();
}

prepare = () => {
prepare() {
let resize = false;
if (this.canvas.width !== this.width) {
this.width = this.canvas.width;
Expand Down Expand Up @@ -205,14 +205,14 @@ export class CanvasSource extends ImageSource {
if (newTilesLoaded) {
this.fire(new Event('data', {dataType: 'source', sourceDataType: 'idle', sourceId: this.id}));
}
};
}

serialize = (): CanvasSourceSpecification => {
serialize(): CanvasSourceSpecification {
return {
type: 'canvas',
coordinates: this.coordinates
};
};
}

hasTransition() {
return this._playing;
Expand Down
2 changes: 1 addition & 1 deletion src/source/geojson_source.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ describe('GeoJSONSource#update', () => {
source.on('data', (e) => {
if (e.sourceDataType === 'metadata') {
source.setData({} as GeoJSON.GeoJSON);
source.loadTile(new Tile(new OverscaledTileID(0, 0, 0, 0, 0), 512), () => {});
source.loadTile(new Tile(new OverscaledTileID(0, 0, 0, 0, 0), 512));
}
});

Expand Down
31 changes: 13 additions & 18 deletions src/source/geojson_source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,9 @@ export class GeoJSONSource extends Evented implements Source {
}
}

load = () => {
load() {
this._updateWorkerData();
};
}

onAdd(map: Map) {
this.map = map;
Expand Down Expand Up @@ -372,7 +372,7 @@ export class GeoJSONSource extends Evented implements Source {
return this._pendingLoads === 0;
}

loadTile(tile: Tile, callback: Callback<void>) {
async loadTile(tile: Tile): Promise<void> {
const message = !tile.actor ? 'loadTile' : 'reloadTile';
tile.actor = this.actor;
const params = {
Expand All @@ -389,44 +389,39 @@ export class GeoJSONSource extends Evented implements Source {
};

tile.abortController = new AbortController();
this.actor.sendAsync({type: message, data: params}, tile.abortController).then((data) => {
delete tile.abortController;
tile.unloadVectorData();

if (tile.aborted) {
return callback(null);
}
const data = await this.actor.sendAsync({type: message, data: params}, tile.abortController);
delete tile.abortController;
tile.unloadVectorData();

if (!tile.aborted) {
tile.loadVectorData(data, this.map.painter, message === 'reloadTile');

return callback(null);
}).catch((err) => callback(err));
}
}

abortTile(tile: Tile) {
async abortTile(tile: Tile) {
if (tile.abortController) {
tile.abortController.abort();
delete tile.abortController;
}
tile.aborted = true;
}

unloadTile(tile: Tile) {
async unloadTile(tile: Tile) {
tile.unloadVectorData();
this.actor.sendAsync({type: 'removeTile', data: {uid: tile.uid, type: this.type, source: this.id}});
await this.actor.sendAsync({type: 'removeTile', data: {uid: tile.uid, type: this.type, source: this.id}});
}

onRemove() {
this._removed = true;
this.actor.sendAsync({type: 'removeSource', data: {type: this.type, source: this.id}});
}

serialize = (): GeoJSONSourceSpecification => {
serialize(): GeoJSONSourceSpecification {
return extend({}, this._options, {
type: this.type,
data: this._data
});
};
}

hasTransition() {
return false;
Expand Down
4 changes: 2 additions & 2 deletions src/source/geojson_worker_source.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ describe('loadData', () => {
return new GeoJSONWorkerSource(actor, layerIndex, []);
}

test('abandons previous callbacks', async () => {
test('abandons previous requests', async () => {
const worker = createWorker();

server.respondWith(request => {
Expand All @@ -228,7 +228,7 @@ describe('loadData', () => {
expect(result && result.abandoned).toBeFalsy();
});

test('removeSource aborts callbacks', async () => {
test('removeSource aborts requests', async () => {
const worker = createWorker();

server.respondWith(request => {
Expand Down
7 changes: 2 additions & 5 deletions src/source/geojson_worker_source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,12 @@ export class GeoJSONWorkerSource extends VectorTileWorkerSource {
* can correctly serve up tiles.
*
* Defers to {@link GeoJSONWorkerSource#loadGeoJSON} for the fetching/parsing,
* expecting `callback(error, data)` to be called with either an error or a
* parsed GeoJSON object.
*
* When a `loadData` request comes in while a previous one is being processed,
* the previous one is aborted.
*
* @param params - the parameters
* @returns a promise that resolves when the data is loaded
* @returns a promise that resolves when the data is loaded and parsed into a GeoJSON object
*/
async loadData(params: LoadGeoJSONParameters): Promise<GeoJSONWorkerSourceLoadDataResult> {
this._pendingRequest?.abort();
Expand Down Expand Up @@ -176,8 +174,7 @@ export class GeoJSONWorkerSource extends VectorTileWorkerSource {
}

/**
* Fetch and parse GeoJSON according to the given params. Calls `callback`
* with `(err, data)`, where `data` is a parsed GeoJSON object.
* Fetch and parse GeoJSON according to the given params.
*
* GeoJSON is loaded and parsed from `params.url` if it exists, or else
* expected as a literal (string or object) `params.data`.
Expand Down
31 changes: 15 additions & 16 deletions src/source/image_source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,15 @@ export class ImageSource extends Evented implements Source {
this.options = options;
}

load = (newCoordinates?: Coordinates, successCallback?: () => void) => {
async load(newCoordinates?: Coordinates): Promise<void> {
this._loaded = false;
this.fire(new Event('dataloading', {dataType: 'source'}));

this.url = this.options.url;

this._request = new AbortController();
ImageRequest.getImage(this.map._requestManager.transformRequest(this.url, ResourceType.Image), this._request).then((image) => {
try {
const image = await ImageRequest.getImage(this.map._requestManager.transformRequest(this.url, ResourceType.Image), this._request);
this._request = null;
this._loaded = true;

Expand All @@ -143,16 +144,13 @@ export class ImageSource extends Evented implements Source {
if (newCoordinates) {
this.coordinates = newCoordinates;
}
if (successCallback) {
successCallback();
}
this._finishLoading();
}
}).catch((err) => {
} catch (err) {
this._request = null;
this.fire(new ErrorEvent(err));
});
};
}
}

loaded(): boolean {
return this._loaded;
Expand All @@ -176,7 +174,7 @@ export class ImageSource extends Evented implements Source {
}

this.options.url = options.url;
this.load(options.coordinates, () => { this.texture = null; });
this.load(options.coordinates).finally(() => { this.texture = null; });
return this;
}

Expand Down Expand Up @@ -246,7 +244,7 @@ export class ImageSource extends Evented implements Source {
return this;
}

prepare = () => {
prepare() {
if (Object.keys(this.tiles).length === 0 || !this.image) {
return;
}
Expand Down Expand Up @@ -280,9 +278,9 @@ export class ImageSource extends Evented implements Source {
if (newTilesLoaded) {
this.fire(new Event('data', {dataType: 'source', sourceDataType: 'idle', sourceId: this.id}));
}
};
}

loadTile(tile: Tile, callback: Callback<void>) {
async loadTile(tile: Tile, callback?: Callback<void>): Promise<void> {
// We have a single tile -- whose coordinates are this.tileID -- that
// covers the image we want to render. If that's the one being
// requested, set it up with the image; otherwise, mark the tile as
Expand All @@ -292,20 +290,21 @@ export class ImageSource extends Evented implements Source {
if (this.tileID && this.tileID.equals(tile.tileID.canonical)) {
this.tiles[String(tile.tileID.wrap)] = tile;
tile.buckets = {};
callback(null);
} else {
tile.state = 'errored';
callback(null);
}
if (callback) {
callback();
}
}

serialize = (): ImageSourceSpecification | VideoSourceSpecification | CanvasSourceSpecification => {
serialize(): ImageSourceSpecification | VideoSourceSpecification | CanvasSourceSpecification {
return {
type: 'image',
url: this.options.url,
coordinates: this.coordinates
};
};
}

hasTransition() {
return false;
Expand Down
Loading