From a0e7287bfa4ec983182a4e53f97996537f0f02c3 Mon Sep 17 00:00:00 2001 From: Mike Barry Date: Tue, 23 Apr 2024 06:39:56 -0400 Subject: [PATCH 1/6] contour source and validation --- build/generate-style-spec.ts | 5 +- docs/src/routes/sources/index.tsx | 20 ++ src/index.test.ts | 4 +- src/reference/v8.json | 50 +++++ src/validate/validate_contour_source.test.ts | 187 ++++++++++++++++++ src/validate/validate_contour_source.ts | 80 ++++++++ src/validate/validate_source.ts | 12 +- .../style-spec/tests/layers.input.json | 9 + .../style-spec/tests/layers.output.json | 46 ++--- .../style-spec/tests/sources.input.json | 12 ++ .../style-spec/tests/sources.output.json | 10 +- 11 files changed, 407 insertions(+), 28 deletions(-) create mode 100644 src/validate/validate_contour_source.test.ts create mode 100644 src/validate/validate_contour_source.ts diff --git a/build/generate-style-spec.ts b/build/generate-style-spec.ts index 1e8ed3dc0..8ee23f69d 100644 --- a/build/generate-style-spec.ts +++ b/build/generate-style-spec.ts @@ -320,10 +320,13 @@ ${objectDeclaration('TerrainSpecification', spec.terrain)} ${spec.source.map(key => { let str = objectDeclaration(sourceTypeName(key), spec[key]); + // This are done in order to overcome the type system's inability to express these types: if (sourceTypeName(key) === 'GeoJSONSourceSpecification') { - // This is done in order to overcome the type system's inability to express this type: str = str.replace(/unknown/, 'GeoJSON.GeoJSON | string'); } + if (sourceTypeName(key) === 'ContourSourceSpecification') { + str = str.replace(/unknown/, '"meters" | "feet" | number'); + } return str; }).join('\n\n')} diff --git a/docs/src/routes/sources/index.tsx b/docs/src/routes/sources/index.tsx index 94e3ecfbb..ec6c7082b 100644 --- a/docs/src/routes/sources/index.tsx +++ b/docs/src/routes/sources/index.tsx @@ -8,6 +8,7 @@ function Sources() { 'vector', 'raster', 'raster-dem', + 'contour', 'geojson', 'image', 'video' @@ -132,6 +133,25 @@ A raster DEM source. Only supports [Mapbox Terrain RGB](https://blog.mapbox.com/ /> + + + =v8) or scalars (<=v7). If objects, check that doc key diff --git a/src/reference/v8.json b/src/reference/v8.json index 517b5ac6c..29ebb735f 100644 --- a/src/reference/v8.json +++ b/src/reference/v8.json @@ -138,6 +138,7 @@ "source_vector", "source_raster", "source_raster_dem", + "source_contour", "source_geojson", "source_video", "source_image" @@ -426,6 +427,55 @@ "doc": "Other keys to configure the data source." } }, + "source_contour": { + "type": { + "required": true, + "type": "enum", + "values": { + "contour": { + "doc": "Contour lines derived from a [`raster-dem`](#raster-dem) source" + } + }, + "doc": "The type of the source." + }, + "source": { + "type": "string", + "doc": "ID of the [`raster-dem`](#raster-dem) source for the contour lines.", + "required": true + }, + "unit": { + "type": "*", + "default": "meters", + "doc": "Elevation unit of the generated contour lines: `\"feet\"`, `\"meters\"`, or a number to multiply by the raw elevation in meters for a custom unit." + }, + "intervals": { + "type": "array", + "value": "number", + "default": [100], + "doc": "Vertical spacing between contour lines by zoom, in the unit specified: `[default_value, zoom_stop_1, value_at_or_above_zoom_stop_1, zoom_stop_2, value_at_or_above_zoom_stop_2, ...]`" + }, + "majorMultiplier": { + "type": "array", + "value": "number", + "default": [5], + "doc": "Set `major=true` tag on every Nth contour line, which can vary by zoom: `[default_value, zoom_stop_1, value_at_or_above_zoom_stop_1, zoom_stop_2, value_at_or_above_zoom_stop_2, ...]`" + }, + "minzoom": { + "type": "number", + "default": 0, + "doc": "Minimum zoom level for which tiles are available. By default, this will be inherited from the `raster-dem` source." + }, + "maxzoom": { + "type": "number", + "default": 22, + "doc": "Maximum zoom level for which tiles are available. When zoomed in past `maxzoom` for the `raster-dem` source, contours will be generated by smoothly overzooming the DEM tiles." + }, + "overzoom": { + "type": "number", + "default": 1, + "doc": "Generate contours at each zoom from `raster-dem` tiles at a lower zoom level so they appear smoother and to limit overfetching border tiles. For example `overzoom: 1` uses z13 `raster-dem` tiles to render z14 contour lines." + } + }, "source_geojson": { "type": { "required": true, diff --git a/src/validate/validate_contour_source.test.ts b/src/validate/validate_contour_source.test.ts new file mode 100644 index 000000000..2c93fbcb4 --- /dev/null +++ b/src/validate/validate_contour_source.test.ts @@ -0,0 +1,187 @@ +import validateSpec from './validate'; +import v8 from '../reference/v8.json' assert {type: 'json'}; +import validateContourSource from './validate_contour_source'; +import {ContourSourceSpecification, StyleSpecification} from '../types.g'; + +function checkErrorMessage(message: string, key: string, expectedType: string, foundType: string) { + expect(message).toContain(key); + expect(message).toContain(expectedType); + expect(message).toContain(foundType); +} + +describe('Validate source_contour', () => { + test('Should pass when value is undefined', () => { + const errors = validateContourSource({validateSpec, value: undefined, styleSpec: v8, style: {} as any}); + expect(errors).toHaveLength(0); + }); + + test('Should return error when value is not an object', () => { + const errors = validateContourSource({validateSpec, value: '' as unknown as ContourSourceSpecification, styleSpec: v8, style: {} as any}); + expect(errors).toHaveLength(1); + expect(errors[0].message).toContain('object'); + expect(errors[0].message).toContain('expected'); + }); + + test('Should return error in case of unknown property', () => { + const errors = validateContourSource({validateSpec, value: {a: 1} as any, styleSpec: v8, style: {} as any}); + expect(errors).toHaveLength(2); + expect(errors[1].message).toContain('a'); + expect(errors[1].message).toContain('unknown'); + }); + + test('Should return errors according to spec violations', () => { + const errors = validateContourSource({ + validateSpec, + value: {type: 'contour', source: {} as any, unit: 'garbage' as any, intervals: {} as any, majorMultiplier: {} as any, maxzoom: '1' as any, minzoom: '2' as any, overzoom: '3' as any}, styleSpec: v8, style: {} as any, + sourceName: 'contour-source' + }); + expect(errors).toHaveLength(8); + checkErrorMessage(errors[0].message, 'source', 'raster-dem', 'contour-source'); + checkErrorMessage(errors[1].message, 'source', 'string', 'object'); + checkErrorMessage(errors[2].message, 'unit', '[meters, feet] or number', 'garbage'); + checkErrorMessage(errors[3].message, 'intervals', 'array', 'object'); + checkErrorMessage(errors[4].message, 'majorMultiplier', 'array', 'object'); + checkErrorMessage(errors[5].message, 'maxzoom', 'number', 'string'); + checkErrorMessage(errors[6].message, 'minzoom', 'number', 'string'); + checkErrorMessage(errors[7].message, 'overzoom', 'number', 'string'); + }); + + test('Should return errors if interval or major definitions are malformed', () => { + const contour: ContourSourceSpecification = { + type: 'contour', + source: 'dem', + unit: 1.5, + intervals: [500, 12], + majorMultiplier: [5, 12, 4, 11, 9] + }; + const style: StyleSpecification = { + sources: { + dem: { + type: 'raster-dem', + maxzoom: 11 + }, + contour + }, + version: 8, + layers: [] + }; + const errors = validateContourSource({ + validateSpec, + value: contour, + styleSpec: v8, + style + }); + expect(errors).toHaveLength(2); + checkErrorMessage(errors[0].message, 'intervals', 'odd', '2'); + checkErrorMessage(errors[1].message, 'majorMultiplier', 'strictly', 'ascending'); + }); + + test('Should return errors when source is missing', () => { + const contour: ContourSourceSpecification = { + type: 'contour', + source: 'dem', + unit: 'feet', + }; + const style: StyleSpecification = { + sources: { + contour + }, + version: 8, + layers: [] + }; + const errors = validateContourSource({ + validateSpec, + value: contour, + styleSpec: v8, + style, + sourceName: 'contour' + }); + expect(errors).toHaveLength(1); + checkErrorMessage(errors[0].message, 'source', 'raster-dem', 'contour'); + }); + + test('Should return errors when source has wrong type', () => { + const contour: ContourSourceSpecification = { + type: 'contour', + source: 'dem', + unit: 'feet', + }; + const style: StyleSpecification = { + sources: { + dem: { + type: 'raster', + maxzoom: 11 + }, + contour + }, + version: 8, + layers: [] + }; + const errors = validateContourSource({ + validateSpec, + value: contour, + styleSpec: v8, + style, + sourceName: 'contour' + }); + expect(errors).toHaveLength(1); + checkErrorMessage(errors[0].message, 'source', 'raster-dem', 'contour'); + }); + + test('Should pass if everything is according to spec', () => { + const contour: ContourSourceSpecification = { + type: 'contour', + source: 'dem', + intervals: [500], + majorMultiplier: [5, 12, 4], + maxzoom: 16, + minzoom: 4, + overzoom: 2, + unit: 'feet', + }; + const style: StyleSpecification = { + sources: { + dem: { + type: 'raster-dem', + maxzoom: 11 + }, + contour + }, + version: 8, + layers: [] + }; + const errors = validateContourSource({ + validateSpec, + value: contour, + styleSpec: v8, + style + }); + expect(errors).toHaveLength(0); + }); + + test('Should pass if everything is according to spec using numeric unit', () => { + const contour: ContourSourceSpecification = { + type: 'contour', + source: 'dem', + unit: 1.5, + }; + const style: StyleSpecification = { + sources: { + dem: { + type: 'raster-dem', + maxzoom: 11 + }, + contour + }, + version: 8, + layers: [] + }; + const errors = validateContourSource({ + validateSpec, + value: contour, + styleSpec: v8, + style + }); + expect(errors).toHaveLength(0); + }); +}); diff --git a/src/validate/validate_contour_source.ts b/src/validate/validate_contour_source.ts new file mode 100644 index 000000000..114503d16 --- /dev/null +++ b/src/validate/validate_contour_source.ts @@ -0,0 +1,80 @@ +import ValidationError from '../error/validation_error'; +import getType from '../util/get_type'; +import type {ContourSourceSpecification, StyleSpecification} from '../types.g'; +import v8 from '../reference/v8.json' assert {type: 'json'}; +import {deepUnbundle, unbundle} from '../util/unbundle_jsonlint'; + +interface ValidateContourSourceOptions { + sourceName?: string; + value: ContourSourceSpecification; + styleSpec: typeof v8; + style: StyleSpecification; + validateSpec: Function; +} + +export default function validateContourSource( + options: ValidateContourSourceOptions +): ValidationError[] { + + const contour = options.value; + const styleSpec = options.styleSpec; + const contourSpec = styleSpec.source_contour; + const style = options.style; + + const errors = []; + + const rootType = getType(contour); + if (contour === undefined) { + return errors; + } else if (rootType !== 'object') { + errors.push(new ValidationError('source_contour', contour, `object expected, ${rootType} found`)); + return errors; + } + + const source = contour.source; + const sourceType = unbundle(style.sources?.[source]?.type); + if (!source) { + errors.push(new ValidationError('source_contour', contour, '"source" is required')); + } else if (sourceType !== 'raster-dem') { + errors.push(new ValidationError('source', source, `${options.sourceName} requires a raster-dem source`)); + } + + for (const key in contour) { + const value = deepUnbundle(contour[key]); + if (key === 'unit') { + if (typeof value !== 'number' && value !== 'meters' && value !== 'feet') { + errors.push(new ValidationError(key, value, `[meters, feet] or number expected, ${JSON.stringify(value)} found`)); + } + } else if (contourSpec[key]) { + const newErrors = options.validateSpec({ + key, + value, + valueSpec: contourSpec[key], + validateSpec: options.validateSpec, + style, + styleSpec + }); + errors.push(...newErrors); + if ((key === 'intervals' || key === 'majorMultiplier') && newErrors.length === 0 && Array.isArray(value)) { + if (value.length < 1) { + errors.push(new ValidationError(key, value, `expected at least 1 argument but found ${value.length}`)); + } else if (value.length % 2 !== 1) { + errors.push(new ValidationError(key, value, `expected an odd number of arguments but found ${value.length}`)); + } else { + let last = 0; + for (let i = 1; i < value.length; i += 2) { + const curr = value[i]; + if (curr <= last) { + errors.push(new ValidationError(key, value, 'zoom stops must be arranged in strictly ascending order')); + } + last = curr; + } + } + } + } else { + errors.push(new ValidationError(key, value, `unknown property "${key}"`)); + } + } + + return errors; +} diff --git a/src/validate/validate_source.ts b/src/validate/validate_source.ts index a3ff80244..3d49f172a 100644 --- a/src/validate/validate_source.ts +++ b/src/validate/validate_source.ts @@ -7,6 +7,7 @@ import validateExpression from './validate_expression'; import validateString from './validate_string'; import getType from '../util/get_type'; import validateRasterDEMSource from './validate_raster_dem_source'; +import validateContourSource from './validate_contour_source'; const objectElementValidators = { promoteId: validatePromoteId @@ -48,6 +49,15 @@ export default function validateSource(options) { validateSpec, }); return errors; + case 'contour': + errors = validateContourSource({ + sourceName: key, + value, + style: options.style, + styleSpec, + validateSpec, + }); + return errors; case 'geojson': errors = validateObject({ @@ -107,7 +117,7 @@ export default function validateSource(options) { return validateEnum({ key: `${key}.type`, value: value.type, - valueSpec: {values: ['vector', 'raster', 'raster-dem', 'geojson', 'video', 'image']}, + valueSpec: {values: ['vector', 'raster', 'raster-dem', 'contour', 'geojson', 'video', 'image']}, style, validateSpec, styleSpec diff --git a/test/integration/style-spec/tests/layers.input.json b/test/integration/style-spec/tests/layers.input.json index ab32cc976..8b673a96c 100644 --- a/test/integration/style-spec/tests/layers.input.json +++ b/test/integration/style-spec/tests/layers.input.json @@ -13,6 +13,10 @@ "type": "raster-dem", "url": "https://demotiles.maplibre.org/terrain-tiles/tiles.json" }, + "contour": { + "type": "contour", + "source": "raster-dem" + }, "geojson": { "type": "geojson", "data": {} @@ -178,6 +182,11 @@ "type": "custom", "source": "vector", "source-layer": "layer" + }, + { + "id": "contour-line", + "type": "line", + "source": "contour" } ] } diff --git a/test/integration/style-spec/tests/layers.output.json b/test/integration/style-spec/tests/layers.output.json index 6b18951de..3fcae032e 100644 --- a/test/integration/style-spec/tests/layers.output.json +++ b/test/integration/style-spec/tests/layers.output.json @@ -1,90 +1,90 @@ [ { "message": "layers[0]: either \"type\" or \"ref\" is required", - "line": 27 + "line": 31 }, { "message": "layers[1]: missing required property \"id\"", - "line": 32 + "line": 36 }, { "message": "layers[3]: \"type\" is prohibited for ref layers", - "line": 46 + "line": 50 }, { "message": "layers[3]: \"source\" is prohibited for ref layers", - "line": 47 + "line": 51 }, { "message": "layers[3]: \"source-layer\" is prohibited for ref layers", - "line": 48 + "line": 52 }, { "message": "layers[3]: \"filter\" is prohibited for ref layers", - "line": 49 + "line": 53 }, { "message": "layers[3]: \"layout\" is prohibited for ref layers", - "line": 50 + "line": 54 }, { "message": "layers[4]: ref layer \"not-found\" not found", - "line": 54 + "line": 58 }, { "message": "layers[5]: ref cannot reference another ref layer", - "line": 58 + "line": 62 }, { "message": "layers[6]: missing required property \"source\"", - "line": 60 + "line": 64 }, { "message": "layers[7]: source \"not-found\" not found", - "line": 67 + "line": 71 }, { "message": "layers[8]: layer \"vector-raster-mismatch\" requires a vector source", - "line": 72 + "line": 76 }, { "message": "layers[9]: layer \"raster-vector-mismatch\" requires a raster source", - "line": 77 + "line": 81 }, { "message": "layers[10]: layer \"raster-hillshade-mismatch\" requires a raster-dem source", - "line": 83 + "line": 87 }, { "message": "layers[11]: layer \"vector-hillshade-mismatch\" requires a raster-dem source", - "line": 89 + "line": 93 }, { "message": "layers[12]: raster-dem source can only be used with layer type 'hillshade'.", - "line": 95 + "line": 99 }, { - "message": "layers[14]: duplicate layer id \"duplicate\", previously used at line 99", - "line": 105 + "message": "layers[14]: duplicate layer id \"duplicate\", previously used at line 103", + "line": 109 }, { "message": "layers[15].type: expected one of [fill, line, symbol, circle, heatmap, fill-extrusion, raster, hillshade, background], \"invalid\" found", - "line": 112 + "line": 116 }, { "message": "layers[16]: layer \"missing-source-layer\" must specify a \"source-layer\"", - "line": 122 + "line": 126 }, { "message": "layers[18]: layer \"line-gradient-bad\" specifies a line-gradient, which requires a GeoJSON source with `lineMetrics` enabled.", - "line": 134 + "line": 138 }, { "message": "layers[19]: layer \"line-gradient-missing-lineMetrics\" specifies a line-gradient, which requires a GeoJSON source with `lineMetrics` enabled.", - "line": 148 + "line": 152 }, { "message": "layers[21].type: expected one of [fill, line, symbol, circle, heatmap, fill-extrusion, raster, hillshade, background], \"custom\" found", - "line": 178 + "line": 182 } ] \ No newline at end of file diff --git a/test/integration/style-spec/tests/sources.input.json b/test/integration/style-spec/tests/sources.input.json index b57e40ce7..1577e46a5 100644 --- a/test/integration/style-spec/tests/sources.input.json +++ b/test/integration/style-spec/tests/sources.input.json @@ -49,6 +49,18 @@ "zoom": ["+", ["zoom"]], "state": ["+", ["feature-state", "foo"]] } + }, + "raster-dem": { + "type": "raster-dem", + "url": "https://demotiles.maplibre.org/terrain-tiles/tiles.json" + }, + "contour-bad-input": { + "type": "contour", + "source": "nonexistant" + }, + "contour-bad-source-type": { + "type": "contour", + "source": "canvas" } }, "layers": [] diff --git a/test/integration/style-spec/tests/sources.output.json b/test/integration/style-spec/tests/sources.output.json index 8fc1e92e9..dd3ee4c33 100644 --- a/test/integration/style-spec/tests/sources.output.json +++ b/test/integration/style-spec/tests/sources.output.json @@ -4,7 +4,7 @@ "line": 4 }, { - "message": "sources.invalid-type.type: expected one of [vector, raster, raster-dem, geojson, video, image], \"invalid\" found", + "message": "sources.invalid-type.type: expected one of [vector, raster, raster-dem, contour, geojson, video, image], \"invalid\" found", "line": 7 }, { @@ -38,5 +38,13 @@ { "message": "sources.cluster-properties.state.map: \"zoom\" and \"feature-state\" expressions are not supported with cluster properties.", "line": 50 + }, + { + "message": "source: sources.contour-bad-input requires a raster-dem source", + "line": 59 + }, + { + "message": "source: sources.contour-bad-source-type requires a raster-dem source", + "line": 63 } ] \ No newline at end of file From 30a936cf4069559d371714a42bba16139ed15985 Mon Sep 17 00:00:00 2001 From: Mike Barry Date: Tue, 23 Apr 2024 06:44:36 -0400 Subject: [PATCH 2/6] changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d76ef992..30ebf315b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,8 @@ ## main ### ✨ Features and improvements -- _...Add new stuff here..._ + +- Add new `contour` source type that renders contour lines from a `raster-dem` source [#623](https://github.com/maplibre/maplibre-style-spec/pull/623) ### 🐞 Bug fixes From 5849e4991ff6291a47cd4f547b2aea4815ece066 Mon Sep 17 00:00:00 2001 From: Mike Barry Date: Tue, 23 Apr 2024 06:48:58 -0400 Subject: [PATCH 3/6] add missing test --- src/validate/validate_contour_source.test.ts | 28 ++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/validate/validate_contour_source.test.ts b/src/validate/validate_contour_source.test.ts index 2c93fbcb4..8c8a60a65 100644 --- a/src/validate/validate_contour_source.test.ts +++ b/src/validate/validate_contour_source.test.ts @@ -76,6 +76,34 @@ describe('Validate source_contour', () => { checkErrorMessage(errors[1].message, 'majorMultiplier', 'strictly', 'ascending'); }); + test('Should return errors if interval array is empty', () => { + const contour: ContourSourceSpecification = { + type: 'contour', + source: 'dem', + unit: 1.5, + intervals: [], + }; + const style: StyleSpecification = { + sources: { + dem: { + type: 'raster-dem', + maxzoom: 11 + }, + contour + }, + version: 8, + layers: [] + }; + const errors = validateContourSource({ + validateSpec, + value: contour, + styleSpec: v8, + style + }); + expect(errors).toHaveLength(1); + checkErrorMessage(errors[0].message, 'intervals', 'at least 1', '0'); + }); + test('Should return errors when source is missing', () => { const contour: ContourSourceSpecification = { type: 'contour', From 61cd4a9da1ff55ef150a0388dbf6fa1fc3a2ba18 Mon Sep 17 00:00:00 2001 From: Mike Barry Date: Thu, 25 Apr 2024 20:19:04 -0400 Subject: [PATCH 4/6] expression --- build/generate-style-spec.ts | 3 +- src/reference/v8.json | 14 +-- src/validate/validate_contour_source.test.ts | 120 +++++++++++++------ src/validate/validate_contour_source.ts | 29 ++--- src/validate/validate_expression.ts | 8 +- 5 files changed, 107 insertions(+), 67 deletions(-) diff --git a/build/generate-style-spec.ts b/build/generate-style-spec.ts index 8ee23f69d..81b462b98 100644 --- a/build/generate-style-spec.ts +++ b/build/generate-style-spec.ts @@ -325,7 +325,8 @@ ${spec.source.map(key => { str = str.replace(/unknown/, 'GeoJSON.GeoJSON | string'); } if (sourceTypeName(key) === 'ContourSourceSpecification') { - str = str.replace(/unknown/, '"meters" | "feet" | number'); + str = str.replace(/("unit"\?: )unknown/, '$1"meters" | "feet" | number'); + str = str.replaceAll(/unknown/g, 'PropertyValueSpecification'); } return str; }).join('\n\n')} diff --git a/src/reference/v8.json b/src/reference/v8.json index 29ebb735f..5e2bd8979 100644 --- a/src/reference/v8.json +++ b/src/reference/v8.json @@ -449,16 +449,14 @@ "doc": "Elevation unit of the generated contour lines: `\"feet\"`, `\"meters\"`, or a number to multiply by the raw elevation in meters for a custom unit." }, "intervals": { - "type": "array", - "value": "number", - "default": [100], - "doc": "Vertical spacing between contour lines by zoom, in the unit specified: `[default_value, zoom_stop_1, value_at_or_above_zoom_stop_1, zoom_stop_2, value_at_or_above_zoom_stop_2, ...]`" + "type": "*", + "default": 100, + "doc": "Vertical spacing between contour lines in the unit specified. This can be a constant value like `100` or an expression that changes by zoom level like `[\"step\", [\"zoom\"], 100, 12, 50]` to use 100 at z11 and below or 50 at z12 and higher." }, "majorMultiplier": { - "type": "array", - "value": "number", - "default": [5], - "doc": "Set `major=true` tag on every Nth contour line, which can vary by zoom: `[default_value, zoom_stop_1, value_at_or_above_zoom_stop_1, zoom_stop_2, value_at_or_above_zoom_stop_2, ...]`" + "type": "*", + "default": 5, + "doc": "Set `major=true` tag on every Nth contour line to help create \"index\" contours. This can be a constant like `5` or an expression that changes by zoom level like `[\"step\", [\"zoom\"], 5, 12, 10]` to use 5 at z11 and below or 10 at z12 and higher." }, "minzoom": { "type": "number", diff --git a/src/validate/validate_contour_source.test.ts b/src/validate/validate_contour_source.test.ts index 8c8a60a65..b08e78996 100644 --- a/src/validate/validate_contour_source.test.ts +++ b/src/validate/validate_contour_source.test.ts @@ -1,7 +1,7 @@ import validateSpec from './validate'; import v8 from '../reference/v8.json' assert {type: 'json'}; import validateContourSource from './validate_contour_source'; -import {ContourSourceSpecification, StyleSpecification} from '../types.g'; +import {ContourSourceSpecification, PropertyValueSpecification, StyleSpecification} from '../types.g'; function checkErrorMessage(message: string, key: string, expectedType: string, foundType: string) { expect(message).toContain(key); @@ -39,8 +39,8 @@ describe('Validate source_contour', () => { checkErrorMessage(errors[0].message, 'source', 'raster-dem', 'contour-source'); checkErrorMessage(errors[1].message, 'source', 'string', 'object'); checkErrorMessage(errors[2].message, 'unit', '[meters, feet] or number', 'garbage'); - checkErrorMessage(errors[3].message, 'intervals', 'array', 'object'); - checkErrorMessage(errors[4].message, 'majorMultiplier', 'array', 'object'); + checkErrorMessage(errors[3].message, 'intervals', 'literal', 'Bare object'); + checkErrorMessage(errors[4].message, 'majorMultiplier', 'literal', 'Bare object'); checkErrorMessage(errors[5].message, 'maxzoom', 'number', 'string'); checkErrorMessage(errors[6].message, 'minzoom', 'number', 'string'); checkErrorMessage(errors[7].message, 'overzoom', 'number', 'string'); @@ -51,8 +51,8 @@ describe('Validate source_contour', () => { type: 'contour', source: 'dem', unit: 1.5, - intervals: [500, 12], - majorMultiplier: [5, 12, 4, 11, 9] + intervals: ['step', ['zoom'], 1, 10], + majorMultiplier: ['step', ['zoom'], 1, 10, 3, 9, 4] }; const style: StyleSpecification = { sources: { @@ -72,38 +72,10 @@ describe('Validate source_contour', () => { style }); expect(errors).toHaveLength(2); - checkErrorMessage(errors[0].message, 'intervals', 'odd', '2'); + checkErrorMessage(errors[0].message, 'intervals', 'at least 4 arguments', 'only 3'); checkErrorMessage(errors[1].message, 'majorMultiplier', 'strictly', 'ascending'); }); - test('Should return errors if interval array is empty', () => { - const contour: ContourSourceSpecification = { - type: 'contour', - source: 'dem', - unit: 1.5, - intervals: [], - }; - const style: StyleSpecification = { - sources: { - dem: { - type: 'raster-dem', - maxzoom: 11 - }, - contour - }, - version: 8, - layers: [] - }; - const errors = validateContourSource({ - validateSpec, - value: contour, - styleSpec: v8, - style - }); - expect(errors).toHaveLength(1); - checkErrorMessage(errors[0].message, 'intervals', 'at least 1', '0'); - }); - test('Should return errors when source is missing', () => { const contour: ContourSourceSpecification = { type: 'contour', @@ -137,8 +109,7 @@ describe('Validate source_contour', () => { const style: StyleSpecification = { sources: { dem: { - type: 'raster', - maxzoom: 11 + type: 'raster' }, contour }, @@ -160,8 +131,8 @@ describe('Validate source_contour', () => { const contour: ContourSourceSpecification = { type: 'contour', source: 'dem', - intervals: [500], - majorMultiplier: [5, 12, 4], + intervals: ['step', ['zoom'], 5, 10, 3], + majorMultiplier: 500, maxzoom: 16, minzoom: 4, overzoom: 2, @@ -212,4 +183,77 @@ describe('Validate source_contour', () => { }); expect(errors).toHaveLength(0); }); + + const goodExpressions: Array> = [ + 5, + ['step', ['zoom'], 100, 10, 50], + ['interpolate', ['linear'], ['zoom'], 1, 5, 10, 10], + ['*', ['zoom'], 10], + ['*', 2, 3], + ]; + + for (const expr of goodExpressions) { + test(`Expression should be allowed: ${JSON.stringify(expr)}`, () => { + const contour: ContourSourceSpecification = { + type: 'contour', + source: 'dem', + intervals: expr, + majorMultiplier: expr, + }; + const style: StyleSpecification = { + sources: { + dem: { + type: 'raster-dem' + }, + contour + }, + version: 8, + layers: [] + }; + expect(validateContourSource({ + validateSpec, + value: contour, + styleSpec: v8, + style + })).toHaveLength(0); + }); + } + + const badExpressions: Array> = [ + ['geometry-type'], + ['get', 'x'], + ['interpolate', ['linear'], ['get', 'prop'], 1, 5, 10, 10], + ['feature-state', 'key'], + ['step', ['zoom'], 100, 10, ['get', 'value']], + ]; + + for (const expr of badExpressions) { + test(`Expression should not be allowed: ${JSON.stringify(expr)}`, () => { + const contour: ContourSourceSpecification = { + type: 'contour', + source: 'dem', + intervals: expr, + majorMultiplier: expr, + }; + const style: StyleSpecification = { + sources: { + dem: { + type: 'raster-dem' + }, + contour + }, + version: 8, + layers: [] + }; + const errors = validateContourSource({ + validateSpec, + value: contour, + styleSpec: v8, + style + }); + expect(errors).toHaveLength(2); + checkErrorMessage(errors[0].message, 'intervals', '\"zoom\"-based expressions', 'contour source expressions'); + checkErrorMessage(errors[1].message, 'majorMultiplier', '\"zoom\"-based expressions', 'contour source expressions'); + }); + } }); diff --git a/src/validate/validate_contour_source.ts b/src/validate/validate_contour_source.ts index 114503d16..831833508 100644 --- a/src/validate/validate_contour_source.ts +++ b/src/validate/validate_contour_source.ts @@ -3,6 +3,7 @@ import getType from '../util/get_type'; import type {ContourSourceSpecification, StyleSpecification} from '../types.g'; import v8 from '../reference/v8.json' assert {type: 'json'}; import {deepUnbundle, unbundle} from '../util/unbundle_jsonlint'; +import validateExpression from './validate_expression'; interface ValidateContourSourceOptions { sourceName?: string; @@ -45,32 +46,22 @@ export default function validateContourSource( if (typeof value !== 'number' && value !== 'meters' && value !== 'feet') { errors.push(new ValidationError(key, value, `[meters, feet] or number expected, ${JSON.stringify(value)} found`)); } + } else if (key === 'intervals' || key === 'majorMultiplier') { + errors.push(...validateExpression({ + key, + value, + validateSpec: options.validateSpec, + expressionContext: `contour-${key}` + })); } else if (contourSpec[key]) { - const newErrors = options.validateSpec({ + errors.push(...options.validateSpec({ key, value, valueSpec: contourSpec[key], validateSpec: options.validateSpec, style, styleSpec - }); - errors.push(...newErrors); - if ((key === 'intervals' || key === 'majorMultiplier') && newErrors.length === 0 && Array.isArray(value)) { - if (value.length < 1) { - errors.push(new ValidationError(key, value, `expected at least 1 argument but found ${value.length}`)); - } else if (value.length % 2 !== 1) { - errors.push(new ValidationError(key, value, `expected an odd number of arguments but found ${value.length}`)); - } else { - let last = 0; - for (let i = 1; i < value.length; i += 2) { - const curr = value[i]; - if (curr <= last) { - errors.push(new ValidationError(key, value, 'zoom stops must be arranged in strictly ascending order')); - } - last = curr; - } - } - } + })); } else { errors.push(new ValidationError(key, value, `unknown property "${key}"`)); } diff --git a/src/validate/validate_expression.ts b/src/validate/validate_expression.ts index 2ab2012f9..de8653bec 100644 --- a/src/validate/validate_expression.ts +++ b/src/validate/validate_expression.ts @@ -33,7 +33,7 @@ export default function validateExpression(options: any): Array 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 (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.')]; } @@ -42,5 +42,11 @@ export default function validateExpression(options: any): Array } } + if (options.expressionContext?.indexOf('contour') === 0) { + if (!isGlobalPropertyConstant(expressionObj, ['feature-state']) || !isFeatureConstant(expressionObj)) { + return [new ValidationError(options.key, options.value, 'Only constant or "zoom"-based expressions are supported with contour source expressions.')]; + } + } + return []; } From 411074767dbab1cf9bf0bcce3065b8beee684a9d Mon Sep 17 00:00:00 2001 From: Mike Barry Date: Thu, 25 Apr 2024 20:33:41 -0400 Subject: [PATCH 5/6] tweak --- build/generate-docs.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/build/generate-docs.ts b/build/generate-docs.ts index b0d7263e4..29c1e7062 100644 --- a/build/generate-docs.ts +++ b/build/generate-docs.ts @@ -317,6 +317,26 @@ function createSourcesContent() { } } }, + 'contour': { + doc: 'Contour lines generated from a [\`raster-dem\`](#raster-dem) source.', + example: { + 'maplibre-terrain-rgb': { + 'type': 'raster-dem', + 'encoding': 'mapbox', + 'tiles': [ + 'http://a.example.com/dem-tiles/{z}/{x}/{y}.png' + ], + }, + 'contour': { + 'type': 'contour', + 'source': 'maplibre-terrain-rgb' + } + }, + 'sdk-support': { + 'basic functionality': { + } + } + }, geojson: { doc: 'A [GeoJSON](http://geojson.org/) source. Data must be provided via a \`"data"\` property, whose value can be a URL or inline GeoJSON. When using in a browser, the GeoJSON data must be on the same domain as the map or served with [CORS](https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS) headers.', example: { From 101e4fc52f5bcea7a3102dd9084053dd8fa5cb7f Mon Sep 17 00:00:00 2001 From: Mike Barry Date: Sun, 5 May 2024 14:38:15 -0400 Subject: [PATCH 6/6] remove minzoom/maxzoom --- src/reference/v8.json | 12 +----------- src/validate/validate_contour_source.test.ts | 10 +++------- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/src/reference/v8.json b/src/reference/v8.json index e0b12b149..d37dae44a 100644 --- a/src/reference/v8.json +++ b/src/reference/v8.json @@ -465,7 +465,7 @@ "unit": { "type": "*", "default": "meters", - "doc": "Elevation unit of the generated contour lines: `\"feet\"`, `\"meters\"`, or a number to multiply by the raw elevation in meters for a custom unit." + "doc": "Elevation unit of the generated contour lines: `\"feet\"`, `\"meters\"`, or a number to specify the length of a custom unit to divide raw elevation in meters by." }, "intervals": { "type": "*", @@ -477,16 +477,6 @@ "default": 5, "doc": "Set `major=true` tag on every Nth contour line to help create \"index\" contours. This can be a constant like `5` or an expression that changes by zoom level like `[\"step\", [\"zoom\"], 5, 12, 10]` to use 5 at z11 and below or 10 at z12 and higher." }, - "minzoom": { - "type": "number", - "default": 0, - "doc": "Minimum zoom level for which tiles are available. By default, this will be inherited from the `raster-dem` source." - }, - "maxzoom": { - "type": "number", - "default": 22, - "doc": "Maximum zoom level for which tiles are available. When zoomed in past `maxzoom` for the `raster-dem` source, contours will be generated by smoothly overzooming the DEM tiles." - }, "overzoom": { "type": "number", "default": 1, diff --git a/src/validate/validate_contour_source.test.ts b/src/validate/validate_contour_source.test.ts index b08e78996..edc52bf95 100644 --- a/src/validate/validate_contour_source.test.ts +++ b/src/validate/validate_contour_source.test.ts @@ -32,18 +32,16 @@ describe('Validate source_contour', () => { test('Should return errors according to spec violations', () => { const errors = validateContourSource({ validateSpec, - value: {type: 'contour', source: {} as any, unit: 'garbage' as any, intervals: {} as any, majorMultiplier: {} as any, maxzoom: '1' as any, minzoom: '2' as any, overzoom: '3' as any}, styleSpec: v8, style: {} as any, + value: {type: 'contour', source: {} as any, unit: 'garbage' as any, intervals: {} as any, majorMultiplier: {} as any, overzoom: '3' as any}, styleSpec: v8, style: {} as any, sourceName: 'contour-source' }); - expect(errors).toHaveLength(8); + expect(errors).toHaveLength(6); checkErrorMessage(errors[0].message, 'source', 'raster-dem', 'contour-source'); checkErrorMessage(errors[1].message, 'source', 'string', 'object'); checkErrorMessage(errors[2].message, 'unit', '[meters, feet] or number', 'garbage'); checkErrorMessage(errors[3].message, 'intervals', 'literal', 'Bare object'); checkErrorMessage(errors[4].message, 'majorMultiplier', 'literal', 'Bare object'); - checkErrorMessage(errors[5].message, 'maxzoom', 'number', 'string'); - checkErrorMessage(errors[6].message, 'minzoom', 'number', 'string'); - checkErrorMessage(errors[7].message, 'overzoom', 'number', 'string'); + checkErrorMessage(errors[5].message, 'overzoom', 'number', 'string'); }); test('Should return errors if interval or major definitions are malformed', () => { @@ -133,8 +131,6 @@ describe('Validate source_contour', () => { source: 'dem', intervals: ['step', ['zoom'], 5, 10, 3], majorMultiplier: 500, - maxzoom: 16, - minzoom: 4, overzoom: 2, unit: 'feet', };