Add "default" property for functions

It is used in the following circumstances: * In categorical functions, when the feature value does not match any of the stop domain values. * In property and zoom-and-property functions, when a feature does not contain a value for the specified property. * In identity functions, when the feature value is not valid for the style property (for example, if the function is being used for a `circle-color` property but the feature property value is not a string or not a valid color). * In interval or exponential property and zoom-and-property functions, when the feature value is not numeric. If no default is provided, the style property's default is used in these circumstances. Implementing this required breaking changes to the function API in mapbox-gl-style-spec. The export is now a function which accepts the function definition and the spec JSON for a style property. It handles color parsing and validation of feature values internally.
mapbox · Feb 3, 2017 · 5e61f88 · 5e61f88
1 parent f7e2e8f
commit 5e61f88
Show file tree

Hide file tree

Showing 24 changed files with 964 additions and 161 deletions.
diff --git a/docs/style-spec/_generate/index.html b/docs/style-spec/_generate/index.html
@@ -856,6 +856,17 @@ <h3 class='space-bottom1'><a href='#types-function' title='link to function'>Fun
                 <dd>functions return the output value of the stop equal to the function input.</dd>
               </dl>
             </div>
+            <div class="col12 clearfix pad0y pad2x space-bottom2">
+              <div><span class='code'><a id="function-default" href="#function-default">default</a></span></div>
+              <div>A value to serve as a fallback function result when a value isn't otherwise available. It is used in the following circumstances:</div>
+              <ul>
+                <li>In categorical functions, when the feature value does not match any of the stop domain values.</li>
+                <li>In property and zoom-and-property functions, when a feature does not contain a value for the specified property.</li>
+                <li>In identity functions, when the feature value is not valid for the style property (for example, if the function is being used for a <var>circle-color</var> property but the feature property value is not a string or not a valid color).</li>
+                <li>In interval or exponential property and zoom-and-property functions, when the feature value is not numeric.</li>
+              </ul>
+              <div>If no default is provided, the style property's default is used in these circumstances.</div>
+            </div>
             <div class="col12 clearfix pad0y pad2x space-bottom2">
               <div><span class='code'><a id="function-colorSpace" href="#function-colorSpace">colorSpace</a></span></div>
               <div><em class='quiet'>Optional <a href='#types-string'>enum</a>. One of <var>rgb</var>, <var>lab</var>, <var>hcl</var>.</em></div>

diff --git a/js/style-spec/CHANGELOG.md b/js/style-spec/CHANGELOG.md
@@ -1,3 +1,12 @@
+## master
+
+* BREAKING CHANGE: the API for the `function` module has changed. The `interpolated` and `piecewise-constant` exports
+were replaced with a single unnamed export, a function which accepts an object conforming to the style spec "function"
+definition, and an object defining a style spec property. It handles color parsing and validation of feature values
+internally.
+* Functions now support a "default" property.
+* `parseColor` was promoted from gl-js.
+
 ## 8.11.0
 
 * Merge `feature-filter` repository into this repository #639

diff --git a/js/style-spec/function/index.js b/js/style-spec/function/index.js
@@ -1,15 +1,23 @@
 'use strict';
 
 const colorSpaces = require('./color_spaces');
+const parseColor = require('../util/parse_color');
+const extend = require('../util/extend');
+const getType = require('../util/get_type');
 
 function identityFunction(x) {
     return x;
 }
 
