[mkgmap-dev] [Patch] Error in BoundaryUtil

Sun Mar 11 20:02:56 GMT 2012

> Hi WanMil,
>
>
>
>  > Date: Sun, 11 Mar 2012 20:08:54 +0100
>  > From: wmgcnfg at web.de
>  > To: mkgmap-dev at lists.mkgmap.org.uk
>  > Subject: Re: [mkgmap-dev] [Patch] Error in BoundaryUtil
>  >
>  > i Gerd,
>  >
>  > can you please explain in short what's the difference between the three
>  > formats (my thinking):
>  > * Legacy (original - int coords)
>  > * Raw (float coords)
>  > * Quadtree (admin level areas merged to the quadtree format)
>
> yes, correct
>
>  >
>  > loadQuadTree (Quadtree format?) => loadQuadTreeFromStream (Quadtree
>  > format?) => readStreamRawFormat (Raw format?) => readArea (Legacy format)
>  >
>  > At least it is irritating to me if there is a method readStreamRawFormat
>  > which reads raw and/or legacy format.
>
> I agree. I'll change that so that we have two methods
> readStreamLegacyFormat and readStreamRawFormat.

Great! Maybe it makes also sense to put the detection of the format into 
one method? In the moment there are a lot of rel.endsWith(...) checks in 
the code.

>
>  >
>  > I would also propose a change to the header of the new raw and quadtree
>  > format. In the legacy format the first String in the file gives the
>  > release of mkgmap. You added a "_raw" or "_quadtree" (?) to mark the new
>  > formats. This is unclean but required to support the legacy format. But
>  > legacy format support can be dropped in the future (I think at least in
>  > the end of 2012). So to be open for future other changes it would be
>  > good to have a kind of format identification. This could be added after
>  > the creation time. Having this format identification future changes to
>  > the format could be easily and much cleaner added.
>
> Yes, my problem with the legacy format was that it did not contain such
> a format identifier.
> I've worked a long time with programs on IBMs mainframe system (OS/390,
> z/OS).
> They typically use an eye catcher to identify the record type and a
> numeric field that
> identifies the record layout version. In some programs, they have also a
> length field.
> Old programs can read newer structures because the newer fields are only
> added
> at the end of a structure, and the length field allows to skip them.
> I think this is a good idea, but we probably don't have to support all
> kinds of
> versions in programs and files.
>
> So, here is my suggestion:
> instead of adding _raw or _quadtree to the 1st string I replace it with BND.
> Legacy format is identified because it doesn't contain this string.
> A new String field fileFormat after the timestamp identifies the file
> format,
> either RAW or QUADTREE.
> One more int field recId identifies the record layouts used (0 for this
> first version).
> Whenever a new format is needed, we change fileFormat, if only a new
> field is needed in e.g. the AREA section, we increase recId.
> All methods that read the file verify that they are able to process the
> recId
> or they stop with an FormatException .
> OK?

Sounds good. But I propose two additional things:
1. a header size or id (otherwise we will have the same problem again to 
add an additional information to the header)
2. Keep an mkgmap release identifier.

So the new format would look like:
Format type: String: BND
Header size: int
| Record format: String: (RAW, QUADTREE)
| Record layout: int
| Timestamp: long
| mkgmap release: String
... Records ...

WanMil

