Skip to content
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 line barrier catch-all rendering #1986

Merged

Conversation

nebulon42
Copy link
Contributor

Removes line-barrier catch-all rendering and fixes a bug where the line-barrier layer was displayed from z16, but a rendering rule for z14 was present.

Line barrier values > 1000 uses (with taginfo usage and comments):

fence 1 748 621 ok
wall 1 306 878 ok
hedge 646 979 ok
retaining_wall 93 566 ok
yes 47 152 no wiki page, too general
kerb 44 915 ok
ditch 20 143 ok
city_wall 19 589 ok
guard_rail 17 077 ok
hedge_bank 14 508 only german wiki page available
wire_fence 10 379 no wiki page, should be fence
line 8 166 no wiki page, not a barrier (access=no?), tagging for the renderer
chain 5 405 ok
embankment 4 599 no wiki page, but included since rendering rule already present
field_boundary 3 847 no wiki page
wood_fence 2 171 no wiki page, should be fence
avalanche_protection 1 865 no wiki page
handrail 969 ok

@matkoniecz
Copy link
Contributor

fixes a bug where the line-barrier layer was displayed from z16, but a rendering rule for z14 was present

From reading code - this PR adds rendering of barrier lines on z14 and z15. I am guessing that it will give poor results at z15 (maybe except remote areas) and nearly always will make map worse on z14.

Also, what are before/after results? Especially on z14 and z15.

@nebulon42
Copy link
Contributor Author

this PR adds rendering of barrier lines on z14 and z15

https://github.com/gravitystorm/openstreetmap-carto/blob/master/landcover.mss#L628 should prevent that, or am I overlooking something?

One more note: Removal of barrier=line is likely to upset mappers who like to map sports field markings, but I think this is tagging for the renderer and should not be encouraged.

@trigpoint
Copy link

Field_boundary is used when the boundary is not easy to determine, usually fence or hedge.
Sometimes it is used when a boundary frequently changes.
The primary purpose is to map the boundary, as barriers are the most important feature when mapping the countryside, the detail doesn't really matter.

@matkoniecz
Copy link
Contributor

https://github.com/gravitystorm/openstreetmap-carto/blob/master/landcover.mss#L628 should prevent that, or am I overlooking something?

So only barrier = embankment will be now rendered on z14 and z15. But it is worth checking is it a good idea.

One more note: Removal of barrier=line is likely to upset mappers who like to map sports field markings, but I think this is tagging for the renderer and should not be encouraged.

It is undocumented and weird - the only reason to use barrier as main key is tagging for renderer.

Field_boundary is used when (...)

It would be far better to document it on wiki and link it here.

Field_boundary is used when the boundary is not easy to determine, usually fence or hedge.

So it is basically equivalent of barrier=yes.

@matkoniecz
Copy link
Contributor

I would consider adding also http://taginfo.openstreetmap.org/tags/barrier=handrail
(used 969 times, documented on wiki).

Maybe also other values with 100 and more usages (like it was done for shops).

@matthijsmelissen
Copy link
Collaborator

Looks good to me at first sight.

@imagico
Copy link
Collaborator

imagico commented Nov 29, 2015

Right now barrier, in particular barrier=ditch in combination with waterway tags looks strange and non-intuitive, like a waterway with a different color:

http://www.openstreetmap.org/#map=19/48.00544/7.78167

Maybe you can change this PR to not render barrier=ditch when in combination with a waterway tag that is rendered (i.e. stream/drain/ditch).

@nebulon42 nebulon42 force-pushed the line-barrier-catchall branch from 74a882b to a31a471 Compare November 29, 2015 20:26
@nebulon42
Copy link
Contributor Author

So only barrier = embankment will be now rendered on z14 and z15. But it is worth checking is it a good idea.

I cannot test this as the only occurrence in my data is covered by a road at lower zooms. But since the code was in I assumed that it was ok once. Maybe somebody else can confirm.

I would consider adding also http://taginfo.openstreetmap.org/tags/barrier=handrail

