Ticket #18189: 18189-v3.patch

File 18189-v3.patch, 20.7 KB (added by GerdP, 7 years ago)

simplify algo to find the right way segment

  • src/org/openstreetmap/josm/actions/JoinNodeWayAction.java

     
    1111import java.util.Collections;
    1212import java.util.Comparator;
    1313import java.util.HashMap;
     14import java.util.HashSet;
     15import java.util.LinkedHashMap;
    1416import java.util.LinkedList;
    1517import java.util.List;
    1618import java.util.Map;
    1719import java.util.Set;
    18 import java.util.SortedSet;
    19 import java.util.TreeSet;
    2020
     21import javax.swing.JOptionPane;
     22
    2123import org.openstreetmap.josm.command.ChangeCommand;
    2224import org.openstreetmap.josm.command.Command;
    2325import org.openstreetmap.josm.command.MoveCommand;
    … …  
    3234import org.openstreetmap.josm.data.projection.ProjectionRegistry;
    3335import org.openstreetmap.josm.gui.MainApplication;
    3436import org.openstreetmap.josm.gui.MapView;
     37import org.openstreetmap.josm.gui.Notification;
    3538import org.openstreetmap.josm.tools.Geometry;
    3639import org.openstreetmap.josm.tools.MultiMap;
    3740import org.openstreetmap.josm.tools.Shortcut;
    … …  
    8891        DataSet ds = getLayerManager().getEditDataSet();
    8992        Collection<Node> selectedNodes = ds.getSelectedNodes();
    9093        Collection<Command> cmds = new LinkedList<>();
    91         Map<Way, MultiMap<Integer, Node>> data = new HashMap<>();
     94        Map<Way, MultiMap<Integer, Node>> data = new LinkedHashMap<>();
    9295
    9396        // If the user has selected some ways, only join the node to these.
    9497        boolean restrictToSelectedWays = !ds.getSelectedWays().isEmpty();
    … …  
    97100        MapView mapView = MainApplication.getMap().mapView;
    98101        for (Node node : selectedNodes) {
    99102            List<WaySegment> wss = mapView.getNearestWaySegments(mapView.getPoint(node), OsmPrimitive::isSelectable);
    100             MultiMap<Way, Integer> insertPoints = new MultiMap<>();
     103            Set<Way> seenWays = new HashSet<>();
    101104            for (WaySegment ws : wss) {
    102105                // Maybe cleaner to pass a "isSelected" predicate to getNearestWaySegments, but this is less invasive.
    103106                if (restrictToSelectedWays && !ws.way.isSelected()) {
    104107                    continue;
    105108                }
    106 
    107                 if (!ws.getFirstNode().equals(node) && !ws.getSecondNode().equals(node)) {
    108                     insertPoints.put(ws.way, ws.lowerIndex);
    109                 }
    110             }
    111             for (Map.Entry<Way, Set<Integer>> entry : insertPoints.entrySet()) {
    112                 final Way w = entry.getKey();
    113                 final Set<Integer> insertPointsForWay = entry.getValue();
    114                 for (int i : pruneSuccs(insertPointsForWay)) {
    115                     MultiMap<Integer, Node> innerMap;
    116                     if (!data.containsKey(w)) {
     109                // only use the closest WaySegment of each way and ignore those that already contain the node
     110                if (!ws.getFirstNode().equals(node) && !ws.getSecondNode().equals(node)
     111                        && !seenWays.contains(ws.way)) {
     112                    MultiMap<Integer, Node> innerMap = data.get(ws.way);
     113                    if (innerMap == null) {
    117114                        innerMap = new MultiMap<>();
    118                     } else {
    119                         innerMap = data.get(w);
     115                        data.put(ws.way, innerMap);
    120116                    }
    121                     innerMap.put(i, node);
    122                     data.put(w, innerMap);
     117                    innerMap.put(ws.lowerIndex, node);
     118                    seenWays.add(ws.way);
    123119                }
    124120            }
    125121        }
    126122
    127123        // Execute phase: traverse the structure "data" and finally put the nodes into place
     124        Map<Node, EastNorth> movedNodes = new HashMap<>();
    128125        for (Map.Entry<Way, MultiMap<Integer, Node>> entry : data.entrySet()) {
    129126            final Way w = entry.getKey();
    130127            final MultiMap<Integer, Node> innerEntry = entry.getValue();
    … …  
    142139                                w.getNode(segmentIndex).getEastNorth(),
    143140                                w.getNode(segmentIndex+1).getEastNorth(),
    144141                                node.getEastNorth());
    145                         MoveCommand c = new MoveCommand(
    146                                 node, ProjectionRegistry.getProjection().eastNorth2latlon(newPosition));
    147                         // Avoid moving a given node several times at the same position in case of overlapping ways
    148                         if (!cmds.contains(c)) {
    149                             cmds.add(c);
     142                        EastNorth prevMove = movedNodes.get(node);
     143                        if (prevMove != null) {
     144                            if (!prevMove.equalsEpsilon(newPosition, 1e-4)) {
     145                                new Notification(tr("Multiple target ways, no common point found. Nothing was changed."))
     146                                        .setIcon(JOptionPane.INFORMATION_MESSAGE)
     147                                        .show();
     148                                return;
     149                            }
     150                            continue;
    150151                        }
     152                        MoveCommand c = new MoveCommand(node,
     153                                ProjectionRegistry.getProjection().eastNorth2latlon(newPosition));
     154                        cmds.add(c);
     155                        movedNodes.put(node, newPosition);
    151156                    }
    152157                }
    153158                List<Node> nodesToAdd = new LinkedList<>();
    … …  
    165170        UndoRedoHandler.getInstance().add(new SequenceCommand(getValue(NAME).toString(), cmds));
    166171    }
    167172
    168     private static SortedSet<Integer> pruneSuccs(Collection<Integer> is) {
    169         SortedSet<Integer> is2 = new TreeSet<>();
    170         for (int i : is) {
    171             if (!is2.contains(i - 1) && !is2.contains(i + 1)) {
    172                 is2.add(i);
    173             }
    174         }
    175         return is2;
    176     }
    177 
    178173    /**
    179174     * Sorts collinear nodes by their distance to a common reference node.
    180175     */

  • test/data/regress/11508/11508_example.osm

     
     1<?xml version='1.0' encoding='UTF-8'?>
     2<osm version='0.6' upload='never' generator='JOSM'>
     3  <bounds minlat='47.5627143' minlon='8.7998664' maxlat='47.5636555' maxlon='8.8013148' origin='CGImap 0.4.0 (30121 thorn-01.openstreetmap.org)' />
     4  <bounds minlat='47.5627143' minlon='8.7998664' maxlat='47.5636555' maxlon='8.8013148' origin='OpenStreetMap server' />
     5  <node id='-150' action='modify' visible='true' lat='47.5632954399' lon='8.80074575728' />
     6  <node id='-149' action='modify' visible='true' lat='47.56334946504' lon='8.80075650412' />
     7  <node id='-95' action='modify' visible='true' lat='47.56329197754' lon='8.80078398419' />
     8  <node id='-94' action='modify' visible='true' lat='47.56328823282' lon='8.80082532865' />
     9  <node id='-93' action='modify' visible='true' lat='47.56334225796' lon='8.80083607549' />
     10  <node id='-92' action='modify' visible='true' lat='47.56334600268' lon='8.80079473103' />
     11  <node id='-21' action='modify' visible='true' lat='47.56331891179' lon='8.8007846789'>
     12    <tag k='name' v='select me and press N' />
     13  </node>
     14  <way id='-151' action='modify' visible='true'>
     15    <nd ref='-95' />
     16    <nd ref='-150' />
     17    <nd ref='-149' />
     18    <nd ref='-92' />
     19    <nd ref='-95' />
     20    <tag k='name' v='b1' />
     21    <tag k='building' v='yes' />
     22  </way>
     23  <way id='-91' visible='true'>
     24    <nd ref='-92' />
     25    <nd ref='-93' />
     26    <nd ref='-94' />
     27    <nd ref='-95' />
     28    <nd ref='-92' />
     29    <tag k='building' v='yes' />
     30    <tag k='name' v='b2' />
     31  </way>
     32</osm>

  • test/data/regress/18189/data.osm

     
     1<?xml version='1.0' encoding='UTF-8'?>
     2<osm version='0.6' upload='never' generator='JOSM'>
     3  <node id='-102246' action='modify' visible='true' lat='-21.09080213032' lon='-50.38733331697' />
     4  <node id='-102249' action='modify' visible='true' lat='-21.08854905386' lon='-50.38603158022' />
     5  <node id='-102254' action='modify' visible='true' lat='-21.08931905396' lon='-50.38647645301' />
     6  <node id='-102256' action='modify' visible='true' lat='-21.08993744825' lon='-50.38524692707' />
     7  <node id='-102258' action='modify' visible='true' lat='-21.09098595234' lon='-50.3869678287' />
     8  <node id='-102262' action='modify' visible='true' lat='-21.08871918838' lon='-50.38569331158' />
     9  <node id='-102264' action='modify' visible='true' lat='-21.08844814677' lon='-50.38623220779' />
     10  <node id='-102268' action='modify' visible='true' lat='-21.09066223204' lon='-50.38761147258' />
     11  <node id='-102271' action='modify' visible='true' lat='-21.08885730223' lon='-50.38657306493' />
     12  <node id='-102273' action='modify' visible='true' lat='-21.088988263' lon='-50.38631058843'>
     13    <tag k='name' v='select me and press N' />
     14  </node>
     15  <way id='-102274' action='modify' visible='true'>
     16    <nd ref='-102246' />
     17    <nd ref='-102254' />
     18    <nd ref='-102249' />
     19  </way>
     20  <way id='-102275' action='modify' visible='true'>
     21    <nd ref='-102254' />
     22    <nd ref='-102256' />
     23  </way>
     24  <way id='-102276' action='modify' visible='true'>
     25    <nd ref='-102258' />
     26    <nd ref='-102246' />
     27    <nd ref='-102254' />
     28    <nd ref='-102249' />
     29    <nd ref='-102262' />
     30  </way>
     31  <way id='-102277' action='modify' visible='true'>
     32    <nd ref='-102264' />
     33    <nd ref='-102249' />
     34    <nd ref='-102254' />
     35    <nd ref='-102246' />
     36    <nd ref='-102268' />
     37  </way>
     38  <way id='-102278' action='modify' visible='true'>
     39    <nd ref='-102271' />
     40    <nd ref='-102273' />
     41  </way>
     42</osm>

  • test/data/regress/18189/moveontocrossing.osm

     
     1<?xml version='1.0' encoding='UTF-8'?>
     2<osm version='0.6' upload='never' generator='JOSM'>
     3  <node id='-117814' action='modify' visible='true' lat='-21.08763547741' lon='-50.39117567184' />
     4  <node id='-117816' action='modify' visible='true' lat='-21.09088329715' lon='-50.38820000246' />
     5  <node id='-117818' action='modify' visible='true' lat='-21.09002420335' lon='-50.39142270855' />
     6  <node id='-117820' action='modify' visible='true' lat='-21.08836886226' lon='-50.38800911046' />
     7  <node id='-117822' action='modify' visible='true' lat='-21.089215371' lon='-50.38971309121'>
     8    <tag k='name' v='select me and press N' />
     9  </node>
     10  <node id='-117824' action='modify' visible='true' lat='-21.08920644697' lon='-50.38973634946' />
     11  <way id='-117825' action='modify' visible='true'>
     12    <nd ref='-117814' />
     13    <nd ref='-117824' />
     14    <nd ref='-117816' />
     15    <tag k='name' v='w1' />
     16  </way>
     17  <way id='-117826' action='modify' visible='true'>
     18    <nd ref='-117818' />
     19    <nd ref='-117824' />
     20    <nd ref='-117820' />
     21    <tag k='name' v='w2' />
     22  </way>
     23</osm>

  • test/data/regress/18189/moveontoway.osm

     
     1<?xml version='1.0' encoding='UTF-8'?>
     2<osm version='0.6' upload='never' generator='JOSM'>
     3  <node id='-104728' action='modify' visible='true' lat='59.92881498658' lon='30.30104052971' />
     4  <node id='-104729' action='modify' visible='true' lat='59.92881459851' lon='30.30104056556' />
     5  <node id='-104734' action='modify' visible='true' lat='59.92881498658' lon='30.3010405297' />
     6  <node id='-104735' action='modify' visible='true' lat='59.92881459851' lon='30.30104056556' />
     7  <node id='-104756' action='modify' visible='true' lat='59.92881483122' lon='30.30104056465' />
     8  <way id='-104730' action='modify' visible='true'>
     9    <nd ref='-104728' />
     10    <nd ref='-104729' />
     11  </way>
     12  <way id='-104736' action='modify' visible='true'>
     13    <nd ref='-104734' />
     14    <nd ref='-104735' />
     15  </way>
     16</osm>

  • test/unit/org/openstreetmap/josm/actions/JoinNodeWayActionTest.java

     
     1// License: GPL. For details, see LICENSE file.
     2package org.openstreetmap.josm.actions;
     3
     4import static org.junit.Assert.assertTrue;
     5
     6import java.awt.Rectangle;
     7import java.util.Arrays;
     8import java.util.List;
     9import java.util.stream.Collectors;
     10
     11import org.junit.Rule;
     12import org.junit.Test;
     13import org.openstreetmap.josm.TestUtils;
     14import org.openstreetmap.josm.data.coor.EastNorth;
     15import org.openstreetmap.josm.data.coor.LatLon;
     16import org.openstreetmap.josm.data.osm.DataSet;
     17import org.openstreetmap.josm.data.osm.Node;
     18import org.openstreetmap.josm.data.osm.Way;
     19import org.openstreetmap.josm.data.osm.visitor.BoundingXYVisitor;
     20import org.openstreetmap.josm.gui.MainApplication;
     21import org.openstreetmap.josm.gui.layer.Layer;
     22import org.openstreetmap.josm.gui.layer.OsmDataLayer;
     23import org.openstreetmap.josm.io.OsmReader;
     24import org.openstreetmap.josm.testutils.JOSMTestRules;
     25import org.openstreetmap.josm.tools.Geometry;
     26
     27import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
     28
     29/**
     30 * Unit tests for class {@link JoinNodeWayAction}.
     31 */
     32public final class JoinNodeWayActionTest {
     33    /**
     34     * Setup test.
     35     */
     36    @Rule
     37    @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
     38    public JOSMTestRules test = new JOSMTestRules().projection().main().preferences();
     39
     40    private void setupMapView(DataSet ds) {
     41        // setup a reasonable screen size
     42        MainApplication.getMap().mapView.setBounds(new Rectangle(1920, 1080));
     43        if (ds.getDataSourceBoundingBox() != null) {
     44            MainApplication.getMap().mapView.zoomTo(ds.getDataSourceBoundingBox());
     45        } else {
     46            BoundingXYVisitor v = new BoundingXYVisitor();
     47            for (Layer l : MainApplication.getLayerManager().getLayers()) {
     48                l.visitBoundingBox(v);
     49            }
     50            MainApplication.getMap().mapView.zoomTo(v);
     51        }
     52    }
     53
     54    /**
     55     * Test case: Move node onto two almost overlapping ways
     56     * see #18189 moveontoway.osm
     57     * @throws Exception if an error occurs
     58     */
     59    @Test
     60    public void testTicket18189() throws Exception {
     61        DataSet dataSet = new DataSet();
     62        OsmDataLayer layer = new OsmDataLayer(dataSet, OsmDataLayer.createNewName(), null);
     63        MainApplication.getLayerManager().addLayer(layer);
     64        try {
     65            Node n1 = new Node(new LatLon(59.92881498658, 30.30104052971));
     66            Node n2 = new Node(new LatLon(59.92881459851, 30.30104056556));
     67            Node n3 = new Node(new LatLon(59.92881498658, 30.3010405297));
     68            Node n4 = new Node(new LatLon(59.92881459851, 30.30104056556));
     69            Node n5 = new Node(new LatLon(59.92881483122, 30.30104056465));
     70
     71            dataSet.addPrimitive(n1);
     72            dataSet.addPrimitive(n2);
     73            dataSet.addPrimitive(n3);
     74            dataSet.addPrimitive(n4);
     75            dataSet.addPrimitive(n5);
     76
     77            Way w1 = new Way();
     78            w1.setNodes(Arrays.asList(n1, n2));
     79            dataSet.addPrimitive(w1);
     80            Way w2 = new Way();
     81            w2.setNodes(Arrays.asList(n3, n4));
     82            dataSet.addPrimitive(w2);
     83
     84            dataSet.addSelected(n5);
     85            EastNorth expected = Geometry.closestPointToSegment(n1.getEastNorth(), n2.getEastNorth(), n5.getEastNorth());
     86
     87            setupMapView(dataSet);
     88            JoinNodeWayAction action = JoinNodeWayAction.createMoveNodeOntoWayAction();
     89            action.setEnabled(true);
     90            action.actionPerformed(null);
     91            // Make sure the node was only moved once
     92            assertTrue("Node n5 wasn't added to way w1.", w1.containsNode(n5));
     93            assertTrue("Node n5 wasn't added to way w2.", w2.containsNode(n5));
     94            assertTrue("Node was moved to an unexpected position", n5.getEastNorth().equalsEpsilon(expected, 1e-7));
     95        } finally {
     96            MainApplication.getLayerManager().removeLayer(layer);
     97        }
     98    }
     99
     100    /**
     101     * Non-regression test for <a href="https://josm.openstreetmap.de/ticket/11508">Bug #11508</a>.
     102     * @throws Exception if an error occurs
     103     */
     104    @Test
     105    public void testTicket11508() throws Exception {
     106        DataSet ds = OsmReader.parseDataSet(TestUtils.getRegressionDataStream(11508, "11508_example.osm"), null);
     107        Layer layer = new OsmDataLayer(ds, OsmDataLayer.createNewName(), null);
     108        MainApplication.getLayerManager().addLayer(layer);
     109        try {
     110            List<Node> nodesToMove = ds.getNodes().stream().filter(n -> n.hasTag("name", "select me and press N"))
     111                    .collect(Collectors.toList());
     112            assertTrue(nodesToMove.size() == 1);
     113            Node toMove = nodesToMove.iterator().next();
     114            Node expected = new Node(new LatLon(47.56331849690742, 8.800789259499311));
     115            ds.setSelected(toMove);
     116            setupMapView(ds);
     117            JoinNodeWayAction action = JoinNodeWayAction.createMoveNodeOntoWayAction();
     118            action.setEnabled(true);
     119            action.actionPerformed(null);
     120
     121            assertTrue("Node was moved to an unexpected position", toMove.getEastNorth().equalsEpsilon(expected.getEastNorth(), 1e-7));
     122            assertTrue("Node was not added to expected number of ways", toMove.getParentWays().size() == 2);
     123        } finally {
     124            MainApplication.getLayerManager().removeLayer(layer);
     125        }
     126    }
     127
     128    /**
     129     * Check that nothing is changed if ways are too far.
     130     * @throws Exception if an error occurs
     131     */
     132    @Test
     133    public void testTicket18189Crossing() throws Exception {
     134        DataSet ds = OsmReader.parseDataSet(TestUtils.getRegressionDataStream(18189, "moveontocrossing.osm"), null);
     135        Layer layer = new OsmDataLayer(ds, OsmDataLayer.createNewName(), null);
     136        MainApplication.getLayerManager().addLayer(layer);
     137        try {
     138            setupMapView(ds);
     139            JoinNodeWayAction action = JoinNodeWayAction.createMoveNodeOntoWayAction();
     140            action.setEnabled(true);
     141            List<Node> nodesToMove = ds.getNodes().stream().filter(n -> n.hasTag("name", "select me and press N"))
     142                    .collect(Collectors.toList());
     143            assertTrue(nodesToMove.size() == 1);
     144            Node toMove = nodesToMove.iterator().next();
     145            ds.setSelected(toMove);
     146            action.actionPerformed(null);
     147            assertTrue(toMove.getParentWays().isEmpty());
     148        } finally {
     149            MainApplication.getLayerManager().removeLayer(layer);
     150        }
     151    }
     152
     153    /**
     154     * Check that nothing is changed if ways are too far.
     155     * @throws Exception if an error occurs
     156     */
     157    @Test
     158    public void testTicket18189ThreeWays() throws Exception {
     159        DataSet ds = OsmReader.parseDataSet(TestUtils.getRegressionDataStream(18189, "data.osm"), null);
     160        Layer layer = new OsmDataLayer(ds, OsmDataLayer.createNewName(), null);
     161        MainApplication.getLayerManager().addLayer(layer);
     162        try {
     163            setupMapView(ds);
     164            JoinNodeWayAction action = JoinNodeWayAction.createMoveNodeOntoWayAction();
     165            action.setEnabled(true);
     166            List<Node> nodesToMove = ds.getNodes().stream().filter(n -> n.hasTag("name", "select me and press N"))
     167                    .collect(Collectors.toList());
     168            assertTrue(nodesToMove.size() == 1);
     169            Node toMove = nodesToMove.iterator().next();
     170            Node expected = new Node(new LatLon(-21.088998104148224, -50.38629102179512));
     171            ds.setSelected(toMove);
     172            action.actionPerformed(null);
     173            assertTrue("Node was moved to an unexpected position", toMove.getEastNorth().equalsEpsilon(expected.getEastNorth(), 1e-7));
     174            assertTrue("Node was not added to expected number of ways", toMove.getParentWays().size() == 4);
     175
     176        } finally {
     177            MainApplication.getLayerManager().removeLayer(layer);
     178        }
     179    }
     180
     181}