-
Notifications
You must be signed in to change notification settings - Fork 823
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove polygon fill rendering for barrier=hedge #3844
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to make sure i understand this change - you remove the rule specifically for the area-barriers layer completely because you stop filling the areas and the outline is to be drawn according to the class based rules that apply both to the area and the lines layers.
It might be worth considering then combining polygons and lines into a single layer (using a compound UNION ALL
query as we do for points and polygons in many cases).
In any case i think the area-barriers and line-barriers queries should follow each other and have the same starting zoom level - otherwise this can easily be confusing.
That's right.
Good idea, I had not considered that. I hope this would lead to slightly faster rendering performance, by reducing the number of layers by 1? |
The point of having just one layer would be not so much performance but simplifying code since you only need to have the value lists once in an outer query wrapping the two. Best example probably is: openstreetmap-carto/project.mml Lines 1539 to 1594 in 5fd5107
The main question would be if there would be any issues with the query returning mixed POLYGON and LINESTRING feature types (in which case you would need to do do an |
I've added new commits which combine the two layers, line-barriers and area-barries, into one, and removes the class: barriers. I could also try to change the indenting, but I wanted to minimize the number of changed lines if possible, and I'm still unclear on the proper indenting style for the SQL queries. (Honestly I was surprised that my first attempt worked properly and rendered without errors) |
I didn't use |
You need the Would look like (untested code):
|
Thank you, that code works. Hedges and city walls still render fine on
linestrings and polygons. I've tested the changes and pushed a new
commit.
|
I'd rather not render them at all. To do this implies that an object can be both an area and not an area at the same time. |
There used to be a tag “fenced=yes” which could be added as a property of
another feature, but now the wiki page says this has been discouraged for
many years and barrier=fence should be used instead. Similarly, there isn’t
any documented tag like hedge=yes for adding to other features; I believe
many mappers think that barrier=fence and barrier=hedge etc can be added to
an area feature as a property, “this area is surrounded by a hedge /
fence”. So we would need a proposal for new tags for the property, to help
clarify the situation.
I wouldn’t mind doing a proposal for hedge=yes, wall=yes, fence=yes -
because I agree that one database element should only represent one Maine
feature. But under the current documentation it appears that barrier=hedge
can also be a property, not only a feature. Also, it’s clear that area=yes
can be used with barrier=hedge mapped as a polygon, according to
documentation and current usage.
So for now I would continuing rendering hedges on objects which become
polygons, until the tagging changes.
|
That's a good point - however this would be a high impact change and i am not sure if it is sufficiently broadly accepted among mappers that this is wrong. At least in one case this is quite universally accepted mapping - with natural=coastline + place=island/islet (150k combinations). For barrier=fence there are nearly 300k combinations with landuse/leisure/amenity - which is not exactly rare either. I don't think either of these is indicated by editor validation rules as wrong. |
I opened https://josm.openstreetmap.de/ticket/10551 long time ago and it remains open, but response was at best mixed |
Note the core of the problem is the ambiguity when one OSM feature is used to represent several different real world concepts at the same time. This can be cases where one of them is linear and one is polygon-like. But it does not have to be. Example: Something tagged building=yes, shop=bakery, name=foo. It is not clear if the name is the name of the building or the name of the bakery. In cases like this we simply assume the name applies to both the shop and the building (and render them with whatever priority they have for us). The analog interpretation for the linear vs. polygon-like case would be to interpret it as both polygon and linestring. |
You could change the sql, make a commit, adjust indenting, make another commit. So the code would be clean, but both diffs would be readable. |
Good idea. I've pushed a new commit with the indenting changes.
|
@imagico has approved this PR. Are there other comments or reviews? I'll wait a few more days. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still 👎 on the idea of rendering a polygon in a way that makes it look like it's a linear feature.
Have you considered the arguments above - that barrier tags in combination with landuse/leisure/amenity are very common and that we render multiple uses of the same geometry for different things already in other cases (like building + shop)? Note right now we already interpret barrier tags on polygons as line tags and render them with a line for all barriers: openstreetmap-carto/landcover.mss Lines 849 to 850 in eae0909
The only change this PR makes is making this consistent also for barrier=hedge which has so far been rendered with a fill color. |
@pnorman - hedges which are tagged on polygons are currently rendered both with a green fill and a dark gray 0.4 pixel width line. The line rendering is identical to that for The wiki page Tag:barrier=hedge says "For a thick hedge draw a closed way around the perimeter of the hedge and tag it with area=yes." and "onArea" is also set to "yes" in the However, Tag:barrier=hedge also mentions mapping as areas (with Area rendering of See the similar discussion about hedges imported as areas in #971 especially #971 (comment) and below. I tried rendering just So there are 4 options now:
What are your thoughts on the way forward? |
Oh, I forgot another option (probably not a good one):
|
@pnorman - have you had a chance to think about this issue again, and the 4 possible solutions above? |
I'd be okay with this. |
OK, you would prefer that we not render anything for polygons tagged as barrier=hedge. This means that an area of Would this need to be discussed on the mailing lists first? Perhaps there should be a tag like |
Re: "broke the rendering" - this is not exactly true. This PR renders all lines and areas with the Previously, This style is now interpreting I previously considered rendering Another option was to only render Thus, this PR is the best option. An alternative supported above by @pnorman was to not render anything for As noted above, the fill color will be rendered if there is also appropriate tag to describe the vegetation such as |
For me area=yes on a polygon means, the whole plane need a render color, not only the outline.
A hedge should be decribed as a hedge, and not as something like natural=scrub. The above example:
There where the highway enters the caravan_site on the highway, there is not a hedge, so the site does not have a hedge on that part and can not be mapped entire circumference. The area:* methodology. area=yes and area:* are always next to each other and should be render the same. |
@jeisenbe The problem lies not in rendering normal closed ways (i.e., tagged without The reason hedges use |
I completely agree that this is inconsistent, but given that there is a valid use case for walls mapped as areas too, I would opt for adding rendering to Barriers can be mapped as areas if sufficient detail can be surmised, and in case of massive, thick walls or huge blocks acting as barriers this certainly makes sense. |
An alternative is fine too of course, but please add that to this PR instead of first removing the existing documented and accepted solution. I could live with Please provide a migration path (with rendering) and time to re-tag. |
No, area=yes has one and exactly one meaning: It indicates that a closed way is a polygon. And as @jeisenbe has explained it is common practice right now that barrier tags are applied to polygons in a way indicating this polygon is enclosed by a barrier of this type. There are >350k combinations of a barrier and a landuse/leisure tag the vast majority of which represent exactly that use case. If you want to change/overload the meaning of the area=yes you need to discuss this with the mapper community. As @jeisenbe also explained there are only two possible approaches that are in line with the goals of this style and that could achieve consensus here:
|
EXACTLY. So why is barrier=hedge + area=yes not rendered as one? That is the point. |
For better understanding of how OSM data interpretation works: Every feature is imported into the rendering database either as a point, a linestring or a polygon. Features imported as a polygon with a barrier tag could either be rendered with an outline or with a fill. If you render them with a fill (like it has been done so far inconsistently for barrier=hedge but not for other barriers like barrier=fence/wall/etc.) that means misinterpreting >350k features were the barrier tag is used as a secondary tag characterizing the outline of the polygon. Interpreting the area=yes tag beyond its established meaning regarding the import decision linestring or polygon is not an option. We will not use our influence for pushing for an even more complex rule set for interpreting OSM data beyond the already existing complexity. If you want to lobby the mapper community for such added complexity you are free to do so of course. |
There are exactly 36665 hedges that carry the Not rendering I'm fine with exploring alternatives for |
We are talking about the situations, where there is area=yes added to it. And how many have also the area=yes tag? In Nederland is this 7829. from what with landuse 8 with leisure 0 . |
I've run into several situations ( One workaround that consistently works - and it's hard to imagine future changes breaking it - is to describe the interior as a multipolygon relation, while continuing to describe the boundary as one or more linestrings. Tagging relevant to the boundary can go on the boundary, and cause the linestrings to be imported as linear features. Tagging relevant to the interior goes on the multipolygon relation, and causes the area to be imported as a (multi)polygon feature. I concede that the approach seems awkward, but in my experience, it is less awkward than mapping the boundary twice - once for treatment as a boundary (or barrier, or whatever) and again for treatment as an area. It's also less error-prone than dual-mapping in that editing the member ways cannot introduce an inconsistency. |
In my opinion this change discriminate against those who care about a correct and precise mapping and supports bad tagging. Please revert it and render all |
I agree that this usage is ambiguous. However, that usage is very common, more common than the use of https://wiki.openstreetmap.org/wiki/Key:fenced - "Whether the outer perimeter of something is fenced." "This feature has been labeled as deprecated. The recommended replacement is: barrier=fence." "Once upon a time there were no barrier tags, because nobody had invented them yet. In that time the only way to map a fence in Openstreetmap was with fenced=yes. Almost all fences mapped since the invention of barrier=fence are (re)mapped using that new tag." Since the original mapping was to add "fenced=yes" to "landuse=meadow" or "leisure=playground", it is still very common to add 'barrier=fence' to one of these features, and by analogy this is also done with walls, hedges and other barriers. According to http://overpass-turbo.eu/s/Qr1 there are 36k closed ways with barrier=hedge + area=yes without another area feature, but there are 16k uses of The situation is worse for While the average wall is thinner than the average hedge, the difference is less than an order of magnitude, and there is another feature that is about as wide as a hedge: This means that if mappers want to use barrier=hedge and barrier=wall and barrier=city_wall for both linear features and areas, database users are going to have to look for tags like area=yes and type=multipolygon to try to figure out if it is a linear or area features, and often there will be mistakes. For this reason, we should not rely on the presence of A real solution to this would require changing common tagging practices, but since overall the tagging of linear barriers on other features is very common (~360k uses, mainly with |
The Netherlands has been claimed as a place where barrier=hedge areas are used properly and are necessary. I have already downloaded one whole provicne, Zeeland, which has quite complete landcover and landuse mapping due to an import. In Zeeland there are 149 uses of
But in most areas, you will find similar features tagged with There are only 3 places in Zeeland where but https://www.openstreetmap.org/way/733907216 is tagged https://www.openstreetmap.org/way/624268968 - this is also barrier=hedge + area=yes Afterward, I randomly looked for https://www.openstreetmap.org/way/433123082 https://www.openstreetmap.org/way/444439556 (Note also that JOSM always renders hedges as linear features, no matter whether they are tagged with area=yes or not, so all the screenshots from JOSM show the hedge as a yellow line) |
To summarize the previous posts:
So our possible solutions are:
|
Maybe also consider providing support for an alternative tagging system like the one proposed by jdhoek as a valid solution. I believe this would be better than just the 2 solutions you proposed. The advantage is that it solves both the issue of ambiguity of barriers on polygons, and still allows the mapper to specify the hedge or other barrier as an area. In the end would keep most mappers happy. |
Maybe also consider providing support for an alternative tagging system
like the one proposed by jdhoek as a valid solution. I believe this would
be better than just the 2 solutions you proposed.
That is an option, if a new method of tagging becomes widely used.
Currently natural=scrub and landuse=forest or natural=wood are most widely
used for mapping such areas, with some use of leisure=garden and some
(mis-)use of village_green.
If something like leisure/landuse/natural=shrubbery/greenery or another tag
becomes established and common over the next year or two, we could
certainly consider rendering it in the future.
But our policy is to avoid rendering brand-new tags before they are
accepted by mappers, as shown by widespread use.
|
If anyone wants to develop a followup to this change - i prepared two branches that could be helpful: https://github.com/imagico/osm-carto-alternative-colors/tree/no-barrier-polygons This implements the complete removal of rendering https://github.com/imagico/osm-carto-alternative-colors/tree/hedge_polygons_as_landcover This adds I will not submit a PR for either of these changes at this time but i don't want to choke a potential consensus in either direction. If anyone wants to prepare and argue for a change based on either of these approaches that would be welcome and i would be willing to listen to their arguments. What i would as said strongly oppose however is any change that tries to interpret the |
I also agree that area=yes should indicate a filled area for hedge or wall as the Area tag notes "An area (or filled polygon)" The proper way of adding a hedge as a single line barrier would be to leave off area=yes from the tags. It would be improper to add a scrub tag, which is meant to be for a point, not an area, to the feature for only rendering proposes. |
In terms of displayed position accuracy i can not agree with the current rendering. Assuming that a hedge is accurately mapped according to the wiki with an closed polygon for me as a mapper this polygon should represent the edge of the object. Actually the current rendering makes such hedges thicker as they really are. Not mentioning the uncolorized center of the hedge. Here you can see an example of an overlap with benches. Both the bench an the edge of the hedge are positioned correctly. https://www.openstreetmap.org/#map=19/48.51844/9.05444 We don't map for the renderer, but whats about rendering for the mapper? Or only rendering for the renderer? |
Follow-up to closed PR #3834
Fixes #971
Also see discussion in about walls mapped as areas (not rendered) #3453
Changes proposed in this pull request:
Explanation:
Currently all closed ways tagged
barrier=hedge
will render with the dark green hedge fill color when they are imported as a polygon feature. This happens for closed ways tagged area=yes, relations with type=multipolygon or type=boundary, and for closed ways that are tagged with another polygon key like landuse=* or amenity=*. Unfortunately, many mappers add barrier=yes to other features like landuse=meadow to describe that there is a hedge around most of the meadow. In this case the meadow will often render like a solid hedge area, which is incorrect.Also, hedges imported as areas currently have a narrow black outline, unlike those mapped as linear ways.
This PR will remove the fill color for hedges on areas, and instead render them with the standard green line rendering used for other hedges. This is consistent with how other barrier features are rendered, including walls.
There are currently 30,177 ways with
barrier=hedge
andarea=yes
which may currently be rendered with the fill color, which will have the rendering changed by this PR. Some also have another key such as "landuse", "amenity" etc which we import as polygons: there are 11,677 of these features, which are often currently being rendered incorrectly as hedge areas, since hedge areas are rendered above other landcover fill colors.As review in #3834 showed, most of the features with
area=yes
are not clearly hedges, but many appears to be areas of shrubs or garden areas or scrub, etc. The current rendering does not make it clear that these areas are barrier features, rather than just a general "green" area, which may contribute to such mis-tagging.So to summarize, as a result of this change:
Over 11,000 features currently incorrectly rendered as hedge fill because of another polygon key on the same way will now be correctly rendered
30,000 ways with area=yes will loose the area fill rendering but have a clearer "hedge" barrier style outline.
Test renderings with links to the example places:
1) Long Acres Caravan & Camping
https://www.openstreetmap.org/way/225683577/#map=17/51.1651/-0.04265
way 225683577
barrier=hedge +
tourism=caravan_site
Before:
After:
2) Wide "hedgebank" next to a road (like an area of shrubs used to stabilise the slope?) with current rendering and rendering after:
https://www.openstreetmap.org/way/379793281#map=17/47.6418/10.1072
z16 before
z16 after
z17 before
z17 after (1 pixel wide outline in green)
3) Hedge along orchard border
https://www.openstreetmap.org/way/417705052#map=18/48.58993/-1.46011 - France
z16 hedge before ecosite France
z16 after
z17 before
z17 after
z18 Rue de la gare, Luxembourg - compare linear hedge in parking lot (upper left) to those mapped as areas (lower left)
z18 before
z17 before
z18 after
z17 after
Place de les Martyrs, Luxembourg
z18 before
z18 after