From f8bbb995ac3ddcfe55eacadc453a5ace52e10a1b Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Sat, 10 Mar 2018 15:20:20 -0500 Subject: [PATCH] Properly split ways which are members of a via way turn restriction (closes #4861) --- modules/actions/split.js | 61 +++++++++++++-- test/spec/actions/split.js | 153 ++++++++++++++++++++++++++++++++++--- 2 files changed, 197 insertions(+), 17 deletions(-) diff --git a/modules/actions/split.js b/modules/actions/split.js index 982632484..54c41f380 100644 --- a/modules/actions/split.js +++ b/modules/actions/split.js @@ -1,5 +1,6 @@ import _extend from 'lodash-es/extend'; import _indexOf from 'lodash-es/indexOf'; +import _intersection from 'lodash-es/intersection'; import _some from 'lodash-es/some'; import { actionAddMember } from './add_member'; @@ -31,7 +32,7 @@ import { utilWrap } from '../util'; export function actionSplit(nodeId, newWayIds) { var _wayIDs; - // if the way is closed, we need to search for a partner node + // If the way is closed, we need to search for a partner node // to split the way at. // // The following looks for a node that is both far away from @@ -87,7 +88,7 @@ export function actionSplit(nodeId, newWayIds) { function split(graph, wayA, newWayId) { - var wayB = osmWay({id: newWayId, tags: wayA.tags}); + var wayB = osmWay({id: newWayId, tags: wayA.tags}); // `wayB` is the NEW way var origNodes = wayA.nodes.slice(); var nodesA; var nodesB; @@ -119,12 +120,58 @@ export function actionSplit(nodeId, newWayIds) { graph = graph.replace(wayB); graph.parentRelations(wayA).forEach(function(relation) { + var member; + + // Turn restrictions - make sure: + // 1. Splitting a FROM/TO way - only `wayA` OR `wayB` remains in relation + // (whichever one is connected to the VIA node/ways) + // 2. Splitting a VIA way - `wayB` remains in relation as a VIA way if (relation.isRestriction()) { - var via = relation.memberByRole('via'); - if (via && wayB.contains(via.id)) { - relation = relation.replaceMember(wayA, wayB); - graph = graph.replace(relation); + var f = relation.memberByRole('from'); + var v = relation.membersByRole('via'); + var t = relation.memberByRole('to'); + var i; + + // 1. split a FROM/TO + if (f.id === wayA.id || t.id === wayA.id) { + var keepB = false; + if (v.length === 1 && v[0].type === 'node') { // check via node + keepB = wayB.contains(v[0].id); + } else { // check via way(s) + for (i = 0; i < v.length; i++) { + if (v[i].type === 'way') { + var wayVia = graph.hasEntity(v[i].id); + if (wayVia && _intersection(wayB.nodes, wayVia.nodes).length) { + keepB = true; + break; + } + } + } + } + + if (keepB) { + relation = relation.replaceMember(wayA, wayB); + graph = graph.replace(relation); + } + + // 2. split a VIA + } else { + for (i = 0; i < v.length; i++) { + if (v[i].type === 'way' && v[i].id === wayA.id) { + member = { + id: wayB.id, + type: 'way', + role: 'via' + }; + graph = actionAddMember(relation.id, member, v[i].index + 1)(graph); + break; + } + } } + + // All other relations (Routes, Multipolygons, etc): + // 1. Both `wayA` and `wayB` remain in the relation + // 2. But must be inserted as a pair (see `actionAddMember` for details) } else { if (relation === isOuter) { graph = graph.replace(relation.mergeTags(wayA.tags)); @@ -132,7 +179,7 @@ export function actionSplit(nodeId, newWayIds) { graph = graph.replace(wayB.update({tags: {}})); } - var member = { + member = { id: wayB.id, type: 'way', role: relation.memberById(wayA.id).role diff --git a/test/spec/actions/split.js b/test/spec/actions/split.js index 8e536a754..833b3542f 100644 --- a/test/spec/actions/split.js +++ b/test/spec/actions/split.js @@ -903,16 +903,16 @@ describe('iD.actionSplit', function () { ['restriction', 'restriction:bus'].forEach(function (type) { describe('type = ' + type, function () { - it('updates a restriction\'s \'from\' role', function () { + it('updates a restriction\'s \'from\' role - via node', function () { // Situation: // a ----> b ----> c ~~~~ d - // A restriction from ---- to ~~~~ via c. + // A restriction from ---- to ~~~~ via node c. // // Split at b. // // Expected result: // a ----> b ====> c ~~~~ d - // A restriction from ==== to ~~~~ via c. + // A restriction from ==== to ~~~~ via node c. // var graph = iD.coreGraph([ iD.osmNode({id: 'a'}), @@ -937,16 +937,16 @@ describe('iD.actionSplit', function () { ]); }); - it('updates a restriction\'s \'to\' role', function () { + it('updates a restriction\'s \'to\' role - via node', function () { // Situation: // a ----> b ----> c ~~~~ d - // A restriction from ~~~~ to ---- via c. + // A restriction from ~~~~ to ---- via node c. // // Split at b. // // Expected result: // a ----> b ====> c ~~~~ d - // A restriction from ~~~~ to ==== via c. + // A restriction from ~~~~ to ==== via node c. // var graph = iD.coreGraph([ iD.osmNode({id: 'a'}), @@ -971,17 +971,16 @@ describe('iD.actionSplit', function () { ]); }); - - it('updates both \'to\' and \'from\' roles for u-turn restrictions', function () { + it('updates both \'to\' and \'from\' roles for via-node u-turn restrictions', function () { // Situation: // a ----> b ----> c ~~~~ d - // A restriction from ---- to ---- via c. + // A restriction from ---- to ---- via node c. // // Split at b. // // Expected result: // a ----> b ====> c ~~~~ d - // A restriction from ==== to ==== via c. + // A restriction from ==== to ==== via node c. // var graph = iD.coreGraph([ iD.osmNode({id: 'a'}), @@ -1006,6 +1005,140 @@ describe('iD.actionSplit', function () { ]); }); + it('updates a restriction\'s \'from\' role - via way', function () { + // Situation: + // e <~~~~ d + // | + // | + // a ----> b ----> c + // + // A restriction from ---- to ~~~~ via way | + // + // Split at b. + // + // Expected result: + // e <~~~~ d + // | + // | + // a ----> b ====> c + // + // A restriction from ==== to ~~~~ via way | + // + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmNode({id: 'd'}), + iD.osmNode({id: 'e'}), + iD.osmWay({id: '-', nodes: ['a', 'b', 'c']}), + iD.osmWay({id: '|', nodes: ['c', 'd']}), + iD.osmWay({id: '~', nodes: ['d', 'e']}), + iD.osmRelation({id: 'r', tags: {type: type}, members: [ + {id: '-', role: 'from', type: 'way'}, + {id: '~', role: 'to', type: 'way'}, + {id: '|', role: 'via', type: 'way'} + ]}) + ]); + + graph = iD.actionSplit('b', ['='])(graph); + + expect(graph.entity('r').members).to.eql([ + {id: '=', role: 'from', type: 'way'}, + {id: '~', role: 'to', type: 'way'}, + {id: '|', role: 'via', type: 'way'} + ]); + }); + + it('updates a restriction\'s \'to\' role - via way', function () { + // Situation: + // e <~~~~ d + // | + // | + // a ----> b ----> c + // + // A restriction from ~~~~ to ---- via way | + // + // Split at b. + // + // Expected result: + // e <~~~~ d + // | + // | + // a ----> b ====> c + // + // A restriction from ~~~~ to ==== via way | + // + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmNode({id: 'd'}), + iD.osmNode({id: 'e'}), + iD.osmWay({id: '-', nodes: ['a', 'b', 'c']}), + iD.osmWay({id: '|', nodes: ['c', 'd']}), + iD.osmWay({id: '~', nodes: ['d', 'e']}), + iD.osmRelation({id: 'r', tags: {type: type}, members: [ + {id: '~', role: 'from', type: 'way'}, + {id: '-', role: 'to', type: 'way'}, + {id: '|', role: 'via', type: 'way'} + ]}) + ]); + + graph = iD.actionSplit('b', ['='])(graph); + + expect(graph.entity('r').members).to.eql([ + {id: '~', role: 'from', type: 'way'}, + {id: '=', role: 'to', type: 'way'}, + {id: '|', role: 'via', type: 'way'} + ]); + }); + + + it('updates a restriction\'s \'via\' role when splitting via way', function () { + // Situation: + // d e + // | ‖ + // | ‖ + // a ----> b ----> c + // + // A restriction from | to ‖ via way ---- + // + // Split at b. + // + // Expected result: + // d e + // | ‖ + // | ‖ + // a ----> b ====> c + // + // A restriction from | to ‖ via ways ----, ==== + // + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmNode({id: 'd'}), + iD.osmNode({id: 'e'}), + iD.osmWay({id: '-', nodes: ['a', 'b', 'c']}), + iD.osmWay({id: '|', nodes: ['d', 'a']}), + iD.osmWay({id: '‖', nodes: ['e', 'c']}), + iD.osmRelation({id: 'r', tags: {type: type}, members: [ + {id: '|', role: 'from', type: 'way'}, + {id: '-', role: 'via', type: 'way'}, + {id: '‖', role: 'to', type: 'way'} + ]}) + ]); + + graph = iD.actionSplit('b', ['='])(graph); + + expect(graph.entity('r').members).to.eql([ + {id: '|', role: 'from', type: 'way'}, + {id: '-', role: 'via', type: 'way'}, + {id: '=', role: 'via', type: 'way'}, + {id: '‖', role: 'to', type: 'way'} + ]); + }); + it('leaves unaffected restrictions unchanged', function () { // Situation: // a <---- b <---- c ~~~~ d