Merge pull request #2516 from bhousel/zorrofix

Avoid zorroing connected ways when moving a way
2026-05-22 08:17:30 +02:00 · 2015-03-20 22:30:34 -04:00
parent 1d2b613bd6 bf1270908b
commit 55c4fce55f
8 changed files with 381 additions and 38 deletions
@@ -1,31 +1,260 @@
 // 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) {
-            var entity = graph.entity(id);
-            if (entity.type === 'node') {
-                nodes.push(id);
-            } else if (entity.type === 'way') {
-                nodes.push.apply(nodes, entity.nodes);
-            } else {
-                addNodes(_.pluck(entity.members, 'id'), nodes, graph);
+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;
+
+                    if (moved.isArea() || unmoved.isArea()) 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);
+            cacheIntersections(cache.ways);
+            cache.nodes = _.filter(cache.nodes, canMove);
+
+            cache.ok = true;
+        }
+    }
+
+
+    // Place a vertex where the moved vertex used to be, to preserve way shape..
+    function replaceMovedVertex(nodeId, wayId, graph, delta) {
+        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 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]);
+            end = projection.invert(vecAdd(start, delta));
+        } else {
+            end = cache.startLoc[nodeId];
+        }
+        orig = orig.move(end);
+
+        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 > 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 ||
+            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),
+            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);
+    }
+
+    // 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;
+
+        var nodes1 = _.without(graph.childNodes(way1), vertex),
+            nodes2 = _.without(graph.childNodes(way2), vertex);
+
+        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),
+            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(vertex.id)) {
+            way1 = way1.removeNode(vertex.id).addNode(vertex.id, edge1.index);
+            graph = graph.replace(way1);
+        }
+        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;
+    }
+
+
+    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);
+        });
+
+        return graph;
+    }
+
+    // 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 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);
+
+            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) {
-        var nodes = [];
+        if (delta[0] === 0 && delta[1] === 0) return graph;

-        addNodes(ids, nodes, graph);
+        setupCache(graph);

-        _.uniq(nodes).forEach(function(id) {
+        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)));
        });

+        if (!_.isEmpty(cache.intersection)) {
+            graph = cleanupIntersections(graph);
+        }
+
        return graph;
    };

@@ -35,9 +264,13 @@ 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';
    };

+    action.delta = function() {
+        return delta;
+    };
+
    return action;
 };
@@ -81,6 +81,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;

@@ -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.
@@ -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] &&
@@ -167,6 +167,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);

@@ -9,9 +9,12 @@ 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 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];
@@ -25,11 +28,14 @@ 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),
-                annotation);
-            var c = context.projection(origin);
-            origin = context.projection.invert([c[0] - nudge[0], c[1] - nudge[1]]);
+
+            var currMouse = context.mouse(),
+                origMouse = context.projection(origin),
+                delta = vecSub(vecSub(currMouse, origMouse), nudge),
+                action = iD.actions.Move(entityIDs, delta, context.projection, cache);
+
+            context.overwrite(action, annotation);
+
        }, 50);
    }

@@ -39,35 +45,27 @@ iD.modes.Move = function(context, entityIDs) {
    }

    function move() {
-        var p = context.mouse();
+        var currMouse = context.mouse(),
+            origMouse = context.projection(origin),
+            delta = vecSub(currMouse, origMouse),
+            action = iD.actions.Move(entityIDs, delta, context.projection, cache);

-        var delta = origin ?
-            [p[0] - context.projection(origin)[0],
-                p[1] - context.projection(origin)[1]] :
-            [0, 0];
+        context.overwrite(action, annotation);

-        var nudge = edge(p, context.map().dimensions());
+        var nudge = edge(currMouse, context.map().dimensions());
        if (nudge) startNudge(nudge);
        else stopNudge();
-
-        origin = context.map().mouseCoordinates();
-
-        context.replace(
-            iD.actions.Move(entityIDs, delta, context.projection),
-            annotation);
    }

    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();
    }

@@ -76,6 +74,9 @@ iD.modes.Move = function(context, entityIDs) {
    }

    mode.enter = function() {
+        origin = context.map().mouseCoordinates();
+        cache = {};
+
        context.install(edit);

        context.perform(
@@ -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({});
@@ -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;
        });