>
> Gerd
>
>  >
>  > WanMil
>  >
>  > > Hi WanMil,
>  > >
>  > > hmm, we have two errors here. I can confirm that my patch doesn't solve
>  > > all problems,
>  > > it just allows again to read the old *.bnd format and the new quadtree
>  > > format, but mkgmap
>  > > fails to read the intermediate format produced by the preparers 1st
> pass.
>  > >
>  > > > Date: Sun, 11 Mar 2012 18:25:21 +0100
>  > > > From: wmgcnfg at web.de
>  > > > To: mkgmap-dev at lists.mkgmap.org.uk
>  > > > Subject: Re: [mkgmap-dev] [Patch] Error in BoundaryUtil
>  > > >
>  > > > Hi Gerd,
>  > > >
>  > > > without your patch preparing boundaries works.
>  > >
>  > > No, it doesn't. It stops (crashes) without any error message when
> trying
>  > > to read the *.bnd
>  > > files produced in the 1st pass. You can verify this by looking at the
>  > > header of
>  > > the new *.bnd files, they have the suffix _raw, not _quadtree.
>  > >
>  > > The new patch fixes this, but not the missing error detection.
>  > >
>  > > I think we need a try/catch block somewhere, either in Preparer or
>  > > BoundaryPreparer.
>  > > Will you take care of this?
>  > >
>  > > Gerd
>  > >
>  > >
>  > > > Using your patch I get the following exception.
>  > > >
>  > > > java.util.concurrent.ExecutionException:
>  > > > java.lang.IllegalArgumentException: Ill
>  > > > egal Capacity: -572471432
>  > > > at java.util.concurrent.FutureTask$Sync.innerGet(Unknown Source)
>  > > > at java.util.concurrent.FutureTask.get(Unknown Source)
>  > > > at
>  > > > uk.me.parabola.mkgmap.main.Preparer.runPreparer(Preparer.java:92)
>  > > > at uk.me.parabola.mkgmap.main.Main.endOptions(Main.java:349)
>  > > > at
>  > > > uk.me.parabola.mkgmap.CommandArgsReader.readArgs(CommandArgsReader.ja
>  > > > va:126)
>  > > > at uk.me.parabola.mkgmap.main.Main.main(Main.java:114)
>  > > > Caused by: java.lang.IllegalArgumentException: Illegal Capacity:
>  > > -572471432
>  > > > at java.util.ArrayList.<init>(Unknown Source)
>  > > > at
>  > > > uk.me.parabola.mkgmap.reader.osm.boundary.BoundaryUtil.readStreamRawF
>  > > > ormat(BoundaryUtil.java:282)
>  > > > at
>  > > > uk.me.parabola.mkgmap.reader.osm.boundary.BoundaryUtil.loadQuadTreeFr
>  > > > omStream(BoundaryUtil.java:459)
>  > > > at
>  > > > uk.me.parabola.mkgmap.reader.osm.boundary.BoundaryUtil.loadQuadTree(B
>  > > > oundaryUtil.java:145)
>  > > > at
>  > > > uk.me.parabola.mkgmap.reader.osm.boundary.BoundaryUtil.loadQuadTree(B
>  > > > oundaryUtil.java:120)
>  > > > at
>  > > > uk.me.parabola.mkgmap.reader.osm.boundary.BoundaryPreparer$QuadTreeWo
>  > > > rker.call(BoundaryPreparer.java:236)
>  > > > at
>  > > > uk.me.parabola.mkgmap.reader.osm.boundary.BoundaryPreparer$QuadTreeWo
>  > > > rker.call(BoundaryPreparer.java:221)
>  > > > at java.util.concurrent.FutureTask$Sync.innerRun(Unknown Source)
>  > > > at java.util.concurrent.FutureTask.run(Unknown Source)
>  > > > at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown
>  > > > Source)
>  > > > at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown
>  > > > Source)
>  > > > at java.lang.Thread.run(Unknown Source)
>  > > >
>  > > > My mkgmap call is:
>  > > > java -jar mkgmap.jar --max-jobs=4
>  > > > --createboundsfile=africa-boundaries.osm.gz
> --bounds=bounds/africa *.osm
>  > > >
>  > > > WanMil
>  > > >
>  > > > > Hi WanMil,
>  > > > >
>  > > > > sorry, one of my last patches for the performance branch corrupted
>  > > > > BoundaryUtil.
>  > > > > It was no longer able to read the legacy *.bnd format (also created
>  > > in the
>  > > > > preparer) :-(
>  > > > >
>  > > > > Attached is the patch.
>  > > > >
>  > > > > Gerd
>  > > > >
>  > > > >
> http://gis.19327.n5.nabble.com/file/n5554370/BoundaryUtil.java.patch
>  > > > > BoundaryUtil.java.patch
>  > > > >
>  > > > > --
>  > > > > View this message in context:
>  > >
> http://gis.19327.n5.nabble.com/Patch-Error-in-BoundaryUtil-tp5554370p5554370.html
>  > > > > Sent from the Mkgmap Development mailing list archive at
> Nabble.com.
>  > > > > _______________________________________________
>  > > > > mkgmap-dev mailing list
>  > > > > mkgmap-dev at lists.mkgmap.org.uk
>  > > > > http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
>  > > >
>  > > > _______________________________________________
>  > > > mkgmap-dev mailing list
>  > > > mkgmap-dev at lists.mkgmap.org.uk
>  > > > http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
>  > >
>  > >
>  > > _______________________________________________
>  > > mkgmap-dev mailing list
>  > > mkgmap-dev at lists.mkgmap.org.uk
>  > > http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
>  >
>  > _______________________________________________
>  > mkgmap-dev mailing list
>  > mkgmap-dev at lists.mkgmap.org.uk
>  > http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
>
>
> _______________________________________________
> mkgmap-dev mailing list
> mkgmap-dev at lists.mkgmap.org.uk
> http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev