Fixed wrong intersections splitting

src/main/java/com/conveyal/osmlib/OSM.java

@@ -13,6 +13,7 @@
 import java.io.OutputStream;
 import java.util.Map;
 import java.util.NavigableSet;
+import java.util.stream.LongStream;
 
 /**
  * osm-lib representation of a subset of OpenStreetMap. One or more OSM files (e.g. PBF) can be loaded into this
@@ -180,15 +181,7 @@ public void readFromFile(String filePath) {
                 // and without it edge creation is wrong (since edges aren't split in intersections)
                 // FIXME this takes two minutes on NL OSM. We should probably save the intersections in a MapDB table.
                 LOG.info("Detecting intersections...");
-                for (Way way : ways.values()) {
-                    for (long nodeId : way.nodes) {
-                        if (referencedNodes.contains(nodeId)) {
-                            intersectionNodes.add(nodeId);
-                        } else {
-                            referencedNodes.add(nodeId);
-                        }
-                    }
-                }
+                ways.values().forEach(this::findIntersectionNodes);
                 //referenceNodes isn't needed after intersectionNodes is built
                 referencedNodes = null;
                 LOG.info("Done detecting intersections.");
@@ -204,6 +197,36 @@ public void readFromFile(String filePath) {
         }
     }
 
+    /**
+     * Functions finds nodes which appear in multiple ways
+     *
+     * First time node appears it is added in referencedNodes and if it appears again it is intersection
+     *
+     * Function skips all ways shorter then 3 nodes since you can't split 2 node way more then it already is
+     *
+     * It also skips consecutive duplicate nodes in one way. If duplicate nodes aren't skipped there are a lot
+     * of zero length edges because OSM ways have sometimes duplicate nodes.
+     * @param way
+     */
+    void findIntersectionNodes(Way way) {
+        if (way.nodes.length <= 2) {
+            return;
+        }
+        long prevNodeId = way.nodes[0];
+        for(int i=1; i < way.nodes.length; i++) {
+            long nodeId = way.nodes[i];
+            //This skips consecutive duplicate nodes
+            if (nodeId!=prevNodeId) {
+                if (referencedNodes.contains(nodeId)) {
+                    intersectionNodes.add(nodeId);
+                } else {
+                    referencedNodes.add(nodeId);
+                }
+            }
+            prevNodeId = nodeId;
+        }
+    }
+
     public void readFromUrl(String urlString) {
         try {
             LOG.info("Reading OSM from URL '{}'.", urlString);
@@ -351,13 +374,7 @@ public void writeWay(long id, Way way) {
 
         // Optionally track which nodes are referenced by more than one way.
         if (intersectionDetection) {
-            for (long nodeId : way.nodes) {
-                if (referencedNodes.contains(nodeId)) {
-                    intersectionNodes.add(nodeId);
-                } else {
-                    referencedNodes.add(nodeId);
-                }
-            }
+            findIntersectionNodes(way);
         }
 
         // Insert the way into the tile-based spatial index according to its first node.

src/test/java/com/conveyal/osmlib/OSMTest.java

@@ -7,10 +7,17 @@
 import java.nio.file.Files;
 import java.nio.file.Paths;
 import java.util.Map;
+import java.util.stream.LongStream;
 
 public class OSMTest extends TestCase {
+	private OSM osm;
+
+	public void setUp() throws Exception {
+		osm = new OSM("./src/test/resources/tmp");
+
+	}
+
 	public void testOSM(){
-		OSM osm = new OSM("./src/test/resources/tmp");
 		osm.readFromFile("./src/test/resources/bangor_maine.osm.pbf");
 		assertEquals( osm.nodes.size(), 35747 );
 		assertEquals( osm.ways.size(), 2976 );
@@ -31,7 +38,69 @@ else if (member.type == OSMEntity.Type.RELATION)
 			}
 		}
 	}
-
+
+	public void testIntersectionNodes() throws Exception {
+		Way way1 = new Way();
+		way1.nodes = new long[] { 1, 55, 22, 13 };
+		osm.findIntersectionNodes(way1);
+
+		//No intersections currently
+		assertTrue(
+			LongStream.of(way1.nodes).noneMatch(nodeid -> osm.intersectionNodes.contains(nodeid)));
+
+		//This way intersects with previous one in node 55
+		Way wayIntersectsWithWay1 = new Way();
+		wayIntersectsWithWay1.nodes = new long[] { 5, 88, 3, 18, 55 };
+		osm.findIntersectionNodes(wayIntersectsWithWay1);
+
+		long[] nonIntersections = new long[] { 5, 88, 3, 18 };
+
+		assertTrue(LongStream.of(nonIntersections)
+			.noneMatch(nodeid -> osm.intersectionNodes.contains(nodeid)));
+		assertTrue(osm.intersectionNodes.contains(55));
+
+		//way with consecutive duplicate nodes which shouldn't be counted as intersection
+		Way wayWithDuplicateNodes = new Way();
+		wayWithDuplicateNodes.nodes = new long[] { 8, 7, 7, 33, 15 };
+		nonIntersections = new long[] { 8, 7, 33, 15 };
+
+		osm.findIntersectionNodes(wayWithDuplicateNodes);
+
+		assertTrue(LongStream.of(nonIntersections)
+			.noneMatch(nodeid -> osm.intersectionNodes.contains(nodeid)));
+
+		//Way with consecutive duplicate nodes which are actually an intersection with a different road
+		Way wayWithDuplicateNodesAndIntersection = new Way();
+		wayWithDuplicateNodesAndIntersection.nodes = new long[] { 2, 98, 3, 3 };
+		nonIntersections = new long[] { 2, 98 };
+		osm.findIntersectionNodes(wayWithDuplicateNodesAndIntersection);
+
+		assertTrue(LongStream.of(nonIntersections)
+			.noneMatch(nodeid -> osm.intersectionNodes.contains(nodeid)));
+		assertTrue(osm.intersectionNodes.contains(3));
+
+		//Way with duplicate non consecutive nodes are actually an self intersection (node 34 self intersects)
+		Way wayWithDuplicateNonConsecutiveNodesAndIntersection = new Way();
+		wayWithDuplicateNonConsecutiveNodesAndIntersection.nodes = new long[] { 50, 34, 105, 111, 34, 85 };
+		nonIntersections = new long[] { 50, 105, 111, 85 };
+		osm.findIntersectionNodes(wayWithDuplicateNonConsecutiveNodesAndIntersection);
+
+		assertTrue(LongStream.of(nonIntersections)
+			.noneMatch(nodeid -> osm.intersectionNodes.contains(nodeid)));
+		assertTrue(osm.intersectionNodes.contains(34));
+
+		//Way with duplicate non consecutive nodes are actually an self intersection (node 134 self intersects)
+		wayWithDuplicateNonConsecutiveNodesAndIntersection = new Way();
+		wayWithDuplicateNonConsecutiveNodesAndIntersection.nodes = new long[] { 150, 134, 1105, 1111, 134};
+		nonIntersections = new long[] { 150, 1105, 1111};
+		osm.findIntersectionNodes(wayWithDuplicateNonConsecutiveNodesAndIntersection);
+
+		assertTrue(LongStream.of(nonIntersections)
+			.noneMatch(nodeid -> osm.intersectionNodes.contains(nodeid)));
+		assertTrue(osm.intersectionNodes.contains(134));
+
+	}
+
 	public void tearDown() throws IOException{
 		Files.delete( Paths.get("./src/test/resources/tmp") );
 		Files.delete( Paths.get("./src/test/resources/tmp.p") );