added, 100 times is a bit low, but if there is a consensus about that I'm ok with it

Maybe you can change this PR to not render barrier=ditch when in combination with a waterway tag that is rendered

Interesting, never seen this before. Added an exception for waterways.

@nebulon42 nebulon42 force-pushed the line-barrier-catchall branch from a31a471 to d12cbd8 Compare November 29, 2015 20:30
@polarbearing
Copy link
Contributor

Values less than 1000 should be on consensual selection, without numerical limit. Thus useful new values can be added, while wrong spelling and tagging for the renderer can be suppressed. Just created a deprecation page for barrier=curb (595) as an AE spelling of kerb (44915).

@dieterdreist
Copy link

2015-11-29 19:57 GMT+01:00 Christoph Hormann notifications@github.com:

Maybe you can change this PR to not render barrier=ditch when in
combination with a waterway tag that is rendered (i.e. stream/drain/ditch).

+1. Any waterway will be in a depression (or in a man made tube / "open
tube"), so this combination seems weird anyway. If there is a significant
cutting or lateral embankment/slope, it should be mapped like that.

@dieterdreist
Copy link

yes 47 152 no wiki page, too general

yes, it's generic (and though not too helpful without further information)
but is OK IMHO (as a preliminary mapping).

hedge_bank 14 508 only german wiki page available

how does this relate to barrier=hedge?
IMHO we should generally develop a way to map barriers that consist of
several layers (e.g. retaining wall, above a fence, etc.), maybe with a
linear relation (to reuse the geometry and avoid drawing overlapping ways)
and layer tags.

wire_fence 10 379 no wiki page, should be fence

+1

embankment 4 599 no wiki page, but included since rendering rule already
present

what's the difference between this and a man_made=embankment? If it's very,
very steep, it will be a retaining wall or a cliff, otherwise it is an
embankment like any embankment, not?

wood_fence 2 171 no wiki page, should be fence

+1

@HolgerJeromin
Copy link
Contributor

I dont know if everybody is able to read the german wikipage to barrier=hedge_bank.
It is kind of barrier=hedge+embankment=yes.

@nebulon42 nebulon42 force-pushed the line-barrier-catchall branch from d12cbd8 to 35b70ad Compare December 6, 2015 13:18
@nebulon42 nebulon42 force-pushed the line-barrier-catchall branch from 35b70ad to 6567eeb Compare December 6, 2015 13:53
@nebulon42
Copy link
Contributor Author

There was an error in the SQL related to waterways, which I have fixed.

@matthijsmelissen matthijsmelissen merged commit 6567eeb into gravitystorm:master Feb 9, 2016
@matthijsmelissen
Copy link
Collaborator

Thanks!

@nebulon42 nebulon42 deleted the line-barrier-catchall branch February 9, 2016 17:42
@nebulon42
Copy link
Contributor Author

@math1985 @pnorman @matkoniecz @gravitystorm I plan to inform the community (especially the German one) beforehand of this change to minimise a probable backlash. When a good tagging scheme for e.g. barrier=line is established re-adding sports field markings to this style should be no problem.

Objections, thoughts?

@imagico
Copy link
Collaborator

imagico commented Feb 14, 2016

My opinion is that explicit mapping of sports field markings is not a good idea and should not be encouraged by renderers. The problem is of course that the alternative, i.e. rule based rendering of markings based on a generic sport type tagging of fields (#1126) is complex to implement within the current framework here.

@matthijsmelissen
Copy link
Collaborator

I plan to inform the community (especially the German one) beforehand of this change to minimise a probable backlash.

Thanks, good idea!

When a good tagging scheme for e.g. barrier=line is established re-adding sports field markings to this style should be no problem

I think the discussion whether or not to map field lines is out of the scope of the repository. We could discuss whether we want to render them, independently of the tagging scheme. I suspect the answer to that question would be 'no'.

@matkoniecz
Copy link
Contributor

I plan to inform the community (especially the German one) beforehand of this change to minimise a probable backlash.

Great idea, thanks for doing it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants