Index: src/uk/me/parabola/imgfmt/app/Coord.java =================================================================== --- src/uk/me/parabola/imgfmt/app/Coord.java (revision 4131) +++ src/uk/me/parabola/imgfmt/app/Coord.java (working copy) @@ -40,7 +40,7 @@ private final static short ON_BOUNDARY_MASK = 0x0001; // bit in flags is true if point lies on a boundary private final static short PRESERVED_MASK = 0x0002; // bit in flags is true if point should not be filtered out private final static short REPLACED_MASK = 0x0004; // bit in flags is true if point was replaced - private final static short TREAT_AS_NODE_MASK = 0x0008; // bit in flags is true if point should be treated as a node + private final static short ADDED_BY_CLIPPER_MASK = 0x0008; // bit in flags is true if point was added by clipper private final static short FIXME_NODE_MASK = 0x0010; // bit in flags is true if a node with this coords has a fixme tag private final static short REMOVE_MASK = 0x0020; // bit in flags is true if this point should be removed private final static short VIA_NODE_MASK = 0x0040; // bit in flags is true if a node with this coords is the via node of a RestrictionRelation @@ -217,12 +217,10 @@ } /** - * Should this Coord be treated like a Garmin node in short arc removal? - * The value has no meaning outside of short arc removal. - * @return true if this coord should be treated like a Garmin node, else false + * @return true if this coord was added by a clipper */ - public boolean isTreatAsNode() { - return (flags & TREAT_AS_NODE_MASK) != 0; + public boolean isAddedByClipper() { + return (flags & ADDED_BY_CLIPPER_MASK) != 0; } /** @@ -229,11 +227,11 @@ * Mark the Coord to be treated like a Node in short arc removal * @param treatAsNode true or false */ - public void setTreatAsNode(boolean treatAsNode) { - if (treatAsNode) - this.flags |= TREAT_AS_NODE_MASK; + public void setAddedByClipper(boolean b) { + if (b) + this.flags |= ADDED_BY_CLIPPER_MASK; else - this.flags &= ~TREAT_AS_NODE_MASK; + this.flags &= ~ADDED_BY_CLIPPER_MASK; } /** Index: src/uk/me/parabola/mkgmap/general/LineClipper.java =================================================================== --- src/uk/me/parabola/mkgmap/general/LineClipper.java (revision 4131) +++ src/uk/me/parabola/mkgmap/general/LineClipper.java (working copy) @@ -172,6 +172,7 @@ // its position (in map coordinates) is different from the // original point, use the new point as a boundary node Coord new0 = Coord.makeHighPrecCoord(calcCoord(y0, dy, t[0]), calcCoord(x0, dx, t[0])); + new0.setAddedByClipper(true); // check the maths worked out assert a.onBoundary(new0) : "New boundary point at " + new0.toString() + " not on boundary of [" + a.getMinLat() + ", " + a.getMinLong() + ", " + a.getMaxLat() + ", " + a.getMaxLong() + "]"; if(!new0.highPrecEquals(orig0)) @@ -192,6 +193,7 @@ // its position (in map coordinates) is different from the // original point, use the new point as a boundary node Coord new1 = Coord.makeHighPrecCoord(calcCoord(y0, dy, t[1]), calcCoord(x0, dx, t[1])); + new1.setAddedByClipper(true); // check the maths worked out assert a.onBoundary(new1) : "New boundary point at " + new1.toString() + " not on boundary of [" + a.getMinLat() + ", " + a.getMinLong() + ", " + a.getMaxLat() + ", " + a.getMaxLong() + "]"; Index: src/uk/me/parabola/mkgmap/osmstyle/WrongAngleFixer.java =================================================================== --- src/uk/me/parabola/mkgmap/osmstyle/WrongAngleFixer.java (revision 4131) +++ src/uk/me/parabola/mkgmap/osmstyle/WrongAngleFixer.java (working copy) @@ -20,6 +20,10 @@ import java.util.IdentityHashMap; import java.util.List; import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; + +import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap; import uk.me.parabola.imgfmt.Utils; import uk.me.parabola.imgfmt.app.Area; import uk.me.parabola.imgfmt.app.Coord; @@ -57,6 +61,8 @@ static final int MODE_ROADS = 0; static final int MODE_LINES = 1; private int mode = MODE_ROADS; + private int pass; + private boolean extraPass; public WrongAngleFixer(Area bbox) { this.bbox = bbox; @@ -87,6 +93,10 @@ printBadAngles("bad_angles_start", roads); writeOSM("roads_orig", roads); writeOSM("lines_orig", lines); + Long2ObjectOpenHashMap coordMap = new Long2ObjectOpenHashMap<>(); + replaceDuplicateBoundaryNodes(roads, coordMap); + replaceDuplicateBoundaryNodes(lines, coordMap); + coordMap.clear(); removeWrongAngles(roads, lines, modifiedRoads, deletedRoads, restrictions); writeOSM("roads_post_rem_wrong_angles", roads); removeObsoletePoints(roads, modifiedRoads); @@ -100,7 +110,35 @@ writeOSM("lines_final", lines); } - private void replaceCoord(Coord toRepl, Coord replacement, Map replacements) { + /** + * Make boundary nodes unique. + * @param convertedWays + * @param coordMap + */ + private void replaceDuplicateBoundaryNodes(List convertedWays, Long2ObjectOpenHashMap coordMap) { + for (ConvertedWay cw : convertedWays) { + if (!cw.isValid() || cw.isOverlay()) + continue; + Way way = cw.getWay(); + List points = way.getPoints(); + for (int i = 0; i < points.size(); i++) { + Coord co = points.get(i); + if (!co.getOnBoundary()) + continue; + Coord repl = coordMap.get(Utils.coord2Long(co)); + if (repl == null) + coordMap.put(Utils.coord2Long(co), co); + else { + if (!co.isAddedByClipper() && repl.isAddedByClipper()) { + log.error("check replaced original boundary node at",co); + } + points.set(i, repl); + } + } + } + } + + private static void replaceCoord(Coord toRepl, Coord replacement, Map replacements) { assert toRepl != replacement; if (toRepl.getOnBoundary()){ if (replacement.equals(toRepl) == false){ @@ -127,7 +165,7 @@ replacements.put(toRepl, replacement); while (toRepl.getHighwayCount() > replacement.getHighwayCount()) replacement.incHighwayCount(); - if (mode == MODE_LINES && toRepl.isEndOfWay() ){ + if (toRepl.isEndOfWay() ){ replacement.setEndOfWay(true); } } @@ -207,15 +245,18 @@ int numNodesMerged = 0; HashSet waysWithBearingErrors = new HashSet<>(); HashSet waysThatMapToOnePoint = new HashSet<>(); - int pass = 0; + Way lastWay = null; List convertedWays = (roads != null) ? roads: lines; boolean anotherPassRequired = true; - while (anotherPassRequired && pass < 20) { + for (pass = 1; pass < 20; pass++) { + if (!anotherPassRequired && !extraPass) + break; anotherPassRequired = false; - log.info("Removing wrong angles - PASS " + ++pass); - writeOSM(((mode==MODE_LINES) ? "lines_pass_" + pass:"roads_pass_" + pass), convertedWays); + log.info("Removing wrong angles - PASS", pass); + writeOSM(((mode==MODE_LINES) ? "lines_pass_" + pass:"roads_pass_" + pass), convertedWays); + // Step 1: detect points which are parts of line segments with wrong bearings lastWay = null; for (ConvertedWay cw : convertedWays) { @@ -243,7 +284,7 @@ if (i == 0 || i == points.size()-1){ p.setEndOfWay(true); } - + if (prev != null) { if (pass == 1 && p.equals(prev) == false) hasNonEqualPoints = true; @@ -264,7 +305,7 @@ // Step 2: collect the line segments that are connected to critical points IdentityHashMap centerMap = new IdentityHashMap<>(); List centers = new ArrayList<>(); // needed for ordered processing - int centerId = 0; + Map> overlaps = new HashMap<>(); lastWay = null; for (ConvertedWay cw : convertedWays) { @@ -299,18 +340,8 @@ // save both points with their neighbour Coord p1 = prev; Coord p2 = p; - CenterOfAngle coa1 = centerMap.get(p); - if (coa1 == null) { - coa1 = new CenterOfAngle(p, centerId++); - centerMap.put(p, coa1); - centers.add(coa1); - } - CenterOfAngle coa2 = centerMap.get(prev); - if (coa2 == null) { - coa2 = new CenterOfAngle(prev, centerId++); - centerMap.put(prev, coa2); - centers.add(coa2); - } + CenterOfAngle coa1 = getOrCreateCenter(p, way, centerMap, centers, overlaps); + CenterOfAngle coa2 = getOrCreateCenter(prev, way, centerMap, centers, overlaps); coa1.addNeighbour(coa2); coa2.addNeighbour(coa1); if (points.size() == 2) { @@ -332,6 +363,8 @@ if (pass == 1 && wayHasSpecialPoints) waysWithBearingErrors.add(way); } + markOverlaps(overlaps,centers); + overlaps.clear(); // Step 3: Update list of ways with bearing errors or points next to them lastWay = null; for (ConvertedWay cw : convertedWays) { @@ -449,7 +482,24 @@ modifiedRoads.put(way.getId(), cw); } } + if (extraPass) { + anotherPassRequired = false; + break; + } else { + if (!anotherPassRequired) { + // check if we have centres on different ways that overlap + for (CenterOfAngle coa : centers) { + if (coa.forceChange) { + anotherPassRequired = true; + extraPass = true; + break; + } + } + } + } } + + // finish: remove remaining duplicate points int numWaysDeleted = 0; lastWay = null; @@ -542,6 +592,40 @@ log.info("Removing wrong angles - finished in", pass, "passes (", numNodesMerged, "nodes merged,", numWaysDeleted, "ways deleted)"); } + private CenterOfAngle getOrCreateCenter(Coord p, Way way, IdentityHashMap centerMap, List centers, Map> overlaps) { + CenterOfAngle coa = centerMap.get(p); + if (coa == null) { + coa = new CenterOfAngle(p, centerMap.size() + 1); + centerMap.put(p, coa); + centers.add(coa); + if (mode == MODE_ROADS && pass > 1) { + Set set = overlaps.get(p); + if (set == null) { + set = new HashSet<>(); + overlaps.put(p, set); + } + set.add(way); + } + + } + return coa; + } + + private void markOverlaps(Map> overlaps, List centers) { + for ( Entry> entry: overlaps.entrySet()) { + if (entry.getValue().size() > 1) { + Coord p = entry.getKey(); +// log.error("roads",mode==MODE_ROADS,"pass",pass,p,p.toDegreeString()); + for (CenterOfAngle coa : centers) { + if (coa.center.equals(p)) { + // two different centres are on the same Garmin point and they + // appear on different ways. We try hard to change them. + coa.forceChange = true; + } + } + } + } + } /** * remove obsolete points in ways. Obsolete are points which are @@ -750,6 +834,7 @@ * helper class */ private class CenterOfAngle { + public boolean forceChange; final Coord center; final List neighbours; final int id; // debugging aid @@ -764,27 +849,11 @@ neighbours = new ArrayList<>(); } - @Override public String toString() { - return "CenterOfAngle [id=" + id + ", wasMerged=" + wasMerged + ", num Neighbours="+neighbours.size()+"]"; + return "CenterOfAngle [id=" + id + " " + center.toString() + " " + center.toDegreeString() + ", wasMerged=" + wasMerged + ", num Neighbours="+neighbours.size()+"]"; } - - @Override - public int hashCode() { - return center.hashCode(); - } - - @Override - public boolean equals(Object obj) { - if (this == obj) - return true; - if (obj == null) - return false; - return center == ((CenterOfAngle) obj).center; - } - /** * returns current center position or null if removed * @param replacements @@ -861,10 +930,12 @@ Coord worstNP = null; double initialMaxError = 0; double initialSumErr = 0; + HashSet dupCheck = new HashSet<>(); for (CenterOfAngle neighbour : neighbours) { Coord n = neighbour.getCurrentLocation(replacements); if (n == null) return false; // neighbour was removed + dupCheck.add(n); if (currentCenter.highPrecEquals(n)){ if (currentCenter == n){ log.error(id + ": bad neighbour " + neighbour.id + " zero distance"); @@ -889,14 +960,21 @@ } if (initialMaxError < MAX_BEARING_ERROR) return false; + double removeErr = calcRemoveError(replacements); if (removeErr == 0){ -// createGPX(gpxPath+id+"_rem_0", replacements); currentCenter.setRemove(true); return true; } + if (dupCheck.size() != neighbours.size()) { + // two or more neighbours are on the same Garmin point. + // Better improve one of them. + return false; + } + if (initialMaxError == Double.MAX_VALUE) initialSumErr = initialMaxError; + double bestReplErr = initialMaxError; Coord bestCenterReplacement = null; List altPositions = currentCenter.getAlternativePositions(); @@ -936,7 +1014,7 @@ if (removeErr < bestReplErr && initialMaxError - removeErr >= MAX_BEARING_ERROR_HALF && removeErr < MAX_BEARING_ERROR_HALF){ bestCenterReplacement = null; // createGPX(gpxPath+id+"_rem_pref", replacements); - } else if (initialMaxError - bestReplErr < MAX_BEARING_ERROR_HALF || bestReplErr > MAX_BEARING_ERROR_HALF){ +// } else if (initialMaxError - bestReplErr < MAX_BEARING_ERROR_HALF || bestReplErr > MAX_BEARING_ERROR_HALF){ // msg = "_rather_good"; } if (bestCenterReplacement != null){ @@ -1199,11 +1277,10 @@ private double calcRemoveError(Map replacements) { if (allowedToRemove(center) == false) return Double.MAX_VALUE; - Coord c = getCurrentLocation(replacements); if (neighbours.size() > 2) return Double.MAX_VALUE; + Coord c = getCurrentLocation(replacements); Coord[] outerPoints = new Coord[neighbours.size()]; - for (int i = 0; i < neighbours.size(); i++) { CenterOfAngle neighbour = neighbours.get(i); Coord n = neighbour.getCurrentLocation(replacements); @@ -1217,8 +1294,13 @@ } if (neighbours.size() < 2) return Double.MAX_VALUE; - if (c.getDistToDisplayedPoint() < Math.max(outerPoints[0].getDistToDisplayedPoint(), outerPoints[1].getDistToDisplayedPoint())) - return Double.MAX_VALUE; + if (extraPass && forceChange) { + } else { + for (int i = 0; i < neighbours.size(); i++) { + if (c.getDistToDisplayedPoint() < outerPoints[i].getDistToDisplayedPoint()) + return Double.MAX_VALUE; + } + } double dsplAngle = Utils.getDisplayedAngle(outerPoints[0], c, outerPoints[1]); if (Math.abs( dsplAngle ) < 3) return Double.MAX_VALUE;