Skip to content

Commit

Permalink
Introduce clusterProperties option for aggregated cluster properties (#…
Browse files Browse the repository at this point in the history
…7584)

* introduce clusterProperties option for aggregating cluster properties

* fix flow; more robust error handling

* fix unit tests

* add docs for clusterProperties

* add a render test for clusterProperties

* move cluster properties validation logic to style-spec; allow custom reduce fn

* expose accumulated expression in the docs
  • Loading branch information
mourner authored Jan 10, 2019
1 parent 2119eaa commit 47925a6
Show file tree
Hide file tree
Showing 14 changed files with 241 additions and 26 deletions.
13 changes: 9 additions & 4 deletions debug/cluster.html
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

var map = window.map = new mapboxgl.Map({
container: 'map',
zoom: 0,
zoom: 1,
center: [0, 0],
style: 'mapbox://styles/mapbox/cjf4m44iw0uza2spb3q0a7s41',
hash: true
Expand All @@ -31,15 +31,20 @@
"type": "geojson",
"data": "/test/integration/data/places.geojson",
"cluster": true,
"clusterRadius": 50
"clusterRadius": 50,
"clusterProperties": {
"max": ["max", 0, ["get", "scalerank"]],
"sum": ["+", 0, ["get", "scalerank"]],
"has_island": ["any", false, ["==", ["get", "featureclass"], "island"]]
}
});
map.addLayer({
"id": "cluster",
"type": "circle",
"source": "geojson",
"filter": ["==", "cluster", true],
"paint": {
"circle-color": "rgba(0, 200, 0, 1)",
"circle-color": ["case", ["get", "has_island"], "orange", "rgba(0, 200, 0, 1)"],
"circle-radius": 20
}
});
Expand All @@ -49,7 +54,7 @@
"source": "geojson",
"filter": ["==", "cluster", true],
"layout": {
"text-field": "{point_count}",
"text-field": "{point_count} ({max})",
"text-font": ["Open Sans Semibold", "Arial Unicode MS Bold"],
"text-size": 12,
"text-allow-overlap": true,
Expand Down
3 changes: 2 additions & 1 deletion src/source/geojson_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ class GeoJSONSource extends Evented implements Source {
extent: EXTENT,
radius: (options.clusterRadius || 50) * scale,
log: false
}
},
clusterProperties: options.clusterProperties
}, options.workerOptions);
}

Expand Down
59 changes: 57 additions & 2 deletions src/source/geojson_worker_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import Supercluster from 'supercluster';
import geojsonvt from 'geojson-vt';
import assert from 'assert';
import VectorTileWorkerSource from './vector_tile_worker_source';
import { createExpression } from '../style-spec/expression';

