From b07cf7c1c1da766d62696e7b30b4c2ddd2ebcbb3 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Sat, 14 Jun 2014 01:33:20 -0400 Subject: [PATCH 1/4] Always draw midpoint handles within viewport (like #1840) --- js/id/geo.js | 33 +++++++++++++++++++++++++++++++++ js/id/renderer/map.js | 12 ++++++++---- js/id/svg/midpoints.js | 39 ++++++++++++++++++++++++++++++--------- 3 files changed, 71 insertions(+), 13 deletions(-) diff --git a/js/id/geo.js b/js/id/geo.js index 8978828b0..2137cdc35 100644 --- a/js/id/geo.js +++ b/js/id/geo.js @@ -87,6 +87,39 @@ iD.geo.chooseEdge = function(nodes, point, projection) { }; }; +// Return the intersection point of 2 line segments. +// From https://github.com/pgkelley4/line-segments-intersect +// This uses the vector cross product approach described below: +// http://stackoverflow.com/a/565282/786339 +iD.geo.lineIntersection = function(a, b) { + function subtractPoints(point1, point2) { + return [point1[0] - point2[0], point1[1] - point2[1]]; + } + function crossProduct(point1, point2) { + return point1[0] * point2[1] - point1[1] * point2[0]; + } + + var p = [a[0][0], a[0][1]], + p2 = [a[1][0], a[1][1]], + q = [b[0][0], b[0][1]], + q2 = [b[1][0], b[1][1]], + r = subtractPoints(p2, p), + s = subtractPoints(q2, q), + uNumerator = crossProduct(subtractPoints(q, p), r), + denominator = crossProduct(r, s); + + if (uNumerator && denominator) { + var u = uNumerator / denominator, + t = crossProduct(subtractPoints(q, p), s) / denominator; + + if ((t >= 0) && (t <= 1) && (u >= 0) && (u <= 1)) { + return iD.geo.interp(p, p2, t); + } + } + + return null; +} + // Return whether point is contained in polygon. // // `point` should be a 2-item array of coordinates. diff --git a/js/id/renderer/map.js b/js/id/renderer/map.js index 352d33cab..999628785 100644 --- a/js/id/renderer/map.js +++ b/js/id/renderer/map.js @@ -76,10 +76,9 @@ iD.Map = function(context) { if (map.editable() && !transformed) { var all = context.intersects(map.extent()), filter = d3.functor(true), - extent = map.extent(), graph = context.graph(); - surface.call(vertices, graph, all, filter, extent, map.zoom()); - surface.call(midpoints, graph, all, filter, extent); + surface.call(vertices, graph, all, filter, map.extent(), map.zoom()); + surface.call(midpoints, graph, all, filter, map.trimmedExtent()); dispatch.drawn({full: false}); } }); @@ -114,7 +113,7 @@ iD.Map = function(context) { .call(vertices, graph, all, filter, map.extent(), map.zoom()) .call(lines, graph, all, filter) .call(areas, graph, all, filter) - .call(midpoints, graph, all, filter, map.extent()) + .call(midpoints, graph, all, filter, map.trimmedExtent()) .call(labels, graph, all, filter, dimensions, !difference && !extent); if (points.points(context.intersects(map.extent()), 100).length >= 100) { @@ -366,6 +365,11 @@ iD.Map = function(context) { } }; + map.trimmedExtent = function() { + return new iD.geo.Extent(projection.invert([10, dimensions[1] - 40]), + projection.invert([dimensions[0] - 10, 70])); + }; + map.extentZoom = function(_) { var extent = iD.geo.Extent(_), tl = projection([extent[0][0], extent[1][1]]), diff --git a/js/id/svg/midpoints.js b/js/id/svg/midpoints.js index 75cdf0ef7..2d395fbd6 100644 --- a/js/id/svg/midpoints.js +++ b/js/id/svg/midpoints.js @@ -22,15 +22,36 @@ iD.svg.Midpoints = function(projection, context) { if (midpoints[id]) { midpoints[id].parents.push(entity); } else { - var loc = iD.geo.interp(a.loc, b.loc, 0.5); - if (extent.intersects(loc) && iD.geo.euclideanDistance(projection(a.loc), projection(b.loc)) > 40) { - midpoints[id] = { - type: 'midpoint', - id: id, - loc: loc, - edge: [a.id, b.id], - parents: [entity] - }; + if (iD.geo.euclideanDistance(projection(a.loc), projection(b.loc)) > 40) { + var point = iD.geo.interp(a.loc, b.loc, 0.5), + loc; + + if (extent.intersects(point)) { + loc = point; + } else { + var poly = extent.polygon(); + for (var k = 0; k < 4; k++) { + point = iD.geo.lineIntersection([a.loc, b.loc], [poly[k], poly[k+1]]); + if (point && + iD.geo.euclideanDistance(projection(a.loc), projection(point)) > 20 && + iD.geo.euclideanDistance(projection(b.loc), projection(point)) > 20) + { + loc = point; + break; + } + } + } + + if (loc) { + midpoints[id] = { + type: 'midpoint', + id: id, + loc: loc, + edge: [a.id, b.id], + parents: [entity] + }; + } + } } } From 1c61b4b0f2fa2a0697532bbc7dbd5190e768cf15 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 16 Jun 2014 11:11:50 -0400 Subject: [PATCH 2/4] reset loc btw loop iterations (because hoisting) --- js/id/svg/midpoints.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/js/id/svg/midpoints.js b/js/id/svg/midpoints.js index 2d395fbd6..d743565ad 100644 --- a/js/id/svg/midpoints.js +++ b/js/id/svg/midpoints.js @@ -1,6 +1,7 @@ iD.svg.Midpoints = function(projection, context) { return function drawMidpoints(surface, graph, entities, filter, extent) { - var midpoints = {}; + var poly = extent.polygon(), + midpoints = {}; for (var i = 0; i < entities.length; i++) { var entity = entities[i]; @@ -24,12 +25,11 @@ iD.svg.Midpoints = function(projection, context) { } else { if (iD.geo.euclideanDistance(projection(a.loc), projection(b.loc)) > 40) { var point = iD.geo.interp(a.loc, b.loc, 0.5), - loc; + loc = null; if (extent.intersects(point)) { loc = point; } else { - var poly = extent.polygon(); for (var k = 0; k < 4; k++) { point = iD.geo.lineIntersection([a.loc, b.loc], [poly[k], poly[k+1]]); if (point && @@ -51,7 +51,6 @@ iD.svg.Midpoints = function(projection, context) { parents: [entity] }; } - } } } From 1a2b9c82f63563e9da6c49bf8707082f0d79d5a5 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 16 Jun 2014 11:49:07 -0400 Subject: [PATCH 3/4] add tests, jshint cleanup --- js/id/geo.js | 2 +- test/spec/geo.js | 28 ++++++++++++++++++++++++ test/spec/svg/midpoints.js | 44 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 71 insertions(+), 3 deletions(-) diff --git a/js/id/geo.js b/js/id/geo.js index 2137cdc35..7f3eb9457 100644 --- a/js/id/geo.js +++ b/js/id/geo.js @@ -118,7 +118,7 @@ iD.geo.lineIntersection = function(a, b) { } return null; -} +}; // Return whether point is contained in polygon. // diff --git a/test/spec/geo.js b/test/spec/geo.js index a6a86ecb2..289bb8911 100644 --- a/test/spec/geo.js +++ b/test/spec/geo.js @@ -156,6 +156,34 @@ describe('iD.geo', function() { }); }); + describe('.lineIntersection', function() { + it('returns null if lines are colinear with overlap', function() { + var a = [[0, 0], [10, 0]], + b = [[-5, 0], [5, 0]]; + expect(iD.geo.lineIntersection(a, b)).to.be.null; + }); + it('returns null if lines are colinear but disjoint', function() { + var a = [[5, 0], [10, 0]], + b = [[-10, 0], [-5, 0]]; + expect(iD.geo.lineIntersection(a, b)).to.be.null; + }); + it('returns null if lines are parallel', function() { + var a = [[0, 0], [10, 0]], + b = [[0, 5], [10, 5]]; + expect(iD.geo.lineIntersection(a, b)).to.be.null; + }); + it('returns the intersection point between 2 lines', function() { + var a = [[0, 0], [10, 0]], + b = [[5, 10], [5, -10]]; + expect(iD.geo.lineIntersection(a, b)).to.eql([5, 0]); + }); + it('returns null if lines are not parallel but not intersecting', function() { + var a = [[0, 0], [10, 0]], + b = [[-5, 10], [-5, -10]]; + expect(iD.geo.lineIntersection(a, b)).to.be.null; + }); + }); + describe('.pointInPolygon', function() { it('says a point in a polygon is on a polygon', function() { var poly = [[0, 0], [0, 1], [1, 1], [1, 0], [0, 0]]; diff --git a/test/spec/svg/midpoints.js b/test/spec/svg/midpoints.js index 499e9405b..55634d170 100644 --- a/test/spec/svg/midpoints.js +++ b/test/spec/svg/midpoints.js @@ -10,7 +10,7 @@ describe("iD.svg.Midpoints", function () { .call(iD.svg.Surface(context)); }); - it("finds the location of the midpoints", function () { + it("creates midpoint on segment completely within the extent", function () { var a = iD.Node({loc: [0, 0]}), b = iD.Node({loc: [50, 0]}), line = iD.Way({nodes: [a.id, b.id]}), @@ -23,7 +23,7 @@ describe("iD.svg.Midpoints", function () { expect(surface.select('.midpoint').datum().loc).to.eql([25, 0]); }); - it("doesn't create midpoints on segments with pixel length less than 40", function () { + it("doesn't create midpoint on segment with pixel length less than 40", function () { var a = iD.Node({loc: [0, 0]}), b = iD.Node({loc: [39, 0]}), line = iD.Way({nodes: [a.id, b.id]}), @@ -35,4 +35,44 @@ describe("iD.svg.Midpoints", function () { expect(surface.selectAll('.midpoint')[0]).to.have.length(0); }); + + it("doesn't create midpoint on segment completely outside of the extent", function () { + var a = iD.Node({loc: [-100, 0]}), + b = iD.Node({loc: [-50, 0]}), + line = iD.Way({nodes: [a.id, b.id]}), + graph = iD.Graph([a, b, line]), + extent = iD.geo.Extent([0, 0], [100, 100]); + + context.selectedIDs = function() { return [line.id]; }; + surface.call(iD.svg.Midpoints(projection, context), graph, [line], filter, extent); + + expect(surface.selectAll('.midpoint')[0]).to.have.length(0); + }); + + it("creates midpoint on extent edge for segment partially outside of the extent", function () { + var a = iD.Node({loc: [50, 0]}), + b = iD.Node({loc: [500, 0]}), + line = iD.Way({nodes: [a.id, b.id]}), + graph = iD.Graph([a, b, line]), + extent = iD.geo.Extent([0, 0], [100, 100]); + + context.selectedIDs = function() { return [line.id]; }; + surface.call(iD.svg.Midpoints(projection, context), graph, [line], filter, extent); + + expect(surface.select('.midpoint').datum().loc).to.eql([100, 0]); + }); + + it("doesn't create midpoint on extent edge for segment with pixel length less than 20", function () { + var a = iD.Node({loc: [81, 0]}), + b = iD.Node({loc: [500, 0]}), + line = iD.Way({nodes: [a.id, b.id]}), + graph = iD.Graph([a, b, line]), + extent = iD.geo.Extent([0, 0], [100, 100]); + + context.selectedIDs = function() { return [line.id]; }; + surface.call(iD.svg.Midpoints(projection, context), graph, [line], filter, extent); + + expect(surface.selectAll('.midpoint')[0]).to.have.length(0); + }); + }); From 2c69f7e82362d3b04ef63ba4d14f750787a007ad Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Wed, 18 Jun 2014 11:50:50 -0400 Subject: [PATCH 4/4] Replace magic numbers in trimmedExtent with variables.. --- js/id/renderer/map.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/js/id/renderer/map.js b/js/id/renderer/map.js index 999628785..4138dad1a 100644 --- a/js/id/renderer/map.js +++ b/js/id/renderer/map.js @@ -366,8 +366,9 @@ iD.Map = function(context) { }; map.trimmedExtent = function() { - return new iD.geo.Extent(projection.invert([10, dimensions[1] - 40]), - projection.invert([dimensions[0] - 10, 70])); + var headerY = 60, footerY = 30, pad = 10; + return new iD.geo.Extent(projection.invert([pad, dimensions[1] - footerY - pad]), + projection.invert([dimensions[0] - pad, headerY + pad])); }; map.extentZoom = function(_) {