contrapunctus/MapComplete
1
0
Fork 0
forked from MapComplete/MapComplete
Code Pull requests Activity

Add unused node removal

Browse source
This commit is contained in:
Pieter Vander Vennet 2021-12-23 03:36:03 +01:00
parent 4131e9b9e2
commit adade2e8b0
12 changed files with 289 additions and 96 deletions
Download patch file Download diff file Expand all files Collapse all files

43
Logic/Osm/Actions/CreateWayWithPointReuseAction.ts
View file

@ -1,4 +1,4 @@
import OsmChangeAction, {OsmCreateAction} from "./OsmChangeAction";
import {OsmCreateAction} from "./OsmChangeAction";
import {Tag} from "../../Tags/Tag";
import {Changes} from "../Changes";
import {ChangeDescription} from "./ChangeDescription";
@ -18,10 +18,34 @@ export interface MergePointConfig {
mode: "reuse_osm_point" | "move_osm_point"
}
/**
* CreateWayWithPointreuse will create a 'CoordinateInfo' for _every_ point in the way to be created.
*
* The CoordinateInfo indicates the action to take, e.g.:
*
* - Create a new point
* - Reuse an existing OSM point (and don't move it)
* - Reuse an existing OSM point (and leave it where it is)
* - Reuse another Coordinate info (and don't do anything else with it)
*
*/
interface CoordinateInfo {
/**
* The new coordinate
*/
lngLat: [number, number],
/**
* If set: indicates that this point is identical to an earlier point in the way and that that point should be used.
* This is especially needed in closed ways, where the last CoordinateInfo will have '0' as identicalTo
*/
identicalTo?: number,
/**
* Information about the closebyNode which might be reused
*/
closebyNodes?: {
/**
* Distance in meters between the target coordinate and this candidate coordinate
*/
d: number,
node: any,
config: MergePointConfig
@ -53,6 +77,8 @@ export default class CreateWayWithPointReuseAction extends OsmCreateAction {
this._tags = tags;
this._state = state;
this._config = config;
// The main logic of this class: the coordinateInfo contains all the changes
this._coordinateInfo = this.CalculateClosebyNodes(coordinates);
}
@ -219,6 +245,9 @@ export default class CreateWayWithPointReuseAction extends OsmCreateAction {
return allChanges
}
/**
* Calculates the main changes.
*/
private CalculateClosebyNodes(coordinates: [number, number][]): CoordinateInfo[] {
const bbox = new BBox(coordinates)
@ -226,6 +255,7 @@ export default class CreateWayWithPointReuseAction extends OsmCreateAction {
const allNodes = [].concat(...state.featurePipeline.GetFeaturesWithin("type_node", bbox.pad(1.2)))
const maxDistance = Math.max(...this._config.map(c => c.withinRangeOfM))
// Init coordianteinfo with undefined but the same length as coordinates
const coordinateInfo: {
lngLat: [number, number],
identicalTo?: number,
@ -236,6 +266,8 @@ export default class CreateWayWithPointReuseAction extends OsmCreateAction {
}[]
}[] = coordinates.map(_ => undefined)
// First loop: gather all information...
for (let i = 0; i < coordinates.length; i++) {
if (coordinateInfo[i] !== undefined) {
@ -243,8 +275,11 @@ export default class CreateWayWithPointReuseAction extends OsmCreateAction {
continue
}
const coor = coordinates[i]
// Check closeby (and probably identical) point further in the coordinate list, mark them as duplicate
// Check closeby (and probably identical) points further in the coordinate list, mark them as duplicate
for (let j = i + 1; j < coordinates.length; j++) {
// We look into the 'future' of the way and mark those 'future' locations as being the same as this location
// The continue just above will make sure they get ignored
// This code is important to 'close' ways
if (GeoOperations.distanceBetween(coor, coordinates[j]) < 0.1) {
coordinateInfo[j] = {
lngLat: coor,
@ -280,6 +315,7 @@ export default class CreateWayWithPointReuseAction extends OsmCreateAction {
}
}
// Sort by distance, closest first
closebyNodes.sort((n0, n1) => {
return n0.d - n1.d
})
@ -292,8 +328,9 @@ export default class CreateWayWithPointReuseAction extends OsmCreateAction {
}
// Second loop: figure out which point moves where without creating conflicts
let conflictFree = true;
do {
conflictFree = true;
for (let i = 0; i < coordinateInfo.length; i++) {

2
Logic/Osm/Actions/DeleteAction.ts
View file

@ -37,7 +37,7 @@ export default class DeleteAction extends OsmChangeAction {
}
protected async CreateChangeDescriptions(changes: Changes): Promise<ChangeDescription[]> {
public async CreateChangeDescriptions(changes: Changes): Promise<ChangeDescription[]> {
const osmObject = await OsmObject.DownloadObjectAsync(this._id)

243
Logic/Osm/Actions/ReplaceGeometryAction.ts
View file

@ -11,28 +11,36 @@ import ChangeTagAction from "./ChangeTagAction";
import {And} from "../../Tags/And";
import {Utils} from "../../../Utils";
import {OsmConnection} from "../OsmConnection";
import {GeoJSONObject} from "@turf/turf";
import FeaturePipeline from "../../FeatureSource/FeaturePipeline";
import DeleteAction from "./DeleteAction";
export default class ReplaceGeometryAction extends OsmChangeAction {
/**
* The target feature - mostly used for the metadata
*/
private readonly feature: any;
private readonly state: {
osmConnection: OsmConnection
osmConnection: OsmConnection,
featurePipeline: FeaturePipeline
};
private readonly wayToReplaceId: string;
private readonly theme: string;
/**
* The target coordinates that should end up in OpenStreetMap
* The target coordinates that should end up in OpenStreetMap.
* This is identical to either this.feature.geometry.coordinates or -in case of a polygon- feature.geometry.coordinates[0]
*/
private readonly targetCoordinates: [number, number][];
/**
* If a target coordinate is close to another target coordinate, 'identicalTo' will point to the first index.
* @private
*/
private readonly identicalTo: number[]
private readonly newTags: Tag[] | undefined;
constructor(
state: {
osmConnection: OsmConnection
osmConnection: OsmConnection,
featurePipeline: FeaturePipeline
},
feature: any,
wayToReplaceId: string,
@ -54,6 +62,7 @@ export default class ReplaceGeometryAction extends OsmChangeAction {
} else if (geom.type === "Polygon") {
coordinates = geom.coordinates[0]
}
this.targetCoordinates = coordinates
this.identicalTo = coordinates.map(_ => undefined)
@ -68,21 +77,18 @@ export default class ReplaceGeometryAction extends OsmChangeAction {
}
}
}
this.targetCoordinates = coordinates
this.newTags = options.newTags
}
// noinspection JSUnusedGlobalSymbols
public async getPreview(): Promise<FeatureSource> {
const {closestIds, allNodesById} = await this.GetClosestIds();
const {closestIds, allNodesById, detachedNodeIds} = await this.GetClosestIds();
console.debug("Generating preview, identicals are ",)
const preview = closestIds.map((newId, i) => {
const preview: GeoJSONObject[] = closestIds.map((newId, i) => {
if (this.identicalTo[i] !== undefined) {
return undefined
}
if (newId === undefined) {
return {
type: "Feature",
@ -110,6 +116,24 @@ export default class ReplaceGeometryAction extends OsmChangeAction {
}
};
})
for (const detachedNodeId of detachedNodeIds) {
const origPoint = allNodesById.get(detachedNodeId).centerpoint()
const feature = {
type: "Feature",
properties: {
"detach": "yes",
"id": "replace-geometry-detach-" + detachedNodeId
},
geometry: {
type: "Point",
coordinates: [origPoint[1], origPoint[0]]
}
};
preview.push(feature)
}
return new StaticFeatureSource(Utils.NoNull(preview), false)
}
@ -119,7 +143,7 @@ export default class ReplaceGeometryAction extends OsmChangeAction {
const allChanges: ChangeDescription[] = []
const actualIdsToUse: number[] = []
const {closestIds, osmWay} = await this.GetClosestIds()
const {closestIds, osmWay, detachedNodeIds} = await this.GetClosestIds()
for (let i = 0; i < closestIds.length; i++) {
if (this.identicalTo[i] !== undefined) {
@ -170,7 +194,7 @@ export default class ReplaceGeometryAction extends OsmChangeAction {
allChanges.push(...await addExtraTags.CreateChangeDescriptions(changes))
}
// AT the very last: actually change the nodes of the way!
// Actually change the nodes of the way!
allChanges.push({
type: "way",
id: osmWay.id,
@ -185,6 +209,43 @@ export default class ReplaceGeometryAction extends OsmChangeAction {
})
// Some nodes might need to be deleted
if (detachedNodeIds.length > 0) {
const nodeDb = this.state.featurePipeline.fullNodeDatabase;
if (nodeDb === undefined) {
throw "PANIC: replaceGeometryAction needs the FullNodeDatabase, which is undefined. This should be initialized by having the 'type_node'-layer enabled in your theme. (NB: the replacebutton has type_node as dependency)"
}
for (const nodeId of detachedNodeIds) {
const osmNode = nodeDb.GetNode(nodeId)
const parentWayIds: number[] = JSON.parse(osmNode.tags["parent_way_ids"])
const index = parentWayIds.indexOf(osmWay.id)
if(index < 0){
console.error("ReplaceGeometryAction is trying to detach node "+nodeId+", but it isn't listed as being part of way "+osmWay.id)
continue;
}
parentWayIds.splice(index, 1)
osmNode.tags["parent_way_ids"] = JSON.stringify(parentWayIds)
if(parentWayIds.length == 0){
// This point has no other ways anymore - lets clean it!
console.log("Removing node "+nodeId, "as it isn't needed anymore by any way")
allChanges.push({
meta: {
theme: this.theme,
changeType: "delete"
},
doDelete: true,
type: "node",
id: nodeId,
})
}
}
}
return allChanges
}
@ -193,84 +254,124 @@ export default class ReplaceGeometryAction extends OsmChangeAction {
* @constructor
* @private
*/
private async GetClosestIds(): Promise<{ closestIds: number[], allNodesById: Map<number, OsmNode>, osmWay: OsmWay }> {
private async GetClosestIds(): Promise<{
// A list of the same length as targetCoordinates, containing which OSM-point to move. If undefined, a new point will be created
closestIds: number[],
allNodesById: Map<number, OsmNode>,
osmWay: OsmWay,
detachedNodeIds: number[]
}> {
// TODO FIXME: cap move length on points which are embedded into other ways (ev. disconnect them)
// TODO FIXME: if a new point has to be created, snap to already existing ways
// TODO FIXME: detect intersections with other ways if moved
const splitted = this.wayToReplaceId.split("/");
const type = splitted[0];
const idN = Number(splitted[1]);
if (idN < 0 || type !== "way") {
throw "Invalid ID to conflate: " + this.wayToReplaceId
}
const url = `${this.state.osmConnection._oauth_config.url}/api/0.6/${this.wayToReplaceId}/full`;
const rawData = await Utils.downloadJsonCached(url, 1000)
const parsed = OsmObject.ParseObjects(rawData.elements);
const allNodesById = new Map<number, OsmNode>()
const allNodes = parsed.filter(o => o.type === "node")
for (const node of allNodes) {
allNodesById.set(node.id, <OsmNode>node)
}
let parsed: OsmObject[];
{
// Gather the needed OsmObjects
const splitted = this.wayToReplaceId.split("/");
const type = splitted[0];
const idN = Number(splitted[1]);
if (idN < 0 || type !== "way") {
throw "Invalid ID to conflate: " + this.wayToReplaceId
}
const url = `${this.state.osmConnection._oauth_config.url}/api/0.6/${this.wayToReplaceId}/full`;
const rawData = await Utils.downloadJsonCached(url, 1000)
parsed = OsmObject.ParseObjects(rawData.elements);
}
const allNodes = parsed.filter(o => o.type === "node")
/**
* Allright! We know all the nodes of the original way and all the nodes of the target coordinates.
* For each of the target coordinates, we search the closest, already existing point and reuse this point
* For every already existing OSM-point, we calculate the distance to every target point
*/
const closestIds = []
const distances = []
for (let i = 0; i < this.targetCoordinates.length; i++) {
const target = this.targetCoordinates[i];
let closestDistance = undefined
let closestId = undefined;
for (const osmNode of allNodes) {
const cp = osmNode.centerpoint()
const d = GeoOperations.distanceBetween(target, [cp[1], cp[0]])
if (closestId === undefined || closestDistance > d) {
closestId = osmNode.id
closestDistance = d
const distances = new Map<number /* osmId*/, number[] /* target coordinate index --> distance (or undefined if a duplicate)*/>();
for (const node of allNodes) {
const nodeDistances = this.targetCoordinates.map(_ => undefined)
for (let i = 0; i < this.targetCoordinates.length; i++) {
if (this.identicalTo[i] !== undefined) {
continue;
}
const targetCoordinate = this.targetCoordinates[i];
const cp = node.centerpoint()
nodeDistances[i] = GeoOperations.distanceBetween(targetCoordinate, [cp[1], cp[0]])
}
closestIds.push(closestId)
distances.push(closestDistance)
distances.set(node.id, nodeDistances)
}
// Next step: every closestId can only occur once in the list
// We skip the ones which are identical
console.log("Erasing double ids")
for (let i = 0; i < closestIds.length; i++) {
if (this.identicalTo[i] !== undefined) {
closestIds[i] = closestIds[this.identicalTo[i]]
continue
}
const closestId = closestIds[i]
for (let j = i + 1; j < closestIds.length; j++) {
if (this.identicalTo[j] !== undefined) {
continue
/**
* Then, we search the node that has to move the least distance and add this as mapping.
* We do this until no points are left
*/
let candidate: number;
let moveDistance: number;
const closestIds = this.targetCoordinates.map(_ => undefined)
/**
* The list of nodes that are _not_ used anymore, typically if there are less targetCoordinates then source coordinates
*/
const unusedIds = []
do {
candidate = undefined;
moveDistance = Infinity;
distances.forEach((distances, nodeId) => {
const minDist = Math.min(...Utils.NoNull(distances))
if (moveDistance > minDist) {
// We have found a candidate to move
candidate = nodeId
moveDistance = minDist
}
const otherClosestId = closestIds[j]
if (closestId !== otherClosestId) {
continue
})
if (candidate !== undefined) {
// We found a candidate... Search the corresponding target id:
let targetId: number = undefined;
let lowestDistance = Number.MAX_VALUE
let nodeDistances = distances.get(candidate)
for (let i = 0; i < nodeDistances.length; i++) {
const d = nodeDistances[i]
if (d !== undefined && d < lowestDistance) {
lowestDistance = d;
targetId = i;
}
}
// We have two occurences of 'closestId' - we only keep the closest instance!
const di = distances[i]
const dj = distances[j]
if (di < dj) {
closestIds[j] = undefined
// This candidates role is done, it can be removed from the distance matrix
distances.delete(candidate)
if (targetId !== undefined) {
// At this point, we have our target coordinate index: targetId!
// Lets map it...
closestIds[targetId] = candidate
// To indicate that this targetCoordinate is taken, we remove them from the distances matrix
distances.forEach(dists => {
dists[targetId] = undefined
})
} else {
closestIds[i] = undefined
// Seems like all the targetCoordinates have found a source point
unusedIds.push(candidate)
}
}
}
} while (candidate !== undefined)
const osmWay = <OsmWay>parsed[parsed.length - 1]
if (osmWay.type !== "way") {
throw "WEIRD: expected an OSM-way as last element here!"
// If there are still unused values in 'distances', they are definitively unused
distances.forEach((_, nodeId) => {
unusedIds.push(nodeId)
})
{
// Some extra data is included for rendering
const osmWay = <OsmWay>parsed[parsed.length - 1]
if (osmWay.type !== "way") {
throw "WEIRD: expected an OSM-way as last element here!"
}
const allNodesById = new Map<number, OsmNode>()
for (const node of allNodes) {
allNodesById.set(node.id, <OsmNode>node)
}
return {closestIds, allNodesById, osmWay, detachedNodeIds: unusedIds};
}
return {closestIds, allNodesById, osmWay};
}

5
Logic/Osm/Changes.ts
View file

@ -384,8 +384,8 @@ export class Changes {
states.set(o.type + "/" + o.id, "unchanged")
}
let changed = false;
for (const change of changes) {
let changed = false;
const id = change.type + "/" + change.id
if (!objects.has(id)) {
// The object hasn't been seen before, so it doesn't exist yet and is newly created by its very definition
@ -493,7 +493,7 @@ export class Changes {
}
if (changed && state === "unchanged") {
if (changed && states.get(id) === "unchanged") {
states.set(id, "modified")
}
}
@ -520,6 +520,7 @@ export class Changes {
})
console.debug("Calculated the pending changes: ", result.newObjects.length,"new; ", result.modifiedObjects.length,"modified;",result.deletedObjects,"deleted")
return result
}
}