diff --git a/js/id/actions/restrict_turn.js b/js/id/actions/restrict_turn.js index c2430908a..67236c964 100644 --- a/js/id/actions/restrict_turn.js +++ b/js/id/actions/restrict_turn.js @@ -28,6 +28,10 @@ iD.actions.RestrictTurn = function(turn, projection, restrictionId) { via = graph.entity(turn.via.node), to = graph.entity(turn.to.way); + function isClosingNode(way, nodeId) { + return nodeId === way.first() && nodeId === way.last(); + } + function split(toOrFrom) { var newID = toOrFrom.newID || iD.Way().id; graph = iD.actions.Split(via.id, [newID]) @@ -43,12 +47,12 @@ iD.actions.RestrictTurn = function(turn, projection, restrictionId) { } } - if (!from.affix(via.id)) { + if (!from.affix(via.id) || isClosingNode(from, via.id)) { if (turn.from.node === turn.to.node) { // U-turn from = to = split(turn.from)[0]; } else if (turn.from.way === turn.to.way) { - // Straight-on + // Straight-on or circular var s = split(turn.from); from = s[0]; to = s[1]; @@ -58,7 +62,7 @@ iD.actions.RestrictTurn = function(turn, projection, restrictionId) { } } - if (!to.affix(via.id)) { + if (!to.affix(via.id) || isClosingNode(to, via.id)) { to = split(turn.to)[0]; } diff --git a/js/id/geo/intersection.js b/js/id/geo/intersection.js index 05f70ef01..b189f46d1 100644 --- a/js/id/geo/intersection.js +++ b/js/id/geo/intersection.js @@ -6,43 +6,82 @@ iD.geo.Turn = function(turn) { iD.geo.Intersection = function(graph, vertexId) { var vertex = graph.entity(vertexId), - highways = []; + parentWays = graph.parentWays(vertex), + coincident = [], + highways = {}; + + function addHighway(way, adjacentNodeId) { + if (highways[adjacentNodeId]) { + coincident.push(adjacentNodeId); + } else { + highways[adjacentNodeId] = way; + } + } // Pre-split ways that would need to be split in // order to add a restriction. The real split will // happen when the restriction is added. - graph.parentWays(vertex).forEach(function(way) { + parentWays.forEach(function(way) { if (!way.tags.highway || way.isArea() || way.isDegenerate()) return; - if (way.affix(vertexId)) { - highways.push(way); + var isFirst = (vertexId === way.first()), + isLast = (vertexId === way.last()), + isAffix = (isFirst || isLast), + isClosingNode = (isFirst && isLast); + + if (isAffix && !isClosingNode) { + var index = (isFirst ? 1 : way.nodes.length - 2); + addHighway(way, way.nodes[index]); + } else { - var idx = _.indexOf(way.nodes, vertex.id, 1), - wayA = iD.Way({id: way.id + '-a', tags: way.tags, nodes: way.nodes.slice(0, idx + 1)}), - wayB = iD.Way({id: way.id + '-b', tags: way.tags, nodes: way.nodes.slice(idx)}); - - graph = graph.replace(wayA); - graph = graph.replace(wayB); - - highways.push(wayA); - highways.push(wayB); + var splitIndex, wayA, wayB, indexA, indexB; + if (isClosingNode) { + splitIndex = Math.ceil(way.nodes.length / 2); // split at midpoint + wayA = iD.Way({id: way.id + '-a', tags: way.tags, nodes: way.nodes.slice(0, splitIndex)}); + wayB = iD.Way({id: way.id + '-b', tags: way.tags, nodes: way.nodes.slice(splitIndex)}); + indexA = 1; + indexB = way.nodes.length - 2; + } else { + splitIndex = _.indexOf(way.nodes, vertex.id, 1); // split at vertexid + wayA = iD.Way({id: way.id + '-a', tags: way.tags, nodes: way.nodes.slice(0, splitIndex + 1)}); + wayB = iD.Way({id: way.id + '-b', tags: way.tags, nodes: way.nodes.slice(splitIndex)}); + indexA = splitIndex - 1; + indexB = splitIndex + 1; + } + graph = graph.replace(wayA).replace(wayB); + addHighway(wayA, way.nodes[indexA]); + addHighway(wayB, way.nodes[indexB]); } }); + // remove any ways from this intersection that are coincident + // (i.e. any adjacent node used by more than one intersecting way) + coincident.forEach(function (n) { + delete highways[n]; + }); + + var intersection = { highways: highways, + ways: _.values(highways), graph: graph }; - intersection.turns = function(fromNodeID) { - if (!fromNodeID) + intersection.adjacentNodeId = function(fromWayId) { + return _.find(_.keys(highways), function(k) { + return highways[k].id === fromWayId; + }); + }; + + intersection.turns = function(fromNodeId) { + var start = highways[fromNodeId]; + if (!start) return []; - var way = _.find(highways, function(way) { return way.contains(fromNodeID); }); - if (way.first() === vertex.id && way.tags.oneway === 'yes') + if (start.first() === vertex.id && start.tags.oneway === 'yes') return []; - if (way.last() === vertex.id && way.tags.oneway === '-1') + if (start.last() === vertex.id && start.tags.oneway === '-1') return []; function withRestriction(turn) { @@ -71,39 +110,44 @@ iD.geo.Intersection = function(graph, vertexId) { } var from = { - node: way.nodes[way.first() === vertex.id ? 1 : way.nodes.length - 2], - way: way.id.split(/-(a|b)/)[0] + node: fromNodeId, + way: start.id.split(/-(a|b)/)[0] }, - via = {node: vertex.id}, + via = { node: vertex.id }, turns = []; - highways.forEach(function(parent) { - if (parent === way) + _.each(highways, function(end, adjacentNodeId) { + if (end === start) return; - var index = parent.nodes.indexOf(vertex.id); - // backward - if (parent.first() !== vertex.id && parent.tags.oneway !== 'yes') { + if (end.first() !== vertex.id && end.tags.oneway !== 'yes') { turns.push(withRestriction({ from: from, via: via, - to: {node: parent.nodes[index - 1], way: parent.id.split(/-(a|b)/)[0]} + to: { + node: adjacentNodeId, + way: end.id.split(/-(a|b)/)[0] + } })); } // forward - if (parent.last() !== vertex.id && parent.tags.oneway !== '-1') { + if (end.last() !== vertex.id && end.tags.oneway !== '-1') { turns.push(withRestriction({ from: from, via: via, - to: {node: parent.nodes[index + 1], way: parent.id.split(/-(a|b)/)[0]} + to: { + node: adjacentNodeId, + way: end.id.split(/-(a|b)/)[0] + } })); } + }); // U-turn - if (way.tags.oneway !== 'yes' && way.tags.oneway !== '-1') { + if (start.tags.oneway !== 'yes' && start.tags.oneway !== '-1') { turns.push(withRestriction({ from: from, via: via, diff --git a/js/id/ui/preset/restrictions.js b/js/id/ui/preset/restrictions.js index 7f9824958..9a8572a89 100644 --- a/js/id/ui/preset/restrictions.js +++ b/js/id/ui/preset/restrictions.js @@ -43,7 +43,7 @@ iD.ui.preset.restrictions = function(field, context) { surface .call(vertices, graph, [vertex], filter, extent, z) - .call(lines, graph, intersection.highways, filter) + .call(lines, graph, intersection.ways, filter) .call(turns, graph, intersection.turns(fromNodeID)); surface @@ -57,7 +57,7 @@ iD.ui.preset.restrictions = function(field, context) { if (fromNodeID) { surface - .selectAll('.' + _.find(intersection.highways, function(way) { return way.contains(fromNodeID); }).id) + .selectAll('.' + intersection.highways[fromNodeID].id) .classed('selected', true); } @@ -72,7 +72,7 @@ iD.ui.preset.restrictions = function(field, context) { function click() { var datum = d3.event.target.__data__; if (datum instanceof iD.Entity) { - fromNodeID = datum.nodes[(datum.first() === vertexID) ? 1 : datum.nodes.length - 2]; + fromNodeID = intersection.adjacentNodeId(datum.id); render(); } else if (datum instanceof iD.geo.Turn) { if (datum.restriction) { diff --git a/test/spec/actions/restrict_turn.js b/test/spec/actions/restrict_turn.js index d093c3bd4..d1dae1a30 100644 --- a/test/spec/actions/restrict_turn.js +++ b/test/spec/actions/restrict_turn.js @@ -138,6 +138,68 @@ describe("iD.actions.RestrictTurn", function() { expect(_.pick(r.memberByRole('to'), 'id', 'type')).to.eql({id: '=', type: 'way'}); }); + it('splits the from way when necessary (vertex closes from)', function() { + // + // b -- c + // | | + // a -- * === w + // + var graph = iD.Graph([ + iD.Node({id: 'a', loc: [-1, 0]}), + iD.Node({id: 'b', loc: [-1, 1]}), + iD.Node({id: 'c', loc: [ 0, 1]}), + iD.Node({id: '*', loc: [ 0, 0]}), + iD.Node({id: 'w', loc: [ 1, 0]}), + iD.Way({id: '-', nodes: ['*', 'a', 'b', 'c', '*']}), + iD.Way({id: '=', nodes: ['*', 'w']}) + ]), + action = iD.actions.RestrictTurn({ + from: {node: 'c', way: '-', newID: '--'}, + via: {node: '*'}, + to: {node: 'w', way: '='}, + restriction: 'no_left_turn' + }, projection, 'r'); + + graph = action(graph); + + var r = graph.entity('r'); + expect(r.tags).to.eql({type: 'restriction', restriction: 'no_left_turn'}); + expect(_.pick(r.memberByRole('from'), 'id', 'type')).to.eql({id: '--', type: 'way'}); + expect(_.pick(r.memberByRole('via'), 'id', 'type')).to.eql({id: '*', type: 'node'}); + expect(_.pick(r.memberByRole('to'), 'id', 'type')).to.eql({id: '=', type: 'way'}); + }); + + it('splits the from/to way when necessary (vertex closes from/to)', function() { + // + // b -- c + // | | + // a -- * === w + // + var graph = iD.Graph([ + iD.Node({id: 'a', loc: [-1, 0]}), + iD.Node({id: 'b', loc: [-1, 1]}), + iD.Node({id: 'c', loc: [ 0, 1]}), + iD.Node({id: '*', loc: [ 0, 0]}), + iD.Node({id: 'w', loc: [ 1, 0]}), + iD.Way({id: '-', nodes: ['*', 'a', 'b', 'c', '*']}), + iD.Way({id: '=', nodes: ['*', 'w']}) + ]), + action = iD.actions.RestrictTurn({ + from: {node: 'a', way: '-', newID: '--'}, + via: {node: '*'}, + to: {node: 'c', way: '-'}, + restriction: 'no_left_turn' + }, projection, 'r'); + + graph = action(graph); + + var r = graph.entity('r'); + expect(r.tags).to.eql({type: 'restriction', restriction: 'no_left_turn'}); + expect(_.pick(r.memberByRole('from'), 'id', 'type')).to.eql({id: '-', type: 'way'}); + expect(_.pick(r.memberByRole('via'), 'id', 'type')).to.eql({id: '*', type: 'node'}); + expect(_.pick(r.memberByRole('to'), 'id', 'type')).to.eql({id: '--', type: 'way'}); + }); + it('splits the to way when necessary (forward)', function() { // u====*===>w // | @@ -194,6 +256,37 @@ describe("iD.actions.RestrictTurn", function() { expect(_.pick(r.memberByRole('to'), 'id', 'type')).to.eql({id: '=', type: 'way'}); }); + it('splits the to way when necessary (vertex closes to)', function() { + // + // b -- c + // | | + // a -- * === w + // + var graph = iD.Graph([ + iD.Node({id: 'a', loc: [-1, 0]}), + iD.Node({id: 'b', loc: [-1, 1]}), + iD.Node({id: 'c', loc: [ 0, 1]}), + iD.Node({id: '*', loc: [ 0, 0]}), + iD.Node({id: 'w', loc: [ 1, 0]}), + iD.Way({id: '-', nodes: ['*', 'a', 'b', 'c', '*']}), + iD.Way({id: '=', nodes: ['*', 'w']}) + ]), + action = iD.actions.RestrictTurn({ + from: {node: 'w', way: '='}, + via: {node: '*'}, + to: {node: 'c', way: '-', newID: '--'}, + restriction: 'no_right_turn' + }, projection, 'r'); + + graph = action(graph); + + var r = graph.entity('r'); + expect(r.tags).to.eql({type: 'restriction', restriction: 'no_right_turn'}); + expect(_.pick(r.memberByRole('from'), 'id', 'type')).to.eql({id: '=', type: 'way'}); + expect(_.pick(r.memberByRole('via'), 'id', 'type')).to.eql({id: '*', type: 'node'}); + expect(_.pick(r.memberByRole('to'), 'id', 'type')).to.eql({id: '--', type: 'way'}); + }); + it('splits the from/to way of a U-turn (forward)', function() { // u====*===>w // | diff --git a/test/spec/geo/intersection.js b/test/spec/geo/intersection.js index bcf083542..b747a5720 100644 --- a/test/spec/geo/intersection.js +++ b/test/spec/geo/intersection.js @@ -8,7 +8,7 @@ describe("iD.geo.Intersection", function() { iD.Way({id: '=', nodes: ['u', '*']}), iD.Way({id: '-', nodes: ['*', 'w']}) ]); - expect(iD.geo.Intersection(graph, '*').highways).to.eql([]); + expect(iD.geo.Intersection(graph, '*').ways).to.eql([]); }); it("excludes degenerate highways", function() { @@ -18,7 +18,17 @@ describe("iD.geo.Intersection", function() { iD.Way({id: '=', nodes: ['u', '*'], tags: {highway: 'residential'}}), iD.Way({id: '-', nodes: ['*'], tags: {highway: 'residential'}}) ]); - expect(_.pluck(iD.geo.Intersection(graph, '*').highways, 'id')).to.eql(['=']); + expect(_.pluck(iD.geo.Intersection(graph, '*').ways, 'id')).to.eql(['=']); + }); + + it("excludes coincident highways", function() { + var graph = iD.Graph([ + iD.Node({id: 'u'}), + iD.Node({id: '*'}), + iD.Way({id: '=', nodes: ['u', '*'], tags: {highway: 'residential'}}), + iD.Way({id: '-', nodes: ['u', '*'], tags: {highway: 'residential'}}) + ]); + expect(iD.geo.Intersection(graph, '*').ways).to.eql([]); }); it('includes line highways', function() { @@ -29,7 +39,7 @@ describe("iD.geo.Intersection", function() { iD.Way({id: '=', nodes: ['u', '*'], tags: {highway: 'residential'}}), iD.Way({id: '-', nodes: ['*', 'w']}) ]); - expect(_.pluck(iD.geo.Intersection(graph, '*').highways, 'id')).to.eql(['=']); + expect(_.pluck(iD.geo.Intersection(graph, '*').ways, 'id')).to.eql(['=']); }); it('excludes area highways', function() { @@ -39,7 +49,7 @@ describe("iD.geo.Intersection", function() { iD.Node({id: 'w'}), iD.Way({id: '=', nodes: ['u', '*', 'w'], tags: {highway: 'pedestrian', area: 'yes'}}) ]); - expect(iD.geo.Intersection(graph, '*').highways).to.eql([]); + expect(iD.geo.Intersection(graph, '*').ways).to.eql([]); }); it('auto-splits highways at the intersection', function() { @@ -49,7 +59,7 @@ describe("iD.geo.Intersection", function() { iD.Node({id: 'w'}), iD.Way({id: '=', nodes: ['u', '*', 'w'], tags: {highway: 'residential'}}) ]); - expect(_.pluck(iD.geo.Intersection(graph, '*').highways, 'id')).to.eql(['=-a', '=-b']); + expect(_.pluck(iD.geo.Intersection(graph, '*').ways, 'id')).to.eql(['=-a', '=-b']); }); }); @@ -372,5 +382,200 @@ describe("iD.geo.Intersection", function() { u: true }); }); + + it("permits turns to a circular way", function() { + // + // b -- c + // | | + // a -- * === u + // + var graph = iD.Graph([ + iD.Node({id: 'a'}), + iD.Node({id: 'b'}), + iD.Node({id: 'c'}), + iD.Node({id: '*'}), + iD.Node({id: 'u'}), + iD.Way({id: '-', nodes: ['*', 'a', 'b', 'c', '*'], tags: {highway: 'residential'}}), + iD.Way({id: '=', nodes: ['*', 'u'], tags: {highway: 'residential'}}) + ]), + turns = iD.geo.Intersection(graph, '*').turns('u'); + + expect(turns.length).to.eql(3); + expect(turns[0]).to.eql({ + from: {node: 'u', way: '='}, + via: {node: '*'}, + to: {node: 'a', way: '-'} + }); + expect(turns[1]).to.eql({ + from: {node: 'u', way: '='}, + via: {node: '*'}, + to: {node: 'c', way: '-'} + }); + expect(turns[2]).to.eql({ + from: {node: 'u', way: '='}, + via: {node: '*'}, + to: {node: 'u', way: '='}, + u: true + }); + }); + + it("permits turns from a circular way", function() { + // + // b -- c + // | | + // a -- * === u + // + var graph = iD.Graph([ + iD.Node({id: 'a'}), + iD.Node({id: 'b'}), + iD.Node({id: 'c'}), + iD.Node({id: '*'}), + iD.Node({id: 'u'}), + iD.Way({id: '-', nodes: ['*', 'a', 'b', 'c', '*'], tags: {highway: 'residential'}}), + iD.Way({id: '=', nodes: ['*', 'u'], tags: {highway: 'residential'}}) + ]), + turns = iD.geo.Intersection(graph, '*').turns('a'); + + expect(turns.length).to.eql(3); + expect(turns[0]).to.eql({ + from: {node: 'a', way: '-'}, + via: {node: '*'}, + to: {node: 'c', way: '-'} + }); + expect(turns[1]).to.eql({ + from: {node: 'a', way: '-'}, + via: {node: '*'}, + to: {node: 'u', way: '='} + }); + expect(turns[2]).to.eql({ + from: {node: 'a', way: '-'}, + via: {node: '*'}, + to: {node: 'a', way: '-'}, + u: true + }); + }); + + it("permits turns to a oneway circular way", function() { + // + // b -- c + // | | + // a -- * === u + // + var graph = iD.Graph([ + iD.Node({id: 'a'}), + iD.Node({id: 'b'}), + iD.Node({id: 'c'}), + iD.Node({id: '*'}), + iD.Node({id: 'u'}), + iD.Way({id: '-', nodes: ['*', 'a', 'b', 'c', '*'], tags: {highway: 'residential', oneway: 'yes'}}), + iD.Way({id: '=', nodes: ['*', 'u'], tags: {highway: 'residential'}}) + ]), + turns = iD.geo.Intersection(graph, '*').turns('u'); + + expect(turns.length).to.eql(2); + expect(turns[0]).to.eql({ + from: {node: 'u', way: '='}, + via: {node: '*'}, + to: {node: 'a', way: '-'} + }); + expect(turns[1]).to.eql({ + from: {node: 'u', way: '='}, + via: {node: '*'}, + to: {node: 'u', way: '='}, + u: true + }); + }); + + it("permits turns to a reverse oneway circular way", function() { + // + // b -- c + // | | + // a -- * === u + // + var graph = iD.Graph([ + iD.Node({id: 'a'}), + iD.Node({id: 'b'}), + iD.Node({id: 'c'}), + iD.Node({id: '*'}), + iD.Node({id: 'u'}), + iD.Way({id: '-', nodes: ['*', 'a', 'b', 'c', '*'], tags: {highway: 'residential', oneway: '-1'}}), + iD.Way({id: '=', nodes: ['*', 'u'], tags: {highway: 'residential'}}) + ]), + turns = iD.geo.Intersection(graph, '*').turns('u'); + + expect(turns.length).to.eql(2); + expect(turns[0]).to.eql({ + from: {node: 'u', way: '='}, + via: {node: '*'}, + to: {node: 'c', way: '-'} + }); + expect(turns[1]).to.eql({ + from: {node: 'u', way: '='}, + via: {node: '*'}, + to: {node: 'u', way: '='}, + u: true + }); + }); + + it("permits turns from a oneway circular way", function() { + // + // b -- c + // | | + // a -- * === u + // + var graph = iD.Graph([ + iD.Node({id: 'a'}), + iD.Node({id: 'b'}), + iD.Node({id: 'c'}), + iD.Node({id: '*'}), + iD.Node({id: 'u'}), + iD.Way({id: '-', nodes: ['*', 'a', 'b', 'c', '*'], tags: {highway: 'residential', oneway: 'yes'}}), + iD.Way({id: '=', nodes: ['*', 'u'], tags: {highway: 'residential'}}) + ]), + turns = iD.geo.Intersection(graph, '*').turns('c'); + + expect(turns.length).to.eql(2); + expect(turns[0]).to.eql({ + from: {node: 'c', way: '-'}, + via: {node: '*'}, + to: {node: 'a', way: '-'} + }); + expect(turns[1]).to.eql({ + from: {node: 'c', way: '-'}, + via: {node: '*'}, + to: {node: 'u', way: '='} + }); + }); + + it("permits turns from a reverse oneway circular way", function() { + // + // b -- c + // | | + // a -- * === u + // + var graph = iD.Graph([ + iD.Node({id: 'a'}), + iD.Node({id: 'b'}), + iD.Node({id: 'c'}), + iD.Node({id: '*'}), + iD.Node({id: 'u'}), + iD.Way({id: '-', nodes: ['*', 'a', 'b', 'c', '*'], tags: {highway: 'residential', oneway: '-1'}}), + iD.Way({id: '=', nodes: ['*', 'u'], tags: {highway: 'residential'}}) + ]), + turns = iD.geo.Intersection(graph, '*').turns('a'); + + expect(turns.length).to.eql(2); + expect(turns[0]).to.eql({ + from: {node: 'a', way: '-'}, + via: {node: '*'}, + to: {node: 'c', way: '-'} + }); + expect(turns[1]).to.eql({ + from: {node: 'a', way: '-'}, + via: {node: '*'}, + to: {node: 'u', way: '='} + }); + }); + }); });