Make iD.actions.Join agnostic to selection order

This is accomplished by reusing iD.geo.joinMemberWays,
which was refactored, generalized, and renamed to joinWays.
This commit is contained in:
John Firebaugh
2013-06-18 12:19:53 -07:00
parent ac70b68e35
commit ddd9e4e8cc
7 changed files with 112 additions and 111 deletions

View File

@@ -6,7 +6,7 @@ iD.actions.AddMember = function(relationId, member, memberIndex) {
var members = relation.indexedMembers();
members.push(member);
var joined = iD.geo.joinMemberWays(members, graph);
var joined = iD.geo.joinWays(members, graph);
for (var i = 0; i < joined.length; i++) {
var segment = joined[i];
for (var j = 0; j < segment.length && segment.length >= 2; j++) {

View File

@@ -14,99 +14,47 @@ iD.actions.Join = function(ids) {
}
var action = function(graph) {
var existing = 0,
nodes,
a, b,
i, j;
function replaceWithA(parent) {
graph = graph.replace(parent.replaceMember(b, a));
}
var ways = ids.map(graph.entity, graph),
survivor = ways[0];
// Prefer to keep an existing way.
for (i = 0; i < ids.length; i++) {
if (!graph.entity(ids[i]).isNew()) {
existing = i;
for (var i = 0; i < ways.length; i++) {
if (!ways[i].isNew()) {
survivor = ways[i];
break;
}
}
// 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;
}
var joined = iD.geo.joinWays(ways, graph)[0];
a = graph.entity(ids[existing]);
b = graph.entity(ids[j]);
survivor = survivor.update({nodes: _.pluck(joined.nodes, 'id')});
graph = graph.replace(survivor);
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));
joined.forEach(function(way) {
if (way.id === survivor.id)
return;
} else if (a.first() === b.last()) {
// a <-- b <== c
// Expected result:
// a <-- b <-- c
nodes = b.nodes.concat(a.nodes.slice(1));
graph.parentRelations(way).forEach(function(parent) {
graph = graph.replace(parent.replaceMember(way, survivor));
});
} else if (a.last() === b.first()) {
// a --> b ==> c
// Expected result:
// a --> b --> c
nodes = a.nodes.concat(b.nodes.slice(1));
survivor = survivor.mergeTags(way.tags);
} 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 = graph.replace(survivor);
graph = iD.actions.DeleteWay(way.id)(graph);
});
return graph;
};
action.disabled = function(graph) {
var geometries = groupEntitiesByGeometry(graph),
i;
// direction of the previous way -- the next way can join only on the opposite side than the previous joint
var prev_direction = 0;
var geometries = groupEntitiesByGeometry(graph);
if (ids.length < 2 || ids.length !== geometries.line.length)
return 'not_eligible';
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;
}
var joined = iD.geo.joinWays(ids.map(graph.entity, graph), graph);
if (joined.length > 1)
return 'not_adjacent';
}
};
return action;

View File

