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

Adding abandoned buildings at z18. #163

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

AndrewBuck
Copy link

abandoned buildings
I drawing ways with abandoned:building=* on them like normal buildings, but with an X over them. They show up at z18.

The rendering style works as shown in the screenshot, but it will not function properly until the DB that the rendering is done from is re-imported using the new default.style file for osm2pgsql that pnorman committed with this commit.

@yohanboniface
Copy link
Collaborator

See #126.

@yohanboniface
Copy link
Collaborator

Thanks!

Some comments:

  • we are missing factorization here: the building styling code is defined twice. Two solutions I can see: first is to add #abandoned[building!=null][zoom>=18] line 33; second is to have a dedicated layer for abandoned buildings instead of having every abandoned features in one, and then again to add a selector with the actual building selector; and then in any case to have a new selector for the "X"
  • the "X" mark is a little bit too raw in my opinion, we need some more iterations to get the it right; I'm balanced between having a feature dedicated symbol, so more adapted, or keeping the idea of a generic "abandoned" symbol; not sure this last will fit every case; as a first try, maybe "×" will be a little bit better

Thanks again for your work! :)

@yohanboniface
Copy link
Collaborator

Aie, we have a process issue here :/
You are mixing features in the same pull request, this is not the way to go.
Can you revert the barrier thing or provide new clean pull requests please?

@AndrewBuck
Copy link
Author

barrier-dry_bush

Here is the screeenshot for the barriers commit.

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.

2 participants