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

Increase "office=" initial zoom to z18 and move deprecated values to z19 #3796

Merged
merged 3 commits into from
Sep 20, 2019

Conversation

jeisenbe
Copy link
Collaborator

@jeisenbe jeisenbe commented Jun 4, 2019

Related to #3794

Changes proposed in this pull request:

  • Increase office= initial zoom to z18 for documented values
  • Remove office=physician, office=therapist, and office=administrative from the list of documented values that are rendered at z18

Explanation

In issue #3794 it is noted that "the office dots render before or at the same level as the buildings and the building name gets "covered" at z17, where most office= names are not yet rendered.

This PR moves the initial rendering of the dot marker from z17 to z18, and all frequently-used, documented values will have a name label rendered at z18. Currently a small subset are rendered at z17.

The three deprecated tags office=physician, office=therapist, office=administrative were removed from the explict list of documented values which are rendered at z18, but they will still render at z19.

The subset of office values that currently rendered at z17 will now render at z18. The dot markers were removed from z17.

After this PR, dots will still be shown for shops and restaurants at z17, but the office dots will not be shown at this level. This is reasonable, because while a concentration of shop/restaurant dots can be helpful to find an area for shopping or eating, a concentration of unspecified offices without names is not helpful to the general map user.

Test rendering with links to the example places:

Singapore Parliament House, z17 - before
https://www.openstreetmap.org/#map=17/1.2880/103.8497
z17-parliament-house-office-before

z17 after - Parliament House building name visible
z17-parliament-office-after

z18 is unchanged:
z18-parliament-office-after

Telok Ayer, Singapore z17 - before
z17-telok-ayer-before-office
z18 before
z18-telok-ayer-office-before

z17 after - building name visible (The Octogon)
z17-telok-ayer-singapore-office-after
z18 after
z18-telok-ayer-singapore-office-after

…istrative

These three deprecated tags office=physician, office=therapist, office=administrative -  were removed from the explict list of documented values which are rendered at z18, but they will still render at z19. The subset of office values that currently rendered at z17 will now render at z18. The dot markers were removed from z17
@matthijsmelissen
Copy link
Collaborator

matthijsmelissen commented Jun 4, 2019

Thanks for preparing this!

Can you please update the title to something that encompasses all changes (like 'update office selection and zoom level' or something like that)? Many people scan issues only by title, so it is very important that titles do not underrepresent the proposed changes.

Repository owner deleted a comment from mmelissen Jun 4, 2019
@jeisenbe jeisenbe changed the title Increase "office=" initial zoom to z18 Increase "office=" initial zoom to z18 and move deprecated values to z19 Jun 5, 2019
@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Jun 5, 2019

Thank you for the suggestion, I've updated the title

@kocio-pl
Copy link
Collaborator

What do you think about such big offices like this on z17?

Screenshot_2019-06-11 OpenStreetMap

@imagico
Copy link
Collaborator

imagico commented Jun 11, 2019

Note this change as is does not fix #3794 - which is about prioritizing rendering of building names over offices. This change does not affect the priorities, it just removes the offices from z17 leading to more building names appearing on z17 but these vanish at z18 in many cases as offices (and other POI types) appear with higher priority than building names.

The underlying problem is that starting zoom levels and priorities of labels and symbols are not in sync. This is a more complex problem to fix.

@jeisenbe
Copy link
Collaborator Author

does not fix #3794

That's true. It looks like developing a prioritization system will be complex, so we should consider #3794 still open after this is merged, though it will partially address the issue.

I think we will have to render amenity-points and the text labels in the same layer to get the priority right?

@jeisenbe
Copy link
Collaborator Author

"What do you think about such big offices like this on z17?"

In this case the building name could be shown at z17, since there is sufficient space.

I don't know enough about this particular example to know if the name of this office is something that is important to be shown on a general map.

Here in Indonesia, the offices that I would show on z17 would be important government offices that provide public services. The Humanitarian / HDM style shows government offices a little sooner and more prominently. However, I am not aware of a specific tag that differentiates between a government office which only has internal functions versus one that is open to the public - not all government offices are significant for the general map user.

@jeisenbe
Copy link
Collaborator Author

I've added 2 commits:

  1. Added office=diplomatic to the list of documented tags - this tag was approved a few months ago
  2. Fixed a redundancy in the code about the marker-width

@andrzej-r
Copy link
Contributor

The current solution was adopted to stop people gaming the prominence rules. As building names were displayed much earlier than shop/office names people were putting all sorts of information into building names to make it visible. Most issues like #3794 are solvable by simply removing office=* tag from the building and (optionally) adding a separate office=* POI.

Another solution is to simply ignore all office=/shop= tags when building=* or landuse=* are defined. That is, treat such objects as buildings first and offices second. From there, it follows that the name=* is the name of the building, not the office.

As for the zoom levels, z17 is perfectly fine. Choosing Singapore to illustrate the issue is a bit dishonest, I could provide counter examples from Norway where z17 is already too high. Everything in Singapore should be pushed 1 level up, and everything in Norway 1 level down, it's just there is no easy way of doing it.

@andrzej-r
Copy link
Contributor

I've tested this solution (giving priority to buildings over offices and shops).

For offices, it works reasonably well - the blue dots on building are not displayed and labels are normal building name labels. Stand-alone office POIs are, of course, still rendered as blue dots and names.

