El 03/01/13 12:41, Minko escribió:

Routing works fine on my Openfietsmap: http://osm.pleiades.uni-wuppertal.de/garmin/openfietsmap_lite/29-12-2012/16b...

I use remove-short-arcs, Map created with mkgmap-r2423 No issues with older versions either. I also use --remove-short-arcs in my scripts for the mapas.alternativaslibres.es maps and latest mkgmap.

I now tried all options that I could think of. with or without remove-short-arcs, and with or without --adjust-turn-headings. All fails so far. Here are my default option: start /low /b /wait java -jar -Xms4000M -Xmx6600M mkgmap.jar --max-jobs=4 --generate-sea --latin1 --nsis --index --transparent --adjust-turn-headings --add-pois-to-areas --ignore-maxspeeds --reduce-point-density=3 --x-reduce-point-density-polygon=6 --link-pois-to-ways --precomp-sea=c:\openmtbmap\maps\sea --ignore-turn-restrictions --min-size-polygon=14 --description=openmtbmap_es --show-profiles=0 --merge-lines --location-autofill=bounds,is_in,nearest --bounds=c:\openmtbmap\maps\bounds --route --country-abbr=es --country-name=spain --mapname=64130000 --family-id=6413 --product-id=1 --series-name=openmtbmap_spain_03.01.2013 --family-name=mtbmap_es_03.01.2013 --tdbfile --overview-mapname=mapsetx --keep-going --area-name="spain_03.01.2013_openmtbmap.org" -c c:\openmtbmap\maps\template.spain 1>NUL The bug isn't new, I found that even maps from May 2012 had the exact same problem at that road. On 03.01.2013 13:16, Chris66 wrote:

Hi, is --adjust-turn-headings used ?

Chris

_______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://lists.mkgmap.org.uk/mailman/listinfo/mkgmap-dev

-- keep on biking and discovering new trails Felix openmtbmap.org & www.velomap.org

usually two routable lines of top of each other don't matter at all. There can be up to 6 routable lines on top of each other without problems. Only if there are 7 routable lines on top of each other or more, there are problems. On 03.01.2013 14:15, Minko wrote:

I can reproduce the problem if I use the default line style file instead of my own line style file so it is a style related issue.

I think the problem is that a tertiary highway http://www.openstreetmap.org/browse/way/114405617 forms part of the square which is tagged as residential highway & area=yes. It is drawn first as tertiary highway and also as residential road, so two routable lines on top of each other.

The issue is gone when you use continue in the lines style: highway=tertiary [0x05 road_class=1 road_speed=3 resolution 20 continue]

I dont know exactly what the difference is but it solves the routing issue.

Felix wrote:

...

The bug isn't new, I found that even maps from May 2012 had the exact same problem at that road.

-- keep on biking and discovering new trails Felix openmtbmap.org & www.velomap.org

I agree with Chris that it is related with the MP, it solves the issue if you use continue. I dont recommend to use it here, because it draws also a 0x07 line on top: # Mop up any unrecognised highway types highway=* & highway!=proposed & area!=yes [0x07] The issue also disappears when you use highway=residential & area!=yes [0x06 road_class=0 road_speed=2 resolution 22] instead of the default rule highway=residential [0x06 road_class=0 road_speed=2 resolution 22] So it got to do with those two routable lines on top of each other

Note that the square is tagged as MP.

So maybe the MP processing is doing some strange things here.

Chris

Hi

because in general creating several routable lines on top of each other is perfectly okay. Are they maybe not correctly connected?

The 'continue' is making *more* lines on top of each other, its not preventing lines being on top of each other. I do not see the routing failure with my small extract of the area or version of mapsource. However I do see one strange difference in the NOD file with and without the 'continue' Does the attached patch make any difference? ..Steve

On 03/01/13 15:05, Felix Hartmann wrote:

Nope, the patch doesn't help here (at least for my style)....

Oh well. I will see if I can reproduce, I get the problem with Lambertus's tile on a newer version of mapsource, but not with my small extract, so downloading and compiling complete country map now. ..Steve

On 03/01/13 15:24, Minko wrote:

Steve, you could also try it with this tile: http://mijndev.openstreetmap.nl/~ligfietser/test/33340072.osm.pbf

Thanks, but that tile works with and without the patch for me. Don't know what that means?! ..Steve

yip same for me. Without the patch, it didn't work in either Mapsource 6.16.3, 6.13.6 and newest Basecamp. On 03.01.2013 17:33, Minko wrote:

Thats strange, I can reproduce Felix' issue with the default style with that tile. It is splitted from the europe.osm.pbf extract from 22/12.

Steve wrote:

...

Thanks, but that tile works with and without the patch for me. Don't know what that means?!

