oneway=reverse handling broken.
I noticed that during the last updates oneway=reverse is behaving very very strange. If I have the following rules: highway=* & oneway=reverse & incline=up & mtb:scale:uphill>4 {set oneway=-1} [0x01] highway=* & incline=up & mtb:scale:uphill>4 {set oneway=yes} [0x02] Strangely way is output as 0x02. This has come since the latest additions to the style-file rules. Before it was working very well. Anyone using rules that do set oneway artificially you will have to update your styles. The old way how it worked is now completly broken. I really have problems to understand how "continue" is now interpreted. I personnaly liked the old ("broken") way better, but except for oneway=reverse I think I have been able to adapt (I had to add many many key!=value to stop mkgmap continuing where before it did not continue). The problem is only happening on oneway=reverse only, oneway=-1 is working like it is supposed to do! Maybe oneway=reverse got broken earlier already but I missed it. I just had many many roads being put into the wrong direction since the last changes to the style-mechanism and then tried to accomplish the same result using new rules.
Hi
And once again I plead for a resurrection of the style branch, so that we can clean-up the style handling of mkgmap.
OK I shall start on it very soon. I'm sure it will conflict a lot with the 'continue' patch that was added to the trunk, so that will have to be sorted out. ..Steve
On 21.01.2010 18:26, Steve Ratcliffe wrote:
Hi
And once again I plead for a resurrection of the style branch, so that we can clean-up the style handling of mkgmap.
OK I shall start on it very soon. I'm sure it will conflict a lot with the 'continue' patch that was added to the trunk, so that will have to be sorted out.
..Steve
Wouldn't it be enough for everyone to create rules like condition blablabla [continue with_actions] Not that no 0x* is included here. This would be simple and effective (and I can't think of any reason why we would need the style branch then).
_______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
On 21/01/10 17:28, Felix Hartmann wrote:
Not that no 0x* is included here. This would be simple and effective (and I can't think of any reason why we would need the style branch then).
We need the style branch because the trunk just doesn't work in a consistent and predictable way. I know that by adding lots of extra rules you can get almost anything to work, but you shouldn't have to do that. Perhaps the patch that was applied fixed some things and I will investigate fully before doing anything. I could never understand exactly what problems you had with the style branch, but I am sure that they can be easily overcome once I have good examples to work with. ..Steve
On 21.01.2010 18:55, Steve Ratcliffe wrote:
On 21/01/10 17:28, Felix Hartmann wrote:
Not that no 0x* is included here. This would be simple and effective (and I can't think of any reason why we would need the style branch then).
We need the style branch because the trunk just doesn't work in a consistent and predictable way.
I wouldn't see (except the strange handling of oneway=reverse - but maybe there is some error on my side) any strange behaviour with the trunk currently. There would only be the need for [continue_withactions] without direct output.
I know that by adding lots of extra rules you can get almost anything to work, but you shouldn't have to do that. Perhaps the patch that was applied fixed some things and I will investigate fully before doing anything.
I could never understand exactly what problems you had with the style branch, but I am sure that they can be easily overcome once I have good examples to work with.
Well continue never worked in any way predictable and my maps were output with half of the lines missing, routing broken,.....
..Steve _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Felix Hartmann schrieb am 21.01.2010 19:07:
I wouldn't see (except the strange handling of oneway=reverse - but maybe there is some error on my side) any strange behaviour with the trunk currently.
We had quite some topics lately, e.g. that you can not use the same expression twice, or that two independent style rules only worked, when they were arranged in a specific order. Basically I think, that you have fine-tuned your style to the current style handling of the trunk. If there was a problem in the style handling, it was not fixed but you found a way around it.
Well continue never worked in any way predictable and my maps were output with half of the lines missing, routing broken,.....
I guess, this was caused by some of your workarounds, which were based on "errors" in the trunk's style interpretation. In my experience the style brunch provides every capability of the continue patch, but some of the rules must be rephrased (actually they must be corrected) so that they would work as before. Gruss Torsten
On 21.01.2010 21:54, Torsten Leistikow wrote:
Felix Hartmann schrieb am 21.01.2010 19:07:
I wouldn't see (except the strange handling of oneway=reverse - but maybe there is some error on my side) any strange behaviour with the trunk currently.
We had quite some topics lately, e.g. that you can not use the same expression twice, or that two independent style rules only worked, when they were arranged in a specific order.
That is solved (I spent nearly 10 hours to adapt my style-files that you continue now works endless)
Basically I think, that you have fine-tuned your style to the current style handling of the trunk. If there was a problem in the style handling, it was not fixed but you found a way around it.
Well continue never worked in any way predictable and my maps were output with half of the lines missing, routing broken,.....
I guess, this was caused by some of your workarounds, which were based on "errors" in the trunk's style interpretation. In my experience the style brunch provides every capability of the continue patch, but some of the rules must be rephrased (actually they must be corrected) so that they would work as before.
There is besides one bug, I think no more bug inside. The oneway=reverse problem I'm having seems to be related that the street is first set oneway=1 (general open rule), then it is set to oneway=-1 using [continue_withactions], then I want to set it to oneway=1 using plain [continue], and then for the final output it is used again as oneway=1. However maybe I have a typo somewhere when I changed all my rules to adapt to the current behaviour. previously (which was a bug, but I found it quite useful) - on the same highway=primary keya=123 & highway=primary [continue] output keya=123 & highway=* [continue] OLD no output; NEW output keya=123 [final] output This ment you could write keya=123 as many times as you liked, but it was only output 2 times. Now to achieve the same situation as before one has to keya=123 & highway=primary[continue] output keya=123 & higwhay=* & highway!=primary [continue] no output keya=123 [final] output So you have to build a quite an extensive !=abc list to not output the same lines several times. This now needs a lot of code if you for example want to have 4 different designs for bridges, depending on the width of the way it goes with. On the other hand, multilayered maps are now much easier: highway=motorway [resolution 16-18 0x10100 continue] highway=motorway [resolution 20-22 0x10101 continue] highway=motorway [resolution 24 road_speed=1 road_class=4] is working as intended, without additional code. It's okay, took 10 hours to change all my code so that it fits now with the repaired behaviour (as I use 0x01 for many many roads for routing I already have a lot doubles and it is not easy to keep track where a line is doubled and where not, but the code change of course gave me triples and quadruples of the same way where I only wanted doubles). My only problem is that oneway=reverse handling, I'm not sure if theirs a problem with my style, or if there is some special behaviour related to oneway=reverse that does not occur with oneway=-1 (as in my style I have to change the direction of a way 2-3 times to get the result I want with opposing oneways but also arrows pointing in the direction of traffic).
Gruss Torsten _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Felix Hartmann schrieb am 21.01.2010 22:14:
keya=123 & highway=primary[continue] output keya=123 & higwhay=* & highway!=primary [continue] no output keya=123 [final] output
So you have to build a quite an extensive !=abc list to not output the same lines several times. This now needs a lot of code if you for example want to have 4 different designs for bridges, depending on the width of the way it goes with.
I use auxiliary tags for such tasks: keya=123 & highway= primary {set keya_done=true} [continue] output keya=123 & highway=* & keya_done!=true [continue] no output keya=123 [final] output So you do have only one !=abc tag for each key, if you want to suppress the output. I know one related bug in the style branch, the following does not work (but it also does not work reliable in the trunk): highway=primary & keya=123 {set highway=deleted} [continue] highway=primary [final] Both rules will result in an output. My understanding is, that you can not change the first (or perhaps the main) expression of a rule in the action part. I think it is related to the rule matching done via a hash table, but I am not really familiar with the source, so this is just an educated guess. Gruss Torsten
On 22.01.2010 16:20, Torsten Leistikow wrote:
Felix Hartmann schrieb am 21.01.2010 22:14:
keya=123& highway=primary[continue] output keya=123& higwhay=*& highway!=primary [continue] no output keya=123 [final] output
So you have to build a quite an extensive !=abc list to not output the same lines several times. This now needs a lot of code if you for example want to have 4 different designs for bridges, depending on the width of the way it goes with.
I use auxiliary tags for such tasks:
keya=123& highway= primary {set keya_done=true} [continue] output keya=123& highway=*& keya_done!=true [continue] no output keya=123 [final] output
So you do have only one !=abc tag for each key, if you want to suppress the output.
Well until 1497 auxiliary tags worked, now they are broken. I tried it too. At least if you provide a list of say 15 auxiliary keys. Maybe only one or two are parsed??? I can only say that no matter what I do as soon as oneway=* is involved, it is completly fucked up right now. There is no consistency at all. Before this was working.
I know one related bug in the style branch, the following does not work (but it also does not work reliable in the trunk):
highway=primary& keya=123 {set highway=deleted} [continue] highway=primary [final]
Both rules will result in an output. My understanding is, that you can not change the first (or perhaps the main) expression of a rule in the action part. I think it is related to the rule matching done via a hash table, but I am not really familiar with the source, so this is just an educated guess.
Gruss Torsten _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Felix Hartmann schrieb am 22.01.2010 16:24:
Well until 1497 auxiliary tags worked, now they are broken. I tried it too. At least if you provide a list of say 15 auxiliary keys. Maybe only one or two are parsed???
I can't really comment on the latest changes, since my main styles are depending on the correct execution of the action rules, which is only done by the style branch. My styles are much more simple than yours, but nevertheless I will never build such complex. Instead of checking ( highway=primary | highway=secondary | highway=tertiary | highway=minor | highway=unclassified ) & ... in a single rule, I use something like the following: highway=primary {set road_usable=yes} highway=secondary {set road_usable=yes} highway=tertiary {set road_usable=yes} highway=minor {set road_usable=yes} highway=unclassified {set road_usable=yes} highway=* & road_usable!=* {set road_usable=no} Now in the following rules I just have to check highway=* & road_usable=yes & ... Based on my experiance this gives much more maintainable style rules. Because if I have to change the conditions for the auxiliary tag, e.g. I have to include another road type, then I just have to change the rules where the road_usable flag is set and all concerned output rules can stay unchanged. Gruss Torsten
On 22.01.2010 16:46, Torsten Leistikow wrote:
Felix Hartmann schrieb am 22.01.2010 16:24:
Well until 1497 auxiliary tags worked, now they are broken. I tried it too. At least if you provide a list of say 15 auxiliary keys. Maybe only one or two are parsed???
I can't really comment on the latest changes, since my main styles are depending on the correct execution of the action rules, which is only done by the style branch.
My styles are much more simple than yours, but nevertheless I will never build such complex. Instead of checking
( highway=primary | highway=secondary | highway=tertiary | highway=minor | highway=unclassified )& ...
in a single rule, I use something like the following:
highway=primary {set road_usable=yes} highway=secondary {set road_usable=yes} highway=tertiary {set road_usable=yes} highway=minor {set road_usable=yes} highway=unclassified {set road_usable=yes} highway=*& road_usable!=* {set road_usable=no}
Now in the following rules I just have to check
highway=*& road_usable=yes& ...
Based on my experiance this gives much more maintainable style rules. Because if I have to change the conditions for the auxiliary tag, e.g. I have to include another road type, then I just have to change the rules where the road_usable flag is set and all concerned output rules can stay unchanged.
Well it was working 100% with 1497. I need to check for all those streets because I have 9 different oneway styles, 8 different bridges, 8 different tunnels, ...... Also many roads have only invisible routing line ..... The biggest problem is that after changing around, I thought it all works well and did not do any backups of how my style-file worked before until realising the mess. 1498 and upwards if !=* is not working 100% correct, is simply unusable. This was a real degradation. Now I will need to spend hours trying to get a working style-file for 1497 again if soon >1500 is not getting correct behaviour again. 1505 is not doing things in order anymore. If I add a line further back in the style, it trashes stuff further to the front. It was never possible before to damage lines that were used with [continue] or [continue with_actions] with the old way of working. Now you never know if you add a line that it does crash your lines in front. Just like the conditional rules are working. I also heavily depend on the conditional rules, and can live pretty well with the current behaviour. It just really makes long long lines as you have to check for many many combinations. But now it really means, that a lines file of some thousand lines only (1.2MB) becomes unmaintainable because you never know what happens when you add some lines. It's like a lottery.
Gruss Torsten _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Hello Felix,
The biggest problem is that after changing around, I thought it all works well and did not do any backups of how my style-file worked before until realising the mess. 1498 and upwards if !=* is not working 100% correct, is simply unusable. This was a real degradation. Now I will need to spend hours trying to get a working style-file for 1497 again if soon >1500 is not getting correct behaviour again.
I can't believe you don't have your style files under version control. It's so easy with any of the "modern" SCM programs (e.g. bazaar, hg, git) - you don't have to make a separate repo like you do with old-fashioned SCM systems (cvs, svn). Just go to the directory/folder that contains the files you want to put under version control and say: hg/bazaar/git init Then you can start committing changes. It's a no-brainer. Mark
On 22.01.2010 17:19, Mark Burton wrote:
Hello Felix,
The biggest problem is that after changing around, I thought it all works well and did not do any backups of how my style-file worked before until realising the mess. 1498 and upwards if !=* is not working 100% correct, is simply unusable. This was a real degradation. Now I will need to spend hours trying to get a working style-file for 1497 again if soon>1500 is not getting correct behaviour again.
I can't believe you don't have your style files under version control.
It's so easy with any of the "modern" SCM programs (e.g. bazaar, hg, git) - you don't have to make a separate repo like you do with old-fashioned SCM systems (cvs, svn). Just go to the directory/folder that contains the files you want to put under version control and say:
hg/bazaar/git init
Then you can start committing changes. It's a no-brainer.
Mark
Yeah, I should have done so. I was allways to lazy to set it up. I got both Bazar and TortoiseSVN setup anyhow, to apply patches. But never used it for local files. Till now I kept good track myself of the changes and did once a week backups. Just with the latest changes I thought it will be an easy change until I had thrown all around and noticed that since the mp merge my styles simply output rubbish.
_______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
On 22.01.2010 16:20, Torsten Leistikow wrote:
Felix Hartmann schrieb am 21.01.2010 22:14:
keya=123& highway=primary[continue] output keya=123& higwhay=*& highway!=primary [continue] no output keya=123 [final] output
So you have to build a quite an extensive !=abc list to not output the same lines several times. This now needs a lot of code if you for example want to have 4 different designs for bridges, depending on the width of the way it goes with.
I use auxiliary tags for such tasks:
keya=123& highway= primary {set keya_done=true} [continue] output keya=123& highway=*& keya_done!=true [continue] no output keya=123 [final] output
So you do have only one !=abc tag for each key, if you want to suppress the output.
I know one related bug in the style branch, the following does not work (but it also does not work reliable in the trunk):
highway=primary& keya=123 {set highway=deleted} [continue] highway=primary [final]
Both rules will result in an output. My understanding is, that you can not change the first (or perhaps the main) expression of a rule in the action part. I think it is related to the rule matching done via a hash table, but I am not really familiar with the source, so this is just an educated guess.
See below for an example that does not work. It works with 1497, but 1498 breaks it. highway=* & copy=99 & mtb:scale:uphill!=5 & mtb:scale:uphill!=4 { set dontadd=yes; set taxi=no; set dontadd=oneway; set mkgmap:unpaved=1 } [0x10711 resolution 21 continue with_actions] *output* highway=* & copy=98 & mtb:scale:uphill!=5 & mtb:scale:uphill!=4 { set dontadd=yes; set taxi=no; set dontadd=oneway; set mkgmap:unpaved=1 } [0x10713 resolution 21 continue with_actions] *output* highway=* & copy=99 & mtb:scale:uphill=4 { set dontadd=yes; set taxi=no; set dontadd=oneway; set mkgmap:unpaved=1 } [0x10711 resolution 21 continue with_actions] *output* highway=* & copy=98 & mtb:scale:uphill=4 { set dontadd=yes; set taxi=no; set dontadd=oneway; set mkgmap:unpaved=1 } [0x10713 resolution 21 continue with_actions] *output* highway=* & copy=99 & mtb:scale:uphill=5 { set dontadd=yes; set taxi=no; set dontadd=oneway; set mkgmap:unpaved=1 } [0x10710 resolution 21 continue with_actions] *output* highway=* & copy=98 & mtb:scale:uphill=5 { set dontadd=yes; set taxi=no; set dontadd=oneway; set mkgmap:unpaved=1 } [0x10712 resolution 21 continue with_actions] *output* highway=* & dontadd=oneway & copy=99 & mtb:scale:uphill=4 { set oneway=-1; set taxi=no; set mkgmap:unpaved=1 } [0x01 road_class=0 road_speed=0 resolution 21 continue] *output* highway=* & dontadd=oneway & copy=98 & mtb:scale:uphill=4 { set oneway=yes; set taxi=no; set mkgmap:unpaved=1 } [0x01 road_class=0 road_speed=0 resolution 21 continue] *output* highway=* & dontadd=oneway & copy=99 & mtb:scale:uphill=5 { set oneway=-1; set taxi=no; set mkgmap:unpaved=1 } [0x01 road_class=0 road_speed=0 resolution 21 continue] *output* highway=* & dontadd=oneway & copy=98 & mtb:scale:uphill=5 { set oneway=yes; set taxi=no; set mkgmap:unpaved=1 } [0x01 road_class=0 road_speed=0 resolution 21 continue] *output* highway=* & dontadd=oneway & copy=99 & mtb:scale:uphill!=5 & mtb:scale:uphill!=4 { set oneway=-1; set taxi=no; set mkgmap:unpaved=1 } [0x01 road_class=0 road_speed=0 resolution 21 continue] *output* highway=* & dontadd=oneway & copy=98 & mtb:scale:uphill!=5 & mtb:scale:uphill!=4 { set oneway=yes; set taxi=no; set mkgmap:unpaved=1 } [0x01 road_class=0 road_speed=0 resolution 21 continue] *output* dontadd=yes [0x04 resolution 16 continue] *no output*
Gruss Torsten _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Felix Hartmann schrieb am 22.01.2010 16:30:
highway=* & copy=99 & mtb:scale:uphill!=5 & mtb:scale:uphill!=4 { set dontadd=yes; set taxi=no; set dontadd=oneway; set mkgmap:unpaved=1 } [0x10711 resolution 21 continue with_actions] *output*
Actually, what is the difference between "continue with_actions" and "continue"? I think I have never really understood, which version must be used in what case. Can you provide an easy example, wich demonstrates the different results, whether the with_actions flag is used or not? Gruss Torsten
On 22.01.2010 16:56, Torsten Leistikow wrote:
Felix Hartmann schrieb am 22.01.2010 16:30:
highway=*& copy=99& mtb:scale:uphill!=5& mtb:scale:uphill!=4 { set dontadd=yes; set taxi=no; set dontadd=oneway; set mkgmap:unpaved=1 } [0x10711 resolution 21 continue with_actions] *output*
Actually, what is the difference between "continue with_actions" and "continue"? I think I have never really understood, which version must be used in what case. Can you provide an easy example, wich demonstrates the different results, whether the with_actions flag is used or not?
auxiliary tags can only set by [continue with_actions] if you did (until 1498 it was working well) something like highway=primary & oneway=yes & cycleway=opposite_lane & route=bicycle {set oneway=-1} [0x... continue] then all actions inside {} are only applied to the current way, and not forwarded. In order to keep {oneway=-1} one had to use [continue with_actions].
Gruss Torsten _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Hi Felix
See below for an example that does not work. It works with 1497, but 1498 breaks it. highway=* & copy=99 & mtb:scale:uphill!=5 & mtb:scale:uphill!=4 { set dontadd=yes; set taxi=no; set dontadd=oneway; set mkgmap:unpaved=1 } [0x10711 resolution 21 continue with_actions] *output*
Revision 1498 didn't change anything to do with continue or oneway as far as I can tell. You are just discovering the problem with the styles on trunk that the actions are run in an arbitary order. Although you can work around that to get something that sort of works, any change at all in the code is likely to break the workarounds, because the actions will now run in a different order. This doesn't happen on the style branch (or if it did it would be a bug). I am certain that r1498 would not have changed the way your rules worked with the strict ordering as on the style branch (except perhaps for performance, but that could be fixed). So what I will do is revert 1498 on trunk and then put the patch in to the style branch instead. Is that OK? Once the style branch is merged and settled you should never have to spend so much time changing your rules again. Regards, ..Steve
On 22.01.2010 17:10, Steve Ratcliffe wrote:
Hi Felix
See below for an example that does not work. It works with 1497, but 1498 breaks it. highway=*& copy=99& mtb:scale:uphill!=5& mtb:scale:uphill!=4 { set dontadd=yes; set taxi=no; set dontadd=oneway; set mkgmap:unpaved=1 } [0x10711 resolution 21 continue with_actions] *output*
Revision 1498 didn't change anything to do with continue or oneway as far as I can tell. You are just discovering the problem with the styles on trunk that the actions are run in an arbitary order. Although you can work around that to get something that sort of works, any change at all in the code is likely to break the workarounds, because the actions will now run in a different order. This doesn't happen on the style branch (or if it did it would be a bug).
Before [continue with_actions] worked with auxiliary tags no matter how much you mistreated it. Since 1500 this is broken. And oneway behaviour got completly random.
I am certain that r1498 would not have changed the way your rules worked with the strict ordering as on the style branch (except perhaps for performance, but that could be fixed).
So what I will do is revert 1498 on trunk and then put the patch in to the style branch instead. Is that OK? Once the style branch is merged and settled you should never have to spend so much time changing your rules again.
Regards,
..Steve
Oh sorry, I meant 1488 (though 1498 maybe too). 1487 was definitly still working and only had one bug: keya=123 [continue] keya=123 [continue] -- no output. Everything else worked correctly. Sinde 1500 it simply got unpredictable. The lines file is NOT read in order anymore for [continue] Best would be to revert to 1487 from the style rules, then change the conditional rules to work like [continue with_action] but without tpye, and dont't fix keya=123 [continue] keya=123 [continue] -- no output -- My not correctly working attempts to overcome this meant 50% more memory use, and around 40-100% more rendering time. The above problem can easily be fixed by doing keya=123 {set keynew1=...; set keynew2=.... ; set keynew3=.... [continue with_actions] keynew1= ...... [continue]/[continue with_actions].
_______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Hi Steve, Felix, On Fri, Jan 22, 2010 at 04:10:54PM +0000, Steve Ratcliffe wrote:
So what I will do is revert 1498 on trunk and then put the patch in to the style branch instead. Is that OK? Once the style branch is merged and settled you should never have to spend so much time changing your rules again.
You would have to merge -r1497:1499 (or better, everything up to HEAD) to branches/style and then revert -r1499:1497. r1499 is some cosmetic fixes for r1498. I look forward to seeing the style branch merged to trunk soon. I would like to improve the handling of relations. As you can read in the comments in resources/styles/default/relations, I would like to have more filtering options for apply rules, and also applying the matching relations in a user-specified order. I think that it would be best to have the style fixes in trunk first. Best regards, Marko
participants (5)
-
Felix Hartmann -
Mark Burton -
Marko Mäkelä -
Steve Ratcliffe -
Torsten Leistikow