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

Order POI by way_area #860

Merged

Conversation

matthijsmelissen
Copy link
Collaborator

POI are now ordered by way area. Note that icons are never rendered on top
of each other, so this has only influence on priority decision of icon
rendering.

Large POI (such as universities, malls, etc.) are now rendered with higher
priority.

The old order was:

  • Render points with higher priority than areas
  • Icon priority from high to low: tourism, amenity, highway, waterway, man_made, historic, shop, leisure
  • Text priority from high to low: place, amenity, natural, historic, leisure, landuse, tourism, waterway, man_made, shop, aeroway

It is clear that the old order was quite a mess.

@matthijsmelissen
Copy link
Collaborator Author

Rebased.

@matthijsmelissen
Copy link
Collaborator Author

I think we can speed up the merging of pull requests if more people assist in reviewing them. Is anyone interesting in reviewing this pull request?

@gravitystorm
Copy link
Owner

I've picked up a few errors with this PR:

  • natural is an SQL keyword and so needs escaping (in json it becomes 'natural_' || \"natural\"
  • there's a double comma in the amenity-points query (way,,coalesce)
  • the coalesce is missing an as feature in amenity-points
  • a few columns are missing from amenity-points (definitely religion, most likely religion,waterway,lock,historic,leisure since that matches the other layer)
  • there's still a combinatorial explosion from having the [shop != ''][zoom >= 17] selector in there.

@matthijsmelissen
Copy link
Collaborator Author

Thanks for the review!

@matthijsmelissen
Copy link
Collaborator Author

Strange, I thought it parsed when I submitted the PR. Obviously submitted too fast.

@matthijsmelissen matthijsmelissen force-pushed the poi-rendering-order branch 2 times, most recently from f8151ef to 868c2fc Compare August 27, 2014 23:36
POI are now ordered by way area. Note that icons are never rendered on top
of each other, so this has only influence on priority decision of icon
rendering.

Large POI (such as universities, malls, etc.) are now rendered with higher
priority.

The old order was:
* Render points with higher priority than areas
* Icon priority from high to low: tourism, amenity, highway, waterway,
  man_made, historic, shop, leisure
* Text priority from high to low: place, amenity, natural, historic, leisure,
  landuse, tourism, waterway, man_made, shop, aeroway
It is clear that the old order was quite a mess.

* Resolves partially gravitystorm#287
* Resolves partially gravitystorm#797
* Resolves gravitystorm#802
* Resolves https://trac.openstreetmap.org/ticket/4100
@matthijsmelissen
Copy link
Collaborator Author

I fixed the issues above, and I believe it now works as intended.

I also fixed #757 (about department store icons) in the process.

@gravitystorm gravitystorm merged commit ca80283 into gravitystorm:master Aug 28, 2014
@matkoniecz
Copy link
Contributor

Is it fixing also #353 ?

@matthijsmelissen
Copy link
Collaborator Author

Thanks, yes it does.

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