From 80b7fd7f40d2bdbf4d89742579df2c9a914c02b3 Mon Sep 17 00:00:00 2001 From: ProtD Date: Sun, 9 Jun 2013 01:19:48 +0200 Subject: [PATCH] Allow to join more than two ways (#649) --- js/id/actions/join.js | 123 +++++++++++++++++++++++--------------- test/spec/actions/join.js | 52 ++++++++++++---- 2 files changed, 116 insertions(+), 59 deletions(-) diff --git a/js/id/actions/join.js b/js/id/actions/join.js index 286a2c3fe..06f818189 100644 --- a/js/id/actions/join.js +++ b/js/id/actions/join.js @@ -7,8 +7,6 @@ // https://github.com/openstreetmap/josm/blob/mirror/src/org/openstreetmap/josm/actions/CombineWayAction.java // iD.actions.Join = function(ids) { - var idA = ids[0], - idB = ids[1]; function groupEntitiesByGeometry(graph) { var entities = ids.map(function(id) { return graph.entity(id); }); @@ -16,70 +14,99 @@ iD.actions.Join = function(ids) { } var action = function(graph) { - var a = graph.entity(idA), - b = graph.entity(idB), - nodes; + var existing = 0, + nodes, + a, b, + i, j; + + function replaceWithA(parent) { + graph = graph.replace(parent.replaceMember(b, a)); + } // Prefer to keep an existing way. - if (a.isNew() && !b.isNew()) { - var tmp = a; - a = b; - b = tmp; - idA = a.id; - idB = b.id; + for (i = 0; i < ids.length; i++) { + if (!graph.entity(ids[i]).isNew()) { + existing = i; + break; + } } - if (a.first() === b.first()) { - // a <-- b ==> c - // Expected result: - // a <-- b <-- c - b = iD.actions.Reverse(idB)(graph).entity(idB); - nodes = b.nodes.slice().concat(a.nodes.slice(1)); + // Join ways to 'a' in the following order: a-1, a-2, ..., 0, a+1, a+2, ..., ids.length-1 + for (i = 0; i < ids.length; i++) { + j = (i <= existing) ? (existing - i) : i; + if (j === existing) { + continue; + } - } else if (a.first() === b.last()) { - // a <-- b <== c - // Expected result: - // a <-- b <-- c - nodes = b.nodes.concat(a.nodes.slice(1)); + a = graph.entity(ids[existing]); + b = graph.entity(ids[j]); - } else if (a.last() === b.first()) { - // a --> b ==> c - // Expected result: - // a --> b --> c - nodes = a.nodes.concat(b.nodes.slice(1)); + if (a.first() === b.first()) { + // a <-- b ==> c + // Expected result: + // a <-- b <-- c + b = iD.actions.Reverse(ids[j])(graph).entity(ids[j]); + nodes = b.nodes.slice().concat(a.nodes.slice(1)); - } else if (a.last() === b.last()) { - // a --> b <== c - // Expected result: - // a --> b --> c - b = iD.actions.Reverse(idB)(graph).entity(idB); - nodes = a.nodes.concat(b.nodes.slice().slice(1)); + } else if (a.first() === b.last()) { + // a <-- b <== c + // Expected result: + // a <-- b <-- c + nodes = b.nodes.concat(a.nodes.slice(1)); + + } else if (a.last() === b.first()) { + // a --> b ==> c + // Expected result: + // a --> b --> c + nodes = a.nodes.concat(b.nodes.slice(1)); + + } else if (a.last() === b.last()) { + // a --> b <== c + // Expected result: + // a --> b --> c + b = iD.actions.Reverse(ids[j])(graph).entity(ids[j]); + nodes = a.nodes.concat(b.nodes.slice().slice(1)); + } + + graph.parentRelations(b).forEach(replaceWithA); + + graph = graph.replace(a.mergeTags(b.tags).update({ nodes: nodes })); + graph = iD.actions.DeleteWay(ids[j])(graph); } - graph.parentRelations(b).forEach(function(parent) { - graph = graph.replace(parent.replaceMember(b, a)); - }); - - graph = graph.replace(a.mergeTags(b.tags).update({ nodes: nodes })); - graph = iD.actions.DeleteWay(idB)(graph); - return graph; }; action.disabled = function(graph) { - var geometries = groupEntitiesByGeometry(graph); + var geometries = groupEntitiesByGeometry(graph), + i; - if (ids.length !== 2 || ids.length !== geometries.line.length) + // direction of the previous way -- the next way can join only on the opposite side than the previous joint + var prev_direction = 0; + + if (ids.length < 2 || ids.length !== geometries.line.length) return 'not_eligible'; - var a = graph.entity(idA), - b = graph.entity(idB); + for (i = 0; i+1 < ids.length; i++) { + var a = graph.entity(ids[i+0]), + b = graph.entity(ids[i+1]); + + if (a.first() === b.first() && prev_direction <= 0) { + prev_direction = 1; + continue; + } else if (a.first() === b.last() && prev_direction <= 0) { + prev_direction = -1; + continue; + } else if (a.last() === b.first() && prev_direction >= 0) { + prev_direction = 1; + continue; + } else if (a.last() === b.last() && prev_direction >= 0) { + prev_direction = -1; + continue; + } - if (a.first() !== b.first() && - a.first() !== b.last() && - a.last() !== b.first() && - a.last() !== b.last()) return 'not_adjacent'; + } }; return action; diff --git a/test/spec/actions/join.js b/test/spec/actions/join.js index 268cc5ac1..02eefdbb2 100644 --- a/test/spec/actions/join.js +++ b/test/spec/actions/join.js @@ -149,23 +149,51 @@ describe("iD.actions.Join", function () { expect(graph.entity('-').tags).to.eql({'lanes:backward': 2}); }); - it("prefers to keep existing ways", function () { - // a --> b ==> c - // --- is new, === is existing + it("joins a --> b <== c <++ d **> e", function () { // Expected result: - // a ==> b ==> c + // a --> b --> c --> d --> e + // tags on === reversed var graph = iD.Graph({ 'a': iD.Node({id: 'a'}), 'b': iD.Node({id: 'b'}), 'c': iD.Node({id: 'c'}), - 'w-1': iD.Way({id: 'w-1', nodes: ['a', 'b']}), - 'w1': iD.Way({id: 'w1', nodes: ['b', 'c']}) + 'd': iD.Node({id: 'd'}), + 'e': iD.Node({id: 'e'}), + '-': iD.Way({id: '-', nodes: ['a', 'b']}), + '=': iD.Way({id: '=', nodes: ['c', 'b'], tags: {'lanes:forward': 2}}), + '+': iD.Way({id: '+', nodes: ['d', 'c']}), + '*': iD.Way({id: '*', nodes: ['d', 'e'], tags: {'lanes:backward': 2}}) }); - graph = iD.actions.Join(['w-1', 'w1'])(graph); + graph = iD.actions.Join(['-', '=', '+', '*'])(graph); - expect(graph.entity('w1').nodes).to.eql(['a', 'b', 'c']); + expect(graph.entity('-').nodes).to.eql(['a', 'b', 'c', 'd', 'e']); + expect(graph.hasEntity('=')).to.be.undefined; + expect(graph.hasEntity('+')).to.be.undefined; + expect(graph.hasEntity('*')).to.be.undefined; + expect(graph.entity('-').tags).to.eql({'lanes:backward': 2}); + }); + + it("prefers to keep existing ways", function () { + // a --> b ==> c ++> d + // --- is new, === is existing, +++ is new + // Expected result: + // a ==> b ==> c ==> d + var graph = iD.Graph({ + 'a': iD.Node({id: 'a'}), + 'b': iD.Node({id: 'b'}), + 'c': iD.Node({id: 'c'}), + 'd': iD.Node({id: 'd'}), + 'w-1': iD.Way({id: 'w-1', nodes: ['a', 'b']}), + 'w1': iD.Way({id: 'w1', nodes: ['b', 'c']}), + 'w-2': iD.Way({id: 'w-2', nodes: ['c', 'd']}) + }); + + graph = iD.actions.Join(['w-1', 'w1', 'w-2'])(graph); + + expect(graph.entity('w1').nodes).to.eql(['a', 'b', 'c', 'd']); expect(graph.hasEntity('w-1')).to.be.undefined; + expect(graph.hasEntity('w-2')).to.be.undefined; }); it("merges tags", function () { @@ -173,13 +201,15 @@ describe("iD.actions.Join", function () { 'a': iD.Node({id: 'a'}), 'b': iD.Node({id: 'b'}), 'c': iD.Node({id: 'c'}), + 'd': iD.Node({id: 'd'}), '-': iD.Way({id: '-', nodes: ['a', 'b'], tags: {a: 'a', b: '-', c: 'c'}}), - '=': iD.Way({id: '=', nodes: ['b', 'c'], tags: {a: 'a', b: '=', d: 'd'}}) + '=': iD.Way({id: '=', nodes: ['b', 'c'], tags: {a: 'a', b: '=', d: 'd'}}), + '+': iD.Way({id: '+', nodes: ['c', 'd'], tags: {a: 'a', b: '=', e: 'e'}}) }); - graph = iD.actions.Join(['-', '='])(graph); + graph = iD.actions.Join(['-', '=', '+'])(graph); - expect(graph.entity('-').tags).to.eql({a: 'a', b: '-;=', c: 'c', d: 'd'}); + expect(graph.entity('-').tags).to.eql({a: 'a', b: '-;=', c: 'c', d: 'd', e: 'e'}); }); it("merges relations", function () {