(PATCH needs rework) have even more flexiblity in mapcss

[cmuelle8]

Hi,

I wondered why the follow mapcss was not working:

relation[type=route][wiki:symbol] > way
{
    wicon: get_tag_value("wiki:symbol");
    is-cycle-route: true;
}

way[eval(prop(is-cycle-route))] > node
{
    icon-image: eval(concat(SOMEURL, prop("wicon")));
}

If we have descendant selectors, the trouble is, that the parents Environment is created from scratch. The patch attached hashes the environment in ElemStyles and makes it possible to question the proper environment of the parent when descendant selectors (+cond) are used. Moreover I put some patches into Expression to make it possible to query the parents environment in the childs rules. So the following works:

relation[type=route][wiki:symbol] > way
{
    icon: get_tag_value("parent.wiki:symbol");
    is-cycle-route: true;
}

way[eval(prop(is-cycle-route))] > node
{
    icon-image: eval(concat(SOMEURL, prop("parent.icon")));
}

With "parent." we can reference the environment of the parent in the descendant selector. This makes it possible to extract symbols out of relations and style the nodes accordingly.

Currently I'm missing a way to find out the correct URL of a wp commons image if I just have the filename (as is tagged on some relations with wiki:symbol). The idea is to add yet another function to Expression class that is able to find out the right url for us, maybe find_wiki_symbol(filename).. So ATM it works only if you build the URL yourself (with concat) or if the whole URL is supplied in the primitive's tag.

Another nice thing would be a "osmc:symbol" decoder in ImageProvider, or directly in mappaint's node code..

Regards,
Christian

[cmuelle8]

have more mapcss features..

[cmuelle8]

Ticket #6122 has been marked as a duplicate of this ticket.

[cmuelle8]

typo fixed..

[bastiK]

Hi Christian, this looks very advanced, but let's discuss some things first. ;)

Caching the environments permanently requires a lot of memory. In a first test, the memory use for the painting cache increased from some value < 1 MB to 13 MB, this is too much.

Then I have a question about order. Normally, the style cache is created for one primitive at a time. How do you ensure, that the dependent primitive (e.g. parent) comes before the current? What about cyclic relation dependencies?

Isn't there also a different environment for each zoom scale value?

Maybe you can explain the intended use a little more, there could be other ways to solve the whole problem!

In the current example, something like

relation[type=route][wiki:symbol] > way > node
{
    icon-image: eval(concat(SOMEURL, get_parent_tag_value("wiki:symbol", 2));
}

where the "2" in get_parent_tag_value would refer to a nesting level of 2.

Currently I'm missing a way to find out the correct URL of a wp commons image if I just have the filename (as is tagged on some relations with wiki:symbol). The idea is to add yet another function to Expression class that is able to find out the right url for us, maybe find_wiki_symbol(filename).. So ATM it works only if you build the URL yourself (with concat) or if the whole URL is supplied in the primitive's tag.

Sounds good.

Another nice thing would be a "osmc:symbol" decoder in ImageProvider, or directly in mappaint's node code..

Would be fancy. Probably doesn't matter much if you create an image in advance or paint it in the rendering loop. I'd say the latter so it can be scaled more easily.

[bastiK]

Btw., wouldn't it be more useful to put the symbol alongside the way?

[cmuelle8]

Replying to bastiK:

Btw., wouldn't it be more useful to put the symbol alongside the way?

For this particular use case, yes. But after all, mapcss should give you the possibility - why not? maybe some guy has a use for it..

[cmuelle8]

Replying to bastiK:

Hi Christian, this looks very advanced, but let's discuss some things first. ;)

Caching the environments permanently requires a lot of memory. In a first test, the memory use for the painting cache increased from some value < 1 MB to 13 MB, this is too much.

Everything comes at some cost :) Maybe we can do envs=null after each run of generateStyles..

Then I have a question about order. Normally, the style cache is created for one primitive at a time. How do you ensure, that the dependent primitive (e.g. parent) comes before the current? What about cyclic relation dependencies?

• ATM I do not, but you could easily solve this by just running generateStyles() twice, since the rules are additive
  • running it once already has to cope with overwriting rules
  • running twice suffices as long as the descendant selector stays binary (a tertiary one probably needs more runs!?)
