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 rendering of barrier=kerb #3969

Merged
merged 3 commits into from
Jan 28, 2020

Conversation

jeisenbe
Copy link
Collaborator

@jeisenbe jeisenbe commented Nov 11, 2019

Closes #3714

Changes proposed in this pull request:

  • Remove the rendering of barrier=embankment
  • Remove the rendering of barrier=kerb

Explanation:

  • Kerbs (aka curbs) are rendered the same as significant obstacles like walls and fences. In issue Different rendering for barrier=kerb #3714 I attempted to find a subtle rendering for kerbs which would not look like a fence or wall, but failed to find a good option for z19. Without rendering sidewalk and street areas these features are not intuitive.
  • The tag barrier=embankment was once somewhat common but never was documented in the wiki. Instead man_made=embankment or man_made=dyke are the much more common ways to make one-sided embankments and levees. The definition of barrier=embankment was not clear, so it's best to no longer support this tag. (See Stop rendering barrier=embankment #3744)

Test rendering:

(Test shows fence, kerb, wall, embankment from top to bottom)
Before
kerb-embankment-test-before

After
kerb-embankment-barrier-after

@Adamant36
Copy link
Contributor

Adamant36 commented Nov 11, 2019

How about some real world examples (mainly for curb)? Also, is there even another prominent style out that renders curbs? I don't even think OsmAnd does.

@jeisenbe
Copy link
Collaborator Author

https://www.openstreetmap.org/#map=16/31.9764/10.6907
barrier=embankment in Libya - there is a barrier=ditch parallel to it which still renders

before
barrier-embankment-before

after
barrier-embankment-after

https://www.openstreetmap.org/#map=19/43.60627/3.87779
barrier=kerb in Montpellier, France

before
barrier-kerb-before

after
barrier-kerb-after

@Adamant36
Copy link
Contributor

Yeah, looks better without them. No big surprise there though, but thinks for the test renderings anyway.

@polarbearing
Copy link
Contributor

I disagree with removing them in the same PR. The reasons are completely different.

  • barrier=embankment removal is fine for me, since is is a mistagging and deprecated
  • kerbs are not deprecated, and are a physical barrier, see my reasoning in Different rendering for barrier=kerb #3714, they should not be removed.

@Prince-Kassad
Copy link

a curb is no more a physical barrier than a stairwell is, except possibly to wheelchair users but we don't even distinguish between high curb and low curb so that's pretty useless. OpenStreetMap is not really suited to routing for wheelchair or similarly impared people (elderly people for example) and it's not a trivial thing to do.

If you want an extreme example look at Neu-Isenburg: https://www.openstreetmap.org/#map=19/50.05338/8.68732&layers=N It makes the city look like some kind of walled city.

@polarbearing
Copy link
Contributor

That Neu-Isenburg example has other weird mapping, e.g. mapping small green patches on the sidewalk as 'garden'.
However it shows that if we remove kerb but keep rendering parking_space, we would render lines painted on the road but ignoring barriers set in stone.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Nov 11, 2019

I tried a number of different renderings for barrier=kerb above (1, 2, 3 4), some of which required extensive changes to the rendering of other barriers, but none worked well.

Currently kerbs are rendered the same as retaining walls, walls and fences, which are mostly tall barriers that prevent entry by pedestrians. Since barrier=wall and barrier=fence are used millions of times and barrier=kerb is rare uncommon, the cases when barrier=kerb is rendered are going to be interpreted by most map users as a fence or wall. This is misleading and should be fixed.

@Prince-Kassad
Copy link

However it shows that if we remove kerb but keep rendering parking_space, we would render lines painted on the road but ignoring barriers set in stone.

I don't think I need to say I'm not a fan of rendering parking spaces like this. It kind of reminds me of back when barrier= was rendered regardless of value, and people used barrier=line to draw the white lines of soccer fields so it shows as a soccer field on the map instead of just green square. We got rid of that for a reason.

@polarbearing
Copy link
Contributor

barrier=kerb is rare

225000 uses is not 'rare'.

misleading and should be fixed

rendering it later would fix it.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Nov 11, 2019

rendering it later would fix it.

Rendering it only at z20, but still the same as barrier=wall, would still be highly misleading at z20. I attempted a subtle distinction between barrier=kerb and the other tags in #3714 (comment) but this did not work well.

At z19 the current rendering is poor (I did not hand-pick this example, it was the first place I found in Montpellier with kerbs):

barrier-kerb-before

225000 uses is not 'rare'.