For shops, however, this means all fancy shop icons are lost. Depending on preference, this may look like a step back (information that was visible is now gone), or forward - building names are rendered consistently and shops can be added as separate POIs.

This is a part of a broader problem - how to deal with objects that have multiple properties. I've seen objects with all building= shop= office= and amenity= defined. We can't cover all possible combinations, so setting right priorities is important.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Jul 5, 2019

Choosing Singapore to illustrate the issue is a bit dishonest, I could provide counter examples from Norway where z17 is already too high

Nothing dishonest about it. I've never been to Norway, while Singapore is the closest large, highly developed city (Singapore city/national population is slightly greater than that of Norway, and GDP is similar), and I visit more often than I'd like due to immigration issues. It also has a rather manageable database extract size which my old laptop is able to render, unlike most European countries.

I could show some examples from Northern Ireland or Bremen or Washington DC, if that would help

For shops, however, this means all fancy shop icons are lost.

I agree that shops should be shown in preference to the building name. It's quite common for a shop=* tag to share the same polygon as a building=, even though this is not best mapping practice. I'd probably show the office in preference to the building name too; buildings are more generic features, and usually if building= and another feature is tagged on the same OSM object, the name probably belongs to the other feature.

This is a part of a broader problem - how to deal with objects that have multiple properties. I've seen objects with all building= shop= office= and amenity= defined

That is a problem. I've tried clarifying the wik page One Feature, One OSM Element to remind mappers to avoid tagging multiple features on one database object, but this will continue to happen.

@andrzej-r
Copy link
Contributor

andrzej-r commented Jul 5, 2019

I was referring to the Mercator projection. Singapore is a located near the equator - this style sheet is simply not optimal in such scenarios, so we should be careful not adjust prominence of single features there. By picking a right location (equator or poles) I could argue that any object is rendered too prominently or not prominently enough.

As for the multiple tag support, in this case I can see two consistent solutions:

  1. Pick one feature according to some priority rules. So, if we render a building, there should be no shop icon, if we render a shop icon, there should be no building.
  2. Render selected combinations properly. This means, having separate rendering rules for (a) buildings, (b) buildings+shops and (c) shops only.

(2) is obviously preferred but we would be risking opening a can of worms (how about supporting any combination of 2 or more tags we already render?). However, having a special case for building+(some POI) seems like a reasonable target. This could be done relatively easily (for example, we could simply drop rendering of the dots and reposition the office/shop labels) but it would require refactoring the rules.

@jeisenbe
Copy link
Collaborator Author

Are any of the maintainers available to review this? I understand that kocio-pl can't do it this month.

The underlying problem is that starting zoom levels and priorities of labels and symbols are not in sync.

We recently had labels for large landuse and natural areas disappear when they take up the full screen. So it's not clear if we would want the labels that appear first to always have priority. I think that's true for point features and the name labels for icons, but not necessarily true for building names and other labels on areas.

@jeisenbe
Copy link
Collaborator Author

Ready for review.

@imagico
Copy link
Collaborator

imagico commented Aug 22, 2019

I have to say that i am somewhat at odds with the whole office rendering scheme. I don't really like the catch-all for office types. Has this been discussed back when office rendering was added?

Regarding the things @andrzej-r mentioned - yes, these are mostly valid concerns. As long as this change does not further aggravate problems it would still be fine though. And raising the starting zoom level for some offices would not be a problem.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Sep 5, 2019

@imagico, did you want to review this PR, or request changes?

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.

Tests fine - but as indicated i would very much prefer it if we did not render any value for office=* with a dot but only a fixed list of values we have verified to be used consistently by mappers.

Since this change does not introduce this problem i don't mind merging it.

@jeisenbe
Copy link
Collaborator Author

Thanks. I'll merge this in a couple of days if there are no other comments.

not render any value for office=* with a dot but only a fixed list of values

I'll open a new issue to discuss this.

@jeisenbe jeisenbe merged commit 3e80994 into gravitystorm:master Sep 20, 2019
@jeisenbe jeisenbe deleted the office branch September 20, 2019 04:51
jeisenbe added a commit to jeisenbe/openstreetmap-carto that referenced this pull request Sep 20, 2019
@andrzej-r
Copy link
Contributor

This commit has reintroduced an old bug where offices PoIs are rendered as house numbers (at zoom level 17).

That was one of the reasons the offices and shops were rendered from z17 when this feature was introduced.

Please fix it or revert the commit.

@jeisenbe
Copy link
Collaborator Author

Thank you for reporting this issue. Could you provide a link to a location or a screenshot of an area where this is a problem?

The addresses layer is one of the last to render, so unlike the office icon or text, it will not block the rendering of a building name or other POIs in the building.

(Usually it is best to open a new issue for a problem like this)

@andrzej-r
Copy link
Contributor

Hi jeisenbe,

I have reported it in the past (#108 (comment)) and haven't had time to check this PR. Sorry about that.

The fundamental issue is that all addr:housenumber tags are still rendered in addition to shop/office tags. As you said, they are rendered later, so if the location is already occupied, mapnik will skip address rendering for us. This is a workaround, though. It leads to regressions like this one, when underlying labels are accidentally "uncovered". It may also slow down the rendering (more data pulled from the database and processed, only to be ignored at the end).

A proper solution would be to filter out any already rendered addr tags at query time or in the stylesheet.

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.

5 participants