• no problem with cyclic relation dependencies, since there is no loop involved
• length of the "parent." chain (i.e. the stylesource writer) determines maximum parent depth in any expression
• no more than one step is used in Selector.java

Isn't there also a different environment for each zoom scale value?

• the range property is part of the multicascade (like env.mc.range)
• a multicascade seems to exist for each zoom scale value like you suggest
• maybe I should clear out envs, before generateStyles runs, so that the hashmap is fresh for each zoom val. this should ensure that only parents are available that have a permissive range for current zoom (parents not in range are not added, as the instructions to add rules to their env.mc are not carried out)

• note that this was not handled before either, actually the error was bigger, since in "e2" the mc of the child was silently copied to the parent (but since range is not checked in selector code, this effectively made parent part of selector valid for all zoom levels)

Maybe you can explain the intended use a little more, there could be other ways to solve the whole problem!

In the current example, something like

relation[type=route][wiki:symbol] > way > node
{
    icon-image: eval(concat(SOMEURL, get_parent_tag_value("wiki:symbol", 2));
}

where the "2" in get_parent_tag_value would refer to a nesting level of 2.

I've thought about this, but it makes the code even more complex and, depending on your thought of get_parent_tag_value (do you mean parent env OR osm parent - the later is tricky, because a node can be part of many ways (that I did not select in the selector). All I'm saying is, you will not get around saving Envs - either in full, which allows arbitrary and consistent css source, or stripped, allowing a parent ladder within one rule only (i dislike that, since I suggest this is less powerful).

Greetings,
Christian

[bastiK]

Replying to cmuelle8:

Replying to bastiK:

Caching the environments permanently requires a lot of memory. In a first test, the memory use for the painting cache increased from some value < 1 MB to 13 MB, this is too much.

Everything comes at some cost :)

I meant way too much...

Maybe we can do envs=null after each run of generateStyles..

Maybe there is a misunderstanding how generateStyles() works, there is little documentation at the moment.

It takes a primitive and a scale and returns the list of styles to display + the valid scale range for that list. It is indirectly called by getStyleCacheWithRange() which is called for each renderer loop, but makes sure that the result is cached. That means generateStyles() is called only once per primitive and zoom range. Everything is strictly lazy, i.e. the style is not generated unless the primitive is really displayed in the current view (and with the given scale).

If you set envs=null after each run, i think this makes everything pointless.

Then I have a question about order. Normally, the style cache is created for one primitive at a time. How do you ensure, that the dependent primitive (e.g. parent) comes before the current? What about cyclic relation dependencies?

• ATM I do not, but you could easily solve this by just running generateStyles() twice, since the rules are additive
  • running it once already has to cope with overwriting rules
  • running twice suffices as long as the descendant selector stays binary (a tertiary one probably needs more runs!?)
• no problem with cyclic relation dependencies, since there is no loop involved
• length of the "parent." chain (i.e. the stylesource writer) determines maximum parent depth in any expression
• no more than one step is used in Selector.java

I have to admit, I don't really understand your patch... E.g. you set an Environment object as value of a magic property "env-parent" in a Cascade. This is quite a dirty trick, why not simply add a new field to Cascade class?

Isn't there also a different environment for each zoom scale value?

• the range property is part of the multicascade (like env.mc.range)
• a multicascade seems to exist for each zoom scale value like you suggest

Where? I don't think this is right. A mc object only exists while generateStyles() is running. And it refers to one particular primitive and one particular scale. Afterwards, all cascades are garbage collected. (prior to the patch)

• maybe I should clear out envs, before generateStyles runs

see above, generateStyles does not run on all primitives, it is "on demand".

, so that the hashmap is fresh for each zoom val. this should ensure that only parents are available that have a permissive range for current zoom (parents not in range are not added, as the instructions to add rules to their env.mc are not carried out)

• note that this was not handled before either, actually the error was bigger, since in "e2" the mc of the child was silently copied to the parent (but since range is not checked in selector code, this effectively made parent part of selector valid for all zoom levels)

Actually, the Selector code does not read the e.mc in applies(Environment e), so it does not matter what you set there.

Maybe you can explain the intended use a little more, there could be other ways to solve the whole problem!

In the current example, something like

relation[type=route][wiki:symbol] > way > node
{
    icon-image: eval(concat(SOMEURL, get_parent_tag_value("wiki:symbol", 2));
}

where the "2" in get_parent_tag_value would refer to a nesting level of 2.

I've thought about this, but it makes the code even more complex and, depending on your thought of get_parent_tag_value (do you mean parent env OR osm parent - the later is tricky, because a node can be part of many ways (that I did not select in the selector). All I'm saying is, you will not get around saving Envs - either in full, which allows arbitrary and consistent css source, or stripped, allowing a parent ladder within one rule only (i dislike that, since I suggest this is less powerful).

You'd have to save the list of primitives that make the child selector chain succeed.

[cmuelle8]

Replying to bastiK:

I meant way too much...

• For a production machine you should not bother about 13 MB of RAM, there are probably other memory hogs one could care more about. For smaller netbooks, it is interesting, true. I did not say we shouldn't look for memory optimization ;-). Optionally we can build in the support with a config flag (again :) )..

Maybe there is a misunderstanding how generateStyles() works, there is little documentation at the moment.

It takes a primitive and a scale and returns the list of styles to display + the valid scale range for that list. It is indirectly called by getStyleCacheWithRange() which is called for each renderer loop, but makes sure that the result is cached. That means generateStyles() is called only once per primitive and zoom range. Everything is strictly lazy, i.e. the style is not generated unless the primitive is really displayed in the current view (and with the given scale).

If you set envs=null after each run, i think this makes everything pointless.

• absolutely true, this was just a quick, untested thought..
• i have an understanding of lazy evaluation (it's very prominent in haskell btw), but IMHO no problems arise from the type of evaluation in our case. i'd step as far forward and 'd say it is a complete non-issue - think about it:
  • relations are "always" in view
  • ways that are parents of a node and vice versa will always be in the view pending evaluation together
  • > i will not have objects whose parents are dangling, the other way around does not matter
    • say we have a long way with node range A that is out of view, and B that is in view
    • node range B and its parent will be evaluated
    • if node range A comes into view, parent evaluation was already done
    • > of course, scraping *envs* in a way I suggested in one of the comments above, is non-sense
• a bigger problem IMHO is the missing range check of the parent selector you talked about earlier..

Then I have a question about order. Normally, the style cache is created for one primitive at a time. How do you ensure, that the dependent primitive (e.g. parent) comes before the current? What about cyclic relation dependencies?

• ATM I do not, but you could easily solve this by just running generateStyles() twice, since the rules are additive
  • running it once already has to cope with overwriting rules
  • running twice suffices as long as the descendant selector stays binary (a tertiary one probably needs more runs!?)
• no problem with cyclic relation dependencies, since there is no loop involved
• length of the "parent." chain (i.e. the stylesource writer) determines maximum parent depth in any expression
• no more than one step is used in Selector.java

I have to admit, I don't really understand your patch... E.g. you set an Environment object as value of a magic property "env-parent" in a Cascade. This is quite a dirty trick, why not simply add a new field to Cascade class?

• either way is fine, but there are points where putting this property into user's space comes in handy
  • e.g. if I wanted to debug the style sheet, I could let JOSM paint strings out of the Environment structures right onto the objects
  • in my mind doing it this way is all about flexibility and it _only_ might break, if the user makes it break
  • then again, writing style sheets is an advanced task where we should not think so much about it's ease and security of use, but more from the programmer's perspective, i.e. "how do I help the style writer", "what can I give into his hands to understand what the style does and how.."
  • again, you are right that this can be used in a "dirty" way, and maybe we should choose a more unique string than "env-parent", but by default it's clean - then it is the choice of the style writer to make his style break :)

Isn't there also a different environment for each zoom scale value?

• the range property is part of the multicascade (like env.mc.range)
• a multicascade seems to exist for each zoom scale value like you suggest

Where? I don't think this is right. A mc object only exists while generateStyles() is running. And it refers to one particular primitive and one particular scale. Afterwards, all cascades are garbage collected. (prior to the patch)

• yes, using the patch, the garbage collector does not scrape the envs, hence mc's stay alive as well - this is why you complain about higher mem footprint :)
• ATM, since you've made a strong point about the lazy evaluation of object styles, I cannot get rid of *envs*
  • say a way is already evaluated, view is changed, more of its nodes get evaluated
  • > then I need the env of the way, if we have a "evaluated_way > new_nodes" style selector
• a complete different approach that just comes to my mind would be
  • evaluating the style of the parent /lazy/ in Selector.java
  • this would give the memory back to you, but costs you (a lot) more CPU cycles
  • (the well known trade-off, if u want speed sacrifice your memory, if u want low mem consumption, sacrifice CPU cycles..)
  • > e.g. for every node of a way in a "way > node" selector, way's style is evaluated (if way is a child in another "rel > way" selector, this goes up the chain and gets expensive - so my preference is clear: sacrifice the memory)

Actually, the Selector code does not read the e.mc in applies(Environment e), so it does not matter what you set there.

• No, I was saying that we CAN - if I have the ENV of an evaluated parent, the range is included in env.mc
• > hence I COULD use that to check against it in the Selector.java code

I've thought about this, but it makes the code even more complex and, depending on your thought of get_parent_tag_value (do you mean parent env OR osm parent - the later is tricky, because a node can be part of many ways (that I did not select in the selector). All I'm saying is, you will not get around saving Envs - either in full, which allows arbitrary and consistent css source, or stripped, allowing a parent ladder within one rule only (i dislike that, since I suggest this is less powerful).

You'd have to save the list of primitives that make the child selector chain succeed.

• exactly, which would map from env.osm of the current approach ;-)
• and you cannot inherit custom props this way, just query the parent primitives tags..

all the best,
Christian

[bastiK]

Regarding memory:
If everyone would think like that, josm would be <edit>unusable</edit> by now, even on the desktop. :)

