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 29 commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
- Convert plantuml diagrams to mermaid ([#3217](https://github.com/maplibre/maplibre-gl-js/pull/3217))
- Improve buffer transfer in Safari after Safari fixed a memory leak bug ([#3225](https://github.com/maplibre/maplibre-gl-js/pull/3225))
- Minify internal exports to reduce bundle size ([#3216](https://github.com/maplibre/maplibre-gl-js/pull/3216))
- ⚠️ Change the undeling worker communication from callbacks to promises. This has a breaking effect on the implementation of custom `WorkerSource` and how it behaves ([#3233](https://github.com/maplibre/maplibre-gl-js/pull/3233))
- _...Add new stuff here..._

### 🐞 Bug fixes
Expand Down
4 changes: 2 additions & 2 deletions src/data/dem_data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {register} from '../util/web_worker_transfer';
export type DEMEncoding = 'mapbox' | 'terrarium' | 'custom'

export class DEMData {
uid: string;
uid: string | number;
data: Uint32Array;
stride: number;
dim: number;
Expand All @@ -29,7 +29,7 @@ export class DEMData {

// RGBAImage data has uniform 1px padding on all sides: square tile edge size defines stride
// and dim is calculated as stride - 2.
constructor(uid: string, data: RGBAImage, encoding: DEMEncoding, redFactor = 1.0, greenFactor = 1.0, blueFactor = 1.0, baseShift = 0.0) {
constructor(uid: string | number, data: RGBAImage | ImageData, encoding: DEMEncoding, redFactor = 1.0, greenFactor = 1.0, blueFactor = 1.0, baseShift = 0.0) {
this.uid = uid;
if (data.height !== data.width) throw new RangeError('DEM tiles must be square');
if (encoding && !['mapbox', 'terrarium', 'custom'].includes(encoding)) {
Expand Down
9 changes: 3 additions & 6 deletions src/render/glyph_atlas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import {AlphaImage} from '../util/image';
import {register} from '../util/web_worker_transfer';
import potpack from 'potpack';

import type {GlyphMetrics, StyleGlyph} from '../style/style_glyph';
import type {GlyphMetrics} from '../style/style_glyph';
import type {GetGlyphsResponse} from '../util/actor_messages';

const padding = 1;

Expand Down Expand Up @@ -37,11 +38,7 @@ export class GlyphAtlas {
image: AlphaImage;
positions: GlyphPositions;

constructor(stacks: {
[_: string]: {
[_: number]: StyleGlyph;
};
}) {
constructor(stacks: GetGlyphsResponse) {
const positions = {};
const bins = [];

Expand Down
32 changes: 11 additions & 21 deletions src/render/glyph_manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ describe('GlyphManager', () => {
createLoadGlyphRangeStub();
const manager = createGlyphManager();

manager.getGlyphs({'Arial Unicode MS': [55]}, (err, glyphs) => {
expect(err).toBeFalsy();
manager.getGlyphs({'Arial Unicode MS': [55]}).then((glyphs) => {
expect(glyphs['Arial Unicode MS']['55'].metrics.advance).toBe(12);
done();
});
Expand All @@ -47,16 +46,14 @@ describe('GlyphManager', () => {
const stub = createLoadGlyphRangeStub();
const manager = createGlyphManager();

manager.getGlyphs({'Arial Unicode MS': [0.5]}, (err) => {
expect(err).toBeFalsy();
manager.getGlyphs({'Arial Unicode MS': [0.5]}).then(() => {
expect(manager.entries['Arial Unicode MS'].ranges[0]).toBe(true);
expect(stub).toHaveBeenCalledTimes(1);

// We remove all requests as in getGlyphs code.
delete manager.entries['Arial Unicode MS'].requests[0];

manager.getGlyphs({'Arial Unicode MS': [0.5]}, (err) => {
expect(err).toBeFalsy();
manager.getGlyphs({'Arial Unicode MS': [0.5]}).then(() => {
expect(manager.entries['Arial Unicode MS'].ranges[0]).toBe(true);
expect(stub).toHaveBeenCalledTimes(1);
done();
Expand All @@ -71,8 +68,7 @@ describe('GlyphManager', () => {

const manager = createGlyphManager();

manager.getGlyphs({'Arial Unicode MS': [0x5e73]}, (err, glyphs) => {
expect(err).toBeFalsy();
manager.getGlyphs({'Arial Unicode MS': [0x5e73]}).then((glyphs) => {
expect(glyphs['Arial Unicode MS'][0x5e73]).toBeNull(); // The fixture returns a PBF without the glyph we requested
done();
});
Expand All @@ -92,12 +88,10 @@ describe('GlyphManager', () => {
const manager = createGlyphManager('sans-serif');

//Request char that overlaps Katakana range
manager.getGlyphs({'Arial Unicode MS': [0x3005]}, (err, glyphs) => {
expect(err).toBeFalsy();
manager.getGlyphs({'Arial Unicode MS': [0x3005]}).then((glyphs) => {
expect(glyphs['Arial Unicode MS'][0x3005]).not.toBeNull();
//Request char from Katakana range (te テ)
manager.getGlyphs({'Arial Unicode MS': [0x30C6]}, (err, glyphs) => {
expect(err).toBeFalsy();
manager.getGlyphs({'Arial Unicode MS': [0x30C6]}).then((glyphs) => {
const glyph = glyphs['Arial Unicode MS'][0x30c6];
//Ensure that te is locally generated.
expect(glyph.bitmap.height).toBe(12);
Expand All @@ -111,8 +105,7 @@ describe('GlyphManager', () => {
const manager = createGlyphManager('sans-serif');

// character 平
manager.getGlyphs({'Arial Unicode MS': [0x5e73]}, (err, glyphs) => {
expect(err).toBeFalsy();
manager.getGlyphs({'Arial Unicode MS': [0x5e73]}).then((glyphs) => {
expect(glyphs['Arial Unicode MS'][0x5e73].metrics.advance).toBe(0.5);
done();
});
Expand All @@ -122,8 +115,7 @@ describe('GlyphManager', () => {
const manager = createGlyphManager('sans-serif');

// Katakana letter te テ
manager.getGlyphs({'Arial Unicode MS': [0x30c6]}, (err, glyphs) => {
expect(err).toBeFalsy();
manager.getGlyphs({'Arial Unicode MS': [0x30c6]}).then((glyphs) => {
expect(glyphs['Arial Unicode MS'][0x30c6].metrics.advance).toBe(0.5);
done();
});
Expand All @@ -133,8 +125,7 @@ describe('GlyphManager', () => {
const manager = createGlyphManager('sans-serif');

//Hiragana letter te て
manager.getGlyphs({'Arial Unicode MS': [0x3066]}, (err, glyphs) => {
expect(err).toBeFalsy();
manager.getGlyphs({'Arial Unicode MS': [0x3066]}).then((glyphs) => {
expect(glyphs['Arial Unicode MS'][0x3066].metrics.advance).toBe(0.5);
done();
});
Expand All @@ -148,10 +139,9 @@ describe('GlyphManager', () => {
});

// Katakana letter te
manager.getGlyphs({'Arial Unicode MS': [0x30c6]}, (err, glyphs) => {
expect(err).toBeFalsy();
manager.getGlyphs({'Arial Unicode MS': [0x30c6]}).then((glyphs) => {
expect(glyphs['Arial Unicode MS'][0x30c6].metrics.advance).toBe(24);
manager.getGlyphs({'Arial Unicode MS': [0x30c6]}, () => {
manager.getGlyphs({'Arial Unicode MS': [0x30c6]}).then(() => {
expect(drawSpy).toHaveBeenCalledTimes(1);
done();
});
Expand Down
174 changes: 81 additions & 93 deletions src/render/glyph_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import {loadGlyphRange} from '../style/load_glyph_range';

import TinySDF from '@mapbox/tiny-sdf';
import {unicodeBlockLookup} from '../util/is_char_in_unicode_block';
import {asyncAll} from '../util/util';
import {AlphaImage} from '../util/image';

import type {StyleGlyph} from '../style/style_glyph';
import type {RequestManager} from '../util/request_manager';
import type {Callback} from '../types/callback';
import type {GetGlyphsResponse} from '../util/actor_messages';

type Entry = {
// null means we've requested the range, but the glyph wasn't included in the result.
Expand Down Expand Up @@ -47,117 +47,105 @@ export class GlyphManager {
this.url = url;
}

getGlyphs(glyphs: {
[stack: string]: Array<number>;
}, callback: Callback<{
[stack: string]: {
[id: number]: StyleGlyph;
};
}>) {
async getGlyphs(glyphs: {[stack: string]: Array<number>}): Promise<GetGlyphsResponse> {
const all = [];

for (const stack in glyphs) {
for (const id of glyphs[stack]) {
all.push({stack, id});
}
}
const promises = all.map(({stack, id}) => {
HarelM marked this conversation as resolved.
Show resolved Hide resolved
return new Promise<{stack: string; id: number; glyph: StyleGlyph}>((resolve, reject) => {
let entry = this.entries[stack];
if (!entry) {
entry = this.entries[stack] = {
glyphs: {},
requests: {},
ranges: {}
};
}

asyncAll(all, ({stack, id}, callback: Callback<{
stack: string;
id: number;
glyph: StyleGlyph;
}>) => {
let entry = this.entries[stack];
if (!entry) {
entry = this.entries[stack] = {
glyphs: {},
requests: {},
ranges: {}
};
}

let glyph = entry.glyphs[id];
if (glyph !== undefined) {
callback(null, {stack, id, glyph});
return;
}
let glyph = entry.glyphs[id];
if (glyph !== undefined) {
resolve({stack, id, glyph});
return;
}

glyph = this._tinySDF(entry, stack, id);
if (glyph) {
entry.glyphs[id] = glyph;
callback(null, {stack, id, glyph});
return;
}
glyph = this._tinySDF(entry, stack, id);
if (glyph) {
entry.glyphs[id] = glyph;
resolve({stack, id, glyph});
return;
}

const range = Math.floor(id / 256);
if (range * 256 > 65535) {
callback(new Error('glyphs > 65535 not supported'));
return;
}
const range = Math.floor(id / 256);
if (range * 256 > 65535) {
reject(new Error('glyphs > 65535 not supported'));
return;
}

if (entry.ranges[range]) {
callback(null, {stack, id, glyph});
return;
}
if (entry.ranges[range]) {
resolve({stack, id, glyph});
return;
}

if (!this.url) {
callback(new Error('glyphsUrl is not set'));
return;
}
if (!this.url) {
reject(new Error('glyphsUrl is not set'));
return;
}

let requests = entry.requests[range];
if (!requests) {
requests = entry.requests[range] = [];
GlyphManager.loadGlyphRange(stack, range, this.url, this.requestManager,
(err, response?: {
[_: number]: StyleGlyph | null;
} | null) => {
if (response) {
for (const id in response) {
if (!this._doesCharSupportLocalGlyph(+id)) {
entry.glyphs[+id] = response[+id];
let requests = entry.requests[range];
if (!requests) {
requests = entry.requests[range] = [];
GlyphManager.loadGlyphRange(stack, range, this.url, this.requestManager,
(err, response?: {
[_: number]: StyleGlyph | null;
} | null) => {
if (response) {
for (const id in response) {
if (!this._doesCharSupportLocalGlyph(+id)) {
entry.glyphs[+id] = response[+id];
}
}
entry.ranges[range] = true;
}
entry.ranges[range] = true;
}
for (const cb of requests) {
cb(err, response);
}
delete entry.requests[range];
});
}

requests.push((err, result?: {
[_: number]: StyleGlyph | null;
} | null) => {
if (err) {
callback(err);
} else if (result) {
callback(null, {stack, id, glyph: result[id] || null});
for (const cb of requests) {
cb(err, response);
}
delete entry.requests[range];
});
}

requests.push((err, result?: {
[_: number]: StyleGlyph | null;
} | null) => {
if (err) {
reject(err);
} else if (result) {
resolve({stack, id, glyph: result[id] || null});
}
});
});
}, (err, glyphs?: Array<{
stack: string;
id: number;
glyph: StyleGlyph;
}> | null) => {
if (err) {
callback(err);
} else if (glyphs) {
const result = {};

for (const {stack, id, glyph} of glyphs) {
// Clone the glyph so that our own copy of its ArrayBuffer doesn't get transferred.
(result[stack] || (result[stack] = {}))[id] = glyph && {
id: glyph.id,
bitmap: glyph.bitmap.clone(),
metrics: glyph.metrics
};
}
});

const updatedGlyphs = await Promise.all(promises);
HarelM marked this conversation as resolved.
Show resolved Hide resolved

const result: GetGlyphsResponse = {};

callback(null, result);
for (const {stack, id, glyph} of updatedGlyphs) {
if (!result[stack]) {
result[stack] = {};
}
});
// Clone the glyph so that our own copy of its ArrayBuffer doesn't get transferred.
result[stack][id] = glyph && {
id: glyph.id,
bitmap: glyph.bitmap.clone(),
metrics: glyph.metrics
};
}

return result;
}

_doesCharSupportLocalGlyph(id: number): boolean {
Expand Down
3 changes: 2 additions & 1 deletion src/render/image_atlas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type {StyleImage} from '../style/style_image';
import type {ImageManager} from './image_manager';
import type {Texture} from './texture';
import type {Rect} from './glyph_atlas';
import type {GetImagesResponse} from '../util/actor_messages';

const IMAGE_PADDING: number = 1;
export {IMAGE_PADDING};
Expand Down Expand Up @@ -71,7 +72,7 @@ export class ImageAtlas {
haveRenderCallbacks: Array<string>;
uploaded: boolean;

constructor(icons: {[_: string]: StyleImage}, patterns: {[_: string]: StyleImage}) {
constructor(icons: GetImagesResponse, patterns: GetImagesResponse) {
const iconPositions = {}, patternPositions = {};
this.haveRenderCallbacks = [];

Expand Down
Loading