Fix lazy load of RTL text plugin on GeoJSON sources

src/data/bucket/symbol_bucket.js

@@ -521,7 +521,9 @@ class SymbolBucket implements Bucket {
     }
 
     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() {

src/source/rtl_text_plugin.js

@@ -141,3 +141,12 @@ export const plugin: {
         };
     }
 };
+
+export const lazyLoadRTLTextPlugin = function() {
+    if (!plugin.isLoading() &&
+        !plugin.isLoaded() &&
+        getRTLTextPluginStatus() === 'deferred'
+    ) {
+        downloadRTLTextPlugin();
+    }
+};

src/source/tile.js

@@ -11,6 +11,7 @@ import Texture from '../render/texture';
 import browser from '../util/browser';
 import EvaluationParameters from '../style/evaluation_parameters';
 import SourceFeatureState from '../source/source_state';
+import {lazyLoadRTLTextPlugin} from './rtl_text_plugin';
 
 const CLOCK_SKEW_RETRY_TIMEOUT = 30000;
 
@@ -183,6 +184,7 @@ class Tile {
                 if (bucket instanceof SymbolBucket) {
                     if (bucket.hasRTLText) {
                         this.hasRTLText = true;
+                        lazyLoadRTLTextPlugin();
                         break;
                     }
                 }

src/source/vector_tile_source.js

@@ -9,7 +9,6 @@ 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, getRTLTextPluginStatus, downloadRTLTextPlugin} from './rtl_text_plugin';
 
 import type {Source} from './source';
 import type {OverscaledTileID} from './tile_id';
@@ -156,15 +155,6 @@ 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() &&
-                    getRTLTextPluginStatus() === 'deferred'
-                ) {
-                    downloadRTLTextPlugin();
-                }
-            }
 
             cacheEntryPossiblyAdded(this.dispatcher);
 

test/unit/data/symbol_bucket.test.js

@@ -5,15 +5,14 @@ import Protobuf from 'pbf';
 import {VectorTile} from '@mapbox/vector-tile';
 import SymbolBucket from '../../../src/data/bucket/symbol_bucket';
 import {CollisionBoxArray} from '../../../src/data/array_types';
-import SymbolStyleLayer from '../../../src/style/style_layer/symbol_style_layer';
-import featureFilter from '../../../src/style-spec/feature_filter';
 import {performSymbolLayout} from '../../../src/symbol/symbol_layout';
 import {Placement} from '../../../src/symbol/placement';
 import Transform from '../../../src/geo/transform';
 import {OverscaledTileID} from '../../../src/source/tile_id';
 import Tile from '../../../src/source/tile';
 import CrossTileSymbolIndex from '../../../src/symbol/cross_tile_symbol_index';
 import FeatureIndex from '../../../src/data/feature_index';
+import {createSymbolBucket} from '../../util/create_symbol_layer';
 
 // Load a point feature from fixture tile.
 const vt = new VectorTile(new Protobuf(fs.readFileSync(path.join(__dirname, '/../../fixtures/mbsv5-6-18-23.vector.pbf'))));
@@ -29,21 +28,8 @@ transform.cameraToCenterDistance = 100;
 
 const stacks = {'Test': glyphs};
 
