I've now tracked down the problem. The tertiary way 114405617 and its
copy C114405617 share the list of Coord objects. Not just the actual
Coord's (which is correct) but the ArrayList too. Code in
addRoadWithoutLoops() replaces the nodes that have a highwayCount
greater than 1 with CoordNode objects.  If the tertiary way is
processed first then it will end up properly routable. When the copy
is processed however, the nodes have already been replaced and have no
highwayCount, so it appears to have no connection to other roads to
the code. So you end up with a road without the correct routing
information which happens to crash MapSource.

The previous workaround removed the piece of road that ended up without
routing information. Sometimes that would be the MP copy and sometimes
the original road - it just depends on which is done last.

Two solutions I can think of
1. Always ensure you have a (shallow) copy of the list.
2. Fix up the addRoadsWithoutLoops() to recognise that a way has
already been processed and do everything that would have been done
anyway.

I favour 1) hoping that there is no intentional sharing of lists
anywhere.

Attached is a patch that copies the list in the Way constructor. I'll
check all call sites to make sure there is no duplicate copying
later. This patch is instead of the previous workaround.

Steve,

I could also create a copy of the points list in the mp algorithm (I would call it a bug that it hasn't been done :-).

But having a look at the signature of the constructor I would expect that it takes a copy of the points list. On the other side copying the points list will create some temporary lists but I think that's ok.

I have removed three copies of the points list that are unneccessary now. See attached patch Routing_error3.patch.


This kind of tagging that produces the problem is particularly common in
Spain, in the UK I couldn't find any examples and in Germany there are
only a few; perhaps one per tile. In fact the only German example I
tried to trace had been changed since I downloaded it and was no longer
a problem. Every Spanish example I looked at was a sure routing failure
however.

..Steve


What do you think of Felix proposal to add a special handling for highways in multipolygons? I doesn't feel good to me but I could add it if required.

WanMil
Index: src/uk/me/parabola/mkgmap/reader/osm/MultiPolygonRelation.java
===================================================================
--- src/uk/me/parabola/mkgmap/reader/osm/MultiPolygonRelation.java	(revision 2434)
+++ src/uk/me/parabola/mkgmap/reader/osm/MultiPolygonRelation.java	(working copy)
@@ -2224,8 +2224,7 @@
 		private Rectangle bounds;
 
 		public JoinedWay(Way originalWay) {
-			super(FakeIdGenerator.makeFakeId(), new ArrayList<Coord>(
-					originalWay.getPoints()));
+			super(FakeIdGenerator.makeFakeId(), originalWay.getPoints());
 			this.originalWays = new ArrayList<Way>();
 			addWay(originalWay);
 
Index: src/uk/me/parabola/mkgmap/reader/osm/Way.java
===================================================================
--- src/uk/me/parabola/mkgmap/reader/osm/Way.java	(revision 2434)
+++ src/uk/me/parabola/mkgmap/reader/osm/Way.java	(working copy)
@@ -50,12 +50,12 @@
 	}
 
 	public Way(long id, List<Coord> points) {
-		this.points = points;
+		this.points = new ArrayList<Coord>(points);
 		setId(id);
 	}
 
 	public Way copy() {
-		Way dup = new Way(getId(), new ArrayList<Coord>(points));
+		Way dup = new Way(getId(), points);
 		dup.setName(getName());
 		dup.copyTags(this);
 		dup.closed = this.closed;
Index: src/uk/me/parabola/mkgmap/osmstyle/StyledConverter.java
===================================================================
--- src/uk/me/parabola/mkgmap/osmstyle/StyledConverter.java	(revision 2434)
+++ src/uk/me/parabola/mkgmap/osmstyle/StyledConverter.java	(working copy)
@@ -1487,7 +1487,9 @@
 
 		int numNodes = nodeIndices.size();
 		road.setNumNodes(numNodes);
-
+		if (numNodes == 0) {
+			System.out.println("ZERO NODES: " + way.getId());
+		}
 		if(numNodes > 0) {
 			// replace Coords that are nodes with CoordNodes
 			boolean hasInternalNodes = false;
Index: src/uk/me/parabola/mkgmap/sea/optional/PrecompSeaMerger.java
===================================================================
--- src/uk/me/parabola/mkgmap/sea/optional/PrecompSeaMerger.java	(revision 2434)
+++ src/uk/me/parabola/mkgmap/sea/optional/PrecompSeaMerger.java	(working copy)
@@ -160,13 +160,12 @@
 				landWays.put(landWay.getId(), landWay);
 			}
 
-			List<Coord> seaCoords = new ArrayList<Coord>(5);
-			seaCoords.add(new Coord(-90.0d, -180.0d));
-			seaCoords.add(new Coord(90.0d, -180.0d));
-			seaCoords.add(new Coord(90.0d, 180.0d));
-			seaCoords.add(new Coord(-90.0d, 180.0d));
-			seaCoords.add(new Coord(-90.0d, -180.0d));
-			Way seaWay = new Way(FakeIdGenerator.makeFakeId(), seaCoords);
+			Way seaWay = new Way(FakeIdGenerator.makeFakeId());
+			seaWay.addPoint(new Coord(-90.0d, -180.0d));
+			seaWay.addPoint(new Coord(90.0d, -180.0d));
+			seaWay.addPoint(new Coord(90.0d, 180.0d));
+			seaWay.addPoint(new Coord(-90.0d, 180.0d));
+			seaWay.addPoint(new Coord(-90.0d, -180.0d));
 			landWays.put(seaWay.getId(), seaWay);
 
 			Relation rel = new GeneralRelation(FakeIdGenerator.makeFakeId());
_______________________________________________
mkgmap-dev mailing list
[email protected]
http://lists.mkgmap.org.uk/mailman/listinfo/mkgmap-dev

Reply via email to