From 8fd3fbc0b73ece03e1c264c84ad30f4c9b8d35b2 Mon Sep 17 00:00:00 2001 From: Pieter Vander Vennet Date: Tue, 16 May 2023 03:27:49 +0200 Subject: [PATCH] Refactoring: fix metatagging --- Logic/ExtraFunctions.ts | 89 ++++---- Logic/FeatureSource/Actors/GeoIndexedStore.ts | 4 +- Logic/MetaTagging.ts | 200 +++++++++--------- Models/ThemeConfig/DependencyCalculator.ts | 7 +- UI/BigComponents/BackgroundSwitcher.svelte | 90 ++++++++ UI/InputElement/ValidatedInput.svelte | 2 +- UI/Map/MaplibreMap.svelte | 2 +- .../TagRendering/TagRenderingAnswer.svelte | 2 +- .../TagRendering/TagRenderingEditable.svelte | 2 +- .../TagRendering/TagRenderingQuestion.svelte | 18 +- UI/ThemeViewGUI.svelte | 9 +- Utils.ts | 2 +- assets/layers/address/address.json | 2 +- .../layers/climbing_area/climbing_area.json | 28 +-- .../layers/drinking_water/drinking_water.json | 8 +- assets/layers/etymology/etymology.json | 4 +- assets/layers/note/note.json | 14 +- .../walls_and_buildings.json | 16 +- assets/themes/bag/bag.json | 6 +- assets/themes/buurtnatuur/buurtnatuur.json | 6 +- assets/themes/climbing/climbing.json | 4 +- assets/themes/grb/grb.json | 14 +- .../mapcomplete-changes.json | 2 +- .../mapcomplete-changes.proto.json | 2 +- assets/themes/onwheels/onwheels.json | 8 +- assets/themes/postal_codes/postal_codes.json | 6 +- assets/themes/speelplekken/speelplekken.json | 12 +- .../street_lighting_assen.json | 10 +- assets/themes/uk_addresses/uk_addresses.json | 10 +- assets/themes/waste_assen/waste_assen.json | 14 +- index.css | 4 + public/css/index-tailwind-output.css | 28 ++- test.ts | 2 +- test/Logic/ExtraFunctions.spec.ts | 16 +- 34 files changed, 378 insertions(+), 265 deletions(-) create mode 100644 UI/BigComponents/BackgroundSwitcher.svelte diff --git a/Logic/ExtraFunctions.ts b/Logic/ExtraFunctions.ts index 80311e375..e2c129aea 100644 --- a/Logic/ExtraFunctions.ts +++ b/Logic/ExtraFunctions.ts @@ -1,10 +1,11 @@ -import { GeoOperations } from "./GeoOperations" +import {GeoOperations} from "./GeoOperations" import Combine from "../UI/Base/Combine" import BaseUIElement from "../UI/BaseUIElement" import List from "../UI/Base/List" import Title from "../UI/Base/Title" -import { BBox } from "./BBox" -import { Feature, Geometry, MultiPolygon, Polygon } from "geojson" +import {BBox} from "./BBox" +import {Feature, Geometry, MultiPolygon, Polygon} from "geojson" +import {GeoJSONFeature} from "maplibre-gl"; export interface ExtraFuncParams { /** @@ -12,7 +13,7 @@ export interface ExtraFuncParams { * Note that more features then requested can be given back. * Format: [ [ geojson, geojson, geojson, ... ], [geojson, ...], ...] */ - getFeaturesWithin: (layerId: string, bbox: BBox) => Feature>[] + getFeaturesWithin: (layerId: string, bbox: BBox) => Feature>[][] getFeatureById: (id: string) => Feature> } @@ -52,24 +53,27 @@ class EnclosingFunc implements ExtraFunction { if (otherFeaturess.length === 0) { continue } - for (const otherFeature of otherFeaturess) { - if (seenIds.has(otherFeature.properties.id)) { - continue - } - seenIds.add(otherFeature.properties.id) - if ( - otherFeature.geometry.type !== "Polygon" && - otherFeature.geometry.type !== "MultiPolygon" - ) { - continue - } - if ( - GeoOperations.completelyWithin( - feat, - >otherFeature - ) - ) { - result.push({ feat: otherFeature }) + for (const otherFeatures of otherFeaturess) { + for (const otherFeature of otherFeatures) { + + if (seenIds.has(otherFeature.properties.id)) { + continue + } + seenIds.add(otherFeature.properties.id) + if ( + otherFeature.geometry.type !== "Polygon" && + otherFeature.geometry.type !== "MultiPolygon" + ) { + continue + } + if ( + GeoOperations.completelyWithin( + feat, + >otherFeature + ) + ) { + result.push({feat: otherFeature}) + } } } } @@ -157,7 +161,7 @@ class IntersectionFunc implements ExtraFunction { if (intersections.length === 0) { continue } - result.push({ feat: otherFeature, intersections }) + result.push({feat: otherFeature, intersections}) } } @@ -241,20 +245,22 @@ class ClosestNObjectFunc implements ExtraFunction { static GetClosestNFeatures( params: ExtraFuncParams, feature: any, - features: string | any[], + features: string | Feature[], options?: { maxFeatures?: number; uniqueTag?: string | undefined; maxDistance?: number } ): { feat: any; distance: number }[] { const maxFeatures = options?.maxFeatures ?? 1 const maxDistance = options?.maxDistance ?? 500 const uniqueTag: string | undefined = options?.uniqueTag + console.log("Calculating 'closestn' features") + let allFeatures: Feature[][] if (typeof features === "string") { const name = features const bbox = GeoOperations.bbox( GeoOperations.buffer(GeoOperations.bbox(feature), maxDistance) ) - features = params.getFeaturesWithin(name, new BBox(bbox.geometry.coordinates)) + allFeatures = params.getFeaturesWithin(name, new BBox(bbox.geometry.coordinates)) } else { - features = [features] + allFeatures = [features] } if (features === undefined) { return @@ -263,9 +269,9 @@ class ClosestNObjectFunc implements ExtraFunction { const selfCenter = GeoOperations.centerpointCoordinates(feature) let closestFeatures: { feat: any; distance: number }[] = [] - for (const featureList of features) { - // Features is provided by 'getFeaturesWithin' which returns a list of lists of features, hence the double loop here - for (const otherFeature of featureList) { + for (const feats of allFeatures) { + + for (const otherFeature of feats) { if ( otherFeature === feature || otherFeature.properties.id === feature.properties.id @@ -333,7 +339,7 @@ class ClosestNObjectFunc implements ExtraFunction { // We want to see the tag `uniquetag=some_value` only once in the entire list (e.g. to prevent road segements of identical names to fill up the list of 'names of nearby roads') // AT this point, we have found a closer segment with the same, identical tag // so we replace directly - closestFeatures[i] = { feat: otherFeature, distance: distance } + closestFeatures[i] = {feat: otherFeature, distance: distance} } break } @@ -423,6 +429,8 @@ class GetParsed implements ExtraFunction { } } +export type ExtraFuncType = typeof ExtraFunctions.types[number] + export class ExtraFunctions { static readonly intro = new Combine([ new Title("Calculating tags with Javascript", 2), @@ -440,7 +448,7 @@ export class ExtraFunctions { '"calculatedTags": [', ' "_someKey=javascript-expression",', ' "name=feat.properties.name ?? feat.properties.ref ?? feat.properties.operator",', - " \"_distanceCloserThen3Km=feat.distanceTo( some_lon, some_lat) < 3 ? 'yes' : 'no'\" ", + " \"_distanceCloserThen3Km=distanceTo(feat)( some_lon, some_lat) < 3 ? 'yes' : 'no'\" ", " ]", "````", "", @@ -455,7 +463,8 @@ export class ExtraFunctions { .SetClass("flex-col") .AsMarkdown() - private static readonly allFuncs: ExtraFunction[] = [ + static readonly types = ["distanceTo", "overlapWith", "enclosingFeatures", "intersectionsWith", "closest", "closestn", "get"] as const + private static readonly allFuncs = [ new DistanceToFunc(), new OverlapFunc(), new EnclosingFunc(), @@ -465,14 +474,16 @@ export class ExtraFunctions { new GetParsed(), ] - public static FullPatchFeature(params: ExtraFuncParams, feature) { - if (feature._is_patched) { - return - } - feature._is_patched = true - for (const func of ExtraFunctions.allFuncs) { - feature[func._name] = func._f(params, feature) + + public static constructHelpers(params: ExtraFuncParams): Record Function> { + const record: Record Function> = {} + for (const f of ExtraFunctions.allFuncs) { + if (this.types.indexOf(f._name) < 0) { + throw "Invalid extraFunc-type: " + f._name + } + record[f._name] = (feat) => f._f(params, feat) } + return record } public static HelpText(): BaseUIElement { diff --git a/Logic/FeatureSource/Actors/GeoIndexedStore.ts b/Logic/FeatureSource/Actors/GeoIndexedStore.ts index 314706536..15bc28fdf 100644 --- a/Logic/FeatureSource/Actors/GeoIndexedStore.ts +++ b/Logic/FeatureSource/Actors/GeoIndexedStore.ts @@ -23,8 +23,7 @@ export default class GeoIndexedStore implements FeatureSource { * @param bbox * @constructor */ - public GetFeaturesWithin(bbox: BBox, strict: boolean = false): Feature[] { - // TODO optimize + public GetFeaturesWithin(bbox: BBox): Feature[] { const bboxFeature = bbox.asGeojsonCached() return this.features.data.filter((f) => { if (f.geometry.type === "Point") { @@ -40,7 +39,6 @@ export default class GeoIndexedStore implements FeatureSource { if (f.geometry.type === "Polygon" || f.geometry.type === "MultiPolygon") { return GeoOperations.intersect(f, bboxFeature) !== undefined } - console.log("Calculating intersection between", bboxFeature, "and", f) return GeoOperations.intersect(f, bboxFeature) !== undefined }) } diff --git a/Logic/MetaTagging.ts b/Logic/MetaTagging.ts index 99d7068d5..d3df3da43 100644 --- a/Logic/MetaTagging.ts +++ b/Logic/MetaTagging.ts @@ -1,12 +1,14 @@ -import SimpleMetaTaggers, { MetataggingState, SimpleMetaTagger } from "./SimpleMetaTagger" -import { ExtraFuncParams, ExtraFunctions } from "./ExtraFunctions" +import SimpleMetaTaggers, {MetataggingState, SimpleMetaTagger} from "./SimpleMetaTagger" +import {ExtraFuncParams, ExtraFunctions, ExtraFuncType} from "./ExtraFunctions" import LayerConfig from "../Models/ThemeConfig/LayerConfig" -import { Feature } from "geojson" +import {Feature} from "geojson" import FeaturePropertiesStore from "./FeatureSource/Actors/FeaturePropertiesStore" import LayoutConfig from "../Models/ThemeConfig/LayoutConfig" -import { GeoIndexedStoreForLayer } from "./FeatureSource/Actors/GeoIndexedStore" -import { IndexedFeatureSource } from "./FeatureSource/FeatureSource" +import {GeoIndexedStoreForLayer} from "./FeatureSource/Actors/GeoIndexedStore" +import {IndexedFeatureSource} from "./FeatureSource/FeatureSource" import OsmObjectDownloader from "./Osm/OsmObjectDownloader" +import {Utils} from "../Utils"; +import {GeoJSONFeature} from "maplibre-gl"; /** * Metatagging adds various tags to the elements, e.g. lat, lon, surface area, ... @@ -27,8 +29,16 @@ export default class MetaTagging { }) { const params: ExtraFuncParams = { getFeatureById: (id) => state.indexedFeatures.featuresById.data.get(id), - getFeaturesWithin: (layerId, bbox) => - state.perLayer.get(layerId).GetFeaturesWithin(bbox), + getFeaturesWithin: (layerId, bbox) => { + if(layerId === '*' || layerId === null || layerId === undefined){ + const feats: Feature[][] = [] + state.perLayer.forEach((layer) => { + feats.push(layer.GetFeaturesWithin(bbox)) + }) + return feats + } + return [state.perLayer.get(layerId).GetFeaturesWithin(bbox)]; + }, } for (const layer of state.layout.layers) { if (layer.source === null) { @@ -60,7 +70,7 @@ export default class MetaTagging { } /** - * This method (re)calculates all metatags and calculated tags on every given object. + * This method (re)calculates all metatags and calculated tags on every given feature. * The given features should be part of the given layer * * Returns true if at least one feature has changed properties @@ -96,16 +106,26 @@ export default class MetaTagging { } // The calculated functions - per layer - which add the new keys - const layerFuncs = this.createRetaggingFunc(layer) + // Calculated functions are defined by the layer + const layerFuncs = this.createRetaggingFunc(layer, ExtraFunctions.constructHelpers(params)) const state: MetataggingState = { layout, osmObjectDownloader } let atLeastOneFeatureChanged = false - + let strictlyEvaluated = 0 for (let i = 0; i < features.length; i++) { const feature = features[i] const tags = featurePropertiesStores?.getStore(feature.properties.id) let somethingChanged = false let definedTags = new Set(Object.getOwnPropertyNames(feature.properties)) + if (layerFuncs !== undefined) { + let retaggingChanged = false + try { + retaggingChanged = layerFuncs(feature) + } catch (e) { + console.error(e) + } + somethingChanged = somethingChanged || retaggingChanged + } for (const metatag of metatagsToApply) { try { if (!metatag.keys.some((key) => !(key in feature.properties))) { @@ -123,7 +143,10 @@ export default class MetaTagging { metatag.applyMetaTagsOnFeature(feature, layer, tags, state) if (options?.evaluateStrict) { for (const key of metatag.keys) { - feature.properties[key] + const evaluated = feature.properties[key] + if(evaluated !== undefined){ + strictlyEvaluated++ + } } } } else { @@ -153,15 +176,7 @@ export default class MetaTagging { } } - if (layerFuncs !== undefined) { - let retaggingChanged = false - try { - retaggingChanged = layerFuncs(params, feature) - } catch (e) { - console.error(e) - } - somethingChanged = somethingChanged || retaggingChanged - } + if (somethingChanged) { try { @@ -175,91 +190,83 @@ export default class MetaTagging { return atLeastOneFeatureChanged } - private static createFunctionsForFeature( - layerId: string, - calculatedTags: [string, string, boolean][] - ): ((feature: any) => void)[] { - const functions: ((feature: any) => any)[] = [] - for (const entry of calculatedTags) { - const key = entry[0] - const code = entry[1] - const isStrict = entry[2] - if (code === undefined) { - continue - } + /** + * Creates a function that implements that calculates a property and adds this property onto the feature properties + * @param specification + * @param helperFunctions + * @param layerId + * @private + */ + private static createFunctionForFeature( [key, code, isStrict]: [string, string, boolean], + helperFunctions: Record Function>, + layerId: string = "unkown layer" + ): ((feature: GeoJSONFeature) => void) | undefined { + if (code === undefined) { + return undefined + } - const calculateAndAssign: (feat: any) => any = (feat) => { - try { - let result = new Function("feat", "return " + code + ";")(feat) - if (result === "") { - result === undefined - } - if (result !== undefined && typeof result !== "string") { - // Make sure it is a string! - result = JSON.stringify(result) - } - delete feat.properties[key] - feat.properties[key] = result - return result - } catch (e) { - if (MetaTagging.errorPrintCount < MetaTagging.stopErrorOutputAt) { - console.warn( - "Could not calculate a " + - (isStrict ? "strict " : "") + - " calculated tag for key " + - key + - " defined by " + - code + - " (in layer" + - layerId + - ") due to \n" + - e + - "\n. Are you the theme creator? Doublecheck your code. Note that the metatags might not be stable on new features", - e, - e.stack - ) - MetaTagging.errorPrintCount++ - if (MetaTagging.errorPrintCount == MetaTagging.stopErrorOutputAt) { - console.error( - "Got ", - MetaTagging.stopErrorOutputAt, - " errors calculating this metatagging - stopping output now" - ) - } - } - return undefined + + + const calculateAndAssign: ((feat: GeoJSONFeature) => (string | undefined)) = (feat) => { + try { + let result = new Function("feat", "{"+ExtraFunctions.types.join(", ")+"}", "return " + code + ";")(feat, helperFunctions) + if (result === "") { + result = undefined + } + if (result !== undefined && typeof result !== "string") { + // Make sure it is a string! + result = JSON.stringify(result) + } + delete feat.properties[key] + feat.properties[key] = result + return result + } catch (e) { + if (MetaTagging.errorPrintCount < MetaTagging.stopErrorOutputAt) { + console.warn( + "Could not calculate a " + + (isStrict ? "strict " : "") + + " calculated tag for key " + + key + + " defined by " + + code + + " (in layer" + + layerId + + ") due to \n" + + e + + "\n. Are you the theme creator? Doublecheck your code. Note that the metatags might not be stable on new features", + e, + e.stack + ) + MetaTagging.errorPrintCount++ + if (MetaTagging.errorPrintCount == MetaTagging.stopErrorOutputAt) { + console.error( + "Got ", + MetaTagging.stopErrorOutputAt, + " errors calculating this metatagging - stopping output now" + ) + } } - } - - if (isStrict) { - functions.push(calculateAndAssign) - continue - } - - // Lazy function - const f = (feature: any) => { - delete feature.properties[key] - Object.defineProperty(feature.properties, key, { - configurable: true, - enumerable: false, // By setting this as not enumerable, the localTileSaver will _not_ calculate this - get: function () { - return calculateAndAssign(feature) - }, - }) return undefined } - - functions.push(f) } - return functions + + if(isStrict){ + return calculateAndAssign + } + return (feature: any) => { + delete feature.properties[key] + Utils.AddLazyProperty(feature.properties, key, () => calculateAndAssign(feature)) + return undefined + } } /** * Creates the function which adds all the calculated tags to a feature. Called once per layer */ private static createRetaggingFunc( - layer: LayerConfig - ): (params: ExtraFuncParams, feature: any) => boolean { + layer: LayerConfig, + helpers: Record Function> + ): (feature: any) => boolean { const calculatedTags: [string, string, boolean][] = layer.calculatedTags if (calculatedTags === undefined || calculatedTags.length === 0) { return undefined @@ -267,18 +274,17 @@ export default class MetaTagging { let functions: ((feature: Feature) => void)[] = MetaTagging.retaggingFuncCache.get(layer.id) if (functions === undefined) { - functions = MetaTagging.createFunctionsForFeature(layer.id, calculatedTags) + functions = calculatedTags.map(spec => this.createFunctionForFeature(spec, helpers, layer.id)) MetaTagging.retaggingFuncCache.set(layer.id, functions) } - return (params: ExtraFuncParams, feature) => { + return (feature: Feature) => { const tags = feature.properties if (tags === undefined) { return } try { - ExtraFunctions.FullPatchFeature(params, feature) for (const f of functions) { f(feature) } diff --git a/Models/ThemeConfig/DependencyCalculator.ts b/Models/ThemeConfig/DependencyCalculator.ts index 9262326f6..3267abef3 100644 --- a/Models/ThemeConfig/DependencyCalculator.ts +++ b/Models/ThemeConfig/DependencyCalculator.ts @@ -96,16 +96,15 @@ export default class DependencyCalculator { return [] }, } - // Init the extra patched functions... - ExtraFunctions.FullPatchFeature(params, obj) + const helpers = ExtraFunctions.constructHelpers(params) // ... Run the calculated tag code, which will trigger the getFeaturesWithin above... for (let i = 0; i < layer.calculatedTags.length; i++) { const [key, code] = layer.calculatedTags[i] currentLine = i // Leak the state... currentKey = key try { - const func = new Function("feat", "return " + code + ";") - const result = func(obj) + const func = new Function("feat", "{"+ExtraFunctions.types.join(",")+"}", "return " + code + ";") + const result = func(obj, helpers) obj.properties[key] = JSON.stringify(result) } catch (e) {} } diff --git a/UI/BigComponents/BackgroundSwitcher.svelte b/UI/BigComponents/BackgroundSwitcher.svelte new file mode 100644 index 000000000..7c948cb42 --- /dev/null +++ b/UI/BigComponents/BackgroundSwitcher.svelte @@ -0,0 +1,90 @@ + +
+
+ +
+
+ +
+
+ Current background: + +
+
diff --git a/UI/InputElement/ValidatedInput.svelte b/UI/InputElement/ValidatedInput.svelte index 3a621802c..ca73d34cc 100644 --- a/UI/InputElement/ValidatedInput.svelte +++ b/UI/InputElement/ValidatedInput.svelte @@ -56,7 +56,7 @@ {:else } - + {#if !$isValid} {/if} diff --git a/UI/Map/MaplibreMap.svelte b/UI/Map/MaplibreMap.svelte index a3dd0c198..26ce59214 100644 --- a/UI/Map/MaplibreMap.svelte +++ b/UI/Map/MaplibreMap.svelte @@ -16,7 +16,7 @@ */ export let map: Writable - export let attribution = true + export let attribution = false let center = {}; onMount(() => { diff --git a/UI/Popup/TagRendering/TagRenderingAnswer.svelte b/UI/Popup/TagRendering/TagRenderingAnswer.svelte index 24012815f..94a6660c0 100644 --- a/UI/Popup/TagRendering/TagRenderingAnswer.svelte +++ b/UI/Popup/TagRendering/TagRenderingAnswer.svelte @@ -31,7 +31,7 @@ {#if config !== undefined && (config?.condition === undefined || config.condition.matchesProperties(_tags))} -
+