Changeset 15610 in josm


Ignore:
Timestamp:
2019-12-24T11:01:03+01:00 (5 months ago)
Author:
GerdP
Message:

fix #18420: Move node onto ways not working anymore

  • sort result from NavigatableComponent again using higher precision
  • improve handling of multiple target ways with similar distance so that the node is added to the closest segment and also to segments with almost the same position but NOT to segments further away or with a similar distance but different position.
  • fix possible error when multiple nodes are selected
  • use LinkedHashMap for field data to avoid random results from iterator (hope this fixes the unit test problems)
Location:
trunk
Files:
2 added
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/org/openstreetmap/josm/actions/JoinNodeWayAction.java

    r15428 r15610  
    1313import java.util.HashMap;
    1414import java.util.HashSet;
     15import java.util.LinkedHashMap;
    1516import java.util.LinkedList;
    1617import java.util.List;
    1718import java.util.Map;
     19import java.util.Map.Entry;
    1820import java.util.Set;
     21import java.util.TreeMap;
    1922
    2023import javax.swing.JOptionPane;
     
    9194        Collection<Node> selectedNodes = ds.getSelectedNodes();
    9295        Collection<Command> cmds = new LinkedList<>();
    93         Map<Way, MultiMap<Integer, Node>> data = new HashMap<>();
     96        Map<Way, MultiMap<Integer, Node>> data = new LinkedHashMap<>();
    9497
    9598        // If the user has selected some ways, only join the node to these.
     
    100103        for (Node node : selectedNodes) {
    101104            List<WaySegment> wss = mapView.getNearestWaySegments(mapView.getPoint(node), OsmPrimitive::isSelectable);
    102             Set<Way> seenWays = new HashSet<>();
     105            // we cannot trust the order of elements in wss because it was calculated based on rounded position value of node
     106            TreeMap<Double, List<WaySegment>> nearestMap = new TreeMap<>();
     107            EastNorth en = node.getEastNorth();
    103108            for (WaySegment ws : wss) {
    104109                // Maybe cleaner to pass a "isSelected" predicate to getNearestWaySegments, but this is less invasive.
     
    106111                    continue;
    107112                }
    108                 // only use the closest WaySegment of each way and ignore those that already contain the node
    109                 if (!ws.getFirstNode().equals(node) && !ws.getSecondNode().equals(node)
    110                         && !seenWays.contains(ws.way)) {
    111                     MultiMap<Integer, Node> innerMap = data.get(ws.way);
    112                     if (innerMap == null) {
    113                         innerMap = new MultiMap<>();
    114                         data.put(ws.way, innerMap);
     113                /* perpendicular distance squared
     114                 * loose some precision to account for possible deviations in the calculation above
     115                 * e.g. if identical (A and B) come about reversed in another way, values may differ
     116                 * -- zero out least significant 32 dual digits of mantissa..
     117                 */
     118                double distSq = en.distanceSq(Geometry.closestPointToSegment(ws.getFirstNode().getEastNorth(),
     119                        ws.getSecondNode().getEastNorth(), en));
     120                // resolution in numbers with large exponent not needed here..
     121                distSq = Double.longBitsToDouble(Double.doubleToLongBits(distSq) >> 32 << 32);
     122                List<WaySegment> wslist = nearestMap.computeIfAbsent(distSq, k -> new LinkedList<>());
     123                wslist.add(ws);
     124            }
     125            Set<Way> seenWays = new HashSet<>();
     126            Double usedDist = null;
     127            while (!nearestMap.isEmpty()) {
     128                Entry<Double, List<WaySegment>> entry = nearestMap.pollFirstEntry();
     129                if (usedDist != null) {
     130                    double delta = entry.getKey() - usedDist;
     131                    if (delta > 1e-4)
     132                        break;
     133                }
     134                for (WaySegment ws : entry.getValue()) {
     135                    // only use the closest WaySegment of each way and ignore those that already contain the node
     136                    if (!ws.getFirstNode().equals(node) && !ws.getSecondNode().equals(node)
     137                            && !seenWays.contains(ws.way)) {
     138                        if (usedDist == null)
     139                            usedDist = entry.getKey();
     140                        MultiMap<Integer, Node> innerMap = data.get(ws.way);
     141                        if (innerMap == null) {
     142                            innerMap = new MultiMap<>();
     143                            data.put(ws.way, innerMap);
     144                        }
     145                        innerMap.put(ws.lowerIndex, node);
    115146                    }
    116                     innerMap.put(ws.lowerIndex, node);
    117147                    seenWays.add(ws.way);
    118148                }
     
    142172                        if (prevMove != null) {
    143173                            if (!prevMove.equalsEpsilon(newPosition, 1e-4)) {
     174                                // very unlikely: node has same distance to multiple ways which are not nearly overlapping
    144175                                new Notification(tr("Multiple target ways, no common point found. Nothing was changed."))
    145176                                        .setIcon(JOptionPane.INFORMATION_MESSAGE)
  • trunk/test/data/regress/18189/moveontocrossing.osm

    r15428 r15610  
    11<?xml version='1.0' encoding='UTF-8'?>
    22<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'>
     3  <node id='-117848' action='modify' lat='-21.08763547741' lon='-50.39117567184' />
     4  <node id='-117850' action='modify' lat='-21.09088329715' lon='-50.38820000246' />
     5  <node id='-117852' action='modify' lat='-21.09002420335' lon='-50.39142270855' />
     6  <node id='-117854' action='modify' lat='-21.08836886226' lon='-50.38800911046' />
     7  <node id='-117856' action='modify' lat='-21.08922243916' lon='-50.38958051884'>
    88    <tag k='name' v='select me and press N' />
    99  </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' />
     10  <node id='-117858' action='modify' lat='-21.08920644697' lon='-50.38973634946' />
     11  <way id='-117859' action='modify'>
     12    <nd ref='-117848' />
     13    <nd ref='-117858' />
     14    <nd ref='-117850' />
    1515    <tag k='name' v='w1' />
    1616  </way>
    17   <way id='-117826' action='modify' visible='true'>
    18     <nd ref='-117818' />
    19     <nd ref='-117824' />
    20     <nd ref='-117820' />
     17  <way id='-117860' action='modify'>
     18    <nd ref='-117852' />
     19    <nd ref='-117858' />
     20    <nd ref='-117854' />
    2121    <tag k='name' v='w2' />
    2222  </way>
  • trunk/test/unit/org/openstreetmap/josm/actions/JoinNodeWayActionTest.java

    r15575 r15610  
    66import java.awt.Rectangle;
    77import java.util.Arrays;
     8import java.util.Collections;
    89import java.util.List;
    910import java.util.stream.Collectors;
    1011
    11 import org.junit.Ignore;
    1212import org.junit.Rule;
    1313import org.junit.Test;
     
    3737    @Rule
    3838    @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
    39     public JOSMTestRules test = new JOSMTestRules().projection().main().preferences().projection();
     39    public JOSMTestRules test = new JOSMTestRules().projection().main().preferences();
    4040
    4141    private void setupMapView(DataSet ds) {
    42         // setup a reasonable screen size
    43         MainApplication.getMap().mapView.setBounds(new Rectangle(1920, 1080));
     42        // setup a reasonable size for the edit window
     43        MainApplication.getMap().mapView.setBounds(new Rectangle(1345, 939));
    4444        if (ds.getDataSourceBoundingBox() != null) {
    4545            MainApplication.getMap().mapView.zoomTo(ds.getDataSourceBoundingBox());
     
    5959     */
    6060    @Test
    61     @Ignore
    6261    public void testTicket18189() throws Exception {
    6362        DataSet dataSet = new DataSet();
     
    105104     */
    106105    @Test
    107     @Ignore
    108106    public void testTicket11508() throws Exception {
    109107        DataSet ds = OsmReader.parseDataSet(TestUtils.getRegressionDataStream(11508, "11508_example.osm"), null);
     
    134132     */
    135133    @Test
    136     @Ignore
    137134    public void testTicket18189Crossing() throws Exception {
    138135        DataSet ds = OsmReader.parseDataSet(TestUtils.getRegressionDataStream(18189, "moveontocrossing.osm"), null);
     
    160157     */
    161158    @Test
    162     @Ignore
    163159    public void testTicket18189ThreeWays() throws Exception {
    164160        DataSet ds = OsmReader.parseDataSet(TestUtils.getRegressionDataStream(18189, "data.osm"), null);
     
    184180    }
    185181
     182    /**
     183     * Non-regression test for <a href="https://josm.openstreetmap.de/ticket/18420">Bug #18420</a>.
     184     * @throws Exception if an error occurs
     185     */
     186    @Test
     187    public void testTicket18420() throws Exception {
     188        DataSet ds = OsmReader.parseDataSet(TestUtils.getRegressionDataStream(18420, "user-sample.osm"), null);
     189        Layer layer = new OsmDataLayer(ds, OsmDataLayer.createNewName(), null);
     190        MainApplication.getLayerManager().addLayer(layer);
     191        try {
     192            List<Node> nodesToMove = ds.getNodes().stream().filter(n -> n.hasTag("name")).collect(Collectors.toList());
     193            assertTrue(nodesToMove.size() == 2);
     194            Node n = nodesToMove.iterator().next();
     195            if (!n.hasTag("name", "select me 1st"))
     196                Collections.reverse(nodesToMove);
     197            Node toMove1 = nodesToMove.get(0);
     198            Node toMove2 = nodesToMove.get(1);
     199            Node expected1 = new Node(new LatLon(49.8546658263727, 6.206059532463773));
     200            Node expected2 = new Node(new LatLon(49.854738602108085, 6.206213646054511));
     201            ds.setSelected(nodesToMove);
     202            setupMapView(ds);
     203            JoinNodeWayAction action = JoinNodeWayAction.createMoveNodeOntoWayAction();
     204            action.setEnabled(true);
     205            action.actionPerformed(null);
     206            assertTrue("Node was moved to an unexpected position", toMove1.getEastNorth().equalsEpsilon(expected1.getEastNorth(), 1e-7));
     207            assertTrue("Node was moved to an unexpected position", toMove2.getEastNorth().equalsEpsilon(expected2.getEastNorth(), 1e-7));
     208            assertTrue("Node was not added to expected number of ways", toMove1.getParentWays().size() == 2);
     209            assertTrue("Node was not added to expected number of ways", toMove2.getParentWays().size() == 2);
     210        } finally {
     211            MainApplication.getLayerManager().removeLayer(layer);
     212        }
     213    }
     214
    186215}
Note: See TracChangeset for help on using the changeset viewer.