Hi Gerd

Patch attached that I hope fixes the splitting problem.

I haven't looked at your change yet, but your split-failed.osm test
had, and needed, lots of points visited twice and I don't see how this
can, or should, be avoided.

Ticker

On Tue, 2021-05-25 at 15:56 +0000, Gerd Petermann wrote:
> Hi Ticker,
> 
> with the attached patch ShapeMerger avoids to produce shapes where
> more than the start point is visited twice.
> This produces much larger img files (even larger than trunk) but
> PolygonSplitter doesn't complain.
> 
> I have to find out what GpsMapEdit means with "has a jitter". Maybe
> only those are problematic.
> 
> Gerd
> 
> 
> ________________________________________
> Von: mkgmap-dev <mkgmap-dev-boun...@lists.mkgmap.org.uk> im Auftrag
> von Ticker Berkin <rwb-mkg...@jagit.co.uk>
> Gesendet: Dienstag, 25. Mai 2021 14:00
> An: Development list for mkgmap
> Betreff: Re: [mkgmap-dev] special case where splitting fails without
> a log message
> 
> Sorry - sea.zip!
> 
> On Tue, 2021-05-25 at 12:36 +0100, Ticker Berkin wrote:
> > Hi Gerd
> > 
> > I'm getting very confused by this. When I build with my trunk (some
> > filter order changes + a few others), I see a whole lot more that
> > JOSM
> > shows. It and your 6324001.img just show a central bit. Looking
> > with
> > GPSMapEdit with "Highlight Polygon Contours". I also see areas
> > where
> > all islands are inverted.
> > 
> > I'll undo my trunk changes and look again.
> > 
> > Does JOSM have self-intersection checking?
> > 
> > Was the gpx trace hi-res (30bit) and then this kept when generating
> > the
> > osm. If not, then I'd expect problems.
> > 
> > Ticker
> > 
> > 
> > On Tue, 2021-05-25 at 11:18 +0000, Gerd Petermann wrote:
> > > Hi Ticker,
> > > 
> > > I didn't investigate the details. I've created the osm data by
> > > converting a gpx back to osm. I guess in the original input
> > > duplicate
> > > points are identical. I found more such cases where parts are
> > > removed.
> > > 
> > > Gerd
> > > 
> > > 
> > > ________________________________________
> > > Von: mkgmap-dev <mkgmap-dev-boun...@lists.mkgmap.org.uk> im
> > > Auftrag
> > > von Ticker Berkin <rwb-mkg...@jagit.co.uk>
> > > Gesendet: Dienstag, 25. Mai 2021 12:51
> > > An: Development list for mkgmap
> > > Betreff: Re: [mkgmap-dev] special case where splitting fails
> > > without
> > > a log message
> > > 
> > > Hi Gerd
> > > 
> > > OK - I've reproduced this. Is split-failed.osm self-intersecting?
> > > I'm
> > > not sure how to tell from Josm, but it looks like it isn't. I get
> > > the
> > > messages from shapeSplitter because it thinks it is and then the
> > > result
> > > is not good - as expected.
> > > 
> > > I'll investigate more.
> > > 
> > > Ticker
> > > 
> > > 
> > > On Tue, 2021-05-25 at 10:04 +0000, Gerd Petermann wrote:
> > > > Hi Ticker,
> > > > 
> > > > while looking at the problems with sea I found this case where
> > > > ShapeSplitter removes a small part of an island from a self
> > > > -intersecting polygon in res 24.
> > > > Compiled with mkgmap from trunk, default style and no options.
> > > > 
> > > > Look at 67.6742611, 14.6783525
> > > > 
> > > > Found this with some check code that compares the area of the
> > > > original polygon with that of sum of the parts.
> > > > 
> > > > Gerd
> > > > _______________________________________________
> > > > mkgmap-dev mailing list
> > > > mkgmap-dev@lists.mkgmap.org.uk
> > > > https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> > > _______________________________________________
> > > mkgmap-dev mailing list
> > > mkgmap-dev@lists.mkgmap.org.uk
> > > https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> > > _______________________________________________
> > > mkgmap-dev mailing list
> > > mkgmap-dev@lists.mkgmap.org.uk
> > > https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> > _______________________________________________
> > mkgmap-dev mailing list
> > mkgmap-dev@lists.mkgmap.org.uk
> > https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> _______________________________________________
> mkgmap-dev mailing list
> mkgmap-dev@lists.mkgmap.org.uk
> https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> _______________________________________________
> mkgmap-dev mailing list
> mkgmap-dev@lists.mkgmap.org.uk
> https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Index: resources/help/en/logging
===================================================================
--- resources/help/en/logging	(revision 4740)
+++ resources/help/en/logging	(working copy)
@@ -25,7 +25,9 @@
 uk.me.parabola.mkgmap.reader.osm.xml.level=INFO
 uk.me.parabola.mkgmap.reader.osm.RestrictionRelation.level=FINE
 uk.me.parabola.mkgmap.reader.osm.Restriction.level=FINE