-function createFunction(parameters, defaultType) {
+function createFunction(parameters, propertySpec) {
+    const isColor = propertySpec.type === 'color';
+
     let fun;
 
     if (!isFunctionDefinition(parameters)) {
+        if (isColor && parameters) {
+            parameters = parseColor(parameters);
+        }
         fun = function() {
             return parameters;
         };
@@ -20,7 +28,23 @@ function createFunction(parameters, defaultType) {
         const zoomAndFeatureDependent = parameters.stops && typeof parameters.stops[0][0] === 'object';
         const featureDependent = zoomAndFeatureDependent || parameters.property !== undefined;
         const zoomDependent = zoomAndFeatureDependent || !featureDependent;
-        const type = parameters.type || defaultType || 'exponential';
+        const type = parameters.type || (propertySpec.function === 'interpolated' ? 'exponential' : 'interval');
+
+        if (isColor) {
+            parameters = extend({}, parameters);
+
+            if (parameters.stops) {
+                parameters.stops = parameters.stops.map((stop) => {
+                    return [stop[0], parseColor(stop[1])];
+                });
+            }
+
+            if (parameters.default) {
+                parameters.default = parseColor(parameters.default);
+            } else {
+                parameters.default = parseColor(propertySpec.default);
+            }
+        }
 
         let innerFun;
         let hashedStops;
@@ -84,26 +108,30 @@ function createFunction(parameters, defaultType) {
             }
 
             for (const z in featureFunctions) {
-                featureFunctionStops.push([featureFunctions[z].zoom, createFunction(featureFunctions[z])]);
+                featureFunctionStops.push([featureFunctions[z].zoom, createFunction(featureFunctions[z], propertySpec)]);
             }
             fun = function(zoom, feature) {
                 return outputFunction(evaluateExponentialFunction({
                     stops: featureFunctionStops,
                     base: parameters.base
-                }, zoom)(zoom, feature));
+                }, propertySpec, zoom)(zoom, feature));
             };
             fun.isFeatureConstant = false;
             fun.isZoomConstant = false;
 
         } else if (zoomDependent) {
             fun = function(zoom) {
-                return outputFunction(innerFun(parameters, zoom, hashedStops));
+                return outputFunction(innerFun(parameters, propertySpec, zoom, hashedStops));
             };
             fun.isFeatureConstant = true;
             fun.isZoomConstant = false;
         } else {
             fun = function(zoom, feature) {
-                return outputFunction(innerFun(parameters, feature[parameters.property], hashedStops));
+                const value = feature[parameters.property];
+                if (value === undefined) {
+                    return coalesce(parameters.default, propertySpec.default);
+                }
+                return outputFunction(innerFun(parameters, propertySpec, value, hashedStops));
             };
             fun.isFeatureConstant = false;
             fun.isZoomConstant = true;
@@ -113,21 +141,21 @@ function createFunction(parameters, defaultType) {
     return fun;
 }
 
-function evaluateCategoricalFunction(parameters, input, hashedStops) {
-    const value = hashedStops[input];
-    if (value === undefined) {
-      // If the input is not found, return the first value from the original array by default
-        return parameters.stops[0][1];
-    }
+function coalesce(a, b, c) {
+    if (a !== undefined) return a;
+    if (b !== undefined) return b;
+    if (c !== undefined) return c;
+}
 
-    return value;
+function evaluateCategoricalFunction(parameters, propertySpec, input, hashedStops) {
+    return coalesce(hashedStops[input], parameters.default, propertySpec.default);
 }
 
-function evaluateIntervalFunction(parameters, input) {
+function evaluateIntervalFunction(parameters, propertySpec, input) {
     // Edge cases
+    if (getType(input) !== 'number') return coalesce(parameters.default, propertySpec.default);
     const n = parameters.stops.length;
     if (n === 1) return parameters.stops[0][1];
-    if (input === undefined || input === null) return parameters.stops[n - 1][1];
     if (input <= parameters.stops[0][0]) return parameters.stops[0][1];
     if (input >= parameters.stops[n - 1][0]) return parameters.stops[n - 1][1];
 
@@ -136,13 +164,13 @@ function evaluateIntervalFunction(parameters, input) {
     return parameters.stops[index][1];
 }
 
-function evaluateExponentialFunction(parameters, input) {
+function evaluateExponentialFunction(parameters, propertySpec, input) {
     const base = parameters.base !== undefined ? parameters.base : 1;
 
     // Edge cases
+    if (getType(input) !== 'number') return coalesce(parameters.default, propertySpec.default);
     const n = parameters.stops.length;
     if (n === 1) return parameters.stops[0][1];
-    if (input === undefined || input === null) return parameters.stops[n - 1][1];
     if (input <= parameters.stops[0][0]) return parameters.stops[0][1];
     if (input >= parameters.stops[n - 1][0]) return parameters.stops[n - 1][1];
 
@@ -158,8 +186,13 @@ function evaluateExponentialFunction(parameters, input) {
     );
 }
 
-function evaluateIdentityFunction(parameters, input) {
-    return input;
+function evaluateIdentityFunction(parameters, propertySpec, input) {
+    if (propertySpec.type === 'color') {
+        input = parseColor(input);
+    } else if (getType(input) !== propertySpec.type) {
+        input = undefined;
+    }
+    return coalesce(input, parameters.default, propertySpec.default);
 }
 
 function binarySearchForIndex(stops, input) {
@@ -190,6 +223,10 @@ function interpolate(input, base, inputLower, inputUpper, outputLower, outputUpp
         return function() {
             const evaluatedLower = outputLower.apply(undefined, arguments);
             const evaluatedUpper = outputUpper.apply(undefined, arguments);
+            // Special case for fill-outline-color, which has no spec default.
+            if (evaluatedLower === undefined || evaluatedUpper === undefined) {
+                return undefined;
+            }
             return interpolate(input, base, inputLower, inputUpper, evaluatedLower, evaluatedUpper);
         };
     } else if (outputLower.length) {
@@ -225,13 +262,5 @@ function isFunctionDefinition(value) {
     return typeof value === 'object' && (value.stops || value.type === 'identity');
 }
 
-
+module.exports = createFunction;
 module.exports.isFunctionDefinition = isFunctionDefinition;
-
-module.exports.interpolated = function(parameters) {
-    return createFunction(parameters, 'exponential');
-};
-
-module.exports['piecewise-constant'] = function(parameters) {
-    return createFunction(parameters, 'interval');
-};
diff --git a/js/style-spec/reference/v8.json b/js/style-spec/reference/v8.json
@@ -1643,6 +1643,11 @@
       },
       "doc": "The color space in which colors interpolated. Interpolating colors in perceptual color spaces like LAB and HCL tend to produce color ramps that look more consistent and produce colors that can be differentiated more easily than those interpolated in RGB space.",
       "default": "rgb"
+    },
+    "default": {
+      "type": "*",
+      "required": false,
+      "doc": "A value to serve as a fallback function result when a value isn't otherwise available. It is used in the following circumstances:\n* In categorical functions, when the feature value does not match any of the stop domain values.\n* In property and zoom-and-property functions, when a feature does not contain a value for the specified property.\n* In identity functions, when the feature value is not valid for the style property (for example, if the function is being used for a `circle-color` property but the feature property value is not a string or not a valid color).\n* In interval or exponential property and zoom-and-property functions, when the feature value is not numeric.\nIf no default is provided, the style property's default is used in these circumstances."
     }
   },
   "function_stop": {

diff --git a/js/style-spec/util/parse_color.js b/js/style-spec/util/parse_color.js
@@ -0,0 +1,25 @@
+'use strict';
+
+const parseColorString = require('csscolorparser').parseCSSColor;
+
+module.exports = function parseColor(input) {
+    if (typeof input === 'string') {
+        const rgba = parseColorString(input);
+        if (!rgba) { return undefined; }
+
+        // GL expects all components to be in the range [0, 1] and to be
+        // multipled by the alpha value.
+        return [
+            rgba[0] / 255 * rgba[3],
+            rgba[1] / 255 * rgba[3],
+            rgba[2] / 255 * rgba[3],
+            rgba[3]
+        ];
+
+    } else if (Array.isArray(input)) {
+        return input;
+
+    } else {
+        return undefined;
+    }
+};
diff --git a/js/style-spec/validate/validate_function.js b/js/style-spec/validate/validate_function.js
@@ -29,7 +29,10 @@ module.exports = function validateFunction(options) {
         valueSpec: options.styleSpec.function,
         style: options.style,
         styleSpec: options.styleSpec,
-        objectElementValidators: { stops: validateFunctionStops }
+        objectElementValidators: {
+            stops: validateFunctionStops,
+            default: validateFunctionDefault
+        }
     });
 
     if (functionType !== 'identity' && !options.value.stops) {
@@ -184,4 +187,13 @@ module.exports = function validateFunction(options) {
         return [];
     }
 
+    function validateFunctionDefault(options) {
+        return validate({
+            key: options.key,
+            value: options.value,
+            valueSpec: functionValueSpec,
+            style: options.style,
+            styleSpec: options.styleSpec
+        });
+    }
 };
diff --git a/js/style/parse_color.js b/js/style/parse_color.js
diff --git a/js/style/style_declaration.js b/js/style/style_declaration.js
@@ -1,26 +1,19 @@
 'use strict';
 
-const MapboxGLFunction = require('../style-spec/function');
-const parseColor = require('./parse_color');
+const createFunction = require('../style-spec/function');
 const util = require('../util/util');
 
 class StyleDeclaration {
 
     constructor(reference, value) {
         this.value = util.clone(value);
-        this.isFunction = MapboxGLFunction.isFunctionDefinition(value);
+        this.isFunction = createFunction.isFunctionDefinition(value);
 
         // immutable representation of value. used for comparison
         this.json = JSON.stringify(this.value);
 
         this.minimum = reference.minimum;
-        this.isColor = reference.type === 'color';
-
-        const parsedValue = this.isColor && this.value ? parseColor(this.value) : value;
-        let specDefault = reference.default;
-        if (specDefault && reference.type === 'color') specDefault = parseColor(specDefault);
-
-        this.function = MapboxGLFunction[reference.function || 'piecewise-constant'](parsedValue, specDefault);
+        this.function = createFunction(this.value, reference);
         this.isFeatureConstant = this.function.isFeatureConstant;
         this.isZoomConstant = this.function.isZoomConstant;
 
@@ -35,19 +28,18 @@ class StyleDeclaration {
                 }
             }
 
-            this.functionInterpolationT = MapboxGLFunction.interpolated({
+            this.functionInterpolationT = createFunction({
+                type: 'exponential',
                 stops: interpolationAmountStops,
-                base: value.base,
-                colorSpace: value.colorSpace
+                base: value.base
+            }, {
+                type: 'number'
             });
         }
     }
 
     calculate(globalProperties, featureProperties) {
         const value = this.function(globalProperties && globalProperties.zoom, featureProperties || {});
-        if (this.isColor && value) {
-            return parseColor(value);
-        }
         if (this.minimum !== undefined && value < this.minimum) {
             return this.minimum;
         }

diff --git a/js/style/style_layer.js b/js/style/style_layer.js
@@ -5,7 +5,7 @@ const StyleTransition = require('./style_transition');
 const StyleDeclaration = require('./style_declaration');
 const styleSpec = require('../style-spec/reference/latest');
 const validateStyle = require('./validate_style');
-const parseColor = require('./parse_color');
+const parseColor = require('./../style-spec/util/parse_color');
 const Evented = require('../util/evented');
 
 const TRANSITION_SUFFIX = '-transition';

diff --git a/package.json b/package.json
@@ -16,7 +16,6 @@
     "@mapbox/unitbezier": "^0.0.0",
     "brfs": "^1.4.0",
     "bubleify": "^0.5.1",
-    "csscolorparser": "^1.0.2",
     "earcut": "^2.0.3",
     "geojson-rewind": "^0.1.0",
     "geojson-vt": "^2.4.0",

diff --git a/test/integration/render-tests/regressions/mapbox-gl-js#4124/expected.png b/test/integration/render-tests/regressions/mapbox-gl-js#4124/expected.png