---

mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://lists.mkgmap.org.uk/mailman/listinfo/mkgmap-dev

-- keep on biking and discovering new trails Felix openmtbmap.org & www.velomap.org

On 03/01/13 15:05, Felix Hartmann wrote:

Nope, the patch doesn't help here (at least for my style)....

Attached patch fixes it for me. Its more of a work-around or problem avoidance than a fix. It does not add roads that are not attached to anything else. Might be quite reasonable for roads, but from Gerd's post earlier this week, we can't be sure that only roads are being processed.

On 03/01/13 15:40, Felix Hartmann wrote:

what does that mean exactly?

It will not add a road, until it is connected to at least a second road or polygon? Or it will not add roads from multipolygons?

All it means is that at a certain point in the code there is a fragment of road that has no points that it thinks are connected to anything. I'll have to find out which bit of road is affected before I can add anything more. ..Steve

Actually, the patch removes the error, but the outcome is wrong. It drops the tertiary street, instead of dropping the residential street. I still think that as I wrote back on WanMill's email, the multipolygon highway application is per se not really correct. On 03.01.2013 16:35, Steve Ratcliffe wrote:

On 03/01/13 15:05, Felix Hartmann wrote:

...

Nope, the patch doesn't help here (at least for my style)....

Attached patch fixes it for me.

Its more of a work-around or problem avoidance than a fix. It does not add roads that are not attached to anything else. Might be quite reasonable for roads, but from Gerd's post earlier this week, we can't be sure that only roads are being processed.

-- keep on biking and discovering new trails Felix openmtbmap.org & www.velomap.org

Felix wrote:

But that change causes problems for other parts. There are some highway=residential areas without ways that cross them, so in order to have routing work in some inner city cases, you must make sure that you route along the edges.

Yes, I'm aware of this, I only pointed this option to check where the problem lies. This wouldnt be a good solution.

If multipolygons were treated the same way as relations, that would make it easier to make sure only one routable highway gets created (because add highway onto highway would not add the highway, because there is already an underlying highway).

The only thing I could think that does cause problems, if two routable lines are on top of each other, and both have the same road_type and same road_speed. Maybe there could be a check in the mkgmap continue function, never to create two routable lines on top of each other, if both road_speed and road_type are identical.

Yes thats the case here I think, there are two lines on top of each other with different road_classes and road_speeds: 0x05 road_class=1 road_speed=3 0x06 road_class=0 road_speed=2

Ok, so we can exclude the multiple lines on top of each other. If I use the continue statement it draws the same lines as without continue, so that confirms it too. But why does this 'continue' solves the issue? If I exclude the following rule, # highway=* & highway!=proposed & area!=yes [0x07] it looks like it draws exactly the same highways, except that the routing doesnt break anymore.

Well in my map, they are indeed the same, and the problem exists too. For my map it's

0x05 road_class=1 road_speed=1 0x06 road_class=1 road_speed=1

and it breaks too. I think having identical routable lines on top of each other, is more likely to cause error, than different road_class and road_speed.

