Skip to content

Commit

Permalink
Deferred loading of RTL text plugin detects labels rendered from GeoJ…
Browse files Browse the repository at this point in the history
…SON sources (#9091)
  • Loading branch information
Arindam Bose authored Dec 16, 2019
1 parent dc3cfa2 commit d9e9ffe
Show file tree
Hide file tree
Showing 7 changed files with 97 additions and 30 deletions.
4 changes: 3 additions & 1 deletion src/data/bucket/symbol_bucket.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
9 changes: 9 additions & 0 deletions src/source/rtl_text_plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,12 @@ export const plugin: {
};
}
};

export const lazyLoadRTLTextPlugin = function() {
if (!plugin.isLoading() &&
!plugin.isLoaded() &&
getRTLTextPluginStatus() === 'deferred'
) {
downloadRTLTextPlugin();
}
};
2 changes: 2 additions & 0 deletions src/source/tile.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -183,6 +184,7 @@ class Tile {
if (bucket instanceof SymbolBucket) {
if (bucket.hasRTLText) {
this.hasRTLText = true;
lazyLoadRTLTextPlugin();
break;
}
}
Expand Down
10 changes: 0 additions & 10 deletions src/source/vector_tile_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down
54 changes: 37 additions & 17 deletions test/unit/data/symbol_bucket.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'))));
Expand All @@ -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) => {
Expand Down Expand Up @@ -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();
});

28 changes: 26 additions & 2 deletions test/unit/source/tile.test.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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'));
}
Expand All @@ -276,6 +300,6 @@ function createVectorData(options) {
}, options);
}

function createPainter() {
return {style: {}};
function createPainter(styleStub = {}) {
return {style: styleStub};
}
20 changes: 20 additions & 0 deletions test/util/create_symbol_layer.js
Original file line number Diff line number Diff line change
@@ -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]
});
}

0 comments on commit d9e9ffe

Please sign in to comment.