Improve cross way check

1. Add more way types and relation types to check
2. Add more legit crossing cases
3. Add tests for 1 and 2
This commit is contained in:
Xiaoming Gao
2018-12-20 15:42:21 -05:00
committed by Xiaoming Gao
parent c9350c632e
commit dee005e9aa
2 changed files with 288 additions and 21 deletions
+103 -9
View File
@@ -1,3 +1,4 @@
import _clone from 'lodash-es/clone';
import { geoExtent, geoLineIntersection } from '../geo';
import { set as d3_set } from 'd3-collection';
import { t } from '../util/locale';
@@ -39,10 +40,104 @@ export function validationHighwayCrossingOtherWays() {
return n1 > nA ? n1 + n2 + nA + nB : nA + nB + n1 + n2;
}
function getWayAndParentRelationTags(way, graph) {
var tags = _clone(way.tags);
var parentRels = graph.parentRelations(way);
for (var i = 0; i < parentRels.length; i++) {
var rel = parentRels[i];
for (k in rel.tags) {
if (!tags[k]) tags[k] = rel.tags[k];
}
}
return tags;
}
function hasTag(tags, key) {
return tags[key] !== undefined && tags[key] !== 'no';
}
function getFeatureTypeForCrossingCheck(way, graph) {
var tags = getWayAndParentRelationTags(way, graph);
if (hasTag(tags, 'highway')) return 'highway';
if (hasTag(tags, 'building')) return 'building';
if (hasTag(tags, 'railway')) return 'railway';
if (hasTag(tags, 'waterway') || tags.natural === 'water') return 'water';
return null;
}
function extendTagsByInferredLayer(tags, way) {
if (!hasTag(tags, 'layer')) {
tags.layer = way.layer().toString();
}
return tags;
}
function isLegitCrossing(way1, featureType1, way2, featureType2, graph) {
var tags1 = getWayAndParentRelationTags(way1, graph),
tags2 = getWayAndParentRelationTags(way2, graph);
tags1 = extendTagsByInferredLayer(tags1, way1);
tags2 = extendTagsByInferredLayer(tags2, way2);
// For better readability, not chaining all the true conditions into one if statement.
if ((featureType1 === 'highway' && featureType2 === 'highway') ||
(featureType1 === 'highway' && featureType2 === 'railway') ||
(featureType1 === 'railway' && featureType2 === 'railway')) {
// Legit cases:
// (1) they're on different layers
// (2) only one of the two ways is on a bridge
// (3) only one of the two ways is in a tunnel
if (tags1.layer !== tags2.layer) return true;
if (hasTag(tags1, 'bridge') && !hasTag(tags2, 'bridge')) return true;
if (!hasTag(tags1, 'bridge') && hasTag(tags2, 'bridge')) return true;
if (hasTag(tags1, 'tunnel') && !hasTag(tags2, 'tunnel')) return true;
if (!hasTag(tags1, 'tunnel') && hasTag(tags2, 'tunnel')) return true;
}
if ((featureType1 === 'highway' && featureType2 === 'water') ||
(featureType1 === 'railway' && featureType2 === 'water')) {
// Legit cases:
// (1) highway/railway is on a bridge
// (2) only one of the two ways is in a tunnel
// (3) both are in tunnels but on different layers
if (hasTag(tags1, 'bridge')) return true;
if (hasTag(tags1, 'tunnel') && !hasTag(tags2, 'tunnel')) return true;
if (!hasTag(tags1, 'tunnel') && hasTag(tags2, 'tunnel')) return true;
if (hasTag(tags1, 'tunnel') && hasTag(tags2, 'tunnel') && tags1.layer !== tags2.layer) return true;
}
if ((featureType1 === 'highway' && featureType2 === 'building') ||
(featureType1 === 'railway' && featureType2 === 'building')) {
// Legit cases:
// (1) highway/railway has a bridge or tunnel tag
// (2) highway/railway has a covered tag
if (hasTag(tags1, 'bridge') || hasTag(tags1, 'tunnel') || hasTag(tags1, 'covered')) return true;
}
if (featureType1 === 'water' && featureType2 === 'water') {
// Legit cases:
// (1) only one of the water is in a tunnel
// (2) both are in tunnels but on differnt layers
if (hasTag(tags1, 'tunnel') && !hasTag(tags2, 'tunnel')) return true;
if (!hasTag(tags1, 'tunnel') && hasTag(tags2, 'tunnel')) return true;
if (hasTag(tags1, 'tunnel') && hasTag(tags2, 'tunnel') && tags1.layer !== tags2.layer) return true;
}
if (featureType1 === 'water' && featureType2 === 'building') {
// Legit cases:
// (1) water is in a tunnel
// (2) water has a covered tag
if (hasTag(tags1, 'tunnel') || hasTag(tags1, 'covered')) return true;
}
if (featureType1 === 'building' && featureType2 === 'building') {
// Legit case: they're on different layers
if (tags1.layer !== tags2.layer) return true;
}
return false;
}
function findCrossingsByWay(entity, graph, tree, edgePairsVisited) {
var edgeCrossInfos = [];
if (entity.type !== 'way' || !entity.tags.highway) return edgeCrossInfos;
if (entity.geometry(graph) !== 'line') return edgeCrossInfos;
if (entity.geometry(graph) !== 'line' || entity.type !== 'way') return edgeCrossInfos;
var entFeatureType = getFeatureTypeForCrossingCheck(entity, graph);
if (entFeatureType === null) return edgeCrossInfos;
for (var i = 0; i < entity.nodes.length - 1; i++) {
var nid1 = entity.nodes[i],
@@ -64,14 +159,14 @@ export function validationHighwayCrossingOtherWays() {
if (intersected[j].type !== 'way') continue;
// only check crossing highway, waterway, building, and railway
var way = intersected[j];
if (!(way.tags.highway || way.tags.building ||
way.tags.railway || way.tags.waterway)) {
continue;
}
if (way.tags.waterway && entity.tags.bridge === 'yes') {
var way = intersected[j],
wayFeatureType = getFeatureTypeForCrossingCheck(way, graph);
if (wayFeatureType === null ||
isLegitCrossing(entity, entFeatureType, way, wayFeatureType, graph) ||
isLegitCrossing(way, wayFeatureType, entity, entFeatureType, graph)) {
continue;
}
var crossCoords = findEdgeToWayCrossCoords(n1, n2, way, graph, edgePairsVisited);
for (var k = 0; k < crossCoords.length; k++) {
edgeCrossInfos.push({
@@ -84,7 +179,6 @@ export function validationHighwayCrossingOtherWays() {
return edgeCrossInfos;
}
var validation = function(changes, graph, tree) {
// create one issue per crossing point
var edited = changes.created.concat(changes.modified),
+185 -12
View File
@@ -5,10 +5,10 @@ describe('iD.validations.crossing_ways', function () {
context = iD.Context();
});
function createWaysWithOneCrossingPoint() {
function createWaysWithOneCrossingPoint(tags1, tags2) {
var n1 = iD.Node({id: 'n-1', loc: [1,1]});
var n2 = iD.Node({id: 'n-2', loc: [2,2]});
var w1 = iD.Way({id: 'w-1', nodes: ['n-1', 'n-2'], tags: { highway: 'residential' }});
var w1 = iD.Way({id: 'w-1', nodes: ['n-1', 'n-2'], tags: tags1});
context.perform(
iD.actionAddEntity(n1),
@@ -18,7 +18,7 @@ describe('iD.validations.crossing_ways', function () {
var n3 = iD.Node({id: 'n-3', loc: [1,2]});
var n4 = iD.Node({id: 'n-4', loc: [2,1]});
var w2 = iD.Way({id: 'w-2', nodes: ['n-3', 'n-4'], tags: { highway: 'residential' }});
var w2 = iD.Way({id: 'w-2', nodes: ['n-3', 'n-4'], tags: tags2});
context.perform(
iD.actionAddEntity(n3),
@@ -53,20 +53,35 @@ describe('iD.validations.crossing_ways', function () {
);
}
function createHighwayCrossingRailway() {
var n1 = iD.Node({id: 'n-1', loc: [1,1]});
var n2 = iD.Node({id: 'n-2', loc: [2,2]});
var w1 = iD.Way({id: 'w-1', nodes: ['n-1', 'n-2'], tags: { highway: 'residential' }});
context.perform(
iD.actionAddEntity(n1),
iD.actionAddEntity(n2),
iD.actionAddEntity(w1)
);
var n3 = iD.Node({id: 'n-3', loc: [1,2]});
var n4 = iD.Node({id: 'n-4', loc: [2,1]});
var w2 = iD.Way({id: 'w-2', nodes: ['n-3', 'n-4'], tags: { railway: 'rail' }});
context.perform(
iD.actionAddEntity(n3),
iD.actionAddEntity(n4),
iD.actionAddEntity(w2)
);
}
function validate() {
var validator = iD.validationHighwayCrossingOtherWays();
var changes = context.history().changes();
return validator(changes, context.graph(), context.history().tree());
}
it('has no errors on init', function() {
var issues = validate();
expect(issues).to.have.lengthOf(0);
});
it('one cross point between two ways', function() {
createWaysWithOneCrossingPoint();
var issues = validate();
function verifySingleCrossingIssue(issues) {
expect(issues).to.have.lengthOf(1);
var issue = issues[0];
expect(issue.type).to.eql(iD.ValidationIssueType.crossing_ways);
@@ -77,9 +92,126 @@ describe('iD.validations.crossing_ways', function () {
expect(issue.coordinates).to.have.lengthOf(2);
expect(issue.coordinates[0]).to.eql(1.5);
expect(issue.coordinates[1]).to.eql(1.5);
}
it('has no errors on init', function() {
var issues = validate();
expect(issues).to.have.lengthOf(0);
});
it('two cross points between two ways', function() {
// legit crossing cases
it('legit crossing between highway and highway', function() {
createWaysWithOneCrossingPoint({ highway: 'residential', tunnel: 'yes', layer: '-1' }, { highway: 'residential' });
var issues = validate();
expect(issues).to.have.lengthOf(0);
});
it('legit crossing between highway and railway', function() {
createWaysWithOneCrossingPoint({ highway: 'residential' }, { railway: 'rail', bridge: 'yes' });
var issues = validate();
expect(issues).to.have.lengthOf(0);
});
it('legit crossing between highway and waterway', function() {
createWaysWithOneCrossingPoint({ highway: 'residential', bridge: 'yes' }, { waterway: 'river' });
var issues = validate();
expect(issues).to.have.lengthOf(0);
});
it('legit crossing between highway and building', function() {
createWaysWithOneCrossingPoint({ highway: 'residential', covered: 'yes' }, { building: 'yes' });
var issues = validate();
expect(issues).to.have.lengthOf(0);
});
it('legit crossing between railway and railway', function() {
createWaysWithOneCrossingPoint({ railway: 'rail', layer: '1' }, { railway: 'rail' });
var issues = validate();
expect(issues).to.have.lengthOf(0);
});
it('legit crossing between railway and waterway', function() {
createWaysWithOneCrossingPoint({ railway: 'rail' }, { waterway: 'river', tunnel: 'yes' });
var issues = validate();
expect(issues).to.have.lengthOf(0);
});
it('legit crossing between railway and building', function() {
createWaysWithOneCrossingPoint({ railway: 'rail', covered: 'yes' }, { building: 'yes' });
var issues = validate();
expect(issues).to.have.lengthOf(0);
});
it('legit crossing between waterway and waterway', function() {
createWaysWithOneCrossingPoint({ waterway: 'canal', tunnel: 'yes' }, { waterway: 'river' });
var issues = validate();
expect(issues).to.have.lengthOf(0);
});
it('legit crossing between waterway and building', function() {
createWaysWithOneCrossingPoint({ waterway: 'river', covered: 'yes' }, { building: 'yes' });
var issues = validate();
expect(issues).to.have.lengthOf(0);
});
it('legit crossing between building and building', function() {
createWaysWithOneCrossingPoint({ building: 'yes' }, { building: 'yes', covered: 'yes' });
var issues = validate();
expect(issues).to.have.lengthOf(0);
});
// warning crossing cases between ways
it('one cross point between highway and highway', function() {
createWaysWithOneCrossingPoint({ highway: 'residential' }, { highway: 'residential' });
verifySingleCrossingIssue(validate());
});
it('one cross point between highway and railway', function() {
createWaysWithOneCrossingPoint({ highway: 'residential' }, { railway: 'rail' });
verifySingleCrossingIssue(validate());
});
it('one cross point between highway and waterway', function() {
createWaysWithOneCrossingPoint({ highway: 'residential' }, { waterway: 'river' });
verifySingleCrossingIssue(validate());
});
it('one cross point between highway and building', function() {
createWaysWithOneCrossingPoint({ highway: 'residential' }, { building: 'yes' });
verifySingleCrossingIssue(validate());
});
it('one cross point between railway and railway', function() {
createWaysWithOneCrossingPoint({ railway: 'rail' }, { railway: 'rail' });
verifySingleCrossingIssue(validate());
});
it('one cross point between railway and waterway', function() {
createWaysWithOneCrossingPoint({ railway: 'rail' }, { waterway: 'river' });
verifySingleCrossingIssue(validate());
});
it('one cross point between railway and building', function() {
createWaysWithOneCrossingPoint({ railway: 'rail' }, { building: 'yes' });
verifySingleCrossingIssue(validate());
});
it('one cross point between waterway and waterway', function() {
createWaysWithOneCrossingPoint({ waterway: 'canal' }, { waterway: 'river' });
verifySingleCrossingIssue(validate());
});
it('one cross point between waterway and building', function() {
createWaysWithOneCrossingPoint({ waterway: 'river' }, { building: 'yes' });
verifySingleCrossingIssue(validate());
});
it('one cross point between building and building', function() {
createWaysWithOneCrossingPoint({ building: 'yes' }, { building: 'yes' });
verifySingleCrossingIssue(validate());
});
it('two cross points between two highways', function() {
createWaysWithTwoCrossingPoint();
var issues = validate();
expect(issues).to.have.lengthOf(2);
@@ -103,4 +235,45 @@ describe('iD.validations.crossing_ways', function () {
expect(issue.coordinates[0]).to.eql(2.5);
expect(issue.coordinates[1]).to.eql(2.5);
});
function createWayAndRelationWithOneCrossingPoint(wayTags, relTags) {
var n1 = iD.Node({id: 'n-1', loc: [1,1]});
var n2 = iD.Node({id: 'n-2', loc: [2,2]});
var w1 = iD.Way({id: 'w-1', nodes: ['n-1', 'n-2'], tags: wayTags});
context.perform(
iD.actionAddEntity(n1),
iD.actionAddEntity(n2),
iD.actionAddEntity(w1)
);
var n3 = iD.Node({id: 'n-3', loc: [1,2]});
var n4 = iD.Node({id: 'n-4', loc: [2,1]});
var n5 = iD.Node({id: 'n-5', loc: [3,2]});
var n6 = iD.Node({id: 'n-6', loc: [2,3]});
var w2 = iD.Way({id: 'w-2', nodes: ['n-3', 'n-4', 'n-5'], tags: {}});
var w3 = iD.Way({id: 'w-3', nodes: ['n-5', 'n-6', 'n-3'], tags: {}});
var r1 = iD.Relation({id: 'r-1', members: [{id: 'w-2'}, {id: 'w-3'}], tags: relTags});
context.perform(
iD.actionAddEntity(n3),
iD.actionAddEntity(n4),
iD.actionAddEntity(n5),
iD.actionAddEntity(n6),
iD.actionAddEntity(w2),
iD.actionAddEntity(w3),
iD.actionAddEntity(r1),
);
}
// warning crossing cases between way and relation
it('one cross point between highway and water relation', function() {
createWayAndRelationWithOneCrossingPoint({ highway: 'residential' }, { natural: 'water' });
verifySingleCrossingIssue(validate());
});
it('one cross point between railway and building relation', function() {
createWayAndRelationWithOneCrossingPoint({ highway: 'residential' }, { building: 'yes' });
verifySingleCrossingIssue(validate());
});
});