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 #3233

Merged
merged 48 commits into from
Nov 23, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
495f45d
Initial concept
HarelM Oct 18, 2023
3c368b0
More improvements
HarelM Oct 20, 2023
f850217
Fix lint
HarelM Oct 20, 2023
1cef230
Main commit to move most of the actor code to use promises and regist…
HarelM Oct 21, 2023
ba3831a
Fix actor tests
HarelM Oct 22, 2023
b485c9b
Remove more any when it comes to worker self typings.
HarelM Oct 22, 2023
0dca144
Improve more types, add more tests
HarelM Oct 22, 2023
931e11b
Improve more types, add more tests
HarelM Oct 22, 2023
7f983e1
Fix lint
HarelM Oct 22, 2023
614524c
Fix more tests
HarelM Oct 22, 2023
4292d0a
Fix tests
HarelM Oct 22, 2023
5734e5e
Fix some sytle tests
HarelM Oct 22, 2023
af2aabb
Add comment for todo.
HarelM Oct 22, 2023
210c425
Fix test related to message parameters change
HarelM Oct 23, 2023
746fe2d
Fix remaining tests
HarelM Oct 23, 2023
8d19b99
Added todo to accomodate for code review.
HarelM Oct 23, 2023
35a58bd
Fix build test
HarelM Oct 23, 2023
62a6a4a
Merge branch 'main' into async-actor
HarelM Oct 23, 2023
9451485
Fix geojson types
HarelM Oct 23, 2023
af73f12
Rename some events, remove clutter a bit
HarelM Oct 23, 2023
6b2c265
Fix issue with multiple maps found in integration tests
HarelM Oct 23, 2023
7a1c9a8
Fix tests and improve types and docs a bit more
HarelM Oct 23, 2023
a18507b
Final fixes to make the tests pass
HarelM Oct 23, 2023
b92e486
Fix build test
HarelM Oct 23, 2023
3a2fdfc
a couple of tweaks (#3267)
msbarry Oct 24, 2023
a365145
Merge remote-tracking branch 'origin/main' into async-actor
HarelM Oct 24, 2023
712cf53
Merge branch 'main' into async-actor
HarelM Oct 24, 2023
fe8e847
Add changelog item
HarelM Oct 24, 2023
23db865
More improvement to async `Actor` and replace more callbacks with pro…
HarelM Oct 25, 2023
b045e38
Replace getJson call with async code and remove more callbacks (#3273)
HarelM Oct 27, 2023
2893ab3
Move image queue to use promises, move getArrayBuffer to use promises…
HarelM Oct 30, 2023
8259409
Remove TODOs, tidy up last things (#3301)
HarelM Nov 2, 2023
837695c
Merge branch 'main' into async-actor
HarelM Nov 2, 2023
5e4cdde
Async actor docs additions (#3305)
HarelM Nov 7, 2023
3c374d6
Merge branch 'main' into async-actor
HarelM Nov 7, 2023
4513eb6
Improve docs for RTL plugin status and reduce code in geojson worker …
HarelM Nov 7, 2023
2a773bf
Last simplification to actor, simplify a style test.
HarelM Nov 7, 2023
001b7a8
Add missing docs comments
HarelM Nov 7, 2023
c63e731
Fix lint...
HarelM Nov 7, 2023
5b2b2bd
Fix Actor XSS, introduce subscribe (#3329)
HarelM Nov 10, 2023
4d99526
Fix developer diagram with updated flow
HarelM Nov 13, 2023
c944799
Merge branch 'main' into async-actor
HarelM Nov 14, 2023
da95a03
Async actor no log warnings and errors in unit tests (#3368)
HarelM Nov 15, 2023
9feae22
Async actor remove cancelables (#3371)
HarelM Nov 15, 2023
6c90f58
Merge branch 'main' into async-actor
HarelM Nov 22, 2023
78bb4a3
Async actor callback and promise (#3374)
HarelM Nov 22, 2023
40846ff
Merge branch 'main' into async-actor
HarelM Nov 23, 2023
3fbff23
Update changelog with most of the changes
HarelM Nov 23, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 13 additions & 16 deletions src/source/raster_dem_tile_source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import type {RasterDEMSourceSpecification} from '@maplibre/maplibre-gl-style-spe
import type {ExpiryData} from '../util/ajax';
import {isOffscreenCanvasDistorted} from '../util/offscreen_canvas_distorted';
import {RGBAImage} from '../util/image';
import {LoadDEMTileMessage} from '../util/actor_messages';

/**
* A source containing raster DEM tiles (See the [Style Specification](https://maplibre.org/maplibre-style-spec/) for detailed documentation of options.)
Expand Down Expand Up @@ -84,7 +85,18 @@ export class RasterDEMTileSource extends RasterTileSource implements Source {

if (!tile.actor || tile.state === 'expired') {
tile.actor = this.dispatcher.getActor();
tile.actor.send('loadDEMTile', params, done);
try {
const data = await tile.actor.sendAsync<LoadDEMTileMessage>({type: 'loadDEMTile', data: params});
HarelM marked this conversation as resolved.
Show resolved Hide resolved
// HM TODO: find a way to fix this linting errors
tile.dem = data; // eslint-disable-line require-atomic-updates
tile.needsHillshadePrepare = true; // eslint-disable-line require-atomic-updates
tile.needsTerrainPrepare = true; // eslint-disable-line require-atomic-updates
tile.state = 'loaded'; // eslint-disable-line require-atomic-updates
callback(null);
} catch (err) {
tile.state = 'errored'; // eslint-disable-line require-atomic-updates
HarelM marked this conversation as resolved.
Show resolved Hide resolved
callback(err);
}
}
}
}, this.map._refreshExpiredTiles);
Expand All @@ -101,21 +113,6 @@ export class RasterDEMTileSource extends RasterTileSource implements Source {
}
return browser.getImageData(img, 1);
}

function done(err, data) {
if (err) {
tile.state = 'errored';
callback(err);
}

if (data) {
tile.dem = data;
tile.needsHillshadePrepare = true;
tile.needsTerrainPrepare = true;
tile.state = 'loaded';
callback(null);
}
}
}

_getNeighboringTiles(tileID: OverscaledTileID) {
Expand Down
13 changes: 5 additions & 8 deletions src/source/raster_dem_tile_worker_source.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,17 @@ import {DEMData} from '../data/dem_data';
import {WorkerDEMTileParameters} from './worker_source';

describe('loadTile', () => {
test('loads DEM tile', done => {
test('loads DEM tile', async () => {
const source = new RasterDEMTileWorkerSource();

source.loadTile({
const data = await source.loadTile({
source: 'source',
uid: '0',
rawImageData: {data: new Uint8ClampedArray(256), height: 8, width: 8},
dim: 256
} as any as WorkerDEMTileParameters, (err, data) => {
if (err) done(err);
expect(Object.keys(source.loaded)).toEqual(['0']);
expect(data instanceof DEMData).toBeTruthy();
done();
});
} as any as WorkerDEMTileParameters);
expect(Object.keys(source.loaded)).toEqual(['0']);
expect(data instanceof DEMData).toBeTruthy();
});
});

Expand Down
5 changes: 2 additions & 3 deletions src/source/raster_dem_tile_worker_source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import {RGBAImage} from '../util/image';
import type {Actor} from '../util/actor';
import type {
WorkerDEMTileParameters,
WorkerDEMTileCallback,
TileParameters
} from './worker_source';
import {getImageData, isImageBitmap} from '../util/util';
Expand All @@ -16,7 +15,7 @@ export class RasterDEMTileWorkerSource {
this.loaded = {};
}

async loadTile(params: WorkerDEMTileParameters, callback: WorkerDEMTileCallback) {
async loadTile(params: WorkerDEMTileParameters): Promise<DEMData | null> {
const {uid, encoding, rawImageData, redFactor, greenFactor, blueFactor, baseShift} = params;
const width = rawImageData.width + 2;
const height = rawImageData.height + 2;
Expand All @@ -26,7 +25,7 @@ export class RasterDEMTileWorkerSource {
const dem = new DEMData(uid, imagePixels, encoding, redFactor, greenFactor, blueFactor, baseShift);
this.loaded = this.loaded || {};
this.loaded[uid] = dem;
callback(null, dem);
return dem;
}

removeTile(params: TileParameters) {
Expand Down
9 changes: 4 additions & 5 deletions src/source/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import type {
WorkerTileParameters,
WorkerDEMTileParameters,
WorkerTileCallback,
WorkerDEMTileCallback,
TileParameters
} from '../source/worker_source';

Expand Down Expand Up @@ -85,6 +84,10 @@ export default class Worker {
globalRTLTextPlugin['processBidirectionalText'] = rtlTextPlugin.processBidirectionalText;
globalRTLTextPlugin['processStyledBidirectionalText'] = rtlTextPlugin.processStyledBidirectionalText;
};

this.actor.registerMessageHandler('loadDEMTile', (mapId: string, params: WorkerDEMTileParameters) => {
return this.getDEMWorkerSource(mapId, params.source).loadTile(params);
});
}

setReferrer(mapID: string, referrer: string) {
Expand Down Expand Up @@ -121,10 +124,6 @@ export default class Worker {
this.getWorkerSource(mapId, params.type, params.source).loadTile(params, callback);
}

loadDEMTile(mapId: string, params: WorkerDEMTileParameters, callback: WorkerDEMTileCallback) {
this.getDEMWorkerSource(mapId, params.source).loadTile(params, callback);
}

reloadTile(mapId: string, params: WorkerTileParameters & {
type: string;
}, callback: WorkerTileCallback) {
Expand Down
3 changes: 1 addition & 2 deletions src/source/worker_source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type {OverscaledTileID} from './tile_id';
import type {Bucket} from '../data/bucket';
import type {FeatureIndex} from '../data/feature_index';
import type {CollisionBoxArray} from '../data/array_types.g';
import type {DEMData, DEMEncoding} from '../data/dem_data';
import type {DEMEncoding} from '../data/dem_data';
import type {StyleGlyph} from '../style/style_glyph';
import type {StyleImage} from '../style/style_image';
import type {PromoteIdSpecification} from '@maplibre/maplibre-gl-style-spec';
Expand Down Expand Up @@ -69,7 +69,6 @@ export type WorkerTileResult = {
};

export type WorkerTileCallback = (error?: Error | null, result?: WorkerTileResult | null) => void;
export type WorkerDEMTileCallback = (err?: Error | null, result?: DEMData | null) => void;

/**
* May be implemented by custom source types to provide code that can be run on
Expand Down
34 changes: 25 additions & 9 deletions src/util/actor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type {WorkerSource} from '../source/worker_source';
import type {OverscaledTileID} from '../source/tile_id';
import type {Callback} from '../types/callback';
import type {StyleGlyph} from '../style/style_glyph';
import type {AsyncMessage, MessageType} from './actor_messages';

export interface ActorTarget {
addEventListener: typeof window.addEventListener;
Expand All @@ -31,13 +32,6 @@ export interface GlyphsProvider {
);
}

export type MessageType = '<response>' | '<cancel>' |
'geojson.getClusterExpansionZoom' | 'geojson.getClusterChildren' | 'geojson.getClusterLeaves' | 'geojson.loadData' |
'removeSource' | 'loadWorkerSource' | 'loadDEMTile' | 'removeDEMTile' |
'removeTile' | 'reloadTile' | 'abortTile' | 'loadTile' | 'getTile' |
'getGlyphs' | 'getImages' | 'setImages' |
'syncRTLPluginState' | 'setReferrer' | 'setLayers' | 'updateLayers';

export type MessageData = {
id: string;
type: MessageType;
Expand Down Expand Up @@ -70,6 +64,7 @@ export class Actor {
cancelCallbacks: { [x: number]: () => void };
invoker: ThrottledInvoker;
globalScope: ActorTarget;
messageHandlers: { [x in MessageType]?: (...args: any[]) => any };

/**
* @param target - The target
Expand All @@ -84,11 +79,28 @@ export class Actor {
this.tasks = {};
this.taskQueue = [];
this.cancelCallbacks = {};
this.messageHandlers = {};
this.invoker = new ThrottledInvoker(this.process);
this.target.addEventListener('message', this.receive, false);
this.globalScope = isWorker() ? target : window;
}

registerMessageHandler(type: MessageType, handler: (...args: any[]) => any) {
this.messageHandlers[type] = handler;
}

sendAsync<T extends AsyncMessage<any>>(message: T): Promise<any> {
HarelM marked this conversation as resolved.
Show resolved Hide resolved
return new Promise((resolve, reject) => {
this.send(message.type, message.data, (err: Error, data: any) => {
if (err) {
reject(err);
} else {
resolve(data);
}
}, message.targetMapId, message.mustQueue);
});
}

/**
* Sends a message from a main-thread map to a Worker or from a Worker back to
* a main-thread map instance.
Expand All @@ -100,7 +112,7 @@ export class Actor {
type: MessageType,
data: unknown,
callback?: Function | null,
targetMapId?: string | null,
targetMapId?: string | number | null,
mustQueue: boolean = false
): Cancelable {
// We're using a string ID instead of numbers because they are being used as object keys
Expand Down Expand Up @@ -236,7 +248,11 @@ export class Actor {

let callback: Cancelable = null;
const params = deserialize(task.data);
if (this.parent[task.type]) {
if (this.messageHandlers[task.type]) {
callback = this.messageHandlers[task.type](task.sourceMapId, params)
.then((data) => done(null, data))
.catch((err) => done(err, null));
} else if (this.parent[task.type]) {
// task.type == 'loadTile', 'removeTile', etc.
callback = this.parent[task.type](task.sourceMapId, params, done);
} else if ('getWorkerSource' in this.parent) {
Expand Down
32 changes: 32 additions & 0 deletions src/util/actor_messages.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import type {DEMEncoding} from '../data/dem_data';
import type {OverscaledTileID} from '../source/tile_id';
import type {RGBAImage} from './image';

export type MessageType = '<response>' | '<cancel>' |
'geojson.getClusterExpansionZoom' | 'geojson.getClusterChildren' | 'geojson.getClusterLeaves' | 'geojson.loadData' |
'removeSource' | 'loadWorkerSource' | 'loadDEMTile' | 'removeDEMTile' |
'removeTile' | 'reloadTile' | 'abortTile' | 'loadTile' | 'getTile' |
'getGlyphs' | 'getImages' | 'setImages' |
'syncRTLPluginState' | 'setReferrer' | 'setLayers' | 'updateLayers';

export type AsyncMessage<T> = {
type: MessageType;
data: T;
targetMapId?: string | number | null;
mustQueue?: boolean;
sourceMapId?: string | number | null;
}

export type LoadDEMTileData = {
uid: number;
coord: OverscaledTileID;
source: string;
rawImageData: ImageBitmap | RGBAImage | ImageData;
encoding: DEMEncoding;
redFactor: number;
greenFactor: number;
blueFactor: number;
baseShift: number;
}

export type LoadDEMTileMessage = AsyncMessage<LoadDEMTileData> & { type: 'loadDEMTile' };
HarelM marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 2 additions & 1 deletion src/util/dispatcher.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import {asyncAll} from './util';
import {Actor, GlyphsProvider, MessageType} from './actor';
import {Actor, GlyphsProvider} from './actor';

import type {WorkerPool} from './worker_pool';
import type {WorkerSource} from '../source/worker_source'; /* eslint-disable-line */ // this is used for the docs' import
import type {MessageType} from './actor_messages';
/**
* Responsible for sending messages from a {@link Source} to an associated
* {@link WorkerSource}.
Expand Down