[mkgmap-dev] [Patch V1] Subdivision width is 36627 at 3230916/1236133

Thu May 3 22:53:18 BST 2012

Hi Gerd,

> Hi WanMil,
>
>  > Date: Thu, 3 May 2012 22:10:57 +0200
>  > From: wmgcnfg at web.de
>  > To: mkgmap-dev at lists.mkgmap.org.uk
>  > Subject: Re: [mkgmap-dev] [Patch V1] Subdivision width is 36627 at
> 3230916/1236133
>  >
>  > Ok, I missed that the split routine is called only if the maxdimension
>  > is larger than max width/height.
>  >
>  > Why do you use maxSize-MapSplitter.MIN_AREA_DIMENSION(10) as max
> dimension?
>
> I think any large value smaller than maxSize will work. There is no good
> reason to use this constant, I forgot to remove it
> when I tried to create a test case for the problem described in your
> picture. I failed to create such a case, but I think it is
> possible to hit that case, and my patch doesn't solve it.

Ok, 10 sounds like a reasonable value to subtract from the maxSize.
I am not 100% sure if the test case I have drawn really fails. It's only 
a good guess with a reasonable knowledge of the source code. And I think 
using two bounds for the subdivision (bounds and fullbounds) is a 
potential problem. I am also not sure if your patch fixes that already 
for all cases. Mmh, "I am not sure" seems to be a common sentence when 
handling with subdivisions...

Anyhow the bugfix for the LineSizeSplitterFilter is a good catch and 
will reduce the number of subdivision problems.

WanMil

>
> Gerd
>
>  >
>  > WanMil
>  >
>  > > Hi WanMil,
>  > >
>  > > makeSplittable is called very seldom, and the number of points is
> typically
>  > > also small,
>  > > so performance really doesn't matter here (at least not with my
> test cases).
>  > > I did not want to modify the points of the original MapLine object
> to avoid
>  > > side effects.
>  > > I did also want to avoid complex calculations for the new points, so I
>  > > simply always add one
>  > > new point in the middle of the existing ones.
>  > >
>  > > Adding the calculated points to the List coords that is used to
> create the
>  > > new MapLine
>  > > objects might be an option, but I did not find a simple way to
> handle the
>  > > loop control for
>  > > cases where we have to add multiple points.
>  > >
>  > > So, in short, I didn't find a better solution.
>  > >
>  > > Regarding the other changes in the patch:
>  > > I agree that the code is complex, but I did not see a reason for a
> complete
>  > > rework.
>  > >
>  > > Gerd
>  > >
>  > >
>  > >
>  > >
>  > > WanMil wrote
>  > >>
>  > >> Hi Gerd,
>  > >>
>  > >> the LineSizeSplitterFilter patch is a good idea. After reading the
> code
>  > >> I am not sure if the other changes work for all situations (just
> because
>  > >> the things are quite complex there). I think I will wait for at least
>  > >> one week if someone complains...
>  > >>
>  > >> Anyhow I want you as our performance guy to have a look on the
>  > >> LineSizeSplitterFilter :-) makeSplittable always copies all points
>  > >> although it changes them in very rare cases.
>  > >> Can you check (and implement) the following approach?
>  > >> ListIterator<Coord> iter = points.listIterator();
>  > >> Check all subsequent points. If maxWidth or maxHeight is exceeded
>  > >> calculate the number of points that must be added (something like int
>  > >> split = Math.max(realWith/maxWidth, realHeigh/maxHeight))
>  > >> Go back one point in the list iterator and add the additional
> points to
>  > >> the list iterator.
>  > >>
>  > >> WanMil
>  > >>
>  > >>> Hi all,
>  > >>>
>  > >>> attached is a corrected version of the patch. Please note the
> change in
>  > >>> the
>  > >>> logger initialisation for
>  > >>> LineSizeSplitterFilter. I guess the old code was not intended.
>  > >>>
>  > >>> Gerd
>  > >>>
> http://gis.19327.n5.nabble.com/file/n5680163/subdivision_width_v2.patch
>  > >>> subdivision_width_v2.patch
>  > >>>
>  > >>>
>  > >>> GerdP wrote
>  > >>>>
>  > >>>> Hi Marko,
>  > >>>>
>  > >>>> unfortunately it produces new errors for a tile in south-america
> which
>  > >>>> wasn't in my test data yesterday, so it should not be used yet :-(
>  > >>>>
>  > >>>> Gerd
>  > >>>>
>  > >>>>
>  > >>>>
>  > >>>>
>  > >>>> Marko Mäkelä wrote
>  > >>>>>
>  > >>>>> Hi Gerd,
>  > >>>>>
>  > >>>>>>
> http://gis.19327.n5.nabble.com/file/n5672934/subdivision_width_v1.patch
>  > >>>>>
>  > >>>>> Thanks, this removed the message. I did not test the resulting
> map yet,
>  > >>>>> but I will do that when downloading and compiling the next map
> extract.
>  > >>>>>
>  > >>>>> Marko
>  > >>>>> _______________________________________________
>  > >>>>> mkgmap-dev mailing list
>  > >>>>> mkgmap-dev at .org
>  > >>>>> http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
>  > >>>>>
>  > >>>>
>  > >>>
>  > >>>
>  > >>> --
>  > >>> View this message in context:
>  > >>>
> http://gis.19327.n5.nabble.com/Patch-V1-Subdivision-width-is-36627-at-3230916-1236133-tp5672934p5680163.html
>  > >>> Sent from the Mkgmap Development mailing list archive at Nabble.com.
>  > >>> _______________________________________________
>  > >>> mkgmap-dev mailing list
>  > >>> mkgmap-dev at .org
>  > >>> http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
>  > >>
>  > >> _______________________________________________
>  > >> mkgmap-dev mailing list
>  > >> mkgmap-dev at .org
>  > >> http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
>  > >>
>  > >
>  > >
>  > > --
>  > > View this message in context:
> http://gis.19327.n5.nabble.com/Patch-V1-Subdivision-width-is-36627-at-3230916-1236133-tp5672934p5682325.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