[Patch V1] Subdivision width is 36627 at 3230916/1236133
Hi all, attached is a patch that fixes a few problems with r2272 1) When the initial area is created, the area bounds were only updated when a point was added. Lines and shapes were added without updating the area bounds. The patch changes this. 2) The splitMaxSize() method did used the calculated bounds, not the bounds of the elements. Therefore, it did not split the area when the number of elements was small. 3) The LineSizeSplitterFilter did not split a line to parts that are smaller than maxSize, it always added the point that made the bbox of the line too large. If this last point is far away from the others, the bbox is far too large. The patch makes sure that a line is split into parts that are small enough. Regarding the problem described by WanMils picture: This is not yet solved with this patch. I have to create some test cases to analyse this also. Gerd http://gis.19327.n5.nabble.com/file/n5672934/subdivision_width_v1.patch subdivision_width_v1.patch -- View this message in context: http://gis.19327.n5.nabble.com/Patch-V1-Subdivision-width-is-36627-at-323091... Sent from the Mkgmap Development mailing list archive at Nabble.com.
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
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@.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-323091... Sent from the Mkgmap Development mailing list archive at Nabble.com.
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@.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-323091... Sent from the Mkgmap Development mailing list archive at Nabble.com.
On Wed, May 02, 2012 at 03:40:45AM -0700, GerdP wrote:
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
Thanks, this makes the http://www.openstreetmap.org/browse/way/127417755 visible on the map and grows gmapsupp.img a little bit. Marko
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@.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-323091... Sent from the Mkgmap Development mailing list archive at Nabble.com. _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
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@.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-323091... Sent from the Mkgmap Development mailing list archive at Nabble.com. _______________________________________________ mkgmap-dev mailing list mkgmap-dev@.org http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
_______________________________________________ mkgmap-dev mailing list mkgmap-dev@.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-323091... Sent from the Mkgmap Development mailing list archive at Nabble.com.
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? 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@.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-323091... Sent from the Mkgmap Development mailing list archive at Nabble.com. _______________________________________________ mkgmap-dev mailing list mkgmap-dev@.org http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
_______________________________________________ mkgmap-dev mailing list mkgmap-dev@.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-323091... Sent from the Mkgmap Development mailing list archive at Nabble.com. _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Hi WanMil,
Date: Thu, 3 May 2012 22:10:57 +0200 From: wmgcnfg@web.de To: mkgmap-dev@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. 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@.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-323091... Sent from the Mkgmap Development mailing list archive at Nabble.com. _______________________________________________ mkgmap-dev mailing list mkgmap-dev@.org http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
_______________________________________________ mkgmap-dev mailing list mkgmap-dev@.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-323091... Sent from the Mkgmap Development mailing list archive at Nabble.com. _______________________________________________ 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
Hi Gerd,
Hi WanMil,
Date: Thu, 3 May 2012 22:10:57 +0200 From: wmgcnfg@web.de To: mkgmap-dev@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@.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-323091...
Sent from the Mkgmap Development mailing list archive at Nabble.com. _______________________________________________ mkgmap-dev mailing list mkgmap-dev@.org http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
_______________________________________________ mkgmap-dev mailing list mkgmap-dev@.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-323091... Sent from the Mkgmap Development mailing list archive at Nabble.com. _______________________________________________ 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
Hi, I have uploaded a patched mkgmap to http://files.mkgmap.org.uk/detail/66. @all: please test with your maps if it works fine or if you see any strange effects. 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@.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-323091... Sent from the Mkgmap Development mailing list archive at Nabble.com. _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Hi WanMil, I tried your patched version and it seems to work well. No errors while creating my maps, but there wasn't an error before. So I don't know if it helps. Henning Am 03.05.2012 22:06, schrieb WanMil:
Hi,
I have uploaded a patched mkgmap to http://files.mkgmap.org.uk/detail/66. @all: please test with your maps if it works fine or if you see any strange effects.
WanMil
participants (5)
-
aighes -
Gerd Petermann -
GerdP
-
Marko Mäkelä -
WanMil