From b4dacdad2a9d0855b13e5fbda8596f5039758c94 Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Thu, 2 May 2019 11:39:12 -0700 Subject: [PATCH] Reduce very close nodes validation threshold (close #6292) Use spherical distances for very close nodes validation Don't flag very close nodes from different ways Don't flag very close nodes if both have interesting tags Update very close nodes validation reference string --- data/core.yaml | 3 +- dist/locales/en.json | 3 +- modules/core/validator.js | 5 +- modules/validations/close_nodes.js | 109 ++++++++++++++--------------- 4 files changed, 59 insertions(+), 61 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index 77c5d5a27..e09743918 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -1323,8 +1323,7 @@ en: title: "Very Close Points" message: "Two points in {way} are very close together" tip: "Find redundant points in ways" - ref_merge: "Nodes are less than 2 meters away; you may want to merge them." - ref_move_away: "Nodes are less than 2 meters away but not mergable; you may want to move them further apart." + reference: "Redundant points in a way should be merged or moved apart." crossing_ways: title: Crossings Ways message: "{feature} crosses {feature2}" diff --git a/dist/locales/en.json b/dist/locales/en.json index 1a28e5652..209c96547 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1623,8 +1623,7 @@ "title": "Very Close Points", "message": "Two points in {way} are very close together", "tip": "Find redundant points in ways", - "ref_merge": "Nodes are less than 2 meters away; you may want to merge them.", - "ref_move_away": "Nodes are less than 2 meters away but not mergable; you may want to move them further apart." + "reference": "Redundant points in a way should be merged or moved apart." }, "crossing_ways": { "title": "Crossings Ways", diff --git a/modules/core/validator.js b/modules/core/validator.js index 5beb5ff0b..7ddf18a5f 100644 --- a/modules/core/validator.js +++ b/modules/core/validator.js @@ -284,9 +284,12 @@ export function coreValidator(context) { var entityIDsToCheck = entityIDs.reduce(function(acc, entityID) { if (acc.has(entityID)) return acc; + + var entity = graph.hasEntity(entityID); + if (!entity) return acc; + acc.add(entityID); - var entity = graph.entity(entityID); var checkParentRels = [entity]; if (entity.type === 'node') { // include parent ways diff --git a/modules/validations/close_nodes.js b/modules/validations/close_nodes.js index 0c36a8235..7db98799e 100644 --- a/modules/validations/close_nodes.js +++ b/modules/validations/close_nodes.js @@ -3,86 +3,75 @@ import { utilDisplayLabel } from '../util'; import { t } from '../util/locale'; import { validationIssue, validationIssueFix } from '../core/validation'; import { osmRoutableHighwayTagValues } from '../osm/tags'; -import { geoExtent } from '../geo'; +import { geoExtent, geoSphericalDistance } from '../geo'; export function validationCloseNodes() { var type = 'close_nodes'; + var thresholdMeters = 0.2; + + function getVeryCloseNodeIssues(node, context) { + + var issues = []; + + function checkForCloseness(node1, node2, way) { + if (node1.id !== node2.id && + !(node1.hasInterestingTags() && node2.hasInterestingTags()) && + geoSphericalDistance(node1.loc, node2.loc) < thresholdMeters) { + + issues.push(makeIssue(node1, node2, way, context)); + } + } - function isNodeOnRoad(node, context) { var parentWays = context.graph().parentWays(node); + for (var i = 0; i < parentWays.length; i++) { var parentWay = parentWays[i]; - if (osmRoutableHighwayTagValues[parentWay.tags.highway]) { - return parentWay; + + if (!parentWay.tags.highway || !osmRoutableHighwayTagValues[parentWay.tags.highway]) continue; + + var lastIndex = parentWay.nodes.length - 1; + for (var j in parentWay.nodes) { + if (j !== 0) { + if (parentWay.nodes[j-1] === node.id) { + checkForCloseness(node, context.entity(parentWay.nodes[j]), parentWay); + } + } + if (j !== lastIndex) { + if (parentWay.nodes[j+1] === node.id) { + checkForCloseness(context.entity(parentWay.nodes[j]), node, parentWay); + } + } } } - return false; + + return issues; } - function findDupeNode(node, context) { - var epsilon = 2e-5, - extent = geoExtent([ - [node.loc[0] - epsilon, node.loc[1] - epsilon], - [node.loc[0] + epsilon, node.loc[1] + epsilon] - ]); - var filteredEnts = context.intersects(extent); - for (var i = 0; i < filteredEnts.length; i++) { - var entity = filteredEnts[i]; - if (entity.type === 'node' && entity.id !== node.id && - Math.abs(node.loc[0] - entity.loc[0]) < epsilon && - Math.abs(node.loc[1] - entity.loc[1]) < epsilon && - isNodeOnRoad(entity, context) ) { - return entity; - } - } - return null; - } + function makeIssue(node1, node2, way, context) { - - var validation = function(entity, context) { - - if (entity.type !== 'node') return []; - - var road = isNodeOnRoad(entity, context); - if (!road) return []; - - var dupe = findDupeNode(entity, context); - if (dupe === null) return []; - - var mergable = !operationMerge([entity.id, dupe.id], context).disabled(); - var fixes = []; - if (mergable) { - fixes.push( + return new validationIssue({ + type: type, + severity: 'warning', + message: t('issues.close_nodes.message', { way: utilDisplayLabel(way, context) }), + reference: showReference, + entityIds: [node1.id, node2.id], + fixes: [ new validationIssueFix({ icon: 'iD-icon-plus', title: t('issues.fix.merge_points.title'), onClick: function() { var entityIds = this.issue.entityIds, operation = operationMerge([entityIds[0], entityIds[1]], context); - if (!operation.disabled()) { - operation(); - } + operation(); } }) - ); - } - - return [new validationIssue({ - type: type, - severity: 'warning', - message: t('issues.close_nodes.message', { way: utilDisplayLabel(road, context) }), - reference: showReference, - entityIds: [entity.id, dupe.id], - fixes: fixes - })]; - + ] + }); function showReference(selection) { - var referenceText = mergable - ? t('issues.close_nodes.ref_merge') - : t('issues.close_nodes.ref_move_away'); + var referenceText = t('issues.close_nodes.reference'); selection.selectAll('.issue-reference') .data([0]) .enter() @@ -90,6 +79,14 @@ export function validationCloseNodes() { .attr('class', 'issue-reference') .text(referenceText); } + } + + + var validation = function(entity, context) { + + if (entity.type !== 'node') return []; + + return getVeryCloseNodeIssues(entity, context); };