Skip to content

Commit

Permalink
Fix race condition causing SymbolBucket isEmpty errors (#3681)
Browse files Browse the repository at this point in the history
* Fix race condition causing SymbolBucket isEmpty errors: don't let workerTile reparse while in the middle of parsing for a given tile
* Remove unnecessary redoPlacementAfterDone concept, move collisionTile to be more localized since it is not needed in featureIndex
* Create CollisionTile only after async glyph/icon returns to ensure it uses the most up-to-date angle and pitch
* Set this.angle and this.pitch in redoPlacement
  • Loading branch information
Lauren Budorick authored Dec 5, 2016
1 parent cedce50 commit 9e2e4bd
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 23 deletions.
21 changes: 19 additions & 2 deletions js/source/vector_tile_worker_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,27 @@ class VectorTileWorkerSource {
*/
reloadTile(params, callback) {
const loaded = this.loaded[params.source],
uid = params.uid;
uid = params.uid,
vtSource = this;
if (loaded && loaded[uid]) {
const workerTile = loaded[uid];
workerTile.parse(workerTile.vectorTile, this.layerIndex, this.actor, callback);

if (workerTile.status === 'parsing') {
workerTile.reloadCallback = callback;
} else if (workerTile.status === 'done') {
workerTile.parse(workerTile.vectorTile, this.layerIndex, this.actor, done.bind(workerTile));
}

}

function done(err, data) {
if (this.reloadCallback) {
const reloadCallback = this.reloadCallback;
delete this.reloadCallback;
this.parse(this.vectorTile, vtSource.layerIndex, vtSource.actor, reloadCallback);
}

callback(err, data);
}
}

Expand Down
42 changes: 21 additions & 21 deletions js/source/worker_tile.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,9 @@ class WorkerTile {
this.collisionBoxArray = new CollisionBoxArray();
this.symbolInstancesArray = new SymbolInstancesArray();
this.symbolQuadsArray = new SymbolQuadsArray();
const collisionTile = new CollisionTile(this.angle, this.pitch, this.collisionBoxArray);
const sourceLayerCoder = new DictionaryCoder(Object.keys(data.layers).sort());

const featureIndex = new FeatureIndex(this.coord, this.overscaling, collisionTile, data.layers);
const featureIndex = new FeatureIndex(this.coord, this.overscaling);
featureIndex.bucketLayerIDs = {};

const buckets = {};
Expand Down Expand Up @@ -103,13 +102,9 @@ class WorkerTile {
}
}

const done = () => {
this.status = 'done';

if (this.redoPlacementAfterDone) {
this.redoPlacement(this.angle, this.pitch, null);
this.redoPlacementAfterDone = false;
}
const done = (collisionTile) => {
this.status = 'done';

const transferables = [];
callback(null, {
Expand All @@ -132,7 +127,7 @@ class WorkerTile {
}

if (this.symbolBuckets.length === 0) {
return done();
return done(new CollisionTile(this.angle, this.pitch, this.collisionBoxArray));
}

let deps = 0;
Expand All @@ -143,16 +138,16 @@ class WorkerTile {
if (err) return callback(err);
deps++;
if (deps === 2) {
const collisionTile = new CollisionTile(this.angle, this.pitch, this.collisionBoxArray);

for (const bucket of this.symbolBuckets) {
// Layers are shared and may have been used by a WorkerTile with a different zoom.
for (const layer of bucket.layers) {
layer.recalculate(this.zoom);
}
recalculateLayers(bucket, this.zoom);

bucket.prepare(stacks, icons);
bucket.place(collisionTile, this.showCollisionBoxes);
}
done();

done(collisionTile);
}
};

Expand All @@ -176,19 +171,17 @@ class WorkerTile {
}

redoPlacement(angle, pitch, showCollisionBoxes) {
this.angle = angle;
this.pitch = pitch;

if (this.status !== 'done') {
this.redoPlacementAfterDone = true;
this.angle = angle;
return {};
}

const collisionTile = new CollisionTile(angle, pitch, this.collisionBoxArray);
const collisionTile = new CollisionTile(this.angle, this.pitch, this.collisionBoxArray);

for (const bucket of this.symbolBuckets) {
// Layers are shared and may have been used by a WorkerTile with a different zoom.
for (const layer of bucket.layers) {
layer.recalculate(this.zoom);
}
recalculateLayers(bucket, this.zoom);

bucket.place(collisionTile, showCollisionBoxes);
}
Expand All @@ -204,6 +197,13 @@ class WorkerTile {
}
}

function recalculateLayers(bucket, zoom) {
// Layers are shared and may have been used by a WorkerTile with a different zoom.
for (const layer of bucket.layers) {
layer.recalculate(zoom);
}
}

function serializeBuckets(buckets, transferables) {
return buckets
.filter((b) => !b.isEmpty())
Expand Down

0 comments on commit 9e2e4bd

Please sign in to comment.