Move insert way pairing code from osmJoinWays to actionAddMember

(tests for actionAddMember now passing!)
This commit is contained in:
Bryan Housel
2018-01-16 17:41:14 -05:00
parent 221158e918
commit be46e85ec0
4 changed files with 128 additions and 85 deletions
+92 -36
View File
@@ -1,49 +1,105 @@
import { osmJoinWays } from '../osm';
import _groupBy from 'lodash-es/groupBy';
import { osmJoinWays, osmWay } from '../osm';
export function actionAddMember(relationId, member, memberIndex, insertHint) {
export function actionAddMember(relationId, member, memberIndex, insertPair) {
// Relation.replaceMember() removes duplicates, and we don't want that.. #4696
function replaceMemberAll(relation, needleID, replacement) {
var members = [];
for (var i = 0; i < relation.members.length; i++) {
var member = relation.members[i];
if (member.id !== needleID) {
members.push(member);
} else {
members.push({id: replacement.id, type: replacement.type, role: member.role});
}
}
return relation.update({members: members});
}
var action = function(graph) {
var relation = graph.entity(relationId);
var numAdded = 0;
// If we weren't passed a memberIndex,
// try to perform sensible inserts based on how the ways join together
if ((isNaN(memberIndex) || insertHint) && member.type === 'way') {
var members = relation.indexedMembers();
if (!insertHint) {
members.push(member); // just push and let osmJoinWays sort it out
}
var joined = osmJoinWays(members, graph, insertHint);
for (var i = 0; i < joined.length; i++) {
var segment = joined[i];
for (var j = 0; j < segment.length && segment.length >= 2; j++) {
if (segment[j] !== member)
continue;
if (j === 0) {
memberIndex = segment[j + 1].index;
} else if (j === segment.length - 1) {
memberIndex = segment[j - 1].index + 1;
} else {
memberIndex = Math.min(segment[j - 1].index + 1, segment[j + 1].index + 1);
}
relation = relation.addMember(member, memberIndex + (numAdded++));
}
}
if ((isNaN(memberIndex) || insertPair) && member.type === 'way') {
// Try to perform sensible inserts based on how the ways join together
graph = addWayMember(relation, graph);
} else {
graph = graph.replace(relation.addMember(member, memberIndex));
}
// By default, add at index (or append to end if index undefined)
if (!numAdded) {
relation = relation.addMember(member, memberIndex);
}
return graph.replace(relation);
return graph;
};
// Add a way member into the relation "wherever it makes sense".
// In this situation we were not supplied a memberIndex.
function addWayMember(relation, graph) {
var groups;
var tempWay;
var i, j;
if (insertPair) {
// We're adding a member that must stay paired with an existing member.
// (This feature is used by `actionSplit`)
//
// This is tricky because the members may exist multiple times in the
// member list, and with different A-B/B-A ordering and different roles.
// (e.g. a bus route that loops out and back - #4589).
//
// Replace the existing member with a temporary way,
// so that `osmJoinWays` can treat the pair like a single way.
tempWay = osmWay({ id: 'wTemp', nodes: insertPair.nodes });
graph = graph.replace(tempWay);
var tempMember = { id: tempWay.id, type: 'way', role: '' };
var tempRelation = replaceMemberAll(relation, insertPair.originalID, tempMember);
groups = _groupBy(tempRelation.members, function(m) { return m.type; });
groups.way = groups.way || [];
} else {
// Add the member anywhere.. Just push and let `osmJoinWays` decide where to put it.
groups = _groupBy(relation.members, function(m) { return m.type; });
groups.way = groups.way || [];
groups.way.push(member);
}
var joined = osmJoinWays(groups.way, graph);
var newWayMembers = [];
for (i = 0; i < joined.length; i++) {
var segment = joined[i];
var nodes = segment.nodes.slice();
for (j = 0; j < segment.length; j++) {
var way = graph.entity(segment[j].id);
if (tempWay && segment[j].id === tempWay.id) {
if (nodes[0].id === insertPair.nodes[0]) {
newWayMembers.push({ id: insertPair.originalID, type: 'way', role: segment[j].role });
newWayMembers.push({ id: insertPair.insertedID, type: 'way', role: segment[j].role });
} else {
newWayMembers.push({ id: insertPair.insertedID, type: 'way', role: segment[j].role });
newWayMembers.push({ id: insertPair.originalID, type: 'way', role: segment[j].role });
}
} else {
newWayMembers.push(segment[j]);
}
nodes.splice(0, way.nodes.length - 1);
}
}
if (tempWay) {
graph = graph.remove(tempWay);
}
// Write members in the order: nodes, ways, relations
// This is reccomended for Public Transport routes:
// see https://wiki.openstreetmap.org/wiki/Public_transport#Service_routes
var newMembers = (groups.node || []).concat(newWayMembers, (groups.relation || []));
return graph.replace(relation.update({members: newMembers}));
}
return action;
}
+6 -4
View File
@@ -85,6 +85,7 @@ export function actionSplit(nodeId, newWayIds) {
function split(graph, wayA, newWayId) {
var wayB = osmWay({id: newWayId, tags: wayA.tags});
var origNodes = wayA.nodes.slice();
var nodesA;
var nodesB;
var isArea = wayA.isArea();
@@ -134,12 +135,13 @@ export function actionSplit(nodeId, newWayIds) {
role: relation.memberById(wayA.id).role
};
var insertHint = {
item: member,
nextTo: wayA.id
var insertPair = {
originalID: wayA.id,
insertedID: wayB.id,
nodes: origNodes
};
graph = actionAddMember(relation.id, member, undefined, insertHint)(graph);
graph = actionAddMember(relation.id, member, undefined, insertPair)(graph);
}
});
+12 -35
View File
@@ -88,12 +88,7 @@ export function osmSimpleMultipolygonOuterMember(entity, graph) {
// Incomplete members (those for which `graph.hasEntity(element.id)` returns
// false) and non-way members are ignored.
//
// `insertHint` is an optional object.
// If supplied, insert the given way/member next to an existing way/member:
// `{ item: wayOrMember, nextTo: id }`
// (This feature is used by `actionSplit`)
//
export function osmJoinWays(toJoin, graph, insertHint) {
export function osmJoinWays(toJoin, graph) {
function resolve(member) {
return graph.childNodes(graph.entity(member.id));
}
@@ -109,6 +104,7 @@ export function osmJoinWays(toJoin, graph, insertHint) {
return member.type === 'way' && graph.hasEntity(member.id);
});
var sequences = [];
sequences.actions = [];
@@ -118,7 +114,6 @@ export function osmJoinWays(toJoin, graph, insertHint) {
var currWays = [item];
var currNodes = resolve(item).slice();
var doneSequence = false;
var isInserting = false;
// add to it
while (toJoin.length && !doneSequence) {
@@ -129,19 +124,8 @@ export function osmJoinWays(toJoin, graph, insertHint) {
var i;
// Find the next way/member to join.
// If it is time to attempt an insert, try that item first.
// Otherwise, search for a next item in `toJoin`
var toCheck;
if (!isInserting && insertHint && insertHint.nextTo === currWays[currWays.length - 1].id) {
toCheck = [insertHint.item];
isInserting = true;
} else {
toCheck = toJoin.slice();
isInserting = false;
}
for (i = 0; i < toCheck.length; i++) {
item = toCheck[i];
for (i = 0; i < toJoin.length; i++) {
item = toJoin[i];
nodes = resolve(item);
// Strongly prefer to generate a forward path that preserves the order
@@ -181,22 +165,15 @@ export function osmJoinWays(toJoin, graph, insertHint) {
}
}
if (nodes) { // we found something to join
fn.apply(currWays, [item]);
fn.apply(currNodes, nodes);
if (!isInserting) {
toJoin.splice(i, 1);
}
} else { // couldn't find a joinable way/member
// if inserting, restart the loop and look in `toJoin` next time.
// if not inserting, there is nowhere else to look, mark as done.
if (!isInserting) {
doneSequence = true;
break;
}
if (!nodes) { // couldn't find a joinable way/member
doneSequence = true;
break;
}
fn.apply(currWays, [item]);
fn.apply(currNodes, nodes);
toJoin.splice(i, 1);
}
currWays.nodes = currNodes;
+18 -10
View File
@@ -101,7 +101,7 @@ describe('iD.actionAddMember', function() {
expect(members(graph)).to.eql(['-', '=', '~']);
});
it('inserts the member multiple times if hint provided (middle)', function() {
it('inserts the member multiple times if insertPair provided (middle)', function() {
// Before: a ---> b .. c ~~~> d <~~~ c .. b <--- a
// After: a ---> b ===> c ~~~> d <~~~ c <=== b <--- a
var graph = iD.coreGraph([
@@ -121,21 +121,25 @@ describe('iD.actionAddMember', function() {
]);
var member = { id: '=', type: 'way' };
var hint = { item: member, nextTo: '-' };
graph = iD.actionAddMember('r', member, undefined, hint)(graph);
var insertPair = {
originalID: '-',
insertedID: '=',
nodes: ['a','b','c']
};
graph = iD.actionAddMember('r', member, undefined, insertPair)(graph);
expect(members(graph)).to.eql(['-', '=', '~', '~', '=', '-']);
});
it('inserts the member multiple times if hint provided (beginning/end)', function() {
// Before: b ===> c ~~~> d <~~~ c <=== b
// After: a ---> b ===> c ~~~> d <~~~ c <=== b <--- a
it('inserts the member multiple times if insertPair provided (beginning/end)', function() {
// Before: b <=== c ~~~> d <~~~ c ===> b
// After: a <--- b <=== c ~~~> d <~~~ c ===> b ---> a
var graph = iD.coreGraph([
iD.osmNode({id: 'a', loc: [0, 0]}),
iD.osmNode({id: 'b', loc: [0, 0]}),
iD.osmNode({id: 'c', loc: [0, 0]}),
iD.osmNode({id: 'd', loc: [0, 0]}),
iD.osmWay({id: '-', nodes: ['a', 'b']}),
iD.osmWay({id: '=', nodes: ['b', 'c']}),
iD.osmWay({id: '-', nodes: ['b', 'a']}),
iD.osmWay({id: '=', nodes: ['c', 'b']}),
iD.osmWay({id: '~', nodes: ['c', 'd']}),
iD.osmRelation({id: 'r', members: [
{id: '=', type: 'way'},
@@ -146,8 +150,12 @@ describe('iD.actionAddMember', function() {
]);
var member = { id: '-', type: 'way' };
var hint = { item: member, nextTo: '=' };
graph = iD.actionAddMember('r', member, undefined, hint)(graph);
var insertPair = {
originalID: '=',
insertedID: '-',
nodes: ['c','b','a']
};
graph = iD.actionAddMember('r', member, undefined, insertPair)(graph);
expect(members(graph)).to.eql(['-', '=', '~', '~', '=', '-']);
});