It took continuous effort by multiple developers to keep the memory footprint low. At the moment we have about 186 Bytes per primitive. With your patch applied, it is 628 Bytes per primitive. I can open and process country extracts from geofabrik with the current josm, but I run out of memory with your patch applied.

Even though I dislike the idea to permanently save the multicascades, it can be done with negligible memory use with flyweight pattern / intern pool. Most straightforward would be to include it in StyleCache.java.

[bastiK]

Replying to cmuelle8:

Replying to bastiK:

I meant way too much...

• For a production machine you should not bother about 13 MB of RAM, there are probably other memory hogs one could care more about. For smaller netbooks, it is interesting, true. I did not say we shouldn't look for memory optimization ;-). Optionally we can build in the support with a config flag (again :) )..

Maybe there is a misunderstanding how generateStyles() works, there is little documentation at the moment.

It takes a primitive and a scale and returns the list of styles to display + the valid scale range for that list. It is indirectly called by getStyleCacheWithRange() which is called for each renderer loop, but makes sure that the result is cached. That means generateStyles() is called only once per primitive and zoom range. Everything is strictly lazy, i.e. the style is not generated unless the primitive is really displayed in the current view (and with the given scale).

If you set envs=null after each run, i think this makes everything pointless.

• absolutely true, this was just a quick, untested thought..
• i have an understanding of lazy evaluation (it's very prominent in haskell btw), but IMHO no problems arise from the type of evaluation in our case. i'd step as far forward and 'd say it is a complete non-issue - think about it:
  • relations are "always" in view
  • ways that are parents of a node and vice versa will always be in the view pending evaluation together
  • > i will not have objects whose parents are dangling, the other way around does not matter

Why not? JOSM supports something like

node[traffic_calming] < way {}

• say we have a long way with node range A that is out of view, and B that is in view
• node range B and its parent will be evaluated
• if node range A comes into view, parent evaluation was already done
• > of course, scraping *envs* in a way I suggested in one of the comments above, is non-sense

• a bigger problem IMHO is the missing range check of the parent selector you talked about earlier..

Then I have a question about order. Normally, the style cache is created for one primitive at a time. How do you ensure, that the dependent primitive (e.g. parent) comes before the current? What about cyclic relation dependencies?

• ATM I do not, but you could easily solve this by just running generateStyles() twice, since the rules are additive
  • running it once already has to cope with overwriting rules
  • running twice suffices as long as the descendant selector stays binary (a tertiary one probably needs more runs!?)
• no problem with cyclic relation dependencies, since there is no loop involved
• length of the "parent." chain (i.e. the stylesource writer) determines maximum parent depth in any expression
• no more than one step is used in Selector.java

I have to admit, I don't really understand your patch... E.g. you set an Environment object as value of a magic property "env-parent" in a Cascade. This is quite a dirty trick, why not simply add a new field to Cascade class?

• either way is fine, but there are points where putting this property into user's space comes in handy
  • e.g. if I wanted to debug the style sheet, I could let JOSM paint strings out of the Environment structures right onto the objects
  • in my mind doing it this way is all about flexibility and it _only_ might break, if the user makes it break
  • then again, writing style sheets is an advanced task where we should not think so much about it's ease and security of use, but more from the programmer's perspective, i.e. "how do I help the style writer", "what can I give into his hands to understand what the style does and how.."
  • again, you are right that this can be used in a "dirty" way, and maybe we should choose a more unique string than "env-parent", but by default it's clean - then it is the choice of the style writer to make his style break :)

This does not convince me. You can add special functions in Expression.java that return these objects.

• a complete different approach that just comes to my mind would be
  • evaluating the style of the parent /lazy/ in Selector.java
  • this would give the memory back to you, but costs you (a lot) more CPU cycles
  • (the well known trade-off, if u want speed sacrifice your memory, if u want low mem consumption, sacrifice CPU cycles..)
  • > e.g. for every node of a way in a "way > node" selector, way's style is evaluated (if way is a child in another "rel > way" selector, this goes up the chain and gets expensive - so my preference is clear: sacrifice the memory)

Sounds good, I guess it can't be that much if done right.

Actually, the Selector code does not read the e.mc in applies(Environment e), so it does not matter what you set there.

• No, I was saying that we CAN - if I have the ENV of an evaluated parent, the range is included in env.mc
• > hence I COULD use that to check against it in the Selector.java code

I've thought about this, but it makes the code even more complex and, depending on your thought of get_parent_tag_value (do you mean parent env OR osm parent - the later is tricky, because a node can be part of many ways (that I did not select in the selector). All I'm saying is, you will not get around saving Envs - either in full, which allows arbitrary and consistent css source, or stripped, allowing a parent ladder within one rule only (i dislike that, since I suggest this is less powerful).

You'd have to save the list of primitives that make the child selector chain succeed.

• exactly, which would map from env.osm of the current approach ;-)
• and you cannot inherit custom props this way, just query the parent primitives tags..

