From d9e9ffe39a823b2f4a790b805b4fd2859c363cc4 Mon Sep 17 00:00:00 2001 From: Arindam Bose Date: Mon, 16 Dec 2019 12:49:01 -0800 Subject: [PATCH] Deferred loading of RTL text plugin detects labels rendered from GeoJSON sources (#9091) --- src/data/bucket/symbol_bucket.js | 4 ++- src/source/rtl_text_plugin.js | 9 +++++ src/source/tile.js | 2 ++ src/source/vector_tile_source.js | 10 ------ test/unit/data/symbol_bucket.test.js | 54 +++++++++++++++++++--------- test/unit/source/tile.test.js | 28 +++++++++++++-- test/util/create_symbol_layer.js | 20 +++++++++++ 7 files changed, 97 insertions(+), 30 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 728d6ee1056..a034b7ce431 100644 --- a/src/data/bucket/symbol_bucket.js +++ b/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() { diff --git a/src/source/rtl_text_plugin.js b/src/source/rtl_text_plugin.js index 8681a98536f..fbd1b7fd3fa 100644 --- a/src/source/rtl_text_plugin.js +++ b/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(); + } +}; diff --git a/src/source/tile.js b/src/source/tile.js index 9602db15b43..d1d502cbf11 100644 --- a/src/source/tile.js +++ b/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; } } diff --git a/src/source/vector_tile_source.js b/src/source/vector_tile_source.js index 5ec313a6264..1554b45bd1d 100644 --- a/src/source/vector_tile_source.js +++ b/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); diff --git a/test/unit/data/symbol_bucket.test.js b/test/unit/data/symbol_bucket.test.js index 5a1355c969b..bb830a5b27c 100644 --- a/test/unit/data/symbol_bucket.test.js +++ b/test/unit/data/symbol_bucket.test.js @@ -5,8 +5,6 @@ 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'; @@ -14,6 +12,7 @@ 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(); +}); + diff --git a/test/unit/source/tile.test.js b/test/unit/source/tile.test.js index e36b2994892..11f6f3b8420 100644 --- a/test/unit/source/tile.test.js +++ b/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 has not been populated 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}; } diff --git a/test/util/create_symbol_layer.js b/test/util/create_symbol_layer.js new file mode 100644 index 00000000000..7ddde7b7101 --- /dev/null +++ b/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] + }); +}