From 33cf029d43abc27e57b28f5aaf4fd90952316d1a Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 5 Jun 2013 12:37:09 -0700 Subject: [PATCH] Extract and refactor iD.geo.joinMemberWays --- js/id/actions/merge_polygon.js | 20 ++++----- js/id/core/relation.js | 80 +++++----------------------------- js/id/geo/multipolygon.js | 71 ++++++++++++++++++++++++++++++ test/spec/core/relation.js | 8 ++-- test/spec/geo/multipolygon.js | 50 +++++++++++++++++++++ 5 files changed, 144 insertions(+), 85 deletions(-) diff --git a/js/id/actions/merge_polygon.js b/js/id/actions/merge_polygon.js index a771d7c1f..6f7847ce3 100644 --- a/js/id/actions/merge_polygon.js +++ b/js/id/actions/merge_polygon.js @@ -20,18 +20,16 @@ iD.actions.MergePolygon = function(ids, newRelationId) { var action = function(graph) { var entities = groupEntities(graph); - // An array of objects representing all the polygons that are part of the multipolygon. + // An array representing all the polygons that are part of the multipolygon. // - // Each object has two properties: - // ids - an array of ids of entities that are part of that polygon - // locs - an array of the locations forming the polygon + // 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(m.joinMemberWays(null, graph)); + return polygons.concat(iD.geo.joinMemberWays(m.members, graph)); }, []).concat(entities.closedWay.map(function(d) { - return { - ids: [d.id], - locs: graph.childNodes(d).map(function(n) { return n.loc; }) - }; + var member = [{id: d.id}]; + member.locs = graph.childNodes(d).map(function(n) { return n.loc; }); + return member; })); // contained is an array of arrays of boolean values, @@ -65,10 +63,10 @@ iD.actions.MergePolygon = function(ids, newRelationId) { function extractUncontained(polygons) { polygons.forEach(function(d, i) { if (!isContained(d, i)) { - d.ids.forEach(function(id) { + d.forEach(function(member) { members.push({ type: 'way', - id: id, + id: member.id, role: outer ? 'outer' : 'inner' }); }); diff --git a/js/id/core/relation.js b/js/id/core/relation.js index cc84e25c0..1a174d9f4 100644 --- a/js/id/core/relation.js +++ b/js/id/core/relation.js @@ -172,9 +172,16 @@ _.extend(iD.Relation.prototype, { // rings not matched with the intended outer ring. // multipolygon: function(resolver) { - var members = this.members - .filter(function(m) { return m.type === 'way' && resolver.hasEntity(m.id); }) - .map(function(m) { return { role: m.role || 'outer', id: m.id, nodes: resolver.childNodes(resolver.entity(m.id)) }; }); + 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 = _.pluck(outers, 'locs'); + inners = _.pluck(inners, 'locs'); + + var result = outers.map(function(o) { return [o]; }); function findOuter(inner) { var o, outer; @@ -192,10 +199,6 @@ _.extend(iD.Relation.prototype, { } } - var outers = _.pluck(this.joinMemberWays(members.filter(function(m) { return m.role === 'outer'; })), 'locs'), - inners = _.pluck(this.joinMemberWays(members.filter(function(m) { return m.role === 'inner'; })), 'locs'), - result = outers.map(function(o) { return [o]; }); - for (var i = 0; i < inners.length; i++) { var o = findOuter(inners[i]); if (o !== undefined) @@ -205,68 +208,5 @@ _.extend(iD.Relation.prototype, { } return result; - }, - - joinMemberWays: function(ways, resolver) { - var joined = [], way, current, first, last, i, how, what; - - ways = ways || this.members.filter(function(m) { - return m.type === 'way'; - }).map(function(m) { - return { - id: m.id, - nodes: resolver.childNodes(resolver.entity(m.id)) - }; - }); - - while (ways.length) { - way = ways.pop(); - current = way.nodes.slice(); - current.ids = [way.id]; - joined.push(current); - - while (ways.length && _.first(current) !== _.last(current)) { - first = _.first(current); - last = _.last(current); - - for (i = 0; i < ways.length; i++) { - what = ways[i].nodes; - - if (last === _.first(what)) { - how = current.push; - what = what.slice(1); - break; - } else if (last === _.last(what)) { - how = current.push; - what = what.slice(0, -1).reverse(); - break; - } else if (first == _.last(what)) { - how = current.unshift; - what = what.slice(0, -1); - break; - } else if (first == _.first(what)) { - how = current.unshift; - what = what.slice(1).reverse(); - break; - } else { - what = how = null; - } - } - - if (!what) - break; // Invalid geometry (unclosed ring) - - current.ids.push(ways[i].id); - ways.splice(i, 1); - how.apply(current, what); - } - } - return joined.map(function(nodes) { - return { - ids: nodes.ids, - locs: _.pluck(nodes, 'loc') - }; - }); } - }); diff --git a/js/id/geo/multipolygon.js b/js/id/geo/multipolygon.js index df43ae8f0..a95d536e2 100644 --- a/js/id/geo/multipolygon.js +++ b/js/id/geo/multipolygon.js @@ -48,3 +48,74 @@ iD.geo.simpleMultipolygonOuterMember = function(entity, graph) { return outerMember && graph.hasEntity(outerMember.id); }; + +// Join an array of relation `members` into sequences of connecting segments. +// +// 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, +// with appropriate order reversal and start/end coordinate de-duplication. +// +// Incomplete members are ignored. +// +iD.geo.joinMemberWays = function(members, graph) { + var joined = [], member, current, locs, first, last, i, how, what; + + members = members.filter(function(member) { + return member.type === 'way' && graph.hasEntity(member.id); + }); + + function resolve(member) { + return _.pluck(graph.childNodes(graph.entity(member.id)), 'loc'); + } + + while (members.length) { + member = members.pop(); + current = [member]; + current.locs = locs = resolve(member); + joined.push(current); + + while (members.length && _.first(locs) !== _.last(locs)) { + first = _.first(locs); + last = _.last(locs); + + for (i = 0; i < members.length; i++) { + member = members[i]; + what = resolve(member); + + if (last === _.first(what)) { + how = locs.push; + what = what.slice(1); + break; + } else if (last === _.last(what)) { + how = locs.push; + what = what.slice(0, -1).reverse(); + break; + } else if (first === _.last(what)) { + how = locs.unshift; + what = what.slice(0, -1); + break; + } else if (first === _.first(what)) { + how = locs.unshift; + what = what.slice(1).reverse(); + break; + } else { + what = how = null; + } + } + + if (!what) + break; // No more joinable ways. + + how.apply(current, [member]); + how.apply(locs, what); + + members.splice(i, 1); + } + } + + return joined; +}; diff --git a/test/spec/core/relation.js b/test/spec/core/relation.js index 23e549b0d..1849834db 100644 --- a/test/spec/core/relation.js +++ b/test/spec/core/relation.js @@ -318,10 +318,10 @@ describe('iD.Relation', function () { }); specify("invalid geometry: unclosed ring consisting of multiple ways, one needing reversal, alternate order", function () { - var a = iD.Node(), - b = iD.Node(), - c = iD.Node(), - d = iD.Node(), + var a = iD.Node({loc: [1, 1]}), + b = iD.Node({loc: [2, 2]}), + c = iD.Node({loc: [3, 3]}), + d = iD.Node({loc: [4, 4]}), w1 = iD.Way({nodes: [c.id, d.id]}), w2 = iD.Way({nodes: [c.id, b.id, a.id]}), r = iD.Relation({members: [{id: w2.id, type: 'way'}, {id: w1.id, type: 'way'}]}), diff --git a/test/spec/geo/multipolygon.js b/test/spec/geo/multipolygon.js index 19ac5559f..4db4839e8 100644 --- a/test/spec/geo/multipolygon.js +++ b/test/spec/geo/multipolygon.js @@ -39,3 +39,53 @@ describe("iD.geo.simpleMultipolygonOuterMember", function() { expect(iD.geo.simpleMultipolygonOuterMember(way, graph)).to.be.undefined; }); }); + +describe("iD.geo.joinMemberWays", function() { + it("returns an array of members with locs 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); + + 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].length).to.equal(1); + expect(result[0][0]).to.equal(member); + }); + + it("returns the members in the correct order", function() { + // a<===b--->c~~~>d + var graph = iD.Graph({ + 'a': iD.Node({id: 'a', loc: [0, 0]}), + 'b': iD.Node({id: 'b', loc: [0, 0]}), + 'c': iD.Node({id: 'c', loc: [0, 0]}), + 'd': iD.Node({id: 'd', loc: [0, 0]}), + '=': iD.Way({id: '=', nodes: ['b', 'a']}), + '-': iD.Way({id: '-', nodes: ['b', 'c']}), + '~': iD.Way({id: '~', nodes: ['c', 'd']}), + 'r': iD.Relation({id: 'r', members: [ + {id: '-', type: 'way'}, + {id: '~', type: 'way'}, + {id: '=', type: 'way'} + ]}) + }); + + var result = iD.geo.joinMemberWays(graph.entity('r').members, graph); + expect(_.pluck(result[0], 'id')).to.eql(['=', '-', '~']); + }); + + 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([]); + }); + + it("ignores incomplete members", function() { + var member = {id: 'w', type: 'way'}, + graph = iD.Graph(); + expect(iD.geo.joinMemberWays([member], graph)).to.eql([]); + }); +});