Maybe one problem is, that i cannot see the full potential of your patch. Could you give more examples / ideas, especially where it wouldn't be sufficient to query parent primitive tags?

[cmuelle8]

Replying to bastiK:

Regarding memory:
If everyone would think like that, josm would be <edit>unusable</edit> by now, even on the desktop. :)

You were originally talking about some fix number of 13mb which I was refering to.. But, yes, for a country extract it probably does not scale at all. On the other side, people doing serious editing will not open a country extract in josm. Either you have a small area you're editing or you load objects by id (administrative boundaries, etc.)

It took continuous effort by multiple developers to keep the memory footprint low. At the moment we have about 186 Bytes per primitive. With your patch applied, it is 628 Bytes per primitive. I can open and process country extracts from geofabrik with the current josm, but I run out of memory with your patch applied.

Even though I dislike the idea to permanently save the multicascades, it can be done with negligible memory use with flyweight pattern / intern pool. Most straightforward would be to include it in StyleCache.java.

Thx for sharing this, as I understand it, this approach minimizes memory by deduplicating identical env.mc, right?

greetings

[cmuelle8]

Replying to bastiK:

Why not? JOSM supports something like

node[traffic_calming] < way {}

Yes and actually this case seems to be the more interesting one, if way is in view and node is not (in case node has not previously been evaluated). So I guess applying lazy evaluation for all childs or referers of DescendantSelector.b's primtive is the way to go then. To put performance into this even for large datasets we could cache the results with a fixed size LRU-Cache..

This does not convince me. You can add special functions in Expression.java that return these objects.

True, this is a cleaner alternative.

• a complete different approach that just comes to my mind would be
  • evaluating the style of the parent /lazy/ in Selector.java
  • this would give the memory back to you, but costs you (a lot) more CPU cycles
  • (the well known trade-off, if u want speed sacrifice your memory, if u want low mem consumption, sacrifice CPU cycles..)
  • > e.g. for every node of a way in a "way > node" selector, way's style is evaluated (if way is a child in another "rel > way" selector, this goes up the chain and gets expensive - so my preference is clear: sacrifice the memory)