-function bucketSetup() {
-    const layer = new SymbolStyleLayer({
-        id: 'test',
-        type: 'symbol',
-        layout: {'text-font': ['Test'], 'text-field': 'abcde'},
-        filter: featureFilter()
-    });
-    layer.recalculate({zoom: 0, zoomHistory: {}});
-
-    return new SymbolBucket({
-        overscaling: 1,
-        zoom: 0,
-        collisionBoxArray,
-        layers: [layer]
-    });
+function bucketSetup(text = 'abcde') {
+    return createSymbolBucket('test', 'Test', text, collisionBoxArray);
 }
 
 test('SymbolBucket', (t) => {
@@ -98,3 +84,37 @@ test('SymbolBucket integer overflow', (t) => {
     t.ok(console.warn.getCall(0).calledWithMatch(/Too many glyphs being rendered in a tile./));
     t.end();
 });
+
+test('SymbolBucket detects rtl text', (t) => {
+    const rtlBucket = bucketSetup('مرحبا');
+    const ltrBucket = bucketSetup('hello');
+    const options = {iconDependencies: {}, glyphDependencies: {}};
+    rtlBucket.populate([{feature}], options);
+    ltrBucket.populate([{feature}], options);
+
+    t.ok(rtlBucket.hasRTLText);
+    t.notOk(ltrBucket.hasRTLText);
+    t.end();
+});
+
+// Test to prevent symbol bucket with rtl from text being culled by worker serialization.
+test('SymbolBucket with rtl text is NOT empty even though no symbol instances are created', (t) => {
+    const rtlBucket = bucketSetup('مرحبا');
+    const options = {iconDependencies: {}, glyphDependencies: {}};
+    rtlBucket.createArrays();
+    rtlBucket.populate([{feature}], options);
+
+    t.notOk(rtlBucket.isEmpty());
+    t.equal(rtlBucket.symbolInstances.length, 0);
+    t.end();
+});
+
+test('SymbolBucket detects rtl text mixed with ltr text', (t) => {
+    const mixedBucket = bucketSetup('مرحبا translates to hello');
+    const options = {iconDependencies: {}, glyphDependencies: {}};
+    mixedBucket.populate([{feature}], options);
+
+    t.ok(mixedBucket.hasRTLText);
+    t.end();
+});
+

test/unit/source/tile.test.js

@@ -1,4 +1,5 @@
 import {test} from '../../util/test';
+import {createSymbolBucket} from '../../util/create_symbol_layer';
 import Tile from '../../../src/source/tile';
 import GeoJSONWrapper from '../../../src/source/geojson_wrapper';
 import {OverscaledTileID} from '../../../src/source/tile_id';
@@ -263,6 +264,29 @@ test('expiring tiles', (t) => {
     t.end();
 });
 
+test('rtl text detection', (t) => {
+    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('test', 'Test', 'test', new CollisionBoxArray());
+        // symbolBucket phasnt been populateed yet so we force override the value in the stub
+        symbolBucket.hasRTLText = true;
+        tile.loadVectorData(
+            createVectorData({rawTileData: createRawTileData(), buckets: [symbolBucket]}),
+            createPainter({
+                getLayer() {
+                    return symbolBucket.layers[0];
+                }
+            })
+        );
+
+        t.ok(tile.hasRTLText);
+        t.end();
+    });
+
+    t.end();
+});
+
 function createRawTileData() {
     return fs.readFileSync(path.join(__dirname, '/../../fixtures/mbsv5-6-18-23.vector.pbf'));
 }
@@ -276,6 +300,6 @@ function createVectorData(options) {
     }, options);
 }
 
-function createPainter() {
-    return {style: {}};
+function createPainter(styleStub = {}) {
+    return {style: styleStub};
 }

test/util/create_symbol_layer.js

@@ -0,0 +1,20 @@
+import SymbolBucket from '../../src/data/bucket/symbol_bucket';
+import SymbolStyleLayer from '../../src/style/style_layer/symbol_style_layer';
+import featureFilter from '../../src/style-spec/feature_filter';
+
+export function createSymbolBucket(layerId, font, text, collisionBoxArray) {
+    const layer = new SymbolStyleLayer({
+        id: layerId,
+        type: 'symbol',
+        layout: {'text-font': [font], 'text-field': text},
+        filter: featureFilter()
+    });
+    layer.recalculate({zoom: 0, zoomHistory: {}});
+
+    return new SymbolBucket({
+        overscaling: 1,
+        zoom: 0,
+        collisionBoxArray,
+        layers: [layer]
+    });
+}