@@ -25,10 +25,10 @@ iD.actions.MergePolygon = function(ids, newRelationId) {
// Each element is itself an array of objects with an id property, and has a
// locs property which is an array of the locations forming the polygon.
var polygons = entities.multipolygon.reduce(function(polygons, m) {
return polygons.concat(iD.geo.joinMemberWays(m.members, graph));
return polygons.concat(iD.geo.joinWays(m.members, graph));
}, []).concat(entities.closedWay.map(function(d) {
var member = [{id: d.id}];
member.locs = graph.childNodes(d).map(function(n) { return n.loc; });
member.nodes = graph.childNodes(d);
return member;
}));
@@ -38,7 +38,9 @@ iD.actions.MergePolygon = function(ids, newRelationId) {
var contained = polygons.map(function(w, i) {
return polygons.map(function(d, n) {
if (i === n) return null;
return iD.geo.polygonContainsPolygon(d.locs, w.locs);
return iD.geo.polygonContainsPolygon(
_.pluck(d.nodes, 'loc'),
_.pluck(w.nodes, 'loc'));
});
});

View File

@@ -185,11 +185,11 @@ _.extend(iD.Relation.prototype, {
var outers = this.members.filter(function(m) { return 'outer' === (m.role || 'outer'); }),
inners = this.members.filter(function(m) { return 'inner' === m.role; });
outers = iD.geo.joinMemberWays(outers, resolver);
inners = iD.geo.joinMemberWays(inners, resolver);
outers = iD.geo.joinWays(outers, resolver);
inners = iD.geo.joinWays(inners, resolver);
outers = _.pluck(outers, 'locs');
inners = _.pluck(inners, 'locs');
outers = outers.map(function(outer) { return _.pluck(outer.nodes, 'loc'); });
inners = inners.map(function(inner) { return _.pluck(inner.nodes, 'loc'); });
var result = outers.map(function(o) { return [o]; });

View File

@@ -49,58 +49,72 @@ iD.geo.simpleMultipolygonOuterMember = function(entity, graph) {
return outerMember && graph.hasEntity(outerMember.id);
};
// Join an array of relation `members` into sequences of connecting segments.
// Join `array` into sequences of connecting ways.
//
// Segments which share identical start/end nodes will, as much as possible,
// be connected with each other.
//
// The return value is a nested array. Each constituent array contains elements
// of `members` which have been determined to connect. Each consitituent array
// also has a `locs` property whose value is an ordered array of member coordinates,
// of `array` which have been determined to connect. Each consitituent array
// also has a `nodes` property whose value is an ordered array of member nodes,
// with appropriate order reversal and start/end coordinate de-duplication.
//
// Incomplete members are ignored.
// Members of `array` must have, at minimum, `type` and `id` properties.
// Thus either an array of `iD.Way`s or a relation member array may be
// used.
//
iD.geo.joinMemberWays = function(members, graph) {
var joined = [], member, current, locs, first, last, i, how, what;
// If an member has a `tags` property, its tags will be reversed via
// `iD.actions.Reverse` in the output.
//
// Incomplete members (those for which `graph.hasEntity(element.id)` returns
// false) and non-way members are ignored.
//
iD.geo.joinWays = function(array, graph) {
var joined = [], member, current, nodes, first, last, i, how, what;
members = members.filter(function(member) {
array = array.filter(function(member) {
return member.type === 'way' && graph.hasEntity(member.id);
});
function resolve(member) {
return _.pluck(graph.childNodes(graph.entity(member.id)), 'loc');
return graph.childNodes(graph.entity(member.id));
}
while (members.length) {
member = members.shift();
function reverse(member) {
return member.tags ? iD.actions.Reverse(member.id)(graph).entity(member.id) : member;
}
while (array.length) {
member = array.shift();
current = [member];
current.locs = locs = resolve(member);
current.nodes = nodes = resolve(member);
joined.push(current);
while (members.length && _.first(locs) !== _.last(locs)) {
first = _.first(locs);
last = _.last(locs);
while (array.length && _.first(nodes) !== _.last(nodes)) {
first = _.first(nodes);
last = _.last(nodes);
for (i = 0; i < members.length; i++) {
member = members[i];
for (i = 0; i < array.length; i++) {
member = array[i];
what = resolve(member);
if (last === _.first(what)) {
how = locs.push;
how = nodes.push;
what = what.slice(1);
break;
} else if (last === _.last(what)) {
how = locs.push;
how = nodes.push;
what = what.slice(0, -1).reverse();
member = reverse(member);
break;
} else if (first === _.last(what)) {
how = locs.unshift;
how = nodes.unshift;
what = what.slice(0, -1);
break;
} else if (first === _.first(what)) {
how = locs.unshift;
how = nodes.unshift;
what = what.slice(1).reverse();
member = reverse(member);
break;
} else {
what = how = null;
@@ -111,9 +125,9 @@ iD.geo.joinMemberWays = function(members, graph) {
break; // No more joinable ways.
how.apply(current, [member]);
how.apply(locs, what);
how.apply(nodes, what);
members.splice(i, 1);
array.splice(i, 1);
}
}

View File

@@ -52,6 +52,26 @@ describe("iD.actions.Join", function () {
expect(iD.actions.Join(['-', '=']).disabled(graph)).not.to.be.ok;
});
it("returns falsy for more than two ways when connected, regardless of order", function () {
// 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'}),
'-': iD.Way({id: '-', nodes: ['a', 'b']}),
'=': iD.Way({id: '=', nodes: ['b', 'c']}),
'~': iD.Way({id: '~', nodes: ['c', 'd']})
});
expect(iD.actions.Join(['-', '=', '~']).disabled(graph)).not.to.be.ok;
expect(iD.actions.Join(['-', '~', '=']).disabled(graph)).not.to.be.ok;
expect(iD.actions.Join(['=', '-', '~']).disabled(graph)).not.to.be.ok;
expect(iD.actions.Join(['=', '~', '-']).disabled(graph)).not.to.be.ok;
expect(iD.actions.Join(['~', '=', '-']).disabled(graph)).not.to.be.ok;
expect(iD.actions.Join(['~', '-', '=']).disabled(graph)).not.to.be.ok;
});
it("returns 'not_eligible' for non-line geometries", function () {
var graph = iD.Graph({
'a': iD.Node({id: 'a'})

View File

@@ -40,17 +40,17 @@ describe("iD.geo.simpleMultipolygonOuterMember", function() {
});
});
describe("iD.geo.joinMemberWays", function() {
it("returns an array of members with locs properties", function() {
describe("iD.geo.joinWays", function() {
it("returns an array of members with nodes properties", function() {
var node = iD.Node({loc: [0, 0]}),
way = iD.Way({nodes: [node.id]}),
member = {id: way.id, type: 'way'},
graph = iD.Graph([node, way]),
result = iD.geo.joinMemberWays([member], graph);
result = iD.geo.joinWays([member], graph);
expect(result.length).to.equal(1);
expect(result[0].locs.length).to.equal(1);
expect(result[0].locs[0]).to.equal(node.loc);
expect(result[0].nodes.length).to.equal(1);
expect(result[0].nodes[0]).to.equal(node);
expect(result[0].length).to.equal(1);
expect(result[0][0]).to.equal(member);
});
@@ -72,20 +72,37 @@ describe("iD.geo.joinMemberWays", function() {
]})
});
var result = iD.geo.joinMemberWays(graph.entity('r').members, graph);
var result = iD.geo.joinWays(graph.entity('r').members, graph);
expect(_.pluck(result[0], 'id')).to.eql(['=', '-', '~']);
});
it("reverses member tags of reversed segements", function() {
// a --> b <== c
// Expected result:
// a --> b --> c
// tags on === reversed
var graph = iD.Graph({
'a': iD.Node({id: 'a'}),
'b': iD.Node({id: 'b'}),
'c': iD.Node({id: 'c'}),
'-': iD.Way({id: '-', nodes: ['a', 'b']}),
'=': iD.Way({id: '=', nodes: ['c', 'b'], tags: {'lanes:forward': 2}})
});
var result = iD.geo.joinWays([graph.entity('-'), graph.entity('=')], graph);
expect(result[0][1].tags).to.eql({'lanes:backward': 2});
});
it("ignores non-way members", function() {
var node = iD.Node({loc: [0, 0]}),
member = {id: 'n', type: 'node'},
graph = iD.Graph([node]);
expect(iD.geo.joinMemberWays([member], graph)).to.eql([]);
expect(iD.geo.joinWays([member], graph)).to.eql([]);
});
it("ignores incomplete members", function() {
var member = {id: 'w', type: 'way'},
graph = iD.Graph();
expect(iD.geo.joinMemberWays([member], graph)).to.eql([]);
expect(iD.geo.joinWays([member], graph)).to.eql([]);
});
});