From 9233be9cd62002342ece7ef199e5e59f9859f381 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Sat, 24 Jan 2015 01:07:27 -0500 Subject: [PATCH 01/11] Avoid zorroing connected ways when moving a way (closes #729) --- js/id/actions/move.js | 138 ++++++++++++++++++++++++++++++++++++++---- js/id/modes/move.js | 29 ++++++++- 2 files changed, 153 insertions(+), 14 deletions(-) diff --git a/js/id/actions/move.js b/js/id/actions/move.js index 6642368a1..a731e8e30 100644 --- a/js/id/actions/move.js +++ b/js/id/actions/move.js @@ -1,31 +1,145 @@ // https://github.com/openstreetmap/josm/blob/mirror/src/org/openstreetmap/josm/command/MoveCommand.java // https://github.com/openstreetmap/potlatch2/blob/master/net/systemeD/halcyon/connection/actions/MoveNodeAction.as -iD.actions.Move = function(ids, delta, projection) { - function addNodes(ids, nodes, graph) { - ids.forEach(function(id) { +iD.actions.Move = function(moveIds, delta, projection, cache) { + + function addIds(ids, nodes, ways, graph) { + _.each(ids, function(id) { var entity = graph.entity(id); + if (entity.type === 'node') { - nodes.push(id); + nodes.push(entity); } else if (entity.type === 'way') { - nodes.push.apply(nodes, entity.nodes); + ways.push(entity); + addIds(entity.nodes, nodes, ways, graph); } else { - addNodes(_.pluck(entity.members, 'id'), nodes, graph); + addIds(_.pluck(entity.members, 'id'), nodes, ways, graph); } }); } + // Place a vertex where the moved vertex used to be, to preserve way shape.. + function replaceMovedVertex(nodeId, wayId, graph) { + var way = graph.entity(wayId), + moved = graph.entity(nodeId), + movedIndex = way.nodes.indexOf(nodeId), + len, prevIndex, nextIndex; + + if (way.isClosed()) { + len = way.nodes.length - 1; + prevIndex = (movedIndex + len - 1) % len; + nextIndex = (movedIndex + len + 1) % len; + } else { + len = way.nodes.length; + prevIndex = movedIndex - 1; + nextIndex = movedIndex + 1; + } + + var prev = graph.hasEntity(way.nodes[prevIndex]), + next = graph.hasEntity(way.nodes[nextIndex]); + + // Don't add orig vertex at endpoint.. + if (!prev || !next) return graph; + + var orig = iD.Node({loc: cache.startLoc[nodeId]}), + angle = Math.abs(iD.geo.angle(orig, prev, projection) - + iD.geo.angle(orig, next, projection)) * 180 / Math.PI; + + // Don't add orig vertex if it would just make a straight line.. + if (angle > 175 && angle < 185) return graph; + + // moving forward or backward along way? + var p1 = [prev.loc, orig.loc, moved.loc, next.loc].map(projection), + p2 = [prev.loc, moved.loc, orig.loc, next.loc].map(projection), + d1 = iD.geo.pathLength(p1), + d2 = iD.geo.pathLength(p2), + insertAt = (d1 < d2) ? movedIndex : nextIndex; + + // moving around closed loop? + if (way.isClosed() && insertAt === 0) insertAt = len; + + way = way.addNode(orig.id, insertAt); + return graph.replace(orig).replace(way); + } + + function isEndpoint(id, way) { + return !way.isClosed() && way.affix(id); + } + + function cleanupWays(movedWays, graph) { + _.each(movedWays, function(movedWay) { + var movedVertices = _.filter(graph.childNodes(movedWay), + function(vertex) { return (graph.parentWays(vertex).length === 2); }); + + _.each(movedVertices, function(movedVertex) { + var id = movedVertex.id, + loc = movedVertex.loc, + firstTime = !cache.lastEdge[id], + way = _.find(graph.parentWays(movedVertex), + function(way) { return way.id !== movedWay.id; }); + + if (firstTime) { + graph = replaceMovedVertex(id, way.id, graph); + way = graph.entity(way.id); + } + + // get closest edge on connected way.. + var nodes = _.without(graph.childNodes(way), movedVertex); + if (way.isClosed() && way.first() === id) nodes.push(nodes[0]); + + var lastEdge = cache.lastEdge[id], + currEdge = iD.geo.chooseEdge(nodes, projection(loc), projection); + + // zorro happened, reorder nodes.. + if (lastEdge && lastEdge.index !== currEdge.index) { + way = way.removeNode(id).addNode(id, currEdge.index); + graph = graph.replace(way); + } + + // snap movedVertex to edge of connected way.. + if (!isEndpoint(id, way)) { + graph = graph.replace(movedVertex.move(currEdge.loc)); + } + + // TODO: + // extend search to a connected way beyond end of way? + // don't mess up points between two intersections + + cache.lastEdge[id] = currEdge; + + }); + }); + return graph; + } + + // Don't move a vertex where >2 ways meet, unless all parentWays are moving too.. + function canMove(entity, graph) { + var parents = graph.parentWays(entity); + if (parents.length < 3) return true; + + return _.all(_.pluck(parents, 'id'), function(id) { + return _.contains(moveIds, id); + }); + } + var action = function(graph) { - var nodes = []; + if (_.isEqual(delta, [0,0])) return graph; - addNodes(ids, nodes, graph); + var nodes = [], + ways = []; - _.uniq(nodes).forEach(function(id) { - var node = graph.entity(id), - start = projection(node.loc), + addIds(moveIds, nodes, ways, graph); + nodes = _.filter(nodes, function(node) { return canMove(node, graph); }); + + _.uniq(nodes).forEach(function(node) { + var start = projection(node.loc), end = projection.invert([start[0] + delta[0], start[1] + delta[1]]); graph = graph.replace(node.move(end)); }); + if (cache) { + graph = cleanupWays(_.uniq(ways), graph); + } + return graph; }; @@ -35,7 +149,7 @@ iD.actions.Move = function(ids, delta, projection) { return entity.type === 'relation' && !entity.isComplete(graph); } - if (_.any(ids, incompleteRelation)) + if (_.any(moveIds, incompleteRelation)) return 'incomplete_relation'; }; diff --git a/js/id/modes/move.js b/js/id/modes/move.js index c7414d647..6de4186d9 100644 --- a/js/id/modes/move.js +++ b/js/id/modes/move.js @@ -9,9 +9,31 @@ iD.modes.Move = function(context, entityIDs) { annotation = entityIDs.length === 1 ? t('operations.move.annotation.' + context.geometry(entityIDs[0])) : t('operations.move.annotation.multiple'), + cache, origin, nudgeInterval; + function clearCache() { + cache = { + startLoc: {}, + lastEdge: {} + }; + } + + function cacheLocs(ids) { + _.each(ids, function(id) { + var entity = context.entity(id); + + if (entity.type === 'node') { + cache.startLoc[id] = entity.loc; + } else if (entity.type === 'way') { + cacheLocs(entity.nodes); + } else { + cacheLocs(_.pluck(entity.members, 'id')); + } + }); + } + function edge(point, size) { var pad = [30, 100, 30, 100]; if (point[0] > size[0] - pad[0]) return [-10, 0]; @@ -26,7 +48,7 @@ iD.modes.Move = function(context, entityIDs) { nudgeInterval = window.setInterval(function() { context.pan(nudge); context.replace( - iD.actions.Move(entityIDs, [-nudge[0], -nudge[1]], context.projection), + iD.actions.Move(entityIDs, [-nudge[0], -nudge[1]], context.projection, cache), annotation); var c = context.projection(origin); origin = context.projection.invert([c[0] - nudge[0], c[1] - nudge[1]]); @@ -53,7 +75,7 @@ iD.modes.Move = function(context, entityIDs) { origin = context.map().mouseCoordinates(); context.replace( - iD.actions.Move(entityIDs, delta, context.projection), + iD.actions.Move(entityIDs, delta, context.projection, cache), annotation); } @@ -76,6 +98,9 @@ iD.modes.Move = function(context, entityIDs) { } mode.enter = function() { + clearCache(); + cacheLocs(entityIDs); + context.install(edit); context.perform( From e51e46a885994fa447cf53f6d0e8fadf29015dae Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 26 Jan 2015 21:52:31 -0500 Subject: [PATCH 02/11] refactor and improve unzorroing, vertex replacement --- js/id/actions/move.js | 131 +++++++++++++++++++++++++++--------------- js/id/modes/move.js | 1 - 2 files changed, 86 insertions(+), 46 deletions(-) diff --git a/js/id/actions/move.js b/js/id/actions/move.js index a731e8e30..b09fe0507 100644 --- a/js/id/actions/move.js +++ b/js/id/actions/move.js @@ -7,9 +7,9 @@ iD.actions.Move = function(moveIds, delta, projection, cache) { var entity = graph.entity(id); if (entity.type === 'node') { - nodes.push(entity); + nodes.push(entity.id); } else if (entity.type === 'way') { - ways.push(entity); + ways.push(entity.id); addIds(entity.nodes, nodes, ways, graph); } else { addIds(_.pluck(entity.members, 'id'), nodes, ways, graph); @@ -18,7 +18,9 @@ iD.actions.Move = function(moveIds, delta, projection, cache) { } // Place a vertex where the moved vertex used to be, to preserve way shape.. - function replaceMovedVertex(nodeId, wayId, graph) { + function replaceMovedVertex(nodeId, wayId, graph, delta) { + if (!cache.startLoc[nodeId]) return graph; + var way = graph.entity(wayId), moved = graph.entity(nodeId), movedIndex = way.nodes.indexOf(nodeId), @@ -40,12 +42,24 @@ iD.actions.Move = function(moveIds, delta, projection, cache) { // Don't add orig vertex at endpoint.. if (!prev || !next) return graph; - var orig = iD.Node({loc: cache.startLoc[nodeId]}), + var start, end; + if (delta) { + start = projection(cache.startLoc[nodeId]); + end = projection.invert([start[0] + delta[0], start[1] + delta[1]]); + } else { + end = cache.startLoc[nodeId]; + } + + var orig = iD.Node({loc: end}), angle = Math.abs(iD.geo.angle(orig, prev, projection) - iD.geo.angle(orig, next, projection)) * 180 / Math.PI; // Don't add orig vertex if it would just make a straight line.. - if (angle > 175 && angle < 185) return graph; + if (angle > 170 && angle < 190) return graph; + + // Don't add orig vertex if points are already close (within 20m) + if (iD.geo.sphericalDistance(prev.loc, orig.loc) < 20 || + iD.geo.sphericalDistance(orig.loc, next.loc) < 20) return graph; // moving forward or backward along way? var p1 = [prev.loc, orig.loc, moved.loc, next.loc].map(projection), @@ -61,59 +75,85 @@ iD.actions.Move = function(moveIds, delta, projection, cache) { return graph.replace(orig).replace(way); } + function unZorro(nodeId, wayId1, wayId2, graph) { + var vertex = graph.entity(nodeId), + way1 = graph.entity(wayId1), + way2 = graph.entity(wayId2), + isEP1 = isEndpoint(nodeId, way1), + isEP2 = isEndpoint(nodeId, way2); + + // don't move the vertex if it is the endpoint of both ways. + if (isEP1 && isEP2) return graph; + + var nodes1 = _.without(graph.childNodes(way1), vertex), + nodes2 = _.without(graph.childNodes(way2), vertex); + + if (way1.isClosed() && way1.first() === nodeId) nodes1.push(nodes1[0]); + if (way2.isClosed() && way2.first() === nodeId) nodes2.push(nodes2[0]); + + var edge1 = !isEP1 && iD.geo.chooseEdge(nodes1, projection(vertex.loc), projection), + edge2 = !isEP2 && iD.geo.chooseEdge(nodes2, projection(vertex.loc), projection), + loc; + + // snap vertex to nearest edge (or some point between them).. + if (!isEP1 && !isEP2) { + var epsilon = 1e-4, maxIter = 10; + for (var i = 0; i < maxIter; i++) { + loc = iD.geo.interp(edge1.loc, edge2.loc, 0.5); + edge1 = iD.geo.chooseEdge(nodes1, projection(loc), projection); + edge2 = iD.geo.chooseEdge(nodes2, projection(loc), projection); + if (Math.abs(edge1.distance - edge2.distance) < epsilon) break; + } + } else if (!isEP1) { + loc = edge1.loc; + } else { + loc = edge2.loc; + } + + graph = graph.replace(vertex.move(loc)); + + // if zorro happened, reorder nodes.. + if (!isEP1 && edge1.index !== way1.nodes.indexOf(nodeId)) { + way1 = way1.removeNode(nodeId).addNode(nodeId, edge1.index); + graph = graph.replace(way1); + } + if (!isEP2 && edge2.index !== way2.nodes.indexOf(nodeId)) { + way2 = way2.removeNode(nodeId).addNode(nodeId, edge2.index); + graph = graph.replace(way2); + } + + return graph; + } + function isEndpoint(id, way) { - return !way.isClosed() && way.affix(id); + return !way.isClosed() && !!way.affix(id); } function cleanupWays(movedWays, graph) { - _.each(movedWays, function(movedWay) { - var movedVertices = _.filter(graph.childNodes(movedWay), + _.each(movedWays, function(movedId) { + var movedWay = graph.entity(movedId), + intersections = _.filter(graph.childNodes(movedWay), function(vertex) { return (graph.parentWays(vertex).length === 2); }); - _.each(movedVertices, function(movedVertex) { - var id = movedVertex.id, - loc = movedVertex.loc, - firstTime = !cache.lastEdge[id], - way = _.find(graph.parentWays(movedVertex), + _.each(intersections, function(vertex) { + var fixedWay = _.find(graph.parentWays(vertex), function(way) { return way.id !== movedWay.id; }); - if (firstTime) { - graph = replaceMovedVertex(id, way.id, graph); - way = graph.entity(way.id); + if (cache.startLoc[vertex.id]) { + graph = replaceMovedVertex(vertex.id, movedWay.id, graph, delta); + graph = replaceMovedVertex(vertex.id, fixedWay.id, graph, null); + delete cache.startLoc[vertex.id]; } - // get closest edge on connected way.. - var nodes = _.without(graph.childNodes(way), movedVertex); - if (way.isClosed() && way.first() === id) nodes.push(nodes[0]); - - var lastEdge = cache.lastEdge[id], - currEdge = iD.geo.chooseEdge(nodes, projection(loc), projection); - - // zorro happened, reorder nodes.. - if (lastEdge && lastEdge.index !== currEdge.index) { - way = way.removeNode(id).addNode(id, currEdge.index); - graph = graph.replace(way); - } - - // snap movedVertex to edge of connected way.. - if (!isEndpoint(id, way)) { - graph = graph.replace(movedVertex.move(currEdge.loc)); - } - - // TODO: - // extend search to a connected way beyond end of way? - // don't mess up points between two intersections - - cache.lastEdge[id] = currEdge; - + graph = unZorro(vertex.id, movedWay.id, fixedWay.id, graph); }); }); return graph; } // Don't move a vertex where >2 ways meet, unless all parentWays are moving too.. - function canMove(entity, graph) { - var parents = graph.parentWays(entity); + function canMove(nodeId, graph) { + var parents = graph.parentWays(graph.entity(nodeId)); if (parents.length < 3) return true; return _.all(_.pluck(parents, 'id'), function(id) { @@ -128,10 +168,11 @@ iD.actions.Move = function(moveIds, delta, projection, cache) { ways = []; addIds(moveIds, nodes, ways, graph); - nodes = _.filter(nodes, function(node) { return canMove(node, graph); }); + nodes = _.filter(nodes, function(id) { return canMove(id, graph); }); - _.uniq(nodes).forEach(function(node) { - var start = projection(node.loc), + _.uniq(nodes).forEach(function(id) { + var node = graph.entity(id), + start = projection(node.loc), end = projection.invert([start[0] + delta[0], start[1] + delta[1]]); graph = graph.replace(node.move(end)); }); diff --git a/js/id/modes/move.js b/js/id/modes/move.js index 6de4186d9..06736cbfe 100644 --- a/js/id/modes/move.js +++ b/js/id/modes/move.js @@ -16,7 +16,6 @@ iD.modes.Move = function(context, entityIDs) { function clearCache() { cache = { startLoc: {}, - lastEdge: {} }; } From f1f665626929641a172a26dcf53c22ab57781020 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 2 Feb 2015 11:21:18 -0500 Subject: [PATCH 03/11] Perform moves idempotently * instead of passing small deltas to the move action and accumulating errors, always pass the absolute delta since entering move mode * also fix cache to handle circular/self-referencing relations --- js/id/modes/move.js | 41 ++++++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/js/id/modes/move.js b/js/id/modes/move.js index 06736cbfe..63ec19a86 100644 --- a/js/id/modes/move.js +++ b/js/id/modes/move.js @@ -11,17 +11,23 @@ iD.modes.Move = function(context, entityIDs) { t('operations.move.annotation.multiple'), cache, origin, + delta, nudgeInterval; function clearCache() { cache = { - startLoc: {}, + moving: {}, + startLoc: {} }; } function cacheLocs(ids) { _.each(ids, function(id) { - var entity = context.entity(id); + if (cache.moving[id]) return; + cache.moving[id] = true; + + var entity = context.hasEntity(id); + if (!entity) return; if (entity.type === 'node') { cache.startLoc[id] = entity.loc; @@ -46,11 +52,17 @@ iD.modes.Move = function(context, entityIDs) { if (nudgeInterval) window.clearInterval(nudgeInterval); nudgeInterval = window.setInterval(function() { context.pan(nudge); - context.replace( - iD.actions.Move(entityIDs, [-nudge[0], -nudge[1]], context.projection, cache), + + var orig = context.projection(origin); + + orig = [orig[0] - nudge[1], orig[1] - nudge[1]]; + delta = [delta[0] - nudge[0], delta[1] - nudge[1]]; + origin = context.projection.invert(orig); + + context.pop(); + context.perform( + iD.actions.Move(entityIDs, delta, context.projection, cache), annotation); - var c = context.projection(origin); - origin = context.projection.invert([c[0] - nudge[0], c[1] - nudge[1]]); }, 50); } @@ -60,20 +72,17 @@ iD.modes.Move = function(context, entityIDs) { } function move() { - var p = context.mouse(); + var mouse = context.mouse(), + orig = context.projection(origin); - var delta = origin ? - [p[0] - context.projection(origin)[0], - p[1] - context.projection(origin)[1]] : - [0, 0]; + delta = [mouse[0] - orig[0], mouse[1] - orig[1]]; - var nudge = edge(p, context.map().dimensions()); + var nudge = edge(mouse, context.map().dimensions()); if (nudge) startNudge(nudge); else stopNudge(); - origin = context.map().mouseCoordinates(); - - context.replace( + context.pop(); + context.perform( iD.actions.Move(entityIDs, delta, context.projection, cache), annotation); } @@ -99,6 +108,8 @@ iD.modes.Move = function(context, entityIDs) { mode.enter = function() { clearCache(); cacheLocs(entityIDs); + origin = context.map().mouseCoordinates(); + delta = [0, 0]; context.install(edit); From 147e369c3bc4d4db12dc391a64cf1028c2d396c3 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 2 Feb 2015 16:56:50 -0500 Subject: [PATCH 04/11] Add graph#overwrite (same as pop followed by perform) to remove some of the overhead in creating difference and dispatching change event multiple times.. --- js/id/core/history.js | 15 ++++++++++++++ js/id/id.js | 1 + test/spec/core/history.js | 43 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/js/id/core/history.js b/js/id/core/history.js index 87c9b0075..f4922d7cd 100644 --- a/js/id/core/history.js +++ b/js/id/core/history.js @@ -77,6 +77,21 @@ iD.History = function(context) { } }, + // Same as calling pop and then perform + overwrite: function() { + var previous = stack[index].graph; + + if (index > 0) { + index--; + stack.pop(); + } + stack = stack.slice(0, index + 1); + stack.push(perform(arguments)); + index++; + + return change(previous); + }, + undo: function() { var previous = stack[index].graph; diff --git a/js/id/id.js b/js/id/id.js index d9546502d..03702711c 100644 --- a/js/id/id.js +++ b/js/id/id.js @@ -126,6 +126,7 @@ window.iD = function () { context.perform = withDebouncedSave(history.perform); context.replace = withDebouncedSave(history.replace); context.pop = withDebouncedSave(history.pop); + context.overwrite = withDebouncedSave(history.overwrite); context.undo = withDebouncedSave(history.undo); context.redo = withDebouncedSave(history.redo); diff --git a/test/spec/core/history.js b/test/spec/core/history.js index f46907f21..d8be41a63 100644 --- a/test/spec/core/history.js +++ b/test/spec/core/history.js @@ -122,6 +122,49 @@ describe("iD.History", function () { }); }); + describe("#overwrite", function () { + it("returns a difference", function () { + history.perform(action, "annotation"); + expect(history.overwrite(action).changes()).to.eql({}); + }); + + it("updates the graph", function () { + history.perform(action, "annotation"); + var node = iD.Node(); + history.overwrite(function (graph) { return graph.replace(node); }); + expect(history.graph().entity(node.id)).to.equal(node); + }); + + it("replaces the undo annotation", function () { + history.perform(action, "annotation1"); + history.overwrite(action, "annotation2"); + expect(history.undoAnnotation()).to.equal("annotation2"); + }); + + it("does not push the redo stack", function () { + history.perform(action, "annotation"); + history.overwrite(action, "annotation2"); + expect(history.redoAnnotation()).to.be.undefined; + }); + + it("emits a change event", function () { + history.perform(action, "annotation"); + history.on('change', spy); + var difference = history.overwrite(action, "annotation2"); + expect(spy).to.have.been.calledWith(difference); + }); + + it("performs multiple actions", function () { + var action1 = sinon.stub().returns(iD.Graph()), + action2 = sinon.stub().returns(iD.Graph()); + history.perform(action, "annotation"); + history.overwrite(action1, action2, "annotation2"); + expect(action1).to.have.been.called; + expect(action2).to.have.been.called; + expect(history.undoAnnotation()).to.equal("annotation2"); + }); + }); + describe("#undo", function () { it("returns a difference", function () { expect(history.undo().changes()).to.eql({}); From fb90cd90b0dd9eed9e460d6fbb44c6cdf04d6b7f Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 2 Feb 2015 16:58:45 -0500 Subject: [PATCH 05/11] Use graph#overwrite for move mode also move cache stuff from here down into iD.actions.Move --- js/id/modes/move.js | 34 +++------------------------------- 1 file changed, 3 insertions(+), 31 deletions(-) diff --git a/js/id/modes/move.js b/js/id/modes/move.js index 63ec19a86..26381d452 100644 --- a/js/id/modes/move.js +++ b/js/id/modes/move.js @@ -14,31 +14,6 @@ iD.modes.Move = function(context, entityIDs) { delta, nudgeInterval; - function clearCache() { - cache = { - moving: {}, - startLoc: {} - }; - } - - function cacheLocs(ids) { - _.each(ids, function(id) { - if (cache.moving[id]) return; - cache.moving[id] = true; - - var entity = context.hasEntity(id); - if (!entity) return; - - if (entity.type === 'node') { - cache.startLoc[id] = entity.loc; - } else if (entity.type === 'way') { - cacheLocs(entity.nodes); - } else { - cacheLocs(_.pluck(entity.members, 'id')); - } - }); - } - function edge(point, size) { var pad = [30, 100, 30, 100]; if (point[0] > size[0] - pad[0]) return [-10, 0]; @@ -59,8 +34,7 @@ iD.modes.Move = function(context, entityIDs) { delta = [delta[0] - nudge[0], delta[1] - nudge[1]]; origin = context.projection.invert(orig); - context.pop(); - context.perform( + context.overwrite( iD.actions.Move(entityIDs, delta, context.projection, cache), annotation); }, 50); @@ -81,8 +55,7 @@ iD.modes.Move = function(context, entityIDs) { if (nudge) startNudge(nudge); else stopNudge(); - context.pop(); - context.perform( + context.overwrite( iD.actions.Move(entityIDs, delta, context.projection, cache), annotation); } @@ -106,10 +79,9 @@ iD.modes.Move = function(context, entityIDs) { } mode.enter = function() { - clearCache(); - cacheLocs(entityIDs); origin = context.map().mouseCoordinates(); delta = [0, 0]; + cache = {}; context.install(edit); From 66ab2fe0db89a0059a3256dc4cca3809d5642f79 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 2 Feb 2015 23:22:27 -0500 Subject: [PATCH 06/11] Better caching, code cleanups.. --- js/id/actions/move.js | 187 +++++++++++++++++++++++++++++++----------- 1 file changed, 138 insertions(+), 49 deletions(-) diff --git a/js/id/actions/move.js b/js/id/actions/move.js index b09fe0507..68a33814e 100644 --- a/js/id/actions/move.js +++ b/js/id/actions/move.js @@ -2,25 +2,46 @@ // https://github.com/openstreetmap/potlatch2/blob/master/net/systemeD/halcyon/connection/actions/MoveNodeAction.as iD.actions.Move = function(moveIds, delta, projection, cache) { - function addIds(ids, nodes, ways, graph) { + function setupCache(graph) { + if (!cache) { + cache = {}; + } + if (!cache.ok) { + cache.moving = {}; + cache.replacedVertex = {}; + cache.startLoc = {}; + cache.nodes = []; + cache.ways = []; + + cacheEntities(moveIds, graph); + cache.nodes = _.filter(cache.nodes, function(id) { return canMove(id, graph); }); + + cache.ok = true; + } + } + + function cacheEntities(ids, graph) { _.each(ids, function(id) { - var entity = graph.entity(id); + if (cache.moving[id]) return; + cache.moving[id] = true; + + var entity = graph.hasEntity(id); + if (!entity) return; if (entity.type === 'node') { - nodes.push(entity.id); + cache.nodes.push(id); + cache.startLoc[id] = entity.loc; } else if (entity.type === 'way') { - ways.push(entity.id); - addIds(entity.nodes, nodes, ways, graph); + cache.ways.push(id); + cacheEntities(entity.nodes, graph); } else { - addIds(_.pluck(entity.members, 'id'), nodes, ways, graph); + cacheEntities(_.pluck(entity.members, 'id'), graph); } }); } // Place a vertex where the moved vertex used to be, to preserve way shape.. function replaceMovedVertex(nodeId, wayId, graph, delta) { - if (!cache.startLoc[nodeId]) return graph; - var way = graph.entity(wayId), moved = graph.entity(nodeId), movedIndex = way.nodes.indexOf(nodeId), @@ -42,6 +63,14 @@ iD.actions.Move = function(moveIds, delta, projection, cache) { // Don't add orig vertex at endpoint.. if (!prev || !next) return graph; + var key = wayId + '_' + nodeId, + orig = cache.replacedVertex[key]; + if (!orig) { + orig = iD.Node(); + cache.replacedVertex[key] = orig; + cache.startLoc[orig.id] = cache.startLoc[nodeId]; + } + var start, end; if (delta) { start = projection(cache.startLoc[nodeId]); @@ -49,18 +78,14 @@ iD.actions.Move = function(moveIds, delta, projection, cache) { } else { end = cache.startLoc[nodeId]; } + orig = orig.move(end); - var orig = iD.Node({loc: end}), - angle = Math.abs(iD.geo.angle(orig, prev, projection) - + var angle = Math.abs(iD.geo.angle(orig, prev, projection) - iD.geo.angle(orig, next, projection)) * 180 / Math.PI; // Don't add orig vertex if it would just make a straight line.. if (angle > 170 && angle < 190) return graph; - // Don't add orig vertex if points are already close (within 20m) - if (iD.geo.sphericalDistance(prev.loc, orig.loc) < 20 || - iD.geo.sphericalDistance(orig.loc, next.loc) < 20) return graph; - // moving forward or backward along way? var p1 = [prev.loc, orig.loc, moved.loc, next.loc].map(projection), p2 = [prev.loc, moved.loc, orig.loc, next.loc].map(projection), @@ -75,11 +100,16 @@ iD.actions.Move = function(moveIds, delta, projection, cache) { return graph.replace(orig).replace(way); } - function unZorro(nodeId, wayId1, wayId2, graph) { + // Reorder nodes around intersections of ways that have moved.. + function unZorroIntersection(nodeId, graph) { var vertex = graph.entity(nodeId), - way1 = graph.entity(wayId1), - way2 = graph.entity(wayId2), - isEP1 = isEndpoint(nodeId, way1), + parents = graph.parentWays(vertex), + way1 = parents[0], + way2 = parents[1]; + + if (!way1 || !way2) return graph; + + var isEP1 = isEndpoint(nodeId, way1), isEP2 = isEndpoint(nodeId, way2); // don't move the vertex if it is the endpoint of both ways. @@ -125,61 +155,120 @@ iD.actions.Move = function(moveIds, delta, projection, cache) { return graph; } + // // scale the vertices between keynodes.. + // function rescaleSegments(wayId, keyNodeIds, graph) { + // function scale(p, dmin, dmax, rmin, rmax) { + // var x = (dmax[0] === dmin[0]) ? p[0] : + // ((rmax[0] - rmin[0]) * (p[0] - dmin[0]) / (dmax[0] - dmin[0])) + rmin[0], + // y = (dmax[1] === dmin[1]) ? p[1] : + // ((rmax[1] - rmin[1]) * (p[1] - dmin[1]) / (dmax[1] - dmin[1])) + rmin[1]; + // return [x, y]; + // } + + // function rescaleSegment(from, to, ids, graph) { + // if (!ids.length) return graph; + + // var dmin = projection(cache.startLoc[from]), + // dmax = projection(cache.startLoc[to]), + // rmin = projection(graph.entity(from).loc), + // rmax = projection(graph.entity(to).loc); + // // var dmin = (cache.startLoc[from]), + // // dmax = (cache.startLoc[to]), + // // rmin = (graph.entity(from).loc), + // // rmax = (graph.entity(to).loc); + + // var j, node, p1, p2; + + // // console.log(''); + // for (j = 0; j < ids.length; j++) { + // node = graph.entity(ids[j]); + // p1 = projection(cache.startLoc[ids[j]]); + // // p1 = (cache.startLoc[ids[j]]); + // p2 = scale(p1, dmin, dmax, rmin, rmax); + // // console.log(ids[j] + ': move from ' + iD.geo.roundCoords(p1) + ' to ' + iD.geo.roundCoords(p2)); + // graph = graph.replace(node.move(projection.invert(p2))); + // // graph = graph.replace(node.move(p2)); + // } + // return graph; + // } + + // var way = graph.entity(wayId); + // if (way.isClosed()) return graph; + + // var ids = [], + // from, to; + + // for (var i = 0; i < way.nodes.length; i++) { + // if (keyNodeIds.indexOf(way.nodes[i]) !== -1) { + // if (!from) { + // from = way.nodes[i]; + // } else { + // to = way.nodes[i]; + // graph = rescaleSegment(from, to, ids, graph); + + // ids = []; + // from = to; + // to = undefined; + // } + // } else { + // ids.push(way.nodes[i]); + // } + // } + + // return graph; + // } + function isEndpoint(id, way) { return !way.isClosed() && !!way.affix(id); } - function cleanupWays(movedWays, graph) { - _.each(movedWays, function(movedId) { - var movedWay = graph.entity(movedId), - intersections = _.filter(graph.childNodes(movedWay), - function(vertex) { return (graph.parentWays(vertex).length === 2); }); + function cleanupWay(id, graph) { + var movedWay = graph.entity(id), + movedNodes = graph.childNodes(movedWay), + intersections = _.filter(movedNodes, + function(node) { return (graph.parentWays(node).length === 2); }); + // keyNodeIds = [movedWay.first()].concat(_.pluck(intersections, 'id'), [movedWay.last()]); - _.each(intersections, function(vertex) { - var fixedWay = _.find(graph.parentWays(vertex), - function(way) { return way.id !== movedWay.id; }); - - if (cache.startLoc[vertex.id]) { - graph = replaceMovedVertex(vertex.id, movedWay.id, graph, delta); - graph = replaceMovedVertex(vertex.id, fixedWay.id, graph, null); - delete cache.startLoc[vertex.id]; - } - - graph = unZorro(vertex.id, movedWay.id, fixedWay.id, graph); - }); + _.each(intersections, function(node) { + var unmovedWay = _.find(graph.parentWays(node), function(way) { return !cache.moving[way.id]; }); + if (unmovedWay) { + graph = replaceMovedVertex(node.id, movedWay.id, graph, delta); + graph = replaceMovedVertex(node.id, unmovedWay.id, graph, null); + graph = unZorroIntersection(node.id, graph); + } }); + + // graph = rescaleSegments(movedWay.id, keyNodeIds, graph); + return graph; } // Don't move a vertex where >2 ways meet, unless all parentWays are moving too.. function canMove(nodeId, graph) { - var parents = graph.parentWays(graph.entity(nodeId)); + var parents = _.pluck(graph.parentWays(graph.entity(nodeId)), 'id'); if (parents.length < 3) return true; - return _.all(_.pluck(parents, 'id'), function(id) { - return _.contains(moveIds, id); - }); + var parentsMoving = _.all(parents, function(id) { return cache.moving[id]; }); + if (!parentsMoving) delete cache.moving[nodeId]; + + return parentsMoving; } var action = function(graph) { - if (_.isEqual(delta, [0,0])) return graph; + if (delta[0] === 0 && delta[1] === 0) return graph; - var nodes = [], - ways = []; + setupCache(graph); - addIds(moveIds, nodes, ways, graph); - nodes = _.filter(nodes, function(id) { return canMove(id, graph); }); - - _.uniq(nodes).forEach(function(id) { + _.each(cache.nodes, function(id) { var node = graph.entity(id), start = projection(node.loc), end = projection.invert([start[0] + delta[0], start[1] + delta[1]]); graph = graph.replace(node.move(end)); }); - if (cache) { - graph = cleanupWays(_.uniq(ways), graph); - } + _.each(cache.ways, function(id) { + graph = cleanupWay(id, graph); + }); return graph; }; From 54594cd296a91f1b3530640fd8d65307b72b402a Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Fri, 6 Feb 2015 10:31:35 -0500 Subject: [PATCH 07/11] unzorro vertices that have moved past an endpoint (while this works, it causes jerky confusing movement when moving the way, so probably taking a different approach involving point scaling) --- js/id/actions/move.js | 145 +++++++++++++++++++++++------------------- js/id/geo.js | 13 ++++ 2 files changed, 91 insertions(+), 67 deletions(-) diff --git a/js/id/actions/move.js b/js/id/actions/move.js index 68a33814e..e81c91199 100644 --- a/js/id/actions/move.js +++ b/js/id/actions/move.js @@ -84,7 +84,11 @@ iD.actions.Move = function(moveIds, delta, projection, cache) { iD.geo.angle(orig, next, projection)) * 180 / Math.PI; // Don't add orig vertex if it would just make a straight line.. - if (angle > 170 && angle < 190) return graph; + if (angle > 175 && angle < 195) return graph; + + // Don't add orig vertex if another point is already nearby (within 10m) + if (iD.geo.sphericalDistance(prev.loc, orig.loc) < 10 || + iD.geo.sphericalDistance(orig.loc, next.loc) < 10) return graph; // moving forward or backward along way? var p1 = [prev.loc, orig.loc, moved.loc, next.loc].map(projection), @@ -155,64 +159,58 @@ iD.actions.Move = function(moveIds, delta, projection, cache) { return graph; } - // // scale the vertices between keynodes.. - // function rescaleSegments(wayId, keyNodeIds, graph) { - // function scale(p, dmin, dmax, rmin, rmax) { - // var x = (dmax[0] === dmin[0]) ? p[0] : - // ((rmax[0] - rmin[0]) * (p[0] - dmin[0]) / (dmax[0] - dmin[0])) + rmin[0], - // y = (dmax[1] === dmin[1]) ? p[1] : - // ((rmax[1] - rmin[1]) * (p[1] - dmin[1]) / (dmax[1] - dmin[1])) + rmin[1]; - // return [x, y]; - // } + // // Adjust any vertices that have been moved beyond the endpoints.. + // function unZorroEndpoint(nodeId, movedId, unmovedId, graph) { - // function rescaleSegment(from, to, ids, graph) { - // if (!ids.length) return graph; - - // var dmin = projection(cache.startLoc[from]), - // dmax = projection(cache.startLoc[to]), - // rmin = projection(graph.entity(from).loc), - // rmax = projection(graph.entity(to).loc); - // // var dmin = (cache.startLoc[from]), - // // dmax = (cache.startLoc[to]), - // // rmin = (graph.entity(from).loc), - // // rmax = (graph.entity(to).loc); - - // var j, node, p1, p2; - - // // console.log(''); - // for (j = 0; j < ids.length; j++) { - // node = graph.entity(ids[j]); - // p1 = projection(cache.startLoc[ids[j]]); - // // p1 = (cache.startLoc[ids[j]]); - // p2 = scale(p1, dmin, dmax, rmin, rmax); - // // console.log(ids[j] + ': move from ' + iD.geo.roundCoords(p1) + ' to ' + iD.geo.roundCoords(p2)); - // graph = graph.replace(node.move(projection.invert(p2))); - // // graph = graph.replace(node.move(p2)); - // } - // return graph; - // } - - // var way = graph.entity(wayId); - // if (way.isClosed()) return graph; - - // var ids = [], - // from, to; - - // for (var i = 0; i < way.nodes.length; i++) { - // if (keyNodeIds.indexOf(way.nodes[i]) !== -1) { - // if (!from) { - // from = way.nodes[i]; - // } else { - // to = way.nodes[i]; - // graph = rescaleSegment(from, to, ids, graph); - - // ids = []; - // from = to; - // to = undefined; + // function diffMultiPoint(a1, a2) { + // var result = []; + // outer: for (var i = 0; i < a1.length; i++) { + // for (var j = 0; j < a2.length; j++) { + // if (a1[i][0] === a2[j][0] && a1[i][1] === a2[j][1]) continue outer; // } - // } else { - // ids.push(way.nodes[i]); + // result.push(a1[i]); // } + // return result; + // } + + // var movedWay = graph.entity(movedId), + // unmovedWay = graph.entity(unmovedId), + // movedNodes = graph.childNodes(movedWay), + // unmovedNodes = graph.childNodes(unmovedWay); + + // if (movedNodes.length < 3) return graph; + // if (movedWay.last() === nodeId) movedNodes.reverse(); + + // var movedPath = _.pluck(movedNodes, 'loc').map(projection), + // unmovedPath = _.pluck(unmovedNodes, 'loc').map(projection), + // testPath = _.clone(movedPath), + // keep = [], + // remove = [], + // index = 0; + + // // try removing segments from movedPath until zorros disappear.. + // do { + // // paths intersect where there is not a vertex? + // var intersections = iD.geo.pathIntersections(testPath, unmovedPath), + // zorros = diffMultiPoint(intersections, testPath); + // if (!zorros.length) break; + + // var node = movedNodes[++index]; + // (node.hasInterestingTags() ? keep : keep).push(node); + // testPath.splice(1, 1); + + // } while (testPath.length > 2); + + // // either movedPath ok, or testPath left with nothing but the 2 endpoints.. + // if (index === 0 || testPath.length === 2) return graph; + + // if (remove.length) { + // graph = iD.actions.DeleteMultiple(_.pluck(remove, 'id'))(graph); + // } + + // for (var i = 0; i < keep.length; i++) { + // var point = iD.geo.interp(movedPath[0], movedPath[index+1], (i + 1) / keep.length); + // graph = graph.replace(keep[i].move(projection.invert(point))); // } // return graph; @@ -225,20 +223,33 @@ iD.actions.Move = function(moveIds, delta, projection, cache) { function cleanupWay(id, graph) { var movedWay = graph.entity(id), movedNodes = graph.childNodes(movedWay), - intersections = _.filter(movedNodes, - function(node) { return (graph.parentWays(node).length === 2); }); - // keyNodeIds = [movedWay.first()].concat(_.pluck(intersections, 'id'), [movedWay.last()]); + intersections = {}; - _.each(intersections, function(node) { - var unmovedWay = _.find(graph.parentWays(node), function(way) { return !cache.moving[way.id]; }); - if (unmovedWay) { - graph = replaceMovedVertex(node.id, movedWay.id, graph, delta); - graph = replaceMovedVertex(node.id, unmovedWay.id, graph, null); - graph = unZorroIntersection(node.id, graph); - } + // consider only intersections with 1 moved and 1 unmoved way. + _.each(movedNodes, function(node) { + var parents = graph.parentWays(node); + if (parents.length !== 2) return; + + var unmovedWay = _.find(parents, function(way) { return !cache.moving[way.id]; }); + if (!unmovedWay) return; + + intersections[node.id] = { + movedWay: movedWay, + unmovedWay: unmovedWay + }; }); - // graph = rescaleSegments(movedWay.id, keyNodeIds, graph); + _.each(intersections, function(obj, id) { + graph = replaceMovedVertex(id, obj.movedWay.id, graph, delta); + graph = replaceMovedVertex(id, obj.unmovedWay.id, graph, null); + graph = unZorroIntersection(id, graph); + }); + + // _.each(intersections, function(obj, id) { + // if (isEndpoint(id, obj.movedWay)) { + // graph = unZorroEndpoint(id, obj.movedWay.id, obj.unmovedWay.id, graph); + // } + // }); return graph; } diff --git a/js/id/geo.js b/js/id/geo.js index 059953d3a..7e1dfc527 100644 --- a/js/id/geo.js +++ b/js/id/geo.js @@ -147,6 +147,19 @@ iD.geo.lineIntersection = function(a, b) { return null; }; +iD.geo.pathIntersections = function(path1, path2) { + var intersections = []; + for (var i = 0; i < path1.length - 1; i++) { + for (var j = 0; j < path2.length - 1; j++) { + var a = [ path1[i], path1[i+1] ], + b = [ path2[j], path2[j+1] ], + hit = iD.geo.lineIntersection(a, b); + if (hit) intersections.push(hit); + } + } + return intersections; +}; + // Return whether point is contained in polygon. // // `point` should be a 2-item array of coordinates. From 6d82ae4146a7bec26b8836915a18e3b079671d19 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Fri, 6 Feb 2015 13:56:51 -0500 Subject: [PATCH 08/11] add iD.geo.Extent#contains --- js/id/geo/extent.js | 8 ++++++++ test/spec/geo/extent.js | 31 ++++++++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/js/id/geo/extent.js b/js/id/geo/extent.js index a32b5783c..912bcf08a 100644 --- a/js/id/geo/extent.js +++ b/js/id/geo/extent.js @@ -55,6 +55,14 @@ _.extend(iD.geo.Extent.prototype, { ]; }, + contains: function(obj) { + if (!(obj instanceof iD.geo.Extent)) obj = new iD.geo.Extent(obj); + return obj[0][0] >= this[0][0] && + obj[0][1] >= this[0][1] && + obj[1][0] <= this[1][0] && + obj[1][1] <= this[1][1]; + }, + intersects: function(obj) { if (!(obj instanceof iD.geo.Extent)) obj = new iD.geo.Extent(obj); return obj[0][0] <= this[1][0] && diff --git a/test/spec/geo/extent.js b/test/spec/geo/extent.js index fb880548e..cd2e84cb0 100644 --- a/test/spec/geo/extent.js +++ b/test/spec/geo/extent.js @@ -109,6 +109,35 @@ describe("iD.geo.Extent", function () { }); }); + describe('#contains', function () { + it("returns true for a point inside self", function () { + expect(iD.geo.Extent([0, 0], [5, 5]).contains([2, 2])).to.be.true; + }); + + it("returns true for a point on the boundary of self", function () { + expect(iD.geo.Extent([0, 0], [5, 5]).contains([0, 0])).to.be.true; + }); + + it("returns false for a point outside self", function () { + expect(iD.geo.Extent([0, 0], [5, 5]).contains([6, 6])).to.be.false; + }); + + it("returns true for an extent contained by self", function () { + expect(iD.geo.Extent([0, 0], [5, 5]).contains([[1, 1], [2, 2]])).to.be.true; + expect(iD.geo.Extent([1, 1], [2, 2]).contains([[0, 0], [5, 5]])).to.be.false; + }); + + it("returns false for an extent partially contained by self", function () { + expect(iD.geo.Extent([0, 0], [5, 5]).contains([[1, 1], [6, 6]])).to.be.false; + expect(iD.geo.Extent([1, 1], [6, 6]).contains([[0, 0], [5, 5]])).to.be.false; + }); + + it("returns false for an extent not intersected by self", function () { + expect(iD.geo.Extent([0, 0], [5, 5]).contains([[6, 6], [7, 7]])).to.be.false; + expect(iD.geo.Extent([[6, 6], [7, 7]]).contains([[0, 0], [5, 5]])).to.be.false; + }); + }); + describe('#intersects', function () { it("returns true for a point inside self", function () { expect(iD.geo.Extent([0, 0], [5, 5]).intersects([2, 2])).to.be.true; @@ -127,7 +156,7 @@ describe("iD.geo.Extent", function () { expect(iD.geo.Extent([1, 1], [2, 2]).intersects([[0, 0], [5, 5]])).to.be.true; }); - it("returns true for an extent intersected by self", function () { + it("returns true for an extent partially contained by self", function () { expect(iD.geo.Extent([0, 0], [5, 5]).intersects([[1, 1], [6, 6]])).to.be.true; expect(iD.geo.Extent([1, 1], [6, 6]).intersects([[0, 0], [5, 5]])).to.be.true; }); From 7232e0d769410866ae85c14fff1d68288ae87dfe Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Wed, 4 Mar 2015 16:05:33 -0500 Subject: [PATCH 09/11] Restrict delta so that user can not move way across intersection --- js/id/actions/move.js | 260 +++++++++++++++++++----------------------- js/id/modes/move.js | 22 ++-- 2 files changed, 130 insertions(+), 152 deletions(-) diff --git a/js/id/actions/move.js b/js/id/actions/move.js index e81c91199..985ef43c1 100644 --- a/js/id/actions/move.js +++ b/js/id/actions/move.js @@ -1,44 +1,87 @@ // https://github.com/openstreetmap/josm/blob/mirror/src/org/openstreetmap/josm/command/MoveCommand.java // https://github.com/openstreetmap/potlatch2/blob/master/net/systemeD/halcyon/connection/actions/MoveNodeAction.as -iD.actions.Move = function(moveIds, delta, projection, cache) { +iD.actions.Move = function(moveIds, tryDelta, projection, cache) { + var delta = tryDelta; + + function vecAdd(a, b) { return [a[0] + b[0], a[1] + b[1]]; } + function vecSub(a, b) { return [a[0] - b[0], a[1] - b[1]]; } function setupCache(graph) { + function canMove(nodeId) { + var parents = _.pluck(graph.parentWays(graph.entity(nodeId)), 'id'); + if (parents.length < 3) return true; + + // Don't move a vertex where >2 ways meet, unless all parentWays are moving too.. + var parentsMoving = _.all(parents, function(id) { return cache.moving[id]; }); + if (!parentsMoving) delete cache.moving[nodeId]; + + return parentsMoving; + } + + function cacheEntities(ids) { + _.each(ids, function(id) { + if (cache.moving[id]) return; + cache.moving[id] = true; + + var entity = graph.hasEntity(id); + if (!entity) return; + + if (entity.type === 'node') { + cache.nodes.push(id); + cache.startLoc[id] = entity.loc; + } else if (entity.type === 'way') { + cache.ways.push(id); + cacheEntities(entity.nodes); + } else { + cacheEntities(_.pluck(entity.members, 'id')); + } + }); + } + + function cacheIntersections(ids) { + function isEndpoint(way, id) { return !way.isClosed() && !!way.affix(id); } + + _.each(ids, function(id) { + // consider only intersections with 1 moved and 1 unmoved way. + _.each(graph.childNodes(graph.entity(id)), function(node) { + var parents = graph.parentWays(node); + if (parents.length !== 2) return; + + var moved = graph.entity(id), + unmoved = _.find(parents, function(way) { return !cache.moving[way.id]; }); + if (!unmoved) return; + + cache.intersection[node.id] = { + nodeId: node.id, + movedId: moved.id, + unmovedId: unmoved.id, + movedIsEP: isEndpoint(moved, node.id), + unmovedIsEP: isEndpoint(unmoved, node.id) + }; + }); + }); + } + + if (!cache) { cache = {}; } if (!cache.ok) { cache.moving = {}; + cache.intersection = {}; cache.replacedVertex = {}; cache.startLoc = {}; cache.nodes = []; cache.ways = []; - cacheEntities(moveIds, graph); - cache.nodes = _.filter(cache.nodes, function(id) { return canMove(id, graph); }); + cacheEntities(moveIds); + cacheIntersections(cache.ways); + cache.nodes = _.filter(cache.nodes, canMove); cache.ok = true; } } - function cacheEntities(ids, graph) { - _.each(ids, function(id) { - if (cache.moving[id]) return; - cache.moving[id] = true; - - var entity = graph.hasEntity(id); - if (!entity) return; - - if (entity.type === 'node') { - cache.nodes.push(id); - cache.startLoc[id] = entity.loc; - } else if (entity.type === 'way') { - cache.ways.push(id); - cacheEntities(entity.nodes, graph); - } else { - cacheEntities(_.pluck(entity.members, 'id'), graph); - } - }); - } // Place a vertex where the moved vertex used to be, to preserve way shape.. function replaceMovedVertex(nodeId, wayId, graph, delta) { @@ -74,7 +117,7 @@ iD.actions.Move = function(moveIds, delta, projection, cache) { var start, end; if (delta) { start = projection(cache.startLoc[nodeId]); - end = projection.invert([start[0] + delta[0], start[1] + delta[1]]); + end = projection.invert(vecAdd(start, delta)); } else { end = cache.startLoc[nodeId]; } @@ -84,7 +127,7 @@ iD.actions.Move = function(moveIds, delta, projection, cache) { iD.geo.angle(orig, next, projection)) * 180 / Math.PI; // Don't add orig vertex if it would just make a straight line.. - if (angle > 175 && angle < 195) return graph; + if (angle > 175 && angle < 185) return graph; // Don't add orig vertex if another point is already nearby (within 10m) if (iD.geo.sphericalDistance(prev.loc, orig.loc) < 10 || @@ -104,17 +147,13 @@ iD.actions.Move = function(moveIds, delta, projection, cache) { return graph.replace(orig).replace(way); } - // Reorder nodes around intersections of ways that have moved.. - function unZorroIntersection(nodeId, graph) { - var vertex = graph.entity(nodeId), - parents = graph.parentWays(vertex), - way1 = parents[0], - way2 = parents[1]; - - if (!way1 || !way2) return graph; - - var isEP1 = isEndpoint(nodeId, way1), - isEP2 = isEndpoint(nodeId, way2); + // Reorder nodes around intersections that have moved.. + function unZorroIntersection(intersection, graph) { + var vertex = graph.entity(intersection.nodeId), + way1 = graph.entity(intersection.movedId), + way2 = graph.entity(intersection.unmovedId), + isEP1 = intersection.movedIsEP, + isEP2 = intersection.unmovedIsEP; // don't move the vertex if it is the endpoint of both ways. if (isEP1 && isEP2) return graph; @@ -122,8 +161,8 @@ iD.actions.Move = function(moveIds, delta, projection, cache) { var nodes1 = _.without(graph.childNodes(way1), vertex), nodes2 = _.without(graph.childNodes(way2), vertex); - if (way1.isClosed() && way1.first() === nodeId) nodes1.push(nodes1[0]); - if (way2.isClosed() && way2.first() === nodeId) nodes2.push(nodes2[0]); + if (way1.isClosed() && way1.first() === vertex.id) nodes1.push(nodes1[0]); + if (way2.isClosed() && way2.first() === vertex.id) nodes2.push(nodes2[0]); var edge1 = !isEP1 && iD.geo.chooseEdge(nodes1, projection(vertex.loc), projection), edge2 = !isEP2 && iD.geo.chooseEdge(nodes2, projection(vertex.loc), projection), @@ -147,139 +186,72 @@ iD.actions.Move = function(moveIds, delta, projection, cache) { graph = graph.replace(vertex.move(loc)); // if zorro happened, reorder nodes.. - if (!isEP1 && edge1.index !== way1.nodes.indexOf(nodeId)) { - way1 = way1.removeNode(nodeId).addNode(nodeId, edge1.index); + if (!isEP1 && edge1.index !== way1.nodes.indexOf(vertex.id)) { + way1 = way1.removeNode(vertex.id).addNode(vertex.id, edge1.index); graph = graph.replace(way1); } - if (!isEP2 && edge2.index !== way2.nodes.indexOf(nodeId)) { - way2 = way2.removeNode(nodeId).addNode(nodeId, edge2.index); + if (!isEP2 && edge2.index !== way2.nodes.indexOf(vertex.id)) { + way2 = way2.removeNode(vertex.id).addNode(vertex.id, edge2.index); graph = graph.replace(way2); } return graph; } - // // Adjust any vertices that have been moved beyond the endpoints.. - // function unZorroEndpoint(nodeId, movedId, unmovedId, graph) { - // function diffMultiPoint(a1, a2) { - // var result = []; - // outer: for (var i = 0; i < a1.length; i++) { - // for (var j = 0; j < a2.length; j++) { - // if (a1[i][0] === a2[j][0] && a1[i][1] === a2[j][1]) continue outer; - // } - // result.push(a1[i]); - // } - // return result; - // } - - // var movedWay = graph.entity(movedId), - // unmovedWay = graph.entity(unmovedId), - // movedNodes = graph.childNodes(movedWay), - // unmovedNodes = graph.childNodes(unmovedWay); - - // if (movedNodes.length < 3) return graph; - // if (movedWay.last() === nodeId) movedNodes.reverse(); - - // var movedPath = _.pluck(movedNodes, 'loc').map(projection), - // unmovedPath = _.pluck(unmovedNodes, 'loc').map(projection), - // testPath = _.clone(movedPath), - // keep = [], - // remove = [], - // index = 0; - - // // try removing segments from movedPath until zorros disappear.. - // do { - // // paths intersect where there is not a vertex? - // var intersections = iD.geo.pathIntersections(testPath, unmovedPath), - // zorros = diffMultiPoint(intersections, testPath); - // if (!zorros.length) break; - - // var node = movedNodes[++index]; - // (node.hasInterestingTags() ? keep : keep).push(node); - // testPath.splice(1, 1); - - // } while (testPath.length > 2); - - // // either movedPath ok, or testPath left with nothing but the 2 endpoints.. - // if (index === 0 || testPath.length === 2) return graph; - - // if (remove.length) { - // graph = iD.actions.DeleteMultiple(_.pluck(remove, 'id'))(graph); - // } - - // for (var i = 0; i < keep.length; i++) { - // var point = iD.geo.interp(movedPath[0], movedPath[index+1], (i + 1) / keep.length); - // graph = graph.replace(keep[i].move(projection.invert(point))); - // } - - // return graph; - // } - - function isEndpoint(id, way) { - return !way.isClosed() && !!way.affix(id); - } - - function cleanupWay(id, graph) { - var movedWay = graph.entity(id), - movedNodes = graph.childNodes(movedWay), - intersections = {}; - - // consider only intersections with 1 moved and 1 unmoved way. - _.each(movedNodes, function(node) { - var parents = graph.parentWays(node); - if (parents.length !== 2) return; - - var unmovedWay = _.find(parents, function(way) { return !cache.moving[way.id]; }); - if (!unmovedWay) return; - - intersections[node.id] = { - movedWay: movedWay, - unmovedWay: unmovedWay - }; + function cleanupIntersections(graph) { + _.each(cache.intersection, function(obj) { + graph = replaceMovedVertex(obj.nodeId, obj.movedId, graph, delta); + graph = replaceMovedVertex(obj.nodeId, obj.unmovedId, graph, null); + graph = unZorroIntersection(obj, graph); }); - _.each(intersections, function(obj, id) { - graph = replaceMovedVertex(id, obj.movedWay.id, graph, delta); - graph = replaceMovedVertex(id, obj.unmovedWay.id, graph, null); - graph = unZorroIntersection(id, graph); - }); - - // _.each(intersections, function(obj, id) { - // if (isEndpoint(id, obj.movedWay)) { - // graph = unZorroEndpoint(id, obj.movedWay.id, obj.unmovedWay.id, graph); - // } - // }); - return graph; } - // Don't move a vertex where >2 ways meet, unless all parentWays are moving too.. - function canMove(nodeId, graph) { - var parents = _.pluck(graph.parentWays(graph.entity(nodeId)), 'id'); - if (parents.length < 3) return true; + // check if moving way endpoint can cross an unmoved way, if so limit delta.. + function limitDelta(graph) { + _.each(cache.intersection, function(obj) { + if (!obj.movedIsEP) return; - var parentsMoving = _.all(parents, function(id) { return cache.moving[id]; }); - if (!parentsMoving) delete cache.moving[nodeId]; + var node = graph.entity(obj.nodeId), + start = projection(node.loc), + end = vecAdd(start, delta), + movedNodes = graph.childNodes(graph.entity(obj.movedId)), + movedPath = _.map(_.pluck(movedNodes, 'loc'), + function(loc) { return vecAdd(projection(loc), delta); }), + unmovedNodes = graph.childNodes(graph.entity(obj.unmovedId)), + unmovedPath = _.map(_.pluck(unmovedNodes, 'loc'), projection), + hits = iD.geo.pathIntersections(movedPath, unmovedPath); - return parentsMoving; + for (var i = 0; i < hits.length; i++) { + if (_.isEqual(hits[i], end)) continue; + var edge = iD.geo.chooseEdge(unmovedNodes, end, projection); + delta = vecSub(projection(edge.loc), start); + } + }); } + var action = function(graph) { if (delta[0] === 0 && delta[1] === 0) return graph; setupCache(graph); + if (!_.isEmpty(cache.intersection)) { + limitDelta(graph); + } + _.each(cache.nodes, function(id) { var node = graph.entity(id), start = projection(node.loc), - end = projection.invert([start[0] + delta[0], start[1] + delta[1]]); - graph = graph.replace(node.move(end)); + end = vecAdd(start, delta); + graph = graph.replace(node.move(projection.invert(end))); }); - _.each(cache.ways, function(id) { - graph = cleanupWay(id, graph); - }); + if (!_.isEmpty(cache.intersection)) { + graph = cleanupIntersections(graph); + } return graph; }; @@ -294,5 +266,9 @@ iD.actions.Move = function(moveIds, delta, projection, cache) { return 'incomplete_relation'; }; + action.delta = function() { + return delta; + }; + return action; }; diff --git a/js/id/modes/move.js b/js/id/modes/move.js index 26381d452..9210c2148 100644 --- a/js/id/modes/move.js +++ b/js/id/modes/move.js @@ -14,6 +14,8 @@ iD.modes.Move = function(context, entityIDs) { delta, nudgeInterval; + function vecSub(a, b) { return [a[0] - b[0], a[1] - b[1]]; } + function edge(point, size) { var pad = [30, 100, 30, 100]; if (point[0] > size[0] - pad[0]) return [-10, 0]; @@ -30,13 +32,13 @@ iD.modes.Move = function(context, entityIDs) { var orig = context.projection(origin); - orig = [orig[0] - nudge[1], orig[1] - nudge[1]]; - delta = [delta[0] - nudge[0], delta[1] - nudge[1]]; + orig = vecSub(orig, nudge); origin = context.projection.invert(orig); - context.overwrite( - iD.actions.Move(entityIDs, delta, context.projection, cache), - annotation); + // delta = vecSub(delta, nudge); + // context.overwrite( + // iD.actions.Move(entityIDs, delta, context.projection, cache), + // annotation); }, 50); } @@ -49,15 +51,15 @@ iD.modes.Move = function(context, entityIDs) { var mouse = context.mouse(), orig = context.projection(origin); - delta = [mouse[0] - orig[0], mouse[1] - orig[1]]; + var action = iD.actions.Move(entityIDs, vecSub(mouse, orig), context.projection, cache); + context.overwrite(action, annotation); + delta = action.delta(); + // TODO restrict nudging if move was restricted.. + // (because geometry may not be under the mouse pointer) var nudge = edge(mouse, context.map().dimensions()); if (nudge) startNudge(nudge); else stopNudge(); - - context.overwrite( - iD.actions.Move(entityIDs, delta, context.projection, cache), - annotation); } function finish() { From 3adcd85c89d3c15118a37b4d669ebe1fb3bcc9e3 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 5 Mar 2015 16:20:50 -0500 Subject: [PATCH 10/11] Fix nudging, cleanup code.. --- js/id/modes/move.js | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/js/id/modes/move.js b/js/id/modes/move.js index 9210c2148..b564fde8f 100644 --- a/js/id/modes/move.js +++ b/js/id/modes/move.js @@ -11,7 +11,6 @@ iD.modes.Move = function(context, entityIDs) { t('operations.move.annotation.multiple'), cache, origin, - delta, nudgeInterval; function vecSub(a, b) { return [a[0] - b[0], a[1] - b[1]]; } @@ -30,15 +29,13 @@ iD.modes.Move = function(context, entityIDs) { nudgeInterval = window.setInterval(function() { context.pan(nudge); - var orig = context.projection(origin); + var currMouse = context.mouse(), + origMouse = context.projection(origin), + delta = vecSub(vecSub(currMouse, origMouse), nudge), + action = iD.actions.Move(entityIDs, delta, context.projection, cache); - orig = vecSub(orig, nudge); - origin = context.projection.invert(orig); + context.overwrite(action, annotation); - // delta = vecSub(delta, nudge); - // context.overwrite( - // iD.actions.Move(entityIDs, delta, context.projection, cache), - // annotation); }, 50); } @@ -48,31 +45,27 @@ iD.modes.Move = function(context, entityIDs) { } function move() { - var mouse = context.mouse(), - orig = context.projection(origin); + var currMouse = context.mouse(), + origMouse = context.projection(origin), + delta = vecSub(currMouse, origMouse), + action = iD.actions.Move(entityIDs, delta, context.projection, cache); - var action = iD.actions.Move(entityIDs, vecSub(mouse, orig), context.projection, cache); context.overwrite(action, annotation); - delta = action.delta(); - // TODO restrict nudging if move was restricted.. - // (because geometry may not be under the mouse pointer) - var nudge = edge(mouse, context.map().dimensions()); + var nudge = edge(currMouse, context.map().dimensions()); if (nudge) startNudge(nudge); else stopNudge(); } function finish() { d3.event.stopPropagation(); - context.enter(iD.modes.Select(context, entityIDs) - .suppressMenu(true)); + context.enter(iD.modes.Select(context, entityIDs).suppressMenu(true)); stopNudge(); } function cancel() { context.pop(); - context.enter(iD.modes.Select(context, entityIDs) - .suppressMenu(true)); + context.enter(iD.modes.Select(context, entityIDs).suppressMenu(true)); stopNudge(); } @@ -82,7 +75,6 @@ iD.modes.Move = function(context, entityIDs) { mode.enter = function() { origin = context.map().mouseCoordinates(); - delta = [0, 0]; cache = {}; context.install(edit); From bf1270908b1b425c090423388e0813cf4d6c65cc Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 5 Mar 2015 16:49:04 -0500 Subject: [PATCH 11/11] Don't attempt clever way movement with areas. --- js/id/actions/move.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/js/id/actions/move.js b/js/id/actions/move.js index 985ef43c1..7dd39c5b3 100644 --- a/js/id/actions/move.js +++ b/js/id/actions/move.js @@ -51,6 +51,8 @@ iD.actions.Move = function(moveIds, tryDelta, projection, cache) { unmoved = _.find(parents, function(way) { return !cache.moving[way.id]; }); if (!unmoved) return; + if (moved.isArea() || unmoved.isArea()) return; + cache.intersection[node.id] = { nodeId: node.id, movedId: moved.id,