The short arc removal code was removing (non-short) arcs when their ends
were close enough to be considered the same point.
Now it just leaves them be.
Please test this patch if possible because I have reworked the short
arc removal code (and some badness could have crept in).
You will probably find some (loopy) ways appearing in your maps that
were not there before because they were being trashed.
Garvan, can you try this patch or shall I send you a mkgmap.jar?
Mark
diff --git a/src/uk/me/parabola/mkgmap/reader/osm/xml/Osm5XmlHandler.java b/src/uk/me/parabola/mkgmap/reader/osm/xml/Osm5XmlHandler.java
index fdeb2a8..d4ac861 100644
--- a/src/uk/me/parabola/mkgmap/reader/osm/xml/Osm5XmlHandler.java
+++ b/src/uk/me/parabola/mkgmap/reader/osm/xml/Osm5XmlHandler.java
@@ -829,113 +829,139 @@ public class Osm5XmlHandler extends DefaultHandler {
previousPoint = p;
anotherPassRequired = true;
}
- if (i > 0) {
- // this is not the first point in the way
- if (p == previousPoint) {
- log.info(" Way " + way.getTag("name") + " (" + way.toBrowseURL() + ") has consecutive identical points at " + p.toOSMURL() + " - deleting the second point");
- points.remove(i);
- // hack alert! rewind the loop index
- --i;
- anotherPassRequired = true;
- continue;
+ if(i == 0) {
+ // first point in way is a node so "preserve"
+ // it to ensure it won't be filtered out later
+ p.preserved(true);
+ continue;
+ }
+
+ // this is not the first point in the way
+ if (p == previousPoint) {
+ log.info(" Way " + way.getTag("name") + " (" + way.toBrowseURL() + ") has consecutive identical points at " + p.toOSMURL() + " - deleting the second point");
+ points.remove(i);
+ // hack alert! rewind the loop index
+ --i;
+ anotherPassRequired = true;
+ continue;
+ }
+ arcLength += p.distance(previousPoint);
+ previousPoint = p;
+ Coord previousNode = points.get(previousNodeIndex);
+
+ // this point is a node if it has an arc count
+ Integer arcCount = arcCounts.get(p);
+ if(arcCount == null)
+ continue;
+
+ // it's a node so make the point "preserved" so
+ // that it won't get filtered out later
+ p.preserved(true);
+
+ if(p == previousNode) {
+ // this node is the same point object as the
+ // previous node - leave it and it will be
+ // handled later by the road loop splitter
+ previousNodeIndex = i;
+ arcLength = 0;
+ continue;
+ }
+
+ boolean mergeNodes = false;
+
+ if(p.equals(previousNode)) {
+ // nodes have identical coordinates but we
+ // only want to merge them if the arc length
+ // is small to avoid trashing loops
+ // (e.g. contours)
+ if(arcLength == 0 || arcLength < minArcLength)
+ mergeNodes = true;
+ else if(complainedAbout.get(way) == null) {
+ log.info(" Way " + way.getTag("name") + " (" + way.toBrowseURL() + ") has consecutive nodes with the same coordinates (" + p.toOSMURL() + ") but I am leaving them unmerged because the arc between them is " + (int)(arcLength * 10) / 10.0 + "m long");
+ complainedAbout.put(way, way);
}
- arcLength += p.distance(previousPoint);
- previousPoint = p;
- Coord previousNode = points.get(previousNodeIndex);
- // this point is a node if it has an arc count
- Integer arcCount = arcCounts.get(p);
- if (arcCount != null) {
- // make the point "preserved" so that it
- // won't get filtered out later
- p.preserved(true);
- // merge this node to previous node if the
- // two points have identical coordinates
- // or are closer than the minimum distance
- // allowed but they are not the same point
- // object
- if(p != previousNode &&
- (p.equals(previousNode) ||
- (minArcLength > 0 &&
- minArcLength > arcLength))) {
- if(previousNode.getOnBoundary() && p.getOnBoundary()) {
- if(p.equals(previousNode)) {
- // the previous node has
- // identical coordinates to
- // the current node so it can
- // be replaced but to avoid
- // the assertion above we need
- // to forget that it is on the
- // boundary
- previousNode.setOnBoundary(false);
- }
- else {
- // both the previous node and
- // this node are on the
- // boundary and they don't
- // have identical coordinates
- if(complainedAbout.get(way) == null) {
- log.warn(" Way " + way.getTag("name") + " (" + way.toBrowseURL() + ") has short arc (" + String.format("%.2f", arcLength) + "m) at " + p.toOSMURL() + " - but it can't be removed because both ends of the arc are boundary nodes!");
- complainedAbout.put(way, way);
- }
- break; // give up with this way
- }
- }
- String thisNodeId = (p.getOnBoundary())? "'boundary node'" : "" + nodeIdMap.get(p);
- String previousNodeId = (previousNode.getOnBoundary())? "'boundary node'" : "" + nodeIdMap.get(previousNode);
-
- if(p.equals(previousNode))
- log.info(" Way " + way.getTag("name") + " (" + way.toBrowseURL() + ") has consecutive nodes with the same coordinates (" + p.toOSMURL() + ") - merging node " + thisNodeId + " into " + previousNodeId);
- else
- log.info(" Way " + way.getTag("name") + " (" + way.toBrowseURL() + ") has short arc (" + String.format("%.2f", arcLength) + "m) at " + p.toOSMURL() + " - removing it by merging node " + thisNodeId + " into " + previousNodeId);
- if(p.getOnBoundary()) {
- // current point is a boundary node so
- // we need to merge the previous node into
- // this node
- ++numNodesMerged;
- replacements.put(previousNode, p);
- // add the previous node's arc
- // count to this node
- incArcCount(arcCounts, p, arcCounts.get(previousNode) - 1);
- // remove the preceding point(s)
- // back to and including the
- // previous node
- for(int j = i - 1; j >= previousNodeIndex; --j) {
- points.remove(j);
- }
- // hack alert! rewind the loop index
- i = previousNodeIndex;
- anotherPassRequired = true;
- }
- else {
- // current point is not on a boundary so
- // merge this node into the previous one
- ++numNodesMerged;
- replacements.put(p, previousNode);
- // add this node's arc count to the node
- // that is replacing it
- incArcCount(arcCounts, previousNode, arcCount - 1);
- // reset previous point to be the
- // previous node
- previousPoint = previousNode;
- // remove the point(s) back to the
- // previous node
- for(int j = i; j > previousNodeIndex; --j) {
- points.remove(j);
- }
- // hack alert! rewind the loop index
- i = previousNodeIndex;
- anotherPassRequired = true;
- }
- } else {
- // the node did not need to be merged so
- // it now becomes the new previous node
- previousNodeIndex = i;
+ }
+ else if(minArcLength > 0 && minArcLength > arcLength) {
+ // nodes have different coordinates but the
+ // arc length is too small so merge the nodes
+ mergeNodes = true;
+ }
+
+ if(!mergeNodes) {
+ // this is now the latest node
+ previousNodeIndex = i;
+ arcLength = 0;
+ continue;
+ }
+
+ // nodes are to be merged
+ if(previousNode.getOnBoundary() && p.getOnBoundary()) {
+ if(p.equals(previousNode)) {
+ // the previous node has identical
+ // coordinates to the current node so it
+ // can be replaced but to avoid the
+ // assertion above we need to forget that
+ // it is on the boundary
+ previousNode.setOnBoundary(false);
+ }
+ else {
+ // both the previous node and this node
+ // are on the boundary and they don't have
+ // identical coordinates
+ if(complainedAbout.get(way) == null) {
+ log.warn(" Way " + way.getTag("name") + " (" + way.toBrowseURL() + ") has short arc (" + String.format("%.2f", arcLength) + "m) at " + p.toOSMURL() + " - but it can't be removed because both ends of the arc are boundary nodes!");
+ complainedAbout.put(way, way);
}
+ break; // give up with this way
+ }
+ }
+ String thisNodeId = (p.getOnBoundary())? "'boundary node'" : "" + nodeIdMap.get(p);
+ String previousNodeId = (previousNode.getOnBoundary())? "'boundary node'" : "" + nodeIdMap.get(previousNode);
- // reset arc length
- arcLength = 0;
+ if(p.equals(previousNode))
+ log.info(" Way " + way.getTag("name") + " (" + way.toBrowseURL() + ") has consecutive nodes with the same coordinates (" + p.toOSMURL() + ") - merging node " + thisNodeId + " into " + previousNodeId);
+ else
+ log.info(" Way " + way.getTag("name") + " (" + way.toBrowseURL() + ") has short arc (" + String.format("%.2f", arcLength) + "m) at " + p.toOSMURL() + " - removing it by merging node " + thisNodeId + " into " + previousNodeId);
+ if(p.getOnBoundary()) {
+ // current point is a boundary node so we need
+ // to merge the previous node into this node
+ ++numNodesMerged;
+ replacements.put(previousNode, p);
+ // add the previous node's arc count to this
+ // node
+ incArcCount(arcCounts, p, arcCounts.get(previousNode) - 1);
+ // remove the preceding point(s) back to and
+ // including the previous node
+ for(int j = i - 1; j >= previousNodeIndex; --j) {
+ points.remove(j);
+ }
+ // hack alert! rewind the loop index
+ i = previousNodeIndex;
+ anotherPassRequired = true;
+ }
+ else {
+ // current point is not on a boundary so merge
+ // this node into the previous one
+ ++numNodesMerged;
+ replacements.put(p, previousNode);
+ // add this node's arc count to the node that
+ // is replacing it
+ incArcCount(arcCounts, previousNode, arcCount - 1);
+ // reset previous point to be the previous
+ // node
+ previousPoint = previousNode;
+ // remove the point(s) back to the previous
+ // node
+ for(int j = i; j > previousNodeIndex; --j) {
+ points.remove(j);
}
+ // hack alert! rewind the loop index
+ i = previousNodeIndex;
+ anotherPassRequired = true;
}
+
+ // reset arc length
+ arcLength = 0;
}
}
}
_______________________________________________
mkgmap-dev mailing list
[email protected]
http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
