Stricter checks to prevent turn restriction breakage when dragging

(re: #4921)
This commit is contained in:
Bryan Housel
2018-04-13 22:50:51 -04:00
parent 8fb083578f
commit 87841fc403
4 changed files with 356 additions and 72 deletions
+2 -1
View File
@@ -116,6 +116,7 @@ en:
line: Connected a way to a line.
area: Connected a way to an area.
relation: These features can't be connected because they have conflicting relation roles.
restriction: "These features can't be connected because it would damage a \"{relation}\" relation."
disconnect:
title: Disconnect
description: Disconnect these lines/areas from each other.
@@ -131,7 +132,7 @@ en:
annotation: "Merged {n} features."
not_eligible: These features can't be merged.
not_adjacent: These features can't be merged because their endpoints aren't connected.
restriction: These features can't be merged because at least one is a member of a "{relation}" relation.
restriction: "These features can't be merged because at least one is a member of a \"{relation}\" relation."
incomplete_relation: These features can't be merged because at least one hasn't been fully downloaded.
conflicting_tags: These features can't be merged because some of their tags have conflicting values.
move:
+2 -1
View File
@@ -151,7 +151,8 @@
"line": "Connected a way to a line.",
"area": "Connected a way to an area."
},
"relation": "These features can't be connected because they have conflicting relation roles."
"relation": "These features can't be connected because they have conflicting relation roles.",
"restriction": "These features can't be connected because it would damage a \"{relation}\" relation."
},
"disconnect": {
"title": "Disconnect",
+167 -18
View File
@@ -1,3 +1,5 @@
import _uniq from 'lodash-es/uniq';
import { actionDeleteNode } from './delete_node';
@@ -15,7 +17,7 @@ import { actionDeleteNode } from './delete_node';
// https://github.com/openstreetmap/potlatch2/blob/master/net/systemeD/halcyon/connection/actions/MergeNodesAction.as
// https://github.com/openstreetmap/josm/blob/mirror/src/org/openstreetmap/josm/actions/MergeNodesAction.java
//
export function actionConnect(nodeIds) {
export function actionConnect(nodeIDs) {
var action = function(graph) {
var survivor;
var node;
@@ -23,14 +25,14 @@ export function actionConnect(nodeIds) {
var i, j;
// Choose a survivor node, prefer an existing (not new) node - #4974
for (i = 0; i < nodeIds.length; i++) {
survivor = graph.entity(nodeIds[i]);
for (i = 0; i < nodeIDs.length; i++) {
survivor = graph.entity(nodeIDs[i]);
if (survivor.version) break; // found one
}
// Replace all non-surviving nodes with the survivor and merge tags.
for (i = 0; i < nodeIds.length; i++) {
node = graph.entity(nodeIds[i]);
for (i = 0; i < nodeIDs.length; i++) {
node = graph.entity(nodeIDs[i]);
if (node.id === survivor.id) continue;
parents = graph.parentWays(node);
@@ -57,27 +59,174 @@ export function actionConnect(nodeIds) {
action.disabled = function(graph) {
var seen = {};
var restrictionIDs = [];
var survivor;
var node;
var relations, relation, role;
var i, j, k;
for (var i = 0; i < nodeIds.length; i++) {
var node = graph.entity(nodeIds[i]);
// Choose a survivor node, prefer an existing (not new) node - #4974
for (i = 0; i < nodeIDs.length; i++) {
survivor = graph.entity(nodeIDs[i]);
if (survivor.version) break; // found one
}
var toCheck = [node].concat(graph.parentWays(node));
for (var j = 0; j < toCheck.length; j++) {
var entity = toCheck[j];
// 1. disable if the nodes being connected have conflicting relation roles
for (i = 0; i < nodeIDs.length; i++) {
node = graph.entity(nodeIDs[i]);
relations = graph.parentRelations(node);
var relations = graph.parentRelations(entity);
for (var k = 0; k < relations.length; k++) {
var relation = relations[k];
var role = relation.memberById(entity.id).role || '';
for (j = 0; j < relations.length; j++) {
relation = relations[j];
role = relation.memberById(node.id).role || '';
if (seen[relation.id] !== undefined && seen[relation.id] !== role) {
return 'relation';
} else {
seen[relation.id] = role;
// if this node is a via node in a restriction, remember for later
if (relation.isRestriction()) {
restrictionIDs.push(relation.id);
}
if (seen[relation.id] !== undefined && seen[relation.id] !== role) {
return 'relation';
} else {
seen[relation.id] = role;
}
}
}
// gather restrictions for parent ways
for (i = 0; i < nodeIDs.length; i++) {
node = graph.entity(nodeIDs[i]);
var parents = graph.parentWays(node);
for (j = 0; j < parents.length; j++) {
var parent = parents[j];
relations = graph.parentRelations(parent);
for (k = 0; k < relations.length; k++) {
relation = relations[k];
if (relation.isRestriction()) {
restrictionIDs.push(relation.id);
}
}
}
}
// test restrictions
restrictionIDs = _uniq(restrictionIDs);
for (i = 0; i < restrictionIDs.length; i++) {
relation = graph.entity(restrictionIDs[i]);
var memberWays = [];
for (j = 0; j < relation.members.length; j++) {
var m = relation.members[j];
var entity = graph.hasEntity(m.id);
if (m.type === 'way') {
memberWays.push(entity);
}
}
// 2a. disable if connection would damage a restriction
// (a key node is a node at the junction of ways)
var nodes = { from: [], via: [], to: [], keyfrom: [], keyto: [] };
for (j = 0; j < relation.members.length; j++) {
collectNodes(relation.members[j], nodes);
}
nodes.keyfrom = _uniq(nodes.keyfrom.filter(hasDuplicates));
nodes.keyto = _uniq(nodes.keyto.filter(hasDuplicates));
var f = keyNodeFilter(nodes.keyfrom, nodes.keyto);
nodes.from = nodes.from.filter(f);
nodes.via = nodes.via.filter(f);
nodes.to = nodes.to.filter(f);
var connectFrom = false;
var connectVia = false;
var connectTo = false;
var connectKeyFrom = false;
var connectKeyTo = false;
for (j = 0; j < nodeIDs.length; j++) {
var n = nodeIDs[j];
if (nodes.from.indexOf(n) !== -1) { connectFrom = true; }
if (nodes.via.indexOf(n) !== -1) { connectVia = true; }
if (nodes.to.indexOf(n) !== -1) { connectTo = true; }
if (nodes.keyfrom.indexOf(n) !== -1) { connectKeyFrom = true; }
if (nodes.keyto.indexOf(n) !== -1) { connectKeyTo = true; }
}
if (connectFrom && connectTo) { return 'restriction'; }
if (connectFrom && connectVia) { return 'restriction'; }
if (connectTo && connectVia) { return 'restriction'; }
// connecting to a key node - must be an adjacent node
if (connectKeyFrom || connectKeyTo) {
if (nodeIDs.length !== 2) { return 'restriction'; }
var ok = false;
for (j = 0; j < memberWays.length; j++) {
ok = memberWays[j].areAdjacent(nodeIDs[0], nodeIDs[1]);
if (ok) break;
}
if (!ok) { return 'restriction'; }
}
// 2b. disable if nodes being connected will destroy a member way in a restriction
// (to test, make a copy and try removing non surviving nodes)
for (j = 0; j < memberWays.length; j++) {
var way = memberWays[j].update({});
for (k = 0; k < nodeIDs.length; k++) {
if (nodeIDs[k] !== survivor.id) {
way = way.removeNode(nodeIDs[k]);
}
}
if (way.isDegenerate()) {
return 'restriction';
}
}
}
return false;
// if a key node appears multiple times (indexOf !== lastIndexOf) it's a FROM-VIA or TO-VIA junction
function hasDuplicates(n, i, arr) {
return arr.indexOf(n) !== arr.lastIndexOf(n);
}
function keyNodeFilter(froms, tos) {
return function(n) {
return froms.indexOf(n) === -1 && tos.indexOf(n) === -1;
};
}
function collectNodes(member, collection) {
var entity = graph.hasEntity(member.id);
if (!entity) return;
var role = member.role || '';
if (!collection[role]) {
collection[role] = [];
}
if (member.type === 'node') {
collection[role].push(member.id);
if (role === 'via') {
collection.keyfrom.push(member.id);
collection.keyto.push(member.id);
}
} else if (member.type === 'way') {
collection[role].push.apply(collection[role], entity.nodes);
if (role === 'from' || role === 'via') {
collection.keyfrom.push(entity.first());
collection.keyfrom.push(entity.last());
}
if (role === 'to' || role === 'via') {
collection.keyto.push(entity.first());
collection.keyto.push(entity.last());
}
}
}
};
+185 -52
View File
@@ -174,98 +174,231 @@ describe('iD.actionConnect', function() {
describe('#disabled', function () {
it('returns falsy when connecting members of the same relation and same roles (different ways)', function () {
//
// "Route 1" "Route 1"
// 'forward' 'forward'
// a ----- b c ===== d
//
it('returns falsy when connecting members of the same relation and same roles', function () {
var graph = iD.coreGraph([
iD.osmNode({id: 'a'}),
iD.osmNode({id: 'b'}),
iD.osmNode({id: 'c'}),
iD.osmNode({id: 'd'}),
iD.osmWay({id: '-', nodes: ['a', 'b']}),
iD.osmWay({id: '=', nodes: ['c', 'd']}),
iD.osmRelation({id: 'r1', tags: { name: 'Route 1'}, members: [
{ id: '-', type: 'way', role: 'forward' },
{ id: '=', type: 'way', role: 'forward' }
iD.osmWay({id: '-', nodes: ['a', 'b', 'c']}),
iD.osmRelation({id: 'r1', members: [
{ id: 'b', type: 'node', role: 'foo' },
{ id: 'c', type: 'node', role: 'foo' }
]})
]);
expect(iD.actionConnect(['b', 'c']).disabled(graph)).to.be.not.ok;
});
it('returns falsy when connecting members of the different relation and different roles (different ways)', function () {
//
// "Route 1" "Route 2"
// 'forward' 'backward'
// a ----- b c ===== d
//
it('returns falsy when connecting members of the different relation and different roles', function () {
var graph = iD.coreGraph([
iD.osmNode({id: 'a'}),
iD.osmNode({id: 'b'}),
iD.osmNode({id: 'c'}),
iD.osmNode({id: 'd'}),
iD.osmWay({id: '-', nodes: ['a', 'b']}),
iD.osmWay({id: '=', nodes: ['c', 'd']}),
iD.osmRelation({id: 'r1', tags: { name: 'Route 1'}, members: [
{ id: '-', type: 'way', role: 'forward' }
]}),
iD.osmRelation({id: 'r2', tags: { name: 'Route 2'}, members: [
{ id: '=', type: 'way', role: 'backward' }
]})
iD.osmWay({id: '-', nodes: ['a', 'b', 'c']}),
iD.osmRelation({id: 'r1', members: [{ id: 'b', type: 'node', role: 'foo' } ]}),
iD.osmRelation({id: 'r2', members: [{ id: 'c', type: 'node', role: 'bar' } ]})
]);
expect(iD.actionConnect(['b', 'c']).disabled(graph)).to.be.not.ok;
});
it('returns \'relation\' when connecting members of the same relation but different roles (different ways)', function () {
//
// "Route 1" "Route 1"
// 'forward' 'backward'
// a ----- b c ===== d
//
it('returns \'relation\' when connecting members of the same relation but different roles', function () {
var graph = iD.coreGraph([
iD.osmNode({id: 'a'}),
iD.osmNode({id: 'b'}),
iD.osmNode({id: 'c'}),
iD.osmNode({id: 'd'}),
iD.osmWay({id: '-', nodes: ['a', 'b']}),
iD.osmWay({id: '=', nodes: ['c', 'd']}),
iD.osmRelation({id: 'r1', tags: { name: 'Route 1'}, members: [
{ id: '-', type: 'way', role: 'forward' },
{ id: '=', type: 'way', role: 'backward' }
iD.osmWay({id: '-', nodes: ['a', 'b', 'c']}),
iD.osmRelation({id: 'r1', members: [
{ id: 'b', type: 'node', role: 'foo' },
{ id: 'c', type: 'node', role: 'bar' }
]})
]);
expect(iD.actionConnect(['b', 'c']).disabled(graph)).to.eql('relation');
});
it('returns \'relation\' when connecting members of the same relation but different roles (joined way)', function () {
it('returns falsy when connecting nodes that would not break a via-node restriction', function () {
//
// from via to
// a --- b === c ~~~ d
// a --- b --- c r1: `no_right_turn`
// | FROM '-'
// d VIA 'c'
// | TO '|'
// e
//
var graph = iD.coreGraph([
iD.osmNode({id: 'a'}),
iD.osmNode({id: 'b'}),
iD.osmNode({id: 'c'}),
iD.osmNode({id: 'd'}),
iD.osmWay({id: '-', nodes: ['a', 'b']}),
iD.osmWay({id: '=', nodes: ['b', 'c']}),
iD.osmWay({id: '~', nodes: ['c', 'd']}),
iD.osmRelation({id: 'r', members: [
iD.osmNode({id: 'e'}),
iD.osmWay({id: '-', nodes: ['a', 'b', 'c']}),
iD.osmWay({id: '|', nodes: ['c', 'd', 'e']}),
iD.osmRelation({id: 'r1', tags: { type: 'restriction', restriction: 'no_right_turn' }, members: [
{ id: '-', type: 'way', role: 'from' },
{ id: '=', type: 'way', role: 'via' },
{ id: '~', type: 'way', role: 'to' }
{ id: 'c', type: 'node', role: 'via' },
{ id: '|', type: 'way', role: 'to' }
]})
]);
expect(iD.actionConnect(['a', 'b']).disabled(graph)).to.eql('relation');
expect(iD.actionConnect(['b', 'c']).disabled(graph)).to.eql('relation');
expect(iD.actionConnect(['c', 'd']).disabled(graph)).to.eql('relation');
// allowed: adjacent connections that don't destroy a way
expect(iD.actionConnect(['a', 'b']).disabled(graph)).to.be.not.ok;
expect(iD.actionConnect(['b', 'c']).disabled(graph)).to.be.not.ok;
expect(iD.actionConnect(['c', 'd']).disabled(graph)).to.be.not.ok;
expect(iD.actionConnect(['d', 'e']).disabled(graph)).to.be.not.ok;
});
});
it('returns falsy when connecting nodes that would not break a via-way restriction', function () {
//
// a --- b --- c r1: `no_u_turn`
// | FROM '='
// d VIA '|'
// | TO '-'
// g === f === e
//
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.osmNode({id: 'f'}),
iD.osmNode({id: 'g'}),
iD.osmWay({id: '-', nodes: ['a', 'b', 'c']}),
iD.osmWay({id: '|', nodes: ['c', 'd', 'e']}),
iD.osmWay({id: '=', nodes: ['e', 'f', 'g']}),
iD.osmRelation({id: 'r1', tags: { type: 'restriction', restriction: 'no_u_turn' }, members: [
{ id: '=', type: 'way', role: 'from' },
{ id: '|', type: 'way', role: 'via' },
{ id: '-', type: 'way', role: 'to' }
]})
]);
// allowed: adjacent connections that don't destroy a way
expect(iD.actionConnect(['a', 'b']).disabled(graph)).to.be.not.ok;
expect(iD.actionConnect(['b', 'c']).disabled(graph)).to.be.not.ok;
expect(iD.actionConnect(['c', 'd']).disabled(graph)).to.be.not.ok;
expect(iD.actionConnect(['d', 'e']).disabled(graph)).to.be.not.ok;
expect(iD.actionConnect(['e', 'f']).disabled(graph)).to.be.not.ok;
expect(iD.actionConnect(['f', 'g']).disabled(graph)).to.be.not.ok;
});
it('returns \'restriction\' when connecting nodes that would break a via-node restriction', function () {
//
// a --- b --- c r1: `no_right_turn`
// | FROM '-'
// d VIA 'c'
// | TO '|'
// e
//
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', 'e']}),
iD.osmRelation({id: 'r1', tags: { type: 'restriction', restriction: 'no_right_turn' }, members: [
{ id: '-', type: 'way', role: 'from' },
{ id: 'c', type: 'node', role: 'via' },
{ id: '|', type: 'way', role: 'to' }
]})
]);
// prevented:
// extra connections to the VIA node, or any connections between FROM and TO
expect(iD.actionConnect(['a', 'c']).disabled(graph)).to.eql('restriction', 'extra connection FROM-VIA');
expect(iD.actionConnect(['e', 'c']).disabled(graph)).to.eql('restriction', 'extra connection TO-VIA');
expect(iD.actionConnect(['b', 'd']).disabled(graph)).to.eql('restriction', 'extra connection FROM-TO');
});
it('returns \'restriction\' when connecting nodes that would break a via-way restriction', function () {
//
// a --- b --- c r1: `no_u_turn`
// | FROM '='
// d VIA '|'
// | TO '-'
// g === f === e
//
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.osmNode({id: 'f'}),
iD.osmNode({id: 'g'}),
iD.osmWay({id: '-', nodes: ['a', 'b', 'c']}),
iD.osmWay({id: '|', nodes: ['c', 'd', 'e']}),
iD.osmWay({id: '=', nodes: ['e', 'f', 'g']}),
iD.osmRelation({id: 'r1', tags: { type: 'restriction', restriction: 'no_u_turn' }, members: [
{ id: '=', type: 'way', role: 'from' },
{ id: '|', type: 'way', role: 'via' },
{ id: '-', type: 'way', role: 'to' }
]})
]);
// prevented:
// extra connections to any node along VIA way
expect(iD.actionConnect(['a', 'c']).disabled(graph)).to.eql('restriction', 'extra connection TO-VIA c');
expect(iD.actionConnect(['b', 'd']).disabled(graph)).to.eql('restriction', 'extra connection TO-VIA d');
expect(iD.actionConnect(['b', 'e']).disabled(graph)).to.eql('restriction', 'extra connection TO-VIA e');
expect(iD.actionConnect(['c', 'e']).disabled(graph)).to.eql('restriction', 'extra connection VIA-VIA');
expect(iD.actionConnect(['f', 'c']).disabled(graph)).to.eql('restriction', 'extra connection FROM-VIA c');
expect(iD.actionConnect(['f', 'd']).disabled(graph)).to.eql('restriction', 'extra connection FROM-VIA d');
expect(iD.actionConnect(['g', 'e']).disabled(graph)).to.eql('restriction', 'extra connection FROM-VIA e');
});
it('returns \'restriction\' when connecting would destroy a way in a via-node restriction', function () {
//
// a --- b r1: `no_right_turn`
// | FROM '-'
// | VIA 'b'
// c TO '|'
//
var graph = iD.coreGraph([
iD.osmNode({id: 'a'}),
iD.osmNode({id: 'b'}),
iD.osmNode({id: 'c'}),
iD.osmWay({id: '-', nodes: ['a', 'b']}),
iD.osmWay({id: '|', nodes: ['b', 'c']}),
iD.osmRelation({id: 'r1', tags: { type: 'restriction', restriction: 'no_right_turn' }, members: [
{ id: '-', type: 'way', role: 'from' },
{ id: 'b', type: 'node', role: 'via' },
{ id: '|', type: 'way', role: 'to' }
]})
]);
expect(iD.actionConnect(['a', 'b']).disabled(graph)).to.eql('restriction', 'destroy FROM');
expect(iD.actionConnect(['b', 'c']).disabled(graph)).to.eql('restriction', 'destroy TO');
});
it('returns \'restriction\' when connecting would destroy a way in via-way restriction', function () {
//
// a --- b r1: `no_u_turn`
// | FROM '='
// | VIA '|'
// d === c TO '-'
//
var graph = iD.coreGraph([
iD.osmNode({id: 'a'}),
iD.osmNode({id: 'b'}),
iD.osmNode({id: 'c'}),
iD.osmNode({id: 'd'}),
iD.osmWay({id: '-', nodes: ['a', 'b']}),
iD.osmWay({id: '|', nodes: ['b', 'c']}),
iD.osmWay({id: '=', nodes: ['c', 'd']}),
iD.osmRelation({id: 'r1', tags: { type: 'restriction', restriction: 'no_u_turn' }, members: [
{ id: '=', type: 'way', role: 'from' },
{ id: '|', type: 'way', role: 'via' },
{ id: '-', type: 'way', role: 'to' }
]})
]);
expect(iD.actionConnect(['a', 'b']).disabled(graph)).to.eql('restriction', 'destroy TO');
expect(iD.actionConnect(['b', 'c']).disabled(graph)).to.eql('restriction', 'destroy VIA');
expect(iD.actionConnect(['c', 'd']).disabled(graph)).to.eql('restriction', 'destroy FROM');
});
});
});