Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -6,6 +6,7 @@
- _...Add new stuff here..._

### 🐞 Bug fixes
- Report worker script load, execution, and message errors through the map `error` event instead of leaving maps with vector sources silently stuck when the worker cannot start ([#8018](https://github.com/maplibre/maplibre-gl-js/issues/8018)) (by [@NearlCrews](https://github.com/NearlCrews))
- Fix a permanent frame rate degradation after switching styles: every sprite reload marked its images as updated forever, making every in-view tile re-check and re-upload them on every frame. Also stop leaking the images of a replaced sprite, which were never removed from the image manager ([#8052](https://github.com/maplibre/maplibre-gl-js/issues/8052)) (by [@HarelM](https://github.com/HarelM))
- Prevent a rejected missing style image resolver from blocking successfully resolved images in the same batch ([#8146](https://github.com/maplibre/maplibre-gl-js/pull/8146/)) (by @birkskyum)
- Explicitly request no browser color management when decoding raster-DEM tiles so their RGB-encoded elevation values are not changed (what would otherwise happen with `gfx.color_management.mode = 1` in Firefox) ([#8125](https://github.com/maplibre/maplibre-gl-js/pull/8125)) (by [@tnikkel](https://github.com/tnikkel))
Expand Down
1 change: 1 addition & 0 deletions src/style/style.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ export class Style extends Evented<MapEventType> {

this.map = map;
this.dispatcher = new Dispatcher(getGlobalWorkerPool(), map._getMapId());
this.dispatcher.setEventedParent(this);
this.dispatcher.registerMessageHandler(MessageType.getGlyphs, (mapId, params) => {
return this.getGlyphs(mapId, params);
});
Expand Down
13 changes: 13 additions & 0 deletions src/ui/map_tests/map_events.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,19 @@ beforeEach(() => {

describe('map events', () => {

test('worker script errors fire on the map', async () => {
const map = createMap();
await map.style.dispatcher.actorsPromise;
const errorPromise = map.once('error');

const worker = map.style.dispatcher.actors[0].target as Worker;
worker.dispatchEvent(new globalThis.ErrorEvent('error', {message: 'Script failed to load'}));

const {error} = await errorPromise;
expect(error.message).toContain('Failed to load or execute the MapLibre worker script');
map.remove();
});

test('Map.on adds a non-delegated event listener', () => {
const map = createMap();
const spy = vi.fn(function (e) {
Expand Down
43 changes: 43 additions & 0 deletions src/util/dispatcher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,47 @@ describe('Dispatcher', () => {
dispatcher.remove();
expect(actorsRemoved).toHaveLength(4);
});

test('fires worker script errors and removes the listener on removal', async () => {
const worker = await workerFactory();
const workerPool = {
acquire() {
return Promise.resolve([worker]);
},
release() {}
} as any as WorkerPool;
const dispatcher = new Dispatcher(workerPool, 1);
await dispatcher.actorsPromise;
const listener = vi.fn();
dispatcher.on('error', listener);

worker.dispatchEvent(new globalThis.ErrorEvent('error', {message: 'Script failed to load'}));

expect(listener).toHaveBeenCalledTimes(1);
expect(listener.mock.calls[0][0].error.message).toContain('Failed to load or execute the MapLibre worker script');
expect(listener.mock.calls[0][0].error.message).toContain('Script failed to load');

dispatcher.remove();
worker.dispatchEvent(new globalThis.ErrorEvent('error', {message: 'Another error'}));
expect(listener).toHaveBeenCalledTimes(1);
});

test('fires worker message deserialization errors', async () => {
const worker = await workerFactory();
const workerPool = {
acquire() {
return Promise.resolve([worker]);
},
release() {}
} as any as WorkerPool;
const dispatcher = new Dispatcher(workerPool, 1);
await dispatcher.actorsPromise;
const errorPromise = dispatcher.once('error');

worker.dispatchEvent(new MessageEvent('messageerror'));

const {error} = await errorPromise;
expect(error.message).toContain('Failed to deserialize a message from the MapLibre worker script');
dispatcher.remove();
});
});
33 changes: 32 additions & 1 deletion src/util/dispatcher.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import {Actor, type ActorTarget, type MessageHandler} from './actor.ts';
import {getGlobalWorkerPool} from './global_worker_pool.ts';
import {GLOBAL_DISPATCHER_ID, makeRequest} from './ajax.ts';
import {ErrorEvent, Evented, type ErrorEventType} from './evented.ts';
import {type Subscription, subscribe} from './util.ts';
import {getWorkerUrl} from './web_worker.ts';

import type {WorkerPool} from './worker_pool.ts';
import type {RequestResponseMessageMap} from './actor_messages.ts';
Expand All @@ -9,27 +12,44 @@ import {MessageType} from './actor_messages.ts';
/**
* Responsible for sending messages from a {@link Source} to an associated worker source (usually with the same name).
*/
export class Dispatcher {
export class Dispatcher extends Evented<ErrorEventType> {
workerPool: WorkerPool;
actors: Actor[];
actorsPromise: Promise<Actor[]>;
currentActor: number;
id: string | number;
private removed: boolean;
private workerErrorSubscriptions: Subscription[];

constructor(workerPool: WorkerPool, mapId: string | number) {
super();
this.workerPool = workerPool;
this.actors = [];
this.currentActor = 0;
this.id = mapId;
this.removed = false;
this.workerErrorSubscriptions = [];
this.actorsPromise = this.initActors(mapId);
}

private async initActors(mapId: string | number): Promise<Actor[]> {
const workers = await this.workerPool.acquire(mapId);
if (this.removed) return [];
this.actors = workers.map((worker: ActorTarget, i: number) => {
this.workerErrorSubscriptions.push(
subscribe(worker, 'error', (event: globalThis.ErrorEvent) => {
const details = event.message ? `${event.message}. ` : '';
this.fireWorkerError(
worker,
'Failed to load or execute the MapLibre worker script',
`${details}If you use a bundler, call setWorkerUrl() with the emitted worker URL. ` +
'See the v5 to v6 migration guide.'
);
}, false),
subscribe(worker, 'messageerror', () => {
this.fireWorkerError(worker, 'Failed to deserialize a message from the MapLibre worker script');
}, false)
);
const actor = new Actor(worker, mapId);
actor.name = `Worker ${i}`;
return actor;
Expand All @@ -38,6 +58,13 @@ export class Dispatcher {
return this.actors;
}

private fireWorkerError(worker: ActorTarget, summary: string, details?: string): void {
const url = getWorkerUrl(worker as Worker);
const location = url ? ` at ${url}` : '';
const message = `${summary}${location}.${details ? ` ${details}` : ''}`;
this.fire(new ErrorEvent(new Error(message)));
}

/**
* Broadcast a message to all Workers.
*/
Expand Down Expand Up @@ -72,6 +99,10 @@ export class Dispatcher {
for (const actor of this.actors) {
actor.remove();
}
for (const subscription of this.workerErrorSubscriptions) {
subscription.unsubscribe();
}
this.workerErrorSubscriptions = [];
this.actors = [];
if (mapRemoved) this.workerPool.release(this.id);
}
Expand Down
29 changes: 16 additions & 13 deletions src/util/web_worker.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import {describe, test, expect, beforeEach, afterEach, vi} from 'vitest';
import {workerFactory} from './web_worker.ts';
import {getWorkerUrl, workerFactory} from './web_worker.ts';
import {config} from './config.ts';

function createWorkerStub() {
return {postMessage: vi.fn(), addEventListener: vi.fn(), removeEventListener: vi.fn(), terminate: vi.fn()};
}

describe('workerFactory', () => {
const originalWorker = (globalThis as any).Worker;
const originalWorkerUrl = config.WORKER_URL;
Expand All @@ -17,17 +21,18 @@ describe('workerFactory', () => {
});

test('creates a module worker when WORKER_URL is empty', async () => {
const WorkerSpy = vi.fn();
const WorkerSpy = vi.fn(createWorkerStub);
(globalThis as any).Worker = WorkerSpy;

await workerFactory();
const worker = await workerFactory();

expect(WorkerSpy).toHaveBeenCalledTimes(1);
expect(WorkerSpy.mock.calls[0]).toEqual(['', {type: 'module'}]);
expect(getWorkerUrl(worker)).toBe('');
});

test('creates a classic worker when WORKER_URL ends with .cjs', async () => {
const WorkerSpy = vi.fn();
const WorkerSpy = vi.fn(createWorkerStub);
(globalThis as any).Worker = WorkerSpy;
config.WORKER_URL = '/path/to/worker.cjs';

Expand All @@ -38,7 +43,7 @@ describe('workerFactory', () => {
});

test('creates a module worker when WORKER_URL ends with .mjs', async () => {
const WorkerSpy = vi.fn();
const WorkerSpy = vi.fn(createWorkerStub);
(globalThis as any).Worker = WorkerSpy;
config.WORKER_URL = '/path/to/worker.mjs';

Expand All @@ -50,7 +55,8 @@ describe('workerFactory', () => {

test('falls back to classic worker if module worker construction throws', async () => {
const WorkerSpy = vi.fn()
.mockImplementationOnce(() => { throw new Error('module workers not supported'); });
.mockImplementationOnce(() => { throw new Error('module workers not supported'); })
.mockImplementation(createWorkerStub);
(globalThis as any).Worker = WorkerSpy;
config.WORKER_URL = '/path/to/worker.mjs';

Expand All @@ -68,9 +74,7 @@ describe('workerFactory', () => {
});

test('cross-origin module worker URL is converted to an import script and the worker is constructed from a Blob URL', async () => {
const WorkerSpy = vi.fn(function() {
return {postMessage: vi.fn(), addEventListener: vi.fn(), removeEventListener: vi.fn(), terminate: vi.fn()};
});
const WorkerSpy = vi.fn(createWorkerStub);
(globalThis as any).Worker = WorkerSpy;

const fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValue({
Expand All @@ -83,20 +87,19 @@ describe('workerFactory', () => {

config.WORKER_URL = 'https://unpkg.com/maplibre-gl/dist/maplibre-gl-worker.mjs';

await workerFactory();
const worker = await workerFactory();

expect(fetchSpy).toHaveBeenCalledTimes(0);
expect(BlobSpy).toHaveBeenCalledWith(['import "https://unpkg.com/maplibre-gl/dist/maplibre-gl-worker.mjs"'], {type: 'text/javascript'});
expect(createObjectURLSpy).toHaveBeenCalled();
expect(WorkerSpy).toHaveBeenCalledTimes(1);
expect(WorkerSpy.mock.calls[0]).toEqual(['blob:http://localhost/abc', {type: 'module'}]);
expect(revokeObjectURLSpy).toHaveBeenCalledWith('blob:http://localhost/abc');
expect(getWorkerUrl(worker)).toBe('https://unpkg.com/maplibre-gl/dist/maplibre-gl-worker.mjs');
});

test('cross-origin classic worker URL is fetched and the worker is constructed from a Blob URL', async () => {
const WorkerSpy = vi.fn(function() {
return {postMessage: vi.fn(), addEventListener: vi.fn(), removeEventListener: vi.fn(), terminate: vi.fn()};
});
const WorkerSpy = vi.fn(createWorkerStub);
(globalThis as any).Worker = WorkerSpy;

const fetchSpy = vi.spyOn(globalThis, 'fetch').mockResolvedValue({
Expand Down
19 changes: 14 additions & 5 deletions src/util/web_worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ export interface WorkerGlobalScopeInterface {
worker: MaplibreWorker;
}

const workerUrls = new WeakMap<Worker, string>();

export function getWorkerUrl(worker: Worker): string | undefined {
return workerUrls.get(worker);
}

function isCrossOrigin(url: string): boolean {
if (!url) return false;
const loc = (globalThis as any).location;
Expand All @@ -32,15 +38,18 @@ function defaultWorkerUrl(): string {
return new URL(`./${workerName}`, moduleUrl).href;
}

function createWorker(url: string, asModule: boolean): Worker {
function createWorker(url: string, asModule: boolean, sourceUrl: string = url): Worker {
let worker: Worker | undefined;
if (asModule) {
try {
return new Worker(url, {type: 'module'});
worker = new Worker(url, {type: 'module'});
} catch (e) {
console.warn('Module worker not supported, falling back to classic worker', e);
}
}
return new Worker(url);
worker ||= new Worker(url);
workerUrls.set(worker, sourceUrl);
return worker;
}

async function fetchAsBlobUrl(url: string): Promise<string> {
Expand Down Expand Up @@ -69,15 +78,15 @@ export async function workerFactory(): Promise<Worker> {
if (asModule) {
const blobUrl = importAsBlobUrl(url);
try {
return createWorker(blobUrl, asModule);
return createWorker(blobUrl, asModule, url);
} finally {
URL.revokeObjectURL(blobUrl);
}
}

const blobUrl = await fetchAsBlobUrl(url);
try {
return createWorker(blobUrl, asModule);
return createWorker(blobUrl, asModule, url);
} finally {
URL.revokeObjectURL(blobUrl);
}
Expand Down
4 changes: 2 additions & 2 deletions test/build/bundle_size.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"dist/maplibre-gl.mjs": {
"raw": 566911,
"gzip": 142498
"raw": 567682,
"gzip": 142783
},
"dist/maplibre-gl-worker.mjs": {
"raw": 18592,
Expand Down
28 changes: 19 additions & 9 deletions test/unit/lib/web_worker_mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export class MessageBus implements WorkerGlobalScopeInterface, ActorTarget {
addListeners: EventListener[];
postListeners: EventListener[];
target: MessageBus;
eventListeners: Record<string, EventListener[]>;

registerWorkerSource: any;
registerRTLTextPlugin: any;
Expand All @@ -17,19 +18,27 @@ export class MessageBus implements WorkerGlobalScopeInterface, ActorTarget {
constructor(addListeners: EventListener[], postListeners: EventListener[]) {
this.addListeners = addListeners;
this.postListeners = postListeners;
this.eventListeners = {message: addListeners};
}

addEventListener(event: 'message', callback: EventListener): void {
if (event === 'message') {
this.addListeners.push(callback);
}
addEventListener(event: string, callback: EventListener): void {
this.eventListeners[event] ||= [];
this.eventListeners[event].push(callback);
}

removeEventListener(event: 'message', callback: EventListener): void {
const i = this.addListeners.indexOf(callback);
removeEventListener(event: string, callback: EventListener): void {
const listeners = this.eventListeners[event] || [];
const i = listeners.indexOf(callback);
if (i >= 0) {
this.addListeners.splice(i, 1);
listeners.splice(i, 1);
}
}

dispatchEvent(event: Event): boolean {
for (const listener of this.eventListeners[event.type] || []) {
listener(event);
}
return true;
}

postMessage(data: unknown): void {
Expand All @@ -45,7 +54,9 @@ export class MessageBus implements WorkerGlobalScopeInterface, ActorTarget {
}

terminate(): void {
this.addListeners.splice(0, this.addListeners.length);
for (const listeners of Object.values(this.eventListeners)) {
listeners.splice(0, listeners.length);
}
this.postListeners.splice(0, this.postListeners.length);
}

Expand All @@ -69,4 +80,3 @@ function setGlobalWorker(MockWorker: { new(...args: any): any}) {
}

setGlobalWorker(MapLibreWorker);