Skip to content

Commit

Permalink
WIP: generalize expression/calculation-dependent properties
Browse files Browse the repository at this point in the history
  • Loading branch information
Lauren Budorick committed Mar 26, 2018
1 parent 375c6d0 commit 30dc066
Show file tree
Hide file tree
Showing 15 changed files with 115 additions and 20 deletions.
8 changes: 6 additions & 2 deletions build/generate-style-code.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ global.isDataDriven = function (property) {
return property['property-function'] === true;
};

global.isCalculationDependent = function(property) {
return !!property['expression-dependency'];
}

global.flowType = function (property) {
switch (property.type) {
case 'boolean':
Expand Down Expand Up @@ -43,7 +47,7 @@ global.propertyType = function (property) {
return `DataDrivenProperty<${flowType(property)}>`;
} else if (/-pattern$/.test(property.name) || property.name === 'line-dasharray') {
return `CrossFadedProperty<${flowType(property)}>`;
} else if (property.name === 'heatmap-color' || property.name === 'line-gradient') {
} else if (isCalculationDependent(property) && property.type === 'color') {
return `ColorRampProperty`;
} else {
return `DataConstantProperty<${flowType(property)}>`;
Expand Down Expand Up @@ -95,7 +99,7 @@ global.propertyValue = function (property, type) {
return `new DataDrivenProperty(styleSpec["${type}_${property.layerType}"]["${property.name}"])`;
} else if (/-pattern$/.test(property.name) || property.name === 'line-dasharray') {
return `new CrossFadedProperty(styleSpec["${type}_${property.layerType}"]["${property.name}"])`;
} else if (property.name === 'heatmap-color' || property.name === 'line-gradient') {
} else if (isCalculationDependent(property) && property.type === 'color') {
return `new ColorRampProperty(styleSpec["${type}_${property.layerType}"]["${property.name}"])`;
} else {
return `new DataConstantProperty(styleSpec["${type}_${property.layerType}"]["${property.name}"])`;
Expand Down
3 changes: 2 additions & 1 deletion src/data/bucket.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ export type BucketParameters<Layer: TypedStyleLayer> = {
zoom: number,
pixelRatio: number,
overscaling: number,
collisionBoxArray: CollisionBoxArray
collisionBoxArray: CollisionBoxArray,
lineMetrics: boolean
}

export type PopulateParameters = {
Expand Down
11 changes: 6 additions & 5 deletions src/data/bucket/line_bucket.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ class LineBucket implements Bucket {
this.indexArray = new TriangleIndexArray();
this.programConfigurations = new ProgramConfigurationSet(layoutAttributes, options.layers, options.zoom);
this.segments = new SegmentVector();
this.lineMetrics = options.lineMetrics;
}

populate(features: Array<IndexedFeature>, options: PopulateParameters) {
Expand Down Expand Up @@ -159,12 +160,12 @@ class LineBucket implements Bucket {

addLine(vertices: Array<Point>, feature: VectorTileFeature, join: string, cap: string, miterLimit: number, roundLimit: number) {
let lineDistances = null;
if (!!feature.properties &&
feature.properties.hasOwnProperty('$distance_start') &&
feature.properties.hasOwnProperty('$distance_end')) {
if (this.lineMetrics && !!feature.properties &&
feature.properties.hasOwnProperty('clip_start') && // TODO finalize names
feature.properties.hasOwnProperty('clip_end')) {
lineDistances = {
start: feature.properties.$distance_start,
end: feature.properties.$distance_end,
start: feature.properties.clip_start,
end: feature.properties.clip_end,
tileTotal: undefined
};
}
Expand Down
4 changes: 3 additions & 1 deletion src/source/geojson_source.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ class GeoJSONSource extends Evented implements Source {
this.tileSize = 512;
this.isTileClipped = true;
this.reparseOverscaled = true;
this.lineMetrics = !!options.lineMetrics;
this._removed = false;

this.dispatcher = dispatcher;
Expand Down Expand Up @@ -125,7 +126,7 @@ class GeoJSONSource extends Evented implements Source {
tolerance: (options.tolerance !== undefined ? options.tolerance : 0.375) * scale,
extent: EXTENT,
maxZoom: this.maxzoom,
lineMetrics: options.lineMetrics || false
lineMetrics: this.lineMetrics
},
superclusterOptions: {
maxZoom: options.clusterMaxZoom !== undefined ?
Expand Down Expand Up @@ -238,6 +239,7 @@ class GeoJSONSource extends Evented implements Source {
maxZoom: this.maxzoom,
tileSize: this.tileSize,
source: this.id,
lineMetrics: this.lineMetrics,
pixelRatio: browser.devicePixelRatio,
overscaling: tile.tileID.overscaleFactor(),
showCollisionBoxes: this.map.showCollisionBoxes
Expand Down
4 changes: 3 additions & 1 deletion src/source/worker_tile.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ class WorkerTile {
this.overscaling = params.overscaling;
this.showCollisionBoxes = params.showCollisionBoxes;
this.collectResourceTiming = !!params.collectResourceTiming;
this.lineMetrics = params.lineMetrics;
}

parse(data: VectorTile, layerIndex: StyleLayerIndex, actor: Actor, callback: WorkerTileCallback) {
Expand Down Expand Up @@ -108,7 +109,8 @@ class WorkerTile {
zoom: this.zoom,
pixelRatio: this.pixelRatio,
overscaling: this.overscaling,
collisionBoxArray: this.collisionBoxArray
collisionBoxArray: this.collisionBoxArray,
lineMetrics: this.lineMetrics
});

bucket.populate(features, options);
Expand Down
2 changes: 2 additions & 0 deletions src/style-spec/reference/v8.json
Original file line number Diff line number Diff line change
Expand Up @@ -3286,6 +3286,7 @@
"zoom-function": false,
"property-function": false,
"transition": false,
"expression-dependency": "line-progress",
"requires": [
{
"!": "line-dasharray"
Expand Down Expand Up @@ -3646,6 +3647,7 @@
"zoom-function": false,
"property-function": false,
"transition": false,
"expression-dependency": "heatmap-density",
"sdk-support": {
"basic functionality": {
"js": "0.41.0"
Expand Down
5 changes: 5 additions & 0 deletions src/style-spec/validate/validate_expression.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ export default function validateExpression(options: any) {
});
}

const expressionDependency = options.valueSpec['expression-dependency'];
if (expressionDependency && expressionDependency !== expression.value._styleExpression.expression.input.name) {
return new ValidationError(options.key, options.value, `Invalid data expression for ${options.propertyKey}. Expression must take ${expressionDependency} as an input.`);
}

if (options.expressionContext === 'property' && options.propertyKey === 'text-font' &&
(expression.value: any)._styleExpression.expression.possibleOutputs().indexOf(undefined) !== -1) {
return [new ValidationError(options.key, options.value, 'Invalid data expression for "text-font". Output values must be contained as literals within the expression.')];
Expand Down
2 changes: 1 addition & 1 deletion src/style-spec/validate/validate_function.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export default function validateFunction(options) {
if (options.styleSpec.$version >= 8) {
if (isPropertyFunction && !options.valueSpec['property-function']) {
errors.push(new ValidationError(options.key, options.value, 'property functions not supported'));
} else if (isZoomFunction && !options.valueSpec['zoom-function'] && options.objectKey !== 'heatmap-color' && options.objectKey !== 'line-gradient') {
} else if (isZoomFunction && !options.valueSpec['zoom-function'] && !options.valueSpec['expression-dependency']) {
errors.push(new ValidationError(options.key, options.value, 'zoom functions not supported'));
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/style/properties.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ type TimePoint = number;
*
* There are two main implementations of the interface -- one for properties that allow data-driven values,
* and one for properties that don't. There are a few "special case" implementations as well: one for properties
* which cross-fade between two values rather than interpolating, one for `heatmap-color` and `line-gradient`,
* and one for `light-position`.
* which cross-fade between two values rather than interpolating, one for color ramp properties which do very,
* little evaluation here in favor of doing so on the StyleLayer classes, and one for `light-position`.
*
* @private
*/
Expand Down
2 changes: 1 addition & 1 deletion test/expected/text-shaping-linebreak.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,4 @@
"left": -32.5,
"right": 32.5,
"writingMode": 1
}
}
2 changes: 1 addition & 1 deletion test/expected/text-shaping-newline.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,4 @@
"left": -32.5,
"right": 32.5,
"writingMode": 1
}
}
2 changes: 1 addition & 1 deletion test/expected/text-shaping-newlines-in-middle.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,4 @@
"left": -32.5,
"right": 32.5,
"writingMode": 1
}
}
64 changes: 64 additions & 0 deletions test/unit/style-spec/fixture/expression-dependency.input.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
{
"version": 8,
"sources": {
"geojson": {
"type": "geojson",
"lineMetrics": true,
"data": {}
}
},
"layers": [
{
"id": "line-gradient-good",
"type": "line",
"source": "geojson",
"paint": {
"line-gradient": [
"interpolate",
["linear"],
["line-progress"],
0, "#000000"
]
}
},
{
"id": "line-gradient-bad",
"type": "line",
"source": "geojson",
"paint": {
"line-gradient": [
"interpolate",
["linear"],
["wrong-input"],
0, "#000000"
]
}
},
{
"id": "heatmap-density-good",
"type": "heatmap",
"source": "geojson",
"paint": {
"heatmap-color": [
"interpolate",
["linear"],
["heatmap-density"],
0, "#000000"
]
}
},
{
"id": "heatmap-density-bad",
"type": "heatmap",
"source": "geojson",
"paint": {
"heatmap-color": [
"interpolate",
["linear"],
["wrong-input"],
0, "#000000"
]
}
}
]
}
18 changes: 18 additions & 0 deletions test/unit/style-spec/fixture/expression-dependency.output.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
[
{
"message": "sources.geojson: unknown property \"lineMetrics\"",
"line": 6
},
{
"message": "layers[0].paint.line-gradient: unknown property \"line-gradient\"",
"line": 16
},
{
"message": "layers[1].paint.line-gradient: unknown property \"line-gradient\"",
"line": 29
},
{
"message": "layers[3].paint.heatmap-color[2][0]: Unknown expression \"wrong-input\". If you wanted a literal array, use [\"literal\", [...]].",
"line": 55
}
]
4 changes: 0 additions & 4 deletions test/unit/style-spec/fixture/functions.output.json
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,5 @@
{
"message": "layers[53].paint.heatmap-color: zoom expressions not supported",
"line": 952
},
{
"message": "layers[54].paint.heatmap-color: zoom functions not supported",
"line": 960
}
]

0 comments on commit 30dc066

Please sign in to comment.