Index: src/org/openstreetmap/josm/data/validation/OsmValidator.java =================================================================== --- src/org/openstreetmap/josm/data/validation/OsmValidator.java (revision 10992) +++ src/org/openstreetmap/josm/data/validation/OsmValidator.java (working copy) @@ -104,6 +104,7 @@ CrossingWays.Ways.class, // ID 601 .. 699 CrossingWays.Boundaries.class, // ID 601 .. 699 CrossingWays.Barrier.class, // ID 601 .. 699 + CrossingWays.SelfCrossing.class, // ID 601 .. 699 SimilarNamedWays.class, // ID 701 .. 799 Coastlines.class, // ID 901 .. 999 WronglyOrderedWays.class, // ID 1001 .. 1099 Index: src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java =================================================================== --- src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java (revision 10992) +++ src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java (working copy) @@ -5,7 +5,6 @@ import java.awt.geom.Point2D; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -66,6 +65,8 @@ @Override boolean ignoreWaySegmentCombination(Way w1, Way w2) { + if (w1 == w2) + return false; if (!Objects.equals(getLayer(w1), getLayer(w2))) { return true; } @@ -171,6 +172,39 @@ } /** + * Self crossing ways test (for all the rest) + */ + public static class SelfCrossing extends CrossingWays { + CrossingWays.Ways normalTest = new Ways(); + CrossingWays.Barrier barrierTest = new Barrier(); + CrossingWays.Boundaries boundariesTest = new Boundaries(); + + /** + * Constructs a new SelfIntersection test. + */ + public SelfCrossing() { + super(tr("Self crossing")); + } + + @Override + public boolean isPrimitiveUsable(OsmPrimitive p) { + return super.isPrimitiveUsable(p) && !(normalTest.isPrimitiveUsable(p) || barrierTest.isPrimitiveUsable(p) + || boundariesTest.isPrimitiveUsable(p)); + } + + @Override + boolean ignoreWaySegmentCombination(Way w1, Way w2) { + return (w1 != w2); // should not happen + } + + @Override + String createMessage(Way w1, Way w2) { + return tr("Self-crossing ways"); + } + } + + + /** * Constructs a new {@code CrossingWays} test. * @param title The test title * @since 6691 @@ -220,6 +254,11 @@ @Override public void visit(Way w) { + if (this instanceof SelfCrossing) { + // free memory, we are not interested in previous ways + cellSegments.clear(); + seenWays.clear(); + } int nodesSize = w.getNodesCount(); for (int i = 0; i < nodesSize - 1; i++) { @@ -230,7 +269,7 @@ Main.warn("Crossing ways test skipped "+es1); continue; } - for (List segments : getSegments(en1, en2)) { + for (List segments : getSegments(cellSegments, en1, en2)) { for (WaySegment es2 : segments) { List prims; List highlight; @@ -239,7 +278,10 @@ continue; } - prims = Arrays.asList(es1.way, es2.way); + prims = new ArrayList<>(); + prims.add(es1.way); + if (es1.way != es2.way) + prims.add(es2.way); if ((highlight = seenWays.get(prims)) == null) { highlight = new ArrayList<>(); highlight.add(es1); @@ -265,12 +307,12 @@ /** * Returns all the cells this segment crosses. Each cell contains the list * of segments already processed - * + * @param cellSegments map with already collected way segments * @param n1 The first EastNorth * @param n2 The second EastNorth * @return A list with all the cells the segment crosses */ - private List> getSegments(EastNorth n1, EastNorth n2) { + public static List> getSegments(Map> cellSegments, EastNorth n1, EastNorth n2) { List> cells = new ArrayList<>(); for (Point2D cell : ValUtil.getSegmentCells(n1, n2, OsmValidator.griddetail)) { Index: src/org/openstreetmap/josm/data/validation/tests/SelfIntersectingWay.java =================================================================== --- src/org/openstreetmap/josm/data/validation/tests/SelfIntersectingWay.java (revision 10992) +++ src/org/openstreetmap/josm/data/validation/tests/SelfIntersectingWay.java (working copy) @@ -32,8 +32,13 @@ @Override public void visit(Way w) { Set nodes = new HashSet<>(); - - for (int i = 1; i < w.getNodesCount() - 1; i++) { + int last = w.getNodesCount(); + if (last < 2) + return; + if (w.firstNode() == w.lastNode()) + last--; // closed way, ignore last node + nodes.add(w.firstNode()); + for (int i = 1; i < last; i++) { Node n = w.getNode(i); if (nodes.contains(n)) { errors.add(new TestError(this, Index: test/unit/org/openstreetmap/josm/data/validation/tests/SelfIntersectingWayTest.java =================================================================== --- test/unit/org/openstreetmap/josm/data/validation/tests/SelfIntersectingWayTest.java (nonexistent) +++ test/unit/org/openstreetmap/josm/data/validation/tests/SelfIntersectingWayTest.java (working copy) @@ -0,0 +1,133 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.data.validation.tests; + +import java.util.ArrayList; +import java.util.List; + +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; +import org.openstreetmap.josm.JOSMFixture; +import org.openstreetmap.josm.data.coor.LatLon; +import org.openstreetmap.josm.data.osm.Node; +import org.openstreetmap.josm.data.osm.OsmUtils; +import org.openstreetmap.josm.data.osm.Way; + +/** + * JUnit Test of Multipolygon validation test. + */ +public class SelfIntersectingWayTest { + + /** + * Setup test. + * @throws Exception if test cannot be initialized + */ + @BeforeClass + public static void setUp() throws Exception { + JOSMFixture.createUnitTestFixture().init(true); + } + + private static List createNodes() { + List nodes = new ArrayList<>(); + for (int i = 0; i < 5; i++) { + nodes.add(new Node(i+1)); + } + nodes.get(0).setCoor(new LatLon(34.2680878298, 133.56336369008)); + nodes.get(1).setCoor(new LatLon(34.25096598132, 133.54891792012)); + nodes.get(2).setCoor(new LatLon(34.24466741332, 133.56693544639)); + nodes.get(3).setCoor(new LatLon(34.26815342405, 133.56066502976)); + nodes.get(4).setCoor(new LatLon(34.26567411471, 133.56132705125)); + return nodes; + } + + + /** + * Self-Intersection at first (+ last) node of closed way + */ + @Test + public void TestClosedWay() { + List nodes = createNodes(); + + Way w = (Way) OsmUtils.createPrimitive("way "); + List wayNodes = new ArrayList<>(); + wayNodes.add(nodes.get(0)); + wayNodes.add(nodes.get(1)); + wayNodes.add(nodes.get(2)); + wayNodes.add(nodes.get(0)); // problem + wayNodes.add(nodes.get(3)); + wayNodes.add(nodes.get(4)); + wayNodes.add(nodes.get(0)); + w.setNodes(wayNodes); + SelfIntersectingWay test = new SelfIntersectingWay(); + test.visit(w); + Assert.assertEquals(1, test.getErrors().size()); + Assert.assertTrue(test.getErrors().iterator().next().getHighlighted().contains(nodes.get(0))); + } + + /** + * Self-Intersection at first node of unclosed way + */ + @Test + public void TestUnclosedWayFirst() { + List nodes = createNodes(); + + Way w = (Way) OsmUtils.createPrimitive("way "); + List wayNodes = new ArrayList<>(); + wayNodes.add(nodes.get(0)); + wayNodes.add(nodes.get(1)); + wayNodes.add(nodes.get(2)); + wayNodes.add(nodes.get(0)); // problem + wayNodes.add(nodes.get(3)); + wayNodes.add(nodes.get(4)); + w.setNodes(wayNodes); + SelfIntersectingWay test = new SelfIntersectingWay(); + test.visit(w); + Assert.assertEquals(1, test.getErrors().size()); + Assert.assertTrue(test.getErrors().iterator().next().getHighlighted().contains(nodes.get(0))); + } + + /** + * Self-Intersection at first node of unclosed way + */ + @Test + public void TestUnclosedWayLast() { + List nodes = createNodes(); + + Way w = (Way) OsmUtils.createPrimitive("way "); + List wayNodes = new ArrayList<>(); + wayNodes.add(nodes.get(0)); + wayNodes.add(nodes.get(1)); // problem + wayNodes.add(nodes.get(2)); + wayNodes.add(nodes.get(3)); + wayNodes.add(nodes.get(4)); + wayNodes.add(nodes.get(1)); + w.setNodes(wayNodes); + SelfIntersectingWay test = new SelfIntersectingWay(); + test.visit(w); + Assert.assertEquals(1, test.getErrors().size()); + Assert.assertTrue(test.getErrors().iterator().next().getHighlighted().contains(nodes.get(1))); + } + + /** + * Self-Intersection at normal node (not first / last) + */ + @Test + public void TestUnclosedWayNormal() { + List nodes = createNodes(); + + Way w = (Way) OsmUtils.createPrimitive("way "); + List wayNodes = new ArrayList<>(); + wayNodes.add(nodes.get(0)); + wayNodes.add(nodes.get(1)); + wayNodes.add(nodes.get(2)); + wayNodes.add(nodes.get(1)); // problem + wayNodes.add(nodes.get(3)); + wayNodes.add(nodes.get(4)); + w.setNodes(wayNodes); + SelfIntersectingWay test = new SelfIntersectingWay(); + test.visit(w); + Assert.assertEquals(1, test.getErrors().size()); + Assert.assertTrue(test.getErrors().iterator().next().getHighlighted().contains(nodes.get(1))); + } + +}