Hi Gerd

Yes, you're right.

I've changed it so it only keeps polygons correctly closed.

Ticker

On Wed, 2021-03-24 at 18:54 +0000, Gerd Petermann wrote:
> Hi Ticker,
> 
> I don't understand the last changes in RoundCoordsFilter.java. What
> if the replaced 1st point is preserved?
> 
> Gerd
> 
> ________________________________________
> Von: mkgmap-dev <mkgmap-dev-boun...@lists.mkgmap.org.uk> im Auftrag
> von Ticker Berkin <rwb-mkg...@jagit.co.uk>
> Gesendet: Dienstag, 23. März 2021 14:17
> An: Development list for mkgmap
> Betreff: Re: [mkgmap-dev] line/polygon filters fix
> 
> Hi Gerd
> 
> I don't get any detectable failures, but this might cause
> difficulties
> with adjacent junctions and/or housenumbers.
> 
> If there are two adjacent equal points, with the first not preserved
> and the second preserved, the existing RemoveObsoleteFilter will
> replace the first with the second and leave the second in as well.
> The
> code clearly intended not to duplicate the preserved point, and, if
> they were the other way around, doesn't.
> 
> Ticker
> 
> On Tue, 2021-03-23 at 12:38 +0000, Gerd Petermann wrote:
> > Hi Ticker,
> > 
> > please add a unit test that shows where the old code fails.
> > 
> > Gerd
> > 
> > ________________________________________
> > Von: mkgmap-dev <mkgmap-dev-boun...@lists.mkgmap.org.uk> im Auftrag
> > von Ticker Berkin <rwb-mkg...@jagit.co.uk>
> > Gesendet: Dienstag, 23. März 2021 13:10
> > An: mkgmap development
> > Betreff: [mkgmap-dev] line/polygon filters fix
> > 
> > Hi Gerd
> > 
> > I was trying to diagnose a problem with a repeating points in
> > polylines
> > as reported by GPSMapEdit and found a problem in
> > RemoveObsoletePointsFilter where it duplicates a point.
> > 
> > Also in this and/or RoundCoordsFilter I've made some changes:
> > 1/ stop the chain when polygons get too small
> > 2/ keep polygons closed with the same first/last point
> > 3/ slight logic tidy-up
> > 4/ add a couple of debug lines to be consistent
> > 
> > Patch attached.
> > 
> > Actually these didn't make any difference to the repeating points
> > problem. This happens when there are enough unused bits in the last
> > byte of the polyline bitsteam to represent an extra point. I can't
> > see
> > any good way of stopping this.
> > 
> > Ticker
> > _______________________________________________
> > mkgmap-dev mailing list
> > mkgmap-dev@lists.mkgmap.org.uk
> > http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> _______________________________________________
> mkgmap-dev mailing list
> mkgmap-dev@lists.mkgmap.org.uk
> http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> _______________________________________________
> mkgmap-dev mailing list
> mkgmap-dev@lists.mkgmap.org.uk
> http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Index: src/uk/me/parabola/mkgmap/filters/RemoveObsoletePointsFilter.java
===================================================================
--- src/uk/me/parabola/mkgmap/filters/RemoveObsoletePointsFilter.java	(revision 4618)
+++ src/uk/me/parabola/mkgmap/filters/RemoveObsoletePointsFilter.java	(working copy)
@@ -29,8 +29,6 @@
  */
 public class RemoveObsoletePointsFilter implements MapFilter {
 	private static final Logger log = Logger.getLogger(RemoveObsoletePointsFilter.class);
-	
-	final Coord[] areaTest = new Coord[3];
 
 	private boolean checkPreserved;
 	
@@ -48,18 +46,12 @@
 		MapLine line = (MapLine) element;
 		List<Coord> points = line.getPoints();
 		int numPoints = points.size();
-		if (numPoints <= 1){
-			return;
-		}
-		int requiredPoints = (line instanceof MapShape ) ? 4:2; 
+		int requiredPoints = (line instanceof MapShape) ? 4 : 2;
 		List<Coord> newPoints = new ArrayList<>(numPoints);
 		while (true){
 			boolean removedSpike = false;
-			numPoints = points.size();
-			
-
-			Coord lastP = points.get(0);
-			newPoints.add(lastP);
+			Coord lastP;
+			newPoints.add(points.get(0));
 			for (int i = 1; i < numPoints; i++) {
 				Coord newP = points.get(i);
 				int last = newPoints.size()-1;
@@ -72,6 +64,7 @@
 							continue;
 						} else if (!lastP.preserved()){
 							newPoints.set(last, newP); // replace last
+							continue;
 						} 
 					} else {  
 						continue;
@@ -104,36 +97,45 @@
 
 				newPoints.add(newP);
 			}
-			if (!removedSpike || newPoints.size() < requiredPoints)
+			numPoints = newPoints.size();
+			if (numPoints < requiredPoints)
+				return;
+			if (!removedSpike)
 				break;
 			points = newPoints;
-			newPoints = new ArrayList<>(points.size());
+			newPoints = new ArrayList<>(numPoints);
 		}
 		if (line instanceof MapShape){
 			// Check special cases caused by the fact that the first and last point 
 			// in a shape are identical. 
-			while (newPoints.size() > 3){
-				int nPoints = newPoints.size();
-				switch(Utils.isStraight(newPoints.get(newPoints.size()-2), newPoints.get(0), newPoints.get(1))){
+		isStraightCheck:
+			while (true) {
+				switch (Utils.isStraight(newPoints.get(numPoints-2), newPoints.get(0), newPoints.get(1))) {
 				case Utils.STRAIGHT_SPIKE:
+					log.debug("removing closing spike");
+					--numPoints;
 					newPoints.remove(0);
-					newPoints.set(newPoints.size()-1, newPoints.get(0));
-					if (newPoints.get(newPoints.size()-2).equals(newPoints.get(newPoints.size()-1)))
-						newPoints.remove(newPoints.size()-1);
+					newPoints.set(numPoints-1, newPoints.get(0));
+					if (newPoints.get(numPoints-2).equals(newPoints.get(numPoints-1))) {
+						newPoints.remove(numPoints-2); // keep the identical closing point
+						--numPoints;
+					}
 					break;
 				case Utils.STRICTLY_STRAIGHT:
-					newPoints.remove(newPoints.size()-1);
-					newPoints.set(0, newPoints.get(newPoints.size()-1));
+					log.debug("removing straight line across closing");
+					--numPoints;
+					newPoints.remove(numPoints);
+					newPoints.set(0, newPoints.get(numPoints-1));
 					break;
+				default:
+					break isStraightCheck;
 				}
-				if (nPoints == newPoints.size())
-					break;
+				if (numPoints < requiredPoints)
+					return;
 			}
 		}
-		
-		if (newPoints.size() != line.getPoints().size()){
-			if (line instanceof MapShape && newPoints.size() <= 3 || newPoints.size() <= 1)
-				return;
+
+		if (numPoints != line.getPoints().size()) {
 			MapLine newLine = line.copy();
 			newLine.setPoints(newPoints);
 			next.doFilter(newLine);
Index: src/uk/me/parabola/mkgmap/filters/RoundCoordsFilter.java
===================================================================
--- src/uk/me/parabola/mkgmap/filters/RoundCoordsFilter.java	(revision 4618)
+++ src/uk/me/parabola/mkgmap/filters/RoundCoordsFilter.java	(working copy)
@@ -18,11 +18,14 @@
 
 import uk.me.parabola.imgfmt.app.Coord;
 import uk.me.parabola.imgfmt.app.CoordNode;
+import uk.me.parabola.log.Logger;
 import uk.me.parabola.mkgmap.general.MapElement;
 import uk.me.parabola.mkgmap.general.MapLine;
 import uk.me.parabola.mkgmap.general.MapRoad;
+import uk.me.parabola.mkgmap.general.MapShape;
 
 public class RoundCoordsFilter implements MapFilter {
+	private static final Logger log = Logger.getLogger(RoundCoordsFilter.class);
 
 	private int shift;
 	private boolean keepNodes;
@@ -31,8 +34,8 @@
 	@Override
 	public void init(FilterConfig config) {
 		shift = config.getShift();
-		keepNodes = config.getLevel() == 0 && config.hasNet();
 		level = config.getLevel();
+		keepNodes = level == 0 && config.hasNet();
 	}
 
 	/**
@@ -41,12 +44,11 @@
 	 */
 	@Override
 	public void doFilter(MapElement element, MapFilterChain next) {
-		MapLine line = (MapLine) element;
-		if(shift == 0) {
+		if (shift == 0) {
 			// do nothing
-			next.doFilter(line);
-		}
-		else {
+			next.doFilter(element);
+		} else {
+			MapLine line = (MapLine) element;
 			int half = 1 << (shift - 1);	// 0.5 shifted
 			int mask = ~((1 << shift) - 1); // to remove fraction bits
 			
@@ -64,9 +66,9 @@
 				int lon = (p.getLongitude() + half) & mask;
 				Coord newP;
 				
-				if(p instanceof CoordNode && keepNodes)
+				if (p instanceof CoordNode && keepNodes) {
 					newP = new CoordNode(lat, lon, p.getId(), p.getOnBoundary(), p.getOnCountryBorder());
-				else {
+				} else {
 					newP = new Coord(lat, lon);
 					newP.preserved(p.preserved());
 					newP.setNumberNode(hasNumbers && p.isNumberNode());
@@ -87,11 +89,18 @@
 					lastP.preserved(true);
 				}
 			}
-			if(newPoints.size() > 1) {
-				MapLine newLine = line.copy();
-				newLine.setPoints(newPoints);
-				next.doFilter(newLine);
+			int numPoints = newPoints.size();
+			if (numPoints < 2)
+				return;
+			if (line instanceof MapShape) {
+				if (numPoints < 4)
+					return;
+				if (newPoints.get(0).equals(newPoints.get(numPoints-1)))
+					newPoints.set(numPoints-1, newPoints.get(0)); // keep closed
 			}
+			MapLine newLine = line.copy();
+			newLine.setPoints(newPoints);
+			next.doFilter(newLine);
 		}
 	}
 }
_______________________________________________
mkgmap-dev mailing list
mkgmap-dev@lists.mkgmap.org.uk
http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev

Reply via email to