import type {
WorkerTileParameters,
Expand All @@ -30,7 +31,8 @@ export type LoadGeoJSONParameters = {
source: string,
cluster: boolean,
superclusterOptions?: Object,
geojsonVtOptions?: Object
geojsonVtOptions?: Object,
clusterProperties?: Object
};

export type LoadGeoJSON = (params: LoadGeoJSONParameters, callback: ResponseCallback<Object>) => void;
Expand Down Expand Up @@ -171,7 +173,7 @@ class GeoJSONWorkerSource extends VectorTileWorkerSource {

try {
this._geoJSONIndex = params.cluster ?
new Supercluster(params.superclusterOptions).load(data.features) :
new Supercluster(getSuperclusterOptions(params)).load(data.features) :
geojsonvt(data, params.geojsonVtOptions);
} catch (err) {
return callback(err);
Expand Down Expand Up @@ -293,4 +295,57 @@ class GeoJSONWorkerSource extends VectorTileWorkerSource {
}
}

function getSuperclusterOptions({superclusterOptions, clusterProperties}) {
if (!clusterProperties || !superclusterOptions) return superclusterOptions;

const initialValues = {};
const mapExpressions = {};
const reduceExpressions = {};
const globals = {accumulated: null, zoom: 0};
const feature = {properties: null};
const propertyNames = Object.keys(clusterProperties);

for (const key of propertyNames) {
const [operator, initialExpression, mapExpression] = clusterProperties[key];

const initialExpressionParsed = createExpression(initialExpression);
const mapExpressionParsed = createExpression(mapExpression);
const reduceExpressionParsed = createExpression(
typeof operator === 'string' ? [operator, ['accumulated'], ['get', key]] : operator);

assert(initialExpressionParsed.result === 'success');
assert(mapExpressionParsed.result === 'success');
assert(reduceExpressionParsed.result === 'success');

initialValues[key] = (initialExpressionParsed.value: any).evaluate(globals);
mapExpressions[key] = mapExpressionParsed.value;
reduceExpressions[key] = reduceExpressionParsed.value;
}

superclusterOptions.initial = () => {
const properties = {};
for (const key of propertyNames) {
properties[key] = initialValues[key];
}
return properties;
};
superclusterOptions.map = (pointProperties) => {
feature.properties = pointProperties;
const properties = {};
for (const key of propertyNames) {
properties[key] = mapExpressions[key].evaluate(globals, feature);
}
return properties;
};
superclusterOptions.reduce = (accumulated, clusterProperties) => {
feature.properties = clusterProperties;
for (const key of propertyNames) {
globals.accumulated = accumulated[key];
accumulated[key] = reduceExpressions[key].evaluate(globals, feature);
}
};

return superclusterOptions;
}

export default GeoJSONWorkerSource;
5 changes: 5 additions & 0 deletions src/style-spec/expression/definitions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,11 @@ CompoundExpression.register(expressions, {
[],
(ctx) => ctx.globals.lineProgress || 0
],
'accumulated': [
ValueType,
[],
(ctx) => ctx.globals.accumulated === undefined ? null : ctx.globals.accumulated
],
'+': [
NumberType,
varargs(NumberType),
Expand Down
15 changes: 8 additions & 7 deletions src/style-spec/expression/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ export type GlobalProperties = $ReadOnly<{
zoom: number,
heatmapDensity?: number,
lineProgress?: number,
isSupportedScript?: (string) => boolean
isSupportedScript?: (string) => boolean,
accumulated?: Value
}>;

export class StyleExpression {
Expand All @@ -49,12 +50,12 @@ export class StyleExpression {
_warningHistory: {[key: string]: boolean};
_enumValues: ?{[string]: any};

constructor(expression: Expression, propertySpec: StylePropertySpecification) {
constructor(expression: Expression, propertySpec: ?StylePropertySpecification) {
this.expression = expression;
this._warningHistory = {};
this._evaluator = new EvaluationContext();
this._defaultValue = getDefaultValue(propertySpec);
this._enumValues = propertySpec.type === 'enum' ? propertySpec.values : null;
this._defaultValue = propertySpec ? getDefaultValue(propertySpec) : null;
this._enumValues = propertySpec && propertySpec.type === 'enum' ? propertySpec.values : null;
}

evaluateWithoutErrorHandling(globals: GlobalProperties, feature?: Feature, featureState?: FeatureState): any {
Expand Down Expand Up @@ -105,12 +106,12 @@ export function isExpression(expression: mixed) {
*
* @private
*/
export function createExpression(expression: mixed, propertySpec: StylePropertySpecification): Result<StyleExpression, Array<ParsingError>> {
const parser = new ParsingContext(definitions, [], getExpectedType(propertySpec));
export function createExpression(expression: mixed, propertySpec: ?StylePropertySpecification): Result<StyleExpression, Array<ParsingError>> {
const parser = new ParsingContext(definitions, [], propertySpec ? getExpectedType(propertySpec) : undefined);

// For string-valued properties, coerce to string at the top level rather than asserting.
const parsed = parser.parse(expression, undefined, undefined, undefined,
propertySpec.type === 'string' ? {typeAnnotation: 'coerce'} : undefined);
propertySpec && propertySpec.type === 'string' ? {typeAnnotation: 'coerce'} : undefined);

if (!parsed) {
assert(parser.errors.length > 0);
Expand Down
2 changes: 1 addition & 1 deletion src/style-spec/expression/parsing_context.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,5 +226,5 @@ function isConstant(expression: Expression) {
}

return isFeatureConstant(expression) &&
isGlobalPropertyConstant(expression, ['zoom', 'heatmap-density', 'line-progress', 'is-supported-script']);
isGlobalPropertyConstant(expression, ['zoom', 'heatmap-density', 'line-progress', 'accumulated', 'is-supported-script']);
}
13 changes: 13 additions & 0 deletions src/style-spec/reference/v8.json
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,10 @@
"type": "number",
"doc": "Max zoom on which to cluster points if clustering is enabled. Defaults to one zoom less than maxzoom (so that last zoom features are not clustered)."
},
"clusterProperties": {
"type": "*",
"doc": "An object defining custom properties on the generated clusters if clustering is enabled, aggregating values from clustered points. Has the form `{\"property_name\": [operator, initial_expression, map_expression]}`. `operator` is any expression function that accepts at least 2 operands (e.g. `\"+\"` or `\"max\"`) — it accumulates the property value from clusters/points the cluster contains; `initial_expression` evaluates the initial value of the property before accummulating other points/clusters; `map_expression` produces the value of a single point.\n\nExample: `{\"sum\": [\"+\", 0, [\"get\", \"scalerank\"]]}`.\n\nFor more advanced use cases, in place of `operator`, you can use a custom reduce expression that references a special `[\"accumulated\"]` value, e.g.:\n`{\"sum\": [[\"+\", [\"accumulated\"], [\"get\", \"sum\"]], 0, [\"get\", \"scalerank\"]]}`"
},
"lineMetrics": {
"type": "boolean",
"default": false,
Expand Down Expand Up @@ -2803,6 +2807,15 @@
}
}
},
"accumulated": {
"doc": "Gets the value of a cluster property accumulated so far. Can only be used in the `clusterProperties` option of a clustered GeoJSON source.",
"group": "Feature data",
"sdk-support": {
"basic functionality": {
"js": "0.53.0"
}
}
},
"+": {
"doc": "Returns the sum of the inputs.",
"group": "Math",
Expand Down
1 change: 1 addition & 0 deletions src/style-spec/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ export type GeoJSONSourceSpecification = {|
"cluster"?: boolean,
"clusterRadius"?: number,
"clusterMaxZoom"?: number,
"clusterProperties"?: mixed,
"lineMetrics"?: boolean,
"generateId"?: boolean
|}
Expand Down
19 changes: 15 additions & 4 deletions src/style-spec/validate/validate_expression.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import ValidationError from '../error/validation_error';

import { createExpression, createPropertyExpression } from '../expression';
import { deepUnbundle } from '../util/unbundle_jsonlint';
import { isStateConstant } from '../expression/is_constant';
import { isStateConstant, isGlobalPropertyConstant, isFeatureConstant } from '../expression/is_constant';

export default function validateExpression(options: any): Array<ValidationError> {
const expression = (options.expressionContext === 'property' ? createPropertyExpression : createExpression)(deepUnbundle(options.value), options.valueSpec);
Expand All @@ -14,19 +14,30 @@ export default function validateExpression(options: any): Array<ValidationError>
});
}

const expressionObj = (expression.value: any).expression || (expression.value: any)._styleExpression.expression;

if (options.expressionContext === 'property' && (options.propertyKey === 'text-font') &&
(expression.value: any)._styleExpression.expression.possibleOutputs().indexOf(undefined) !== -1) {
expressionObj.possibleOutputs().indexOf(undefined) !== -1) {
return [new ValidationError(options.key, options.value, `Invalid data expression for "${options.propertyKey}". Output values must be contained as literals within the expression.`)];
}

if (options.expressionContext === 'property' && options.propertyType === 'layout' &&
(!isStateConstant((expression.value: any)._styleExpression.expression))) {
(!isStateConstant(expressionObj))) {
return [new ValidationError(options.key, options.value, '"feature-state" data expressions are not supported with layout properties.')];
}

if (options.expressionContext === 'filter' && !isStateConstant((expression.value: any).expression)) {
if (options.expressionContext === 'filter' && !isStateConstant(expressionObj)) {
return [new ValidationError(options.key, options.value, '"feature-state" data expressions are not supported with filters.')];
}

if (options.expressionContext && options.expressionContext.indexOf('cluster') === 0) {
if (!isGlobalPropertyConstant(expressionObj, ['zoom', 'feature-state'])) {
return [new ValidationError(options.key, options.value, '"zoom" and "feature-state" expressions are not supported with cluster properties.')];
}
if (options.expressionContext === 'cluster-initial' && !isFeatureConstant(expressionObj)) {
return [new ValidationError(options.key, options.value, 'Feature data expressions are not supported with initial expression part of cluster properties.')];
}
}

return [];
}
35 changes: 29 additions & 6 deletions src/style-spec/validate/validate_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import ValidationError from '../error/validation_error';
import { unbundle } from '../util/unbundle_jsonlint';
import validateObject from './validate_object';
import validateEnum from './validate_enum';
import validateExpression from './validate_expression';

export default function validateSource(options) {
const value = options.value;
Expand All @@ -15,19 +16,19 @@ export default function validateSource(options) {
}

const type = unbundle(value.type);
let errors = [];
let errors;

switch (type) {
case 'vector':
case 'raster':
case 'raster-dem':
errors = errors.concat(validateObject({
errors = validateObject({
key,
value,
valueSpec: styleSpec[`source_${type.replace('-', '_')}`],
style: options.style,
styleSpec
}));
});
if ('url' in value) {
for (const prop in value) {
if (['type', 'url', 'tileSize'].indexOf(prop) < 0) {
Expand All @@ -38,13 +39,36 @@ export default function validateSource(options) {
return errors;

case 'geojson':
return validateObject({
errors = validateObject({
key,
value,
valueSpec: styleSpec.source_geojson,
style,
styleSpec
});
if (value.cluster) {
for (const prop in value.clusterProperties) {
const [operator, initialExpr, mapExpr] = value.clusterProperties[prop];
const reduceExpr = typeof operator === 'string' ? [operator, ['accumulated'], ['get', prop]] : operator;

errors.push(...validateExpression({
key: `${key}.${prop}.initial`,
value: initialExpr,
expressionContext: 'cluster-initial'
}));
errors.push(...validateExpression({
key: `${key}.${prop}.map`,
value: mapExpr,
expressionContext: 'cluster-map'
}));
errors.push(...validateExpression({
key: `${key}.${prop}.reduce`,
value: reduceExpr,
expressionContext: 'cluster-reduce'
}));
}
}
return errors;

case 'video':
return validateObject({
Expand All @@ -65,8 +89,7 @@ export default function validateSource(options) {
});

case 'canvas':
errors.push(new ValidationError(key, null, `Please use runtime APIs to add canvas sources, rather than including them in stylesheets.`, 'source.canvas'));
return errors;
return [new ValidationError(key, null, `Please use runtime APIs to add canvas sources, rather than including them in stylesheets.`, 'source.canvas')];

default:
return validateEnum({
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading

0 comments on commit 47925a6

Please sign in to comment.