gravitystorm / openstreetmap-carto

A general-purpose OpenStreetMap mapnik style, in CartoCSS
Other
1.53k stars 818 forks source link

Regression in office rendering #3904

Open andrzej-r opened 5 years ago

andrzej-r commented 5 years ago

Until recently offices were rendered from z17. This has been changed to z18+ and z19+ for office=yes. There are several problems with it:

  1. At z17 and z18 offices are rendered as addr:housenumber labels. This is a regression: #108, #2953.
  2. The office=* PoIs are now much less prominent than an alternative solution of tagging office names as building names. This creates incentive to tag offices as building names.
  3. Rendering of offices and shops is inconsistent with each other (shops are more prominent).

image

jeisenbe commented 5 years ago

Could you add a link to the location of the screenshot?

At z17 and z18 offices are rendered as addr:housenumber labels.

All nodes with a tag of addr:housenumber are rendered at z17 and higher, to show address numbers which are often tagged as a separate node (for example, on an entrance). Also, all building areas with addr:housenumber have the number shown, if there is no building name.

So this means that a node with craft=*, an unusual value of amenity=, or any tag that isn't rendered, will have an addr:housenumber rendering at z17 and higher.

Rendering of offices and shops is inconsistent with each other (shops are more prominent)

Most shops are more useful for a general map user than most offices, since shops are supposed to sell retail products or services, while offices do not.

jeisenbe commented 5 years ago

There are 3 options, I think: 1) Rendering office nodes sooner (partial revert of #3796) - however this problem will still be seen with other tags like tourism=apartment 2) Do not render addr:housenumber at z17 when tagged with office= (and for other tags which are not rendered at z17, for example tourism=apartment, amenity=toilets, man_made=storage_tank etc) 3) Change initial zoom level for rendering address nodes to z18, instead of at z17.

kocio-pl commented 5 years ago

I think that office=yes on z19+ is OK, since it gives a feedback that it's an office, but it is pushing people to give more specific value, which is easy, because we render most of the values and the "any tag you like" principle is still there. This is imperfect by design, exactly to encourage people to be more precise.

On the other hand pushing all the office values later does not convince me. These are mere dots and I don't see they make a visual problem or that they are that small or unimportant - offices can be as big and known as shops. This is what I think does not work too good.

andrzej-r commented 5 years ago

I think prominence of addresses, building names, shops, offices, etc should all be matched to minimise temptation of using more prominent tags. Yes, some shops are more important than offices and but there are also offices that are more important than shops. I would upgrade some tag-values but keep the bulk of them at the same level.

Separately, we shouldn't render addresses of pois that are already rendered via another code path. At best it slows down rendering (more data to process in mapnik) but it can also result in issues like this.

Kocio, I strongly disagree with using this style to "encourage" use of certain tags. It is not up to you to decide what and how is mapped. We ask people not to map to a renderer but that assumes the renderrer stays neutral.

kocio-pl commented 5 years ago

It is not up to you to decide what and how is mapped. We ask people not to map to a renderer but that assumes the renderrer stays neutral.

This problem is more complex than it appears to the eye.

In general tags don't have the look, only the meaning (unless this is color=red for example), so one has to find some way of rendering, which is exactly the act of deciding. They also don't have implicit zoom level - it is just not a physical property, it is only a tool for creating typical maps (it is possible to do a map with a sliding scale for example). And this is exactly why it is our job here to decide many things.

The next thing is that a database has 76423 keys (see https://taginfo.openstreetmap.org/keys ) - it is not doable to render all of them, which makes it impossible to truly stick to the "any tag you like" principle.

At the same time I understand that you want to stay as neutral as possible when there are no technical or visual obstacles. I'm close to your opinion, I'm especially glad that you see the "dark number" of tagging for rendering when something is not rendered, because this problem is being skipped when making decisions. I don't see anything wrong with less appealing look when data are sparse and more appealing if it is rich, but I'm also OK with more neutral take.

jeisenbe commented 5 years ago

Any thoughts on the 3 options for fixing this:

  1. Rendering office nodes from z17 (partial revert of #3796) - however this problem will still be seen with other tags like tourism=apartment
  2. Do not render addr:housenumber at z17 when tagged with office= (and for other tags which are not rendered at z17, for example tourism=apartment, amenity=toilets, man_made=storage_tank etc)
  3. Change initial zoom level for rendering address nodes to z18, instead of at z17
jeisenbe commented 4 years ago

We still need input from other contributors about the 3 options above

Adamant36 commented 4 years ago

I say go with 3. Given that address numbers are "often tagged as a separate node (for example, on an entrance)." It would include rendering in situations where a building has multiple addresses that are mapped as nodes close to each other. Like on condos or apartment complexes. As it is they block each other out and make it look like there's only one address.

imagico commented 4 years ago

I'd go with

  1. Fix the point label and POI symbol prioritization problem (#3880) in general.

In the meantime i would suggest to either drop offices from rendering or move them to only show at the highest zoom levels with lower priority.

kocio-pl commented 4 years ago

We have multiple issues to fix with different objects, but that does not justify things like removing them or simply pushing them (which will make the regression even larger). I consider loosing feedback of quite popular objects to be much bigger flaw than priority problems.

imagico commented 4 years ago

I strongly disagree - if a feature that has been added does not work properly in rendering it should either be removed or changed in a way that does not negatively affect the rendering of other features that have been rendered before (like addresses in this case).

The categorical refusal to consider any feature removal in this style is one of the main sources of problems we have. We need to be much more bold in that regard to ensure a sustainable cartographic quality here. Things - especially those which have been added more recently - which do not work properly but no one is able or willing to fix should be scrubbed.

Adamant36 commented 4 years ago

Darn it, I didn't notice an option 4. Oh well 🤷‍♂

kocio-pl commented 4 years ago

Going with this to extreme: please look first how many problems we have with roads for example - 102 open issues currently: https://github.com/gravitystorm/openstreetmap-carto/issues?q=is%3Aissue+is%3Aopen+label%3Aroads. It looks like a huge mess (much bigger than with the offices), though "scrubbing" them or for example simply moving them to the z19 does not seem like a sane idea to me, even if that would be proposed as a temporary solution, until it will get fixed eventually.

I think that we might consider dropping addresses rendering for objects with office tag (and maybe some other that we render too).

jeisenbe commented 4 years ago

Fix the point label and POI symbol prioritization problem (#3880) in general.

I'd like to fix #3880 but I'm uncertain how to address it from a technical standpoint. An example or PR would be helpful. I don't think it has been addressed in the alt-colors fork?

  1. drop offices from rendering or move them to only show at the highest zoom levels with lower priority.

The previous PR #3796 moved offices from z17/z18/z19 to just z18 (small filled circle icon / text of documented values ) / z19 (text of undocumented or rare values), so your suggestion has already been partially implemented.

But the original comment by @andrzej-r and the prior issue #2953 consider it a problem to have address points shown when a feature is also tagged with office=.

We will need to decide how to deal with this when fixing #3880, so I think it's still good to think about now.

Should addresses be moved one zoom level later, or do we need to have all other features start at z17 too, so that we don't get the issue of addresses rendered at z17 but then an icon or dot at z18 and z19?

imagico commented 4 years ago

@kocio-pl - Whataboutism does not help here - the road rendering system has its own very specific problems due to the very nature of roads themselves and the limitations of the rendering framework in that regard. This is nothing new, it has been a problem from when this project started and we are mostly having productive and non-dogmatic discussions about it. All of this is not significantly affected by more recent feature additions emphasizing the problems.

So again: Trying to move stuff around in an attempt to hide the big problems without actually addressing them is not going to be helpful in providing sustainable cartographic quality in this style.

jeisenbe commented 4 years ago

I didn't notice an option 4. Oh well 🤷‍♂

All contributors are always welcome to suggest additional solutions. I just included the 3 options that I had thought of myself, but that does not mean that there are no other options.

imagico commented 4 years ago

@jeisenbe

The previous PR #3796 moved offices from z17/z18/z19 to just z18 (small filled circle icon / text of documented values ) / z19 (text of undocumented or rare values), so your suggestion has already been partially implemented.

No, since it keeps the rendering in the main high priority POI layer and thereby aggravates the problem of #3880. Moving to lower priority like the other highest zoom level only POIs (like bench, waste_basket) would be essential for this to work.

I am strongly against sacrificing intuitive and useful rendering of addresses in order to be able to keep rendering office POIs without addressing the base issues of POI rendering first. Making the rendering of addresses depend on the presence of a different and unrelated tag on the same feature would be highly non-intuitive.

kocio-pl commented 4 years ago

Showing the same addresses (numbers) for the building and objects inside does not look intuitive at all for me, this is one of the oldest bug I see in this style.

This is initial idea that might need some tuning.

kocio-pl commented 4 years ago

@jeisenbe What do you mean by "whataboutism"? I do not propose different scenarios, quite the opposite - I have shown why the "fixing by removing (or neglecting)" idea fails. If the problem was introduced lately, it can be reverted.

imagico commented 4 years ago

https://en.wikipedia.org/wiki/Whataboutism

If there is an issue with the current address rendering that should be a separate issue. If the same address exists twice in the data that is however a mapping error and not a rendering problem.

jeisenbe commented 4 years ago

Showing the same addresses (numbers) for the building and objects inside does not look intuitive at all for me, this is one of the oldest bug I see in this style.

Right, that is still an issue either way, since there are many tags which we do not render. I searched but I did not yet find a specific issue about that problem, though it is mentioned here and in #3861 and perhaps some other comments.

Would you mind opening a new issue with some good examples of where it is a problem to show the addr:housenumber on both the building and POI nodes within?

Edit: sorry, I didn't see the previous comment before posting this

jeisenbe commented 4 years ago

Moving to lower priority like the other highest zoom level only POIs (like bench, waste_basket) would be essential for this to work.

This would put the office icon at lower priority than addresses, so when there was an addr:housenumber listed, this would render instead of the office dot.

kocio-pl commented 4 years ago

Here is this old, still unresolved addressing bug (it's not about bad tagging): #962. Removing or pushing offices down makes it bigger. #108 is even older and would be directly affected by removing offices.

jeisenbe commented 4 years ago

I saw those issues, but we don't have an issue specifically about the problem you mentioned above:

"Showing the same addresses (numbers) for the building and objects inside"

kocio-pl commented 4 years ago

I gave the example here: https://github.com/gravitystorm/openstreetmap-carto/issues/962#issuecomment-69221306 (see https://www.openstreetmap.org/way/32256425 ). It's actually not showing the building number, only multiple numbers of nodes, but it's the same kind of problem.

imagico commented 4 years ago

I am not sure what #962 is trying to call for except sacrificing meaningful rendering of addresses at z17/18 for the purpose of rendering more POI types. If you saturate the map with high priority POIs as z17/18 all the layers coming afterwards in the style become essentially meaningless. And the problem of #3880 would still remain above all of this.

This would put the office icon at lower priority than addresses, so when there was an addr:housenumber listed, this would render instead of the office dot.

Indeed. Address rendering starts at z17 and all POIs starting later will need to have lower priority than addresses for this to make any sense and being intuitively understandable for mappers. If you however start office rendering at z17 or earlier you would - as mentioned above - contribute to saturating the map with POIs and address rendering would become meaningless because they would only appear at the rare chance there is a gap in POI presence.

And again - we do not support mapping the same address several times in violation of basic OSM principles - Even if we'd want to support that there is no way we could efficiently and reliably perform de-duplication of address data on the fly.

kocio-pl commented 4 years ago

Could we please move the addressing problems to the proper ticket, once I have found it?

andrzej-r commented 4 years ago

Potential solution: How about not rendering addresses (addr:housenumber, addr:housename) when name/amenity/shop/office etc tags are defined? In these cases, addr:* tags are not of primary interest to the users - these would be the type of amenity or the name tag. Addr:housenumber/housename should be rendered if there are no other tags (the object is an address point) or in connection with tags like building or entrance. This would fix an immediate issue with displaying spurious house numbers like in the first picture in this ticket.

Separately, I would still encourage matching prominence of generic name/amenity/shop/office tags to prevent gaming and mapping to the renderer. For example, we are currently strongly encouraging users to map POIs as building+name tags, even if better suited and more specific tags exist.