Sounds good, I guess it can't be that much if done right.

For the example you gave at the top: if there is not any kind of caching involved, node[traffic_calming]'s style is re-evaluated for all nodes of all ways of the current view, if you have a country dataset, go figure :-) but then, this probably is not attributed "done right".

Maybe one problem is, that i cannot see the full potential of your patch. Could you give more examples / ideas, especially where it wouldn't be sufficient to query parent primitive tags?

bottom - up synthesis never sees all potential in advance. just saying that I have not thought about it too much about it yet, but having it will find someone who has a use for it, promise.

greetings

[stoecker]

Replying to cmuelle8:

On the other side, people doing serious editing will not open a country extract in josm. Either you have a small area you're editing or you load objects by id (administrative boundaries, etc.)

People do this. Even I do this from time to time.

[bastiK]

Replying to cmuelle8:

• a complete different approach that just comes to my mind would be
  • evaluating the style of the parent /lazy/ in Selector.java
  • this would give the memory back to you, but costs you (a lot) more CPU cycles
  • (the well known trade-off, if u want speed sacrifice your memory, if u want low mem consumption, sacrifice CPU cycles..)
  • > e.g. for every node of a way in a "way > node" selector, way's style is evaluated (if way is a child in another "rel > way" selector, this goes up the chain and gets expensive - so my preference is clear: sacrifice the memory)

Sounds good, I guess it can't be that much if done right.

For the example you gave at the top: if there is not any kind of caching involved, node[traffic_calming]'s style is re-evaluated for all nodes of all ways of the current view, if you have a country dataset, go figure :-) but then, this probably is not attributed "done right".

I don't have a clear picture of what you want to do, so take my prediction with a grain of salt.

What's with this style sheet, how can one be evaluated before the other?

node < way {}
way > node {}

[cmuelle8]

What's with this style sheet, how can one be evaluated before the other?

node < way {}
way > node {}

With saving envs I suppose you can resolve this by just running style evaluation twice (since instructions are additive only, so we wouldn't destroy things in a second pass). If we evaluate parent's or child's style on demand we have a cyclic dependency that needs to be detected.

[bastiK]

parent_tag() support was added in #6150 / [4011]. If you still want to pursue the more general approach, I'd like to see the best example / use case you can come up with. I'm not buying this "bottom - up synthesis" argument - such a drastic change has clearly negative consequences, e.g. more bugs, worse performance and more complicated code. So it would be good to know whether this is useful in any way.

[stoecker]

What is the current status of this ticket?

[bastiK]

Replying to stoecker:

What is the current status of this ticket?

Patch has to be considerably reworked before it can go in.

[bastiK]

@cmulle8:

Here is an example: Let's say that :closed on multipolygons selects those relations that have no broken rings and that have no incomplete member. Let's further assume, there is a :incomplete selector for incomplete primitives.

Then someone might want to highlight broken multipolygons by drawing the member ways in red. However, relations that are merely incomplete should not be highlighted, because this is not necessarily an error in the database. This could be solved as follows:

relation 
{
  is-complete-multipolygon: true;
}

way:incomplete < relation[type=mulitpolygon]
{
  is-complete-multipolygon: false;
}

relation[type=multipolygon][eval(parent_prop("is-complete-multipolygon"))] > way
{
  width: 15;
  color: red;
}

parent_prop would refer to the properties of the parent object. This means, either

(a) the style properties for the parent relation have been calculated in the past and saved in memory until now

or

(b) the style properties for the parent relation are calculated dynamically

In both cases there seem to be recursion involved, but one could cut it off at some level. First option costs memory and second option processor time. I think it is possible to make the memory footprint negligible in (a) but for (b) i cannot say how hard it is to optimize and if optimisation would be necessary.

[michael2402]

I just have the same problem.

I want to be able to evaluate a expression stating 'select all nodes, that are a stop in the stop_area_group (that is the active relation, which is quite easy to flag)'

relation[public_transport="stop_area_group"] > relation[public_transport="stop_area"] >[role="stop"] node

For now, I did not find a way to do this or to work around nicely.