make symbol placement respect symbol-sort-key

fix #8896

src/data/bucket/symbol_bucket.js

@@ -90,6 +90,12 @@ export type SymbolFeature = {|
     id?: any
 |};
 
+export type SortKeyRange = {
+    sortKey: number,
+    symbolInstanceStart: number,
+    symbolInstanceEnd: number
+};
+
 // Opacity arrays are frequently updated but don't contain a lot of information, so we pack them
 // tight. Each Uint32 is actually four duplicate Uint8s for the four corners of a glyph
 // 7 bits are for the current opacity, and the lowest bit is the target opacity
@@ -301,6 +307,7 @@ class SymbolBucket implements Bucket {
     features: Array<SymbolFeature>;
     symbolInstances: SymbolInstanceArray;
     collisionArrays: Array<CollisionArrays>;
+    sortKeyRanges: Array<SortKeyRange>;
     pixelRatio: number;
     tilePixelRatio: number;
     compareText: {[string]: Array<Point>};
@@ -337,6 +344,7 @@ class SymbolBucket implements Bucket {
         this.hasPattern = false;
         this.hasPaintOverrides = false;
         this.hasRTLText = false;
+        this.sortKeyRanges = [];
 
         const layer = this.layers[0];
         const unevaluatedLayoutValues = layer._unevaluatedLayout._values;
@@ -883,6 +891,19 @@ class SymbolBucket implements Bucket {
         return result;
     }
 
+    addToSortKeyRanges(symbolInstanceIndex: number, sortKey: number) {
+        const last = this.sortKeyRanges[this.sortKeyRanges.length - 1];
+        if (last && last.sortKey === sortKey) {
+            last.symbolInstanceEnd = symbolInstanceIndex + 1;
+        } else {
+            this.sortKeyRanges.push({
+                sortKey,
+                symbolInstanceStart: symbolInstanceIndex,
+                symbolInstanceEnd: symbolInstanceIndex + 1
+            });
+        }
+    }
+
     sortFeatures(angle: number) {
         if (!this.sortFeaturesByY) return;
         if (this.sortedAngle === angle) return;

src/style/pauseable_placement.js

@@ -6,28 +6,56 @@ import {Placement} from '../symbol/placement';
 
 import type Transform from '../geo/transform';
 import type StyleLayer from './style_layer';
+import type SymbolStyleLayer from './style_layer/symbol_style_layer';
 import type Tile from '../source/tile';
+import type {BucketPart} from '../symbol/placement';
 
 class LayerPlacement {
+    _sortAcrossTiles: boolean;
     _currentTileIndex: number;
-    _tiles: Array<Tile>;
+    _currentPartIndex: number;
     _seenCrossTileIDs: { [string | number]: boolean };
+    _bucketParts: Array<BucketPart>;
+
+    constructor(styleLayer: SymbolStyleLayer) {
+        this._sortAcrossTiles = styleLayer.type === 'symbol' &&
+            styleLayer.layout.get('symbol-z-order') !== 'viewport-y' &&
+            styleLayer.layout.get('symbol-sort-key').constantOr(1) !== undefined;
 
-    constructor() {
         this._currentTileIndex = 0;
+        this._currentPartIndex = 0;
         this._seenCrossTileIDs = {};
+        this._bucketParts = [];
     }
 
     continuePlacement(tiles: Array<Tile>, placement: Placement, showCollisionBoxes: boolean, styleLayer: StyleLayer, shouldPausePlacement: () => boolean) {
+
+        const bucketParts = this._bucketParts;
+
         while (this._currentTileIndex < tiles.length) {
             const tile = tiles[this._currentTileIndex];
-            placement.placeLayerTile(styleLayer, tile, showCollisionBoxes, this._seenCrossTileIDs);
+            placement.placeLayerTile(bucketParts, styleLayer, tile, this._sortAcrossTiles);
 
             this._currentTileIndex++;
             if (shouldPausePlacement()) {
                 return true;
             }
         }
+
+        if (this._sortAcrossTiles) {
+            this._sortAcrossTiles = false;
+            bucketParts.sort((a, b) => ((a.sortKey: any): number) - ((b.sortKey: any): number));
+        }
+
+        while (this._currentPartIndex < bucketParts.length) {
+            const bucketPart = bucketParts[this._currentPartIndex];
+            placement.placeLayerBucket(bucketPart, this._seenCrossTileIDs, showCollisionBoxes);
+
+            this._currentPartIndex++;
+            if (shouldPausePlacement()) {
+                return true;
+            }
+        }
     }
 }
 
@@ -74,7 +102,7 @@ class PauseablePlacement {
                 (!layer.maxzoom || layer.maxzoom > placementZoom)) {
 
                 if (!this._inProgressLayer) {
-                    this._inProgressLayer = new LayerPlacement();
+                    this._inProgressLayer = new LayerPlacement(((layer: any): SymbolStyleLayer));
                 }
 
                 const pausePlacement = this._inProgressLayer.continuePlacement(layerTiles[layer.source], this.placement, this._showCollisionBoxes, layer, shouldPausePlacement);

src/symbol/placement.js

@@ -157,6 +157,26 @@ export type VariableOffset = {
     prevAnchor?: TextAnchor
 };
 
+type TileLayerParameters = {
+    bucket: SymbolBucket,
+    layout: any,
+    posMatrix: mat4,
+    textLabelPlaneMatrix: mat4,
+    scale: number,
+    textPixelRatio: number,
+    holdingForFade: boolean,
+    collisionBoxArray: ?CollisionBoxArray,
+    partiallyEvaluatedTextSize: any,
+    collisionGroup: any
+};
+
+export type BucketPart = {
+    sortKey?: number | void,
+    symbolInstanceStart: number,
+    symbolInstanceEnd: number,
+    parameters: TileLayerParameters
+};
+
 export type CrossTileID = string | number;
 
 export class Placement {
@@ -196,7 +216,7 @@ export class Placement {
         this.placedOrientations = {};
     }
 
-    placeLayerTile(styleLayer: StyleLayer, tile: Tile, showCollisionBoxes: boolean, seenCrossTileIDs: { [string | number]: boolean }) {
+    placeLayerTile(results: Array<BucketPart>, styleLayer: StyleLayer, tile: Tile, sortAcrossTiles: boolean) {
         const symbolBucket = ((tile.getBucket(styleLayer): any): SymbolBucket);
         const bucketFeatureIndex = tile.latestFeatureIndex;
         if (!symbolBucket || !bucketFeatureIndex || styleLayer.id !== symbolBucket.layerIds[0])
@@ -217,12 +237,6 @@ export class Placement {
                 this.transform,
                 pixelsToTileUnits(tile, 1, this.transform.zoom));
 
-        const iconLabelPlaneMatrix = projection.getLabelPlaneMatrix(posMatrix,
-                layout.get('icon-pitch-alignment') === 'map',
-                layout.get('icon-rotation-alignment') === 'map',
-                this.transform,
-                pixelsToTileUnits(tile, 1, this.transform.zoom));
-
         // As long as this placement lives, we have to hold onto this bucket's
         // matching FeatureIndex/data for querying purposes
         this.retainedQueryData[symbolBucket.bucketInstanceId] = new RetainedQueryData(
@@ -232,8 +246,32 @@ export class Placement {
             symbolBucket.index,
             tile.tileID
         );
-        this.placeLayerBucket(symbolBucket, posMatrix, textLabelPlaneMatrix, iconLabelPlaneMatrix, scale, textPixelRatio,
-                showCollisionBoxes, tile.holdingForFade(), seenCrossTileIDs, collisionBoxArray);
+
+        const parameters = {
+            bucket: symbolBucket,
+            layout,
+            posMatrix,
+            textLabelPlaneMatrix,
+            scale,
+            textPixelRatio,
+            holdingForFade: tile.holdingForFade(),
+            collisionBoxArray,
+            partiallyEvaluatedTextSize: symbolSize.evaluateSizeForZoom(symbolBucket.textSizeData, this.transform.zoom),
+            collisionGroup: this.collisionGroups.get(symbolBucket.sourceID)
+        };
+
+        if (sortAcrossTiles) {
+            for (const range of symbolBucket.sortKeyRanges) {
+                const {sortKey, symbolInstanceStart, symbolInstanceEnd} = range;
+                results.push({sortKey, symbolInstanceStart, symbolInstanceEnd, parameters});
+            }
+        } else {
+            results.push({
+                symbolInstanceStart: 0,
+                symbolInstanceEnd: symbolBucket.symbolInstances.length,
+                parameters
+            });
+        }
     }
 
     attemptAnchorPlacement(anchor: TextAnchor, textBox: SingleCollisionBox, width: number, height: number,
@@ -289,15 +327,30 @@ export class Placement {
         }
     }
 
-    placeLayerBucket(bucket: SymbolBucket, posMatrix: mat4, textLabelPlaneMatrix: mat4, iconLabelPlaneMatrix: mat4,
-            scale: number, textPixelRatio: number, showCollisionBoxes: boolean, holdingForFade: boolean, seenCrossTileIDs: { [string | number]: boolean },
-            collisionBoxArray: ?CollisionBoxArray) {
-        const layout = bucket.layers[0].layout;
-        const partiallyEvaluatedTextSize = symbolSize.evaluateSizeForZoom(bucket.textSizeData, this.transform.zoom);
+    placeLayerBucket(bucketPart: Object, seenCrossTileIDs: { [string | number]: boolean }, showCollisionBoxes: boolean) {
+
+        const {
+            bucket,
+            layout,
+            posMatrix,
+            textLabelPlaneMatrix,
+            scale,
+            textPixelRatio,
+            holdingForFade,
+            collisionBoxArray,
+            partiallyEvaluatedTextSize,
+            collisionGroup
+        } = bucketPart.parameters;
+
         const textOptional = layout.get('text-optional');
         const iconOptional = layout.get('icon-optional');
         const textAllowOverlap = layout.get('text-allow-overlap');
         const iconAllowOverlap = layout.get('icon-allow-overlap');
+        const rotateWithMap = layout.get('text-rotation-alignment') === 'map';
+        const pitchWithMap = layout.get('text-pitch-alignment') === 'map';
+        const hasIconTextFit = layout.get('icon-text-fit') !== 'none';
+        const zOrderByViewportY = layout.get('symbol-z-order') === 'viewport-y';
+
         // This logic is similar to the "defaultOpacityState" logic below in updateBucketOpacities
         // If we know a symbol is always supposed to show, force it to be marked visible even if
         // it wasn't placed into the collision index (because some or all of it was outside the range
@@ -315,13 +368,6 @@ export class Placement {
         const alwaysShowText = textAllowOverlap && (iconAllowOverlap || !bucket.hasIconData() || iconOptional);
         const alwaysShowIcon = iconAllowOverlap && (textAllowOverlap || !bucket.hasTextData() || textOptional);
 
-        const collisionGroup = this.collisionGroups.get(bucket.sourceID);
-
-        const rotateWithMap = layout.get('text-rotation-alignment') === 'map';
-        const pitchWithMap = layout.get('text-pitch-alignment') === 'map';
-        const hasIconTextFit = layout.get('icon-text-fit') !== 'none';
-        const zOrderByViewportY = layout.get('symbol-z-order') === 'viewport-y';
-
         if (!bucket.collisionArrays && collisionBoxArray) {
             bucket.deserializeCollisionBoxes(collisionBoxArray);
         }
@@ -391,7 +437,7 @@ export class Placement {
 
                 if (!layout.get('text-variable-anchor')) {
                     const placeBox = (collisionTextBox, orientation) => {
-                        const placedFeature = this.collisionIndex.placeCollisionBox(collisionTextBox, layout.get('text-allow-overlap'),
+                        const placedFeature = this.collisionIndex.placeCollisionBox(collisionTextBox, textAllowOverlap,
                                                                                   textPixelRatio, posMatrix, collisionGroup.predicate);
                         if (placedFeature && placedFeature.box && placedFeature.box.length) {
                             this.markUsedOrientation(bucket, orientation, symbolInstance);
@@ -503,7 +549,7 @@ export class Placement {
                 const placedSymbol = bucket.text.placedSymbolArray.get(symbolInstance.centerJustifiedTextSymbolIndex);
                 const fontSize = symbolSize.evaluateSizeForFeature(bucket.textSizeData, partiallyEvaluatedTextSize, placedSymbol);
                 placedGlyphCircles = this.collisionIndex.placeCollisionCircles(textCircles,
-                        layout.get('text-allow-overlap'),
+                        textAllowOverlap,
                         scale,
                         textPixelRatio,
                         placedSymbol,
@@ -519,7 +565,7 @@ export class Placement {
                 // In theory there should always be at least one circle placed
                 // in this case, but for now quirks in text-anchor
                 // and text-offset may prevent that from being true.
-                placeText = layout.get('text-allow-overlap') || placedGlyphCircles.circles.length > 0;
+                placeText = textAllowOverlap || placedGlyphCircles.circles.length > 0;
                 offscreen = offscreen && placedGlyphCircles.offscreen;
             }
 
@@ -536,7 +582,7 @@ export class Placement {
                             rotateWithMap, pitchWithMap, this.transform.angle) :
                         iconBox;
                     return this.collisionIndex.placeCollisionBox(shiftedIconBox,
-                        layout.get('icon-allow-overlap'), textPixelRatio, posMatrix, collisionGroup.predicate);
+                        iconAllowOverlap, textPixelRatio, posMatrix, collisionGroup.predicate);
                 };
 
                 if (placedVerticalText && placedVerticalText.box && placedVerticalText.box.length && collisionArrays.verticalIconBox) {
@@ -589,13 +635,14 @@ export class Placement {
         };
 
         if (zOrderByViewportY) {
+            assert(bucket.symbolInstanceStart === 0);
             const symbolIndexes = bucket.getSortedSymbolIndexes(this.transform.angle);
             for (let i = symbolIndexes.length - 1; i >= 0; --i) {
                 const symbolIndex = symbolIndexes[i];
                 placeSymbol(bucket.symbolInstances.get(symbolIndex), bucket.collisionArrays[symbolIndex]);
             }
         } else {
-            for (let i = 0; i < bucket.symbolInstances.length; ++i) {
+            for (let i = bucketPart.symbolInstanceStart; i < bucketPart.symbolInstanceEnd; i++) {
                 placeSymbol(bucket.symbolInstances.get(i), bucket.collisionArrays[i]);
             }
         }

src/symbol/symbol_layout.js

@@ -717,6 +717,10 @@ function addSymbol(bucket: SymbolBucket,
         "Too many glyphs being rendered in a tile. See https://github.com/mapbox/mapbox-gl-js/issues/2907"
     );
 
+    if (feature.sortKey !== undefined) {
+        bucket.addToSortKeyRanges(bucket.symbolInstances.length, feature.sortKey);
+    }
+
     bucket.symbolInstances.emplaceBack(
         anchor.x,
         anchor.y,

test/integration/render-tests/symbol-sort-key/placement-tile-boundary-left-then-right/style.json

@@ -0,0 +1,75 @@
+{
+  "version": 8,
+  "metadata": {
+    "test": {
+      "height": 64,
+      "width": 64
+    }
+  },
+  "center": [0, 30],
+  "zoom": 1,
+  "sources": {
+    "geojson": {
+      "type": "geojson",
+      "data": {
+        "type": "FeatureCollection",
+        "features": [
+          {
+            "type": "Feature",
+            "properties": {
+              "sort-key": 0,
+              "image": "bank-12"
+            },
+            "geometry": {
+              "type": "Point",
+              "coordinates": [
+                -1,
+                34
+              ]
+            }
+          },
+          {
+            "type": "Feature",
+            "properties": {
+              "sort-key": 2,
+              "image": "bank-12"
+            },
+            "geometry": {
+              "type": "Point",
+              "coordinates": [
+                -1,
+                30
+              ]
+            }
+          },
+          {
+            "type": "Feature",
+            "properties": {
+              "sort-key": 1,
+              "image": "fav-campsite-18"
+            },
+            "geometry": {
+              "type": "Point",
+              "coordinates": [
+                1,
+                32
+              ]
+            }
+          }
+        ]
+      }
+    }
+  },
+  "sprite": "local://sprites/sprite",
+  "layers": [
+    {
+      "id": "icon",
+      "type": "symbol",
+      "source": "geojson",
+      "layout": {
+        "symbol-sort-key": ["get", "sort-key"],
+        "icon-image": ["get", "image"]
+      }
+    }
+  ]
+}