OK, you caught me (I didn't check the numbers) :-). It would be better to say they are "uncommon" compared to fence+wall+other barriers rendered in the same way. Note that when I first looked for barrier=kerb features, I did not find very many:

"There are none mapped in Prince Edwards Isle (Canada) and only 3 in Liechtenstein. Delaware, USA has only 1. Washington, DC has a couple dozen, but all of them are also part of another feature. Luxembourg was the first place I found that had some kerbs mapped as separate features"

This means that many map users are not going to be used to seeing them. Rendering them only at high zoom levels would increase this problem.

@kocio-pl
Copy link
Collaborator

kocio-pl commented Nov 11, 2019

Rendering it only at z20, but still the same as barrier=wall, would still be highly misleading at z20.

I don't think there is a chance of such confusion. Rendering black line has multiple uses and using zoom level context helps here (for example train line and tram line has this distinction). There is also common life observation of wall not being built exactly on the edge of the road.

@jeisenbe
Copy link
Collaborator Author

wall not being built exactly on the edge of the road.

Many of the currently-rendered kerbs are not right on the enge of the road (and at low to mid lattitudes most kerbs right on the side of the road are covered by the highway rendering). Example where kerb is about the same distance from the road as a fence:

z19-rue-carlo-current

Kerb on northeast (upper left), fence on southeast (lower right) side.

Here's the test I tried with slightly wider walls/fences and slightly narrower kerb:

z19 after - 0.3 pixel wide kerbs (center, north, west), 0.6 pixel wide fence (southeast)
z19-rue-carlo-after2

It is not clear that there is kerb in the upper left and a fence in the lower right; one would be more inclined to guess fence vs wall perhaps.

I also tried this more complicated change, with a new fence/wall rendering as well:

z19-kerb-halo-fence-dots

But it doesn't work well because kerbs are not consistently mapped in a certain direction

@kocio-pl
Copy link
Collaborator

In the given example there are trees outside the fence, you would not expect them on the road.

But it doesn't work well because kerbs are not consistently mapped in a certain direction

I don't get why the certain direction is needed here?

Just making kerbs lighter like on the last eample (no need to change fences closer to the walls) and rendering them on z19+ would make it different enough. You skipped the core of the comment about zoom level context.

@kocio-pl
Copy link
Collaborator

There are enough context tips from this location you don't show by skipping z18 rendering - the fence is attached to a building, there's a gate and private road leading inside; certainly not a kerb properties:

Screenshot_2019-11-12 OpenStreetMap

I think that kerb on z18 is still too detailed feature, but moving it to appear on z19 together with lighter rendering is good enough combination.

@jeisenbe
Copy link
Collaborator Author

I don't get why the certain direction is needed here?

Note the inconsistency between the curb in the middle, and the ones to the north around the grassy area: the white lines on opposite sides (it should be on the outside, or lower side, of the curb in both cases, theoretically).

Just making kerbs lighter like on the last example

Do you mean the second to last example?

(no need to change fences closer to the walls)

Currently barrier=fence and barrier=wall (and barrier=retaining_wall) are all rendered identically, and in the tests above that is still the case.

But note that none of my ideas above achieved consensus, see critical comments: #3714 (comment), #3714 (comment) (matkoniecz), #3714 (comment) (imagico), and my own comments after the last tests.

core of the comment about zoom level context.

2 other contritubors agreed that this feature (barrier=kerb) will only work for rendering on z20. Currently we are not rendering any features at z20 only, since this is not used on openstreetmap.org yet, and I believe we need to wait until there is a possibility of z20 being used before reconsidering rendering barrier=kerb at that level.

@jeisenbe
Copy link
Collaborator Author

there's a gate and a private road leading inside

Right now there is no established way to tag a "curb cut", or a place where wheelchair and bicycle/scooter users can roll across a curb. This is one of the problems of rendering kerbs currently. See #3969 (comment) and #3714 (comment) which mentioned this issue with kerbs.

@kocio-pl
Copy link
Collaborator

2 other contritubors agreed that this feature (barrier=kerb) will only work for rendering on z20.

There is also a #3969 (comment) about just different rendering (though I'm not sure what initial zoom level @polarbearing meant).

Right now there is no established way to tag a "curb cut", or a place where wheelchair and bicycle/scooter users can roll across a curb.

We talk about rendering, not routing, which has it's own problems (the existence of a gate does not automatically inform you if it's passable nor if it's not passable). There are plenty of lacking gates on the map and that is just incomplete, not wrong.

Do you mean the second to last example?

I mean "z19 after - 0.3 pixel wide kerbs (center, north, west)" example.

Currently barrier=fence and barrier=wall (and barrier=retaining_wall) are all rendered identically, and in the tests above that is still the case.

I meant just no need only in the context of kerb. There might be different reasons to change them and that would be fine. "No need" does not mean "they can't be changed".

Note the inconsistency between the curb in the middle, and the ones to the north around the grassy area: the white lines on opposite sides (it should be on the outside, or lower side, of the curb in both cases, theoretically).

What white lines are you talking about? Did you use them instead of just a lighter shade of grey? I would not try composing lines here at all.

@Adamant36
Copy link
Contributor

a curb is no more a physical barrier than a stairwell is

@Prince-Kassad, they are in parking lots. Like for instance in this one that I probably over mapped, but it gives some visual context of where not to drive, or something.

Curb

Without rendering sidewalk and street areas these features are not intuitive.

@jeisenbe, it seems pretty intuitive with just sidewalk and street ways. Know one will confuse these examples for walls for fences. Its pretty obvious from the context that they are curbs.

Curbs and sidewalks

Garden

I like the way the curbs in this case kind of create a visual border for the space of the theater from the other stuff around it. In some cases, like this one and the parking lot example, they add something to the map.

Theater

@imagico
Copy link
Collaborator

imagico commented Nov 12, 2019

I support:

  • stopping rendering barrier=embankment due to it being widely considered an undesirable synonym for man_made=embankment.
  • stopping rendering of barrier=kerb in the same styling as other barriers due to this being highly misleading and against our documented goals.
  • stopping rendering barrier=kerb for the moment waiting for a convincing suggestion how to render it and arguments why we should render it.

@polarbearing
Copy link
Contributor

I propose:

  • splitting the PR, thus stopping rendering barrier=embankment which is undisputed
  • keeping barrier=kerb while discussing how to change rendering. I find dropping it for a while and then introducing differently counter-productive.

@kocio-pl
Copy link
Collaborator

Thank you both for concise propositions.

@jeisenbe In general it is always good to separate features. I understand that it's easier to write one ticket for a short and similar code, and in some cases that is OK, but you never know what will be the problem to be discussed.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Nov 15, 2019

I don't think that removing barrier=embankment from this discussion will make a significant difference to our decision, but to avoid the distraction I have pushed a new commit which removes the changes to barrier=embankment, so we can focus the discussion on barrier=kerb.

I would be happy to see other rendering ideas for barrier=kerb. So far I spent at least 8 hours trying to adjust the various barrier features to find a reasonable rendering for kerbs at z19 which could not be confused with other barriers, but was still clearly visible, but I was not successful.

If there are no new idea, then we should remove the rendering for now, until a successful rendering option is found. As I've mentioned, I think this is much more likely at z20, in concert with showing area:highway= features, which would make curbs much more intelligible in context.

@imagico
Copy link
Collaborator

imagico commented Nov 15, 2019

keeping barrier=kerb

Obviously this is what will happen if there is no consensus but as mentioned before this would actually be in clear conflict with the goals of this style because rendering kerbs the same way as physical barriers is misleading mapper feedback.

In other words: Our inability to achieve consensus on questions like this results in an inability to pursue the goals and the mission of this project.

@jeisenbe jeisenbe changed the title Remove rendering of barrier=kerb and barrier=embankment Remove rendering of barrier=kerb Nov 15, 2019
@jeisenbe
Copy link
Collaborator Author

rendering kerbs the same way as physical barriers is misleading mapper feedback.

It also interferes with map Legibility and clarity when very different features are rendered identically.

"The map should be intuitively readable by users with some general experience using maps without a map key, preferrably with relatively little effort" - Line 34, CARTOGRAPH.md

@polarbearing
Copy link
Contributor

Sure it could, but it will take a while. In the meantime, we are replacing a minor discrepancy (rendering small and high barriers equal) with a larger one (rendering a painted line but suppressing a stone feature).

@jeisenbe
Copy link
Collaborator Author

4 out of 5 maintainers of this style who have commented have approved of this PR, and of the other 3 contributors who commented 2 are in favor.

I've tried a number of different renderings, in good faith (I wanted to be able to render kerbs) without success, and I don't have any other good ideas for how this feature can be clearly rendered in a way that cannot be confused with significant linear barriers like fences and walls.

I believe this is rough consensus to remove the rendering of barrier=kerb. Of course anyone is welcome to submit a PR with a new type of kerb rendering, especially if it is related to a significant redesign for z20 or z21 and above where it would be more feasible.

@jeisenbe jeisenbe removed the consensus needed Indicates the lack of consensus among maintainers blocks a PR/issue label Jan 12, 2020
@jeisenbe jeisenbe requested a review from imagico January 12, 2020 13:48
@kocio-pl
Copy link
Collaborator

The only thing that IIRC is currently shared by everyone is that it does not need to be rendered before z19, removing is not even proposed in some last comments, so this PR does not work for any of them, so it should be closed anyway.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Jan 12, 2020

That's an interesting interpretation of:

  1. "I fully support removal of barrier=curb rendering" @matkoniecz

  2. "👍 in principle to removing barrier=kerb rendering" @pnorman

  3. "I support: stopping rendering of barrier=kerb in the same styling as other barriers due to this being highly misleading and against our documented goals." @imagico

  4. Myself "Kerbs (aka curbs) are rendered the same as significant obstacles like walls and fences. In issue Different rendering for barrier=kerb #3714 I attempted to find a subtle rendering for kerbs which would not look like a fence or wall, but failed to find a good option for z19. Without rendering sidewalk and street areas these features are not intuitive."

  5. "a curb is no more a physical barrier than a stairwell is" @Prince-Kassad

  6. " I put some thought into this and I'm changing my vote to remove. The reasons are ..." @Adamant36

@imagico
Copy link
Collaborator

imagico commented Jan 12, 2020

removing is not even proposed in some last comments

No @kocio-pl - this is not how we have discussions here - reducing the view to a selection of latest comments and thereby re-iterating every discussion ad infinitum is not how we work. We have had an extensive discussion here and in #3714. There was plenty of opportunity for everyone to make different PRs to resolve #3714. And everyone still would have the change for such with this PR merged of course.

I agree with @jeisenbe that consensus based development here can only work if we all put the goals of the project above our personal preferences and accept decisions against those if we are not able to provide convincing arguments for our point of view.

@kocio-pl
Copy link
Collaborator

kocio-pl commented Jan 12, 2020

reducing the view to a selection of latest comments

IIRC reducing rendering was exactly what you have supported last time I checked.

There was plenty of opportunity for everyone to make different PRs to resolve #3714.

And this was one of the failed attempts. We have a lot of unresolved problems, so I see nothing special about this one.

Copy link
Collaborator

@kocio-pl kocio-pl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make it absolutely clear - this one is a blocker for me, no convincing arguments for removing were given up to this point.

@Prince-Kassad
Copy link

Abusing the "request changes" feature just to prevent a pull request one doesn't like from getting merged is something that I would report to @gravitystorm or perhaps even GitHub admins.

Copy link
Collaborator

@imagico imagico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and works as intended and as previously explained i agree that due to the issues with the current rendering of barrier=kerb (see #3714) and the lack of any viable ideas of solving these issues in a different way removing the rendering for now seems a good idea.

I agree with @jeisenbe it would be good to merge this despite the sustained objection by @kocio-pl. Reasoning behind this from my side:

  • there is an overwhelming majority among the active maintainers for this change.
  • in terms of arguments the situation is similar - a lot of clear and well supported points have been raised why the current rendering is not viable and not in support of the goals of this style and many of these points have been argued for by multiple people from multiple sides while there is a quite clear lack of willingness to argue for and defend the status quo in rendering beyond the mere expression of the desire to do so.
  • there has not even been a well founded and argued original decision to add rendering this feature type (Different rendering for barrier=kerb #3714 (comment)).
  • we are talking about a feature removal here - which as discussed in Feature removal policy #3977 should, for productive improvement of this style, not be subject to too high hurdles. In case of disagreement on rendering something or not rendering something having a default of not rendering is a useful policy.
  • this change would not prejudice any alternative solutions for the issue in the future.

We have discussed the general matter of consensus decision making in #3951 and any principal discussion on the matter should happen there. I would very much prefer it if in such cases the dissenting opinion would actively accept a rough consensus (examples were given in #3951 (comment)) but it should be possible to come to decisions also otherwise.

@Prince-Kassad - a 'request changes' review is an accepted indication for disagreeing with merging a PR here - so nothing wrong with doing that in principle.

@kocio-pl
Copy link
Collaborator

You are referring to discussions that did not yet even ended, let alone ended with finding common ground, so it lacks some backing and I get it as your personal view. I plan to show what fundamental problems are with removing features, which make it exactly counterproductive when looking at the goals of this style. (There are cases where it makes perfect sense, but this is not the one.)

I also don't agree with "abusing". Being clear is what is needed to decide.

@pnorman
Copy link
Collaborator

pnorman commented Jan 19, 2020

Reviewing this issue, there is a consensus about merging it. @kocio-pl disagrees with removing it, but a consensus does not require everyone agree. My reason for not merging it at this time is primarily that I have not tested the code myself.

Also, for the record, a requested changes review is entirely appropriate here - it makes the status clear.

@pnorman pnorman merged commit 1c09169 into gravitystorm:master Jan 28, 2020
@tordans
Copy link

tordans commented Feb 9, 2020

Hey guys, I now understand why I don't see my changes on the map anymore :-). The rendering for barrier=kerb was very helpful to us in Berlin where we use the tag for small structures on roads but also to show where a block of houses is. It helps to show where the sidewalk (…) ends and the car-road starts. See https://overpass-turbo.eu/s/QzA for more cases.

I hope there will be a new rendering in the future. In the meantime, we will have to review our tagging :-).

@nickvet419
Copy link

nickvet419 commented Feb 10, 2020

I hope for a new rendering also. I had also been using them in downtown Chicago to mark the many curbed areas in the city, including the road medians that separate the direction of travel and create a protected island in crosswalks. Without the renderings it creates a lack of detail.

@Adamant36
Copy link
Contributor

@nickvet419 There's a couple of open issuses about rendering traffic islands. Maybe you comment on those. Mapping curbs shouldn't be a substitute for using the proper tags for those things just because it rendered anyway. Its better we get traffic islands rendered instead.

@jaffroy62
Copy link

jaffroy62 commented Mar 8, 2020

I can not understand why you removed the rendering of barrier=kerb. For me it is in some cases a useful structure that can help the user to understand the map.

The removal now is not an improvement but rather a step back in my eyes. Why don't come up with a better suggestion (which have already been presented here) but rather remove it?

How can the rendering be brought back? Arent 120-thousand of barrier=kerb on ways not entough arguments?

@Adamant36
Copy link
Contributor

@jaffroy62, did you look through the discussion? Unfortunately there wasn't a usable way to make them look different then regularly fences. The amount of them mapped doesn't really matter if them being rendered potentially leads to confusion. Perhaps someone can come up with a usable solution at some point though so they can be rendered again.

@nickvet419
Copy link

nickvet419 commented Mar 9, 2020 via email

@Adamant36
Copy link
Contributor

How do you think fences should look instead? The problem with changing how they look is that since they look the same as curbs did we are probably confined to the same solutions that already didnt work for them. There's really only so many ways you can render somthing like until its not intuative anymore and most ways lines can be rendered are already done with other thigs. Not that a lot of them would work for showing fence or curbs anyway.

@kocio-pl
Copy link
Collaborator

kocio-pl commented Mar 9, 2020

It seems to be enough in #4063 to show the lack of a fence by making the line lighter. Just that, no changes in fence rendering are proposed there.

@nickvet419
Copy link

I would go with something like this to represent fencing to solve many issues relating to fences.
New Bitmap Image There are a few issues this would fix to help differ them between quarry outlines, kerbs, walls etc... I would also make walls a little thicker for the same reason.

@jaffroy62
Copy link

@jaffroy62, did you look through the discussion? Unfortunately there wasn't a usable way to make them look different then regularly fences....

@Adamant36 Thank You for the reply. Yes i did look through the discussion(s) of course but not in close detail. But i have already recocnized the problem of the wish of different apearances. From my perspective the logical task would have been to resolve the ambiguities by changing the appearances rather then remove them.

As You probably know, the apearance of some other barriers is ambiguous too (see: https://wiki.openstreetmap.org/wiki/Key:barrier)

For me it yould be for example also a good suggstion to use dotted or dashed lines for some of them. I think this alsready has been suggested elsewhere in this discussion. Different colors could also be an option.

At least i am very happy about the movement that has come now again into this issue.

Greetings!

@Adamant36
Copy link
Contributor

@nickvet419, looks like a path or railroad track to me. Walls could maybe be a little thicker. I think there was an issue about that. Quarry outlines are being dealt with through lightening them I think. Different colors might work, but most colors are already taken and there's a general hesitation with the style to reuse colors for different elements.

jaffroy62, yep. I agree some other barriers are also ambagious, but you have to start somewhere and this had the advantage of not really being a barrier in the first place. Maybe when vector tiles come around, which should allow for different map layers, we can have a barrier layer.

@swedneck
Copy link

I agree with @nickvet419 that making it lighter would help, maybe it could also have triangles (or some other marker) like in JOSM/iD?
image

@kocio-pl
Copy link
Collaborator

Clever observation! Since this is a new idea and this code is already merged, so it makes no sense to continue discussion here, could you please open a fresh ticket for this?

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

Successfully merging this pull request may close these issues.

Different rendering for barrier=kerb