Ticket #18367: 18367-v2.patch

File 18367-v2.patch, 9.4 KB (added by GerdP, 18 months ago)
  • src/org/openstreetmap/josm/actions/CombineWayAction.java

     
    3131import org.openstreetmap.josm.data.osm.OsmUtils;
    3232import org.openstreetmap.josm.data.osm.TagCollection;
    3333import org.openstreetmap.josm.data.osm.Way;
     34import org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon;
     35import org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon.JoinedWay;
    3436import org.openstreetmap.josm.data.preferences.BooleanProperty;
     37import org.openstreetmap.josm.data.validation.Test;
     38import org.openstreetmap.josm.data.validation.tests.OverlappingWays;
     39import org.openstreetmap.josm.data.validation.tests.SelfIntersectingWay;
    3540import org.openstreetmap.josm.gui.ExtendedDialog;
    3641import org.openstreetmap.josm.gui.MainApplication;
    3742import org.openstreetmap.josm.gui.Notification;
     
    122127        }
    123128
    124129        // try to build a new way which includes all the combined ways
    125         NodeGraph graph = NodeGraph.createNearlyUndirectedGraphFromNodeWays(ways);
    126         List<Node> path = graph.buildSpanningPathNoRemove();
     130        List<Node> path = tryJoin(ways);
    127131        if (path == null) {
    128132            warnCombiningImpossible();
    129133            return null;
     
    138142        List<Way> unreversedWays = new LinkedList<>();
    139143        for (Way w: ways) {
    140144            // Treat zero or one-node ways as unreversed as Combine action action is a good way to fix them (see #8971)
    141             if (w.getNodesCount() < 2 || (path.indexOf(w.getNode(0)) + 1) == path.lastIndexOf(w.getNode(1))) {
     145            if (w.getNodesCount() < 2) {
    142146                unreversedWays.add(w);
    143147            } else {
    144                 reversedWays.add(w);
     148                boolean foundStartSegment = false;
     149                int last = path.lastIndexOf(w.getNode(0));
     150
     151                for (int i = path.indexOf(w.getNode(0)); i <= last; i++) {
     152                    if (path.get(i) == w.getNode(0) && i + 1 < path.size() && w.getNode(1) == path.get(i + 1)) {
     153                        foundStartSegment = true;
     154                        break;
     155                    }
     156                }
     157                if (foundStartSegment) {
     158                    unreversedWays.add(w);
     159                } else {
     160                    reversedWays.add(w);
     161                }
    145162            }
    146163        }
    147164        // reverse path if all ways have been reversed
     
    212229        return new Pair<>(targetWay, sequenceCommand);
    213230    }
    214231
     232    static List<Node> tryJoin(Collection<Way> ways) {
     233        List<Node> path = joinWithMultipolygonCode(ways);
     234        if (path == null) {
     235            NodeGraph graph = NodeGraph.createNearlyUndirectedGraphFromNodeWays(ways);
     236            path = graph.buildSpanningPathNoRemove();
     237        }
     238        return path;
     239    }
     240
     241    /**
     242     * Use {@link Multipolygon#joinWays(Collection)} to join ways.
     243     * @param ways the ways
     244     * @return List of nodes of the combined ways or null if ways could not be combined to a single way.
     245     * Result may contain overlapping segments.
     246     */
     247    private static List<Node> joinWithMultipolygonCode(Collection<Way> ways) {
     248        // sort so that old unclosed ways appear first
     249        LinkedList<Way> toJoin = new LinkedList<>(ways);
     250        toJoin.sort((o1, o2) -> {
     251            int d = Boolean.compare(o1.isNew(), o2.isNew());
     252            if (d == 0)
     253                d = Boolean.compare(o1.isClosed(), o2.isClosed());
     254            return d;
     255        });
     256        Collection<JoinedWay> list = Multipolygon.joinWays(toJoin);
     257        if (list.size() == 1) {
     258            // ways form a single line string
     259            return new ArrayList<>(list.iterator().next().getNodes());
     260        }
     261        return null;
     262    }
     263
    215264    @Override
    216265    public void actionPerformed(ActionEvent event) {
    217266        final DataSet ds = getLayerManager().getEditDataSet();
     
    237286
    238287        if (combineResult == null)
    239288            return;
     289
    240290        final Way selectedWay = combineResult.a;
    241291        UndoRedoHandler.getInstance().add(combineResult.b);
     292        Test test = new OverlappingWays();
     293        test.startTest(null);
     294        test.visit(combineResult.a);
     295        test.endTest();
     296        if (test.getErrors().isEmpty()) {
     297            test = new SelfIntersectingWay();
     298            test.startTest(null);
     299            test.visit(combineResult.a);
     300            test.endTest();
     301        }
     302        if (!test.getErrors().isEmpty()) {
     303            new Notification(test.getErrors().get(0).getMessage())
     304            .setIcon(JOptionPane.WARNING_MESSAGE)
     305            .show();
     306        }
    242307        if (selectedWay != null) {
    243308            GuiHelper.runInEDT(() -> ds.setSelected(selectedWay));
    244309        }
     
    261326        }
    262327        setEnabled(numWays >= 2);
    263328    }
     329
    264330}
  • test/data/regress/18367/nocombine.osm

     
     1<?xml version='1.0' encoding='UTF-8'?>
     2<osm version='0.6' generator='JOSM' upload='false'>
     3  <node id='-143178' action='modify' visible='true' lat='0.02196380816' lon='-0.02295195551' />
     4  <node id='-143179' action='modify' visible='true' lat='0.02142481903' lon='0.00624329122' />
     5  <node id='-143181' action='modify' visible='true' lat='0.02708420481' lon='0.01828071603' />
     6  <node id='-143183' action='modify' visible='true' lat='0.03597752476' lon='0.01908919979' />
     7  <node id='-143185' action='modify' visible='true' lat='0.03139611761' lon='0.0279825211' />
     8  <node id='-143187' action='modify' visible='true' lat='-0.00696194343' lon='-0.00112289411' />
     9  <way id='-143180' action='modify' visible='true'>
     10    <nd ref='-143178' />
     11    <nd ref='-143179' />
     12    <nd ref='-143181' />
     13    <nd ref='-143183' />
     14    <nd ref='-143185' />
     15  </way>
     16  <way id='-143188' action='modify' visible='true'>
     17    <nd ref='-143187' />
     18    <nd ref='-143179' />
     19    <nd ref='-143181' />
     20    <nd ref='-143185' />
     21  </way>
     22</osm>
  • test/unit/org/openstreetmap/josm/actions/CombineWayActionTest.java

     
    22package org.openstreetmap.josm.actions;
    33
    44import static org.junit.Assert.assertEquals;
    5 import static org.junit.Assert.assertNull;
    65import static org.junit.Assert.assertTrue;
    76
    87import java.io.IOException;
    98import java.io.InputStream;
    109import java.util.ArrayList;
     10import java.util.Collection;
    1111import java.util.Collections;
    1212import java.util.HashSet;
    1313import java.util.List;
     
    7171    public void testTicket18385() throws IOException, IllegalDataException {
    7272        try (InputStream is = TestUtils.getRegressionDataStream(18385, "data.osm")) {
    7373            DataSet ds = OsmReader.parseDataSet(is, null);
    74             NodeGraph graph = NodeGraph.createNearlyUndirectedGraphFromNodeWays(ds.getWays());
    75             List<Node> path = graph.buildSpanningPathNoRemove();
    76             assertNull(path);
     74            List<Node> path = CombineWayAction.tryJoin(ds.getWays());
     75            assertTrue(path != null);
    7776        }
    7877    }
    7978
     
    9089            assertEquals(2, selection.size());
    9190            if (!selection.get(0).isNew())
    9291                Collections.reverse(selection);
    93             NodeGraph graph = NodeGraph.createNearlyUndirectedGraphFromNodeWays(selection);
    94             List<Node> path = graph.buildSpanningPathNoRemove();
     92            double expectedLen = getOriginalLength(selection);
     93            List<Node> path = CombineWayAction.tryJoin(selection);
    9594            assertTrue(path != null);
     95            Way combined = new Way(0);
     96            combined.setNodes(path);
     97            assertEquals(expectedLen, combined.getLength(), 0.01);
    9698        }
    9799    }
    98100
    99101    /**
     102     * Non-regression test for bug #18367 (Lines cannot be combined when they share an overlapping segment)
     103     * @throws IOException if any I/O error occurs
     104     * @throws IllegalDataException if OSM parsing fails
     105     */
     106    @Test
     107    public void testTicket18367() throws IOException, IllegalDataException {
     108        try (InputStream is = TestUtils.getRegressionDataStream(18367, "nocombine.osm")) {
     109            DataSet ds = OsmReader.parseDataSet(is, null);
     110            ArrayList<Way> selection = new ArrayList<>(ds.getWays());
     111            assertEquals(2, selection.size());
     112            double expectedLen = getOriginalLength(selection);
     113            List<Node> path = CombineWayAction.tryJoin(selection);
     114            assertTrue(path != null);
     115            Way combined = new Way(0);
     116            combined.setNodes(path);
     117            assertEquals(expectedLen, combined.getLength(), 1e-7);
     118        }
     119    }
     120
     121    /**
    100122     * Unit test of methods {@link NodePair#equals} and {@link NodePair#hashCode}.
    101123     */
    102124    @Test
     
    106128            .withPrefabValues(Node.class, new Node(1), new Node(2))
    107129            .verify();
    108130    }
     131
     132    private static double getOriginalLength(Collection<Way> ways) {
     133        double len = 0;
     134        for (Way w : ways) {
     135            len += w.getLength();
     136        }
     137        return len;
     138    }
     139
    109140}