(P.S I'm just trying out Steves sr.patch from above - but map still rendering)

But I still don't get it fully. Is the MP creating a new line, or adding the tags? Because now there would be two conflicting highway keys... I think it would be good to have a behaviour to only add keys to the underlying line, but not overwrite or not double if its a line... because I don't think I can add a check like: highway=* & highway=residential & mkgmap:mp_created=true {delete highway=residential} (without deleting both highway keys...). Are there any cases, where we really need highways from multipolygon in case highway already exists on the line itself? On 03.01.2013 20:17, WanMil wrote:

Maybe it helps if I explain what happens in detail with the mp:

1. The mp calculates the tags for the mp. In this case the tags of the mp are used which are [area=yes; highway=residential] 2. These mp tags are removed from all outer ways if they are identical. In this case no tag is removed from way 114405617. 3. All outer ways are copied and tagged with the mp tags. These copied ways are only processed by the line style file. They can be identified by the additional tag mkgmap:mp_created=true. 4. All outer polygons are tagged with the mp tags and are processed by the polygon style file only. They can also be identified by the additional tag mkgmap:mp_created=true.

Point 2 and 3 are required because in OSM the mp *and* the outer ways are tagged quite often.

In this case it creates a residential way (point 3) with identical coords like the tertiary way 114405617.

WanMil

...

But that change causes problems for other parts. There are some highway=residential areas without ways that cross them, so in order to have routing work in some inner city cases, you must make sure that you route along the edges.

If multipolygons were treated the same way as relations, that would make it easier to make sure only one routable highway gets created (because add highway onto highway would not add the highway, because there is already an underlying highway).

The only thing I could think that does cause problems, if two routable lines are on top of each other, and both have the same road_type and same road_speed. Maybe there could be a check in the mkgmap continue function, never to create two routable lines on top of each other, if both road_speed and road_type are identical. On 03.01.2013 14:44, Minko wrote:

...

The issue also disappears when you use highway=residential & area!=yes [0x06 road_class=0 road_speed=2 resolution 22] instead of the default rule highway=residential [0x06 road_class=0 road_speed=2 resolution 22]

So it got to do with those two routable lines on top of each other

-- keep on biking and discovering new trails Felix openmtbmap.org & www.velomap.org

Okay, now I get it. I think it would be better to have some sorts of multipolygon file similar to the relations file in the style though. At least for everything that's a highway, it should be added to the original way. So here is what I would expect: Output: way 114405617 [highway=tertiary; ref=GI-681; mp_created_highway=residential; area=yes] way 81232137 [area=yes; highway=residential; mkgmap:mp_created=true; mkgmap:stylefilter=polyline] relation 1596434 [area=yes; highway=residential; type=multipolygon] way P1596434 [area=yes; highway=residential; mkgmap:mp_created=true; mkgmap:stylefilter=polygon] I'm not so sure about area=yes though if it should be added as mp_created_area=yes instead. However if it had a name, but way 114405617 would not have a name=XYZ, then it should be name I suppose and not mp_created_name=XYZ This special multipolygon treatment, would only need to be done for MP with highway=*, the rest could stay as it is right now. The outcome with the routing_error.patch is currently (with # = missing). # way 114405617 [highway=tertiary; ref=GI-681] way 81232137 [no tags] way C114405617 [area=yes; highway=residential; mkgmap:mp_created=true; mkgmap:stylefilter=polyline] way C81232137 [area=yes; highway=residential; mkgmap:mp_created=true; mkgmap:stylefilter=polyline] way P1596434 [area=yes; highway=residential; mkgmap:mp_created=true; mkgmap:stylefilter=polygon] so that's not very good either. On 03.01.2013 20:56, WanMil wrote:

Yes it's confusing. So let's track it down.

Input: way 114405617 [highway=tertiary; ref=GI-681] way 81232137 [no tags] relation 1596434 [area=yes; highway=residential; type=multipolygon]

Output of the mp algorithm: way 114405617 [highway=tertiary; ref=GI-681] way 81232137 [no tags] way C114405617 [area=yes; highway=residential; mkgmap:mp_created=true; mkgmap:stylefilter=polyline] way C81232137 [area=yes; highway=residential; mkgmap:mp_created=true; mkgmap:stylefilter=polyline] way P1596434 [area=yes; highway=residential; mkgmap:mp_created=true; mkgmap:stylefilter=polygon]

C114405617 is a copy of 114405617 P1596434 is the resulting polygon of the mp.

Is it now more clear?

WanMil

...

But I still don't get it fully. Is the MP creating a new line, or adding the tags?

Because now there would be two conflicting highway keys...

I think it would be good to have a behaviour to only add keys to the underlying line, but not overwrite or not double if its a line... because I don't think I can add a check like: highway=* & highway=residential & mkgmap:mp_created=true {delete highway=residential}

(without deleting both highway keys...).

Are there any cases, where we really need highways from multipolygon in case highway already exists on the line itself? On 03.01.2013 20:17, WanMil wrote:

...

Maybe it helps if I explain what happens in detail with the mp:

1. The mp calculates the tags for the mp. In this case the tags of the mp are used which are [area=yes; highway=residential] 2. These mp tags are removed from all outer ways if they are identical. In this case no tag is removed from way 114405617. 3. All outer ways are copied and tagged with the mp tags. These copied ways are only processed by the line style file. They can be identified by the additional tag mkgmap:mp_created=true. 4. All outer polygons are tagged with the mp tags and are processed by the polygon style file only. They can also be identified by the additional tag mkgmap:mp_created=true.

Point 2 and 3 are required because in OSM the mp *and* the outer ways are tagged quite often.

In this case it creates a residential way (point 3) with identical coords like the tertiary way 114405617.

WanMil

...

But that change causes problems for other parts. There are some highway=residential areas without ways that cross them, so in order to have routing work in some inner city cases, you must make sure that you route along the edges.

If multipolygons were treated the same way as relations, that would make it easier to make sure only one routable highway gets created (because add highway onto highway would not add the highway, because there is already an underlying highway).

The only thing I could think that does cause problems, if two routable lines are on top of each other, and both have the same road_type and same road_speed. Maybe there could be a check in the mkgmap continue function, never to create two routable lines on top of each other, if both road_speed and road_type are identical. On 03.01.2013 14:44, Minko wrote:

...

The issue also disappears when you use highway=residential & area!=yes [0x06 road_class=0 road_speed=2 resolution 22] instead of the default rule highway=residential [0x06 road_class=0 road_speed=2 resolution 22]

So it got to do with those two routable lines on top of each other

-- keep on biking and discovering new trails Felix openmtbmap.org & www.velomap.org

I've now tracked down the problem. The tertiary way 114405617 and its copy C114405617 share the list of Coord objects. Not just the actual Coord's (which is correct) but the ArrayList too. Code in addRoadWithoutLoops() replaces the nodes that have a highwayCount greater than 1 with CoordNode objects. If the tertiary way is processed first then it will end up properly routable. When the copy is processed however, the nodes have already been replaced and have no highwayCount, so it appears to have no connection to other roads to the code. So you end up with a road without the correct routing information which happens to crash MapSource.

The previous workaround removed the piece of road that ended up without routing information. Sometimes that would be the MP copy and sometimes the original road - it just depends on which is done last.

Two solutions I can think of 1. Always ensure you have a (shallow) copy of the list. 2. Fix up the addRoadsWithoutLoops() to recognise that a way has already been processed and do everything that would have been done anyway.

I favour 1) hoping that there is no intentional sharing of lists anywhere.

Attached is a patch that copies the list in the Way constructor. I'll check all call sites to make sure there is no duplicate copying later. This patch is instead of the previous workaround.

Steve, I could also create a copy of the points list in the mp algorithm (I would call it a bug that it hasn't been done :-). But having a look at the signature of the constructor I would expect that it takes a copy of the points list. On the other side copying the points list will create some temporary lists but I think that's ok. I have removed three copies of the points list that are unneccessary now. See attached patch Routing_error3.patch.

This kind of tagging that produces the problem is particularly common in Spain, in the UK I couldn't find any examples and in Germany there are only a few; perhaps one per tile. In fact the only German example I tried to trace had been changed since I downloaded it and was no longer a problem. Every Spanish example I looked at was a sure routing failure however.

..Steve

What do you think of Felix proposal to add a special handling for highways in multipolygons? I doesn't feel good to me but I could add it if required. WanMil

I just commited it because the error was too obvious for me... Do you want to have the printout in the StyledConverter when a road has zero nodes? I didn't commit it because it was not part of the problem and I wasn't sure if that's only a kind of debugging. WanMil

Hi WanMil

...

I have removed three copies of the points list that are unneccessary now. See attached patch Routing_error3.patch.

Thanks, that is great

..Steve _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://lists.mkgmap.org.uk/mailman/listinfo/mkgmap-dev

Hi WanMil,

I just commited it because the error was too obvious for me...

Do you want to have the printout in the StyledConverter when a road has zero nodes? I didn't commit it because it was not part of the problem and I wasn't sure if that's only a kind of debugging.

In the boundary routimes we often use code like this: Way w = new Way(0, coveredPart); String attr = w.clockwise() ? "o" : "i"; In the past, this did not create a copy of the points, now it does. Will that have an impact on performance? If yes, maybe it would be better to place a static public boolan clockwise(List<Coord>) method into e.g. Java2DConverter ? Gerd

No matter if there is a performance degration it would be good to have a method clockwise(List<Coord>) because creating a Way object to check that is also a workaround. I think Java2DConverter is not the best class for that because there is no direct relation between the clockwise test and the conversion to java.awt.* objects. I would prefer either the Utils class, the Way class or a new OsmUtils class.

Reg. performance: I did not measure it. In most places that I've changed the clockwise() method was just used to determine a file name, so I/O performance would be the bottleneck. I think the Way class is a good place for it, so I've changed that. There is now only the PrecompSeaMerger which might show a performance degration, but I don't expect measurable changes. The area.intersect() methods are copying the ArrayLists much more often, so one more copy action should not cause problems. Gerd

WanMil wrote

Maybe it was not very clear what I meant. The performance degration might not be introduced by changing the clockwise method but by always creating a new list when instantiating a new Way object, e.g. the Java2DConverter creates list of points that are often directly forwarded to a new Way object. This list of points is now created twice (once by the Java2DConverter and once by the Way object). This is often used in the multipolygon code and maybe somewhere else.

Yes, that was understood. The multipolygon code uses the awt.geo.area methods, and these are copying the points very often. That's why I don't think that one additional copy matters. Gerd -- View this message in context: http://gis.19327.n5.nabble.com/Short-Arc-Problem-Error-3-Mapsource-Problem-o... Sent from the Mkgmap Development mailing list archive at Nabble.com.

Am 03.01.2013 14:07, schrieb Felix Hartmann:

--merge-lines

Is also a candidate for producing of routing errors. Chris