From f285c5f956255a11caab41cf90bc66d1a417cff9 Mon Sep 17 00:00:00 2001 From: Arindam Bose Date: Fri, 11 Oct 2019 19:53:31 -0700 Subject: [PATCH 01/13] Autoload rtlTextPlugin using rtlTextPluginURL map option --- debug/rtl-plugin-autoload.html | 32 ++++++++++++++++++++ src/data/bucket/symbol_bucket.js | 11 ++++--- src/source/rtl_text_plugin.js | 7 ++++- src/source/tile.js | 15 +++++++++ src/source/vector_tile_source.js | 11 +++++++ src/style-spec/expression/types/formatted.js | 18 +++++++++++ src/ui/map.js | 3 ++ src/util/script_detection.js | 22 +++++++++++--- 8 files changed, 109 insertions(+), 10 deletions(-) create mode 100644 debug/rtl-plugin-autoload.html diff --git a/debug/rtl-plugin-autoload.html b/debug/rtl-plugin-autoload.html new file mode 100644 index 00000000000..46a9089aaa5 --- /dev/null +++ b/debug/rtl-plugin-autoload.html @@ -0,0 +1,32 @@ + + + + Mapbox GL JS debug page + + + + + + + +
+ + + + + + diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index b506c48f61e..8ccf3c304ed 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -303,6 +303,7 @@ class SymbolBucket implements Bucket { writingModes: Array; allowVerticalPlacement: boolean; hasPaintOverrides: boolean; + hasRTLText: boolean; constructor(options: BucketParameters) { this.collisionBoxArray = options.collisionBoxArray; @@ -315,6 +316,7 @@ class SymbolBucket implements Bucket { this.sourceLayerIndex = options.sourceLayerIndex; this.hasPattern = false; this.hasPaintOverrides = false; + this.hasRTLText = false; const layer = this.layers[0]; const unevaluatedLayoutValues = layer._unevaluatedLayout._values; @@ -407,10 +409,11 @@ class SymbolBucket implements Bucket { // but plain string token evaluation skips that pathway so do the // conversion here. const resolvedTokens = layer.getValueAndResolveTokens('text-field', feature, availableImages); - text = transformText(resolvedTokens instanceof Formatted ? - resolvedTokens : - Formatted.fromString(resolvedTokens), - layer, feature); + const formattedText = Formatted.factory(resolvedTokens); + if (formattedText.containsRTLText()) { + this.hasRTLText = true; + } + text = transformText(formattedText, layer, feature); } let icon: ResolvedImage | void; diff --git a/src/source/rtl_text_plugin.js b/src/source/rtl_text_plugin.js index e2838ae37b4..d4b1a51d5fd 100644 --- a/src/source/rtl_text_plugin.js +++ b/src/source/rtl_text_plugin.js @@ -55,7 +55,8 @@ export const plugin: { applyArabicShaping: ?Function, processBidirectionalText: ?(string, Array) => Array, processStyledBidirectionalText: ?(string, Array, Array) => Array<[string, Array]>, - isLoaded: () => boolean + isLoaded: () => boolean, + isLoading: () => boolean } = { applyArabicShaping: null, processBidirectionalText: null, @@ -63,5 +64,9 @@ export const plugin: { isLoaded() { return foregroundLoadComplete || // Foreground: loaded if the completion callback returned successfully plugin.applyArabicShaping != null; // Background: loaded if the plugin functions have been compiled + }, + isLoading() { + return pluginRequested && + plugin.applyArabicShaping == null; } }; diff --git a/src/source/tile.js b/src/source/tile.js index 9c394a5fd45..74523b0ed5b 100644 --- a/src/source/tile.js +++ b/src/source/tile.js @@ -96,6 +96,7 @@ class Tile { symbolFadeHoldUntil: ?number; hasSymbolBuckets: boolean; + hasRTLText: Boolean; /** * @param {OverscaledTileID} tileID @@ -110,6 +111,7 @@ class Tile { this.expirationTime = null; this.queryPadding = 0; this.hasSymbolBuckets = false; + this.hasRTLText = false; // Counts the number of times a response was already expired when // received. We're using this to add a delay when making a new request @@ -184,6 +186,19 @@ class Tile { } } + this.hasRTLText = false; + if (this.hasSymbolBuckets) { + for (const id in this.buckets) { + const bucket = this.buckets[id]; + if (bucket instanceof SymbolBucket) { + if (bucket.hasRTLText) { + this.hasRTLText = true; + break; + } + } + } + } + this.queryPadding = 0; for (const id in this.buckets) { const bucket = this.buckets[id]; diff --git a/src/source/vector_tile_source.js b/src/source/vector_tile_source.js index 5c8d435db64..b490cb5d0b0 100644 --- a/src/source/vector_tile_source.js +++ b/src/source/vector_tile_source.js @@ -9,6 +9,7 @@ import TileBounds from './tile_bounds'; import {ResourceType} from '../util/ajax'; import browser from '../util/browser'; import {cacheEntryPossiblyAdded} from '../util/tile_request_cache'; +import {plugin as rtlTextPlugin, setRTLTextPlugin} from './rtl_text_plugin'; import type {Source} from './source'; import type {OverscaledTileID} from './tile_id'; @@ -153,6 +154,16 @@ class VectorTileSource extends Evented implements Source { if (this.map._refreshExpiredTiles && data) tile.setExpiryData(data); tile.loadVectorData(data, this.map.painter); + if (tile.hasRTLText) { + const plugin = rtlTextPlugin; + if (!plugin.isLoading() && + !plugin.isLoaded() && + this.map != null && + this.map._rtlTextPluginURL + ) { + setRTLTextPlugin(this.map._rtlTextPluginURL); + } + } cacheEntryPossiblyAdded(this.dispatcher); diff --git a/src/style-spec/expression/types/formatted.js b/src/style-spec/expression/types/formatted.js index 0bc5000b4cc..9029a89527d 100644 --- a/src/style-spec/expression/types/formatted.js +++ b/src/style-spec/expression/types/formatted.js @@ -1,5 +1,6 @@ // @flow +import {stringContainsRTLText} from "../../../util/script_detection"; import type Color from '../../util/color'; export class FormattedSection { @@ -27,10 +28,27 @@ export default class Formatted { return new Formatted([new FormattedSection(unformatted, null, null, null)]); } + static factory(text: Formatted | string): Formatted { + if (text instanceof Formatted) { + return text; + } else { + return Formatted.fromString(text); + } + } + toString(): string { return this.sections.map(section => section.text).join(''); } + containsRTLText(): boolean { + for (const section of this.sections) { + if (stringContainsRTLText(section.text)) { + return true; + } + } + return false; + } + serialize(): Array { const serialized = ["format"]; for (const section of this.sections) { diff --git a/src/ui/map.js b/src/ui/map.js index e803bbbb65c..72f5f8a36ec 100755 --- a/src/ui/map.js +++ b/src/ui/map.js @@ -220,6 +220,7 @@ const defaultOptions = { * @param {number} [options.fadeDuration=300] Controls the duration of the fade-in/fade-out animation for label collisions, in milliseconds. This setting affects all symbol layers. This setting does not affect the duration of runtime styling transitions or raster tile cross-fading. * @param {boolean} [options.crossSourceCollisions=true] If `true`, symbols from multiple sources can collide with each other during collision detection. If `false`, collision detection is run separately for the symbols in each source. * @param {string} [options.accessToken=null] If specified, map will use this token instead of the one defined in mapboxgl.accessToken. + * @param {string} [options.rtlTextPluginURL=null] If specified, map will use this url to load the RTL text plugin, the first time RTL text is encountered. * @example * var map = new mapboxgl.Map({ @@ -278,6 +279,7 @@ class Map extends Camera { _mapId: number; _localIdeographFontFamily: string; _requestManager: RequestManager; + _rtlTextPluginURL: ?string; /** * The map's {@link ScrollZoomHandler}, which implements zooming in and out with a scroll wheel or trackpad. @@ -344,6 +346,7 @@ class Map extends Camera { this._crossSourceCollisions = options.crossSourceCollisions; this._crossFadingFactor = 1; this._collectResourceTiming = options.collectResourceTiming; + this._rtlTextPluginURL = options.rtlTextPluginURL; this._renderTaskQueue = new TaskQueue(); this._controls = []; this._mapId = uniqueId(); diff --git a/src/util/script_detection.js b/src/util/script_detection.js index 5ae5b933ef5..985ae483001 100644 --- a/src/util/script_detection.js +++ b/src/util/script_detection.js @@ -277,6 +277,13 @@ export function charInComplexShapingScript(char: number) { isChar['Arabic Presentation Forms-B'](char); } +export function charInRTLScript(char: number) { + // 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: number, canRenderRTL: boolean) { // This is a rough heuristic: whether we "can render" a script // actually depends on the properties of the font being used @@ -285,11 +292,7 @@ export function charInSupportedScript(char: number, canRenderRTL: boolean) { // 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 ((char >= 0x0900 && char <= 0x0DFF) || @@ -306,6 +309,15 @@ export function charInSupportedScript(char: number, canRenderRTL: boolean) { return true; } +export function stringContainsRTLText(chars: string): boolean { + for (const char of chars) { + if (charInRTLScript(char.charCodeAt(0))) { + return true; + } + } + return false; +} + export function isStringInSupportedScript(chars: string, canRenderRTL: boolean) { for (const char of chars) { if (!charInSupportedScript(char.charCodeAt(0), canRenderRTL)) { From 7b55f598b6b10d1007ffde3ded73ddde6e15343a Mon Sep 17 00:00:00 2001 From: Arindam Bose Date: Sat, 12 Oct 2019 13:11:48 -0700 Subject: [PATCH 02/13] fix flow errors --- src/source/rtl_text_plugin.js | 2 +- src/source/tile.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/source/rtl_text_plugin.js b/src/source/rtl_text_plugin.js index d4b1a51d5fd..cdf0c564288 100644 --- a/src/source/rtl_text_plugin.js +++ b/src/source/rtl_text_plugin.js @@ -30,7 +30,7 @@ export const clearRTLTextPlugin = function() { pluginURL = null; }; -export const setRTLTextPlugin = function(url: string, callback: ErrorCallback) { +export const setRTLTextPlugin = function(url: string, callback: ?ErrorCallback) { if (pluginRequested) { throw new Error('setRTLTextPlugin cannot be called multiple times.'); } diff --git a/src/source/tile.js b/src/source/tile.js index 74523b0ed5b..e714007e2e1 100644 --- a/src/source/tile.js +++ b/src/source/tile.js @@ -96,7 +96,7 @@ class Tile { symbolFadeHoldUntil: ?number; hasSymbolBuckets: boolean; - hasRTLText: Boolean; + hasRTLText: boolean; /** * @param {OverscaledTileID} tileID From ad15029678f3a3a0bc1e2818debee90f3dfd6085 Mon Sep 17 00:00:00 2001 From: Arindam Bose Date: Tue, 15 Oct 2019 11:45:22 -0700 Subject: [PATCH 03/13] Skip rendering rtl text if plugin is not loaded --- src/data/bucket/symbol_bucket.js | 5 ++++- src/source/worker.js | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index 8ccf3c304ed..8f94c54e0ae 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -37,6 +37,7 @@ import {register} from '../../util/web_worker_transfer'; import EvaluationParameters from '../../style/evaluation_parameters'; import Formatted from '../../style-spec/expression/types/formatted'; import ResolvedImage from '../../style-spec/expression/types/resolved_image'; +import {plugin as globalRTLTextPlugin} from '../../source/rtl_text_plugin'; import type { Bucket, @@ -413,7 +414,9 @@ class SymbolBucket implements Bucket { if (formattedText.containsRTLText()) { this.hasRTLText = true; } - text = transformText(formattedText, layer, feature); + if (this.hasRTLText && globalRTLTextPlugin.isLoaded() || !this.hasRTLText) { + text = transformText(formattedText, layer, feature); + } } let icon: ResolvedImage | void; diff --git a/src/source/worker.js b/src/source/worker.js index 7d6f494b58c..d0cb3cce35b 100644 --- a/src/source/worker.js +++ b/src/source/worker.js @@ -59,6 +59,7 @@ export default class Worker { this.workerSourceTypes[name] = WorkerSource; }; + // This is invoked by the RTL text plugin when the download via the `importScripts` call has finished, and the code has been parsed. this.self.registerRTLTextPlugin = (rtlTextPlugin: {applyArabicShaping: Function, processBidirectionalText: Function, processStyledBidirectionalText?: Function}) => { if (globalRTLTextPlugin.isLoaded()) { throw new Error('RTL text plugin already registered.'); From 51158d95ef32e27ebe61ee583d699ea2341f256c Mon Sep 17 00:00:00 2001 From: Arindam Bose Date: Tue, 15 Oct 2019 19:05:31 -0700 Subject: [PATCH 04/13] diferred laoding of plugin while not rendering text --- debug/rtl-plugin-autoload.html | 4 +- src/data/bucket/symbol_bucket.js | 6 +- src/source/rtl_text_plugin.js | 99 +++++++++++++++++++++++++++----- src/source/vector_tile_source.js | 7 +-- src/source/worker.js | 18 ++++-- src/style/style.js | 8 ++- src/ui/map.js | 1 - 7 files changed, 111 insertions(+), 32 deletions(-) diff --git a/debug/rtl-plugin-autoload.html b/debug/rtl-plugin-autoload.html index 46a9089aaa5..aea2d42cfa8 100644 --- a/debug/rtl-plugin-autoload.html +++ b/debug/rtl-plugin-autoload.html @@ -17,14 +17,14 @@ diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index 515c3434341..c5333ad2543 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -415,7 +415,11 @@ class SymbolBucket implements Bucket { this.hasRTLText = true; } - if (this.hasRTLText && globalRTLTextPlugin.isLoaded() || !this.hasRTLText) { + if ( + this.hasRTLText && globalRTLTextPlugin.isLoaded() || // Use the rtlText plugin shape text + !this.hasRTLText || // non-rtl terxt so can proceed safely + !globalRTLTextPlugin.isAvailableInWorker() // We-doent intend to async-load the rtl text plugin, so proceed with incorrect shaping + ) { text = transformText(formattedText, layer, feature); } } diff --git a/src/source/rtl_text_plugin.js b/src/source/rtl_text_plugin.js index 5c748d5662b..668b4d604c2 100644 --- a/src/source/rtl_text_plugin.js +++ b/src/source/rtl_text_plugin.js @@ -1,21 +1,42 @@ // @flow import {Event, Evented} from '../util/evented'; +import {getArrayBuffer} from '../util/ajax'; import browser from '../util/browser'; +import window from '../util/window'; const status = { - unavailable: 'unavailable', - loading: 'loading', + unavailable: 'unavailable', // Not loaded + available: 'available', // Host url specified, but we havent started loading yet + loading: 'loading', // request in-flight + downloaded: 'downloaded', //plugin loaded and cached on main-thread and pluginBlobURL for worker is generated loaded: 'loaded', error: 'error' }; let pluginStatus = status.unavailable; let pluginURL = null; +let pluginBlobURL = null; +let lazy = null; + +// store `pluginAvailable` that have occurred before the `registerPluginAvailability` bind +// so we can flush all the state updates to the workers +let eventQueue = []; +let _pluginAvailableCb = null; + +let _workerAvailable = false; export const evented = new Evented(); +evented.on('pluginAvailable', (args) => { + if (!_pluginAvailableCb) { + eventQueue.push(args); + } else { + _pluginAvailableCb(args); + } +}); type CompletionCallback = (error?: Error) => void; type ErrorCallback = (error: Error) => void; +type PluginAvailableCallback = (args: {pluginURL: ?string, lazy: ?boolean, completionCallback: CompletionCallback}) => void; let _completionCallback; @@ -23,28 +44,30 @@ export const getRTLTextPluginStatus = function () { return pluginStatus; }; -export const registerForPluginAvailability = function( - callback: (args: {pluginURL: string, completionCallback: CompletionCallback}) => void -) { - if (pluginURL) { - callback({pluginURL, completionCallback: _completionCallback}); - } else { - evented.once('pluginAvailable', callback); +export const registerForPluginAvailability = function(callback: PluginAvailableCallback) { + for (const event of eventQueue) { + callback(event); } + eventQueue = []; + + _pluginAvailableCb = callback; return callback; }; export const clearRTLTextPlugin = function() { pluginStatus = status.unavailable; pluginURL = null; + pluginBlobURL = null; + lazy = null; }; -export const setRTLTextPlugin = function(url: string, callback: ?ErrorCallback, lazy: ?boolean) { - if (pluginStatus === status.loading || pluginStatus === status.loaded) { +export const setRTLTextPlugin = function(url: string, callback: ?ErrorCallback, lazyLoad: ?boolean) { + if (pluginStatus === status.available || pluginStatus === status.loading || pluginStatus === status.loaded) { throw new Error('setRTLTextPlugin cannot be called multiple times.'); } - pluginStatus = status.loading; pluginURL = browser.resolveURL(url); + lazy = !!lazyLoad; + pluginStatus = status.available; _completionCallback = (error?: Error) => { if (error) { // Clear loaded state to allow retries @@ -55,10 +78,48 @@ export const setRTLTextPlugin = function(url: string, callback: ?ErrorCallback, } } else { // Called once for each worker - pluginStatus = status.loaded; + if (!lazy) { + pluginStatus = status.loaded; + } } }; - evented.fire(new Event('pluginAvailable', {pluginURL, completionCallback: _completionCallback})); + + if (lazy) { + // Inform the worker-threads that we intend to load the plugin lazily later, + // This is so the workers can skip RTL text parsing. + evented.fire(new Event('pluginAvailable', { + pluginURL: null, + lazy, + completionCallback: _completionCallback + })); + } else { + downloadRTLTextPlugin(); + } +}; + +export const downloadRTLTextPlugin = function() { + if (pluginStatus !== status.available) { + throw new Error('rtl-text-plugin cannot be downloaded unless a pluginURL is specified'); + } + pluginStatus = status.loading; + + if (pluginURL) { + getArrayBuffer({url: pluginURL}, (error, data) => { + if (error || !data) { + throw error; + } else { + const rtlBlob = new window.Blob([data], {type: 'application/javascript'}); + const URL = window.URL || window.webkitURL; + pluginBlobURL = URL.createObjectURL(rtlBlob); + pluginStatus = status.downloaded; + evented.fire(new Event('pluginAvailable', { + pluginURL: pluginBlobURL, + lazy, + completionCallback: _completionCallback + })); + } + }); + } }; export const plugin: { @@ -66,7 +127,9 @@ export const plugin: { processBidirectionalText: ?(string, Array) => Array, processStyledBidirectionalText: ?(string, Array, Array) => Array<[string, Array]>, isLoaded: () => boolean, - isLoading: () => boolean + isLoading: () => boolean, + markWorkerAvailable: () => void, + isAvailableInWorker: () => boolean } = { applyArabicShaping: null, processBidirectionalText: null, @@ -77,5 +140,11 @@ export const plugin: { }, isLoading() { // Main Thread Only: query the loading status, this function does not return the correct value in the worker context. return pluginStatus === status.loading; + }, + markWorkerAvailable() { // Worker thread only: this tells the worker threads that the plugin is available on the Main thread + _workerAvailable = true; + }, + isAvailableInWorker() { + return !!_workerAvailable; } }; diff --git a/src/source/vector_tile_source.js b/src/source/vector_tile_source.js index b490cb5d0b0..c9984a9a85b 100644 --- a/src/source/vector_tile_source.js +++ b/src/source/vector_tile_source.js @@ -9,7 +9,7 @@ import TileBounds from './tile_bounds'; import {ResourceType} from '../util/ajax'; import browser from '../util/browser'; import {cacheEntryPossiblyAdded} from '../util/tile_request_cache'; -import {plugin as rtlTextPlugin, setRTLTextPlugin} from './rtl_text_plugin'; +import {plugin as rtlTextPlugin, getRTLTextPluginStatus, downloadRTLTextPlugin} from './rtl_text_plugin'; import type {Source} from './source'; import type {OverscaledTileID} from './tile_id'; @@ -158,10 +158,9 @@ class VectorTileSource extends Evented implements Source { const plugin = rtlTextPlugin; if (!plugin.isLoading() && !plugin.isLoaded() && - this.map != null && - this.map._rtlTextPluginURL + getRTLTextPluginStatus() === 'available' ) { - setRTLTextPlugin(this.map._rtlTextPluginURL); + downloadRTLTextPlugin(); } } diff --git a/src/source/worker.js b/src/source/worker.js index d0cb3cce35b..4e9074c8b40 100644 --- a/src/source/worker.js +++ b/src/source/worker.js @@ -152,13 +152,19 @@ export default class Worker { } } - loadRTLTextPlugin(map: string, pluginURL: string, callback: Callback) { + loadRTLTextPlugin(map: string, pluginData: Object, callback: Callback) { try { - if (!globalRTLTextPlugin.isLoaded()) { - this.self.importScripts(pluginURL); - callback(globalRTLTextPlugin.isLoaded() ? - null : - new Error(`RTL Text Plugin failed to import scripts from ${pluginURL}`)); + const {pluginURL, lazy} = pluginData; + if (pluginURL) { + if (!globalRTLTextPlugin.isLoaded()) { + this.self.importScripts(pluginURL); + callback(globalRTLTextPlugin.isLoaded() ? + null : + new Error(`RTL Text Plugin failed to import scripts from ${pluginURL}`)); + } + } else if (lazy) { + // Set the state of the rtl text plugin in worker scope, to load the plugin if necessary. + globalRTLTextPlugin.markWorkerAvailable(); } } catch (e) { callback(e.toString()); diff --git a/src/style/style.js b/src/style/style.js index d70f329782c..6a549986978 100644 --- a/src/style/style.js +++ b/src/style/style.js @@ -154,9 +154,11 @@ class Style extends Evented { const self = this; this._rtlTextPluginCallback = Style.registerForPluginAvailability((args) => { - self.dispatcher.broadcast('loadRTLTextPlugin', args.pluginURL, args.completionCallback); - for (const id in self.sourceCaches) { - self.sourceCaches[id].reload(); // Should be a no-op if the plugin loads before any tiles load + self.dispatcher.broadcast('loadRTLTextPlugin', {pluginURL: args.pluginURL, lazy: args.lazy}, args.completionCallback); + if (args.pluginURL) { + for (const id in self.sourceCaches) { + self.sourceCaches[id].reload(); // Should be a no-op if the plugin loads before any tiles load + } } }); diff --git a/src/ui/map.js b/src/ui/map.js index 72f5f8a36ec..6eaec289f11 100755 --- a/src/ui/map.js +++ b/src/ui/map.js @@ -220,7 +220,6 @@ const defaultOptions = { * @param {number} [options.fadeDuration=300] Controls the duration of the fade-in/fade-out animation for label collisions, in milliseconds. This setting affects all symbol layers. This setting does not affect the duration of runtime styling transitions or raster tile cross-fading. * @param {boolean} [options.crossSourceCollisions=true] If `true`, symbols from multiple sources can collide with each other during collision detection. If `false`, collision detection is run separately for the symbols in each source. * @param {string} [options.accessToken=null] If specified, map will use this token instead of the one defined in mapboxgl.accessToken. - * @param {string} [options.rtlTextPluginURL=null] If specified, map will use this url to load the RTL text plugin, the first time RTL text is encountered. * @example * var map = new mapboxgl.Map({ From 8e03389baa2dfe16f17e20f7614712a70a7e0fe3 Mon Sep 17 00:00:00 2001 From: Arindam Bose Date: Thu, 17 Oct 2019 21:48:34 -0700 Subject: [PATCH 05/13] Rearchitected rtlTextPlugin -> worker messaging. - main thread now pings workers with an `pluginStateChange` event that syncs the state of the plugin on the main thread with the worker - worker logic is now reactive to this state change, and will load and parse the plugin on appropriate state change --- debug/rtl-plugin-autoload.html | 6 +- src/data/bucket/symbol_bucket.js | 9 +- src/source/rtl_text_plugin.js | 138 +++++++++++++++---------------- src/source/worker.js | 27 +++--- src/style/style.js | 34 +++++--- src/util/ajax.js | 9 +- src/util/util.js | 12 +++ src/util/window.js | 9 ++ test/unit/style/style.test.js | 29 ++++--- 9 files changed, 155 insertions(+), 118 deletions(-) diff --git a/debug/rtl-plugin-autoload.html b/debug/rtl-plugin-autoload.html index aea2d42cfa8..7dd3e71d538 100644 --- a/debug/rtl-plugin-autoload.html +++ b/debug/rtl-plugin-autoload.html @@ -17,7 +17,11 @@ - - - - diff --git a/debug/rtl.html b/debug/rtl.html index 6257d484629..b589aafe2dd 100644 --- a/debug/rtl.html +++ b/debug/rtl.html @@ -8,16 +8,29 @@
+
+
+
+ Plugin Status:
+ +
diff --git a/src/index.js b/src/index.js index bc9fe4387ab..aae65837d20 100644 --- a/src/index.js +++ b/src/index.js @@ -158,6 +158,8 @@ const exported = { * @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 downloading. * @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/) From 05db0654b0f6f03c171d73675dd109b725bd383f Mon Sep 17 00:00:00 2001 From: Arindam Bose Date: Fri, 18 Oct 2019 16:53:38 -0700 Subject: [PATCH 09/13] Lint --- debug/rtl.html | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/debug/rtl.html b/debug/rtl.html index b589aafe2dd..162eeb500f0 100644 --- a/debug/rtl.html +++ b/debug/rtl.html @@ -41,17 +41,17 @@ document.getElementById('loadSync').onclick = function() { mapboxgl.setRTLTextPlugin(pluginURL); -} +}; document.getElementById('loadAsync').onclick = function() { - mapboxgl.setRTLTextPlugin(pluginURL, function(err){ - if(err){ + mapboxgl.setRTLTextPlugin(pluginURL, function(err) { + if (err) { throw err; } else { console.log('rtl-text-plugin loaded successfully'); } }, true); -} +}; setInterval(function() { document.getElementById('pluginStatus').innerHTML = mapboxgl.getRTLTextPluginStatus(); From 89b3bcb0d3d304ca07c73c1a918863065fa902d9 Mon Sep 17 00:00:00 2001 From: Arindam Bose Date: Tue, 22 Oct 2019 17:06:47 -0700 Subject: [PATCH 10/13] Apply comments change suggestions from code review Co-Authored-By: Asheem Mamoowala --- src/data/bucket/symbol_bucket.js | 2 +- src/index.js | 2 +- src/source/rtl_text_plugin.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/data/bucket/symbol_bucket.js b/src/data/bucket/symbol_bucket.js index 4b07b543fa4..83cd04879fe 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/src/data/bucket/symbol_bucket.js @@ -417,7 +417,7 @@ class SymbolBucket implements Bucket { 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 - this.hasRTLText && globalRTLTextPlugin.isParsed() // Use the rtlText plugin shape text + this.hasRTLText && globalRTLTextPlugin.isParsed() // Use the rtlText plugin to shape text ) { text = transformText(formattedText, layer, feature); } diff --git a/src/index.js b/src/index.js index aae65837d20..ccd3ca97b01 100644 --- a/src/index.js +++ b/src/index.js @@ -159,7 +159,7 @@ const exported = { * @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 downloading. + * 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 6ab516734ec..d9162becd33 100644 --- a/src/source/rtl_text_plugin.js +++ b/src/source/rtl_text_plugin.js @@ -9,7 +9,7 @@ import {isWorker} from '../util/util'; const status = { unavailable: 'unavailable', // Not loaded - available: 'available', // Host url specified, but we havent started loading yet + available: 'available', // The plugin URL has been specified, but loading has been deferred loading: 'loading', // request in-flight loaded: 'loaded', error: 'error' From 247226e6f68a557cd294513cd1f40b226cf95a28 Mon Sep 17 00:00:00 2001 From: Arindam Bose Date: Tue, 22 Oct 2019 17:47:29 -0700 Subject: [PATCH 11/13] Switch to assert instead of throwing error --- src/source/rtl_text_plugin.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/source/rtl_text_plugin.js b/src/source/rtl_text_plugin.js index d9162becd33..a700df7eeaf 100644 --- a/src/source/rtl_text_plugin.js +++ b/src/source/rtl_text_plugin.js @@ -120,9 +120,7 @@ export const plugin: { return pluginStatus === status.loading; }, setState(state: PluginState) { // Worker thread only: this tells the worker threads that the plugin is available on the Main thread - if (!isWorker()) { - throw new Error('Cannot set the state of the rtl-text-plugin when not in the web-worker context'); - } + assert(isWorker(), 'Cannot set the state of the rtl-text-plugin when not in the web-worker context'); pluginStatus = state.pluginStatus; pluginURL = state.pluginURL; From edc94ad4f91718a518e000f8fbe9f56d48bcaacd Mon Sep 17 00:00:00 2001 From: Arindam Bose Date: Thu, 24 Oct 2019 13:11:16 -0700 Subject: [PATCH 12/13] Remove webkitURL and switch to using deferred --- src/source/rtl_text_plugin.js | 19 +++++++++---------- src/source/vector_tile_source.js | 2 +- src/types/window.js | 1 - src/util/ajax.js | 5 ++--- 4 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/source/rtl_text_plugin.js b/src/source/rtl_text_plugin.js index a700df7eeaf..8681a98536f 100644 --- a/src/source/rtl_text_plugin.js +++ b/src/source/rtl_text_plugin.js @@ -9,11 +9,12 @@ import {isWorker} from '../util/util'; const status = { unavailable: 'unavailable', // Not loaded - available: 'available', // The plugin URL has been specified, but loading has been deferred + deferred: 'deferred', // The plugin URL has been specified, but loading has been deferred loading: 'loading', // request in-flight loaded: 'loaded', error: 'error' }; + export type PluginState = { pluginStatus: $Values; pluginURL: ?string, @@ -57,29 +58,28 @@ export const clearRTLTextPlugin = function() { pluginStatus = status.unavailable; pluginURL = null; if (pluginBlobURL) { - const URL = window.URL || window.webkitURL; - URL.revokeObjectURL(pluginBlobURL); + window.URL.revokeObjectURL(pluginBlobURL); } pluginBlobURL = null; }; -export const setRTLTextPlugin = function(url: string, callback: ?ErrorCallback, lazy: boolean = false) { - if (pluginStatus === status.available || pluginStatus === status.loading || pluginStatus === status.loaded) { +export const setRTLTextPlugin = function(url: string, callback: ?ErrorCallback, deferred: boolean = 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.available; + pluginStatus = status.deferred; _completionCallback = callback; sendPluginStateToWorker(); //Start downloading the plugin immediately if not intending to lazy-load - if (!lazy) { + if (!deferred) { downloadRTLTextPlugin(); } }; export const downloadRTLTextPlugin = function() { - if (pluginStatus !== status.available || !pluginURL) { + if (pluginStatus !== status.deferred || !pluginURL) { throw new Error('rtl-text-plugin cannot be downloaded unless a pluginURL is specified'); } pluginStatus = status.loading; @@ -90,8 +90,7 @@ export const downloadRTLTextPlugin = function() { triggerPluginCompletionEvent(error); } else { const rtlBlob = new window.Blob([data], {type: 'application/javascript'}); - const URL = window.URL || window.webkitURL; - pluginBlobURL = URL.createObjectURL(rtlBlob); + pluginBlobURL = window.URL.createObjectURL(rtlBlob); pluginStatus = status.loaded; sendPluginStateToWorker(); } diff --git a/src/source/vector_tile_source.js b/src/source/vector_tile_source.js index c9984a9a85b..957c2ef6b72 100644 --- a/src/source/vector_tile_source.js +++ b/src/source/vector_tile_source.js @@ -158,7 +158,7 @@ class VectorTileSource extends Evented implements Source { const plugin = rtlTextPlugin; if (!plugin.isLoading() && !plugin.isLoaded() && - getRTLTextPluginStatus() === 'available' + getRTLTextPluginStatus() === 'deferred' ) { downloadRTLTextPlugin(); } diff --git a/src/types/window.js b/src/types/window.js index 27ed038086c..8957156fcc7 100644 --- a/src/types/window.js +++ b/src/types/window.js @@ -128,7 +128,6 @@ export interface Window extends EventTarget, IDBEnvironment { URL: typeof URL; URLSearchParams: typeof URLSearchParams; WebGLFramebuffer: typeof WebGLFramebuffer; - webkitURL: typeof URL; WheelEvent: typeof WheelEvent; Worker: typeof Worker; XMLHttpRequest: typeof XMLHttpRequest; diff --git a/src/util/ajax.js b/src/util/ajax.js index 9c90d0d411f..23a8cde1399 100644 --- a/src/util/ajax.js +++ b/src/util/ajax.js @@ -299,16 +299,15 @@ export const getImage = function(requestParameters: RequestParameters, callback: callback(err); } else if (data) { const img: HTMLImageElement = new window.Image(); - const URL = window.URL || window.webkitURL; img.onload = () => { callback(null, img); - URL.revokeObjectURL(img.src); + window.URL.revokeObjectURL(img.src); }; img.onerror = () => callback(new Error('Could not load image. Please make sure to use a supported image type such as PNG or JPEG. Note that SVGs are not supported.')); const blob: Blob = new window.Blob([new Uint8Array(data)], {type: 'image/png'}); (img: any).cacheControl = cacheControl; (img: any).expires = expires; - img.src = data.byteLength ? URL.createObjectURL(blob) : transparentPngUrl; + img.src = data.byteLength ? window.URL.createObjectURL(blob) : transparentPngUrl; } }); From 767c9ddf9028db405e4b6024e51c52d643daf305 Mon Sep 17 00:00:00 2001 From: Arindam Bose Date: Thu, 24 Oct 2019 14:25:26 -0700 Subject: [PATCH 13/13] Fix unit test --- test/unit/style/style.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/style/style.test.js b/test/unit/style/style.test.js index a2097806bf2..b3ab8f23dc6 100644 --- a/test/unit/style/style.test.js +++ b/test/unit/style/style.test.js @@ -73,7 +73,7 @@ test('Style', (t) => { setRTLTextPlugin("/plugin.js",); t.ok(style.dispatcher.broadcast.calledWith('syncRTLPluginState', { - pluginStatus: 'available', + pluginStatus: 'deferred', pluginURL: "/plugin.js", pluginBlobURL: null }));