From 03fa6e7be915156bbe2a6bb5544e288a0c376df0 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 15 Jan 2018 22:02:43 -0500 Subject: [PATCH] Add tryInsert option to osmJoinWays --- modules/actions/add_member.js | 13 +++++- modules/actions/split.js | 56 +++++++++++++----------- modules/osm/multipolygon.js | 81 ++++++++++++++++++++++------------- test/spec/actions/split.js | 61 +++++++++++++++++++------- 4 files changed, 142 insertions(+), 69 deletions(-) diff --git a/modules/actions/add_member.js b/modules/actions/add_member.js index 195dd7df9..3d7654d36 100644 --- a/modules/actions/add_member.js +++ b/modules/actions/add_member.js @@ -4,12 +4,16 @@ import { osmJoinWays } from '../osm'; export function actionAddMember(relationId, member, memberIndex) { return 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) && member.type === 'way') { var members = relation.indexedMembers(); members.push(member); var joined = osmJoinWays(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++) { @@ -23,10 +27,17 @@ export function actionAddMember(relationId, member, memberIndex) { } else { memberIndex = Math.min(segment[j - 1].index + 1, segment[j + 1].index + 1); } + + relation = relation.addMember(member, memberIndex + (numAdded++)); } } } - return 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); }; } diff --git a/modules/actions/split.js b/modules/actions/split.js index 6fd73f0bc..e0bd8b20f 100644 --- a/modules/actions/split.js +++ b/modules/actions/split.js @@ -29,7 +29,7 @@ import { utilWrap } from '../util'; // https://github.com/systemed/potlatch2/blob/master/net/systemeD/halcyon/connection/actions/SplitWayAction.as // export function actionSplit(nodeId, newWayIds) { - var wayIds; + var _wayIDs; // if the way is closed, we need to search for a partner node // to split the way at. @@ -42,11 +42,11 @@ export function actionSplit(nodeId, newWayIds) { // For example: bone-shaped areas get split across their waist // line, circles across the diameter. function splitArea(nodes, idxA, graph) { - var lengths = new Array(nodes.length), - length, - i, - best = 0, - idxB; + var lengths = new Array(nodes.length); + var length; + var i; + var best = 0; + var idxB; function wrap(index) { return utilWrap(index, nodes.length); @@ -84,16 +84,16 @@ export function actionSplit(nodeId, newWayIds) { function split(graph, wayA, newWayId) { - var wayB = osmWay({id: newWayId, tags: wayA.tags}), - nodesA, - nodesB, - isArea = wayA.isArea(), - isOuter = osmIsSimpleMultipolygonOuterMember(wayA, graph); + var wayB = osmWay({id: newWayId, tags: wayA.tags}); + var nodesA; + var nodesB; + var isArea = wayA.isArea(); + var isOuter = osmIsSimpleMultipolygonOuterMember(wayA, graph); if (wayA.isClosed()) { - var nodes = wayA.nodes.slice(0, -1), - idxA = _indexOf(nodes, nodeId), - idxB = splitArea(nodes, idxA, graph); + var nodes = wayA.nodes.slice(0, -1); + var idxA = _indexOf(nodes, nodeId); + var idxB = splitArea(nodes, idxA, graph); if (idxB < idxA) { nodesA = nodes.slice(idxA).concat(nodes.slice(0, idxB + 1)); @@ -134,7 +134,14 @@ export function actionSplit(nodeId, newWayIds) { role: relation.memberById(wayA.id).role }; - graph = actionAddMember(relation.id, member)(graph); + // how many times should this member be inserted? + // var matches = relation.members.filter(function(member) { + // return member.id === wayA.id; + // }); + + // matches.forEach(function() { + graph = actionAddMember(relation.id, member)(graph); + // }); } }); @@ -144,7 +151,8 @@ export function actionSplit(nodeId, newWayIds) { members: [ {id: wayA.id, role: 'outer', type: 'way'}, {id: wayB.id, role: 'outer', type: 'way'} - ]}); + ] + }); graph = graph.replace(multipolygon); graph = graph.replace(wayA.update({tags: {}})); @@ -165,15 +173,15 @@ export function actionSplit(nodeId, newWayIds) { action.ways = function(graph) { - var node = graph.entity(nodeId), - parents = graph.parentWays(node), - hasLines = _some(parents, function(parent) { return parent.geometry(graph) === 'line'; }); + var node = graph.entity(nodeId); + var parents = graph.parentWays(node); + var hasLines = _some(parents, function(parent) { return parent.geometry(graph) === 'line'; }); return parents.filter(function(parent) { - if (wayIds && wayIds.indexOf(parent.id) === -1) + if (_wayIDs && _wayIDs.indexOf(parent.id) === -1) return false; - if (!wayIds && hasLines && parent.geometry(graph) !== 'line') + if (!_wayIDs && hasLines && parent.geometry(graph) !== 'line') return false; if (parent.isClosed()) { @@ -193,14 +201,14 @@ export function actionSplit(nodeId, newWayIds) { action.disabled = function(graph) { var candidates = action.ways(graph); - if (candidates.length === 0 || (wayIds && wayIds.length !== candidates.length)) + if (candidates.length === 0 || (_wayIDs && _wayIDs.length !== candidates.length)) return 'not_eligible'; }; action.limitWays = function(_) { - if (!arguments.length) return wayIds; - wayIds = _; + if (!arguments.length) return _wayIDs; + _wayIDs = _; return action; }; diff --git a/modules/osm/multipolygon.js b/modules/osm/multipolygon.js index 322626df8..c8412fecf 100644 --- a/modules/osm/multipolygon.js +++ b/modules/osm/multipolygon.js @@ -75,31 +75,36 @@ export function osmSimpleMultipolygonOuterMember(entity, graph) { // ordered array of member nodes, with appropriate order reversal and // start/end coordinate de-duplication. // +// Members of `toJoin` must have, at minimum, `type` and `id` properties. +// Thus either an array of `osmWay`s or a relation member array may be used. +// +// If an member is an `osmWay`, its tags and childnodes may be reversed via +// `actionReverse` in the output. +// // The returned sequences array also has an `actions` array property, containing // any reversal actions that should be applied to the graph, should the calling // code attempt to actually join the given ways. // -// Members of `toJoin` must have, at minimum, `type` and `id` properties. -// Thus either an array of `osmWay`s or a relation member array may be used. -// -// If an member is an `osmWay`, its tags and childnodes will be reversed via -// `actionReverse` in the output. -// // Incomplete members (those for which `graph.hasEntity(element.id)` returns // false) and non-way members are ignored. // -export function osmJoinWays(toJoin, graph) { +// `tryInsert` is an optional object. +// If supplied, insert the given way/member after an existing way/member: +// `{ item: wayOrMember, afterID: id }` +// (This feature is used by `actionSplit`) +// +export function osmJoinWays(toJoin, graph, tryInsert) { function resolve(member) { return graph.childNodes(graph.entity(member.id)); } - function reverse(which) { - var action = actionReverse(which.id, { reverseOneway: true }); + function reverse(item) { + var action = actionReverse(item.id, { reverseOneway: true }); sequences.actions.push(action); - return (which instanceof osmWay) ? action(graph).entity(which.id) : which; + return (item instanceof osmWay) ? action(graph).entity(item.id) : item; } - // make a copy containing only the ways to join + // make a copy containing only the items to join toJoin = toJoin.filter(function(member) { return member.type === 'way' && graph.hasEntity(member.id); }); @@ -109,10 +114,11 @@ export function osmJoinWays(toJoin, graph) { while (toJoin.length) { // start a new sequence - var way = toJoin.shift(); - var currWays = [way]; - var currNodes = resolve(way).slice(); + var item = toJoin.shift(); + var currWays = [item]; + var currNodes = resolve(item).slice(); var doneSequence = false; + var isInserting = false; // add to it while (toJoin.length && !doneSequence) { @@ -122,10 +128,21 @@ export function osmJoinWays(toJoin, graph) { var nodes = null; var i; - // find the next way - for (i = 0; i < toJoin.length; i++) { - way = toJoin[i]; - nodes = resolve(way); + // 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 && tryInsert && tryInsert.afterID === currWays[currWays.length - 1].id) { + toCheck = [tryInsert.item]; + isInserting = true; + } else { + toCheck = toJoin.slice(); + isInserting = false; + } + + for (i = 0; i < toCheck.length; i++) { + item = toCheck[i]; + nodes = resolve(item); // Strongly prefer to generate a forward path that preserves the order // of the members array. For multipolygons and most relations, member @@ -148,7 +165,7 @@ export function osmJoinWays(toJoin, graph) { } else if (nodes[nodes.length - 1] === end) { fn = currNodes.push; // join to end nodes = nodes.slice(0, -1).reverse(); - way = reverse(way); + item = reverse(item); break; } else if (nodes[nodes.length - 1] === start) { fn = currNodes.unshift; // join to beginning @@ -157,25 +174,31 @@ export function osmJoinWays(toJoin, graph) { } else if (nodes[0] === start) { fn = currNodes.unshift; // join to beginning nodes = nodes.slice(1).reverse(); - way = reverse(way); + item = reverse(item); break; } else { fn = nodes = null; } } - if (!nodes) { - doneSequence = true; // couldn't find a joinable way - break; + 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; + } } - - fn.apply(currWays, [way]); - fn.apply(currNodes, nodes); - - toJoin.splice(i, 1); } - currWays.nodes = currNodes; sequences.push(currWays); } diff --git a/test/spec/actions/split.js b/test/spec/actions/split.js index cc5fd31c4..1ebbcf89c 100644 --- a/test/spec/actions/split.js +++ b/test/spec/actions/split.js @@ -183,7 +183,7 @@ describe('iD.actionSplit', function () { iD.osmNode({id: 'a'}), iD.osmNode({id: 'b'}), iD.osmNode({id: 'c'}), - iD.osmNode({id: 'c'}), + iD.osmNode({id: 'd'}), iD.osmNode({id: '*'}), iD.osmWay({id: '-', nodes: ['a', '*', 'b']}), iD.osmWay({id: '|', nodes: ['c', '*', 'd']}) @@ -202,7 +202,7 @@ describe('iD.actionSplit', function () { iD.osmNode({id: 'a'}), iD.osmNode({id: 'b'}), iD.osmNode({id: 'c'}), - iD.osmNode({id: 'c'}), + iD.osmNode({id: 'd'}), iD.osmNode({id: '*'}), iD.osmWay({id: '-', nodes: ['a', '*', 'b']}), iD.osmWay({id: '|', nodes: ['c', '*', 'd']}) @@ -244,7 +244,7 @@ describe('iD.actionSplit', function () { iD.osmNode({id: 'a'}), iD.osmNode({id: 'b'}), iD.osmNode({id: 'c'}), - iD.osmNode({id: 'c'}), + iD.osmNode({id: 'd'}), iD.osmWay({id: '-', nodes: ['a', 'b', 'c', 'a', 'd']}) ]); @@ -314,15 +314,15 @@ describe('iD.actionSplit', function () { describe('member ordering', function () { - it('adds the new way to parent relations (no connections)', function () { + it('adds the new way to parent relations (simple)', function () { // Situation: - // a ---- b ---- c + // a ----> b ----> c // Relation: [----] // // Split at b. // // Expected result: - // a ---- b ==== c + // a ----> b ====> c // Relation: [----, ====] // var graph = iD.coreGraph([ @@ -343,13 +343,13 @@ describe('iD.actionSplit', function () { it('adds the new way to parent relations (forward order)', function () { // Situation: - // a ---- b ---- c ~~~~ d + // a ----> b ----> c ~~~~> d // Relation: [----, ~~~~] // // Split at b. // // Expected result: - // a ---- b ==== c ~~~~ d + // a ----> b ====> c ~~~~> d // Relation: [----, ====, ~~~~] // var graph = iD.coreGraph([ @@ -370,13 +370,13 @@ describe('iD.actionSplit', function () { it('adds the new way to parent relations (reverse order)', function () { // Situation: - // a ---- b ---- c ~~~~ d + // a ----> b ----> c ~~~~> d // Relation: [~~~~, ----] // // Split at b. // // Expected result: - // a ---- b ==== c ~~~~ d + // a ----> b ====> c ~~~~> d // Relation: [~~~~, ====, ----] // var graph = iD.coreGraph([ @@ -397,13 +397,13 @@ describe('iD.actionSplit', function () { it('adds the new way to parent relations (unsplit way belongs multiple times)', function () { // Situation: - // a ---- b ---- c ~~~~ d + // a ----> b ----> c ~~~~> d // Relation: [~~~~, ----, ~~~~] // // Split at b. // // Expected result: - // a ---- b ==== c ~~~~ d + // a ----> b ====> c ~~~~> d // Relation: [~~~~, ====, ----, ====, ~~~~] // var graph = iD.coreGraph([ @@ -426,15 +426,15 @@ describe('iD.actionSplit', function () { expect(ids).to.have.ordered.members(['~', '=', '-', '=', '~']); }); - it('adds the new way to parent relations (split way belongs multiple times)', function () { + it('adds the new way to parent relations (forward split way belongs multiple times)', function () { // Situation: - // a ---- b ---- c ~~~~ d + // a ----> b ----> c ~~~~> d // Relation: [----, ~~~~, ----] // // Split at b. // // Expected result: - // a ---- b ==== c ~~~~ d + // a ----> b ====> c ~~~~> d // Relation: [----, ====, ~~~~, ====, ----] // var graph = iD.coreGraph([ @@ -456,6 +456,37 @@ describe('iD.actionSplit', function () { var ids = graph.entity('r').members.map(function(m) { return m.id; }); expect(ids).to.have.ordered.members(['-', '=', '~', '=', '-']); }); + + it('adds the new way to parent relations (reverse split way belongs multiple times)', function () { + // Situation: + // a <---- b <---- c ~~~~> d + // Relation: [----, ~~~~, ----] + // + // Split at b. + // + // Expected result: + // a <==== b <---- c ~~~~> d + // Relation: [====, ----, ~~~~, ----, ====] + // + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmNode({id: 'd'}), + iD.osmWay({id: '-', nodes: ['c', 'b', 'a']}), + iD.osmWay({id: '~', nodes: ['c', 'd']}), + iD.osmRelation({id: 'r', members: [ + {id: '-', type: 'way'}, + {id: '~', type: 'way'}, + {id: '-', type: 'way'} + ]}) + ]); + + graph = iD.actionSplit('b', ['='])(graph); + + var ids = graph.entity('r').members.map(function(m) { return m.id; }); + expect(ids).to.have.ordered.members(['=', '-', '~', '-', '=']); + }); });