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

Give more priority to museums #3924

Open
dieterdreist opened this issue Oct 7, 2019 · 12 comments
Open

Give more priority to museums #3924

dieterdreist opened this issue Oct 7, 2019 · 12 comments

Comments

@dieterdreist
Copy link

Expected behavior

they outrule bars and banks

Actual behavior

the get hidden because of bars and banks

Links and screenshots illustrating the problem

Here's an example https://www.openstreetmap.org/node/6205724716
I am not completely sure whether there are other sideeffects, but it seems as if the museum is hidden because of the bank and the bar. Please check the priorities.

@HolgerJeromin
Copy link
Contributor

Current screenshot shows exactly what you suggest in all zoom levels:
image

@dieterdreist
Copy link
Author

great to read it, but it doesn't represent the situation on my screen (and the museum was added 9 months ago and is still in version 1):
Screenshot 2019-10-07 at 15 42 13
Screenshot 2019-10-07 at 15 42 07
Screenshot 2019-10-07 at 15 41 54
Screenshot 2019-10-07 at 15 41 45

@jeisenbe
Copy link
Collaborator

jeisenbe commented Oct 8, 2019

This is the current line where tourism=museum is selected. It is before all amenity features, so museums should have priority over banks, bars, etc:

COALESCE(
'aeroway_' || CASE WHEN aeroway IN ('gate', 'apron', 'helipad', 'aerodrome') THEN aeroway ELSE NULL END,
'tourism_' || CASE WHEN tourism IN ('alpine_hut', 'apartment', 'artwork', 'camp_site', 'caravan_site', 'chalet', 'gallery', 'guest_house',
'hostel', 'hotel', 'motel', 'museum', 'picnic_site', 'theme_park', 'wilderness_hut',
'zoo') THEN tourism ELSE NULL END,
'amenity_' || CASE WHEN amenity IN ('arts_centre', 'atm', 'bank', 'bar', 'bbq', 'bicycle_rental',

Following the link, I get the same results as @HolgerJeromin - https://www.openstreetmap.org/node/6205724716

So this is a problem with one of the rendering servers, not with this style.

(There is a problem on z18 where the museum icon is shown, but the bar text shows: the museum name is too long to fit in the space, so it is not rendered, but then the bar name fits, so it is shown. This is a known issue, see #234 and PR #2597 which attempted to solve it https://www.openstreetmap.org/node/6205724716#map=18/44.38999/7.54744)

@jeisenbe
Copy link
Collaborator

jeisenbe commented Oct 8, 2019

I suppose we could consider giving museums priority over aeroway=helipad, tourism=motel/apartment/etc - is this also an issue?

@StyXman
Copy link
Contributor

StyXman commented Oct 9, 2019

It is before all amenity features, so museums should have priority over banks, bars, etc

The query has this ordering:

WHERE feature IS NOT NULL
ORDER BY score DESC NULLS LAST,
way_pixels DESC NULLS LAST
) AS amenity_points

and score is defined as:

CASE
WHEN "natural" IN ('peak', 'volcano', 'saddle') THEN
CASE
WHEN tags->'ele' ~ '^-?\d{1,4}(\.\d+)?$' THEN (tags->'ele')::NUMERIC
ELSE NULL
END
WHEN "waterway" IN ('waterfall') THEN
CASE
WHEN tags->'height' ~ '^\d{1,3}(\.\d+)?( m)?$' THEN (SUBSTRING(tags->'height', '^(\d{1,3}(\.\d+)?)( m)?$'))::NUMERIC
ELSE NULL
END
ELSE NULL
END AS score,

This means that it's ordering by (peak, volcano, saddle) elevation or waterfall height, then polygons by size, then the rest. From what I understand about SQL in general, the rest will come in unspecified order, maybe insert order. I also ave the impression that the order in which the style is defined does not impose a rendering order, but I might be mistaken.

@matkoniecz
Copy link
Contributor

matkoniecz commented Oct 9, 2019

This is the current line where tourism=museum is selected. It is before all amenity features, so museums should have priority over banks, bars, etc:

AFAIK order in COALESCE matters when multiple things are tagged at once on one object. So tourism=museum military=bunker will get feature=tourism_museum, not feature=military_bunker.

Compare with say

ORDER BY
CASE
WHEN power = 'tower' THEN 1
WHEN power = 'pole' THEN 2
ELSE NULL
END
where objects are ordered by type for rendering order

In other words @StyXman is right.

@jeisenbe
Copy link
Collaborator

Related to #3880 - Symbol and label prioritization is not in sync with the starting zoom levels - probably would be fixed if that issue is solved.

@StyXman
Copy link
Contributor

StyXman commented Oct 10, 2019

order in COALESCE matters when multiple things are tagged at once on one object. So tourism=museum military=bunker will get feature=tourism_museum, not feature=military_bunker

Agreed, but the original question was, as I understand it, when a museum is close to other amenities (POIs?), render it "with higher priority" than those other amenities.

@matkoniecz
Copy link
Contributor

Yes, I just wanted to explain when this specific order matters.

@dieterdreist
Copy link
Author

FWIW, no matter how often I clear the browser cache I still see the situation like in the screenshots I have provided. If people see different map tiles depending on the server from which they get them, this would be a different issue not concerning this repo, or would it?

@matkoniecz
Copy link
Contributor

or would it?

Looking at code it seems that there is no well defined order of objects, at least in this case. I think that adding something along ORDER_BY osm_id to ensure that all servers serve the same tiles would be a good idea.

@pnorman
Copy link
Collaborator

pnorman commented Oct 11, 2019

FWIW, no matter how often I clear the browser cache I still see the situation like in the screenshots I have provided. If people see different map tiles depending on the server from which they get them, this would be a different issue not concerning this repo, or would it?

If the ordering of objects is not deterministic then it's a style bug.

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

No branches or pull requests

7 participants