From e697bdbeb1ff63ece57bd19f77eb6bc38a378b4a Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Fri, 17 May 2019 00:15:53 -0400 Subject: [PATCH] Flag disconnected ferry routes Flag disconnected highway multipolygon outer member ways Let routing islands cross ferry routes --- CHANGELOG.md | 2 + data/core.yaml | 2 +- dist/locales/en.json | 2 +- modules/validations/disconnected_way.js | 110 +++++++++------------- modules/validations/impossible_oneway.js | 2 +- test/spec/validations/disconnected_way.js | 27 ------ 6 files changed, 50 insertions(+), 95 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f6535ce8..3c6f526a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -128,7 +128,9 @@ _Breaking changes, which may affect downstream projects or sites that embed iD, - Flag unreachable one-way highways and waterways flowing against each other ([#6216]) - Flag disconnected area and multipolygon highways ([#6075]) - Flag new highways disconnected from the larger road network ([#6284], thanks [@Bonkles], [@gaoxm]) +- Flag routing islands - Don't flag highways connected to ferry routes as disconnected ([#6287]) +- Flag disconnected ferry routes - Flag deprecated values among semicolon-delimited tags ([#6038]) - Add quick fixes for setting the `layer` to resolve certain Crossing Ways issues ([#5943]) - Rename "Generic Names" validation rule to "Suspicious Names" diff --git a/data/core.yaml b/data/core.yaml index 971d39de3..960644dde 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -1365,7 +1365,7 @@ en: reference: "Crossing indoor features should be connected or use different levels." disconnected_way: title: Disconnected Ways - tip: "Find unconnected highways and paths" + tip: "Find unroutable roads, paths, and ferry routes" routable: message: multiple: "{count} routable features are connected only to each other." diff --git a/dist/locales/en.json b/dist/locales/en.json index da2d3174f..2906974be 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -1684,7 +1684,7 @@ }, "disconnected_way": { "title": "Disconnected Ways", - "tip": "Find unconnected highways and paths", + "tip": "Find unroutable roads, paths, and ferry routes", "routable": { "message": { "multiple": "{count} routable features are connected only to each other." diff --git a/modules/validations/disconnected_way.js b/modules/validations/disconnected_way.js index f7aac3cac..5d064a80a 100644 --- a/modules/validations/disconnected_way.js +++ b/modules/validations/disconnected_way.js @@ -16,9 +16,7 @@ export function validationDisconnectedWay() { var validation = function checkDisconnectedWay(entity, context) { var graph = context.graph(); - if (!isTaggedAsHighway(entity)) return []; - - var routingIslandWays = routingIslandForWay(entity); + var routingIslandWays = routingIslandForEntity(entity); if (!routingIslandWays) return []; var fixes = []; @@ -113,9 +111,40 @@ export function validationDisconnectedWay() { .text(t('issues.disconnected_way.routable.reference')); } + function routingIslandForEntity(entity) { - function isConnectedVertex(vertex, way, relation, routingIslandSet) { - // can not accurately test vertices on tiles not downloaded from osm - #5938 + if (entity.type !== 'way') return null; + + if (!isRoutableWay(entity, true)) return null; + + var waysToCheck = [entity]; + var routingIsland = new Set([entity]); + + while (waysToCheck.length) { + var wayToCheck = waysToCheck.pop(); + var childNodes = graph.childNodes(wayToCheck); + for (var i in childNodes) { + var vertex = childNodes[i]; + var result = isConnectedVertex(vertex, routingIsland); + if (result === true) { + return null; + } else if (result === false) { + continue; + } + result.forEach(function(connectedWay) { + if (!routingIsland.has(connectedWay)) { + routingIsland.add(connectedWay); + waysToCheck.push(connectedWay); + } + }); + } + } + + return routingIsland; + } + + function isConnectedVertex(vertex, routingIslandWays) { + // assume ways overlapping unloaded tiles are connected to the wider road network - #5938 var osm = context.connection(); if (osm && !osm.isDataLoaded(vertex.loc)) return true; @@ -135,27 +164,9 @@ export function validationDisconnectedWay() { var parentWay = parentsWays[i]; // ignore any way we've already accounted for - if (routingIslandSet.has(parentWay)) continue; + if (routingIslandWays.has(parentWay)) continue; - // count connections to ferry routes as connected - if (parentWay.tags.route === 'ferry') return true; - - if (isTaggedAsHighway(parentWay)) connectedWays.add(parentWay); - - var parentRelations = graph.parentRelations(parentWay); - - for (var j in parentRelations) { - var parentRelation = parentRelations[j]; - - // ignore the relation we're testing, if any - if (relation && parentRelation === relation) continue; - - if (parentRelation.tags.type === 'route' && - parentRelation.tags.route === 'ferry') return true; - - if (parentRelation.isMultipolygon() && - isTaggedAsHighway(parentRelation)) return connectedWays.add(parentWay); - } + if (isRoutableWay(parentWay, false)) connectedWays.add(parentWay); } if (connectedWays.size) return connectedWays; @@ -163,49 +174,18 @@ export function validationDisconnectedWay() { return false; } + function isRoutableWay(way, ignoreInnerWays) { + if (isTaggedAsHighway(way) || way.tags.route === 'ferry') return true; - function routingIslandForWay(way, relation) { - if (way.type !== 'way') return null; + return graph.parentRelations(way).some(function(parentRelation) { + if (parentRelation.tags.type === 'route' && + parentRelation.tags.route === 'ferry') return true; - var waysToCheck = [way]; - var routingIsland = new Set([way]); - - while (waysToCheck.length) { - var wayToCheck = waysToCheck.pop(); - var childNodes = graph.childNodes(wayToCheck); - for (var i in childNodes) { - var vertex = childNodes[i]; - var result = isConnectedVertex(vertex, entity, relation, routingIsland); - if (result === true) { - return null; - } else if (result === false) { - continue; - } - result.forEach(function(connectedWay) { - if (!routingIsland.has(connectedWay)) { - routingIsland.add(connectedWay); - waysToCheck.push(connectedWay); - } - }); - } - } - - return routingIsland; - } - - - /*function isDisconnectedMultipolygon(entity) { - if (entity.type !== 'relation' || !entity.isMultipolygon()) return false; - - return entity.members.every(function(member) { - if (member.type !== 'way') return true; - - var way = graph.hasEntity(member.id); - if (!way) return true; - - return isDisconnectedWay(way, entity); + if (parentRelation.isMultipolygon() && + isTaggedAsHighway(parentRelation) && + (!ignoreInnerWays || parentRelation.memberById(way.id).role !== 'inner')) return true; }); - }*/ + } function continueDrawing(way, vertex) { // make sure the vertex is actually visible and editable diff --git a/modules/validations/impossible_oneway.js b/modules/validations/impossible_oneway.js index 5d5e35c26..7d49ef87e 100644 --- a/modules/validations/impossible_oneway.js +++ b/modules/validations/impossible_oneway.js @@ -207,7 +207,7 @@ export function validationImpossibleOneway() { } } - var validation = function checkDisconnectedWay(entity, context) { + var validation = function checkImpossibleOneway(entity, context) { if (entity.type !== 'way' || entity.geometry(context.graph()) !== 'line') return []; diff --git a/test/spec/validations/disconnected_way.js b/test/spec/validations/disconnected_way.js index dfd757002..00ea24cc4 100644 --- a/test/spec/validations/disconnected_way.js +++ b/test/spec/validations/disconnected_way.js @@ -87,33 +87,6 @@ describe('iD.validations.disconnected_way', function () { expect(issue.entityIds[0]).to.eql('w-1'); }); - it('flags disconnected highway with disconnected entrance vertex', function() { - var n1 = iD.osmNode({id: 'n-1', loc: [4,4], tags: {'entrance': 'yes'}}); - var n2 = iD.osmNode({id: 'n-2', loc: [4,5]}); - var w = iD.osmWay({id: 'w-1', nodes: ['n-1', 'n-2'], tags: {'highway': 'unclassified'}}); - - context.perform( - iD.actionAddEntity(n1), - iD.actionAddEntity(n2), - iD.actionAddEntity(w) - ); - - var issues = validate(); - expect(issues).to.have.lengthOf(1); - var issue = issues[0]; - expect(issue.type).to.eql('disconnected_way'); - expect(issue.severity).to.eql('warning'); - expect(issue.entityIds).to.have.lengthOf(1); - expect(issue.entityIds[0]).to.eql('w-1'); - }); - - it('ignores highways that are connected to existing highways', function() { - createWayAtEndOfExistingOne({'highway': 'secondary'}, {'highway': 'secondary'}); - - var issues = validate(); - expect(issues).to.have.lengthOf(0); - }); - it('ignores highway with connected entrance vertex', function() { var n1 = iD.osmNode({id: 'n-1', loc: [4,4], tags: {'entrance': 'yes'}});