Fix: upload flow deals better with point reuse: it actually opens the feature now

This commit is contained in:
Pieter Vander Vennet 2023-09-21 14:53:52 +02:00
parent 246b16317d
commit c14cbc9fe9
7 changed files with 282 additions and 237 deletions

View file

@ -1,5 +1,6 @@
import { FeatureSource } from "../FeatureSource"
import { UIEventSource } from "../../UIEventSource"
import { OsmTags } from "../../../Models/OsmFeature"
/**
* Constructs a UIEventStore for the properties of every Feature, indexed by id
@ -13,40 +14,6 @@ export default class FeaturePropertiesStore {
}
}
public getStore(id: string): UIEventSource<Record<string, string>> {
return this._elements.get(id)
}
public trackFeatureSource(source: FeatureSource) {
const self = this
source.features.addCallbackAndRunD((features) => {
for (const feature of features) {
const id = feature.properties.id
if (id === undefined) {
console.trace("Error: feature without ID:", feature)
throw "Error: feature without ID"
}
const source = self._elements.get(id)
if (source === undefined) {
self._elements.set(id, new UIEventSource<any>(feature.properties))
continue
}
if (source.data === feature.properties) {
continue
}
// Update the tags in the old store and link them
const changeMade = FeaturePropertiesStore.mergeTags(source.data, feature.properties)
feature.properties = source.data
if (changeMade) {
source.ping()
}
}
})
}
/**
* Overwrites the tags of the old properties object, returns true if a change was made.
* Metatags are overriden if they are in the new properties, but not removed
@ -67,7 +34,7 @@ export default class FeaturePropertiesStore {
}
if (newProperties[oldPropertiesKey] === undefined) {
changeMade = true
delete oldProperties[oldPropertiesKey]
// delete oldProperties[oldPropertiesKey]
}
}
@ -83,6 +50,48 @@ export default class FeaturePropertiesStore {
return changeMade
}
public getStore(id: string): UIEventSource<Record<string, string>> {
const store = this._elements.get(id)
if (store === undefined) {
console.error("PANIC: no store for", id)
}
return store
}
public trackFeature(feature: { properties: OsmTags }) {
const id = feature.properties.id
if (id === undefined) {
console.trace("Error: feature without ID:", feature)
throw "Error: feature without ID"
}
const source = this._elements.get(id)
if (source === undefined) {
this._elements.set(id, new UIEventSource<any>(feature.properties))
return
}
if (source.data === feature.properties) {
return
}
// Update the tags in the old store and link them
const changeMade = FeaturePropertiesStore.mergeTags(source.data, feature.properties)
feature.properties = <any>source.data
if (changeMade) {
source.ping()
}
}
public trackFeatureSource(source: FeatureSource) {
const self = this
source.features.addCallbackAndRunD((features) => {
for (const feature of features) {
self.trackFeature(<any>feature)
}
})
}
// noinspection JSUnusedGlobalSymbols
public addAlias(oldId: string, newId: string): void {
if (newId === undefined) {

View file

@ -4,8 +4,9 @@ import { IndexedFeatureSource, WritableFeatureSource } from "../FeatureSource"
import { UIEventSource } from "../../UIEventSource"
import { ChangeDescription } from "../../Osm/Actions/ChangeDescription"
import { OsmId, OsmTags } from "../../../Models/OsmFeature"
import { Feature } from "geojson"
import OsmObjectDownloader from "../../Osm/OsmObjectDownloader"
import { Feature, Point } from "geojson"
import { TagUtils } from "../../Tags/TagUtils"
import FeaturePropertiesStore from "../Actors/FeaturePropertiesStore"
export class NewGeometryFromChangesFeatureSource implements WritableFeatureSource {
// This class name truly puts the 'Java' into 'Javascript'
@ -15,115 +16,145 @@ export class NewGeometryFromChangesFeatureSource implements WritableFeatureSourc
*
* These elements are probably created by the 'SimpleAddUi' which generates a new point, but the import functionality might create a line or polygon too.
* Other sources of new points are e.g. imports from nodes
*
* Alternatively, an already existing point might suddenly match the layer, especially if a point in a wall is reused
*
* Note that the FeaturePropertiesStore will track a featuresource, such as this one
*/
public readonly features: UIEventSource<Feature[]> = new UIEventSource<Feature[]>([])
private readonly _seenChanges: Set<ChangeDescription>
private readonly _features: Feature[]
private readonly _backend: string
private readonly _allElementStorage: IndexedFeatureSource
private _featureProperties: FeaturePropertiesStore
constructor(changes: Changes, allElementStorage: IndexedFeatureSource, backendUrl: string) {
const seenChanges = new Set<ChangeDescription>()
const features = this.features.data
constructor(
changes: Changes,
allElementStorage: IndexedFeatureSource,
featureProperties: FeaturePropertiesStore
) {
this._allElementStorage = allElementStorage
this._featureProperties = featureProperties
this._seenChanges = new Set<ChangeDescription>()
this._features = this.features.data
this._backend = changes.backend
const self = this
const backend = changes.backend
changes.pendingChanges.addCallbackAndRunD((changes) => {
if (changes.length === 0) {
return
changes.pendingChanges.addCallbackAndRunD((changes) => self.handleChanges(changes))
}
private addNewFeature(feature: Feature) {
const features = this._features
feature.id = feature.properties.id
features.push(feature)
}
/**
* Handles a single pending change
* @returns true if something changed
* @param change
* @private
*/
private handleChange(change: ChangeDescription): boolean {
const backend = this._backend
const allElementStorage = this._allElementStorage
console.log("Handling pending change")
if (change.id > 0) {
// This is an already existing object
// In _most_ of the cases, this means that this _isn't_ a new object
// However, when a point is snapped to an already existing point, we have to create a representation for this point!
// For this, we introspect the change
if (allElementStorage.featuresById.data.has(change.type + "/" + change.id)) {
// The current point already exists, we don't have to do anything here
return false
}
console.debug("Detected a reused point, for", change)
// The 'allElementsStore' does _not_ have this point yet, so we have to create it
// However, we already create a store for it
const { lon, lat } = <{ lon: number; lat: number }>change.changes
const feature = <Feature<Point, OsmTags>>{
type: "Feature",
properties: {
id: <OsmId>change.type + "/" + change.id,
...TagUtils.changeAsProperties(change.tags),
},
geometry: {
type: "Point",
coordinates: [lon, lat],
},
}
this._featureProperties.trackFeature(feature)
this.addNewFeature(feature)
return true
} else if (change.changes === undefined) {
// The geometry is not described - not a new point or geometry change, but probably a tagchange to a newly created point
// Not something that should be handled here
return false
}
try {
const tags: OsmTags & { id: OsmId & string } = {
id: <OsmId & string>(change.type + "/" + change.id),
}
for (const kv of change.tags) {
tags[kv.k] = kv.v
}
let somethingChanged = false
tags["_backend"] = this._backend
function add(feature) {
feature.id = feature.properties.id
features.push(feature)
somethingChanged = true
switch (change.type) {
case "node":
const n = new OsmNode(change.id)
n.tags = tags
n.lat = change.changes["lat"]
n.lon = change.changes["lon"]
const geojson = n.asGeoJson()
this.addNewFeature(geojson)
break
case "way":
const w = new OsmWay(change.id)
w.tags = tags
w.nodes = change.changes["nodes"]
w.coordinates = change.changes["coordinates"].map(([lon, lat]) => [lat, lon])
this.addNewFeature(w.asGeoJson())
break
case "relation":
const r = new OsmRelation(change.id)
r.tags = tags
r.members = change.changes["members"]
this.addNewFeature(r.asGeoJson())
break
}
return true
} catch (e) {
console.error("Could not generate a new geometry to render on screen for:", e)
}
}
private handleChanges(changes: ChangeDescription[]) {
const seenChanges = this._seenChanges
if (changes.length === 0) {
return
}
let somethingChanged = false
for (const change of changes) {
if (seenChanges.has(change)) {
// Already handled
continue
}
seenChanges.add(change)
if (change.tags === undefined) {
// If tags is undefined, this is probably a new point that is part of a split road
continue
}
for (const change of changes) {
if (seenChanges.has(change)) {
// Already handled
continue
}
seenChanges.add(change)
if (change.tags === undefined) {
// If tags is undefined, this is probably a new point that is part of a split road
continue
}
console.log("Handling pending change")
if (change.id > 0) {
// This is an already existing object
// In _most_ of the cases, this means that this _isn't_ a new object
// However, when a point is snapped to an already existing point, we have to create a representation for this point!
// For this, we introspect the change
if (allElementStorage.featuresById.data.has(change.type + "/" + change.id)) {
// The current point already exists, we don't have to do anything here
continue
}
console.debug("Detected a reused point")
// The 'allElementsStore' does _not_ have this point yet, so we have to create it
new OsmObjectDownloader(backend)
.DownloadObjectAsync(change.type + "/" + change.id)
.then((feat) => {
console.log("Got the reused point:", feat)
if (feat === "deleted") {
throw "Panic: snapping to a point, but this point has been deleted in the meantime"
}
for (const kv of change.tags) {
feat.tags[kv.k] = kv.v
}
const geojson = feat.asGeoJson()
self.features.data.push(geojson)
self.features.ping()
})
continue
} else if (change.changes === undefined) {
// The geometry is not described - not a new point or geometry change, but probably a tagchange to a newly created point
// Not something that should be handled here
continue
}
try {
const tags: OsmTags & { id: OsmId & string } = {
id: <OsmId & string>(change.type + "/" + change.id),
}
for (const kv of change.tags) {
tags[kv.k] = kv.v
}
tags["_backend"] = backendUrl
switch (change.type) {
case "node":
const n = new OsmNode(change.id)
n.tags = tags
n.lat = change.changes["lat"]
n.lon = change.changes["lon"]
const geojson = n.asGeoJson()
add(geojson)
break
case "way":
const w = new OsmWay(change.id)
w.tags = tags
w.nodes = change.changes["nodes"]
w.coordinates = change.changes["coordinates"].map(([lon, lat]) => [
lat,
lon,
])
add(w.asGeoJson())
break
case "relation":
const r = new OsmRelation(change.id)
r.tags = tags
r.members = change.changes["members"]
add(r.asGeoJson())
break
}
} catch (e) {
console.error("Could not generate a new geometry to render on screen for:", e)
}
}
if (somethingChanged) {
self.features.ping()
}
})
somethingChanged ||= this.handleChange(change)
}
if (somethingChanged) {
this.features.ping()
}
}
}

View file

@ -97,7 +97,7 @@ export default class CreateNewNodeAction extends OsmCreateAction {
},
meta: this.meta,
}
if (this._snapOnto === undefined) {
if (this._snapOnto?.coordinates === undefined) {
return [newPointChange]
}
@ -113,6 +113,7 @@ export default class CreateNewNodeAction extends OsmCreateAction {
console.log("Attempting to snap:", { geojson, projected, projectedCoor, index })
// We check that it isn't close to an already existing point
let reusedPointId = undefined
let reusedPointCoordinates: [number, number] = undefined
let outerring: [number, number][]
if (geojson.geometry.type === "LineString") {
@ -125,11 +126,13 @@ export default class CreateNewNodeAction extends OsmCreateAction {
if (GeoOperations.distanceBetween(prev, projectedCoor) < this._reusePointDistance) {
// We reuse this point instead!
reusedPointId = this._snapOnto.nodes[index]
reusedPointCoordinates = this._snapOnto.coordinates[index]
}
const next = outerring[index + 1]
if (GeoOperations.distanceBetween(next, projectedCoor) < this._reusePointDistance) {
// We reuse this point instead!
reusedPointId = this._snapOnto.nodes[index + 1]
reusedPointCoordinates = this._snapOnto.coordinates[index + 1]
}
if (reusedPointId !== undefined) {
this.setElementId(reusedPointId)
@ -139,12 +142,13 @@ export default class CreateNewNodeAction extends OsmCreateAction {
type: "node",
id: reusedPointId,
meta: this.meta,
changes: { lat: reusedPointCoordinates[0], lon: reusedPointCoordinates[1] },
},
]
}
const locations = [
...this._snapOnto.coordinates.map(([lat, lon]) => <[number, number]>[lon, lat]),
...this._snapOnto.coordinates?.map(([lat, lon]) => <[number, number]>[lon, lat]),
]
const ids = [...this._snapOnto.nodes]

View file

@ -244,7 +244,7 @@ export default class ThemeViewState implements SpecialVisualizationState {
this.newFeatures = new NewGeometryFromChangesFeatureSource(
this.changes,
indexedElements,
this.osmConnection.Backend()
this.featureProperties
)
layoutSource.addSource(this.newFeatures)

View file

@ -376,12 +376,6 @@ export class MapLibreAdaptor implements MapProperties, ExportableMap {
}
const background: RasterLayerProperties = this.rasterLayer?.data?.properties
if (!background) {
console.error(
"Attempting to 'setBackground', but the background is",
background,
"for",
map.getCanvas()
)
return
}
if (this._currentRasterLayer === background.id) {
@ -457,7 +451,6 @@ export class MapLibreAdaptor implements MapProperties, ExportableMap {
if (!map) {
return
}
console.log("Rotation allowed:", allow)
if (allow === false) {
map.rotateTo(0, { duration: 0 })
map.setPitch(0)

View file

@ -3,109 +3,109 @@
* This component ties together all the steps that are needed to create a new point.
* There are many subcomponents which help with that
*/
import type { SpecialVisualizationState } from "../../SpecialVisualization"
import PresetList from "./PresetList.svelte"
import type PresetConfig from "../../../Models/ThemeConfig/PresetConfig"
import LayerConfig from "../../../Models/ThemeConfig/LayerConfig"
import Tr from "../../Base/Tr.svelte"
import SubtleButton from "../../Base/SubtleButton.svelte"
import FromHtml from "../../Base/FromHtml.svelte"
import Translations from "../../i18n/Translations.js"
import TagHint from "../TagHint.svelte"
import { And } from "../../../Logic/Tags/And.js"
import LoginToggle from "../../Base/LoginToggle.svelte"
import Constants from "../../../Models/Constants.js"
import FilteredLayer from "../../../Models/FilteredLayer"
import { Store, UIEventSource } from "../../../Logic/UIEventSource"
import { EyeIcon, EyeOffIcon } from "@rgossiaux/svelte-heroicons/solid"
import LoginButton from "../../Base/LoginButton.svelte"
import NewPointLocationInput from "../../BigComponents/NewPointLocationInput.svelte"
import CreateNewNodeAction from "../../../Logic/Osm/Actions/CreateNewNodeAction"
import { OsmWay } from "../../../Logic/Osm/OsmObject"
import { Tag } from "../../../Logic/Tags/Tag"
import type { WayId } from "../../../Models/OsmFeature"
import Loading from "../../Base/Loading.svelte"
import type { GlobalFilter } from "../../../Models/GlobalFilter"
import { onDestroy } from "svelte"
import NextButton from "../../Base/NextButton.svelte"
import BackButton from "../../Base/BackButton.svelte"
import ToSvelte from "../../Base/ToSvelte.svelte"
import Svg from "../../../Svg"
import OpenBackgroundSelectorButton from "../../BigComponents/OpenBackgroundSelectorButton.svelte"
import { twJoin } from "tailwind-merge"
import type { SpecialVisualizationState } from "../../SpecialVisualization";
import PresetList from "./PresetList.svelte";
import type PresetConfig from "../../../Models/ThemeConfig/PresetConfig";
import LayerConfig from "../../../Models/ThemeConfig/LayerConfig";
import Tr from "../../Base/Tr.svelte";
import SubtleButton from "../../Base/SubtleButton.svelte";
import FromHtml from "../../Base/FromHtml.svelte";
import Translations from "../../i18n/Translations.js";
import TagHint from "../TagHint.svelte";
import { And } from "../../../Logic/Tags/And.js";
import LoginToggle from "../../Base/LoginToggle.svelte";
import Constants from "../../../Models/Constants.js";
import FilteredLayer from "../../../Models/FilteredLayer";
import { Store, UIEventSource } from "../../../Logic/UIEventSource";
import { EyeIcon, EyeOffIcon } from "@rgossiaux/svelte-heroicons/solid";
import LoginButton from "../../Base/LoginButton.svelte";
import NewPointLocationInput from "../../BigComponents/NewPointLocationInput.svelte";
import CreateNewNodeAction from "../../../Logic/Osm/Actions/CreateNewNodeAction";
import { OsmWay } from "../../../Logic/Osm/OsmObject";
import { Tag } from "../../../Logic/Tags/Tag";
import type { WayId } from "../../../Models/OsmFeature";
import Loading from "../../Base/Loading.svelte";
import type { GlobalFilter } from "../../../Models/GlobalFilter";
import { onDestroy } from "svelte";
import NextButton from "../../Base/NextButton.svelte";
import BackButton from "../../Base/BackButton.svelte";
import ToSvelte from "../../Base/ToSvelte.svelte";
import Svg from "../../../Svg";
import OpenBackgroundSelectorButton from "../../BigComponents/OpenBackgroundSelectorButton.svelte";
import { twJoin } from "tailwind-merge";
export let coordinate: { lon: number; lat: number }
export let state: SpecialVisualizationState
export let coordinate: { lon: number; lat: number };
export let state: SpecialVisualizationState;
let selectedPreset: {
preset: PresetConfig
layer: LayerConfig
icon: string
tags: Record<string, string>
} = undefined
let checkedOfGlobalFilters: number = 0
let confirmedCategory = false
} = undefined;
let checkedOfGlobalFilters: number = 0;
let confirmedCategory = false;
$: if (selectedPreset === undefined) {
confirmedCategory = false
creating = false
checkedOfGlobalFilters = 0
confirmedCategory = false;
creating = false;
checkedOfGlobalFilters = 0;
}
let flayer: FilteredLayer = undefined
let layerIsDisplayed: UIEventSource<boolean> | undefined = undefined
let layerHasFilters: Store<boolean> | undefined = undefined
let globalFilter: UIEventSource<GlobalFilter[]> = state.layerState.globalFilters
let _globalFilter: GlobalFilter[] = []
let flayer: FilteredLayer = undefined;
let layerIsDisplayed: UIEventSource<boolean> | undefined = undefined;
let layerHasFilters: Store<boolean> | undefined = undefined;
let globalFilter: UIEventSource<GlobalFilter[]> = state.layerState.globalFilters;
let _globalFilter: GlobalFilter[] = [];
onDestroy(
globalFilter.addCallbackAndRun((globalFilter) => {
console.log("Global filters are", globalFilter)
_globalFilter = globalFilter ?? []
console.log("Global filters are", globalFilter);
_globalFilter = globalFilter ?? [];
})
)
);
$: {
flayer = state.layerState.filteredLayers.get(selectedPreset?.layer?.id)
layerIsDisplayed = flayer?.isDisplayed
layerHasFilters = flayer?.hasFilter
flayer = state.layerState.filteredLayers.get(selectedPreset?.layer?.id);
layerIsDisplayed = flayer?.isDisplayed;
layerHasFilters = flayer?.hasFilter;
}
const t = Translations.t.general.add
const t = Translations.t.general.add;
const zoom = state.mapProperties.zoom
const zoom = state.mapProperties.zoom;
const isLoading = state.dataIsLoading
let preciseCoordinate: UIEventSource<{ lon: number; lat: number }> = new UIEventSource(undefined)
let snappedToObject: UIEventSource<string> = new UIEventSource<string>(undefined)
const isLoading = state.dataIsLoading;
let preciseCoordinate: UIEventSource<{ lon: number; lat: number }> = new UIEventSource(undefined);
let snappedToObject: UIEventSource<string> = new UIEventSource<string>(undefined);
// Small helper variable: if the map is tapped, we should let the 'Next'-button grab some attention as users have to click _that_ to continue, not the map
let preciseInputIsTapped = false
let preciseInputIsTapped = false;
let creating = false
let creating = false;
/**
* Call when the user should restart the flow by clicking on the map, e.g. because they disabled filters.
* Will delete the lastclick-location
*/
function abort() {
state.selectedElement.setData(undefined)
state.selectedElement.setData(undefined);
// When aborted, we force the contributors to place the pin _again_
// This is because there might be a nearby object that was disabled; this forces them to re-evaluate the map
state.lastClickObject.features.setData([])
preciseInputIsTapped = false
state.lastClickObject.features.setData([]);
preciseInputIsTapped = false;
}
async function confirm() {
creating = true
const location: { lon: number; lat: number } = preciseCoordinate.data
const snapTo: WayId | undefined = <WayId>snappedToObject.data
creating = true;
const location: { lon: number; lat: number } = preciseCoordinate.data;
const snapTo: WayId | undefined = <WayId>snappedToObject.data;
const tags: Tag[] = selectedPreset.preset.tags.concat(
..._globalFilter.map((f) => f?.onNewPoint?.tags ?? [])
)
console.log("Creating new point at", location, "snapped to", snapTo, "with tags", tags)
);
console.log("Creating new point at", location, "snapped to", snapTo, "with tags", tags);
let snapToWay: undefined | OsmWay = undefined
let snapToWay: undefined | OsmWay = undefined;
if (snapTo !== undefined) {
const downloaded = await state.osmObjectDownloader.DownloadObjectAsync(snapTo, 0)
const downloaded = await state.osmObjectDownloader.DownloadObjectAsync(snapTo, 0);
if (downloaded !== "deleted") {
snapToWay = downloaded
snapToWay = downloaded;
}
}
@ -113,33 +113,42 @@
theme: state.layout?.id ?? "unkown",
changeType: "create",
snapOnto: snapToWay,
})
await state.changes.applyAction(newElementAction)
state.newFeatures.features.ping()
reusePointWithinMeters: 1
});
await state.changes.applyAction(newElementAction);
state.newFeatures.features.ping();
// The 'changes' should have created a new point, which added this into the 'featureProperties'
const newId = newElementAction.newElementId
console.log("Applied pending changes, fetching store for", newId)
const tagsStore = state.featureProperties.getStore(newId)
const newId = newElementAction.newElementId;
console.log("Applied pending changes, fetching store for", newId);
const tagsStore = state.featureProperties.getStore(newId);
if (!tagsStore) {
console.error("Bug: no tagsStore found for", newId);
}
{
// Set some metainfo
const properties = tagsStore.data
const properties = tagsStore.data;
if (snapTo) {
// metatags (starting with underscore) are not uploaded, so we can safely mark this
delete properties["_referencing_ways"]
properties["_referencing_ways"] = `["${snapTo}"]`
delete properties["_referencing_ways"];
properties["_referencing_ways"] = `["${snapTo}"]`;
}
properties["_backend"] = state.osmConnection.Backend()
properties["_last_edit:timestamp"] = new Date().toISOString()
const userdetails = state.osmConnection.userDetails.data
properties["_last_edit:contributor"] = userdetails.name
properties["_last_edit:uid"] = "" + userdetails.uid
tagsStore.ping()
properties["_backend"] = state.osmConnection.Backend();
properties["_last_edit:timestamp"] = new Date().toISOString();
const userdetails = state.osmConnection.userDetails.data;
properties["_last_edit:contributor"] = userdetails.name;
properties["_last_edit:uid"] = "" + userdetails.uid;
tagsStore.ping();
}
const feature = state.indexedFeatures.featuresById.data.get(newId)
abort()
state.selectedLayer.setData(selectedPreset.layer)
state.selectedElement.setData(feature)
tagsStore.ping()
const feature = state.indexedFeatures.featuresById.data.get(newId);
console.log("Selecting feature", feature, "and opening their popup");
abort();
state.selectedLayer.setData(selectedPreset.layer);
state.selectedElement.setData(feature);
tagsStore.ping();
}
function confirmSync() {
confirm().then(_ => console.debug("New point successfully handled")).catch(e => console.error("Handling the new point went wrong due to", e));
}
</script>
@ -328,7 +337,7 @@
"absolute top-0 flex w-full justify-center p-12"
)}
>
<NextButton on:click={confirm} clss="primary w-fit">
<NextButton on:click={confirmSync} clss="primary w-fit">
<div class="flex w-full justify-end gap-x-2">
<Tr t={Translations.t.general.add.confirmLocation} />
</div>

View file

@ -88,7 +88,6 @@
}
console.log("Inited 'checkMappings' to", checkedMappings);
if (confg.freeform?.key) {
if (!confg.multiAnswer) {
// Somehow, setting multi-answer freeform values is broken if this is not set