Skip to content

Commit

Permalink
Drop redundant initial input value for [step] curves
Browse files Browse the repository at this point in the history
  • Loading branch information
Anand Thakker committed Aug 31, 2017
1 parent 2929035 commit 354011e
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 20 deletions.
29 changes: 23 additions & 6 deletions src/style-spec/function/convert.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ function convertFunction(parameters, propertySpec) {
} else {
expression = convertPropertyFunction(parameters, propertySpec);
}

if (expression[0] === 'curve' && expression[1][0] === 'step' && expression.length === 4) {
// degenerate step curve (i.e. a constant function): add a noop stop
expression.push(0);
expression.push(expression[3]);
}
} else {
// identity function
expression = annotateValue(['get', parameters.property], propertySpec);
Expand Down Expand Up @@ -115,16 +121,17 @@ function convertZoomAndPropertyFunction(parameters, propertySpec) {
// otherwise.
const functionType = getFunctionType({}, propertySpec);
let interpolationType;
let isStep = false;
if (functionType === 'exponential') {
interpolationType = ['linear'];
} else {
interpolationType = ['step'];
isStep = true;
}
const expression = ['curve', interpolationType, ['zoom']];

for (const z of zoomStops) {
expression.push(z);
expression.push(convertPropertyFunction(featureFunctions[z], propertySpec));
appendStopPair(expression, z, convertPropertyFunction(featureFunctions[z], propertySpec), isStep);
}

return expression;
Expand All @@ -143,6 +150,7 @@ function convertPropertyFunction(parameters, propertySpec) {
let input = [inputType, ['get', parameters.property]];

let expression;
let isStep = false;
if (type === 'categorical' && inputType === 'boolean') {
assert(parameters.stops.length > 0 && parameters.stops.length <= 2);
if (parameters.stops[0][0] === false) {
Expand All @@ -159,6 +167,7 @@ function convertPropertyFunction(parameters, propertySpec) {
expression = ['match', input];
} else if (type === 'interval') {
expression = ['curve', ['step'], input];
isStep = true;
} else if (type === 'exponential') {
const base = parameters.base !== undefined ? parameters.base : 1;
expression = ['curve', ['exponential', base], input];
Expand All @@ -167,8 +176,7 @@ function convertPropertyFunction(parameters, propertySpec) {
}

for (const stop of parameters.stops) {
expression.push(stop[0]);
expression.push(stop[1]);
appendStopPair(expression, stop[0], stop[1], isStep);
}

if (expression[0] === 'match') {
Expand All @@ -181,8 +189,10 @@ function convertPropertyFunction(parameters, propertySpec) {
function convertZoomFunction(parameters, propertySpec) {
const type = getFunctionType(parameters, propertySpec);
let expression;
let isStep = false;
if (type === 'interval') {
expression = ['curve', ['step'], ['zoom']];
isStep = true;
} else if (type === 'exponential') {
const base = parameters.base !== undefined ? parameters.base : 1;
expression = ['curve', ['exponential', base], ['zoom']];
Expand All @@ -191,13 +201,20 @@ function convertZoomFunction(parameters, propertySpec) {
}

for (const stop of parameters.stops) {
expression.push(stop[0]);
expression.push(stop[1]);
appendStopPair(expression, stop[0], stop[1], isStep);
}

return expression;
}

function appendStopPair(curve, input, output, isStep) {
// step curves don't get the first input value, as it is redundant.
if (!(isStep && curve.length === 3)) {
curve.push(input);
}
curve.push(output);
}

function getFunctionType (parameters, propertySpec) {
if (parameters.type) {
return parameters.type;
Expand Down
32 changes: 22 additions & 10 deletions src/style-spec/function/definitions/curve.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,6 @@ class Curve implements Expression {
}

static parse(args: Array<mixed>, context: ParsingContext) {
if (args.length < 5)
return context.error(`Expected at least 4 arguments, but found only ${args.length - 1}.`);

if (args.length % 2 !== 1) {
return context.error("Expected an even number of arguments.");
}

let [ , interpolation, input, ...rest] = args;

if (!Array.isArray(interpolation) || interpolation.length === 0) {
Expand Down Expand Up @@ -98,6 +91,17 @@ class Curve implements Expression {
return context.error(`Unknown interpolation type ${String(interpolation[0])}`, 1, 0);
}

const isStep = interpolation.name === 'step';

const minArgs = isStep ? 5 : 4;
if (args.length - 1 < minArgs)
return context.error(`Expected at least ${minArgs} arguments, but found only ${args.length - 1}.`);

const parity = minArgs % 2;
if ((args.length - 1) % 2 !== parity) {
return context.error(`Expected an ${parity === 0 ? 'even' : 'odd'} number of arguments.`);
}

input = parseExpression(input, context.concat(2, NumberType));
if (!input) return null;

Expand All @@ -107,19 +111,27 @@ class Curve implements Expression {
if (context.expectedType && context.expectedType.kind !== 'Value') {
outputType = context.expectedType;
}

if (isStep) {
rest.unshift(-Infinity);
}

for (let i = 0; i < rest.length; i += 2) {
const label = rest[i];
const value = rest[i + 1];

const labelKey = isStep ? i + 4 : i + 3;
const valueKey = isStep ? i + 5 : i + 4;

if (typeof label !== 'number') {
return context.error('Input/output pairs for "curve" expressions must be defined using literal numeric values (not computed expressions) for the input values.', i + 3);
return context.error('Input/output pairs for "curve" expressions must be defined using literal numeric values (not computed expressions) for the input values.', labelKey);
}

if (stops.length && stops[stops.length - 1][0] > label) {
return context.error('Input/output pairs for "curve" expressions must be arranged with input values in strictly ascending order.', i + 3);
return context.error('Input/output pairs for "curve" expressions must be arranged with input values in strictly ascending order.', labelKey);
}

const parsed = parseExpression(value, context.concat(i + 4, outputType));
const parsed = parseExpression(value, context.concat(valueKey, outputType));
if (!parsed) return null;
outputType = outputType || parsed.type;
stops.push([label, parsed]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
"curve",
["step"],
["number", ["get", "x"]],
0,
["literal", ["one"]],
10,
["literal", ["one", "two"]]
Expand Down
2 changes: 1 addition & 1 deletion test/integration/expression-tests/curve/step/test.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"expectExpressionType": null,
"expression": [
"number",
["curve", ["step"], ["number", ["get", "x"]], -1, 11, 0, 111, 1, 1111]
["curve", ["step"], ["number", ["get", "x"]], 11, 0, 111, 1, 1111]
],
"inputs": [
[{}, {"properties": {"x": -1.5}}],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@
"line-opacity": {
"expression": [
"curve", ["step"], ["zoom"],
0,
["match", ["string", ["get", "class"]],
"motorway", 1,
"trunk", 0.25,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
"text-font": {
"expression": [
"curve", ["step"], ["zoom"],
0,
[ "literal", [ "Open Sans Semibold", "Arial Unicode MS Bold" ]],
1,
["literal", [ "Open Sans Semibold", "Arial Unicode MS Bold" ]]
Expand Down

0 comments on commit 354011e

Please sign in to comment.