- Timestamp:
- 2019-09-04T11:43:24+02:00 (5 years ago)
- Location:
- trunk
- Files:
-
- 4 added
- 3 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/src/org/openstreetmap/josm/data/osm/Node.java
r14964 r15335 373 373 for (final Node n : w.getNodes()) { 374 374 final boolean containsN = visited.contains(n); 375 visited.add(n);376 375 if (!containsN && (predicate == null || predicate.test(n)) 377 376 && n.isConnectedTo(otherNodes, hops - 1, predicate, visited)) { -
trunk/src/org/openstreetmap/josm/data/validation/tests/UnconnectedWays.java
r14624 r15335 8 8 import java.awt.geom.Area; 9 9 import java.awt.geom.Line2D; 10 import java.awt.geom.Point2D;11 10 import java.util.ArrayList; 12 11 import java.util.Collection; … … 47 46 public abstract class UnconnectedWays extends Test { 48 47 private final int code; 48 private final boolean isHighwayTest; 49 50 protected abstract boolean isCandidate(OsmPrimitive p); 51 52 @Override 53 public boolean isPrimitiveUsable(OsmPrimitive p) { 54 return super.isPrimitiveUsable(p) && ((partialSelection && p instanceof Node) || isCandidate(p)); 55 } 49 56 50 57 /** … … 58 65 */ 59 66 public UnconnectedHighways() { 60 super(tr("Unconnected highways"), UNCONNECTED_HIGHWAYS );67 super(tr("Unconnected highways"), UNCONNECTED_HIGHWAYS, true); 61 68 } 62 69 63 70 @Override 64 p ublic boolean isPrimitiveUsable(OsmPrimitive p) {65 return super.isPrimitiveUsable(p) &&p.hasKey(HIGHWAY);71 protected boolean isCandidate(OsmPrimitive p) { 72 return p.hasKey(HIGHWAY); 66 73 } 67 74 } … … 76 83 */ 77 84 public UnconnectedRailways() { 78 super(tr("Unconnected railways"), UNCONNECTED_RAILWAYS );85 super(tr("Unconnected railways"), UNCONNECTED_RAILWAYS, false); 79 86 } 80 87 81 88 @Override 82 p ublic boolean isPrimitiveUsable(OsmPrimitive p) {83 return super.isPrimitiveUsable(p) && p.hasKey("railway");89 protected boolean isCandidate(OsmPrimitive p) { 90 return p.hasKey(RAILWAY) && !p.hasTag(RAILWAY, "abandoned"); 84 91 } 85 92 } … … 94 101 */ 95 102 public UnconnectedWaterways() { 96 super(tr("Unconnected waterways"), UNCONNECTED_WATERWAYS );103 super(tr("Unconnected waterways"), UNCONNECTED_WATERWAYS, false); 97 104 } 98 105 99 106 @Override 100 p ublic boolean isPrimitiveUsable(OsmPrimitive p) {101 return super.isPrimitiveUsable(p) &&p.hasKey("waterway");107 protected boolean isCandidate(OsmPrimitive p) { 108 return p.hasKey("waterway"); 102 109 } 103 110 } … … 112 119 */ 113 120 public UnconnectedNaturalOrLanduse() { 114 super(tr("Unconnected natural lands and landuses"), UNCONNECTED_NATURAL_OR_LANDUSE );121 super(tr("Unconnected natural lands and landuses"), UNCONNECTED_NATURAL_OR_LANDUSE, false); 115 122 } 116 123 117 124 @Override 118 p ublic boolean isPrimitiveUsable(OsmPrimitive p) {119 return super.isPrimitiveUsable(p) && p.hasKey("natural", "landuse");125 protected boolean isCandidate(OsmPrimitive p) { 126 return p.hasKey("natural", "landuse") && !p.hasTag("natural", "tree_row", "cliff"); 120 127 } 121 128 } … … 130 137 */ 131 138 public UnconnectedPower() { 132 super(tr("Unconnected power ways"), UNCONNECTED_POWER );139 super(tr("Unconnected power ways"), UNCONNECTED_POWER, false); 133 140 } 134 141 135 142 @Override 136 p ublic boolean isPrimitiveUsable(OsmPrimitive p) {137 return super.isPrimitiveUsable(p) &&p.hasTag("power", "line", "minor_line", "cable");143 protected boolean isCandidate(OsmPrimitive p) { 144 return p.hasTag("power", "line", "minor_line", "cable"); 138 145 } 139 146 } … … 142 149 protected static final String PREFIX = ValidatorPrefHelper.PREFIX + "." + UnconnectedWays.class.getSimpleName(); 143 150 144 private Set<MyWaySegment> ways; 145 private QuadBuckets<Node> endnodes; // nodes at end of way 146 private QuadBuckets<Node> endnodesHighway; // nodes at end of way 147 private QuadBuckets<Node> middlenodes; // nodes in middle of way 151 private List<MyWaySegment> waySegments; 152 private Set<Node> endnodes; // nodes at end of way 153 private Set<Node> middlenodes; // nodes in middle of way 148 154 private Set<Node> othernodes; // nodes appearing at least twice 155 private QuadBuckets<Node> searchNodes; 156 private Set<Way> waysToTest; 157 private Set<Node> nodesToTest; 149 158 private Area dsArea; 150 159 151 160 private double mindist; 152 161 private double minmiddledist; 162 private DataSet ds; 153 163 154 164 /** … … 158 168 */ 159 169 public UnconnectedWays(String title) { 160 this(title, UNCONNECTED_WAYS );170 this(title, UNCONNECTED_WAYS, false); 161 171 162 172 } … … 166 176 * @param title The test title 167 177 * @param code The test code 178 * @param isHighwayTest use {@code true} if test concerns highways or railways 168 179 * @since 14468 169 180 */ 170 public UnconnectedWays(String title, int code ) {181 public UnconnectedWays(String title, int code, boolean isHighwayTest) { 171 182 super(title, tr("This test checks if a way has an endpoint very near to another way.")); 172 183 this.code = code; 184 this.isHighwayTest = isHighwayTest; 173 185 } 174 186 … … 176 188 public void startTest(ProgressMonitor monitor) { 177 189 super.startTest(monitor); 178 ways = new HashSet<>(); 179 endnodes = new QuadBuckets<>(); 180 endnodesHighway = new QuadBuckets<>(); 181 middlenodes = new QuadBuckets<>(); 190 waySegments = new ArrayList<>(); 191 waysToTest = new HashSet<>(); 192 nodesToTest = new HashSet<>(); 193 endnodes = new HashSet<>(); 194 middlenodes = new HashSet<>(); 182 195 othernodes = new HashSet<>(); 183 196 mindist = Config.getPref().getDouble(PREFIX + ".node_way_distance", 10.0); 184 197 minmiddledist = Config.getPref().getDouble(PREFIX + ".way_way_distance", 0.0); 185 DataSet dataSet= OsmDataManager.getInstance().getEditDataSet();186 dsArea = d ataSet == null ? null : dataSet.getDataSourceArea();198 ds = OsmDataManager.getInstance().getEditDataSet(); 199 dsArea = ds == null ? null : ds.getDataSourceArea(); 187 200 } 188 201 189 202 protected Map<Node, Way> getWayEndNodesNearOtherHighway() { 190 203 Map<Node, Way> map = new HashMap<>(); 191 for (int iter = 0; iter < 1; iter++) { 192 for (MyWaySegment s : ways) { 193 if (isCanceled()) { 194 map.clear(); 195 return map; 196 } 197 if (s.highway) { 198 for (Node en : s.nearbyNodes(mindist)) { 199 if (en == null || !endnodesHighway.contains(en)) { 200 continue; 201 } 202 if (en.hasTag(HIGHWAY, "turning_circle", "bus_stop") 203 || en.hasTag("amenity", "parking_entrance") 204 || en.hasTag(RAILWAY, "buffer_stop") 205 || en.isKeyTrue("noexit") 206 || en.hasKey("entrance", "barrier")) { 207 continue; 208 } 209 // to handle intersections of 't' shapes and similar 210 if (en.isConnectedTo(s.w.getNodes(), 3 /* hops */, null)) { 211 continue; 212 } 213 map.put(en, s.w); 214 } 215 } 216 } 217 } 218 return map; 219 } 220 221 protected Map<Node, Way> getWayEndNodesNearOtherWay() { 222 Map<Node, Way> map = new HashMap<>(); 223 for (MyWaySegment s : ways) { 204 for (MyWaySegment s : waySegments) { 224 205 if (isCanceled()) { 225 206 map.clear(); … … 227 208 } 228 209 for (Node en : s.nearbyNodes(mindist)) { 229 if (en.isConnectedTo(s.w.getNodes(), 3 /* hops */, null)) { 210 if (en.hasTag(HIGHWAY, "turning_circle", "bus_stop") 211 || en.hasTag("amenity", "parking_entrance") 212 || en.hasTag(RAILWAY, "buffer_stop") 213 || en.isKeyTrue("noexit") 214 || en.hasKey("entrance", "barrier")) { 230 215 continue; 231 216 } 232 if (((!s.highway && endnodesHighway.contains(en)) || endnodes.contains(en)) && !s.w.concernsArea()) { 217 // to handle intersections of 't' shapes and similar 218 if (!en.isConnectedTo(s.w.getNodes(), 3 /* hops */, null)) { 233 219 map.put(en, s.w); 234 220 } … … 238 224 } 239 225 240 protected Map<Node, Way> getWay NodesNearOtherWay() {226 protected Map<Node, Way> getWayEndNodesNearOtherWay() { 241 227 Map<Node, Way> map = new HashMap<>(); 242 for (MyWaySegment s : way s) {228 for (MyWaySegment s : waySegments) { 243 229 if (isCanceled()) { 244 230 map.clear(); 245 231 return map; 246 232 } 247 for (Node en : s.nearbyNodes(minmiddledist)) { 248 if (en.isConnectedTo(s.w.getNodes(), 3 /* hops */, null)) { 249 continue; 250 } 251 if (!middlenodes.contains(en)) { 252 continue; 253 } 254 map.put(en, s.w); 233 if (!s.concernsArea) { 234 for (Node en : s.nearbyNodes(mindist)) { 235 if (!en.isConnectedTo(s.w.getNodes(), 3 /* hops */, null)) { 236 map.put(en, s.w); 237 } 238 } 255 239 } 256 240 } … … 258 242 } 259 243 260 protected Map<Node, Way> get ConnectedWayEndNodesNearOtherWay() {244 protected Map<Node, Way> getWayNodesNearOtherWay() { 261 245 Map<Node, Way> map = new HashMap<>(); 262 for (MyWaySegment s : way s) {246 for (MyWaySegment s : waySegments) { 263 247 if (isCanceled()) { 264 248 map.clear(); … … 266 250 } 267 251 for (Node en : s.nearbyNodes(minmiddledist)) { 268 if (en.isConnectedTo(s.w.getNodes(), 3 /* hops */, null)) { 269 continue; 270 } 271 if (!othernodes.contains(en)) { 272 continue; 273 } 274 map.put(en, s.w); 252 if (!en.isConnectedTo(s.w.getNodes(), 3 /* hops */, null)) { 253 map.put(en, s.w); 254 } 275 255 } 276 256 } … … 280 260 protected final void addErrors(Severity severity, Map<Node, Way> errorMap, String message) { 281 261 for (Map.Entry<Node, Way> error : errorMap.entrySet()) { 262 Node node = error.getKey(); 263 Way way = error.getValue(); 264 if (partialSelection && !nodesToTest.contains(node) && !waysToTest.contains(way)) 265 continue; 282 266 errors.add(TestError.builder(this, severity, code) 283 267 .message(message) 284 .primitives( error.getKey(), error.getValue())285 .highlight( error.getKey())268 .primitives(node, way) 269 .highlight(node) 286 270 .build()); 287 271 } … … 290 274 @Override 291 275 public void endTest() { 292 addErrors(Severity.WARNING, getWayEndNodesNearOtherHighway(), tr("Way end node near other highway")); 293 addErrors(Severity.WARNING, getWayEndNodesNearOtherWay(), tr("Way end node near other way")); 276 if (ds == null) 277 return; 278 279 for (Way w : ds.getWays()) { 280 if (w.isUsable() && isCandidate(w) && w.getRealNodesCount() > 1 281 // don't complain about highways ending near platforms 282 && !w.hasTag(HIGHWAY, "platform") && !w.hasTag(RAILWAY, "platform", "platform_edge") 283 ) { 284 waySegments.addAll(getWaySegments(w)); 285 addNode(w.firstNode(), endnodes); 286 addNode(w.lastNode(), endnodes); 287 } 288 } 289 searchNodes = new QuadBuckets<>(); 290 searchNodes.addAll(endnodes); 291 if (isHighwayTest) { 292 addErrors(Severity.WARNING, getWayEndNodesNearOtherHighway(), tr("Way end node near other highway")); 293 } else { 294 addErrors(Severity.WARNING, getWayEndNodesNearOtherWay(), tr("Way end node near other way")); 295 } 296 294 297 /* the following two use a shorter distance */ 295 if (minmiddledist > 0.0) { 298 boolean includeOther = isBeforeUpload ? ValidatorPrefHelper.PREF_OTHER_UPLOAD.get() : ValidatorPrefHelper.PREF_OTHER.get(); 299 if (minmiddledist > 0.0 && includeOther) { 300 searchNodes.clear(); 301 searchNodes.addAll(middlenodes); 296 302 addErrors(Severity.OTHER, getWayNodesNearOtherWay(), tr("Way node near other way")); 297 addErrors(Severity.OTHER, getConnectedWayEndNodesNearOtherWay(), tr("Connected way end node near other way")); 298 } 299 ways = null; 303 searchNodes.clear(); 304 searchNodes.addAll(othernodes); 305 addErrors(Severity.OTHER, getWayNodesNearOtherWay(), tr("Connected way end node near other way")); 306 } 307 waySegments = null; 300 308 endnodes = null; 301 endnodesHighway = null;302 309 middlenodes = null; 303 310 othernodes = null; 311 searchNodes = null; 304 312 dsArea = null; 313 ds = null; 305 314 super.endTest(); 306 315 } 307 316 308 317 private class MyWaySegment { 309 private final Line2D line;310 318 public final Way w; 311 public final boolean isAbandoned;312 public final boolean isBoundary;313 public final boolean highway;314 private final double len;315 private Set<Node> nearbyNodeCache;316 private double nearbyNodeCacheDist = -1.0;317 319 private final Node n1; 318 320 private final Node n2; 319 320 MyWaySegment(Way w, Node n1, Node n2) { 321 private final boolean concernsArea; 322 323 MyWaySegment(Way w, Node n1, Node n2, boolean concersArea) { 321 324 this.w = w; 322 String railway = w.get(RAILWAY);323 String highway = w.get(HIGHWAY);324 this.isAbandoned = "abandoned".equals(railway) || w.isKeyTrue("disused");325 this.highway = (highway != null || railway != null) && !isAbandoned;326 this.isBoundary = !this.highway && w.hasTag("boundary", "administrative");327 line = new Line2D.Double(n1.getEastNorth().east(), n1.getEastNorth().north(),328 n2.getEastNorth().east(), n2.getEastNorth().north());329 len = line.getP1().distance(line.getP2());330 325 this.n1 = n1; 331 326 this.n2 = n2; 327 this.concernsArea = concersArea; 332 328 } 333 329 … … 344 340 if (coord == null) 345 341 return false; 346 Point2D p = new Point2D.Double(coord.east(), coord.north()); 347 if (line.getP1().distance(p) > len+dist) 348 return false; 349 if (line.getP2().distance(p) > len+dist) 350 return false; 351 return line.ptSegDist(p) < dist; 352 } 353 354 public List<LatLon> getBounds(double fudge) { 342 EastNorth en1 = n1.getEastNorth(); 343 EastNorth en2 = n2.getEastNorth(); 344 return Line2D.ptSegDist(en1.getX(), en1.getY(), en2.getX(), en2.getY(), coord.getX(), coord.getY()) < dist; 345 } 346 347 public BBox getBounds(double fudge) { 355 348 double x1 = n1.getCoor().lon(); 356 349 double x2 = n2.getCoor().lon(); … … 369 362 LatLon topLeft = new LatLon(y2+fudge, x1-fudge); 370 363 LatLon botRight = new LatLon(y1-fudge, x2+fudge); 371 List<LatLon> ret = new ArrayList<>(2); 372 ret.add(topLeft); 373 ret.add(botRight); 374 return ret; 364 return new BBox(topLeft, botRight); 375 365 } 376 366 377 367 public Collection<Node> nearbyNodes(double dist) { 378 // If you're looking for nodes that are farther away that we looked for last time,379 // the cached result is no good380 if (dist > nearbyNodeCacheDist) {381 nearbyNodeCache = null;382 }383 if (nearbyNodeCache != null) {384 // If we've cached an area greater than the385 // one now being asked for...386 if (nearbyNodeCacheDist > dist) {387 // Used the cached result and trim out388 // the nodes that are not in the smaller389 // area, but keep the old larger cache.390 Set<Node> trimmed = new HashSet<>(nearbyNodeCache);391 Set<Node> initial = new HashSet<>(nearbyNodeCache);392 for (Node n : initial) {393 if (!nearby(n, dist)) {394 trimmed.remove(n);395 }396 }397 return trimmed;398 }399 return nearbyNodeCache;400 }401 368 /* 402 * We know that any point near the line must be at369 * We know that any point near the line segment must be at 403 370 * least as close as the other end of the line, plus 404 371 * a little fudge for the distance away ('dist'). 405 372 */ 406 373 407 // This needs to be a hash set because the searches 408 // overlap a bit and can return duplicate nodes. 409 nearbyNodeCache = null; 410 List<LatLon> bounds = this.getBounds(dist * (360.0d / (Ellipsoid.WGS84.a * 2 * Math.PI))); 411 List<Node> foundNodes = endnodesHighway.search(new BBox(bounds.get(0), bounds.get(1))); 412 foundNodes.addAll(endnodes.search(new BBox(bounds.get(0), bounds.get(1)))); 413 374 BBox bounds = this.getBounds(dist * (360.0d / (Ellipsoid.WGS84.a * 2 * Math.PI))); 375 List<Node> result = null; 376 List<Node> foundNodes = searchNodes.search(bounds); 414 377 for (Node n : foundNodes) { 415 378 if (!nearby(n, dist) || !n.getCoor().isIn(dsArea)) { … … 419 382 // so defer as much of the work as possible, like 420 383 // allocating the hash set 421 if (nearbyNodeCache == null) { 422 nearbyNodeCache = new HashSet<>(); 423 } 424 nearbyNodeCache.add(n); 425 } 426 nearbyNodeCacheDist = dist; 427 if (nearbyNodeCache == null) { 428 nearbyNodeCache = Collections.emptySet(); 429 } 430 return nearbyNodeCache; 384 if (result == null) { 385 result = new ArrayList<>(); 386 } 387 result.add(n); 388 } 389 return result == null ? Collections.emptyList() : result; 431 390 } 432 391 } … … 434 393 List<MyWaySegment> getWaySegments(Way w) { 435 394 List<MyWaySegment> ret = new ArrayList<>(); 436 if (!w.isUsable() 437 || w.hasKey("barrier") 438 || w.hasTag("natural", "cliff")) 395 if (!w.isUsable() || w.isKeyTrue("disused")) 439 396 return ret; 440 397 441 398 int size = w.getNodesCount(); 442 if (size < 2) 443 return ret; 399 boolean concersArea = w.concernsArea(); 444 400 for (int i = 1; i < size; ++i) { 445 401 if (i < size-1) { … … 449 405 Node b = w.getNode(i); 450 406 if (a.isDrawable() && b.isDrawable()) { 451 MyWaySegment ws = new MyWaySegment(w, a, b); 452 if (ws.isBoundary || ws.isAbandoned) { 453 continue; 454 } 407 MyWaySegment ws = new MyWaySegment(w, a, b, concersArea); 455 408 ret.add(ws); 456 409 } … … 461 414 @Override 462 415 public void visit(Way w) { 463 // do not consider empty ways 464 if (w.getNodesCount() > 0 465 // ignore addr:interpolation ways as they are not physical features and most of 466 // the time very near the associated highway, which is perfectly normal, see #9332 467 && !w.hasKey("addr:interpolation") 468 // similarly for public transport platforms, tree rows 469 && !w.hasTag(HIGHWAY, "platform") && !w.hasTag(RAILWAY, "platform", "platform_edge") && !w.hasTag("natural", "tree_row") 470 ) { 471 ways.addAll(getWaySegments(w)); 472 QuadBuckets<Node> set = endnodes; 473 if (w.hasKey(HIGHWAY, RAILWAY)) { 474 set = endnodesHighway; 475 } 476 addNode(w.firstNode(), set); 477 addNode(w.lastNode(), set); 478 } 479 } 480 481 private void addNode(Node n, QuadBuckets<Node> s) { 416 if (partialSelection) { 417 waysToTest.add(w); 418 } 419 } 420 421 @Override 422 public void visit(Node n) { 423 if (partialSelection) { 424 nodesToTest.add(n); 425 } 426 } 427 428 private void addNode(Node n, Set<Node> s) { 482 429 boolean m = middlenodes.contains(n); 483 430 boolean e = endnodes.contains(n); 484 boolean eh = endnodesHighway.contains(n);485 431 boolean o = othernodes.contains(n); 486 if (!m && !e && !o && !eh) {432 if (!m && !e && !o) { 487 433 s.add(n); 488 434 } else if (!o) { … … 490 436 if (e) { 491 437 endnodes.remove(n); 492 } else if (eh) {493 endnodesHighway.remove(n);494 438 } else { 495 439 middlenodes.remove(n); -
trunk/test/unit/org/openstreetmap/josm/data/validation/tests/UnconnectedWaysTest.java
r15034 r15335 14 14 import org.junit.Test; 15 15 import org.openstreetmap.josm.JOSMFixture; 16 import org.openstreetmap.josm.TestUtils; 16 17 import org.openstreetmap.josm.data.osm.DataSet; 18 import org.openstreetmap.josm.gui.MainApplication; 19 import org.openstreetmap.josm.gui.layer.OsmDataLayer; 17 20 import org.openstreetmap.josm.gui.progress.NullProgressMonitor; 18 21 import org.openstreetmap.josm.io.IllegalDataException; … … 35 38 JOSMFixture.createUnitTestFixture().init(); 36 39 bib.initialize(); 37 bib.startTest(null);38 40 } 39 41 … … 48 50 try (InputStream fis = Files.newInputStream(Paths.get("data_nodist/UnconnectedWaysTest.osm"))) { 49 51 final DataSet ds = OsmReader.parseDataSet(fis, NullProgressMonitor.INSTANCE); 52 MainApplication.getLayerManager().addLayer(new OsmDataLayer(ds, null, null)); 53 54 bib.startTest(null); 55 bib.visit(ds.allPrimitives()); 56 bib.endTest(); 57 assertThat(bib.getErrors(), isEmpty()); 58 } 59 } 60 61 /** 62 * Non-regression test for <a href="https://josm.openstreetmap.de/ticket/6313">Bug #18051</a>. 63 * @throws IOException if any I/O error occurs 64 * @throws IllegalDataException if the OSM data cannot be parsed 65 * @throws FileNotFoundException if the data file cannot be found 66 */ 67 @Test 68 public void testTicket18051() throws IOException, IllegalDataException, FileNotFoundException { 69 try (InputStream fis = TestUtils.getRegressionDataStream(18051, "modified-ways.osm.bz2")) { 70 final DataSet ds = OsmReader.parseDataSet(fis, NullProgressMonitor.INSTANCE); 71 MainApplication.getLayerManager().addLayer(new OsmDataLayer(ds, null, null)); 72 73 bib.startTest(null); 74 bib.setBeforeUpload(true); 75 bib.visit(ds.allModifiedPrimitives()); 76 bib.endTest(); 77 assertThat(bib.getErrors(), isEmpty()); 78 } 79 } 80 81 /** 82 * Non-regression test for <a href="https://josm.openstreetmap.de/ticket/6313">Bug #18106</a>. 83 * @throws IOException if any I/O error occurs 84 * @throws IllegalDataException if the OSM data cannot be parsed 85 * @throws FileNotFoundException if the data file cannot be found 86 */ 87 @Test 88 public void testTicket18106() throws IOException, IllegalDataException, FileNotFoundException { 89 try (InputStream fis = TestUtils.getRegressionDataStream(18106, "uncon3.osm")) { 90 final DataSet ds = OsmReader.parseDataSet(fis, NullProgressMonitor.INSTANCE); 91 MainApplication.getLayerManager().addLayer(new OsmDataLayer(ds, null, null)); 92 93 bib.startTest(null); 94 bib.setBeforeUpload(true); 50 95 bib.visit(ds.allPrimitives()); 51 96 bib.endTest();
Note:
See TracChangeset
for help on using the changeset viewer.