From 5b2cf858ed18f3eff4701ceee00f8fd17a76c8b6 Mon Sep 17 00:00:00 2001 From: melitele Date: Wed, 21 Jan 2026 22:40:19 -0700 Subject: [PATCH 01/12] remove unnecessary test setup --- test/util/web_worker.js | 57 ----------------------------------------- test/util/window.js | 5 ---- 2 files changed, 62 deletions(-) delete mode 100644 test/util/web_worker.js diff --git a/test/util/web_worker.js b/test/util/web_worker.js deleted file mode 100644 index ad78b5c68..000000000 --- a/test/util/web_worker.js +++ /dev/null @@ -1,57 +0,0 @@ -// The main thread interface. Provided by Worker in a browser environment, -// and MessageBus below in a node environment. - -class MessageBus { - constructor(addListeners, postListeners) { - this.addListeners = addListeners; - this.postListeners = postListeners; - } - - addEventListener(event, callback) { - if (event === 'message') { - this.addListeners.push(callback); - } - } - - removeEventListener(event, callback) { - const i = this.addListeners.indexOf(callback); - if (i >= 0) { - this.addListeners.splice(i, 1); - } - } - - postMessage(data) { - setImmediate(() => { - try { - for (const listener of this.postListeners) { - listener({ data: data, target: this.target }); - } - } catch (e) { - console.error(e); - } - }); - } - - terminate() { - this.addListeners.splice(0, this.addListeners.length); - this.postListeners.splice(0, this.postListeners.length); - } - - importScripts() {} -} - -function WebWorker() { - const parentListeners = []; - const workerListeners = []; - const parentBus = new MessageBus(workerListeners, parentListeners); - const workerBus = new MessageBus(parentListeners, workerListeners); - - parentBus.target = workerBus; - workerBus.target = parentBus; - - new WebWorker.Worker(workerBus); - - return parentBus; -} - -export default WebWorker; diff --git a/test/util/window.js b/test/util/window.js index 7b8181920..c5bfd042e 100644 --- a/test/util/window.js +++ b/test/util/window.js @@ -1,7 +1,6 @@ import canvas from 'canvas'; import gl from 'gl'; import jsdom from 'jsdom'; -import WebWorker from './web_worker.js'; import '../../src/source/rtl_text_plugin.js'; const _window = create(); @@ -61,12 +60,8 @@ function create() { }; window.WebGLFramebuffer ??= Object; - window.Worker ??= WebWorker; - globalThis.document ??= window.document; - window.registerRTLTextPlugin ??= globalThis.registerRTLTextPlugin; - return window; } From 212d48607a17579d6a70f63b899290b9bbf8531f Mon Sep 17 00:00:00 2001 From: melitele Date: Wed, 21 Jan 2026 22:48:21 -0700 Subject: [PATCH 02/12] remove obsolete worker config options --- src/index.js | 19 +------------------ src/util/config.js | 10 +--------- test/unit/mapbox-gl.test.js | 6 +----- 3 files changed, 3 insertions(+), 32 deletions(-) diff --git a/src/index.js b/src/index.js index 725f4142b..465606a70 100644 --- a/src/index.js +++ b/src/index.js @@ -27,24 +27,7 @@ const mapwhit = { Evented }; -const properties = { - workerCount: { - get() { - return config.WORKER_COUNT; - }, - set(count) { - config.WORKER_COUNT = count; - } - }, - workerUrl: { - get() { - return config.WORKER_URL; - }, - set(url) { - config.WORKER_URL = url; - } - } -}; +const properties = {}; Object.defineProperties(mapwhit, properties); Object.defineProperties(config, properties); diff --git a/src/util/config.js b/src/util/config.js index 76135d852..eec211e05 100644 --- a/src/util/config.js +++ b/src/util/config.js @@ -1,9 +1,4 @@ import { Evented } from '@mapwhit/events'; -import browser from './browser.js'; - -function getDefaultWorkerCount() { - return Math.max(Math.floor(browser.hardwareConcurrency / 2), 1); -} const config = new Evented(); @@ -18,7 +13,4 @@ config.notify = function () { config.fire('change', config); }; -config.set({ - WORKER_COUNT: getDefaultWorkerCount(), - WORKER_URL: '' -}); +config.set({}); diff --git a/test/unit/mapbox-gl.test.js b/test/unit/mapbox-gl.test.js index e61c4a549..5ebbfa19e 100644 --- a/test/unit/mapbox-gl.test.js +++ b/test/unit/mapbox-gl.test.js @@ -1,12 +1,8 @@ import test from 'node:test'; -import { config, version } from '../../src/index.js'; +import { version } from '../../src/index.js'; test('mapboxgl', async t => { await t.test('version', t => { t.assert.ok(version); }); - - await t.test('workerCount', t => { - t.assert.ok(typeof config.workerCount === 'number'); - }); }); From 583c9e9af8130d21dbf9a64d0d840e592a87c74e Mon Sep 17 00:00:00 2001 From: Ryan Hamley Date: Mon, 14 Oct 2019 22:11:22 -0700 Subject: [PATCH 03/12] Expose mapboxgl.getRTLTextPluginStatus method (#8864) Original commit: 036ddc4fefc0374fb0cae1b3b698b7164575ec79: --- src/index.js | 15 +++++++++++++-- src/source/rtl_text_plugin.js | 31 ++++++++++++++++++++++--------- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/src/index.js b/src/index.js index 465606a70..f6ea48852 100644 --- a/src/index.js +++ b/src/index.js @@ -5,20 +5,21 @@ import { Point } from '@mapwhit/point-geometry'; import packageJSON from '../package.json' with { type: 'json' }; import { default as LngLat } from './geo/lng_lat.js'; import { default as LngLatBounds } from './geo/lng_lat_bounds.js'; -import { setRTLTextPlugin } from './source/rtl_text_plugin.js'; +import { getRTLTextPluginStatus, setRTLTextPlugin } from './source/rtl_text_plugin.js'; import { default as Style } from './style/style.js'; import { default as Map } from './ui/map.js'; import config from './util/config.js'; const { version } = packageJSON; -export { version, config, setRTLTextPlugin, Point, LngLat, LngLatBounds, Style, Map, Evented }; +export { version, config, setRTLTextPlugin, getRTLTextPluginStatus, Point, LngLat, LngLatBounds, Style, Map, Evented }; // for commonjs backward compatibility const mapwhit = { version, config, setRTLTextPlugin, + getRTLTextPluginStatus, Point, LngLat, LngLatBounds, @@ -53,6 +54,16 @@ export default mapwhit; * @see [Add support for right-to-left scripts](https://www.mapbox.com/mapbox-gl-js/example/mapbox-gl-rtl-text/) */ +/** + * Gets the map's [RTL text plugin](https://www.mapbox.com/mapbox-gl-js/plugins/#mapbox-gl-rtl-text) status. + * The status can be `unavailable` (i.e. not requested or removed), `loading`, `loaded` or `error`. + * If the status is `loaded` and the plugin is requested again, an error will be thrown. + * + * @function getRTLTextPluginStatus + * @example + * const pluginStatus = mapboxgl.getRTLTextPluginStatus(); + */ + // canary assert: used to confirm that asserts have been removed from production build import assert from 'assert'; diff --git a/src/source/rtl_text_plugin.js b/src/source/rtl_text_plugin.js index 175619dc4..2019b9504 100644 --- a/src/source/rtl_text_plugin.js +++ b/src/source/rtl_text_plugin.js @@ -1,9 +1,14 @@ import dynload from 'dynload'; import browser from '../util/browser.js'; -let pluginRequested = false; +const status = { + unavailable: 'unavailable', + loading: 'loading', + loaded: 'loaded', + error: 'error' +}; +let pluginStatus = status.unavailable; let pluginURL; -let loading = false; let _completionCallback; const _loadedCallbacks = []; @@ -15,6 +20,10 @@ const rtlPlugin = { setRTLTextPlugin }; +export function getRTLTextPluginStatus() { + return pluginStatus; +} + export function registerForPluginAvailability(callback) { if (plugin.isLoaded()) { callback(); @@ -27,35 +36,37 @@ export function registerForPluginAvailability(callback) { export function clearRTLTextPlugin() { _loadedCallbacks.length = 0; - pluginRequested = false; + pluginStatus = status.unavailable; pluginURL = undefined; } export function setRTLTextPlugin(url, callback) { - if (pluginRequested) { + if (pluginStatus === status.loading || pluginStatus === status.loaded) { throw new Error('setRTLTextPlugin cannot be called multiple times.'); } - pluginRequested = true; pluginURL = browser.resolveURL(url); _completionCallback = error => { if (error) { const msg = `RTL Text Plugin failed to load scripts from ${pluginURL}`; // Clear loaded state to allow retries clearRTLTextPlugin(); + pluginStatus = status.error; if (callback) { callback(new Error(msg)); } + } else { + pluginStatus = status.loaded; } - loading = false; _completionCallback = undefined; }; loadRTLTextPlugin(); } function loadRTLTextPlugin() { - if (pluginURL && !plugin.isLoaded() && _loadedCallbacks.length > 0 && !loading) { + if (pluginURL && !plugin.isLoaded() && _loadedCallbacks.length > 0 && pluginStatus !== status.loading) { + pluginStatus = status.loading; // needs to be called as exported method for mock testing - loading = rtlPlugin.loadScript(pluginURL).catch(_completionCallback); + rtlPlugin.loadScript(pluginURL).catch(_completionCallback); } } @@ -88,7 +99,9 @@ export const plugin = (rtlPlugin.plugin = { applyArabicShaping: null, processBidirectionalText: null, processStyledBidirectionalText: null, - isLoaded: () => plugin.applyArabicShaping != null + // Foreground: loaded if the completion callback returned successfully + // Background: loaded if the plugin functions have been compiled + isLoaded: () => pluginStatus === status.loaded || plugin.applyArabicShaping != null }); export default rtlPlugin; From fcd7bb506e2a23dd843a9e1c7a6079349fa03517 Mon Sep 17 00:00:00 2001 From: Arindam Bose Date: Thu, 24 Oct 2019 16:23:47 -0700 Subject: [PATCH 04/12] Add `deferred` option `setRTLTextPlugin` to load the plugin only when the first time RTL text is encountered. (#8865) Original commit: d69e24420d9ef4e69315d5dc590ee7e047706aa2: --- src/data/bucket/symbol_bucket.js | 16 ++++++---- src/index.js | 2 ++ src/source/rtl_text_plugin.js | 50 ++++++++++++++++++++------------ src/source/tile.js | 2 ++ src/source/vector_tile_source.js | 7 +++++ src/source/worker_tile.js | 3 ++ src/style/style.js | 3 +- src/util/script_detection.js | 26 ++++++++++++----- test/unit/style/style.test.js | 6 ++-- 9 files changed, 81 insertions(+), 34 deletions(-) diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index 89b3e89da..17a1a0279 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -1,6 +1,7 @@ import { dist } from '@mapwhit/point-geometry'; import { Formatted } from '@mapwhit/style-expressions'; import { VectorTileFeature } from '@mapwhit/vector-tile'; +import { getRTLTextPluginStatus, plugin as globalRTLTextPlugin } from '../../source/rtl_text_plugin.js'; import EvaluationParameters from '../../style/evaluation_parameters.js'; import mergeLines from '../../symbol/mergelines.js'; import { getSizeData } from '../../symbol/symbol_size.js'; @@ -86,6 +87,7 @@ export default class SymbolBucket { this.pixelRatio = options.pixelRatio; this.sourceLayerIndex = options.sourceLayerIndex; this.hasPattern = false; + this.hasRTLText = false; this.sortKeyRanges = []; const layer = this.layers[0]; @@ -169,11 +171,15 @@ export default class SymbolBucket { // but plain string token evaluation skips that pathway so do the // conversion here. const resolvedTokens = layer.getValueAndResolveTokens('text-field', feature); - text = transformText( - resolvedTokens instanceof Formatted ? resolvedTokens : Formatted.fromString(resolvedTokens), - layer, - feature - ); + const formattedText = + resolvedTokens instanceof Formatted ? resolvedTokens : Formatted.fromString(resolvedTokens); + if ( + !this.hasRTLText || // non-rtl text so can proceed safely + getRTLTextPluginStatus() === 'unavailable' || // We don't intend to lazy-load the rtl text plugin, so proceed with incorrect shaping + globalRTLTextPlugin.isParsed() // Use the rtlText plugin to shape text + ) { + text = transformText(formattedText, layer, feature); + } } let icon; diff --git a/src/index.js b/src/index.js index f6ea48852..0e962b484 100644 --- a/src/index.js +++ b/src/index.js @@ -49,6 +49,8 @@ export default mapwhit; * @function setRTLTextPlugin * @param {string} pluginURL URL pointing to the Mapbox RTL text plugin source. * @param {Function} callback Called with an error argument if there is an error. + * @param {boolean} lazy If set to `true`, mapboxgl will defer loading the plugin until rtl text is encountered, + * rtl text will then be rendered only after the plugin finishes loading. * @example * mapboxgl.setRTLTextPlugin('https://api.mapbox.com/mapbox-gl-js/plugins/mapbox-gl-rtl-text/v0.2.0/mapbox-gl-rtl-text.js'); * @see [Add support for right-to-left scripts](https://www.mapbox.com/mapbox-gl-js/example/mapbox-gl-rtl-text/) diff --git a/src/source/rtl_text_plugin.js b/src/source/rtl_text_plugin.js index 2019b9504..38fa55827 100644 --- a/src/source/rtl_text_plugin.js +++ b/src/source/rtl_text_plugin.js @@ -2,21 +2,25 @@ import dynload from 'dynload'; import browser from '../util/browser.js'; const status = { - unavailable: 'unavailable', - loading: 'loading', + unavailable: 'unavailable', // Not loaded + deferred: 'deferred', // The plugin URL has been specified, but loading has been deferred + loading: 'loading', // request in-flight loaded: 'loaded', error: 'error' }; + +let _completionCallback; + +//Variables defining the current state of the plugin let pluginStatus = status.unavailable; let pluginURL; -let _completionCallback; const _loadedCallbacks = []; const rtlPlugin = { clearRTLTextPlugin, // exported for testing loadScript, // exported for testing - registerForPluginAvailability, + registerForPluginStateChange, setRTLTextPlugin }; @@ -24,13 +28,13 @@ export function getRTLTextPluginStatus() { return pluginStatus; } -export function registerForPluginAvailability(callback) { +export function registerForPluginStateChange(callback) { if (plugin.isLoaded()) { callback(); return; } _loadedCallbacks.push(callback); - loadRTLTextPlugin(); + downloadRTLTextPlugin(); return () => _loadedCallbacks.splice(_loadedCallbacks.indexOf(callback), 1); } @@ -40,33 +44,38 @@ export function clearRTLTextPlugin() { pluginURL = undefined; } -export function setRTLTextPlugin(url, callback) { - if (pluginStatus === status.loading || pluginStatus === status.loaded) { +export function setRTLTextPlugin(url, callback, deferred = false) { + if (pluginStatus === status.deferred || pluginStatus === status.loading || pluginStatus === status.loaded) { throw new Error('setRTLTextPlugin cannot be called multiple times.'); } pluginURL = browser.resolveURL(url); + pluginStatus = status.deferred; _completionCallback = error => { if (error) { const msg = `RTL Text Plugin failed to load scripts from ${pluginURL}`; // Clear loaded state to allow retries clearRTLTextPlugin(); pluginStatus = status.error; - if (callback) { - callback(new Error(msg)); - } + error = new Error(msg); } else { pluginStatus = status.loaded; } + if (callback) { + callback(error); + } _completionCallback = undefined; }; - loadRTLTextPlugin(); + //Start downloading the plugin immediately if not intending to lazy-load + if (!deferred) { + downloadRTLTextPlugin(); + } } -function loadRTLTextPlugin() { - if (pluginURL && !plugin.isLoaded() && _loadedCallbacks.length > 0 && pluginStatus !== status.loading) { +export function downloadRTLTextPlugin() { + if (pluginURL && !plugin.isLoading() && !plugin.isLoaded() && _loadedCallbacks.length > 0) { pluginStatus = status.loading; // needs to be called as exported method for mock testing - rtlPlugin.loadScript(pluginURL).catch(_completionCallback); + rtlPlugin.loadScript(pluginURL).then(_completionCallback).catch(_completionCallback); } } @@ -99,9 +108,14 @@ export const plugin = (rtlPlugin.plugin = { applyArabicShaping: null, processBidirectionalText: null, processStyledBidirectionalText: null, - // Foreground: loaded if the completion callback returned successfully - // Background: loaded if the plugin functions have been compiled - isLoaded: () => pluginStatus === status.loaded || plugin.applyArabicShaping != null + // loaded if the completion callback returned successfully or the plugin functions have been compiled + isLoaded: () => pluginStatus === status.loaded || plugin.applyArabicShaping != null, + // query the loading status + isLoading: () => pluginStatus === status.loading, + isParsed: () => + plugin.applyArabicShaping != null && + plugin.processBidirectionalText != null && + plugin.processStyledBidirectionalText != null }); export default rtlPlugin; diff --git a/src/source/tile.js b/src/source/tile.js index 0fc959413..7d48eae07 100644 --- a/src/source/tile.js +++ b/src/source/tile.js @@ -31,6 +31,7 @@ class Tile { this.buckets = new Map(); this.queryPadding = 0; this.hasSymbolBuckets = false; + this.hasRTLText = false; this.state = 'loading'; } @@ -94,6 +95,7 @@ class Tile { this.buckets = data.buckets; this.hasSymbolBuckets = data.hasSymbolBuckets; + this.hasRTLText = data.hasRTLText; this.queryPadding = data.queryPadding; if (data.imageAtlas) { diff --git a/src/source/vector_tile_source.js b/src/source/vector_tile_source.js index a8079a719..2ac9d062d 100644 --- a/src/source/vector_tile_source.js +++ b/src/source/vector_tile_source.js @@ -1,6 +1,7 @@ import { ErrorEvent, Event, Evented } from '@mapwhit/events'; import browser from '../util/browser.js'; import loadTileJSON from './load_tilejson.js'; +import { downloadRTLTextPlugin, getRTLTextPluginStatus, plugin as rtlTextPlugin } from './rtl_text_plugin.js'; import TileBounds from './tile_bounds.js'; import VectorTileWorkerSource from './vector_tile_worker_source.js'; @@ -103,6 +104,12 @@ class VectorTileSource extends Evented { data.rawTileData = rawData; } tile.loadVectorData(data, this.map.painter); + if (tile.hasRTLText) { + const plugin = rtlTextPlugin; + if (!plugin.isLoading() && !plugin.isLoaded() && getRTLTextPluginStatus() === 'deferred') { + downloadRTLTextPlugin(); + } + } } catch (err) { if (tile.aborted) { tile.state = 'unloaded'; diff --git a/src/source/worker_tile.js b/src/source/worker_tile.js index 3b2193a20..a58453c0d 100644 --- a/src/source/worker_tile.js +++ b/src/source/worker_tile.js @@ -98,6 +98,7 @@ async function finalizeBuckets(params, options, resources) { const buckets = new Map(); const { glyphAtlas, imageAtlas, glyphMap, iconMap } = await makeAtlasses(options, resources); let hasSymbolBuckets = false; + let hasRTLText = false; let queryPadding = 0; for (const bucket of uniqueBuckets.values()) { if (bucket instanceof SymbolBucket) { @@ -114,6 +115,7 @@ async function finalizeBuckets(params, options, resources) { if (justReloaded) { bucket.justReloaded = true; } + hasRTLText ||= bucket.hasRTLText; } else if ( bucket.hasPattern && (bucket instanceof LineBucket || bucket instanceof FillBucket || bucket instanceof FillExtrusionBucket) @@ -143,6 +145,7 @@ async function finalizeBuckets(params, options, resources) { glyphAtlasImage: glyphAtlas.image, imageAtlas, hasSymbolBuckets, + hasRTLText, queryPadding }; } diff --git a/src/style/style.js b/src/style/style.js index e5099854a..b45ebedce 100644 --- a/src/style/style.js +++ b/src/style/style.js @@ -64,7 +64,7 @@ class Style extends Evented { this._removedLayers = new Map(); this._resetUpdates(); - this._rtlTextPluginCallbackUnregister = plugin.registerForPluginAvailability(this._reloadSources.bind(this)); + this._rtlTextPluginCallbackUnregister = plugin.registerForPluginStateChange(this._reloadSources.bind(this)); this.on('data', event => { if (event.dataType !== 'source' || event.sourceDataType !== 'metadata') { @@ -1181,5 +1181,6 @@ class Style extends Evented { Style.getSourceType = getSourceType; Style.setSourceType = setSourceType; +Style.registerForPluginStateChange = plugin.registerForPluginStateChange; export default Style; diff --git a/src/util/script_detection.js b/src/util/script_detection.js index da6a7f17b..354d739ea 100644 --- a/src/util/script_detection.js +++ b/src/util/script_detection.js @@ -415,6 +415,15 @@ export function charHasRotatedVerticalOrientation(char) { return !(charHasUprightVerticalOrientation(char) || charHasNeutralVerticalOrientation(char)); } +export function charInRTLScript(char) { + // Main blocks for Hebrew, Arabic, Thaana and other RTL scripts + return ( + (char >= 0x0590 && char <= 0x08ff) || + isChar['Arabic Presentation Forms-A'](char) || + isChar['Arabic Presentation Forms-B'](char) + ); +} + export function charInSupportedScript(char, canRenderRTL) { // This is a rough heuristic: whether we "can render" a script // actually depends on the properties of the font being used @@ -423,13 +432,7 @@ export function charInSupportedScript(char, canRenderRTL) { // Even in Latin script, we "can't render" combinations such as the fi // ligature, but we don't consider that semantically significant. - if ( - !canRenderRTL && - ((char >= 0x0590 && char <= 0x08ff) || - isChar['Arabic Presentation Forms-A'](char) || - isChar['Arabic Presentation Forms-B'](char)) - ) { - // Main blocks for Hebrew, Arabic, Thaana and other RTL scripts + if (!canRenderRTL && charInRTLScript(char)) { return false; } if ( @@ -448,6 +451,15 @@ export function charInSupportedScript(char, canRenderRTL) { return true; } +export function stringContainsRTLText(chars) { + for (const char of chars) { + if (charInRTLScript(char.charCodeAt(0))) { + return true; + } + } + return false; +} + export function isStringInSupportedScript(chars, canRenderRTL) { for (const char of chars) { if (!charInSupportedScript(char.charCodeAt(0), canRenderRTL)) { diff --git a/test/unit/style/style.test.js b/test/unit/style/style.test.js index 6b53a5d11..4ebd52404 100644 --- a/test/unit/style/style.test.js +++ b/test/unit/style/style.test.js @@ -63,13 +63,13 @@ test('Style', async t => { style._remove(); }); - await t.test('registers plugin listener', t => { + await t.test('registers plugin state change listener', t => { plugin.clearRTLTextPlugin(); - t.mock.method(plugin, 'registerForPluginAvailability'); + t.mock.method(plugin, 'registerForPluginStateChange'); style = new Style(new StubMap()); - t.assert.equal(plugin.registerForPluginAvailability.mock.callCount(), 1); + t.assert.equal(plugin.registerForPluginStateChange.mock.callCount(), 1); t.mock.method(plugin, 'loadScript', () => Promise.reject()); plugin.setRTLTextPlugin('some-bogus-url'); From 9b25af227481baf4e9ceb3e11249ad575f3cad48 Mon Sep 17 00:00:00 2001 From: Arindam Bose Date: Mon, 9 Dec 2019 14:33:42 -0800 Subject: [PATCH 05/12] Remove `stringContainsRTLText` dependency from within style-spec. (#9088) Original commit: 2e53e7c5ef8654f4ab7162c25103abece00919dc: --- src/data/bucket/symbol_bucket.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index 17a1a0279..ed412e4f4 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -6,7 +6,7 @@ import EvaluationParameters from '../../style/evaluation_parameters.js'; import mergeLines from '../../symbol/mergelines.js'; import { getSizeData } from '../../symbol/symbol_size.js'; import transformText from '../../symbol/transform_text.js'; -import { allowsVerticalWritingMode } from '../../util/script_detection.js'; +import { allowsVerticalWritingMode, stringContainsRTLText } from '../../util/script_detection.js'; import { verticalizedCharacterMap } from '../../util/verticalize_punctuation.js'; import { CollisionBoxLayoutArray, @@ -45,6 +45,15 @@ export function addDynamicAttributes(dynamicLayoutVertexArray, p, angle) { dynamicLayoutVertexArray.emplaceBack(p.x, p.y, angle); } +function containsRTLText(formattedText) { + for (const section of formattedText.sections) { + if (stringContainsRTLText(section.text)) { + return true; + } + } + return false; +} + /** * Unlike other buckets, which simply implement #addFeature with type-specific * logic for (essentially) triangulating feature geometries, SymbolBucket @@ -173,6 +182,9 @@ export default class SymbolBucket { const resolvedTokens = layer.getValueAndResolveTokens('text-field', feature); const formattedText = resolvedTokens instanceof Formatted ? resolvedTokens : Formatted.fromString(resolvedTokens); + if (containsRTLText(formattedText)) { + this.hasRTLText = true; + } if ( !this.hasRTLText || // non-rtl text so can proceed safely getRTLTextPluginStatus() === 'unavailable' || // We don't intend to lazy-load the rtl text plugin, so proceed with incorrect shaping From ec2eabba6c59441f27f52463b386ca60ec7cb754 Mon Sep 17 00:00:00 2001 From: Arindam Bose Date: Mon, 16 Dec 2019 12:49:01 -0800 Subject: [PATCH 06/12] Deferred loading of RTL text plugin detects labels rendered from GeoJSON sources (#9091) create mode 100644 test/util/create_symbol_layer.js Original commit: d9e9ffe39a823b2f4a790b805b4fd2859c363cc4: --- src/data/bucket/symbol_bucket.js | 4 +- src/source/rtl_text_plugin.js | 6 +++ src/source/tile.js | 6 ++- src/source/vector_tile_source.js | 7 ---- test/unit/data/symbol_bucket.test.js | 57 +++++++++++++++++----------- test/unit/source/tile.test.js | 25 +++++++++++- test/util/create_symbol_layer.js | 21 ++++++++++ 7 files changed, 92 insertions(+), 34 deletions(-) create mode 100644 test/util/create_symbol_layer.js diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index ed412e4f4..08af99167 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -262,7 +262,9 @@ export default class SymbolBucket { } isEmpty() { - return this.symbolInstances.length === 0; + // When the bucket encounters only rtl-text but the plugin isnt loaded, no symbol instances will be created. + // In order for the bucket to be serialized, and not discarded as an empty bucket both checks are necessary. + return this.symbolInstances.length === 0 && !this.hasRTLText; } uploadPending() { diff --git a/src/source/rtl_text_plugin.js b/src/source/rtl_text_plugin.js index 38fa55827..c3bba3250 100644 --- a/src/source/rtl_text_plugin.js +++ b/src/source/rtl_text_plugin.js @@ -118,4 +118,10 @@ export const plugin = (rtlPlugin.plugin = { plugin.processStyledBidirectionalText != null }); +export function lazyLoadRTLTextPlugin() { + if (!plugin.isLoading() && !plugin.isLoaded() && getRTLTextPluginStatus() === 'deferred') { + downloadRTLTextPlugin(); + } +} + export default rtlPlugin; diff --git a/src/source/tile.js b/src/source/tile.js index 7d48eae07..b5195c130 100644 --- a/src/source/tile.js +++ b/src/source/tile.js @@ -11,6 +11,7 @@ import browser from '../util/browser.js'; import { deepEqual } from '../util/object.js'; import uniqueId from '../util/unique_id.js'; import GeoJSONFeature from '../util/vectortile_to_geojson.js'; +import { lazyLoadRTLTextPlugin } from './rtl_text_plugin.js'; /** * A tile object is the combination of a Coordinate, which defines @@ -95,9 +96,12 @@ class Tile { this.buckets = data.buckets; this.hasSymbolBuckets = data.hasSymbolBuckets; - this.hasRTLText = data.hasRTLText; this.queryPadding = data.queryPadding; + if (data.hasRTLText) { + this.hasRTLText = data.hasRTLText; + lazyLoadRTLTextPlugin(); + } if (data.imageAtlas) { this.imageAtlas = data.imageAtlas; } diff --git a/src/source/vector_tile_source.js b/src/source/vector_tile_source.js index 2ac9d062d..a8079a719 100644 --- a/src/source/vector_tile_source.js +++ b/src/source/vector_tile_source.js @@ -1,7 +1,6 @@ import { ErrorEvent, Event, Evented } from '@mapwhit/events'; import browser from '../util/browser.js'; import loadTileJSON from './load_tilejson.js'; -import { downloadRTLTextPlugin, getRTLTextPluginStatus, plugin as rtlTextPlugin } from './rtl_text_plugin.js'; import TileBounds from './tile_bounds.js'; import VectorTileWorkerSource from './vector_tile_worker_source.js'; @@ -104,12 +103,6 @@ class VectorTileSource extends Evented { data.rawTileData = rawData; } tile.loadVectorData(data, this.map.painter); - if (tile.hasRTLText) { - const plugin = rtlTextPlugin; - if (!plugin.isLoading() && !plugin.isLoaded() && getRTLTextPluginStatus() === 'deferred') { - downloadRTLTextPlugin(); - } - } } catch (err) { if (tile.aborted) { tile.state = 'unloaded'; diff --git a/test/unit/data/symbol_bucket.test.js b/test/unit/data/symbol_bucket.test.js index 9bfcd103a..9a3645904 100644 --- a/test/unit/data/symbol_bucket.test.js +++ b/test/unit/data/symbol_bucket.test.js @@ -5,11 +5,11 @@ import FeatureIndex from '../../../src/data/feature_index.js'; import Transform from '../../../src/geo/transform.js'; import Tile from '../../../src/source/tile.js'; import { OverscaledTileID } from '../../../src/source/tile_id.js'; -import SymbolStyleLayer from '../../../src/style/style_layer/symbol_style_layer.js'; import CrossTileSymbolIndex from '../../../src/symbol/cross_tile_symbol_index.js'; import { Placement } from '../../../src/symbol/placement.js'; import { performSymbolLayout } from '../../../src/symbol/symbol_layout.js'; import glyphs from '../../fixtures/fontstack-glyphs.json' with { type: 'json' }; +import { createSymbolBucket } from '../../util/create_symbol_layer.js'; import { createPopulateOptions, loadVectorTile } from '../../util/tile.js'; const collisionBoxArray = new CollisionBoxArray(); @@ -20,25 +20,6 @@ transform.cameraToCenterDistance = 100; const stacks = { Test: glyphs }; -function createSymbolBucket(globalState) { - const layer = new SymbolStyleLayer( - { - id: 'test', - type: 'symbol', - layout: { 'text-font': ['Test'], 'text-field': 'abcde' } - }, - globalState - ); - layer.recalculate({ zoom: 0, zoomHistory: {} }); - - return new SymbolBucket({ - overscaling: 1, - zoom: 0, - collisionBoxArray, - layers: [layer] - }); -} - test('SymbolBucket', async t => { let features; t.before(() => { @@ -48,8 +29,8 @@ test('SymbolBucket', async t => { }); await t.test('SymbolBucket', t => { - const bucketA = createSymbolBucket(); - const bucketB = createSymbolBucket(); + const bucketA = createSymbolBucket(collisionBoxArray); + const bucketB = createSymbolBucket(collisionBoxArray); const options = createPopulateOptions(); const placement = new Placement(transform, 0, true); const tileID = new OverscaledTileID(0, 0, 0, 0, 0); @@ -101,7 +82,7 @@ test('SymbolBucket', async t => { }); const warn = t.mock.method(console, 'warn'); - const bucket = createSymbolBucket(); + const bucket = createSymbolBucket(collisionBoxArray); bucket.populate(features, createPopulateOptions()); const fakeGlyph = { rect: { w: 10, h: 10 }, metrics: { left: 10, top: 10, advance: 10 } }; @@ -112,4 +93,34 @@ test('SymbolBucket', async t => { t.assert.equal(warn.mock.callCount(), 1); t.assert.match(warn.mock.calls[0].arguments[0], /Too many glyphs being rendered in a tile./); }); + + await t.test('SymbolBucket detects rtl text', t => { + const rtlBucket = createSymbolBucket(collisionBoxArray, 'مرحبا'); + const ltrBucket = createSymbolBucket(collisionBoxArray, 'hello'); + const options = { iconDependencies: {}, glyphDependencies: {} }; + rtlBucket.populate(features, options); + ltrBucket.populate(features, options); + + t.assert.ok(rtlBucket.hasRTLText); + t.assert.ok(!ltrBucket.hasRTLText); + }); + + // Test to prevent symbol bucket with rtl from text being culled by worker serialization. + await t.test('SymbolBucket with rtl text is NOT empty even though no symbol instances are created', t => { + const rtlBucket = createSymbolBucket(collisionBoxArray, 'مرحبا'); + const options = { iconDependencies: {}, glyphDependencies: {} }; + rtlBucket.createArrays(); + rtlBucket.populate(features, options); + + t.assert.ok(!rtlBucket.isEmpty()); + t.assert.equal(rtlBucket.symbolInstances.length, 0); + }); + + await t.test('SymbolBucket detects rtl text mixed with ltr text', t => { + const mixedBucket = createSymbolBucket(collisionBoxArray, 'مرحبا translates to hello'); + const options = { iconDependencies: {}, glyphDependencies: {} }; + mixedBucket.populate(features, options); + + t.assert.ok(mixedBucket.hasRTLText); + }); }); diff --git a/test/unit/source/tile.test.js b/test/unit/source/tile.test.js index efecc55f6..2d29feaa2 100644 --- a/test/unit/source/tile.test.js +++ b/test/unit/source/tile.test.js @@ -6,6 +6,7 @@ import Context from '../../../src/gl/context.js'; import GeoJSONWrapper from '../../../src/source/geojson_wrapper.js'; import Tile from '../../../src/source/tile.js'; import { OverscaledTileID } from '../../../src/source/tile_id.js'; +import { createSymbolBucket } from '../../util/create_symbol_layer.js'; import { loadVectorTile } from '../../util/tile.js'; import _window from '../../util/window.js'; @@ -228,6 +229,26 @@ test('Tile', async t => { }); }); +test('rtl text detection', async t => { + await t.test('Tile#hasRTLText is true when a tile loads a symbol bucket with rtl text', t => { + const tile = new Tile(new OverscaledTileID(1, 0, 1, 1, 1)); + // Create a stub symbol bucket + const symbolBucket = createSymbolBucket(new CollisionBoxArray()); + // symbolBucket has not been populated yet so we force override the value in the stub + symbolBucket.hasRTLText = true; + tile.loadVectorData( + createVectorData({ vectorTile: loadVectorTile(), buckets: [symbolBucket], hasRTLText: true }), + createPainter({ + getLayer() { + return symbolBucket.layers[0]; + } + }) + ); + + t.assert.ok(tile.hasRTLText); + }); +}); + function createVectorData(options) { const collisionBoxArray = new CollisionBoxArray(); return Object.assign( @@ -240,6 +261,6 @@ function createVectorData(options) { ); } -function createPainter() { - return { style: {} }; +function createPainter(styleStub = {}) { + return { style: styleStub }; } diff --git a/test/util/create_symbol_layer.js b/test/util/create_symbol_layer.js new file mode 100644 index 000000000..74f8e9e7d --- /dev/null +++ b/test/util/create_symbol_layer.js @@ -0,0 +1,21 @@ +import SymbolBucket from '../../src/data/bucket/symbol_bucket.js'; +import SymbolStyleLayer from '../../src/style/style_layer/symbol_style_layer.js'; + +export function createSymbolBucket(collisionBoxArray, text = 'abcde', globalState) { + const layer = new SymbolStyleLayer( + { + id: 'test', + type: 'symbol', + layout: { 'text-font': ['Test'], 'text-field': text } + }, + globalState + ); + layer.recalculate({ zoom: 0, zoomHistory: {} }); + + return new SymbolBucket({ + overscaling: 1, + zoom: 0, + collisionBoxArray, + layers: [layer] + }); +} From ab63b6bd7fac016825180273d1666fc14d046bd8 Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Fri, 14 Apr 2023 20:28:34 -0700 Subject: [PATCH 07/12] Don't reload raster sources when RTL text plugin loads to avoid flicker. (#2381) * Don't reload raster sources when RTL text plugin loads to avoid flicker. Fixes issue https://github.com/maplibre/maplibre-gl-js/issue/2380 * Test fixes: - Make clearRTLTextPlugin also clear the _completionCallback - Having a completion callback for an old plugin run is bad in general - It specifically messed up the style tests by running during later tests - Call clearRTLTextPlugin when last plugin test is done - Set the stubMap style object -- this is a subtle one, but two frames _after_ this test runs, loadTileJSON triggered by this loadJSON will fail if the map doesn't have its style set, and the exception will get passed to a later style test. * Add changelog entry. Original commit: 3df594864bdf3f8713eafa6b573251dda9d89061: --- src/source/rtl_text_plugin.js | 1 + src/style/style.js | 14 +++++++++--- test/unit/style/style.test.js | 43 +++++++++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/source/rtl_text_plugin.js b/src/source/rtl_text_plugin.js index c3bba3250..401e04eb8 100644 --- a/src/source/rtl_text_plugin.js +++ b/src/source/rtl_text_plugin.js @@ -42,6 +42,7 @@ export function clearRTLTextPlugin() { _loadedCallbacks.length = 0; pluginStatus = status.unavailable; pluginURL = undefined; + _completionCallback = null; } export function setRTLTextPlugin(url, callback, deferred = false) { diff --git a/src/style/style.js b/src/style/style.js index b45ebedce..a0242b302 100644 --- a/src/style/style.js +++ b/src/style/style.js @@ -64,7 +64,12 @@ class Style extends Evented { this._removedLayers = new Map(); this._resetUpdates(); - this._rtlTextPluginCallbackUnregister = plugin.registerForPluginStateChange(this._reloadSources.bind(this)); + // Non-vector sources don't have any symbols buckets to reload when the RTL text plugin loads + // They also load more quickly, so they're more likely to have already displaying tiles + // that would be unnecessarily booted by the plugin load event + this._rtlTextPluginCallbackUnregister = plugin.registerForPluginStateChange( + this._reloadSources.bind(this, new Set(['vector', 'geojson'])) + ); this.on('data', event => { if (event.dataType !== 'source' || event.sourceDataType !== 'metadata') { @@ -1064,9 +1069,12 @@ class Style extends Evented { } } - _reloadSources() { + _reloadSources(types) { for (const sourceCache of Object.values(this._sources)) { - sourceCache.reload(); // Should be a no-op if called before any tiles load + const { type } = sourceCache.getSource(); + if (!types || types.has(type)) { + sourceCache.reload(); // Should be a no-op if the plugin loads before any tiles load + } } } diff --git a/test/unit/style/style.test.js b/test/unit/style/style.test.js index 4ebd52404..101524fe4 100644 --- a/test/unit/style/style.test.js +++ b/test/unit/style/style.test.js @@ -86,6 +86,49 @@ test('Style', async t => { }); style = new Style(createStyleJSON()); }); + + await t.test('RTL plugin load reloads vector source but not raster source', (t, done) => { + plugin.clearRTLTextPlugin(); + t.mock.method(plugin, 'loadScript', () => { + globalThis.registerRTLTextPlugin({}); + return Promise.resolve(); + }); + style = new Style(new StubMap()); + style.loadJSON( + createStyleJSON({ + sources: { + raster: { + type: 'raster', + tiles: ['http://tiles.server'] + }, + vector: { + type: 'vector', + tiles: ['http://tiles.server'] + } + }, + layers: [ + { + id: 'raster', + type: 'raster', + source: 'raster' + } + ] + }) + ); + style.on('style.load', () => { + t.mock.method(style._sources.raster, 'reload'); + t.mock.method(style._sources.vector, 'reload'); + plugin.setRTLTextPlugin('some-bogus-url', error => { + t.assert.ok(!error); + setTimeout(() => { + plugin.clearRTLTextPlugin(); + t.assert.equal(style._sources.raster.reload.mock.callCount(), 1); + t.assert.equal(style._sources.vector.reload.mock.callCount(), 2); + done(); + }, 0); + }); + }); + }); }); await t.test('Style.loadJSON', async t => { From ed89ac26005394c2cce9a7b50e0071d513737d17 Mon Sep 17 00:00:00 2001 From: Harel M Date: Tue, 28 Nov 2023 23:56:11 +0200 Subject: [PATCH 08/12] Refactor RTL plugin loading code (#3418) * Initial commit to split rtl to worker and main thread implementations. * Finish all fixes for rtl plugin split * Fix the tests that fail, add proper idle event handling * Changelog, lint, build and render * Improve render test * Fix changelog, add idle console logging * Return a promise from the public API * Add more logs to tests * More logs, more timeout * Fix lint * Clean error for failed tests * Fix typo delete mode 100644 src/source/rtl_text_plugin.ts create mode 100644 src/source/rtl_text_plugin_main_thread.test.ts create mode 100644 src/source/rtl_text_plugin_main_thread.ts create mode 100644 src/source/rtl_text_plugin_status.ts create mode 100644 src/source/rtl_text_plugin_worker.ts Original commit: d68b919b04864d1c11491ddbc75d3f16dc2838e0: --- src/data/bucket/symbol_bucket.js | 6 +- src/index.js | 10 +- src/source/rtl_text_plugin.js | 128 ------------------ src/source/rtl_text_plugin_main_thread.js | 101 ++++++++++++++ src/source/rtl_text_plugin_worker.js | 31 +++++ src/source/tile.js | 4 +- src/style/evaluation_parameters.js | 5 +- src/style/style.js | 37 +++-- src/symbol/shaping.js | 4 +- src/symbol/transform_text.js | 7 +- .../rtl_text_plugin_main_thread.test.js | 91 +++++++++++++ test/unit/style/style.test.js | 67 ++------- test/util/util.js | 9 ++ test/util/window.js | 2 +- 14 files changed, 282 insertions(+), 220 deletions(-) delete mode 100644 src/source/rtl_text_plugin.js create mode 100644 src/source/rtl_text_plugin_main_thread.js create mode 100644 src/source/rtl_text_plugin_worker.js create mode 100644 test/unit/source/rtl_text_plugin_main_thread.test.js diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index 08af99167..57b6a9017 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -1,7 +1,7 @@ import { dist } from '@mapwhit/point-geometry'; import { Formatted } from '@mapwhit/style-expressions'; import { VectorTileFeature } from '@mapwhit/vector-tile'; -import { getRTLTextPluginStatus, plugin as globalRTLTextPlugin } from '../../source/rtl_text_plugin.js'; +import { rtlWorkerPlugin } from '../../source/rtl_text_plugin_worker.js'; import EvaluationParameters from '../../style/evaluation_parameters.js'; import mergeLines from '../../symbol/mergelines.js'; import { getSizeData } from '../../symbol/symbol_size.js'; @@ -187,8 +187,8 @@ export default class SymbolBucket { } if ( !this.hasRTLText || // non-rtl text so can proceed safely - getRTLTextPluginStatus() === 'unavailable' || // We don't intend to lazy-load the rtl text plugin, so proceed with incorrect shaping - globalRTLTextPlugin.isParsed() // Use the rtlText plugin to shape text + rtlWorkerPlugin.getRTLTextPluginStatus() === 'unavailable' || // We don't intend to lazy-load the rtl text plugin, so proceed with incorrect shaping + rtlWorkerPlugin.isParsed() // Use the rtlText plugin to shape text ) { text = transformText(formattedText, layer, feature); } diff --git a/src/index.js b/src/index.js index 0e962b484..5840626d5 100644 --- a/src/index.js +++ b/src/index.js @@ -5,7 +5,7 @@ import { Point } from '@mapwhit/point-geometry'; import packageJSON from '../package.json' with { type: 'json' }; import { default as LngLat } from './geo/lng_lat.js'; import { default as LngLatBounds } from './geo/lng_lat_bounds.js'; -import { getRTLTextPluginStatus, setRTLTextPlugin } from './source/rtl_text_plugin.js'; +import { rtlMainThreadPluginFactory } from './source/rtl_text_plugin_main_thread.js'; import { default as Style } from './style/style.js'; import { default as Map } from './ui/map.js'; import config from './util/config.js'; @@ -48,14 +48,15 @@ export default mapwhit; * * @function setRTLTextPlugin * @param {string} pluginURL URL pointing to the Mapbox RTL text plugin source. - * @param {Function} callback Called with an error argument if there is an error. * @param {boolean} lazy If set to `true`, mapboxgl will defer loading the plugin until rtl text is encountered, * rtl text will then be rendered only after the plugin finishes loading. * @example * mapboxgl.setRTLTextPlugin('https://api.mapbox.com/mapbox-gl-js/plugins/mapbox-gl-rtl-text/v0.2.0/mapbox-gl-rtl-text.js'); * @see [Add support for right-to-left scripts](https://www.mapbox.com/mapbox-gl-js/example/mapbox-gl-rtl-text/) */ - +function setRTLTextPlugin(pluginURL, lazy) { + return rtlMainThreadPluginFactory().setRTLTextPlugin(pluginURL, lazy); +} /** * Gets the map's [RTL text plugin](https://www.mapbox.com/mapbox-gl-js/plugins/#mapbox-gl-rtl-text) status. * The status can be `unavailable` (i.e. not requested or removed), `loading`, `loaded` or `error`. @@ -65,6 +66,9 @@ export default mapwhit; * @example * const pluginStatus = mapboxgl.getRTLTextPluginStatus(); */ +function getRTLTextPluginStatus() { + return rtlMainThreadPluginFactory().getRTLTextPluginStatus(); +} // canary assert: used to confirm that asserts have been removed from production build import assert from 'assert'; diff --git a/src/source/rtl_text_plugin.js b/src/source/rtl_text_plugin.js deleted file mode 100644 index 401e04eb8..000000000 --- a/src/source/rtl_text_plugin.js +++ /dev/null @@ -1,128 +0,0 @@ -import dynload from 'dynload'; -import browser from '../util/browser.js'; - -const status = { - unavailable: 'unavailable', // Not loaded - deferred: 'deferred', // The plugin URL has been specified, but loading has been deferred - loading: 'loading', // request in-flight - loaded: 'loaded', - error: 'error' -}; - -let _completionCallback; - -//Variables defining the current state of the plugin -let pluginStatus = status.unavailable; -let pluginURL; - -const _loadedCallbacks = []; - -const rtlPlugin = { - clearRTLTextPlugin, // exported for testing - loadScript, // exported for testing - registerForPluginStateChange, - setRTLTextPlugin -}; - -export function getRTLTextPluginStatus() { - return pluginStatus; -} - -export function registerForPluginStateChange(callback) { - if (plugin.isLoaded()) { - callback(); - return; - } - _loadedCallbacks.push(callback); - downloadRTLTextPlugin(); - return () => _loadedCallbacks.splice(_loadedCallbacks.indexOf(callback), 1); -} - -export function clearRTLTextPlugin() { - _loadedCallbacks.length = 0; - pluginStatus = status.unavailable; - pluginURL = undefined; - _completionCallback = null; -} - -export function setRTLTextPlugin(url, callback, deferred = false) { - if (pluginStatus === status.deferred || pluginStatus === status.loading || pluginStatus === status.loaded) { - throw new Error('setRTLTextPlugin cannot be called multiple times.'); - } - pluginURL = browser.resolveURL(url); - pluginStatus = status.deferred; - _completionCallback = error => { - if (error) { - const msg = `RTL Text Plugin failed to load scripts from ${pluginURL}`; - // Clear loaded state to allow retries - clearRTLTextPlugin(); - pluginStatus = status.error; - error = new Error(msg); - } else { - pluginStatus = status.loaded; - } - if (callback) { - callback(error); - } - _completionCallback = undefined; - }; - //Start downloading the plugin immediately if not intending to lazy-load - if (!deferred) { - downloadRTLTextPlugin(); - } -} - -export function downloadRTLTextPlugin() { - if (pluginURL && !plugin.isLoading() && !plugin.isLoaded() && _loadedCallbacks.length > 0) { - pluginStatus = status.loading; - // needs to be called as exported method for mock testing - rtlPlugin.loadScript(pluginURL).then(_completionCallback).catch(_completionCallback); - } -} - -function registerRTLTextPlugin(loadedPlugin) { - if (plugin.isLoaded()) { - throw new Error('RTL text plugin already registered.'); - } - plugin['applyArabicShaping'] = loadedPlugin.applyArabicShaping; - plugin['processBidirectionalText'] = loadedPlugin.processBidirectionalText; - plugin['processStyledBidirectionalText'] = loadedPlugin.processStyledBidirectionalText; - - if (_completionCallback) { - _completionCallback(); - } - _loadedCallbacks.forEach(callback => callback()); - _loadedCallbacks.length = 0; -} - -globalThis.registerRTLTextPlugin ??= registerRTLTextPlugin; - -function loadScript(url) { - const { promise, resolve, reject } = Promise.withResolvers(); - const s = dynload(url); - s.onload = () => resolve(); - s.onerror = () => reject(true); - return promise; -} - -export const plugin = (rtlPlugin.plugin = { - applyArabicShaping: null, - processBidirectionalText: null, - processStyledBidirectionalText: null, - // loaded if the completion callback returned successfully or the plugin functions have been compiled - isLoaded: () => pluginStatus === status.loaded || plugin.applyArabicShaping != null, - // query the loading status - isLoading: () => pluginStatus === status.loading, - isParsed: () => - plugin.applyArabicShaping != null && - plugin.processBidirectionalText != null && - plugin.processStyledBidirectionalText != null -}); - -export function lazyLoadRTLTextPlugin() { - if (!plugin.isLoading() && !plugin.isLoaded() && getRTLTextPluginStatus() === 'deferred') { - downloadRTLTextPlugin(); - } -} - -export default rtlPlugin; diff --git a/src/source/rtl_text_plugin_main_thread.js b/src/source/rtl_text_plugin_main_thread.js new file mode 100644 index 000000000..850a883b6 --- /dev/null +++ b/src/source/rtl_text_plugin_main_thread.js @@ -0,0 +1,101 @@ +import { Event, Evented } from '@mapwhit/events'; +import dynload from 'dynload'; +import browser from '../util/browser.js'; +import { rtlWorkerPlugin } from './rtl_text_plugin_worker.js'; + +/** + * The possible option of the plugin's status + * + * `unavailable`: Not loaded. + * + * `deferred`: The plugin URL has been specified, but loading has been deferred. + * + * `loading`: request in-flight. + * + * `loaded`: The plugin is now loaded + * + * `error`: The plugin failed to load + */ + +class RTLMainThreadPlugin extends Evented { + pluginStatus = 'unavailable'; + pluginURL = null; + queue = []; + _sendPluginStateToWorker() { + syncRTLPluginState({ + pluginStatus: this.pluginStatus, + pluginURL: this.pluginURL + }); + this.fire(new Event('pluginStateChange', { pluginStatus: this.pluginStatus, pluginURL: this.pluginURL })); + } + getRTLTextPluginStatus() { + return this.pluginStatus; + } + // public for testing + clearRTLTextPlugin() { + this.pluginStatus = 'unavailable'; + this.pluginURL = null; + } + // public for testing + loadScript({ url }) { + const { promise, resolve, reject } = Promise.withResolvers(); + const s = dynload(url); + s.onload = () => resolve(); + s.onerror = () => reject(true); + return promise; + } + async setRTLTextPlugin(url, deferred = false) { + if (this.pluginStatus === 'deferred' || this.pluginStatus === 'loading' || this.pluginStatus === 'loaded') { + throw new Error('setRTLTextPlugin cannot be called multiple times.'); + } + this.pluginURL = browser.resolveURL(url); + this.pluginStatus = 'deferred'; + this._sendPluginStateToWorker(); + if (!deferred) { + //Start downloading the plugin immediately if not intending to lazy-load + await this._downloadRTLTextPlugin(); + } + } + async _downloadRTLTextPlugin() { + if (this.pluginStatus !== 'deferred' || !this.pluginURL) { + throw new Error('rtl-text-plugin cannot be downloaded unless a pluginURL is specified'); + } + try { + this.pluginStatus = 'loading'; + this._sendPluginStateToWorker(); + await this.loadScript({ url: this.pluginURL }); + } catch { + this.pluginStatus = 'error'; + this._sendPluginStateToWorker(); + } + } + async lazyLoadRTLTextPlugin() { + if (this.pluginStatus === 'deferred') { + await this._downloadRTLTextPlugin(); + } + } +} +let rtlMainThreadPlugin = null; +export function rtlMainThreadPluginFactory() { + if (!rtlMainThreadPlugin) { + rtlMainThreadPlugin = new RTLMainThreadPlugin(); + } + return rtlMainThreadPlugin; +} + +function syncRTLPluginState(state) { + rtlWorkerPlugin.setState(state); +} + +// This is invoked by the RTL text plugin when the download via the `importScripts` call has finished, and the code has been parsed. +function registerRTLTextPlugin(rtlTextPlugin) { + if (rtlWorkerPlugin.isParsed()) { + throw new Error('RTL text plugin already registered.'); + } + const rtlMainThreadPlugin = rtlMainThreadPluginFactory(); + rtlMainThreadPlugin.pluginStatus = 'loaded'; + rtlMainThreadPlugin._sendPluginStateToWorker(); + rtlWorkerPlugin.setMethods(rtlTextPlugin); +} + +globalThis.registerRTLTextPlugin ??= registerRTLTextPlugin; diff --git a/src/source/rtl_text_plugin_worker.js b/src/source/rtl_text_plugin_worker.js new file mode 100644 index 000000000..ed94a5d23 --- /dev/null +++ b/src/source/rtl_text_plugin_worker.js @@ -0,0 +1,31 @@ +class RTLWorkerPlugin { + applyArabicShaping = null; + processBidirectionalText = null; + processStyledBidirectionalText = null; + pluginStatus = 'unavailable'; + pluginURL = null; + setState(state) { + this.pluginStatus = state.pluginStatus; + this.pluginURL = state.pluginURL; + } + setMethods(rtlTextPlugin) { + this.applyArabicShaping = rtlTextPlugin.applyArabicShaping; + this.processBidirectionalText = rtlTextPlugin.processBidirectionalText; + this.processStyledBidirectionalText = rtlTextPlugin.processStyledBidirectionalText; + } + isParsed() { + return ( + this.applyArabicShaping != null && + this.processBidirectionalText != null && + this.processStyledBidirectionalText != null + ); + } + getPluginURL() { + return this.pluginURL; + } + getRTLTextPluginStatus() { + return this.pluginStatus; + } +} + +export const rtlWorkerPlugin = new RTLWorkerPlugin(); diff --git a/src/source/tile.js b/src/source/tile.js index b5195c130..d74ff7b09 100644 --- a/src/source/tile.js +++ b/src/source/tile.js @@ -11,7 +11,7 @@ import browser from '../util/browser.js'; import { deepEqual } from '../util/object.js'; import uniqueId from '../util/unique_id.js'; import GeoJSONFeature from '../util/vectortile_to_geojson.js'; -import { lazyLoadRTLTextPlugin } from './rtl_text_plugin.js'; +import { rtlMainThreadPluginFactory } from './rtl_text_plugin_main_thread.js'; /** * A tile object is the combination of a Coordinate, which defines @@ -100,7 +100,7 @@ class Tile { if (data.hasRTLText) { this.hasRTLText = data.hasRTLText; - lazyLoadRTLTextPlugin(); + rtlMainThreadPluginFactory().lazyLoadRTLTextPlugin(); } if (data.imageAtlas) { this.imageAtlas = data.imageAtlas; diff --git a/src/style/evaluation_parameters.js b/src/style/evaluation_parameters.js index 03b2529ad..397849a91 100644 --- a/src/style/evaluation_parameters.js +++ b/src/style/evaluation_parameters.js @@ -1,4 +1,4 @@ -import { plugin as rtlTextPlugin } from '../source/rtl_text_plugin.js'; +import { rtlWorkerPlugin } from '../source/rtl_text_plugin_worker.js'; import { isStringInSupportedScript } from '../util/script_detection.js'; import ZoomHistory from './zoom_history.js'; @@ -20,7 +20,6 @@ export default class EvaluationParameters { this.transition = {}; } } - crossFadingFactor() { if (this.fadeDuration === 0) { return 1; @@ -40,5 +39,5 @@ export default class EvaluationParameters { } function isSupportedScript(str) { - return isStringInSupportedScript(str, rtlTextPlugin.isLoaded()); + return isStringInSupportedScript(str, rtlWorkerPlugin.getRTLTextPluginStatus() === 'loaded'); } diff --git a/src/style/style.js b/src/style/style.js index a0242b302..0735606fe 100644 --- a/src/style/style.js +++ b/src/style/style.js @@ -5,7 +5,7 @@ import ImageManager from '../render/image_manager.js'; import LineAtlas from '../render/line_atlas.js'; import { queryRenderedFeatures, queryRenderedSymbols, querySourceFeatures } from '../source/query_features.js'; import { resources } from '../source/resources/index.js'; -import plugin from '../source/rtl_text_plugin.js'; +import { rtlMainThreadPluginFactory } from '../source/rtl_text_plugin_main_thread.js'; import { getType as getSourceType, setType as setSourceType } from '../source/source.js'; import SourceCache from '../source/source_cache.js'; import CrossTileSymbolIndex from '../symbol/cross_tile_symbol_index.js'; @@ -42,6 +42,7 @@ class Style extends Evented { }); #layerIndex = new StyleLayerIndex(); #opsQueue = []; + #pluginStateChangeHandler; constructor(map, options = {}) { super(); @@ -63,14 +64,8 @@ class Style extends Evented { this._updatedLayers = new Map(); this._removedLayers = new Map(); this._resetUpdates(); - - // Non-vector sources don't have any symbols buckets to reload when the RTL text plugin loads - // They also load more quickly, so they're more likely to have already displaying tiles - // that would be unnecessarily booted by the plugin load event - this._rtlTextPluginCallbackUnregister = plugin.registerForPluginStateChange( - this._reloadSources.bind(this, new Set(['vector', 'geojson'])) - ); - + this.#pluginStateChangeHandler = this.#rtlTextPluginStateChange.bind(this); + rtlMainThreadPluginFactory().on('pluginStateChange', this.#pluginStateChangeHandler); this.on('data', event => { if (event.dataType !== 'source' || event.sourceDataType !== 'metadata') { return; @@ -94,6 +89,18 @@ class Style extends Evented { }); } + #rtlTextPluginStateChange() { + for (const sourceCache of Object.values(this._sources)) { + const { type } = sourceCache.getSource(); + if (type === 'vector' || type === 'geojson') { + // Non-vector sources don't have any symbols buckets to reload when the RTL text plugin loads + // They also load more quickly, so they're more likely to have already displaying tiles + // that would be unnecessarily booted by the plugin load event + sourceCache.reload(); // Should be a no-op if the plugin loads before any tiles load + } + } + } + setGlobalStateProperty(name, value) { if (!this._loaded) { this.#opsQueue.push(() => this.setGlobalStateProperty(name, value)); @@ -1048,7 +1055,7 @@ class Style extends Evented { } _remove() { - this._rtlTextPluginCallbackUnregister?.(); + rtlMainThreadPluginFactory().off('pluginStateChange', this.#pluginStateChangeHandler); for (const id in this._sources) { this._sources[id].clearTiles(); } @@ -1069,15 +1076,6 @@ class Style extends Evented { } } - _reloadSources(types) { - for (const sourceCache of Object.values(this._sources)) { - const { type } = sourceCache.getSource(); - if (!types || types.has(type)) { - sourceCache.reload(); // Should be a no-op if the plugin loads before any tiles load - } - } - } - _generateCollisionBoxes() { for (const id in this._sources) { this._reloadSource(id); @@ -1189,6 +1187,5 @@ class Style extends Evented { Style.getSourceType = getSourceType; Style.setSourceType = setSourceType; -Style.registerForPluginStateChange = plugin.registerForPluginStateChange; export default Style; diff --git a/src/symbol/shaping.js b/src/symbol/shaping.js index b9349875f..cd797e0db 100644 --- a/src/symbol/shaping.js +++ b/src/symbol/shaping.js @@ -1,4 +1,4 @@ -import { plugin as rtlTextPlugin } from '../source/rtl_text_plugin.js'; +import { rtlWorkerPlugin } from '../source/rtl_text_plugin_worker.js'; import { charAllowsIdeographicBreaking, charHasUprightVerticalOrientation } from '../util/script_detection.js'; import verticalizePunctuation from '../util/verticalize_punctuation.js'; import ONE_EM from './one_em.js'; @@ -112,7 +112,7 @@ export function shapeText( let lines; - const { processBidirectionalText, processStyledBidirectionalText } = rtlTextPlugin; + const { processBidirectionalText, processStyledBidirectionalText } = rtlWorkerPlugin; if (processBidirectionalText && logicalInput.sections.length === 1) { // Bidi doesn't have to be style-aware lines = []; diff --git a/src/symbol/transform_text.js b/src/symbol/transform_text.js index 79261fdaf..3cecb6a4b 100644 --- a/src/symbol/transform_text.js +++ b/src/symbol/transform_text.js @@ -1,4 +1,4 @@ -import { plugin as rtlTextPlugin } from '../source/rtl_text_plugin.js'; +import { rtlWorkerPlugin } from '../source/rtl_text_plugin_worker.js'; function transformText(text, layer, feature) { const transform = layer._layout.get('text-transform').evaluate(feature, {}); @@ -7,9 +7,8 @@ function transformText(text, layer, feature) { } else if (transform === 'lowercase') { text = text.toLocaleLowerCase(); } - - if (rtlTextPlugin.applyArabicShaping) { - text = rtlTextPlugin.applyArabicShaping(text); + if (rtlWorkerPlugin.applyArabicShaping) { + text = rtlWorkerPlugin.applyArabicShaping(text); } return text; diff --git a/test/unit/source/rtl_text_plugin_main_thread.test.js b/test/unit/source/rtl_text_plugin_main_thread.test.js new file mode 100644 index 000000000..579f7506d --- /dev/null +++ b/test/unit/source/rtl_text_plugin_main_thread.test.js @@ -0,0 +1,91 @@ +import test from 'node:test'; +import { rtlMainThreadPluginFactory } from '../../../src/source/rtl_text_plugin_main_thread.js'; +import browser from '../../../src/util/browser.js'; +import { sleep } from '../../util/util.js'; +import _window from '../../util/window.js'; + +const rtlMainThreadPlugin = rtlMainThreadPluginFactory(); +test('RTLMainThreadPlugin', async t => { + let globalWindow; + t.before(() => { + globalWindow = globalThis.window; + globalThis.window = _window; + }); + t.beforeEach(() => { + // Reset the singleton instance before each test + rtlMainThreadPlugin.clearRTLTextPlugin(); + }); + t.afterEach(() => { + t.mock.reset(); + }); + t.after(() => { + globalThis.window = globalWindow; + }); + + await t.test('should get the RTL text plugin status', t => { + const status = rtlMainThreadPlugin.getRTLTextPluginStatus(); + t.assert.equal(status, 'unavailable'); + }); + + await t.test('should set the RTL text plugin and download it', async t => { + const url = 'http://example.com/plugin'; + t.mock.method(rtlMainThreadPlugin, 'loadScript', () => { + globalThis.registerRTLTextPlugin({}); + return Promise.resolve(); + }); + const promise = rtlMainThreadPlugin.setRTLTextPlugin(url); + await sleep(0); + await promise; + t.assert.equal(rtlMainThreadPlugin.pluginURL, url); + t.assert.equal(rtlMainThreadPlugin.loadScript.mock.callCount(), 1); + }); + + await t.test('should set the RTL text plugin but deffer downloading', async t => { + const url = 'http://example.com/plugin'; + t.mock.method(rtlMainThreadPlugin, 'loadScript'); + await rtlMainThreadPlugin.setRTLTextPlugin(url, true); + t.assert.equal(rtlMainThreadPlugin.loadScript.mock.callCount(), 0); + t.assert.equal(rtlMainThreadPlugin.pluginStatus, 'deferred'); + }); + + await t.test('should throw if the plugin is already set', async t => { + const url = 'http://example.com/plugin'; + await rtlMainThreadPlugin.setRTLTextPlugin(url, true); + await t.assert.rejects(rtlMainThreadPlugin.setRTLTextPlugin(url), { + message: 'setRTLTextPlugin cannot be called multiple times.' + }); + }); + + await t.test('should throw if the plugin url is not set', async t => { + t.mock.method(browser, 'resolveURL', () => ''); + await t.assert.rejects(rtlMainThreadPlugin.setRTLTextPlugin(null), { + message: 'rtl-text-plugin cannot be downloaded unless a pluginURL is specified' + }); + }); + + await t.test('should be in error state if download fails', async t => { + const url = 'http://example.com/plugin'; + t.mock.method(rtlMainThreadPlugin, 'loadScript', () => Promise.reject()); + const promise = rtlMainThreadPlugin.setRTLTextPlugin(url); + await sleep(0); + await promise; + t.assert.equal(rtlMainThreadPlugin.pluginURL, url); + t.assert.equal(rtlMainThreadPlugin.pluginStatus, 'error'); + }); + + await t.test('should lazy load the plugin if deffered', async t => { + const url = 'http://example.com/plugin'; + t.mock.method(rtlMainThreadPlugin, 'loadScript', () => { + globalThis.registerRTLTextPlugin({}); + return Promise.resolve(); + }); + await rtlMainThreadPlugin.setRTLTextPlugin(url, true); + t.assert.equal(rtlMainThreadPlugin.loadScript.mock.callCount(), 0); + t.assert.equal(rtlMainThreadPlugin.pluginStatus, 'deferred'); + const promise = rtlMainThreadPlugin.lazyLoadRTLTextPlugin(); + await sleep(0); + await promise; + t.assert.equal(rtlMainThreadPlugin.pluginStatus, 'loaded'); + t.assert.equal(rtlMainThreadPlugin.loadScript.mock.callCount(), 1); + }); +}); diff --git a/test/unit/style/style.test.js b/test/unit/style/style.test.js index 101524fe4..c6d5b10b5 100644 --- a/test/unit/style/style.test.js +++ b/test/unit/style/style.test.js @@ -2,7 +2,7 @@ import test from 'node:test'; import { Event, Evented } from '@mapwhit/events'; import { Color } from '@mapwhit/style-expressions'; import Transform from '../../../src/geo/transform.js'; -import plugin from '../../../src/source/rtl_text_plugin.js'; +import { rtlMainThreadPluginFactory } from '../../../src/source/rtl_text_plugin_main_thread.js'; import SourceCache from '../../../src/source/source_cache.js'; import { OverscaledTileID } from '../../../src/source/tile_id.js'; import Style from '../../../src/style/style.js'; @@ -63,36 +63,7 @@ test('Style', async t => { style._remove(); }); - await t.test('registers plugin state change listener', t => { - plugin.clearRTLTextPlugin(); - - t.mock.method(plugin, 'registerForPluginStateChange'); - - style = new Style(new StubMap()); - t.assert.equal(plugin.registerForPluginStateChange.mock.callCount(), 1); - - t.mock.method(plugin, 'loadScript', () => Promise.reject()); - plugin.setRTLTextPlugin('some-bogus-url'); - t.assert.deepEqual(plugin.loadScript.mock.calls[0].arguments[0], 'https://example.org/some-bogus-url'); - }); - - await t.test('loads plugin immediately if already registered', (t, done) => { - plugin.clearRTLTextPlugin(); - t.mock.method(plugin, 'loadScript', () => Promise.reject(true)); - plugin.setRTLTextPlugin('some-bogus-url', error => { - // Getting this error message shows the bogus URL was succesfully passed to the worker state - t.assert.equal(error.message, 'RTL Text Plugin failed to load scripts from https://example.org/some-bogus-url'); - done(); - }); - style = new Style(createStyleJSON()); - }); - - await t.test('RTL plugin load reloads vector source but not raster source', (t, done) => { - plugin.clearRTLTextPlugin(); - t.mock.method(plugin, 'loadScript', () => { - globalThis.registerRTLTextPlugin({}); - return Promise.resolve(); - }); + await t.test('RTL plugin load reloads vector source but not raster source', async t => { style = new Style(new StubMap()); style.loadJSON( createStyleJSON({ @@ -115,19 +86,12 @@ test('Style', async t => { ] }) ); - style.on('style.load', () => { - t.mock.method(style._sources.raster, 'reload'); - t.mock.method(style._sources.vector, 'reload'); - plugin.setRTLTextPlugin('some-bogus-url', error => { - t.assert.ok(!error); - setTimeout(() => { - plugin.clearRTLTextPlugin(); - t.assert.equal(style._sources.raster.reload.mock.callCount(), 1); - t.assert.equal(style._sources.vector.reload.mock.callCount(), 2); - done(); - }, 0); - }); - }); + await style.once('style.load'); + t.mock.method(style._sources.raster, 'reload'); + t.mock.method(style._sources.vector, 'reload'); + rtlMainThreadPluginFactory().fire(new Event('pluginStateChange')); + t.assert.equal(style._sources.raster.reload.mock.callCount(), 0); + t.assert.equal(style._sources.vector.reload.mock.callCount(), 1); }); }); @@ -394,19 +358,14 @@ test('Style', async t => { }); }); - await t.test('deregisters plugin listener', (t, done) => { + await t.test('deregisters plugin listener', async t => { + t.mock.method(rtlMainThreadPluginFactory(), 'off'); const style = new Style(new StubMap()); - t.mock.method(style, '_reloadSources', () => {}); style.loadJSON(createStyleJSON()); - t.mock.method(plugin, 'loadScript', () => {}); - style.on('style.load', () => { - style._remove(); - plugin.setRTLTextPlugin('some-bogus-url'); - t.assert.equal(plugin.loadScript.mock.callCount(), 0); - t.assert.equal(style._reloadSources.mock.callCount(), 0); - done(); - }); + await style.once('style.load'); + style._remove(); + t.assert.equal(rtlMainThreadPluginFactory().off.mock.callCount(), 1); }); }); diff --git a/test/util/util.js b/test/util/util.js index b6a2e9efd..82d4f8cd4 100644 --- a/test/util/util.js +++ b/test/util/util.js @@ -75,3 +75,12 @@ export function waitForEvent(evented, eventName, predicate) { evented.on(eventName, listener); }); } + +/** + * This allows test to wait for a certain amount of time before continuing. + * @param milliseconds - the amount of time to wait in milliseconds + * @returns - a promise that resolves after the specified amount of time + */ +export function sleep(milliseconds = 0) { + return new Promise(resolve => setTimeout(resolve, milliseconds)); +} diff --git a/test/util/window.js b/test/util/window.js index c5bfd042e..c1c282496 100644 --- a/test/util/window.js +++ b/test/util/window.js @@ -1,7 +1,7 @@ import canvas from 'canvas'; import gl from 'gl'; import jsdom from 'jsdom'; -import '../../src/source/rtl_text_plugin.js'; +import '../../src/source/rtl_text_plugin_main_thread.js'; const _window = create(); From 4ae7f887bff3cd122c4554e612ee3ccf45adb14e Mon Sep 17 00:00:00 2001 From: zhangyiatmicrosoft <37553549+zhangyiatmicrosoft@users.noreply.github.com> Date: Wed, 13 Mar 2024 11:44:55 -0700 Subject: [PATCH 09/12] Overhaul of RTL plugin loading sequence (#3728) * fire pluginStateChange only if at least one receiver changed plugin status * draft * fixes * all ut pass * test cases * change log * worker tests * add change in style.ts * update style test * update UT * Update src/source/rtl_text_plugin_main_thread.test.ts * PR comments * PR comments * dup lazyLoad() calls * multiple * PR * replace exception with console.error * update comment to be more accurate * do not download from main thread * remove extra try/catch * remove extra throw * remove extra import * exception handling * correct version * all UT * UT * add tests (#7) * all UT * UT * Add FT * remove extra import * clean up comments * test update --------- Co-authored-by: Harel M Original commit: 82dce2b2e57abbadffa0fdaf2452ade1ce4f11ec: --- src/data/bucket/symbol_bucket.js | 7 +- src/source/rtl_text_plugin_main_thread.js | 83 ++++++++++++------- src/source/rtl_text_plugin_worker.js | 6 ++ src/source/tile.js | 2 +- src/source/worker_tile.js | 2 + src/style/style.js | 10 +-- .../rtl_text_plugin_main_thread.test.js | 71 ++++++++++++---- test/unit/style/style.test.js | 2 +- 8 files changed, 123 insertions(+), 60 deletions(-) diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index 57b6a9017..ac48479e0 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -182,11 +182,10 @@ export default class SymbolBucket { const resolvedTokens = layer.getValueAndResolveTokens('text-field', feature); const formattedText = resolvedTokens instanceof Formatted ? resolvedTokens : Formatted.fromString(resolvedTokens); - if (containsRTLText(formattedText)) { - this.hasRTLText = true; - } + // on this instance: if hasRTLText is already true, all future calls to containsRTLText can be skipped. + const bucketHasRTLText = (this.hasRTLText = this.hasRTLText || containsRTLText(formattedText)); if ( - !this.hasRTLText || // non-rtl text so can proceed safely + !bucketHasRTLText || // non-rtl text so can proceed safely rtlWorkerPlugin.getRTLTextPluginStatus() === 'unavailable' || // We don't intend to lazy-load the rtl text plugin, so proceed with incorrect shaping rtlWorkerPlugin.isParsed() // Use the rtlText plugin to shape text ) { diff --git a/src/source/rtl_text_plugin_main_thread.js b/src/source/rtl_text_plugin_main_thread.js index 850a883b6..ab852c95c 100644 --- a/src/source/rtl_text_plugin_main_thread.js +++ b/src/source/rtl_text_plugin_main_thread.js @@ -18,24 +18,28 @@ import { rtlWorkerPlugin } from './rtl_text_plugin_worker.js'; */ class RTLMainThreadPlugin extends Evented { - pluginStatus = 'unavailable'; - pluginURL = null; + status = 'unavailable'; + url = null; queue = []; - _sendPluginStateToWorker() { + _syncState(statusToSend) { + this.status = statusToSend; syncRTLPluginState({ - pluginStatus: this.pluginStatus, - pluginURL: this.pluginURL + pluginStatus: this.status, + pluginURL: this.url }); - this.fire(new Event('pluginStateChange', { pluginStatus: this.pluginStatus, pluginURL: this.pluginURL })); } + + /** This one is exposed to outside */ getRTLTextPluginStatus() { - return this.pluginStatus; + return this.status; } + // public for testing clearRTLTextPlugin() { - this.pluginStatus = 'unavailable'; - this.pluginURL = null; + this.status = 'unavailable'; + this.url = null; } + // public for testing loadScript({ url }) { const { promise, resolve, reject } = Promise.withResolvers(); @@ -44,34 +48,51 @@ class RTLMainThreadPlugin extends Evented { s.onerror = () => reject(true); return promise; } - async setRTLTextPlugin(url, deferred = false) { - if (this.pluginStatus === 'deferred' || this.pluginStatus === 'loading' || this.pluginStatus === 'loaded') { - throw new Error('setRTLTextPlugin cannot be called multiple times.'); + + setRTLTextPlugin(url, deferred = false) { + if (this.url) { + // error + return Promise.reject(new Error('setRTLTextPlugin cannot be called multiple times.')); + } + this.url = browser.resolveURL(url); + if (!this.url) { + return Promise.reject(new Error(`requested url ${url} is invalid`)); } - this.pluginURL = browser.resolveURL(url); - this.pluginStatus = 'deferred'; - this._sendPluginStateToWorker(); - if (!deferred) { - //Start downloading the plugin immediately if not intending to lazy-load - await this._downloadRTLTextPlugin(); + if (this.status === 'requested') { + return this._downloadRTLTextPlugin(); + } + if (this.status === 'unavailable') { + // from initial state: + if (!deferred) { + return this._downloadRTLTextPlugin(); + } + this.status = 'deferred'; + // fire and forget: in this case it does not need wait for the broadcasting result + // it is important to sync the deferred status once because + // symbol_bucket will be checking it in worker + this._syncState(this.status); } } + async _downloadRTLTextPlugin() { - if (this.pluginStatus !== 'deferred' || !this.pluginURL) { - throw new Error('rtl-text-plugin cannot be downloaded unless a pluginURL is specified'); - } + this._syncState('loading'); try { - this.pluginStatus = 'loading'; - this._sendPluginStateToWorker(); - await this.loadScript({ url: this.pluginURL }); + await this.loadScript({ url: this.url }); + this.fire(new Event('RTLPluginLoaded')); } catch { - this.pluginStatus = 'error'; - this._sendPluginStateToWorker(); + this.status = 'error'; + this._syncState(this.status); + throw new Error(`RTL Text Plugin failed to import scripts from ${this.url}`); } } - async lazyLoadRTLTextPlugin() { - if (this.pluginStatus === 'deferred') { - await this._downloadRTLTextPlugin(); + + /** Start a lazy loading process of RTL plugin */ + lazyLoad() { + if (this.status === 'unavailable') { + this.status = 'requested'; + this._syncState(this.status); + } else if (this.status === 'deferred') { + return this._downloadRTLTextPlugin(); } } } @@ -93,8 +114,8 @@ function registerRTLTextPlugin(rtlTextPlugin) { throw new Error('RTL text plugin already registered.'); } const rtlMainThreadPlugin = rtlMainThreadPluginFactory(); - rtlMainThreadPlugin.pluginStatus = 'loaded'; - rtlMainThreadPlugin._sendPluginStateToWorker(); + rtlMainThreadPlugin.status = 'loaded'; + rtlMainThreadPlugin._syncState(rtlMainThreadPlugin.status); rtlWorkerPlugin.setMethods(rtlTextPlugin); } diff --git a/src/source/rtl_text_plugin_worker.js b/src/source/rtl_text_plugin_worker.js index ed94a5d23..e65b11eb5 100644 --- a/src/source/rtl_text_plugin_worker.js +++ b/src/source/rtl_text_plugin_worker.js @@ -8,6 +8,12 @@ class RTLWorkerPlugin { this.pluginStatus = state.pluginStatus; this.pluginURL = state.pluginURL; } + getState() { + return { + pluginStatus: this.pluginStatus, + pluginURL: this.pluginURL + }; + } setMethods(rtlTextPlugin) { this.applyArabicShaping = rtlTextPlugin.applyArabicShaping; this.processBidirectionalText = rtlTextPlugin.processBidirectionalText; diff --git a/src/source/tile.js b/src/source/tile.js index d74ff7b09..4c70aa079 100644 --- a/src/source/tile.js +++ b/src/source/tile.js @@ -100,7 +100,7 @@ class Tile { if (data.hasRTLText) { this.hasRTLText = data.hasRTLText; - rtlMainThreadPluginFactory().lazyLoadRTLTextPlugin(); + rtlMainThreadPluginFactory().lazyLoad(); } if (data.imageAtlas) { this.imageAtlas = data.imageAtlas; diff --git a/src/source/worker_tile.js b/src/source/worker_tile.js index a58453c0d..6a08270ad 100644 --- a/src/source/worker_tile.js +++ b/src/source/worker_tile.js @@ -151,6 +151,8 @@ async function finalizeBuckets(params, options, resources) { } async function makeAtlasses({ glyphDependencies, patternDependencies, iconDependencies }, resources) { + // options.glyphDependencies looks like: {"SomeFontName":{"10":true,"32":true}} + // this line makes an object like: {"SomeFontName":[10,32]} const stacks = mapObject(glyphDependencies, glyphs => Object.keys(glyphs).map(Number)); const icons = Object.keys(iconDependencies); const patterns = Object.keys(patternDependencies); diff --git a/src/style/style.js b/src/style/style.js index 0735606fe..d4af4f365 100644 --- a/src/style/style.js +++ b/src/style/style.js @@ -42,7 +42,7 @@ class Style extends Evented { }); #layerIndex = new StyleLayerIndex(); #opsQueue = []; - #pluginStateChangeHandler; + #rtlPluginLoadedHandler; constructor(map, options = {}) { super(); @@ -64,8 +64,8 @@ class Style extends Evented { this._updatedLayers = new Map(); this._removedLayers = new Map(); this._resetUpdates(); - this.#pluginStateChangeHandler = this.#rtlTextPluginStateChange.bind(this); - rtlMainThreadPluginFactory().on('pluginStateChange', this.#pluginStateChangeHandler); + this.#rtlPluginLoadedHandler = this.#rtlPluginLoaded.bind(this); + rtlMainThreadPluginFactory().on('RTLPluginLoaded', this.#rtlPluginLoadedHandler); this.on('data', event => { if (event.dataType !== 'source' || event.sourceDataType !== 'metadata') { return; @@ -89,7 +89,7 @@ class Style extends Evented { }); } - #rtlTextPluginStateChange() { + #rtlPluginLoaded() { for (const sourceCache of Object.values(this._sources)) { const { type } = sourceCache.getSource(); if (type === 'vector' || type === 'geojson') { @@ -1055,7 +1055,7 @@ class Style extends Evented { } _remove() { - rtlMainThreadPluginFactory().off('pluginStateChange', this.#pluginStateChangeHandler); + rtlMainThreadPluginFactory().off('RTLPluginLoaded', this.#rtlPluginLoadedHandler); for (const id in this._sources) { this._sources[id].clearTiles(); } diff --git a/test/unit/source/rtl_text_plugin_main_thread.test.js b/test/unit/source/rtl_text_plugin_main_thread.test.js index 579f7506d..8a1898624 100644 --- a/test/unit/source/rtl_text_plugin_main_thread.test.js +++ b/test/unit/source/rtl_text_plugin_main_thread.test.js @@ -7,6 +7,8 @@ import _window from '../../util/window.js'; const rtlMainThreadPlugin = rtlMainThreadPluginFactory(); test('RTLMainThreadPlugin', async t => { let globalWindow; + const url = 'http://example.com/plugin'; + const failedToLoadMessage = `RTL Text Plugin failed to import scripts from ${url}`; t.before(() => { globalWindow = globalThis.window; globalThis.window = _window; @@ -28,7 +30,6 @@ test('RTLMainThreadPlugin', async t => { }); await t.test('should set the RTL text plugin and download it', async t => { - const url = 'http://example.com/plugin'; t.mock.method(rtlMainThreadPlugin, 'loadScript', () => { globalThis.registerRTLTextPlugin({}); return Promise.resolve(); @@ -36,20 +37,18 @@ test('RTLMainThreadPlugin', async t => { const promise = rtlMainThreadPlugin.setRTLTextPlugin(url); await sleep(0); await promise; - t.assert.equal(rtlMainThreadPlugin.pluginURL, url); + t.assert.equal(rtlMainThreadPlugin.url, url); t.assert.equal(rtlMainThreadPlugin.loadScript.mock.callCount(), 1); }); - await t.test('should set the RTL text plugin but deffer downloading', async t => { - const url = 'http://example.com/plugin'; + await t.test('should set the RTL text plugin but defer downloading', async t => { t.mock.method(rtlMainThreadPlugin, 'loadScript'); await rtlMainThreadPlugin.setRTLTextPlugin(url, true); t.assert.equal(rtlMainThreadPlugin.loadScript.mock.callCount(), 0); - t.assert.equal(rtlMainThreadPlugin.pluginStatus, 'deferred'); + t.assert.equal(rtlMainThreadPlugin.status, 'deferred'); }); await t.test('should throw if the plugin is already set', async t => { - const url = 'http://example.com/plugin'; await rtlMainThreadPlugin.setRTLTextPlugin(url, true); await t.assert.rejects(rtlMainThreadPlugin.setRTLTextPlugin(url), { message: 'setRTLTextPlugin cannot be called multiple times.' @@ -59,33 +58,69 @@ test('RTLMainThreadPlugin', async t => { await t.test('should throw if the plugin url is not set', async t => { t.mock.method(browser, 'resolveURL', () => ''); await t.assert.rejects(rtlMainThreadPlugin.setRTLTextPlugin(null), { - message: 'rtl-text-plugin cannot be downloaded unless a pluginURL is specified' + message: 'requested url null is invalid' }); }); await t.test('should be in error state if download fails', async t => { - const url = 'http://example.com/plugin'; t.mock.method(rtlMainThreadPlugin, 'loadScript', () => Promise.reject()); - const promise = rtlMainThreadPlugin.setRTLTextPlugin(url); - await sleep(0); - await promise; - t.assert.equal(rtlMainThreadPlugin.pluginURL, url); - t.assert.equal(rtlMainThreadPlugin.pluginStatus, 'error'); + await t.assert.rejects(rtlMainThreadPlugin.setRTLTextPlugin(url), { + message: failedToLoadMessage + }); + t.assert.equal(rtlMainThreadPlugin.url, url); + t.assert.equal(rtlMainThreadPlugin.status, 'error'); }); - await t.test('should lazy load the plugin if deffered', async t => { - const url = 'http://example.com/plugin'; + await t.test('should lazy load the plugin if deferred', async t => { t.mock.method(rtlMainThreadPlugin, 'loadScript', () => { globalThis.registerRTLTextPlugin({}); return Promise.resolve(); }); await rtlMainThreadPlugin.setRTLTextPlugin(url, true); t.assert.equal(rtlMainThreadPlugin.loadScript.mock.callCount(), 0); - t.assert.equal(rtlMainThreadPlugin.pluginStatus, 'deferred'); - const promise = rtlMainThreadPlugin.lazyLoadRTLTextPlugin(); + t.assert.equal(rtlMainThreadPlugin.status, 'deferred'); + const promise = rtlMainThreadPlugin.lazyLoad(); await sleep(0); await promise; - t.assert.equal(rtlMainThreadPlugin.pluginStatus, 'loaded'); + t.assert.equal(rtlMainThreadPlugin.status, 'loaded'); t.assert.equal(rtlMainThreadPlugin.loadScript.mock.callCount(), 1); }); + + await t.test('should set status to requested if RTL plugin was not set', t => { + rtlMainThreadPlugin.lazyLoad(); + t.assert.equal(rtlMainThreadPlugin.status, 'requested'); + }); + + await t.test('should immediately download if RTL plugin was already requested, ignoring deferred:true', async t => { + t.mock.method(rtlMainThreadPlugin, 'loadScript', () => { + globalThis.registerRTLTextPlugin({}); + return Promise.resolve(); + }); + rtlMainThreadPlugin.lazyLoad(); + t.assert.equal(rtlMainThreadPlugin.status, 'requested'); + await sleep(1); + // notice even when deferred is true, it should download because already requested + await rtlMainThreadPlugin.setRTLTextPlugin(url, true); + t.assert.equal(rtlMainThreadPlugin.status, 'loaded'); + }); + + await t.test('should allow multiple calls to lazyLoad', t => { + rtlMainThreadPlugin.lazyLoad(); + t.assert.equal(rtlMainThreadPlugin.status, 'requested'); + rtlMainThreadPlugin.lazyLoad(); + t.assert.equal(rtlMainThreadPlugin.status, 'requested'); + }); + + await t.test('should be in error state if lazyLoad fails', async t => { + const resultPromise = rtlMainThreadPlugin.setRTLTextPlugin(url, true); + t.assert.equal(await resultPromise, undefined); + t.assert.equal(rtlMainThreadPlugin.status, 'deferred'); + // the next one should fail + t.mock.method(rtlMainThreadPlugin, 'loadScript', () => Promise.reject()); + await t.assert.rejects(rtlMainThreadPlugin.lazyLoad(), { + message: failedToLoadMessage + }); + t.assert.equal(rtlMainThreadPlugin.url, url); + t.assert.equal(rtlMainThreadPlugin.status, 'error'); + }); }); diff --git a/test/unit/style/style.test.js b/test/unit/style/style.test.js index c6d5b10b5..5cf93a4c2 100644 --- a/test/unit/style/style.test.js +++ b/test/unit/style/style.test.js @@ -89,7 +89,7 @@ test('Style', async t => { await style.once('style.load'); t.mock.method(style._sources.raster, 'reload'); t.mock.method(style._sources.vector, 'reload'); - rtlMainThreadPluginFactory().fire(new Event('pluginStateChange')); + rtlMainThreadPluginFactory().fire(new Event('RTLPluginLoaded')); t.assert.equal(style._sources.raster.reload.mock.callCount(), 0); t.assert.equal(style._sources.vector.reload.mock.callCount(), 1); }); From fed761fb35e3cca49a0788a355aead58ef5d388d Mon Sep 17 00:00:00 2001 From: Harel M Date: Mon, 21 Oct 2024 12:59:24 +0300 Subject: [PATCH 10/12] Support for RTL text plugin 0.3.0 (#4860) * Allow loading rtl plugin async * Allow loading rtl plugin async * Add changelog * Fix typecheck and code * Add tests to make sure both 0.2.3 and 0.3.0 are supported * Improve test name * Change to use latest version everywhere. * Revert unrelated changes create mode 100644 src/source/rtl_text_plugin_worker.test.ts Original commit: a2f1450549c9e977f6d4fcb0f249c4ffe1d6e11b: --- src/index.js | 6 +-- src/source/rtl_text_plugin_worker.js | 10 ++-- .../source/rtl_text_plugin_worker.test.js | 52 +++++++++++++++++++ 3 files changed, 62 insertions(+), 6 deletions(-) create mode 100644 test/unit/source/rtl_text_plugin_worker.test.js diff --git a/src/index.js b/src/index.js index 5840626d5..676198854 100644 --- a/src/index.js +++ b/src/index.js @@ -51,8 +51,8 @@ export default mapwhit; * @param {boolean} lazy If set to `true`, mapboxgl will defer loading the plugin until rtl text is encountered, * rtl text will then be rendered only after the plugin finishes loading. * @example - * mapboxgl.setRTLTextPlugin('https://api.mapbox.com/mapbox-gl-js/plugins/mapbox-gl-rtl-text/v0.2.0/mapbox-gl-rtl-text.js'); - * @see [Add support for right-to-left scripts](https://www.mapbox.com/mapbox-gl-js/example/mapbox-gl-rtl-text/) + * setRTLTextPlugin('https://unpkg.com/@mapbox/mapbox-gl-rtl-text@0.3.0/dist/mapbox-gl-rtl-text.js', false); + * @see [Add support for right-to-left scripts](https://maplibre.org/maplibre-gl-js/docs/examples/mapbox-gl-rtl-text/) */ function setRTLTextPlugin(pluginURL, lazy) { return rtlMainThreadPluginFactory().setRTLTextPlugin(pluginURL, lazy); @@ -64,7 +64,7 @@ function setRTLTextPlugin(pluginURL, lazy) { * * @function getRTLTextPluginStatus * @example - * const pluginStatus = mapboxgl.getRTLTextPluginStatus(); + * const pluginStatus = getRTLTextPluginStatus(); */ function getRTLTextPluginStatus() { return rtlMainThreadPluginFactory().getRTLTextPluginStatus(); diff --git a/src/source/rtl_text_plugin_worker.js b/src/source/rtl_text_plugin_worker.js index e65b11eb5..cdaa3ba87 100644 --- a/src/source/rtl_text_plugin_worker.js +++ b/src/source/rtl_text_plugin_worker.js @@ -4,7 +4,11 @@ class RTLWorkerPlugin { processStyledBidirectionalText = null; pluginStatus = 'unavailable'; pluginURL = null; + setState(state) { + if (this.isParsed()) { + return; + } this.pluginStatus = state.pluginStatus; this.pluginURL = state.pluginURL; } @@ -15,6 +19,9 @@ class RTLWorkerPlugin { }; } setMethods(rtlTextPlugin) { + if (this.isParsed()) { + throw new Error('RTL text plugin already registered.'); + } this.applyArabicShaping = rtlTextPlugin.applyArabicShaping; this.processBidirectionalText = rtlTextPlugin.processBidirectionalText; this.processStyledBidirectionalText = rtlTextPlugin.processStyledBidirectionalText; @@ -26,9 +33,6 @@ class RTLWorkerPlugin { this.processStyledBidirectionalText != null ); } - getPluginURL() { - return this.pluginURL; - } getRTLTextPluginStatus() { return this.pluginStatus; } diff --git a/test/unit/source/rtl_text_plugin_worker.test.js b/test/unit/source/rtl_text_plugin_worker.test.js new file mode 100644 index 000000000..791317be6 --- /dev/null +++ b/test/unit/source/rtl_text_plugin_worker.test.js @@ -0,0 +1,52 @@ +import test from 'node:test'; +import { rtlWorkerPlugin } from '../../../src/source/rtl_text_plugin_worker.js'; + +test('RTLWorkerPlugin', async t => { + t.beforeEach(() => { + // This is a static class, so we need to reset the properties before each test + rtlWorkerPlugin.processStyledBidirectionalText = null; + rtlWorkerPlugin.processBidirectionalText = null; + rtlWorkerPlugin.applyArabicShaping = null; + }); + + await t.test('should throw if already parsed', t => { + const rtlTextPlugin = { + applyArabicShaping: () => {}, + processBidirectionalText: () => {}, + processStyledBidirectionalText: () => {} + }; + rtlWorkerPlugin.setMethods(rtlTextPlugin); + t.assert.throws(() => rtlWorkerPlugin.setMethods(rtlTextPlugin), { + message: 'RTL text plugin already registered.' + }); + }); + + await t.test('should move RTL plugin from unavailable to deferred', t => { + rtlWorkerPlugin.pluginURL = ''; + rtlWorkerPlugin.pluginStatus = 'unavailable'; + const mockMessage = { + pluginURL: 'https://somehost/somescript', + pluginStatus: 'deferred' + }; + rtlWorkerPlugin.setState(mockMessage); + t.assert.equal(rtlWorkerPlugin.getRTLTextPluginStatus(), 'deferred'); + }); + + await t.test('should not change RTL plugin status if already parsed', t => { + const originalUrl = 'https://somehost/somescript1'; + rtlWorkerPlugin.pluginURL = originalUrl; + rtlWorkerPlugin.pluginStatus = 'loaded'; + rtlWorkerPlugin.setMethods({ + applyArabicShaping: () => {}, + processBidirectionalText: () => {}, + processStyledBidirectionalText: () => {} + }); + const mockMessage = { + pluginURL: 'https://somehost/somescript2', + pluginStatus: 'loading' + }; + rtlWorkerPlugin.setState(mockMessage); + t.assert.equal(rtlWorkerPlugin.getRTLTextPluginStatus(), 'loaded'); + t.assert.equal(rtlWorkerPlugin.pluginURL, originalUrl); + }); +}); From 4e24f1c2e95ac6d0e941e8637f5d55c3dd166df4 Mon Sep 17 00:00:00 2001 From: melitele Date: Sat, 24 Jan 2026 00:49:37 -0700 Subject: [PATCH 11/12] refactor RTL plugin to `mapwhit` coding conventions - collate in one module - replace classes by functions - distinguish between `loader` and `plugin` instead of `main thread` and `worker` - remove unnecessary methods and rename according to intended use --- src/data/bucket/symbol_bucket.js | 5 +- src/index.js | 6 +- src/source/rtl_text_plugin.js | 161 ++++++++++++++++++ src/source/rtl_text_plugin_main_thread.js | 122 ------------- src/source/rtl_text_plugin_worker.js | 41 ----- src/source/tile.js | 4 +- src/style/evaluation_parameters.js | 4 +- src/style/style.js | 6 +- src/symbol/shaping.js | 4 +- src/symbol/transform_text.js | 6 +- test/unit/source/rtl_text_plugin.test.js | 68 ++++++++ .../source/rtl_text_plugin_loader.test.js | 150 ++++++++++++++++ .../rtl_text_plugin_main_thread.test.js | 126 -------------- .../source/rtl_text_plugin_worker.test.js | 52 ------ test/unit/style/style.test.js | 8 +- test/util/window.js | 2 +- 16 files changed, 401 insertions(+), 364 deletions(-) create mode 100644 src/source/rtl_text_plugin.js delete mode 100644 src/source/rtl_text_plugin_main_thread.js delete mode 100644 src/source/rtl_text_plugin_worker.js create mode 100644 test/unit/source/rtl_text_plugin.test.js create mode 100644 test/unit/source/rtl_text_plugin_loader.test.js delete mode 100644 test/unit/source/rtl_text_plugin_main_thread.test.js delete mode 100644 test/unit/source/rtl_text_plugin_worker.test.js diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index ac48479e0..55929a387 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -1,7 +1,7 @@ import { dist } from '@mapwhit/point-geometry'; import { Formatted } from '@mapwhit/style-expressions'; import { VectorTileFeature } from '@mapwhit/vector-tile'; -import { rtlWorkerPlugin } from '../../source/rtl_text_plugin_worker.js'; +import { rtlPlugin } from '../../source/rtl_text_plugin.js'; import EvaluationParameters from '../../style/evaluation_parameters.js'; import mergeLines from '../../symbol/mergelines.js'; import { getSizeData } from '../../symbol/symbol_size.js'; @@ -186,8 +186,7 @@ export default class SymbolBucket { const bucketHasRTLText = (this.hasRTLText = this.hasRTLText || containsRTLText(formattedText)); if ( !bucketHasRTLText || // non-rtl text so can proceed safely - rtlWorkerPlugin.getRTLTextPluginStatus() === 'unavailable' || // We don't intend to lazy-load the rtl text plugin, so proceed with incorrect shaping - rtlWorkerPlugin.isParsed() // Use the rtlText plugin to shape text + rtlPlugin.isRTLSupported(true) // Use the rtlText plugin to shape text if available or We don't intend to lazy-load the rtl text plugin, so proceed with incorrect shaping ) { text = transformText(formattedText, layer, feature); } diff --git a/src/index.js b/src/index.js index 676198854..bd773c777 100644 --- a/src/index.js +++ b/src/index.js @@ -5,7 +5,7 @@ import { Point } from '@mapwhit/point-geometry'; import packageJSON from '../package.json' with { type: 'json' }; import { default as LngLat } from './geo/lng_lat.js'; import { default as LngLatBounds } from './geo/lng_lat_bounds.js'; -import { rtlMainThreadPluginFactory } from './source/rtl_text_plugin_main_thread.js'; +import { rtlPluginLoader } from './source/rtl_text_plugin.js'; import { default as Style } from './style/style.js'; import { default as Map } from './ui/map.js'; import config from './util/config.js'; @@ -55,7 +55,7 @@ export default mapwhit; * @see [Add support for right-to-left scripts](https://maplibre.org/maplibre-gl-js/docs/examples/mapbox-gl-rtl-text/) */ function setRTLTextPlugin(pluginURL, lazy) { - return rtlMainThreadPluginFactory().setRTLTextPlugin(pluginURL, lazy); + return rtlPluginLoader.setRTLTextPlugin(pluginURL, lazy); } /** * Gets the map's [RTL text plugin](https://www.mapbox.com/mapbox-gl-js/plugins/#mapbox-gl-rtl-text) status. @@ -67,7 +67,7 @@ function setRTLTextPlugin(pluginURL, lazy) { * const pluginStatus = getRTLTextPluginStatus(); */ function getRTLTextPluginStatus() { - return rtlMainThreadPluginFactory().getRTLTextPluginStatus(); + return rtlPluginLoader.getRTLTextPluginStatus(); } // canary assert: used to confirm that asserts have been removed from production build diff --git a/src/source/rtl_text_plugin.js b/src/source/rtl_text_plugin.js new file mode 100644 index 000000000..8bef13bd5 --- /dev/null +++ b/src/source/rtl_text_plugin.js @@ -0,0 +1,161 @@ +import { Event, Evented } from '@mapwhit/events'; +import dynload from 'dynload'; +import browser from '../util/browser.js'; + +/** + * The possible option of the plugin's status + * + * `unavailable`: Not loaded. + * + * `deferred`: The plugin URL has been specified, but loading has been deferred. + * + * `loading`: request in-flight. + * + * `loaded`: The plugin is now loaded + * + * `error`: The plugin failed to load + */ + +function RTLPlugin() { + const self = { + isRTLSupported + }; + + /** + * Checks whether the RTL language support is available. + * If `canChangeShortly` is false, it will only return true if the RTL language + * is properly supported. + * If `canChangeShortly` is true, it will also return true if the RTL language + * is not supported unless it can obtain the RTL text plugin. + * @param {boolean} canChangeShortly + * @returns + */ + function isRTLSupported(canChangeShortly) { + if (rtlPluginLoader.getRTLTextPluginStatus() === 'loaded') { + return true; + } + if (!canChangeShortly) { + return false; + } + rtlPluginLoader.lazyLoad(); + // any transitive state other than 'loading' means we can consider RTL supported as best as possible for now + return rtlPluginLoader.getRTLTextPluginStatus() !== 'loading'; + } + + return self; +} + +export const rtlPlugin = RTLPlugin(); + +function RTLPluginLoader() { + let status = 'unavailable'; + let url; + + const self = { + getRTLTextPluginStatus, + setRTLTextPlugin, + lazyLoad, + _clearRTLTextPlugin, + _registerRTLTextPlugin + }; + + /** This one is exposed to outside */ + function getRTLTextPluginStatus() { + return status; + } + + // public for testing + function _clearRTLTextPlugin() { + url = undefined; + status = 'unavailable'; + _setMethods(); + } + + function setRTLTextPlugin(pluginURL, deferred = false) { + if (url) { + // error + return Promise.reject(new Error('setRTLTextPlugin cannot be called multiple times.')); + } + url = browser.resolveURL(pluginURL); + if (!url) { + return Promise.reject(new Error(`requested url ${pluginURL} is invalid`)); + } + if (status === 'requested') { + return _downloadRTLTextPlugin(); + } + if (status === 'unavailable') { + // from initial state: + if (!deferred) { + return _downloadRTLTextPlugin(); + } + status = 'deferred'; + } + } + + async function _downloadRTLTextPlugin() { + status = 'loading'; + try { + await rtlPluginLoader._loadScript({ url }); + } catch { + status = 'error'; + } + rtlPluginLoader.fire(new Event('RTLPluginLoaded')); + } + + /** Start a lazy loading process of RTL plugin */ + function lazyLoad() { + if (status === 'unavailable') { + status = 'requested'; + return; + } + if (status === 'deferred') { + return _downloadRTLTextPlugin(); + } + } + + function _setMethods(rtlTextPlugin) { + if (!rtlTextPlugin) { + // for testing only + rtlPlugin.processStyledBidirectionalText = null; + rtlPlugin.processBidirectionalText = null; + rtlPlugin.applyArabicShaping = null; + return; + } + rtlPlugin.applyArabicShaping = rtlTextPlugin.applyArabicShaping; + rtlPlugin.processBidirectionalText = rtlTextPlugin.processBidirectionalText; + rtlPlugin.processStyledBidirectionalText = rtlTextPlugin.processStyledBidirectionalText; + } + + // This is invoked by the RTL text plugin when the download has finished, and the code has been parsed. + function _registerRTLTextPlugin(rtlTextPlugin) { + if (rtlPlugin.isRTLSupported()) { + throw new Error('RTL text plugin already registered.'); + } + status = 'loaded'; + _setMethods(rtlTextPlugin); + } + + return self; +} + +// public for testing +function _loadScript({ url }) { + const { promise, resolve, reject } = Promise.withResolvers(); + const s = dynload(url); + s.onload = () => resolve(); + s.onerror = () => reject(true); + return promise; +} + +const { getRTLTextPluginStatus, setRTLTextPlugin, lazyLoad, _clearRTLTextPlugin, _registerRTLTextPlugin } = + RTLPluginLoader(); + +globalThis.registerRTLTextPlugin ??= _registerRTLTextPlugin; + +export const rtlPluginLoader = Object.assign(new Evented(), { + getRTLTextPluginStatus, + setRTLTextPlugin, + lazyLoad, + _clearRTLTextPlugin, + _loadScript +}); diff --git a/src/source/rtl_text_plugin_main_thread.js b/src/source/rtl_text_plugin_main_thread.js deleted file mode 100644 index ab852c95c..000000000 --- a/src/source/rtl_text_plugin_main_thread.js +++ /dev/null @@ -1,122 +0,0 @@ -import { Event, Evented } from '@mapwhit/events'; -import dynload from 'dynload'; -import browser from '../util/browser.js'; -import { rtlWorkerPlugin } from './rtl_text_plugin_worker.js'; - -/** - * The possible option of the plugin's status - * - * `unavailable`: Not loaded. - * - * `deferred`: The plugin URL has been specified, but loading has been deferred. - * - * `loading`: request in-flight. - * - * `loaded`: The plugin is now loaded - * - * `error`: The plugin failed to load - */ - -class RTLMainThreadPlugin extends Evented { - status = 'unavailable'; - url = null; - queue = []; - _syncState(statusToSend) { - this.status = statusToSend; - syncRTLPluginState({ - pluginStatus: this.status, - pluginURL: this.url - }); - } - - /** This one is exposed to outside */ - getRTLTextPluginStatus() { - return this.status; - } - - // public for testing - clearRTLTextPlugin() { - this.status = 'unavailable'; - this.url = null; - } - - // public for testing - loadScript({ url }) { - const { promise, resolve, reject } = Promise.withResolvers(); - const s = dynload(url); - s.onload = () => resolve(); - s.onerror = () => reject(true); - return promise; - } - - setRTLTextPlugin(url, deferred = false) { - if (this.url) { - // error - return Promise.reject(new Error('setRTLTextPlugin cannot be called multiple times.')); - } - this.url = browser.resolveURL(url); - if (!this.url) { - return Promise.reject(new Error(`requested url ${url} is invalid`)); - } - if (this.status === 'requested') { - return this._downloadRTLTextPlugin(); - } - if (this.status === 'unavailable') { - // from initial state: - if (!deferred) { - return this._downloadRTLTextPlugin(); - } - this.status = 'deferred'; - // fire and forget: in this case it does not need wait for the broadcasting result - // it is important to sync the deferred status once because - // symbol_bucket will be checking it in worker - this._syncState(this.status); - } - } - - async _downloadRTLTextPlugin() { - this._syncState('loading'); - try { - await this.loadScript({ url: this.url }); - this.fire(new Event('RTLPluginLoaded')); - } catch { - this.status = 'error'; - this._syncState(this.status); - throw new Error(`RTL Text Plugin failed to import scripts from ${this.url}`); - } - } - - /** Start a lazy loading process of RTL plugin */ - lazyLoad() { - if (this.status === 'unavailable') { - this.status = 'requested'; - this._syncState(this.status); - } else if (this.status === 'deferred') { - return this._downloadRTLTextPlugin(); - } - } -} -let rtlMainThreadPlugin = null; -export function rtlMainThreadPluginFactory() { - if (!rtlMainThreadPlugin) { - rtlMainThreadPlugin = new RTLMainThreadPlugin(); - } - return rtlMainThreadPlugin; -} - -function syncRTLPluginState(state) { - rtlWorkerPlugin.setState(state); -} - -// This is invoked by the RTL text plugin when the download via the `importScripts` call has finished, and the code has been parsed. -function registerRTLTextPlugin(rtlTextPlugin) { - if (rtlWorkerPlugin.isParsed()) { - throw new Error('RTL text plugin already registered.'); - } - const rtlMainThreadPlugin = rtlMainThreadPluginFactory(); - rtlMainThreadPlugin.status = 'loaded'; - rtlMainThreadPlugin._syncState(rtlMainThreadPlugin.status); - rtlWorkerPlugin.setMethods(rtlTextPlugin); -} - -globalThis.registerRTLTextPlugin ??= registerRTLTextPlugin; diff --git a/src/source/rtl_text_plugin_worker.js b/src/source/rtl_text_plugin_worker.js deleted file mode 100644 index cdaa3ba87..000000000 --- a/src/source/rtl_text_plugin_worker.js +++ /dev/null @@ -1,41 +0,0 @@ -class RTLWorkerPlugin { - applyArabicShaping = null; - processBidirectionalText = null; - processStyledBidirectionalText = null; - pluginStatus = 'unavailable'; - pluginURL = null; - - setState(state) { - if (this.isParsed()) { - return; - } - this.pluginStatus = state.pluginStatus; - this.pluginURL = state.pluginURL; - } - getState() { - return { - pluginStatus: this.pluginStatus, - pluginURL: this.pluginURL - }; - } - setMethods(rtlTextPlugin) { - if (this.isParsed()) { - throw new Error('RTL text plugin already registered.'); - } - this.applyArabicShaping = rtlTextPlugin.applyArabicShaping; - this.processBidirectionalText = rtlTextPlugin.processBidirectionalText; - this.processStyledBidirectionalText = rtlTextPlugin.processStyledBidirectionalText; - } - isParsed() { - return ( - this.applyArabicShaping != null && - this.processBidirectionalText != null && - this.processStyledBidirectionalText != null - ); - } - getRTLTextPluginStatus() { - return this.pluginStatus; - } -} - -export const rtlWorkerPlugin = new RTLWorkerPlugin(); diff --git a/src/source/tile.js b/src/source/tile.js index 4c70aa079..88b57cbfe 100644 --- a/src/source/tile.js +++ b/src/source/tile.js @@ -11,7 +11,7 @@ import browser from '../util/browser.js'; import { deepEqual } from '../util/object.js'; import uniqueId from '../util/unique_id.js'; import GeoJSONFeature from '../util/vectortile_to_geojson.js'; -import { rtlMainThreadPluginFactory } from './rtl_text_plugin_main_thread.js'; +import { rtlPlugin } from './rtl_text_plugin.js'; /** * A tile object is the combination of a Coordinate, which defines @@ -100,7 +100,7 @@ class Tile { if (data.hasRTLText) { this.hasRTLText = data.hasRTLText; - rtlMainThreadPluginFactory().lazyLoad(); + rtlPlugin.setRTLNeeded(); } if (data.imageAtlas) { this.imageAtlas = data.imageAtlas; diff --git a/src/style/evaluation_parameters.js b/src/style/evaluation_parameters.js index 397849a91..d6b49a1f7 100644 --- a/src/style/evaluation_parameters.js +++ b/src/style/evaluation_parameters.js @@ -1,4 +1,4 @@ -import { rtlWorkerPlugin } from '../source/rtl_text_plugin_worker.js'; +import { rtlPlugin } from '../source/rtl_text_plugin.js'; import { isStringInSupportedScript } from '../util/script_detection.js'; import ZoomHistory from './zoom_history.js'; @@ -39,5 +39,5 @@ export default class EvaluationParameters { } function isSupportedScript(str) { - return isStringInSupportedScript(str, rtlWorkerPlugin.getRTLTextPluginStatus() === 'loaded'); + return isStringInSupportedScript(str, rtlPlugin.isRTLSupported()); } diff --git a/src/style/style.js b/src/style/style.js index d4af4f365..b419b583d 100644 --- a/src/style/style.js +++ b/src/style/style.js @@ -5,7 +5,7 @@ import ImageManager from '../render/image_manager.js'; import LineAtlas from '../render/line_atlas.js'; import { queryRenderedFeatures, queryRenderedSymbols, querySourceFeatures } from '../source/query_features.js'; import { resources } from '../source/resources/index.js'; -import { rtlMainThreadPluginFactory } from '../source/rtl_text_plugin_main_thread.js'; +import { rtlPluginLoader } from '../source/rtl_text_plugin.js'; import { getType as getSourceType, setType as setSourceType } from '../source/source.js'; import SourceCache from '../source/source_cache.js'; import CrossTileSymbolIndex from '../symbol/cross_tile_symbol_index.js'; @@ -65,7 +65,7 @@ class Style extends Evented { this._removedLayers = new Map(); this._resetUpdates(); this.#rtlPluginLoadedHandler = this.#rtlPluginLoaded.bind(this); - rtlMainThreadPluginFactory().on('RTLPluginLoaded', this.#rtlPluginLoadedHandler); + rtlPluginLoader.on('RTLPluginLoaded', this.#rtlPluginLoadedHandler); this.on('data', event => { if (event.dataType !== 'source' || event.sourceDataType !== 'metadata') { return; @@ -1055,7 +1055,7 @@ class Style extends Evented { } _remove() { - rtlMainThreadPluginFactory().off('RTLPluginLoaded', this.#rtlPluginLoadedHandler); + rtlPluginLoader.off('RTLPluginLoaded', this.#rtlPluginLoadedHandler); for (const id in this._sources) { this._sources[id].clearTiles(); } diff --git a/src/symbol/shaping.js b/src/symbol/shaping.js index cd797e0db..b219bf86e 100644 --- a/src/symbol/shaping.js +++ b/src/symbol/shaping.js @@ -1,4 +1,4 @@ -import { rtlWorkerPlugin } from '../source/rtl_text_plugin_worker.js'; +import { rtlPlugin } from '../source/rtl_text_plugin.js'; import { charAllowsIdeographicBreaking, charHasUprightVerticalOrientation } from '../util/script_detection.js'; import verticalizePunctuation from '../util/verticalize_punctuation.js'; import ONE_EM from './one_em.js'; @@ -112,7 +112,7 @@ export function shapeText( let lines; - const { processBidirectionalText, processStyledBidirectionalText } = rtlWorkerPlugin; + const { processBidirectionalText, processStyledBidirectionalText } = rtlPlugin; if (processBidirectionalText && logicalInput.sections.length === 1) { // Bidi doesn't have to be style-aware lines = []; diff --git a/src/symbol/transform_text.js b/src/symbol/transform_text.js index 3cecb6a4b..5e43c5275 100644 --- a/src/symbol/transform_text.js +++ b/src/symbol/transform_text.js @@ -1,4 +1,4 @@ -import { rtlWorkerPlugin } from '../source/rtl_text_plugin_worker.js'; +import { rtlPlugin } from '../source/rtl_text_plugin.js'; function transformText(text, layer, feature) { const transform = layer._layout.get('text-transform').evaluate(feature, {}); @@ -7,8 +7,8 @@ function transformText(text, layer, feature) { } else if (transform === 'lowercase') { text = text.toLocaleLowerCase(); } - if (rtlWorkerPlugin.applyArabicShaping) { - text = rtlWorkerPlugin.applyArabicShaping(text); + if (rtlPlugin.applyArabicShaping) { + text = rtlPlugin.applyArabicShaping(text); } return text; diff --git a/test/unit/source/rtl_text_plugin.test.js b/test/unit/source/rtl_text_plugin.test.js new file mode 100644 index 000000000..88bef50d9 --- /dev/null +++ b/test/unit/source/rtl_text_plugin.test.js @@ -0,0 +1,68 @@ +import test from 'node:test'; +import { rtlPlugin, rtlPluginLoader } from '../../../src/source/rtl_text_plugin.js'; +import _window from '../../util/window.js'; + +test('RTLPlugin', async t => { + let globalWindow; + t.before(() => { + globalWindow = globalThis.window; + globalThis.window = _window; + }); + t.beforeEach(() => { + // This is a static class, so we need to reset the properties before each test + rtlPluginLoader._clearRTLTextPlugin(); + }); + t.afterEach(() => { + t.mock.reset(); + }); + t.after(() => { + globalThis.window = globalWindow; + }); + + await t.test('initial state', t => { + t.assert.ok(!rtlPlugin.isRTLSupported()); + t.assert.ok(rtlPlugin.isRTLSupported(true)); + t.assert.ok(rtlPlugin.applyArabicShaping == null); + t.assert.ok(rtlPlugin.processBidirectionalText == null); + t.assert.ok(rtlPlugin.processStyledBidirectionalText == null); + }); + + await t.test('plugin loaded', t => { + const rtlTextPlugin = { + applyArabicShaping: () => {}, + processBidirectionalText: () => {}, + processStyledBidirectionalText: () => {} + }; + globalThis.registerRTLTextPlugin(rtlTextPlugin); + t.assert.ok(rtlPlugin.isRTLSupported()); + t.assert.ok(rtlPlugin.isRTLSupported(true)); + t.assert.equal(rtlPlugin.applyArabicShaping, rtlTextPlugin.applyArabicShaping); + t.assert.equal(rtlPlugin.processBidirectionalText, rtlTextPlugin.processBidirectionalText); + t.assert.equal(rtlPlugin.processStyledBidirectionalText, rtlTextPlugin.processStyledBidirectionalText); + }); + + await t.test('plugin deferred', async t => { + t.mock.method(rtlPluginLoader, '_loadScript', () => { + globalThis.registerRTLTextPlugin({}); + return Promise.resolve(); + }); + await rtlPluginLoader.setRTLTextPlugin('http://example.com/plugin', true); + t.assert.ok(!rtlPlugin.isRTLSupported()); + t.assert.ok(rtlPlugin.isRTLSupported(true)); + }); + + await t.test('plugin requested', t => { + t.assert.ok(rtlPlugin.isRTLSupported(true)); + t.assert.ok(!rtlPlugin.isRTLSupported()); + t.assert.ok(rtlPlugin.isRTLSupported(true)); + }); + + await t.test('plugin download failed', async t => { + t.mock.method(rtlPluginLoader, '_loadScript', () => Promise.reject()); + try { + await rtlPluginLoader.setRTLTextPlugin('http://example.com/plugin'); + } catch {} + t.assert.ok(!rtlPlugin.isRTLSupported()); + t.assert.ok(rtlPlugin.isRTLSupported(true)); + }); +}); diff --git a/test/unit/source/rtl_text_plugin_loader.test.js b/test/unit/source/rtl_text_plugin_loader.test.js new file mode 100644 index 000000000..d5e876d45 --- /dev/null +++ b/test/unit/source/rtl_text_plugin_loader.test.js @@ -0,0 +1,150 @@ +import test from 'node:test'; +import { rtlPlugin, rtlPluginLoader } from '../../../src/source/rtl_text_plugin.js'; +import browser from '../../../src/util/browser.js'; +import { sleep } from '../../util/util.js'; +import _window from '../../util/window.js'; + +test('RTLPluginLoader', async t => { + let globalWindow; + const url = 'http://example.com/plugin'; + t.before(() => { + globalWindow = globalThis.window; + globalThis.window = _window; + }); + t.beforeEach(() => { + // Reset the singleton instance before each test + rtlPluginLoader._clearRTLTextPlugin(); + }); + t.afterEach(() => { + t.mock.reset(); + }); + t.after(() => { + globalThis.window = globalWindow; + }); + + await t.test('should get the RTL text plugin status', t => { + const status = rtlPluginLoader.getRTLTextPluginStatus(); + t.assert.equal(status, 'unavailable'); + }); + + await t.test('should set the RTL text plugin and download it', async t => { + t.mock.method(rtlPluginLoader, '_loadScript', () => { + globalThis.registerRTLTextPlugin({}); + return Promise.resolve(); + }); + const promise = rtlPluginLoader.setRTLTextPlugin(url); + await sleep(0); + await promise; + t.assert.deepEqual(rtlPluginLoader.getRTLTextPluginStatus(), 'loaded'); + t.assert.equal(rtlPluginLoader._loadScript.mock.callCount(), 1); + }); + + await t.test('should set the RTL text plugin but defer downloading', async t => { + t.mock.method(rtlPluginLoader, '_loadScript'); + await rtlPluginLoader.setRTLTextPlugin(url, true); + t.assert.equal(rtlPluginLoader._loadScript.mock.callCount(), 0); + t.assert.equal(rtlPluginLoader.getRTLTextPluginStatus(), 'deferred'); + }); + + await t.test('should throw if the plugin is already set', async t => { + await rtlPluginLoader.setRTLTextPlugin(url, true); + await t.assert.rejects(rtlPluginLoader.setRTLTextPlugin(url), { + message: 'setRTLTextPlugin cannot be called multiple times.' + }); + }); + + await t.test('should throw if the plugin url is not set', async t => { + t.mock.method(browser, 'resolveURL', () => ''); + await t.assert.rejects(rtlPluginLoader.setRTLTextPlugin(null), { + message: 'requested url null is invalid' + }); + }); + + await t.test('should be in error state if download fails', async t => { + t.mock.method(rtlPluginLoader, '_loadScript', () => Promise.reject()); + await rtlPluginLoader.setRTLTextPlugin(url); + t.assert.equal(rtlPluginLoader.getRTLTextPluginStatus(), 'error'); + }); + + await t.test('should lazy load the plugin if deferred', async t => { + t.mock.method(rtlPluginLoader, '_loadScript', () => { + globalThis.registerRTLTextPlugin({}); + return Promise.resolve(); + }); + await rtlPluginLoader.setRTLTextPlugin(url, true); + t.assert.equal(rtlPluginLoader._loadScript.mock.callCount(), 0); + t.assert.equal(rtlPluginLoader.getRTLTextPluginStatus(), 'deferred'); + const promise = rtlPluginLoader.lazyLoad(); + await sleep(0); + await promise; + t.assert.equal(rtlPluginLoader.getRTLTextPluginStatus(), 'loaded'); + t.assert.equal(rtlPluginLoader._loadScript.mock.callCount(), 1); + }); + + await t.test('should set status to requested if RTL plugin was not set', t => { + rtlPluginLoader.lazyLoad(); + t.assert.equal(rtlPluginLoader.getRTLTextPluginStatus(), 'requested'); + }); + + await t.test('should immediately download if RTL plugin was already requested, ignoring deferred:true', async t => { + t.mock.method(rtlPluginLoader, '_loadScript', () => { + globalThis.registerRTLTextPlugin({}); + return Promise.resolve(); + }); + rtlPluginLoader.lazyLoad(); + t.assert.equal(rtlPluginLoader.getRTLTextPluginStatus(), 'requested'); + await sleep(1); + // notice even when deferred is true, it should download because already requested + await rtlPluginLoader.setRTLTextPlugin(url, true); + t.assert.equal(rtlPluginLoader.getRTLTextPluginStatus(), 'loaded'); + }); + + await t.test('should allow multiple calls to lazyLoad', t => { + rtlPluginLoader.lazyLoad(); + t.assert.equal(rtlPluginLoader.getRTLTextPluginStatus(), 'requested'); + rtlPluginLoader.lazyLoad(); + t.assert.equal(rtlPluginLoader.getRTLTextPluginStatus(), 'requested'); + }); + + await t.test('should be in error state if lazyLoad fails', async t => { + const resultPromise = rtlPluginLoader.setRTLTextPlugin(url, true); + t.assert.equal(await resultPromise, undefined); + t.assert.equal(rtlPluginLoader.getRTLTextPluginStatus(), 'deferred'); + // the next one should fail + t.mock.method(rtlPluginLoader, '_loadScript', () => Promise.reject()); + await rtlPluginLoader.lazyLoad(); + t.assert.equal(rtlPluginLoader.getRTLTextPluginStatus(), 'error'); + }); + + await t.test('should throw if already parsed', t => { + const rtlTextPlugin = { + applyArabicShaping: () => {}, + processBidirectionalText: () => {}, + processStyledBidirectionalText: () => {} + }; + globalThis.registerRTLTextPlugin(rtlTextPlugin); + t.assert.throws(() => globalThis.registerRTLTextPlugin(rtlTextPlugin), { + message: 'RTL text plugin already registered.' + }); + }); + + await t.test('should not change RTL plugin status if already parsed', t => { + const rtlTextPlugin = { + applyArabicShaping: () => {}, + processBidirectionalText: () => {}, + processStyledBidirectionalText: () => {} + }; + globalThis.registerRTLTextPlugin(rtlTextPlugin); + const rtlTextPlugin2 = { + applyArabicShaping: () => {}, + processBidirectionalText: () => {}, + processStyledBidirectionalText: () => {} + }; + try { + globalThis.registerRTLTextPlugin(rtlTextPlugin2); + } catch {} + t.assert.equal(rtlPlugin.applyArabicShaping, rtlTextPlugin.applyArabicShaping); + t.assert.equal(rtlPlugin.processBidirectionalText, rtlTextPlugin.processBidirectionalText); + t.assert.equal(rtlPlugin.processStyledBidirectionalText, rtlTextPlugin.processStyledBidirectionalText); + }); +}); diff --git a/test/unit/source/rtl_text_plugin_main_thread.test.js b/test/unit/source/rtl_text_plugin_main_thread.test.js deleted file mode 100644 index 8a1898624..000000000 --- a/test/unit/source/rtl_text_plugin_main_thread.test.js +++ /dev/null @@ -1,126 +0,0 @@ -import test from 'node:test'; -import { rtlMainThreadPluginFactory } from '../../../src/source/rtl_text_plugin_main_thread.js'; -import browser from '../../../src/util/browser.js'; -import { sleep } from '../../util/util.js'; -import _window from '../../util/window.js'; - -const rtlMainThreadPlugin = rtlMainThreadPluginFactory(); -test('RTLMainThreadPlugin', async t => { - let globalWindow; - const url = 'http://example.com/plugin'; - const failedToLoadMessage = `RTL Text Plugin failed to import scripts from ${url}`; - t.before(() => { - globalWindow = globalThis.window; - globalThis.window = _window; - }); - t.beforeEach(() => { - // Reset the singleton instance before each test - rtlMainThreadPlugin.clearRTLTextPlugin(); - }); - t.afterEach(() => { - t.mock.reset(); - }); - t.after(() => { - globalThis.window = globalWindow; - }); - - await t.test('should get the RTL text plugin status', t => { - const status = rtlMainThreadPlugin.getRTLTextPluginStatus(); - t.assert.equal(status, 'unavailable'); - }); - - await t.test('should set the RTL text plugin and download it', async t => { - t.mock.method(rtlMainThreadPlugin, 'loadScript', () => { - globalThis.registerRTLTextPlugin({}); - return Promise.resolve(); - }); - const promise = rtlMainThreadPlugin.setRTLTextPlugin(url); - await sleep(0); - await promise; - t.assert.equal(rtlMainThreadPlugin.url, url); - t.assert.equal(rtlMainThreadPlugin.loadScript.mock.callCount(), 1); - }); - - await t.test('should set the RTL text plugin but defer downloading', async t => { - t.mock.method(rtlMainThreadPlugin, 'loadScript'); - await rtlMainThreadPlugin.setRTLTextPlugin(url, true); - t.assert.equal(rtlMainThreadPlugin.loadScript.mock.callCount(), 0); - t.assert.equal(rtlMainThreadPlugin.status, 'deferred'); - }); - - await t.test('should throw if the plugin is already set', async t => { - await rtlMainThreadPlugin.setRTLTextPlugin(url, true); - await t.assert.rejects(rtlMainThreadPlugin.setRTLTextPlugin(url), { - message: 'setRTLTextPlugin cannot be called multiple times.' - }); - }); - - await t.test('should throw if the plugin url is not set', async t => { - t.mock.method(browser, 'resolveURL', () => ''); - await t.assert.rejects(rtlMainThreadPlugin.setRTLTextPlugin(null), { - message: 'requested url null is invalid' - }); - }); - - await t.test('should be in error state if download fails', async t => { - t.mock.method(rtlMainThreadPlugin, 'loadScript', () => Promise.reject()); - await t.assert.rejects(rtlMainThreadPlugin.setRTLTextPlugin(url), { - message: failedToLoadMessage - }); - t.assert.equal(rtlMainThreadPlugin.url, url); - t.assert.equal(rtlMainThreadPlugin.status, 'error'); - }); - - await t.test('should lazy load the plugin if deferred', async t => { - t.mock.method(rtlMainThreadPlugin, 'loadScript', () => { - globalThis.registerRTLTextPlugin({}); - return Promise.resolve(); - }); - await rtlMainThreadPlugin.setRTLTextPlugin(url, true); - t.assert.equal(rtlMainThreadPlugin.loadScript.mock.callCount(), 0); - t.assert.equal(rtlMainThreadPlugin.status, 'deferred'); - const promise = rtlMainThreadPlugin.lazyLoad(); - await sleep(0); - await promise; - t.assert.equal(rtlMainThreadPlugin.status, 'loaded'); - t.assert.equal(rtlMainThreadPlugin.loadScript.mock.callCount(), 1); - }); - - await t.test('should set status to requested if RTL plugin was not set', t => { - rtlMainThreadPlugin.lazyLoad(); - t.assert.equal(rtlMainThreadPlugin.status, 'requested'); - }); - - await t.test('should immediately download if RTL plugin was already requested, ignoring deferred:true', async t => { - t.mock.method(rtlMainThreadPlugin, 'loadScript', () => { - globalThis.registerRTLTextPlugin({}); - return Promise.resolve(); - }); - rtlMainThreadPlugin.lazyLoad(); - t.assert.equal(rtlMainThreadPlugin.status, 'requested'); - await sleep(1); - // notice even when deferred is true, it should download because already requested - await rtlMainThreadPlugin.setRTLTextPlugin(url, true); - t.assert.equal(rtlMainThreadPlugin.status, 'loaded'); - }); - - await t.test('should allow multiple calls to lazyLoad', t => { - rtlMainThreadPlugin.lazyLoad(); - t.assert.equal(rtlMainThreadPlugin.status, 'requested'); - rtlMainThreadPlugin.lazyLoad(); - t.assert.equal(rtlMainThreadPlugin.status, 'requested'); - }); - - await t.test('should be in error state if lazyLoad fails', async t => { - const resultPromise = rtlMainThreadPlugin.setRTLTextPlugin(url, true); - t.assert.equal(await resultPromise, undefined); - t.assert.equal(rtlMainThreadPlugin.status, 'deferred'); - // the next one should fail - t.mock.method(rtlMainThreadPlugin, 'loadScript', () => Promise.reject()); - await t.assert.rejects(rtlMainThreadPlugin.lazyLoad(), { - message: failedToLoadMessage - }); - t.assert.equal(rtlMainThreadPlugin.url, url); - t.assert.equal(rtlMainThreadPlugin.status, 'error'); - }); -}); diff --git a/test/unit/source/rtl_text_plugin_worker.test.js b/test/unit/source/rtl_text_plugin_worker.test.js deleted file mode 100644 index 791317be6..000000000 --- a/test/unit/source/rtl_text_plugin_worker.test.js +++ /dev/null @@ -1,52 +0,0 @@ -import test from 'node:test'; -import { rtlWorkerPlugin } from '../../../src/source/rtl_text_plugin_worker.js'; - -test('RTLWorkerPlugin', async t => { - t.beforeEach(() => { - // This is a static class, so we need to reset the properties before each test - rtlWorkerPlugin.processStyledBidirectionalText = null; - rtlWorkerPlugin.processBidirectionalText = null; - rtlWorkerPlugin.applyArabicShaping = null; - }); - - await t.test('should throw if already parsed', t => { - const rtlTextPlugin = { - applyArabicShaping: () => {}, - processBidirectionalText: () => {}, - processStyledBidirectionalText: () => {} - }; - rtlWorkerPlugin.setMethods(rtlTextPlugin); - t.assert.throws(() => rtlWorkerPlugin.setMethods(rtlTextPlugin), { - message: 'RTL text plugin already registered.' - }); - }); - - await t.test('should move RTL plugin from unavailable to deferred', t => { - rtlWorkerPlugin.pluginURL = ''; - rtlWorkerPlugin.pluginStatus = 'unavailable'; - const mockMessage = { - pluginURL: 'https://somehost/somescript', - pluginStatus: 'deferred' - }; - rtlWorkerPlugin.setState(mockMessage); - t.assert.equal(rtlWorkerPlugin.getRTLTextPluginStatus(), 'deferred'); - }); - - await t.test('should not change RTL plugin status if already parsed', t => { - const originalUrl = 'https://somehost/somescript1'; - rtlWorkerPlugin.pluginURL = originalUrl; - rtlWorkerPlugin.pluginStatus = 'loaded'; - rtlWorkerPlugin.setMethods({ - applyArabicShaping: () => {}, - processBidirectionalText: () => {}, - processStyledBidirectionalText: () => {} - }); - const mockMessage = { - pluginURL: 'https://somehost/somescript2', - pluginStatus: 'loading' - }; - rtlWorkerPlugin.setState(mockMessage); - t.assert.equal(rtlWorkerPlugin.getRTLTextPluginStatus(), 'loaded'); - t.assert.equal(rtlWorkerPlugin.pluginURL, originalUrl); - }); -}); diff --git a/test/unit/style/style.test.js b/test/unit/style/style.test.js index 5cf93a4c2..230d11a5c 100644 --- a/test/unit/style/style.test.js +++ b/test/unit/style/style.test.js @@ -2,7 +2,7 @@ import test from 'node:test'; import { Event, Evented } from '@mapwhit/events'; import { Color } from '@mapwhit/style-expressions'; import Transform from '../../../src/geo/transform.js'; -import { rtlMainThreadPluginFactory } from '../../../src/source/rtl_text_plugin_main_thread.js'; +import { rtlPluginLoader } from '../../../src/source/rtl_text_plugin.js'; import SourceCache from '../../../src/source/source_cache.js'; import { OverscaledTileID } from '../../../src/source/tile_id.js'; import Style from '../../../src/style/style.js'; @@ -89,7 +89,7 @@ test('Style', async t => { await style.once('style.load'); t.mock.method(style._sources.raster, 'reload'); t.mock.method(style._sources.vector, 'reload'); - rtlMainThreadPluginFactory().fire(new Event('RTLPluginLoaded')); + rtlPluginLoader.fire(new Event('RTLPluginLoaded')); t.assert.equal(style._sources.raster.reload.mock.callCount(), 0); t.assert.equal(style._sources.vector.reload.mock.callCount(), 1); }); @@ -359,13 +359,13 @@ test('Style', async t => { }); await t.test('deregisters plugin listener', async t => { - t.mock.method(rtlMainThreadPluginFactory(), 'off'); + t.mock.method(rtlPluginLoader, 'off'); const style = new Style(new StubMap()); style.loadJSON(createStyleJSON()); await style.once('style.load'); style._remove(); - t.assert.equal(rtlMainThreadPluginFactory().off.mock.callCount(), 1); + t.assert.equal(rtlPluginLoader.off.mock.callCount(), 1); }); }); diff --git a/test/util/window.js b/test/util/window.js index c1c282496..c5bfd042e 100644 --- a/test/util/window.js +++ b/test/util/window.js @@ -1,7 +1,7 @@ import canvas from 'canvas'; import gl from 'gl'; import jsdom from 'jsdom'; -import '../../src/source/rtl_text_plugin_main_thread.js'; +import '../../src/source/rtl_text_plugin.js'; const _window = create(); From 042f8d0a5f586d19c0fdfcbc7d649ed97b0d97bb Mon Sep 17 00:00:00 2001 From: melitele Date: Sat, 24 Jan 2026 15:48:03 -0700 Subject: [PATCH 12/12] remove triggering RTL plugin lazy load from tile it's already triggered when processing text for symbols --- src/source/tile.js | 6 ------ src/source/worker_tile.js | 3 --- test/unit/source/tile.test.js | 21 --------------------- 3 files changed, 30 deletions(-) diff --git a/src/source/tile.js b/src/source/tile.js index 88b57cbfe..0fc959413 100644 --- a/src/source/tile.js +++ b/src/source/tile.js @@ -11,7 +11,6 @@ import browser from '../util/browser.js'; import { deepEqual } from '../util/object.js'; import uniqueId from '../util/unique_id.js'; import GeoJSONFeature from '../util/vectortile_to_geojson.js'; -import { rtlPlugin } from './rtl_text_plugin.js'; /** * A tile object is the combination of a Coordinate, which defines @@ -32,7 +31,6 @@ class Tile { this.buckets = new Map(); this.queryPadding = 0; this.hasSymbolBuckets = false; - this.hasRTLText = false; this.state = 'loading'; } @@ -98,10 +96,6 @@ class Tile { this.hasSymbolBuckets = data.hasSymbolBuckets; this.queryPadding = data.queryPadding; - if (data.hasRTLText) { - this.hasRTLText = data.hasRTLText; - rtlPlugin.setRTLNeeded(); - } if (data.imageAtlas) { this.imageAtlas = data.imageAtlas; } diff --git a/src/source/worker_tile.js b/src/source/worker_tile.js index 6a08270ad..5f1bfe11a 100644 --- a/src/source/worker_tile.js +++ b/src/source/worker_tile.js @@ -98,7 +98,6 @@ async function finalizeBuckets(params, options, resources) { const buckets = new Map(); const { glyphAtlas, imageAtlas, glyphMap, iconMap } = await makeAtlasses(options, resources); let hasSymbolBuckets = false; - let hasRTLText = false; let queryPadding = 0; for (const bucket of uniqueBuckets.values()) { if (bucket instanceof SymbolBucket) { @@ -115,7 +114,6 @@ async function finalizeBuckets(params, options, resources) { if (justReloaded) { bucket.justReloaded = true; } - hasRTLText ||= bucket.hasRTLText; } else if ( bucket.hasPattern && (bucket instanceof LineBucket || bucket instanceof FillBucket || bucket instanceof FillExtrusionBucket) @@ -145,7 +143,6 @@ async function finalizeBuckets(params, options, resources) { glyphAtlasImage: glyphAtlas.image, imageAtlas, hasSymbolBuckets, - hasRTLText, queryPadding }; } diff --git a/test/unit/source/tile.test.js b/test/unit/source/tile.test.js index 2d29feaa2..59999cba4 100644 --- a/test/unit/source/tile.test.js +++ b/test/unit/source/tile.test.js @@ -6,7 +6,6 @@ import Context from '../../../src/gl/context.js'; import GeoJSONWrapper from '../../../src/source/geojson_wrapper.js'; import Tile from '../../../src/source/tile.js'; import { OverscaledTileID } from '../../../src/source/tile_id.js'; -import { createSymbolBucket } from '../../util/create_symbol_layer.js'; import { loadVectorTile } from '../../util/tile.js'; import _window from '../../util/window.js'; @@ -229,26 +228,6 @@ test('Tile', async t => { }); }); -test('rtl text detection', async t => { - await t.test('Tile#hasRTLText is true when a tile loads a symbol bucket with rtl text', t => { - const tile = new Tile(new OverscaledTileID(1, 0, 1, 1, 1)); - // Create a stub symbol bucket - const symbolBucket = createSymbolBucket(new CollisionBoxArray()); - // symbolBucket has not been populated yet so we force override the value in the stub - symbolBucket.hasRTLText = true; - tile.loadVectorData( - createVectorData({ vectorTile: loadVectorTile(), buckets: [symbolBucket], hasRTLText: true }), - createPainter({ - getLayer() { - return symbolBucket.layers[0]; - } - }) - ); - - t.assert.ok(tile.hasRTLText); - }); -}); - function createVectorData(options) { const collisionBoxArray = new CollisionBoxArray(); return Object.assign(