Merge pull request #2178 from bhousel/bhousel-operations

Using disconnect on a closed way can break areas.
This commit is contained in:
John Firebaugh
2014-04-01 14:39:23 -07:00
2 changed files with 183 additions and 36 deletions

View File

@@ -17,41 +17,52 @@ iD.actions.Disconnect = function(nodeId, newNodeId) {
var action = function(graph) {
var node = graph.entity(nodeId),
replacements = action.replacements(graph);
connections = action.connections(graph);
connections.forEach(function(connection) {
var way = graph.entity(connection.wayID),
newNode = iD.Node({id: newNodeId, loc: node.loc, tags: node.tags});
replacements.forEach(function(replacement) {
var newNode = iD.Node({id: newNodeId, loc: node.loc, tags: node.tags});
graph = graph.replace(newNode);
graph = graph.replace(graph.entity(replacement.wayID).updateNode(newNode.id, replacement.index));
if (connection.index === 0 && way.isArea()) {
// replace shared node with shared node..
graph = graph.replace(way.replaceNode(way.nodes[0], newNode.id));
} else {
// replace shared node with multiple new nodes..
graph = graph.replace(way.updateNode(newNode.id, connection.index));
}
});
return graph;
};
action.replacements = function(graph) {
action.connections = function(graph) {
var candidates = [],
keeping = false,
parents = graph.parentWays(graph.entity(nodeId));
parentWays = graph.parentWays(graph.entity(nodeId));
parents.forEach(function(parent) {
if (wayIds && wayIds.indexOf(parent.id) === -1) {
parentWays.forEach(function(way) {
if (wayIds && wayIds.indexOf(way.id) === -1) {
keeping = true;
return;
}
parent.nodes.forEach(function(waynode, index) {
if (waynode === nodeId) {
candidates.push({wayID: parent.id, index: index});
}
});
if (way.isArea() && (way.nodes[0] === nodeId)) {
candidates.push({wayID: way.id, index: 0});
} else {
way.nodes.forEach(function(waynode, index) {
if (waynode === nodeId) {
candidates.push({wayID: way.id, index: index});
}
});
}
});
return keeping ? candidates : candidates.slice(1);
};
action.disabled = function(graph) {
var replacements = action.replacements(graph);
if (replacements.length === 0 || (wayIds && wayIds.length !== replacements.length))
var connections = action.connections(graph);
if (connections.length === 0 || (wayIds && wayIds.length !== connections.length))
return 'not_connected';
};

View File

@@ -2,11 +2,10 @@ describe("iD.actions.Disconnect", function () {
describe("#disabled", function () {
it("returns 'not_connected' for a node shared by less than two ways", function () {
var graph = iD.Graph([iD.Node({id: 'a'})]);
expect(iD.actions.Disconnect('a').disabled(graph)).to.equal('not_connected');
});
it("returns falsy for a node appearing twice in the same way", function () {
it("returns falsy for the closing node in a closed line", function () {
// a ---- b
// | |
// d ---- c
@@ -20,6 +19,36 @@ describe("iD.actions.Disconnect", function () {
expect(iD.actions.Disconnect('a').disabled(graph)).not.to.be.ok;
});
it("returns not_connected for the closing node in a closed area", function () {
// a ---- b
// | |
// d ---- c
var graph = iD.Graph([
iD.Node({id: 'a'}),
iD.Node({id: 'b'}),
iD.Node({id: 'c'}),
iD.Node({id: 'd'}),
iD.Way({id: 'w', nodes: ['a', 'b', 'c', 'd', 'a'], tags: {area: 'yes'}})
]);
expect(iD.actions.Disconnect('a').disabled(graph)).to.equal('not_connected');
});
it("returns falsy for a shared non-closing node in an area", function () {
// a --- b --- c
// | |
// e --- d
var graph = iD.Graph([
iD.Node({id: 'a'}),
iD.Node({id: 'b'}),
iD.Node({id: 'c'}),
iD.Node({id: 'd'}),
iD.Node({id: 'e'}),
iD.Way({id: 'w', nodes: ['a', 'b', 'c', 'd', 'e', 'b', 'a'], tags: {area: 'yes'}})
]);
expect(iD.actions.Disconnect('b').disabled(graph)).not.to.be.ok;
});
it("returns falsy for a node shared by two or more ways", function () {
// a ---- b ---- c
// |
@@ -37,18 +66,21 @@ describe("iD.actions.Disconnect", function () {
});
it("returns falsy for an intersection of two ways with parent way specified", function () {
// a ---- b ---- c
// |
// d
var graph = iD.Graph([
iD.Node({id: 'a'}),
iD.Node({id: 'b'}),
iD.Node({id: 'c'}),
iD.Node({id: 'c'}),
iD.Node({id: '*'}),
iD.Way({id: '-', nodes: ['a', '*', 'b']}),
iD.Way({id: '|', nodes: ['*', 'd']})
iD.Node({id: 'd'}),
iD.Way({id: '-', nodes: ['a', 'b', 'c']}),
iD.Way({id: '|', nodes: ['d', 'b']})
]);
expect(iD.actions.Disconnect('*', ['|']).disabled(graph)).not.to.be.ok;
expect(iD.actions.Disconnect('b', ['|']).disabled(graph)).not.to.be.ok;
});
});
it("replaces the node with a new node in all but the first way", function () {
@@ -88,9 +120,9 @@ describe("iD.actions.Disconnect", function () {
// Disconnect - at b.
//
// Expected result:
// a ---- e b ==== c
// |
// d
// a ---- e b ==== c
// |
// d
//
var graph = iD.Graph([
iD.Node({id: 'a'}),
@@ -111,13 +143,17 @@ describe("iD.actions.Disconnect", function () {
it("replaces later occurrences in a self-intersecting way", function() {
// Situtation:
// a ---- b
// \_ |
// \__ c
// Disconnect at a
// a --- b
// \ /
// \ /
// c
// Disconnect at a
//
// Expected result:
// a ---- b ---- c ---- d
// a --- b
// |
// d --- c
//
var graph = iD.Graph([
iD.Node({id: 'a'}),
iD.Node({id: 'b'}),
@@ -130,12 +166,19 @@ describe("iD.actions.Disconnect", function () {
it("disconnects a way with multiple intersection points", function() {
// Situtation:
// a = b - c
// | |
// e - d
// Where b starts/ends -.
// a == b -- c
// | |
// e -- d
// 2 ways a-b and b-c-d-e-b
//
// Disconnect at b
//
// Expected Result:
// a == b * -- c
// | |
// e -- d
// 2 ways a-b and *-c-d-e-*
//
var graph = iD.Graph([
iD.Node({id: 'a'}),
iD.Node({id: 'b'}),
@@ -152,6 +195,99 @@ describe("iD.actions.Disconnect", function () {
expect(graph.entity('w2').nodes).to.eql(['*', 'c', 'd', 'e', '*']);
});
it("disconnects a shared non-closing node in an area", function() {
// Situtation:
// a -- b -- c
// | |
// e -- d
//
// Disconnect at b
//
// Expected Result:
// a -- b -- c
// | |
// * -- e -- d
//
var graph = iD.Graph([
iD.Node({id: 'a'}),
iD.Node({id: 'b'}),
iD.Node({id: 'c'}),
iD.Node({id: 'd'}),
iD.Node({id: 'e'}),
iD.Way({id: 'w', nodes: ['a', 'b', 'c', 'd', 'e', 'b', 'a'], tags: {area: 'yes'}})
]);
graph = iD.actions.Disconnect('b', '*')(graph);
expect(graph.entity('w').nodes).to.eql(['a', 'b', 'c', 'd', 'e', '*', 'a']);
});
it("disconnects the closing node of an area without breaking the area", function() {
// Situtation:
// a --- b --- d
// \ / \ /
// \ / \ /
// c e
// 2 areas: a-b-c-a and b-d-e-b
//
// Disconnect at b
//
// Expected Result:
// a --- b * --- d
// \ / \ /
// \ / \ /
// c e
// 2 areas: a-b-c-a and *-d-e-*
var graph = iD.Graph([
iD.Node({id: 'a'}),
iD.Node({id: 'b'}),
iD.Node({id: 'c'}),
iD.Node({id: 'd'}),
iD.Node({id: 'e'}),
iD.Way({id: 'w1', nodes: ['a', 'b', 'c', 'a'], tags: {area: 'yes'}}),
iD.Way({id: 'w2', nodes: ['b', 'd', 'e', 'b'], tags: {area: 'yes'}})
]);
graph = iD.actions.Disconnect('b', '*')(graph);
expect(graph.entity('w1').nodes).to.eql(['a', 'b', 'c', 'a']);
expect(graph.entity('w2').nodes).to.eql(['*', 'd', 'e', '*']);
});
it("disconnects multiple closing nodes of multiple areas without breaking the areas", function() {
// Situtation:
// a --- b --- d
// \ / \ /
// \ / \ /
// c e
// 2 areas: b-c-a-b and b-d-e-b
//
// Disconnect at b
//
// Expected Result:
// a --- b * --- d
// \ / \ /
// \ / \ /
// c e
// 2 areas: b-c-a-b and *-d-e-*
var graph = iD.Graph([
iD.Node({id: 'a'}),
iD.Node({id: 'b'}),
iD.Node({id: 'c'}),
iD.Node({id: 'd'}),
iD.Node({id: 'e'}),
iD.Way({id: 'w1', nodes: ['b', 'c', 'a', 'b'], tags: {area: 'yes'}}),
iD.Way({id: 'w2', nodes: ['b', 'd', 'e', 'b'], tags: {area: 'yes'}})
]);
graph = iD.actions.Disconnect('b', '*')(graph);
expect(graph.entity('w1').nodes).to.eql(['b', 'c', 'a', 'b']);
expect(graph.entity('w2').nodes).to.eql(['*', 'd', 'e', '*']);
});
it("copies location and tags to the new nodes", function () {
var tags = {highway: 'traffic_signals'},
loc = [1, 2],