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

amenity=parking icon no longer rendered on buildings #3908

Closed
Prince-Kassad opened this issue Sep 26, 2019 · 9 comments · Fixed by #3923
Closed

amenity=parking icon no longer rendered on buildings #3908

Prince-Kassad opened this issue Sep 26, 2019 · 9 comments · Fixed by #3923

Comments

@Prince-Kassad
Copy link

Example illustrating the problem: https://www.openstreetmap.org/way/221032567

This is a regression - amenity=parking icon used to render on buildings.

@Adamant36
Copy link
Contributor

Displays here: https://www.openstreetmap.org/#map=19/37.76763/-122.41342

But not here: https://www.openstreetmap.org/#map=19/37.74680/-122.42099

Seems to be about the presence of the name.

@matkoniecz
Copy link
Contributor

I am pretty sure that we have an issue for rendering icon off center, so it would not be blocked by label (like it happened here) or by icon.

But I failed to find it.

@jeisenbe
Copy link
Collaborator

jeisenbe commented Oct 2, 2019

The building name is being rendered with higher priority than the parking icon.

This happened because the parking icons were moved to the low-priority layer only in #3874 - previously it was rendered in both amenity-points (at higher priority than building names) and amenity-low-priority. But I should have explained and discussed this change more first, sorry about that.

Previously parking was partially moved to the low-priority layer in #1364 - and there was discussion about moving it all to low-prioirty, after text labels - #1364 (comment)

Then in #3612 I moved parking to only z14 and higher - though it looks like I made a mistake and did not limit the #text-low-priority layer to z14 as well, so that still needs to be fixed.

The question is if buildings or the parking icon and text should have priority. By putting parking in amenity-low-priority only, we are saying that it should be below other text labels in priority, including buildings.

Is this a mistake? Is the parking icon and text style more appropriate for parking structures?

@Jez-C
Copy link

Jez-C commented Oct 3, 2019

It's not just the missing icon, the current rendering is inconsistent at z17.

The example from the OP - way 221032567 shows name=Parkhaus Nürnberger Straße only, but way 221023170 to the north shows addr:housenumber=3 plus name=Parkhaus Langstraße in the blue parking text style:

Hanau_parking

I use name=* for the name of the parking (building names I tag addr:housename=*) just the same as I would on shops for example. Therefore I think the parking icon and text style are more appropriate for parking structures.

@jeisenbe
Copy link
Collaborator

jeisenbe commented Oct 4, 2019

That's happening because addresses now have higher priority than the parking icon, but then the text is still rendered since it is below the (blocked) icon.

I now think it's probably a mistake to put parking in the low-priority layer. Instead we can make it later in amenity-points (and therefore in text-point).

@matkoniecz
Copy link
Contributor

Note that parkings were intentionally split to ensure that small ones will not block anything worth displaying but will be show in places where there is nothing else.

@Prince-Kassad
Copy link
Author

The current rendering encourages removing building=yes from parking garages just to get the parking symbol to render as well as the unique color for parking=multi-storey that, right now, gets overridden by the building color. Since parking garages are in fact buildings, we should not encourage not tagging them as buildings.

@jeisenbe
Copy link
Collaborator

jeisenbe commented Oct 5, 2019

One other issue: the current code renders both parking icons and text labels for areas with >900 way_pixels (for example, a 30 by 31 pixel building, or a 11 by 90 pixel parking lot). This is non-standard: the one other icon which depends on way_pixels uses 3000 as the limit (e.g. marketplace), and all other text labels use 3000 way_pixels.

There are 3 reasonable ways to solve this:

  1. Change the icon and text to start at > 3000 way_pixels so that the text and icon can fit
  2. Change the text to start at > 3000 way_pixels and keep the icon at 900 (since the 14x14 pixel parking icon should fit within most of these, though the text won't)
  3. Change the text to start at > 3000 way_pixels and change the icon to start at 750 way_pixels, which is exactly 1 zoom level sooner. This is a little clearer.

@jeisenbe
Copy link
Collaborator

jeisenbe commented Oct 5, 2019

I think the third option is best, after some quick tests, and I think it should be done for amenity=marketplace icons too.

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