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

Drawing order in amenity-points sometimes undefined #4676

Open
imagico opened this issue Sep 5, 2022 · 4 comments
Open

Drawing order in amenity-points sometimes undefined #4676

imagico opened this issue Sep 5, 2022 · 4 comments

Comments

@imagico
Copy link
Collaborator

imagico commented Sep 5, 2022

The amenity-points layer currently orders by score and way_pixels - see

ORDER BY score DESC NULLS LAST,
way_pixels DESC NULLS LAST
- which are often the same for different POIs leading to the drawing order (and therefore prioritization) being defined how the features happen to be returned by the database - which is inconsistent across different deployments and therefore not a good idea.

This problem is distinct from #3880 - which calls for a specific drawing order matching the starting zoom levels.

The drawing order should match the order of the COALESCE() for the feature attribute for consistent results between double tagging the same node with two different POI types vs. two separate nodes - see #4631.

@pnorman
Copy link
Collaborator

pnorman commented Sep 12, 2022

We should add feature, osm_id. This will ensure that we consistently pick an order in case of multiple amenities of different types, and of the same type of amenity nearby.

@imagico
Copy link
Collaborator Author

imagico commented Sep 13, 2022

Yes, that would solve the issue of having an undefined order.

Note however the need to have the ORDER BY and the COALESCE() use the same order for consistent prioritization. Currently we have at least some cases in the amenity-points COALESCE() were we deliberately don't use alphabetical order - for example we prioritize tourism=hotel over amenity=restaurant (11k occurrences of this tag combination). That would change if we order by feature, osm_id and then modify the COALESCE() to match that. And for #3880 this will not work either, we will need to have a distinct order that is non-alphabetical.

@pnorman
Copy link
Collaborator

pnorman commented Sep 14, 2022

Note however the need to have the ORDER BY and the COALESCE() use the same order for consistent prioritization

No we don't. All that's actually required for a consistent order between two objects is ordering by osm_id and area, where the latter is a proxy for point/polygon.

It's better to have the ordering consistent with feature selection on objects that are multiple things at the same time, but it isn't required to avoid the bug.

@imagico
Copy link
Collaborator Author

imagico commented Sep 14, 2022

It's better to have the ordering consistent with feature selection on objects that are multiple things at the same time, but it isn't required to avoid the bug.

That is what i said. #4631 and #3880 however will not be addressed by such a change. And if splitting a POI node with several primary tags into two nodes changes rendering that is not consistent and irritating for the mapper and might lead to adjusting the mapping to a certain desired rendering outcome.

The clean solution for all these issues would be to have a deliberately designed prioritization order (in case of amenity-points based on the starting zoom levels) and to use that for both the ORDER BY and the COALESCE().

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

2 participants