Avoid sorting of features if the sort-key is constant (#78)

This addresses 2 related bugs:

1. An unevaluated mapbox expression-object was used in a condition (`if (expression)`) to decide if sorting was necessary, the assumption was probably that the default constant value of `undefined` would skip sorting. However, the unevaluated mapbox expression is not the value `undefined`, but an expression-object describing the value and its type. So, even if the mapbox expression was a constant `undefined`, the check would never be true. This leads to a lot of unnecessary sorting.
    This was already correct for the drawing sort in circles and symbols, and this is where the `sortFeaturesByKey` variable name originates.
2. Even where the check was correct, it would explicitly check if the sort-key was a constant with value `undefined` to skip sorting. However, we also know that the sort-key is irrelevant if it's *any* constant, not just `undefined`.

Note that the second optimization would not be possible if cross-layer sorting was implemented (see mapbox/mapbox-gl-js#1349).

I'm unable to provide good performance benchmarks because of #76.

CHANGELOG.md

@@ -2,11 +2,12 @@
 
 ### Features and improvements
 
-- Add new stuff here
+- *...Add new stuff here...*
+- Improve performance of layers with constant `*-sort-key` (#78)
 
 ### 🐞 Bug fixes
 
-- Add fixed bugs here
+- *...Add fixed bugs here...*
 
 ## 1.14.0
 

src/data/bucket/circle_bucket.js

@@ -81,10 +81,12 @@ class CircleBucket<Layer: CircleStyleLayer | HeatmapStyleLayer> implements Bucke
         const styleLayer = this.layers[0];
         const bucketFeatures = [];
         let circleSortKey = null;
+        let sortFeaturesByKey = false;
 
         // Heatmap layers are handled in this bucket and have no evaluated properties, so we check our access
         if (styleLayer.type === 'circle') {
             circleSortKey = ((styleLayer: any): CircleStyleLayer).layout.get('circle-sort-key');
+            sortFeaturesByKey = !circleSortKey.isConstant();
         }
 
         for (const {feature, id, index, sourceLayerIndex} of features) {
@@ -93,7 +95,8 @@ class CircleBucket<Layer: CircleStyleLayer | HeatmapStyleLayer> implements Bucke
 
             if (!this.layers[0]._featureFilter.filter(new EvaluationParameters(this.zoom), evaluationFeature, canonical)) continue;
 
-            const sortKey = circleSortKey ?
+            const sortKey = sortFeaturesByKey ?
+                // $FlowFixMe
                 circleSortKey.evaluate(evaluationFeature, {}, canonical) :
                 undefined;
 
@@ -112,7 +115,7 @@ class CircleBucket<Layer: CircleStyleLayer | HeatmapStyleLayer> implements Bucke
 
         }
 
-        if (circleSortKey) {
+        if (sortFeaturesByKey) {
             bucketFeatures.sort((a, b) => {
                 // a.sortKey is always a number when in use
                 return ((a.sortKey: any): number) - ((b.sortKey: any): number);

src/data/bucket/fill_bucket.js

@@ -78,6 +78,7 @@ class FillBucket implements Bucket {
     populate(features: Array<IndexedFeature>, options: PopulateParameters, canonical: CanonicalTileID) {
         this.hasPattern = hasPattern('fill', this.layers, options);
         const fillSortKey = this.layers[0].layout.get('fill-sort-key');
+        const sortFeaturesByKey = !fillSortKey.isConstant();
         const bucketFeatures = [];
 
         for (const {feature, id, index, sourceLayerIndex} of features) {
@@ -86,7 +87,7 @@ class FillBucket implements Bucket {
 
             if (!this.layers[0]._featureFilter.filter(new EvaluationParameters(this.zoom), evaluationFeature, canonical)) continue;
 
-            const sortKey = fillSortKey ?
+            const sortKey = sortFeaturesByKey ?
                 fillSortKey.evaluate(evaluationFeature, {}, canonical, options.availableImages) :
                 undefined;
 
@@ -104,7 +105,7 @@ class FillBucket implements Bucket {
             bucketFeatures.push(bucketFeature);
         }
 
-        if (fillSortKey) {
+        if (sortFeaturesByKey) {
             bucketFeatures.sort((a, b) => {
                 // a.sortKey is always a number when in use
                 return ((a.sortKey: any): number) - ((b.sortKey: any): number);

src/data/bucket/line_bucket.js

@@ -146,6 +146,7 @@ class LineBucket implements Bucket {
     populate(features: Array<IndexedFeature>, options: PopulateParameters, canonical: CanonicalTileID) {
         this.hasPattern = hasPattern('line', this.layers, options);
         const lineSortKey = this.layers[0].layout.get('line-sort-key');
+        const sortFeaturesByKey = !lineSortKey.isConstant();
         const bucketFeatures = [];
 
         for (const {feature, id, index, sourceLayerIndex} of features) {
@@ -154,7 +155,7 @@ class LineBucket implements Bucket {
 
             if (!this.layers[0]._featureFilter.filter(new EvaluationParameters(this.zoom), evaluationFeature, canonical)) continue;
 
-            const sortKey = lineSortKey ?
+            const sortKey = sortFeaturesByKey ?
                 lineSortKey.evaluate(evaluationFeature, {}, canonical) :
                 undefined;
 
@@ -172,7 +173,7 @@ class LineBucket implements Bucket {
             bucketFeatures.push(bucketFeature);
         }
 
-        if (lineSortKey) {
+        if (sortFeaturesByKey) {
             bucketFeatures.sort((a, b) => {
                 // a.sortKey is always a number when in use
                 return ((a.sortKey: any): number) - ((b.sortKey: any): number);

src/data/bucket/symbol_bucket.js

@@ -374,7 +374,7 @@ class SymbolBucket implements Bucket {
             layout.get('icon-allow-overlap') ||
             layout.get('text-ignore-placement') ||
             layout.get('icon-ignore-placement');
-        this.sortFeaturesByKey = zOrder !== 'viewport-y' && sortKey.constantOr(1) !== undefined;
+        this.sortFeaturesByKey = zOrder !== 'viewport-y' && !sortKey.isConstant();
         const zOrderByViewportY = zOrder === 'viewport-y' || (zOrder === 'auto' && !this.sortFeaturesByKey);
         this.sortFeaturesByY = zOrderByViewportY && this.canOverlap;
 

src/render/draw_circle.js

@@ -40,7 +40,7 @@ function drawCircles(painter: Painter, sourceCache: SourceCache, layer: CircleSt
     const opacity = layer.paint.get('circle-opacity');
     const strokeWidth = layer.paint.get('circle-stroke-width');
     const strokeOpacity = layer.paint.get('circle-stroke-opacity');
-    const sortFeaturesByKey = layer.layout.get('circle-sort-key').constantOr(1) !== undefined;
+    const sortFeaturesByKey = !layer.layout.get('circle-sort-key').isConstant();
 
     if (opacity.constantOr(1) === 0 && (strokeWidth.constantOr(1) === 0 || strokeOpacity.constantOr(1) === 0)) {
         return;

src/render/draw_symbol.js

@@ -236,7 +236,7 @@ function drawLayerSymbols(painter, sourceCache, layer, coords, isText, translate
     // Unpitched point labels need to have their rotation applied after projection
     const rotateInShader = rotateWithMap && !pitchWithMap && !alongLine;
 
-    const hasSortKey = layer.layout.get('symbol-sort-key').constantOr(1) !== undefined;
+    const hasSortKey = !layer.layout.get('symbol-sort-key').isConstant();
     let sortFeaturesByKey = false;
 
     const depthMode = painter.depthModeForSublayer(0, DepthMode.ReadOnly);

src/style/pauseable_placement.js

@@ -19,7 +19,7 @@ class LayerPlacement {
 
     constructor(styleLayer: SymbolStyleLayer) {
         this._sortAcrossTiles = styleLayer.layout.get('symbol-z-order') !== 'viewport-y' &&
-            styleLayer.layout.get('symbol-sort-key').constantOr(1) !== undefined;
+            !styleLayer.layout.get('symbol-sort-key').isConstant();
 
         this._currentTileIndex = 0;
         this._currentPartIndex = 0;