From 9b200cf514b9b5e232f3b71aa7eeb8c7913f115d Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Sun, 28 Jan 2018 23:56:03 -0500 Subject: [PATCH] Code cleanups, changes to replaceMovedVertex in moveAction Trying to eliminate the minimum 10m distance for replacing the original junction vertex - this is a small threshold. When _not_ replacing this vertex, the unzorro code will wreck havok on a traffic circle. (If there is no suitable vertex to use, it will snap to a point between the nearest vertices, see #4146) --- modules/actions/move.js | 132 +++++++++++++++++++++++-------------- modules/modes/move.js | 15 +---- modules/operations/move.js | 14 ++-- 3 files changed, 91 insertions(+), 70 deletions(-) diff --git a/modules/actions/move.js b/modules/actions/move.js index 1f400085c..8699b0672 100644 --- a/modules/actions/move.js +++ b/modules/actions/move.js @@ -1,10 +1,7 @@ -import _each from 'lodash-es/each'; import _every from 'lodash-es/every'; import _filter from 'lodash-es/filter'; -import _find from 'lodash-es/find'; import _intersection from 'lodash-es/intersection'; import _isEqual from 'lodash-es/isEqual'; -import _isEmpty from 'lodash-es/isEmpty'; import _map from 'lodash-es/map'; import _without from 'lodash-es/without'; @@ -44,12 +41,13 @@ export function actionMove(moveIds, tryDelta, projection, cache) { } function cacheEntities(ids) { - ids.forEach(function(id) { - if (cache.moving[id]) return; + for (var i = 0; i < ids.length; i++) { + var id = ids[i]; + if (cache.moving[id]) continue; cache.moving[id] = true; var entity = graph.hasEntity(id); - if (!entity) return; + if (!entity) continue; if (entity.type === 'node') { cache.nodes.push(id); @@ -62,37 +60,48 @@ export function actionMove(moveIds, tryDelta, projection, cache) { return member.id; })); } - }); + } } function cacheIntersections(ids) { - function isEndpoint(way, id) { return !way.isClosed() && !!way.affix(id); } + function isEndpoint(way, id) { + return !way.isClosed() && !!way.affix(id); + } + + for (var i = 0; i < ids.length; i++) { + var id = ids[i]; - ids.forEach(function(id) { // consider only intersections with 1 moved and 1 unmoved way. var childNodes = graph.childNodes(graph.entity(id)); - childNodes.forEach(function(node) { + for (var j = 0; j < childNodes.length; j++) { + var node = childNodes[j]; var parents = graph.parentWays(node); - if (parents.length !== 2) return; + if (parents.length !== 2) continue; - var moved = graph.entity(id), - unmoved = _find(parents, function(way) { return !cache.moving[way.id]; }); - if (!unmoved) return; + var moved = graph.entity(id); + var unmoved = null; + for (var k = 0; k < parents.length; k++) { + var way = parents[k]; + if (!cache.moving[way.id]) { + unmoved = way; + break; + } + } + if (!unmoved) continue; // exclude ways that are overly connected.. - if (_intersection(moved.nodes, unmoved.nodes).length > 2) return; + if (_intersection(moved.nodes, unmoved.nodes).length > 2) continue; + if (moved.isArea() || unmoved.isArea()) continue; - if (moved.isArea() || unmoved.isArea()) return; - - cache.intersection[node.id] = { + cache.intersections.push({ nodeId: node.id, movedId: moved.id, unmovedId: unmoved.id, movedIsEP: isEndpoint(moved, node.id), unmovedIsEP: isEndpoint(unmoved, node.id) - }; - }); - }); + }); + } + } } @@ -101,7 +110,7 @@ export function actionMove(moveIds, tryDelta, projection, cache) { } if (!cache.ok) { cache.moving = {}; - cache.intersection = {}; + cache.intersections = []; cache.replacedVertex = {}; cache.startLoc = {}; cache.nodes = []; @@ -117,7 +126,7 @@ export function actionMove(moveIds, tryDelta, projection, cache) { // Place a vertex where the moved vertex used to be, to preserve way shape.. - function replaceMovedVertex(nodeId, wayId, graph, _delta) { + function replaceMovedVertex(nodeId, wayId, graph, delta) { var way = graph.entity(wayId); var moved = graph.entity(nodeId); var movedIndex = way.nodes.indexOf(nodeId); @@ -148,9 +157,9 @@ export function actionMove(moveIds, tryDelta, projection, cache) { } var start, end; - if (_delta) { + if (delta) { start = projection(cache.startLoc[nodeId]); - end = projection.invert(geoVecAdd(start, _delta)); + end = projection.invert(geoVecAdd(start, delta)); } else { end = cache.startLoc[nodeId]; } @@ -162,16 +171,16 @@ export function actionMove(moveIds, tryDelta, projection, cache) { // 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 (geoSphericalDistance(prev.loc, orig.loc) < 10 || - geoSphericalDistance(orig.loc, next.loc) < 10) return graph; + // // Don't add orig vertex if another point is already nearby (within 10m) + // if (geoSphericalDistance(prev.loc, orig.loc) < 10 || + // geoSphericalDistance(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 = geoPathLength(p1), - d2 = geoPathLength(p2), - insertAt = (d1 < d2) ? movedIndex : nextIndex; + var p1 = [prev.loc, orig.loc, moved.loc, next.loc].map(projection); + var p2 = [prev.loc, moved.loc, orig.loc, next.loc].map(projection); + var d1 = geoPathLength(p1); + var d2 = geoPathLength(p2); + var insertAt = (d1 <= d2) ? movedIndex : nextIndex; // moving around closed loop? if (way.isClosed() && insertAt === 0) insertAt = len; @@ -182,6 +191,22 @@ export function actionMove(moveIds, tryDelta, projection, cache) { // Reorder nodes around intersections that have moved.. + // + // Start: way1.nodes: b,e (moving) + // a - b - c ----- d way2.nodes: a,b,c,d (static) + // | vertex: b + // e isEP1: true, isEP2, false + // + // way1 `b,e` moved here: + // a ----- c = b - d + // | + // e + // + // reorder nodes way1.nodes: b,e + // a ----- c - b - d way2.nodes: a,c,b,d + // | + // e + // function unZorroIntersection(intersection, graph) { var vertex = graph.entity(intersection.nodeId); var way1 = graph.entity(intersection.movedId); @@ -204,7 +229,7 @@ export function actionMove(moveIds, tryDelta, projection, cache) { // snap vertex to nearest edge (or some point between them).. if (!isEP1 && !isEP2) { - var epsilon = 1e-4, maxIter = 10; + var epsilon = 1e-6, maxIter = 10; for (var i = 0; i < maxIter; i++) { loc = geoVecInterp(edge1.loc, edge2.loc, 0.5); edge1 = geoChooseEdge(nodes1, projection(loc), projection); @@ -234,41 +259,46 @@ export function actionMove(moveIds, tryDelta, projection, cache) { function cleanupIntersections(graph) { - _each(cache.intersection, function(obj) { + for (var i = 0; i < cache.intersections.length; i++) { + var obj = cache.intersections[i]; 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.. + // check if moving way endpoint can cross an unmoved way, if so limit delta.. function limitDelta(graph) { - _each(cache.intersection, function(obj) { + function moveNode(loc) { + return geoVecAdd(projection(loc), _delta); + } + + for (var i = 0; i < cache.intersections.length; i++) { + var obj = cache.intersections[i]; + // Don't limit movement if this is vertex joins 2 endpoints.. - if (obj.movedIsEP && obj.unmovedIsEP) return; + if (obj.movedIsEP && obj.unmovedIsEP) continue; // Don't limit movement if this vertex is not an endpoint anyway.. - if (!obj.movedIsEP) return; + if (!obj.movedIsEP) continue; var node = graph.entity(obj.nodeId); var start = projection(node.loc); var end = geoVecAdd(start, _delta); var movedNodes = graph.childNodes(graph.entity(obj.movedId)); - var movedPath = _map(_map(movedNodes, 'loc'), function(loc) { - return geoVecAdd(projection(loc), _delta); - }); + var movedPath = _map(_map(movedNodes, 'loc'), moveNode); var unmovedNodes = graph.childNodes(graph.entity(obj.unmovedId)); var unmovedPath = _map(_map(unmovedNodes, 'loc'), projection); var hits = geoPathIntersections(movedPath, unmovedPath); - for (var i = 0; i < hits.length; i++) { - if (_isEqual(hits[i], end)) continue; + for (var j = 0; i < hits.length; i++) { + if (_isEqual(hits[j], end)) continue; var edge = geoChooseEdge(unmovedNodes, end, projection); _delta = geoVecSubtract(projection(edge.loc), start); } - }); + } } @@ -277,18 +307,18 @@ export function actionMove(moveIds, tryDelta, projection, cache) { setupCache(graph); - if (!_isEmpty(cache.intersection)) { + if (cache.intersections.length) { limitDelta(graph); } - _each(cache.nodes, function(id) { - var node = graph.entity(id); + for (var i = 0; i < cache.nodes.length; i++) { + var node = graph.entity(cache.nodes[i]); var start = projection(node.loc); var end = geoVecAdd(start, _delta); graph = graph.replace(node.move(projection.invert(end))); - }); + } - if (!_isEmpty(cache.intersection)) { + if (cache.intersections.length) { graph = cleanupIntersections(graph); } diff --git a/modules/modes/move.js b/modules/modes/move.js index e91439b1b..8cfbbe6b7 100644 --- a/modules/modes/move.js +++ b/modules/modes/move.js @@ -8,12 +8,8 @@ import { t } from '../util/locale'; import { actionMove } from '../actions'; import { behaviorEdit } from '../behavior'; -import { geoViewportEdge } from '../geo'; - -import { - modeBrowse, - modeSelect -} from './index'; +import { geoViewportEdge, geoVecSubtract } from '../geo'; +import { modeBrowse, modeSelect } from './index'; import { operationCircularize, @@ -51,11 +47,6 @@ export function modeMove(context, entityIDs, baseGraph) { var _nudgeInterval; - function vecSub(a, b) { - return [a[0] - b[0], a[1] - b[1]]; - } - - function doMove(nudge) { nudge = nudge || [0, 0]; @@ -70,7 +61,7 @@ export function modeMove(context, entityIDs, baseGraph) { var currMouse = context.mouse(); var origMouse = context.projection(_origin); - var delta = vecSub(vecSub(currMouse, origMouse), nudge); + var delta = geoVecSubtract(geoVecSubtract(currMouse, origMouse), nudge); fn(actionMove(entityIDs, delta, context.projection, _cache), annotation); _prevGraph = context.graph(); diff --git a/modules/operations/move.js b/modules/operations/move.js index fd09a2783..a5cc74e7f 100644 --- a/modules/operations/move.js +++ b/modules/operations/move.js @@ -1,16 +1,16 @@ import _some from 'lodash-es/some'; import { t } from '../util/locale'; -import { behaviorOperation } from '../behavior/index'; -import { geoExtent } from '../geo/index'; -import { modeMove } from '../modes/index'; +import { behaviorOperation } from '../behavior'; +import { geoExtent } from '../geo'; +import { modeMove } from '../modes'; export function operationMove(selectedIDs, context) { - var multi = (selectedIDs.length === 1 ? 'single' : 'multiple'), - extent = selectedIDs.reduce(function(extent, id) { - return extent.extend(context.entity(id).extent(context.graph())); - }, geoExtent()); + var multi = (selectedIDs.length === 1 ? 'single' : 'multiple'); + var extent = selectedIDs.reduce(function(extent, id) { + return extent.extend(context.entity(id).extent(context.graph())); + }, geoExtent()); var operation = function() {