Ticket #18367: 18367-v2.patch
File 18367-v2.patch, 9.4 KB (added by , 4 years ago) |
---|
-
src/org/openstreetmap/josm/actions/CombineWayAction.java
31 31 import org.openstreetmap.josm.data.osm.OsmUtils; 32 32 import org.openstreetmap.josm.data.osm.TagCollection; 33 33 import org.openstreetmap.josm.data.osm.Way; 34 import org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon; 35 import org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon.JoinedWay; 34 36 import org.openstreetmap.josm.data.preferences.BooleanProperty; 37 import org.openstreetmap.josm.data.validation.Test; 38 import org.openstreetmap.josm.data.validation.tests.OverlappingWays; 39 import org.openstreetmap.josm.data.validation.tests.SelfIntersectingWay; 35 40 import org.openstreetmap.josm.gui.ExtendedDialog; 36 41 import org.openstreetmap.josm.gui.MainApplication; 37 42 import org.openstreetmap.josm.gui.Notification; … … 122 127 } 123 128 124 129 // 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); 127 131 if (path == null) { 128 132 warnCombiningImpossible(); 129 133 return null; … … 138 142 List<Way> unreversedWays = new LinkedList<>(); 139 143 for (Way w: ways) { 140 144 // 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) { 142 146 unreversedWays.add(w); 143 147 } 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 } 145 162 } 146 163 } 147 164 // reverse path if all ways have been reversed … … 212 229 return new Pair<>(targetWay, sequenceCommand); 213 230 } 214 231 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 215 264 @Override 216 265 public void actionPerformed(ActionEvent event) { 217 266 final DataSet ds = getLayerManager().getEditDataSet(); … … 237 286 238 287 if (combineResult == null) 239 288 return; 289 240 290 final Way selectedWay = combineResult.a; 241 291 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 } 242 307 if (selectedWay != null) { 243 308 GuiHelper.runInEDT(() -> ds.setSelected(selectedWay)); 244 309 } … … 261 326 } 262 327 setEnabled(numWays >= 2); 263 328 } 329 264 330 } -
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
2 2 package org.openstreetmap.josm.actions; 3 3 4 4 import static org.junit.Assert.assertEquals; 5 import static org.junit.Assert.assertNull;6 5 import static org.junit.Assert.assertTrue; 7 6 8 7 import java.io.IOException; 9 8 import java.io.InputStream; 10 9 import java.util.ArrayList; 10 import java.util.Collection; 11 11 import java.util.Collections; 12 12 import java.util.HashSet; 13 13 import java.util.List; … … 71 71 public void testTicket18385() throws IOException, IllegalDataException { 72 72 try (InputStream is = TestUtils.getRegressionDataStream(18385, "data.osm")) { 73 73 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); 77 76 } 78 77 } 79 78 … … 90 89 assertEquals(2, selection.size()); 91 90 if (!selection.get(0).isNew()) 92 91 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); 95 94 assertTrue(path != null); 95 Way combined = new Way(0); 96 combined.setNodes(path); 97 assertEquals(expectedLen, combined.getLength(), 0.01); 96 98 } 97 99 } 98 100 99 101 /** 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 /** 100 122 * Unit test of methods {@link NodePair#equals} and {@link NodePair#hashCode}. 101 123 */ 102 124 @Test … … 106 128 .withPrefabValues(Node.class, new Node(1), new Node(2)) 107 129 .verify(); 108 130 } 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 109 140 }