-# For ConsoleHandler
+# Default logger
+global.level=INFO
+# For ConsoleHandlerM
 java.util.logging.ConsoleHandler.level=WARNING
 java.util.logging.ConsoleHandler.formatter=uk.me.parabola.log.UsefulFormatter
 # For FileHandler
Index: scripts/download/mkdoc
===================================================================
--- scripts/download/mkdoc	(revision 4740)
+++ scripts/download/mkdoc	(working copy)
@@ -16,7 +16,9 @@
 for f in *.txt
 do
 	mwconv -t text $f > ../dist/doc/$f
+	mwconv -l 1 -t html $f > ../dist/doc/${f/.txt/.html}
 done
+exit
 
 # Use the actual options help file.
 cp ../resources/help/en/options ../dist/doc/options.txt
Index: src/uk/me/parabola/imgfmt/app/mdr/LargeListSorter.java
===================================================================
--- src/uk/me/parabola/imgfmt/app/mdr/LargeListSorter.java	(revision 4740)
+++ src/uk/me/parabola/imgfmt/app/mdr/LargeListSorter.java	(working copy)
@@ -29,9 +29,11 @@
  */
 public abstract class LargeListSorter<T extends NamedRecord> {
 	private final Sort sort;
+	private final boolean useCache;
 	
-	public LargeListSorter(Sort sort) {
+	public LargeListSorter(Sort sort, boolean useCache) {
 		this.sort = sort;
+		this.useCache = useCache;
 	}
 
 	/**
@@ -39,6 +41,8 @@
 	 * @param list list of records.
 	 */
 	public void sort(List<T> list) {
+		if (list.size() <= 1)  // Mdr7: can have no streets or single element in partial list
+			return;
 		mergeSort(0, list, 0, list.size());
 	}
 	
@@ -51,13 +55,15 @@
 	 */
 	private void mergeSort(int depth, List<T> list, int start, int len) {
 		// we split if the number is very high and recursion is not too deep
-		if (len > 1_000_000 && depth < 3) {
+		if (len > 500_000 && depth < 4) {
 			mergeSort(depth+1,list, start, len / 2); // left
 			mergeSort(depth+1,list, start + len / 2, len - len / 2); // right
 			merge(list,start,len);
 		} else {
 			// sort one chunk
-			Map<String, byte[]> cache = new HashMap<>();
+			Map<String, byte[]> cache = null;
+			if (useCache)
+				cache = new HashMap<>();
 			List<SortKey<T>> keys = new ArrayList<>(len);
 
 			for (int i = start; i < start + len; i++) {
@@ -82,7 +88,7 @@
 		int stop2 = start + len;
 		boolean fetch1 = true;
 		boolean fetch2 = true;
-		List<T> merged = new ArrayList<>();
+		List<T> merged = new ArrayList<>(len);
 		SortKey<T> sk1 = null;
 		SortKey<T> sk2 = null;
 		while (pos1 < stop1 &&  pos2 < stop2) {
Index: src/uk/me/parabola/imgfmt/app/mdr/Mdr11.java
===================================================================
--- src/uk/me/parabola/imgfmt/app/mdr/Mdr11.java	(revision 4740)
+++ src/uk/me/parabola/imgfmt/app/mdr/Mdr11.java	(working copy)
@@ -61,8 +61,8 @@
 		pois.trimToSize();
 		Sort sort = getConfig().getSort();
 
-		LargeListSorter<Mdr11Record> sorter = new LargeListSorter<Mdr11Record>(sort) {
-			
+		LargeListSorter<Mdr11Record> sorter = new LargeListSorter<Mdr11Record>(sort, false) {
+			// typical 15% cache hit-rate so cache probably not worth-while
 			@Override
 			protected SortKey<Mdr11Record> makeKey(Mdr11Record r, Sort sort, Map<String, byte[]> cache) {
 				return sort.createSortKey(r, r.getName(), r.getMapIndex(), cache);
Index: src/uk/me/parabola/imgfmt/app/mdr/Mdr7.java
===================================================================
--- src/uk/me/parabola/imgfmt/app/mdr/Mdr7.java	(revision 4740)
+++ src/uk/me/parabola/imgfmt/app/mdr/Mdr7.java	(working copy)
@@ -51,11 +51,12 @@
 
 	private int partialInfoSize;
 	private Set<String> exclNames;
-	private final Sort sort;
+	private Sort sort;
 	private int maxPrefixCount;
 	private int maxSuffixCount;
 	private boolean magicIsValid;
 	private int magic;
+	private Collator collator;
 
 	public Mdr7(MdrConfig config) {
 		setConfig(config);
@@ -64,6 +65,8 @@
 		exclNames = config.getMdr7Excl();
 		codepage = sort.getCodepage();
 		isMulti = sort.isMulti();
+		collator = sort.getCollator();
+		collator.setStrength(Collator.SECONDARY);
 	}
 
 	public void addStreet(int mapId, String name, int lblOffset, int strOff, Mdr5Record mdrCity) {
@@ -204,7 +207,8 @@
 	@Override
 	protected void preWriteImpl() {
 		
-		LargeListSorter<Mdr7Record> partialSorter = new LargeListSorter<Mdr7Record>(sort) {
+		LargeListSorter<Mdr7Record> partialSorter = new LargeListSorter<Mdr7Record>(sort, true) {
+			// typical 50% cache hit-rate so cache probably worth-while
 			@Override
 			protected SortKey<Mdr7Record> makeKey(Mdr7Record r, Sort sort, Map<String, byte[]> cache) {
 				return sort.createSortKey(r, r.getPartialName(), 0, cache); // first sort by partial name only
@@ -211,14 +215,12 @@
 			}
 		};
 		
-		ArrayList<Mdr7Record> sorted = new ArrayList<>(allStreets);
-		allStreets.clear();
+		ArrayList<Mdr7Record> sorted = allStreets;
+		allStreets = new ArrayList<>(sorted.size());
 		partialSorter.sort(sorted);
 		// list is now sorted by partial name only, we have to group by name and map index now
 		String lastPartial = null;
 		List<Mdr7Record> samePartial = new ArrayList<>();
-		Collator collator = sort.getCollator();
-		collator.setStrength(Collator.SECONDARY);
 		for (int i = 0; i < sorted.size(); i++) {
 			Mdr7Record r = sorted.get(i);
 			String partial = r.getPartialName();
@@ -246,16 +248,20 @@
 		// Basecamp needs the records grouped by partial name, full name, and map index.
 		// This sometimes presents search results in the wrong order. The partial sort fields allow to
 		// tell the right order.
-		
-		LargeListSorter<Mdr7Record> fullNameSorter = new LargeListSorter<Mdr7Record>(sort) {
-			@Override
-			protected SortKey<Mdr7Record> makeKey(Mdr7Record r, Sort sort, Map<String, byte[]> cache) {
-				return sort.createSortKey(r, r.getName(), r.getMapIndex(), cache);
-			}
-		};
-		
-		
-		fullNameSorter.sort(samePartial);
+		if (samePartial.size() > 1) {
+			LargeListSorter<Mdr7Record> fullNameSorter = new LargeListSorter<Mdr7Record>(sort, true) {
+				// Very high hit-rate so use cache.
+				// NB: Mostly there are only a few records to sort here (the common case is 1),
+				// but must use SortKeys rather than a Comparator with Sort.collator.compare(names)
+				// because the special char handling is different.
+				@Override
+				protected SortKey<Mdr7Record> makeKey(Mdr7Record r, Sort sort, Map<String, byte[]> cache) {
+					return sort.createSortKey(r, r.getName(), r.getMapIndex(), cache);
+				}
+			};
+			fullNameSorter.sort(samePartial);
+		}
+
 		Mdr7Record last = null;
 		int recordNumber = streets.size();
 		
@@ -283,8 +289,6 @@
 	public void writeSectData(ImgFileWriter writer) {
 		boolean hasStrings = hasFlag(MDR7_HAS_STRING);
 		boolean hasNameOffset = hasFlag(MDR7_HAS_NAME_OFFSET);
-		Collator collator = sort.getCollator();
-		collator.setStrength(Collator.SECONDARY); 
 		Mdr7Record last = null;
 		for (Mdr7Record s : streets) {
 			addIndexPointer(s.getMapIndex(), s.getIndex());
Index: src/uk/me/parabola/imgfmt/app/net/RouteArc.java
===================================================================
--- src/uk/me/parabola/imgfmt/app/net/RouteArc.java	(revision 4740)
+++ src/uk/me/parabola/imgfmt/app/net/RouteArc.java	(working copy)
@@ -418,7 +418,9 @@
 			int compactedRatio = lengthRatio / 2 - 8;
 			assert compactedRatio > 0 && compactedRatio < 8;
 			curveData = new int[1];
-			curveData[0] = (compactedRatio << 5) | ((dh >> 3) & 0x1f);
+			//curveData[0] = (compactedRatio << 5) | ((dh >> 3) & 0x1f);
+			//try rounding this as well
+			curveData[0] = (compactedRatio << 5) | (((dh+4) >> 3) & 0x1f);
 			
 			/* check math:
 			int dhx = curveData[0] & 0x1f;
Index: src/uk/me/parabola/imgfmt/app/trergn/Polyline.java
===================================================================
--- src/uk/me/parabola/imgfmt/app/trergn/Polyline.java	(revision 4740)
+++ src/uk/me/parabola/imgfmt/app/trergn/Polyline.java	(working copy)
@@ -179,6 +179,8 @@
 
 		if(labelOff != 0)
 			type |= 0x20;		// has label
+		if (direction)
+			type |= FLAG_DIR;   // %%% not sure if this bit means same thing for extended types
 		if(extraBytes != null)
 			type |= 0x80;		// has extra bytes
 		stream.write(type >> 8);
Index: src/uk/me/parabola/mkgmap/filters/ShapeMergeFilter.java
===================================================================
--- src/uk/me/parabola/mkgmap/filters/ShapeMergeFilter.java	(revision 4740)
+++ src/uk/me/parabola/mkgmap/filters/ShapeMergeFilter.java	(working copy)
@@ -54,7 +54,7 @@
 	 * @return list of merged shapes (might be identical to original list)
 	 */
 	public List<MapShape> merge(List<MapShape> shapes) {
-		if (shapes.size() <= 1)
+		if (shapes.size() <= 1 || true)
 			return shapes;
 		List<MapShape> mergedShapes = new ArrayList<>();
 		List<MapShape> usableShapes = new ArrayList<>();
Index: src/uk/me/parabola/mkgmap/main/Main.java
===================================================================
--- src/uk/me/parabola/mkgmap/main/Main.java	(revision 4740)
+++ src/uk/me/parabola/mkgmap/main/Main.java	(working copy)
@@ -605,6 +605,7 @@
 			log.info("nothing to do for combiners.");
 			return;
 		}
+		Logger.defaultLogger.write("Combiners started: " + new Date());
 		log.info("Combining maps");
 
 		args.setSort(getSort(args));
Index: src/uk/me/parabola/util/ShapeSplitter.java
===================================================================
--- src/uk/me/parabola/util/ShapeSplitter.java	(revision 4740)
+++ src/uk/me/parabola/util/ShapeSplitter.java	(working copy)
@@ -471,7 +471,9 @@
 				return cmp;
 			// case where when have same start & end
 			// return the shape as lower than the hole, to handle first
-			cmp = other.areaOrHole - this.areaOrHole;
+//			cmp = other.areaOrHole - this.areaOrHole;
+			// Above is not reliable, there might be an earlier shape. try doing larger area first
+			cmp = Long.compare(this.areaToLine * this.areaOrHole, other.areaToLine * other.areaOrHole);
 			if (cmp != 0)
 				return cmp;
 			log.warn("Lines hit divider at same points and have same area sign", "this:", this, "other:", other);
@@ -644,6 +646,9 @@
 			trailAlong = leadAlong;
 			trailRel = leadRel;
 		} // for leadCoord
+		if (log.isDebugEnabled()) {
+			log.debug("initSplit #points", coords.size(), "on", dividingLine, isLongitude, "Area", runningArea, "#newLess", newLess.size(), "#newMore", newMore.size());
+		}
 		processLineList(newLess, lessList, runningArea);
 		processLineList(newMore, moreList, runningArea);
 	} // splitShape
_______________________________________________
mkgmap-dev mailing list
mkgmap-dev@lists.mkgmap.org.uk
https://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev

Reply via email to