From e05531ad6c28c0c586c5cbb5992b9ef464459c5e Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Fri, 8 Feb 2013 11:46:39 -0800 Subject: [PATCH] Fix circularize boundary cases (fixes #494) There are still more boundary cases even farther out. But this is probably sufficient for the real world. --- js/id/actions/circularize.js | 70 +++++++++++++++++--------------- test/index.html | 1 + test/index_packaged.html | 1 + test/spec/actions/circularize.js | 61 ++++++++++++++++++++++++++++ 4 files changed, 100 insertions(+), 33 deletions(-) create mode 100644 test/spec/actions/circularize.js diff --git a/js/id/actions/circularize.js b/js/id/actions/circularize.js index 532dcdb45..b74dfd976 100644 --- a/js/id/actions/circularize.js +++ b/js/id/actions/circularize.js @@ -1,52 +1,56 @@ -iD.actions.Circularize = function(wayId, projection) { +iD.actions.Circularize = function(wayId, projection, count) { + count = count || 12; + + function closestIndex(nodes, loc) { + var idx, min = Infinity, dist; + for (var i = 0; i < nodes.length; i++) { + dist = iD.geo.dist(nodes[i].loc, loc); + if (dist < min) { + min = dist; + idx = i; + } + } + return idx; + } var action = function(graph) { var way = graph.entity(wayId), - nodes = _.uniq(graph.childNodes(way)); - - var points = nodes.map(function(n) { - return projection(n.loc); - }), + nodes = _.uniq(graph.childNodes(way)), + points = nodes.map(function(n) { return projection(n.loc); }), centroid = d3.geom.polygon(points).centroid(), radius = d3.median(points, function(p) { return iD.geo.dist(centroid, p); }), - circular_nodes = []; + ids = []; - for (var i = 0; i < 12; i++) { - circular_nodes.push(iD.Node({ loc: projection.invert([ - centroid[0] + Math.cos((i / 12) * Math.PI * 2) * radius, - centroid[1] + Math.sin((i / 12) * Math.PI * 2) * radius]) - })); - } + for (var i = 0; i < count; i++) { + var node, + loc = projection.invert([ + centroid[0] + Math.cos((i / 12) * Math.PI * 2) * radius, + centroid[1] + Math.sin((i / 12) * Math.PI * 2) * radius]); - for (i = 0; i < nodes.length; i++) { - if (graph.parentWays(nodes[i]).length > 1) { - var closest, closest_dist = Infinity, dist; - for (var j = 0; j < circular_nodes.length; j++) { - dist = iD.geo.dist(circular_nodes[j].loc, nodes[i].loc); - if (dist < closest_dist) { - closest_dist = dist; - closest = j; - } - } - circular_nodes.splice(closest, 1, nodes[i]); + if (nodes.length) { + var idx = closestIndex(nodes, loc); + node = nodes[idx]; + nodes.splice(idx, 1); + } else { + node = iD.Node(); } - } - for (i = 0; i < circular_nodes.length; i++) { - graph = graph.replace(circular_nodes[i]); + ids.push(node.id); + graph = graph.replace(node.move(loc)); } - var ids = _.pluck(circular_nodes, 'id'), - difference = _.difference(_.uniq(way.nodes), ids); - ids.push(ids[0]); - graph = graph.replace(way.update({nodes: ids})); - for (i = 0; i < difference.length; i++) { - graph = iD.actions.DeleteNode(difference[i])(graph); + for (i = 0; i < nodes.length; i++) { + graph.parentWays(nodes[i]).forEach(function(parent) { + graph = graph.replace(parent.replaceNode(nodes[i].id, + ids[closestIndex(graph.childNodes(way), nodes[i].loc)])); + }); + + graph = iD.actions.DeleteNode(nodes[i].id)(graph); } return graph; diff --git a/test/index.html b/test/index.html index 8fb7da652..ca610d714 100644 --- a/test/index.html +++ b/test/index.html @@ -146,6 +146,7 @@ + diff --git a/test/index_packaged.html b/test/index_packaged.html index 778231edd..1102336d9 100644 --- a/test/index_packaged.html +++ b/test/index_packaged.html @@ -26,6 +26,7 @@ + diff --git a/test/spec/actions/circularize.js b/test/spec/actions/circularize.js new file mode 100644 index 000000000..8215c2253 --- /dev/null +++ b/test/spec/actions/circularize.js @@ -0,0 +1,61 @@ +describe("iD.actions.Circularize", function () { + var projection = d3.geo.mercator(); + + it("creates a circle of 12 nodes", function () { + var graph = iD.Graph({ + 'a': iD.Node({id: 'a', loc: [0, 0]}), + 'b': iD.Node({id: 'b', loc: [2, 0]}), + 'c': iD.Node({id: 'c', loc: [2, 2]}), + 'd': iD.Node({id: 'd', loc: [0, 2]}), + '-': iD.Way({id: '-', nodes: ['a', 'b', 'c', 'd', 'a']}) + }); + + graph = iD.actions.Circularize('-', projection)(graph); + + expect(graph.entity('-').nodes).to.have.length(13); + }); + + it("reuses existing nodes", function () { + var graph = iD.Graph({ + 'a': iD.Node({id: 'a', loc: [0, 0]}), + 'b': iD.Node({id: 'b', loc: [2, 0]}), + 'c': iD.Node({id: 'c', loc: [2, 2]}), + 'd': iD.Node({id: 'd', loc: [0, 2]}), + '-': iD.Way({id: '-', nodes: ['a', 'b', 'c', 'd', 'a']}) + }); + + graph = iD.actions.Circularize('-', projection)(graph); + + expect(graph.entity('-').nodes.slice(0, 4)).to.eql(['c', 'b', 'a', 'd']); + }); + + it("deletes unused nodes that are not members of other ways", function () { + var graph = iD.Graph({ + 'a': iD.Node({id: 'a', loc: [0, 0]}), + 'b': iD.Node({id: 'b', loc: [2, 0]}), + 'c': iD.Node({id: 'c', loc: [2, 2]}), + 'd': iD.Node({id: 'd', loc: [0, 2]}), + '-': iD.Way({id: '-', nodes: ['a', 'b', 'c', 'd', 'a']}) + }); + + graph = iD.actions.Circularize('-', projection, 3)(graph); + + expect(graph.entity('d')).to.be.undefined; + }); + + it("reconnects unused nodes that are members of other ways", function () { + var graph = iD.Graph({ + 'a': iD.Node({id: 'a', loc: [0, 0]}), + 'b': iD.Node({id: 'b', loc: [2, 0]}), + 'c': iD.Node({id: 'c', loc: [2, 2]}), + 'd': iD.Node({id: 'd', loc: [0, 2]}), + '-': iD.Way({id: '-', nodes: ['a', 'b', 'c', 'd', 'a']}), + '=': iD.Way({id: '=', nodes: ['d']}) + }); + + graph = iD.actions.Circularize('-', projection, 3)(graph); + + expect(graph.entity('d')).to.be.undefined; + expect(graph.entity('=').nodes).to.eql(['c']); + }); +});