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

[DO NOT MERGE] Treat barrier=toll_booth as area #3249

Closed
wants to merge 1 commit into from

Conversation

sommerluk
Copy link
Collaborator

Fixes part of #3244

Currently, we treat barrier=toll_booth as line geometry, just like all other barrier.

Resolving #3244 needs barrier=toll_booth as area geometry.

We could make a geometry transformation to get an area in the SQL query with ST_BuildArea(way) even on line geometries. But #2510 has proven that there are pitfalls that can make serious performance problems. So I think it’s better to avoid this.

Without judging here weather it is a good idea or not to render barrier=toll_booth on areas in our style, I think in any case it would be more correct to treat barrier=toll_booth as area geometry in our database tables.

This PR changes the lua file to make this happen. This would mean a database reload. I think this change is not important enough for a database reload. But maybe we can collect somewhere changes like this to the database tables, just to have them ready for a later database reload…

@pnorman
Copy link
Collaborator

pnorman commented May 28, 2018

Resolving #3244 needs barrier=toll_booth as area geometry.

No, it doesn't. That issue is about ways explicitly tagged with area=yes

@kocio-pl
Copy link
Collaborator

As far as I understand, OSMF admins plan to reimport database, so maybe you should contact them to discuss the time frame (if the issue really needs any change in lua) - see openstreetmap/chef#168 (comment).

@mmd-osm
Copy link

mmd-osm commented Jun 24, 2018

OSMF admins plan to reimport database

I think TomH mentioned a reimport of indexes in that issue, which isn't exactly a reimport of the db. Also, upgrading the render server software usually won't include such a db reimport.

@kocio-pl
Copy link
Collaborator

kocio-pl commented Dec 1, 2018

@sommerluk Hi, just revisiting PR - did you find anything more in the meantime? Any plans to do with this code from today perspective? Do you need some help with it?

@sommerluk
Copy link
Collaborator Author

Closing in favour of #3611

@sommerluk sommerluk closed this Dec 29, 2018
@sommerluk sommerluk deleted the database01 branch April 20, 2019 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants