Skip to content

Commit

Permalink
Make symbol query result sort order match render order for overlappin…
Browse files Browse the repository at this point in the history
…g symbols.

Fixes issue #6184.
  • Loading branch information
ChrisLoer committed Apr 13, 2018
1 parent 9bb1d9e commit 6997943
Show file tree
Hide file tree
Showing 6 changed files with 1,415 additions and 4 deletions.
4 changes: 4 additions & 0 deletions src/data/bucket/symbol_bucket.js
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ class SymbolBucket implements Bucket {
fadeStartTime: number;
sortFeaturesByY: boolean;
sortedAngle: number;
featureSortOrder: Array<number>;

text: SymbolBuffers;
icon: SymbolBuffers;
Expand Down Expand Up @@ -656,8 +657,11 @@ class SymbolBucket implements Bucket {
this.text.indexArray.clear();
this.icon.indexArray.clear();

this.featureSortOrder = [];

for (const i of symbolInstanceIndexes) {
const symbolInstance = this.symbolInstances[i];
this.featureSortOrder.push(symbolInstance.featureIndex);

for (const placedTextSymbolIndex of symbolInstance.placedTextSymbolIndices) {
const placedSymbol = this.text.placedSymbolArray.get(placedTextSymbolIndex);
Expand Down
25 changes: 24 additions & 1 deletion src/source/query_features.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type Coordinate from '../geo/coordinate';
import type CollisionIndex from '../symbol/collision_index';
import type Transform from '../geo/transform';
import type { RetainedQueryData } from '../symbol/placement';
import assert from 'assert';

export function queryRenderedFeatures(sourceCache: SourceCache,
styleLayers: {[string]: StyleLayer},
Expand Down Expand Up @@ -56,9 +57,31 @@ export function queryRenderedSymbols(styleLayers: {[string]: StyleLayer},
params.filter,
params.layers,
styleLayers);

for (const layerID in bucketSymbols) {
const resultFeatures = result[layerID] = result[layerID] || [];
for (const symbolFeature of bucketSymbols[layerID]) {
const layerSymbols = bucketSymbols[layerID];
layerSymbols.sort((a, b) => {
// Match topDownFeatureComparator from FeatureIndex, but using
// most recent sorting of features from bucket.sortFeatures
const featureSortOrder = queryData.featureSortOrder;
if (featureSortOrder) {
// queryRenderedSymbols documentation says we'll return features in
// "top-to-bottom" rendering order (aka last-to-first).
// Actually there can be multiple symbol instances per feature, so
// we sort each feature based on the first matching symbol instance.
const sortedA = featureSortOrder.indexOf(a.featureIndex);
const sortedB = featureSortOrder.indexOf(b.featureIndex);
assert(sortedA >= 0);
assert(sortedB >= 0);
return sortedB - sortedA;
} else {
// Bucket hasn't been re-sorted based on angle, so use the
// reverse of the order the features appeared in the data.
return b.featureIndex - a.featureIndex;
}
});
for (const symbolFeature of layerSymbols) {
resultFeatures.push(symbolFeature.feature);
}
}
Expand Down
3 changes: 0 additions & 3 deletions src/symbol/collision_index.js
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,6 @@ class CollisionIndex {
}
result[featureKey.bucketInstanceId].push(featureKey.featureIndex);
}
for (const bucket in result) {
result[bucket].sort((a, b) => b - a); // Match topDownFeatureComparator from FeatureIndex
}

return result;
}
Expand Down
4 changes: 4 additions & 0 deletions src/symbol/placement.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export class RetainedQueryData {
sourceLayerIndex: number;
bucketIndex: number;
tileID: OverscaledTileID;
featureSortOrder: ?Array<number>

constructor(bucketInstanceId: number,
featureIndex: FeatureIndex,
Expand Down Expand Up @@ -393,6 +394,9 @@ export class Placement {
}

bucket.sortFeatures(this.transform.angle);
if (this.retainedQueryData[bucket.bucketInstanceId]) {
this.retainedQueryData[bucket.bucketInstanceId].featureSortOrder = bucket.featureSortOrder;
}

if (bucket.hasTextData() && bucket.text.opacityVertexBuffer) {
bucket.text.opacityVertexBuffer.updateData(bucket.text.opacityVertexArray);
Expand Down
Loading

0 comments on commit 6997943

Please sign in to comment.