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

Move image queue to use promises, move getArrayBuffer to use promises #3280

Merged
merged 15 commits into from
Oct 30, 2023
51 changes: 21 additions & 30 deletions src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,64 +34,55 @@ describe('maplibre', () => {
expect(Object.keys(config.REGISTERED_PROTOCOLS)).toHaveLength(0);
});

test('#addProtocol - getJSON', done => {
test('#addProtocol - getJSON', async () => {
let protocolCallbackCalled = false;
maplibre.addProtocol('custom', (reqParam, callback) => {
protocolCallbackCalled = true;
callback(null, {'foo': 'bar'});
return {cancel: () => {}};
});
getJSON({url: 'custom://test/url/json'}, new AbortController()).then((response) => {
expect(response.data).toEqual({foo: 'bar'});
expect(protocolCallbackCalled).toBeTruthy();
done();
});
const response = await getJSON({url: 'custom://test/url/json'}, new AbortController());
expect(response.data).toEqual({foo: 'bar'});
expect(protocolCallbackCalled).toBeTruthy();
});

test('#addProtocol - getArrayBuffer', done => {
test('#addProtocol - getArrayBuffer', async () => {
let protocolCallbackCalled = false;
maplibre.addProtocol('custom', (reqParam, callback) => {
maplibre.addProtocol('custom', (_reqParam, callback) => {
protocolCallbackCalled = true;
callback(null, new ArrayBuffer(1));
callback(null, new ArrayBuffer(1), 'cache-control', 'expires');
return {cancel: () => {}};
});
getArrayBuffer({url: 'custom://test/url/getArrayBuffer'}, async (error, data) => {
expect(error).toBeFalsy();
expect(data).toBeInstanceOf(ArrayBuffer);
expect(protocolCallbackCalled).toBeTruthy();
done();
});
const response = await getArrayBuffer({url: 'custom://test/url/getArrayBuffer'}, new AbortController());
expect(response.data).toBeInstanceOf(ArrayBuffer);
expect(response.cacheControl).toBe('cache-control');
expect(response.expires).toBe('expires');
expect(protocolCallbackCalled).toBeTruthy();
});

test('#addProtocol - returning ImageBitmap for getImage', done => {
test('#addProtocol - returning ImageBitmap for getImage', async () => {
let protocolCallbackCalled = false;
maplibre.addProtocol('custom', (reqParam, callback) => {
maplibre.addProtocol('custom', (_reqParam, callback) => {
protocolCallbackCalled = true;
callback(null, new ImageBitmap());
return {cancel: () => {}};
});

ImageRequest.getImage({url: 'custom://test/url/getImage'}, async (error, img) => {
expect(error).toBeFalsy();
expect(img).toBeInstanceOf(ImageBitmap);
expect(protocolCallbackCalled).toBeTruthy();
done();
});
const img = await ImageRequest.getImage({url: 'custom://test/url/getImage'}, new AbortController());
expect(img.data).toBeInstanceOf(ImageBitmap);
expect(protocolCallbackCalled).toBeTruthy();
});

test('#addProtocol - returning HTMLImageElement for getImage', done => {
test('#addProtocol - returning HTMLImageElement for getImage', async () => {
let protocolCallbackCalled = false;
maplibre.addProtocol('custom', (reqParam, callback) => {
protocolCallbackCalled = true;
callback(null, new Image());
return {cancel: () => {}};
});
ImageRequest.getImage({url: 'custom://test/url/getImage'}, async (error, img) => {
expect(error).toBeFalsy();
expect(img).toBeInstanceOf(HTMLImageElement);
expect(protocolCallbackCalled).toBeTruthy();
done();
});
const img = await ImageRequest.getImage({url: 'custom://test/url/getImage'}, new AbortController());
expect(img.data).toBeInstanceOf(HTMLImageElement);
expect(protocolCallbackCalled).toBeTruthy();
});

test('#addProtocol - error', () => {
Expand Down
12 changes: 8 additions & 4 deletions src/source/image_source.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,15 @@ describe('ImageSource', () => {
expect(source.tileSize).toBe(512);
});

test('fires dataloading event', () => {
test('fires dataloading event', async () => {
const source = createSource({url: '/image.png'});
source.on('dataloading', (e) => {
expect(e.dataType).toBe('source');
});
source.onAdd(new StubMap() as any);
server.respond();
// HM TODO: move this to a utility method
await new Promise((resolve) => (setTimeout(resolve, 0)));
expect(source.image).toBeTruthy();
});

Expand Down Expand Up @@ -110,7 +112,7 @@ describe('ImageSource', () => {
expect(afterSerialized.coordinates).toEqual([[0, 0], [-1, 0], [-1, -1], [0, -1]]);
});

test('sets coordinates via updateImage', () => {
test('sets coordinates via updateImage', async () => {
const source = createSource({url: '/image.png'});
const map = new StubMap() as any;
source.onAdd(map);
Expand All @@ -122,6 +124,7 @@ describe('ImageSource', () => {
coordinates: [[0, 0], [-1, 0], [-1, -1], [0, -1]]
});
server.respond();
await new Promise((resolve) => (setTimeout(resolve, 0)));
const afterSerialized = source.serialize();
expect(afterSerialized.coordinates).toEqual([[0, 0], [-1, 0], [-1, -1], [0, -1]]);
});
Expand Down Expand Up @@ -179,15 +182,16 @@ describe('ImageSource', () => {
expect(serialized.coordinates).toEqual([[0, 0], [1, 0], [1, 1], [0, 1]]);
});

test('allows using updateImage before initial image is loaded', () => {
test('allows using updateImage before initial image is loaded', async () => {
const source = createSource({url: '/image.png'});
const map = new StubMap() as any;

source.onAdd(map);

expect(source.image).toBeUndefined();
source.updateImage({url: '/image2.png'});
server.respond();
await new Promise((resolve) => (setTimeout(resolve, 10)));

expect(source.image).toBeTruthy();
});

Expand Down
19 changes: 10 additions & 9 deletions src/source/image_source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import type {
ImageSourceSpecification,
VideoSourceSpecification
} from '@maplibre/maplibre-gl-style-spec';
import {Cancelable} from '../types/cancelable';

/**
* Four geographical coordinates,
Expand Down Expand Up @@ -107,7 +106,7 @@ export class ImageSource extends Evented implements Source {
boundsBuffer: VertexBuffer;
boundsSegments: SegmentVector;
_loaded: boolean;
_request: Cancelable;
_request: AbortController;

/** @internal */
constructor(id: string, options: ImageSourceSpecification | VideoSourceSpecification | CanvasSourceSpecification, dispatcher: Dispatcher, eventedParent: Evented) {
Expand All @@ -134,14 +133,13 @@ export class ImageSource extends Evented implements Source {

this.url = this.options.url;

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

if (err) {
this.fire(new ErrorEvent(err));
} else if (image) {
this.image = image;
if (image && image.data) {
this.image = image.data;
if (newCoordinates) {
this.coordinates = newCoordinates;
}
Expand All @@ -150,6 +148,9 @@ export class ImageSource extends Evented implements Source {
}
this._finishLoading();
}
}).catch((err) => {
this._request = null;
this.fire(new ErrorEvent(err));
});
};

Expand All @@ -170,7 +171,7 @@ export class ImageSource extends Evented implements Source {
}

if (this._request) {
this._request.cancel();
this._request.abort();
this._request = null;
}

Expand All @@ -193,7 +194,7 @@ export class ImageSource extends Evented implements Source {

onRemove() {
if (this._request) {
this._request.cancel();
this._request.abort();
this._request = null;
}
}
Expand Down
27 changes: 18 additions & 9 deletions src/source/raster_dem_tile_source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import type {Dispatcher} from '../util/dispatcher';
import type {Tile} from './tile';
import type {Callback} from '../types/callback';
import type {RasterDEMSourceSpecification} from '@maplibre/maplibre-gl-style-spec';
import type {ExpiryData} from '../util/ajax';
import {isOffscreenCanvasDistorted} from '../util/offscreen_canvas_distorted';
import {RGBAImage} from '../util/image';

Expand Down Expand Up @@ -58,16 +57,17 @@ export class RasterDEMTileSource extends RasterTileSource implements Source {
const url = tile.tileID.canonical.url(this.tiles, this.map.getPixelRatio(), this.scheme);
const request = this.map._requestManager.transformRequest(url, ResourceType.Tile);
tile.neighboringTiles = this._getNeighboringTiles(tile.tileID);
tile.request = ImageRequest.getImage(request, async (err: Error, img: (HTMLImageElement | ImageBitmap), expiry: ExpiryData) => {
delete tile.request;
tile.abortController = new AbortController();
ImageRequest.getImage(request, tile.abortController, this.map._refreshExpiredTiles).then(async (response) => {
delete tile.abortController;
if (tile.aborted) {
tile.state = 'unloaded';
callback(null);
} else if (err) {
tile.state = 'errored';
callback(err);
} else if (img) {
if (this.map._refreshExpiredTiles) tile.setExpiryData(expiry);
} else if (response && response.data) {
const img = response.data;
if (this.map._refreshExpiredTiles && response.cacheControl && response.expires) {
tile.setExpiryData({cacheControl: response.cacheControl, expires: response.expires});
}
const transfer = isImageBitmap(img) && offscreenCanvasSupported();
const rawImageData = transfer ? img : await readImageNow(img);
const params = {
Expand Down Expand Up @@ -98,7 +98,16 @@ export class RasterDEMTileSource extends RasterTileSource implements Source {
}
}
}
}, this.map._refreshExpiredTiles);
}).catch((err) => {
delete tile.abortController;
if (tile.aborted) {
tile.state = 'unloaded';
callback(null);
} else if (err) {
tile.state = 'errored';
callback(err);
}
});

async function readImageNow(img: ImageBitmap | HTMLImageElement): Promise<RGBAImage | ImageData> {
if (typeof VideoFrame !== 'undefined' && isOffscreenCanvasDistorted()) {
Expand Down
34 changes: 21 additions & 13 deletions src/source/raster_tile_source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,20 +158,19 @@ export class RasterTileSource extends Evented implements Source {

loadTile(tile: Tile, callback: Callback<void>) {
const url = tile.tileID.canonical.url(this.tiles, this.map.getPixelRatio(), this.scheme);
tile.request = ImageRequest.getImage(this.map._requestManager.transformRequest(url, ResourceType.Tile), (err, img, expiry) => {
delete tile.request;

tile.abortController = new AbortController();
ImageRequest.getImage(this.map._requestManager.transformRequest(url, ResourceType.Tile), tile.abortController, this.map._refreshExpiredTiles).then((response) => {
delete tile.abortController;
if (tile.aborted) {
tile.state = 'unloaded';
callback(null);
} else if (err) {
tile.state = 'errored';
callback(err);
} else if (img) {
if (this.map._refreshExpiredTiles && expiry) tile.setExpiryData(expiry);

} else if (response && response.data) {
if (this.map._refreshExpiredTiles && response.cacheControl && response.expires) {
tile.setExpiryData({cacheControl: response.cacheControl, expires: response.expires});
}
const context = this.map.painter.context;
const gl = context.gl;
const img = response.data;
tile.texture = this.map.painter.getTileTexture(img.width);
if (tile.texture) {
tile.texture.update(img, {useMipmap: true});
Expand All @@ -188,13 +187,22 @@ export class RasterTileSource extends Evented implements Source {

callback(null);
}
}, this.map._refreshExpiredTiles);
}).catch((err) => {
delete tile.abortController;
if (tile.aborted) {
tile.state = 'unloaded';
callback(null);
} else if (err) {
tile.state = 'errored';
callback(err);
}
});
}

abortTile(tile: Tile, callback: Callback<void>) {
if (tile.request) {
tile.request.cancel();
delete tile.request;
if (tile.abortController) {
tile.abortController.abort();
delete tile.abortController;
}
callback();
}
Expand Down
18 changes: 8 additions & 10 deletions src/source/rtl_text_plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,16 +83,14 @@ export const downloadRTLTextPlugin = () => {
}
pluginStatus = status.loading;
sendPluginStateToWorker();
if (pluginURL) {
getArrayBuffer({url: pluginURL}, (error) => {
if (error) {
triggerPluginCompletionEvent(error);
} else {
pluginStatus = status.loaded;
sendPluginStateToWorker();
}
});
}
getArrayBuffer({url: pluginURL}, new AbortController()).then(() => {
pluginStatus = status.loaded;
sendPluginStateToWorker();
}).catch((error) => {
if (error) {
triggerPluginCompletionEvent(error);
}
});
};

export const plugin: {
Expand Down
3 changes: 0 additions & 3 deletions src/source/tile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import type {OverscaledTileID} from './tile_id';
import type {Framebuffer} from '../gl/framebuffer';
import type {Transform} from '../geo/transform';
import type {LayerFeatureStates} from './source_state';
import type {Cancelable} from '../types/cancelable';
import type {FilterSpecification} from '@maplibre/maplibre-gl-style-spec';
import type Point from '@mapbox/point-geometry';
import {mat4} from 'gl-matrix';
Expand Down Expand Up @@ -81,8 +80,6 @@ export class Tile {
aborted: boolean;
needsHillshadePrepare: boolean;
needsTerrainPrepare: boolean;
// HM TODO: remove this once we migrate to abort contoller
request: Cancelable;
abortController: AbortController;
texture: any;
fbo: Framebuffer;
Expand Down
Loading