fix icon-size for small data-driven values (#7125)

fix #7017 fix #7066
mapbox · Aug 14, 2018 · 23469bc · 23469bc
1 parent 7d67106
commit 23469bc
Show file tree

Hide file tree

Showing 6 changed files with 70 additions and 13 deletions.
diff --git a/src/shaders/symbol_icon.vertex.glsl b/src/shaders/symbol_icon.vertex.glsl
@@ -42,9 +42,9 @@ void main() {
 
     float size;
     if (!u_is_size_zoom_constant && !u_is_size_feature_constant) {
-        size = mix(a_size[0], a_size[1], u_size_t) / 10.0;
+        size = mix(a_size[0], a_size[1], u_size_t) / 256.0;
     } else if (u_is_size_zoom_constant && !u_is_size_feature_constant) {
-        size = a_size[0] / 10.0;
+        size = a_size[0] / 256.0;
     } else if (!u_is_size_zoom_constant && u_is_size_feature_constant) {
         size = u_size;
     } else {

diff --git a/src/shaders/symbol_sdf.vertex.glsl b/src/shaders/symbol_sdf.vertex.glsl
@@ -57,9 +57,9 @@ void main() {
     float size;
 
     if (!u_is_size_zoom_constant && !u_is_size_feature_constant) {
-        size = mix(a_size[0], a_size[1], u_size_t) / 10.0;
+        size = mix(a_size[0], a_size[1], u_size_t) / 256.0;
     } else if (u_is_size_zoom_constant && !u_is_size_feature_constant) {
-        size = a_size[0] / 10.0;
+        size = a_size[0] / 256.0;
     } else if (!u_is_size_zoom_constant && u_is_size_feature_constant) {
         size = u_size;
     } else {

diff --git a/src/symbol/symbol_layout.js b/src/symbol/symbol_layout.js
@@ -18,6 +18,7 @@ import EXTENT from '../data/extent';
 import SymbolBucket from '../data/bucket/symbol_bucket';
 import EvaluationParameters from '../style/evaluation_parameters';
 import {Formatted} from '../style-spec/expression/definitions/formatted';
+import {SIZE_PACK_FACTOR} from './symbol_size';
 
 import type {Shaping, PositionedIcon} from './shaping';
 import type {CollisionBoxArray} from '../data/array_types';
@@ -268,6 +269,8 @@ function addFeature(bucket: SymbolBucket,
     }
 }
 
+const MAX_PACKED_SIZE = 65535;
+
 function addTextVertices(bucket: SymbolBucket,
                          anchor: Point,
                          shapedText: Shaping,
@@ -288,13 +291,19 @@ function addTextVertices(bucket: SymbolBucket,
 
     if (sizeData.functionType === 'source') {
         textSizeData = [
-            10 * layer.layout.get('text-size').evaluate(feature, {})
+            SIZE_PACK_FACTOR * layer.layout.get('text-size').evaluate(feature, {})
         ];
+        if (textSizeData[0] > MAX_PACKED_SIZE) {
+            warnOnce(`${bucket.layerIds[0]}: Value for "text-size" is >= 256. Reduce your "text-size".`);
+        }
     } else if (sizeData.functionType === 'composite') {
         textSizeData = [
-            10 * sizes.compositeTextSizes[0].evaluate(feature, {}),
-            10 * sizes.compositeTextSizes[1].evaluate(feature, {})
+            SIZE_PACK_FACTOR * sizes.compositeTextSizes[0].evaluate(feature, {}),
+            SIZE_PACK_FACTOR * sizes.compositeTextSizes[1].evaluate(feature, {})
         ];
+        if (textSizeData[0] > MAX_PACKED_SIZE || textSizeData[1] > MAX_PACKED_SIZE) {
+            warnOnce(`${bucket.layerIds[0]}: Value for "text-size" is >= 256. Reduce your "text-size".`);
+        }
     }
 
     bucket.addSymbols(
@@ -381,13 +390,19 @@ function addSymbol(bucket: SymbolBucket,
 
         if (sizeData.functionType === 'source') {
             iconSizeData = [
-                10 * layer.layout.get('icon-size').evaluate(feature, {})
+                SIZE_PACK_FACTOR * layer.layout.get('icon-size').evaluate(feature, {})
             ];
+            if (iconSizeData[0] > MAX_PACKED_SIZE) {
+                warnOnce(`${bucket.layerIds[0]}: Value for "icon-size" is >= 256. Reduce your "icon-size".`);
+            }
         } else if (sizeData.functionType === 'composite') {
             iconSizeData = [
-                10 * sizes.compositeIconSizes[0].evaluate(feature, {}),
-                10 * sizes.compositeIconSizes[1].evaluate(feature, {})
+                SIZE_PACK_FACTOR * sizes.compositeIconSizes[0].evaluate(feature, {}),
+                SIZE_PACK_FACTOR * sizes.compositeIconSizes[1].evaluate(feature, {})
             ];
+            if (iconSizeData[0] > MAX_PACKED_SIZE || iconSizeData[1] > MAX_PACKED_SIZE) {
+                warnOnce(`${bucket.layerIds[0]}: Value for "icon-size" is >= 256. Reduce your "icon-size".`);
+            }
         }
 
         bucket.addSymbols(

diff --git a/src/symbol/symbol_size.js b/src/symbol/symbol_size.js
@@ -10,7 +10,9 @@ import type {Property, PropertyValue, PossiblyEvaluatedPropertyValue} from '../s
 import type {CameraExpression, CompositeExpression} from '../style-spec/expression/index';
 import type {PropertyValueSpecification} from '../style-spec/types';
 
-export { getSizeData, evaluateSizeForFeature, evaluateSizeForZoom };
+const SIZE_PACK_FACTOR = 256;
+
+export { getSizeData, evaluateSizeForFeature, evaluateSizeForZoom, SIZE_PACK_FACTOR };
 
 export type SizeData = {
     functionType: 'constant',
@@ -89,9 +91,9 @@ function evaluateSizeForFeature(sizeData: SizeData,
                                 symbol: { lowerSize: number, upperSize: number}) {
     const part = partiallyEvaluatedSize;
     if (sizeData.functionType === 'source') {
-        return symbol.lowerSize / 10;
+        return symbol.lowerSize / SIZE_PACK_FACTOR;
     } else if (sizeData.functionType === 'composite') {
-        return interpolate(symbol.lowerSize / 10, symbol.upperSize / 10, part.uSizeT);
+        return interpolate(symbol.lowerSize / SIZE_PACK_FACTOR, symbol.upperSize / SIZE_PACK_FACTOR, part.uSizeT);
     } else {
         return part.uSize;
     }

diff --git a/test/integration/render-tests/regressions/mapbox-gl-js#7066/expected.png b/test/integration/render-tests/regressions/mapbox-gl-js#7066/expected.png
diff --git a/test/integration/render-tests/regressions/mapbox-gl-js#7066/style.json b/test/integration/render-tests/regressions/mapbox-gl-js#7066/style.json
@@ -0,0 +1,40 @@
+{
+  "version": 8,
+  "metadata": {
+    "test": {
+      "pixelRatio": 10,
+      "width": 24,
+      "height": 24 
+    }
+  },
+  "sources": {
+    "geojson": {
+      "type": "geojson",
+      "data": {
+        "type": "Feature",
+        "properties": {
+            "size": 0.05
+        },
+        "geometry": {
+            "type": "Point",
+            "coordinates": [
+              0,
+              0
+            ]
+        }
+      }
+    }
+  },
+  "sprite": "local://sprites/emerald",
+  "layers": [
+    {
+      "id": "symbol",
+      "type": "symbol",
+      "source": "geojson",
+      "layout": {
+        "icon-size": ["get", "size"],
+        "icon-image": "generic_icon"
+      }
+    }
+  ]
+}