[PATCH v1] make DP filter avoid removing points located at nodes
Hi Felix, Please try the attached patch. I've tested it inasmuch that it doesn't blow up but whether it actually does the right thing when you have a routable and non-routable way from the same set of points, I don't know. Cheers, Mark
Mark Burton wrote:
Hi Felix,
Please try the attached patch. I've tested it inasmuch that it doesn't blow up but whether it actually does the right thing when you have a routable and non-routable way from the same set of points, I don't know.
Cheers,
Mark
Blows up for me though... D:\Garmin\mkgmap_680>REM austria at 6365 this is run35 23:05:42 java.lang.NoSuchMethodError: uk.me.parabola.imgfmt.app.CoordNode.isBoundary()Z at uk.me.parabola.mkgmap.filters.RoundCoordsFilter.doFilter(RoundCoordsFilter.java:56) at uk.me.parabola.mkgmap.build.LayerFilterChain.doFilter(LayerFilterChain.java:57) at uk.me.parabola.mkgmap.build.LayerFilterChain.startFilter(LayerFilterChain.java:75) at uk.me.parabola.mkgmap.build.MapBuilder.processLines(MapBuilder.java:883) at uk.me.parabola.mkgmap.build.MapBuilder.makeSubdivision(MapBuilder.java:645) at uk.me.parabola.mkgmap.build.MapBuilder.makeMapAreas(MapBuilder.java:580) at uk.me.parabola.mkgmap.build.MapBuilder.makeMap(MapBuilder.java:188) at uk.me.parabola.mkgmap.main.MapMaker.makeMap(MapMaker.java:96) at uk.me.parabola.mkgmap.main.MapMaker.makeMap(MapMaker.java:61) at uk.me.parabola.mkgmap.main.Main$1.call(Main.java:187) at uk.me.parabola.mkgmap.main.Main$1.call(Main.java:185) at java.util.concurrent.FutureTask$Sync.innerRun(Unknown Source) at java.util.concurrent.FutureTask.run(Unknown Source) at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(Unknown Source) at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source) at java.lang.Thread.run(Unknown Source) Exiting - if you want to carry on regardless, use the --keep-going option Tell me if you need logs or similar stuff. Will (not continue tonight however...)
------------------------------------------------------------------------
_______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Blows up for me though...
ant clean BTW - I forgot to mention that the patch is only effective when the remove-short-arcs option is being used. This is just a convenience because it is scanning all the points in the ways anyway then but it could be done as a separate step if the short arc removal was not being done. Mark
Hi Mark, I've not tested the patch, but looked at the diff. Could not see the sense in it at a glance. For me it makes no difference to the previous state. What do you intend with this patch? Regards, Johann
Hi Johann,
Hi Mark,
I've not tested the patch, but looked at the diff. Could not see the sense in it at a glance. For me it makes no difference to the previous state. What do you intend with this patch?
Regards, Johann
If I understand the problem correctly, if someone (Felix, for example) generates multiple lines from the same OSM way and one of the lines is routable and the other(s) are not, then the original DP code would generate different shaped lines for the routable and non-routable ways because only the routable version of the line had CoordNodes in it. So, my patch just marks those points that are going to be nodes but that's done before the way is duplicated so those marked points will be seen in the DP code for both the routable and non-routable lines that are based on the same set of points. That's the theory, hopefully someone (Felix, for example) will test the patch and let us know if it works as expected. Cheers, Mark
Mark Burton wrote:
Hi Johann,
Hi Mark,
I've not tested the patch, but looked at the diff. Could not see the sense in it at a glance. For me it makes no difference to the previous state. What do you intend with this patch?
Regards, Johann
If I understand the problem correctly, if someone (Felix, for example) generates multiple lines from the same OSM way and one of the lines is routable and the other(s) are not, then the original DP code would generate different shaped lines for the routable and non-routable ways because only the routable version of the line had CoordNodes in it. So, my patch just marks those points that are going to be nodes but that's done before the way is duplicated so those marked points will be seen in the DP code for both the routable and non-routable lines that are based on the same set of points. That's the theory, hopefully someone (Felix, for example) will test the patch and let us know if it works as expected.
Cheers,
Mark
okay back... Well running ant-clean it now doesn't even compile using the patch (of course I know that not ant-clean is the problem here, more that I did not use it before cause I though just another patch on top...) [javac] Compiling 315 source files to d:\Garmin\mkgmap_svn_trunk\build\classes [javac] d:\Garmin\mkgmap_svn_trunk\src\uk\me\parabola\mkgmap\filters\RoundCoordsFilter.java:56: cannot find symbol [javac] symbol : method isBoundary() [javac] location: class uk.me.parabola.imgfmt.app.CoordNode [javac] newP = new CoordNode(lat, lon, ((CoordNode)p).getId(), ((CoordNode)p).isBoundary()); [javac] ^ [javac] d:\Garmin\mkgmap_svn_trunk\src\uk\me\parabola\mkgmap\filters\RoundCoordsFilter.java:56: internal error; cannot instantiate Co [javac] newP = new CoordNode(lat, lon, ((CoordNode)p).getId(), ((CoordNode)p).isBoundary()); [javac] ^ I attach the patches that I am using. - I think the mb-round-coords-v2 is causing some problems in combination. -- I'll remove that patch and try again the DP-avoid filter without round cords. More soon.
_______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Felix Hartmann wrote:
Mark Burton wrote:
Hi Johann,
Hi Mark,
I've not tested the patch, but looked at the diff. Could not see the sense in it at a glance. For me it makes no difference to the previous state. What do you intend with this patch?
Regards, Johann
If I understand the problem correctly, if someone (Felix, for example) generates multiple lines from the same OSM way and one of the lines is routable and the other(s) are not, then the original DP code would generate different shaped lines for the routable and non-routable ways because only the routable version of the line had CoordNodes in it. So, my patch just marks those points that are going to be nodes but that's done before the way is duplicated so those marked points will be seen in the DP code for both the routable and non-routable lines that are based on the same set of points. That's the theory, hopefully someone (Felix, for example) will test the patch and let us know if it works as expected.
Cheers,
Mark
okay back...
Well running ant-clean it now doesn't even compile using the patch (of course I know that not ant-clean is the problem here, more that I did not use it before cause I though just another patch on top...)
[javac] Compiling 315 source files to d:\Garmin\mkgmap_svn_trunk\build\classes [javac] d:\Garmin\mkgmap_svn_trunk\src\uk\me\parabola\mkgmap\filters\RoundCoordsFilter.java:56: cannot find symbol [javac] symbol : method isBoundary() [javac] location: class uk.me.parabola.imgfmt.app.CoordNode [javac] newP = new CoordNode(lat, lon, ((CoordNode)p).getId(), ((CoordNode)p).isBoundary()); [javac] ^ [javac] d:\Garmin\mkgmap_svn_trunk\src\uk\me\parabola\mkgmap\filters\RoundCoordsFilter.java:56: internal error; cannot instantiate Co [javac] newP = new CoordNode(lat, lon, ((CoordNode)p).getId(), ((CoordNode)p).isBoundary()); [javac] ^
I attach the patches that I am using. - I think the mb-round-coords-v2 is causing some problems in combination.
-- I'll remove that patch and try again the DP-avoid filter without round cords. More soon.
Okay, works like a treat. See the attached screenshot what difference it makes. vs before (disregard the brown contourlines) - this is both using detail lowest, to spot the error easier.
_______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Oops, sorry about the compilation problems, I didn't check that this patch was compatible with the round coords patch, v2 is.
If I understand the problem correctly, if someone (Felix, for example) generates multiple lines from the same OSM way and one of the lines is routable and the other(s) are not, then the original DP code would generate different shaped lines for the routable and non-routable ways because only the routable version of the line had CoordNodes in it. OK, I understand the problem so far. So, my patch just marks those points that are going to be nodes but that's done before the way is duplicated so those marked points will be seen in the DP code for both the routable and non-routable lines that are based on the same set of points. That's the theory, hopefully someone (Felix, for example) will test the patch and let us know if it works as expected.
Isn't the only difference between routable and non routable lines the occurence of CoordNodes? Is I understand the structure of the classes, a Coord has only a location info, and a CoordNode is nearly the same, but means, that this coord has connections to multiple ways. This means not neccessarily that the way is routable, but that there are interconnections of lines. (I have not looked into the code up to now, this is my understanding from remembering) Your patch has moved the information 'this is a connecting node' from the class level to a member level. I would not say it is wrong, but this increases memory consumption, as this adds a new member (4 bytes?) to every instance of coord! So in my opinion the error is in the copy function of the line. If a line contains CoordNodes, than also the copy should contain CoordNodes. Can you point me to the code, where the lines gets duplicated? This all was some unfinished work of the dp filter. I would need some time to understand the classes structure in full and prepare a solution. Regards, Johann
Cheers,
Mark _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev .
Hi Johann,
Isn't the only difference between routable and non routable lines the occurence of CoordNodes? Is I understand the structure of the classes, a Coord has only a location info, and a CoordNode is nearly the same, but means, that this coord has connections to multiple ways. This means not neccessarily that the way is routable, but that there are interconnections of lines. (I have not looked into the code up to now, this is my understanding from remembering)
That's correct.
Your patch has moved the information 'this is a connecting node' from the class level to a member level. I would not say it is wrong, but this increases memory consumption, as this adds a new member (4 bytes?) to every instance of coord!
But does it increase the size of a Coord? I admit to not having actually measured the size but I've added a boolean to an object that already contains a boolean. If the Java implementers are smart they would be stored as single bits rather than bytes/shorts/words. Perhaps someone who knows about this could comment. Also, the patch removes a field from the CoordNode class that basically did the same job, so it's actually possible that there is a net reduction in memory required.
So in my opinion the error is in the copy function of the line. If a line contains CoordNodes, than also the copy should contain CoordNodes. Can you point me to the code, where the lines gets duplicated?
Whenever a routable way gets split to limit the number of points or nodes it makes a new list of points so any changes to the points in the new list will not be visible in the original list.
This all was some unfinished work of the dp filter. I would need some time to understand the classes structure in full and prepare a solution.
Well, I think the outstanding issue at the moment is whether the DP filter can be made more aggressive at the lower resolutions. The idea of combining ways to reduce the number of segments is interesting but at odds with our current strategy of limiting the length of ways due to routing problems. More thought required there. Cheers, Mark
MB> But does it increase the size of a Coord? I admit to not having MB> actually measured the size but I've added a boolean to an object MB> that already contains a boolean. If the Java implementers are smart MB> they would be stored as single bits rather than bytes/shorts/words. MB> Perhaps someone who knows about this could comment. I haven't looked at the code/patch in question, however in most/all the VMs I'm aware of a boolean will use 4 bytes (same as an int). Multiple booleans aren't packed, unless they're stored in an array (where they're packed one per byte). There's also the possibility of word alignment causing additional bytes to be wasted, but that depends on whether it's a 32 or 64 bit VM, and the number and types of other fields in the same class. If you need to pack bits as tightly as possible there's java.util.BitSet, but that only starts to make sense if you have a large number of booleans. If you only have a few of them, custom bit manipulation would be required to mimise the memory impact. Chris
Hi Chris, On Mon, 23 Nov 2009 12:06:18 +0000 (UTC) Chris Miller <chris.miller@kbcfp.com> wrote:
MB> But does it increase the size of a Coord? I admit to not having MB> actually measured the size but I've added a boolean to an object MB> that already contains a boolean. If the Java implementers are smart MB> they would be stored as single bits rather than bytes/shorts/words. MB> Perhaps someone who knows about this could comment.
I haven't looked at the code/patch in question, however in most/all the VMs I'm aware of a boolean will use 4 bytes (same as an int). Multiple booleans aren't packed, unless they're stored in an array (where they're packed one per byte). There's also the possibility of word alignment causing additional bytes to be wasted, but that depends on whether it's a 32 or 64 bit VM, and the number and types of other fields in the same class.
If you need to pack bits as tightly as possible there's java.util.BitSet, but that only starts to make sense if you have a large number of booleans. If you only have a few of them, custom bit manipulation would be required to mimise the memory impact.
java version "1.6.0_16" Java(TM) SE Runtime Environment (build 1.6.0_16-b01) Java HotSpot(TM) 64-Bit Server VM (build 14.2-b01, mixed mode) Just tried a quick test and on an Intel box I am seeing 1 byte per boolean, not 4. As the object sizes are rounded to multiples of 8, it looks like a case of buy one, and get 7 for free! Cheers, Mark
As the object sizes are rounded to multiples of 8, it looks like a case of buy one, and get 7 for free!
Yes, just tried that and the same amount of memory is used for 1 boolean and 8 booleans. I attach the test program I am using (snarfed from the web with extra booleans added by me). Cheers, Mark
Interesting, I wonder when that changed? I'm fairly sure that never used to be the case. I don't think it's something that you should rely on, especially since the VM spec says the following: "Although the Java virtual machine defines a boolean type, it only provides very limited support for it. There are no Java virtual machine instructions solely dedicated to operations on boolean values. Instead, expressions in the Java programming language that operate on boolean values are compiled to use values of the Java virtual machine int data type. The Java virtual machine does directly support boolean arrays. Its newarray instruction enables creation of boolean arrays. Arrays of type boolean are accessed and modified using the byte array instructions baload and bastore.2 The Java virtual machine encodes boolean array components using 1 to represent true and 0 to represent false. Where Java programming language boolean values are mapped by compilers to values of Java virtual machine type int, the compilers must use the same encoding." (source: http://java.sun.com/docs/books/jvms/second_edition/html/Overview.doc.html#22...)
As the object sizes are rounded to multiples of 8, it looks like a case of buy one, and get 7 for free!
MB> Yes, just tried that and the same amount of memory is used for 1 MB> boolean and 8 booleans. MB> MB> I attach the test program I am using (snarfed from the web with MB> extra booleans added by me). MB> MB> Cheers, MB> MB> Mark MB>
Hi Chris,
Interesting, I wonder when that changed? I'm fairly sure that never used to be the case. I don't think it's something that you should rely on, especially since the VM spec says the following:
...
Here's something else I guess you shouldn't depend on: further experimentation shows that the order of the class member declarations doesn't make any difference to the object size. So you can intersperse the bools with the ints and the object size doesn't change. The Java compiler must order the fields to minimise the space required. That's fine because (not being C) you can't address the object's fields by pointer so the ordering is not visible to the programmer. Cheers, Mark
Hi Mark, As far as I'm aware that's an implementation detail only, not mandated by the spec, so you're right I don't think it's something you can rely on. Having said that, it never used to be an issue when the minimum size allocated to each primitive was 4 bytes! I guess this change must have come in with Java 6. I'm surprised that boolean is only using one byte given that non-aligned access is more expensive than aligned, but I guess the VM implementors have found a way of dealing with that. Also note that Java bytecode only has 4 and 8 byte types, so it really is the VM that's doing all the clever stuff here, not the compiler. In response to an earlier comment of yours: "If the Java implementers are smart they would be stored as single bits rather than bytes/shorts/words. Perhaps someone who knows about this could comment" I don't pretend to know all that much about what it would take to implement this properly, but my understanding is that this isn't really feasible because operations on the bits would need to be performed atomically to prevent race conditions against the other booleans, and that becomes very expensive. It's quite interesting how the extra level of abstraction the VM provides makes these sort of optimisations a moving target. Chris MB> Here's something else I guess you shouldn't depend on: MB> MB> further experimentation shows that the order of the class member MB> declarations doesn't make any difference to the object size. So you MB> can MB> intersperse the bools with the ints and the object size doesn't MB> change. MB> The Java compiler must order the fields to minimise the space MB> required. MB> That's fine because (not being C) you can't address the object's MB> fields by pointer so the ordering is not visible to the programmer. MB> Cheers, MB> MB> Mark
Mark Burton schrieb:
As the object sizes are rounded to multiples of 8, it looks like a case of buy one, and get 7 for free!
Yes, just tried that and the same amount of memory is used for 1 boolean and 8 booleans.
Sorry, I cannot confirm this. Out of personal interest I have run your test programm with OpenJDK6 and get following results: johann@ubuntu910:~/workspace>java Test Size for LotsOfBooleans: 87978536 Average size: 87.978536 Size for LotsOfInts: 336000000 Average size: 336.0 This means 1Byte/Boolean and 4Bytes/Int plus 8 Byte per Instance of object, as I expected it. Maybe I had to run the vm with a java option? Regards, Johann
Hi Johann,
Mark Burton schrieb:
As the object sizes are rounded to multiples of 8, it looks like a case of buy one, and get 7 for free!
Yes, just tried that and the same amount of memory is used for 1 boolean and 8 booleans.
Sorry, I cannot confirm this.
But your figures below do confirm that, don't they?
Out of personal interest I have run your test programm with OpenJDK6 and get following results:
johann@ubuntu910:~/workspace>java Test Size for LotsOfBooleans: 87978536 Average size: 87.978536 Size for LotsOfInts: 336000000 Average size: 336.0
This means 1Byte/Boolean and 4Bytes/Int plus 8 Byte per Instance of object, as I expected it. Maybe I had to run the vm with a java option?
Yes, but if you only have 1 boolean in the class with the ints it still uses 336 bytes. So you get 7 for free! If you add no booleans to the class, the size is 328. So I guess that's why they say that some people think the class is half empty while the others say that the class is half full! Cheers, Mark
Mark Burton schrieb:
As the object sizes are rounded to multiples of 8, it looks like a case of buy one, and get 7 for free!
Yes, just tried that and the same amount of memory is used for 1 boolean and 8 booleans.
...
Yes, but if you only have 1 boolean in the class with the ints it still uses 336 bytes. So you get 7 for free! If you add no booleans to the class, the size is 328. So I guess that's why they say that some people think the class is half empty while the others say that the class is half full!
Yes, you' re right. I missed the fact that 1 boolean needs the same space as 8 booleans. So you get indeed 7 for free. Thanks for clarifying.
On Nov 23, 2009, at 22:02, Mark Burton wrote:
So I guess that's why they say that some people think the class is half empty while the others say that the class is half full!
It's been quite some time since I have read something so witty. You should be proud. Cheers.
Hi Clinton,
So I guess that's why they say that some people think the class is half empty while the others say that the class is half full!
It's been quite some time since I have read something so witty. You should be proud.
Yes I am, immensely, but I have to own up, it was a Freudian slip. I really meant to write glass! Must have been the beer I had recently consumed. Cheers, Mark
Hi Mark, On Mon, Nov 23, 2009 at 10:35:04PM +0000, Mark Burton wrote:
Hi Clinton,
So I guess that's why they say that some people think the class is half empty while the others say that the class is half full!
It's been quite some time since I have read something so witty. You should be proud.
Yes I am, immensely, but I have to own up, it was a Freudian slip. I really meant to write glass! Must have been the beer I had recently consumed.
I found the pun funny for a different reason. In Finnish, we do not have b and g, and some Finns have difficulties with them when speaking foreign languages. So, we could as well say that your peer class was half empty. :-) Marko
So in my opinion the error is in the copy function of the line. If a line contains CoordNodes, than also the copy should contain CoordNodes. Can you point me to the code, where the lines gets duplicated?
Whenever a routable way gets split to limit the number of points or nodes it makes a new list of points so any changes to the points in the new list will not be visible in the original list.
Yes, its true that changes in the copy will not be visible in the original. But this is not my point. My point is: If the original line contains CoordNodes (instead of Nodes) then the copy should also contain CoordNodes. I think, this will complicate the split/copy functions in some ways, so maybe your solution is the easier way.
This all was some unfinished work of the dp filter. I would need some time to understand the classes structure in full and prepare a solution.
Well, I think the outstanding issue at the moment is whether the DP filter can be made more aggressive at the lower resolutions.
The DP filter could be surely made more aggresive at lower resolutions. The simplest way to do this would be to remove the condition at lower resolutions or even complete. // If a node in the line or coord lies on a boundary // then use the douglas peucker algorithm for upper segment // TODO: Should consider only nodes connected to roads visible at current resolution. - if ((p instanceof CoordNode) || (p.getOnBoundary())) { + if ((p instanceof CoordNode && resolution == 24) || (p.getOnBoundary())) { douglasPeucker(coords, i, endIndex, maxErrorDistance); endIndex = i; } But the side effect will be that T-crossings will not always meet exactly. With the current distance of 5.4 the errors should be small enough to be invisible on a gps unit. We should test it.
The idea of combining ways to reduce the number of segments is interesting but at odds with our current strategy of limiting the length of ways due to routing problems. More thought required there.
Yes, I have played with this idea and made a patch public some time ago. It made the dp filter working really much more better at low resolutions. But there was two or more problems with it. First it disturbs routing. This seems to be caused by merging the lines AFTER the routing tables are created. (Thinking about it, i came to the solution to limit the merge filter to nonroutable lines will solve this problems) Second it combines the lines after your splitting for the MAX_LENGTH. So it is possible to get lines longer than MAX_LENGTH. Maybe the size limit should be checked after all other filters. This would have as a side effect the advantage that it works also for polish input files and not only osm files. Regards, Johann
So in my opinion the error is in the copy function of the line. If a line contains CoordNodes, than also the copy should contain CoordNodes. Can you point me to the code, where the lines gets duplicated?
Whenever a routable way gets split to limit the number of points or nodes it makes a new list of points so any changes to the points in the new list will not be visible in the original list.
Yes, its true that changes in the copy will not be visible in the original. But this is not my point. My point is: If the original line contains CoordNodes (instead of Nodes) then the copy should also contain CoordNodes. I think, this will complicate the split/copy functions in some ways, so maybe your solution is the easier way.
I have now looked into the code and must say, I don't understand why it works. Previous for each node was created a CoordNode. Now instead a flag is set. I see the difference: The flag is copied, the class obviously not. But I didn't find the place, where this copy occurs. Could you point my nose to the file and line number, where the line is duplicated? The only place I found is in the round coord filter class (line 55), where it is handled correctly. So why your patch works as expected and the original solution not?? I have another idea of solving this problem. Instead of do a new Coord or CoordNode call coord.copy(). The function could be overwritten in the CoordNode class to return a CoordNode object. (BTW. This should be done in the CoordPOI class too). So a copied line should contain afterwards the same types as the original.
Hi Johann,
I have now looked into the code and must say, I don't understand why it works. Previous for each node was created a CoordNode. Now instead a flag is set. I see the difference: The flag is copied, the class obviously not. But I didn't find the place, where this copy occurs. Could you point my nose to the file and line number, where the line is duplicated? The only place I found is in the round coord filter class (line 55), where it is handled correctly. So why your patch works as expected and the original solution not??
As I said in a previous posting on this topic: mb> Whenever a routable way gets split to limit the number of points or mb> nodes it makes a new list of points so any changes to the points in the mb> new list will not be visible in the original list. So, wherever you see splitWay() that's where the "copying" occurs.
I have another idea of solving this problem. Instead of do a new Coord or CoordNode call coord.copy(). The function could be overwritten in the CoordNode class to return a CoordNode object. (BTW. This should be done in the CoordPOI class too). So a copied line should contain afterwards the same types as the original.
Doesn't the suggested patch do the job? (Felix, you reported it wasn't working, is that still the case?) Actually, I'm not keen on the CoordNode thing at all but it works well enough to not warrant spending effort on it while there is so much other stuff to be fixed. Cheers, Mark
As I said in a previous posting on this topic:
mb> Whenever a routable way gets split to limit the number of points or mb> nodes it makes a new list of points so any changes to the points in the mb> new list will not be visible in the original list.
So, wherever you see splitWay() that's where the "copying" occurs.
Maybe it is too late in the evening for me to understand the problem. In the splitWay() the line is splitted. Yes, a new list<Coord> is generated. But the points inside it are moved from one list to another. There is no new Coord(). So a CoordNode stays a CoordNode.
Johann, On Thu, 26 Nov 2009 23:53:58 +0100 Johann Gail <johann.gail@gmx.de> wrote:
As I said in a previous posting on this topic:
mb> Whenever a routable way gets split to limit the number of points or mb> nodes it makes a new list of points so any changes to the points in the mb> new list will not be visible in the original list.
So, wherever you see splitWay() that's where the "copying" occurs.
Maybe it is too late in the evening for me to understand the problem. In the splitWay() the line is splitted. Yes, a new list<Coord> is generated. But the points inside it are moved from one list to another. There is no new Coord(). So a CoordNode stays a CoordNode.
Yes, it would stay a CoordNode if it was a CoordNode at the time the line was split. But, Coord->CoordNode transformation happens after all the splitting has occurred. So at the time the lines are split, all Coords are Coords! Cheers, Mark
Mark Burton wrote:
Hi Johann,
I have now looked into the code and must say, I don't understand why it works. Previous for each node was created a CoordNode. Now instead a flag is set. I see the difference: The flag is copied, the class obviously not. But I didn't find the place, where this copy occurs. Could you point my nose to the file and line number, where the line is duplicated? The only place I found is in the round coord filter class (line 55), where it is handled correctly. So why your patch works as expected and the original solution not??
As I said in a previous posting on this topic:
mb> Whenever a routable way gets split to limit the number of points or mb> nodes it makes a new list of points so any changes to the points in the mb> new list will not be visible in the original list.
So, wherever you see splitWay() that's where the "copying" occurs.
I have another idea of solving this problem. Instead of do a new Coord or CoordNode call coord.copy(). The function could be overwritten in the CoordNode class to return a CoordNode object. (BTW. This should be done in the CoordPOI class too). So a copied line should contain afterwards the same types as the original.
Doesn't the suggested patch do the job? (Felix, you reported it wasn't working, is that still the case?)
Well since introducing the round-cords it is not working for me anymore. nothing changed here.
Actually, I'm not keen on the CoordNode thing at all but it works well enough to not warrant spending effort on it while there is so much other stuff to be fixed.
Cheers,
Mark _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
participants (6)
-
Chris Miller -
Clinton Gladstone -
Felix Hartmann -
Johann Gail -
Mark Burton -
Marko Mäkelä