-
Notifications
You must be signed in to change notification settings - Fork 816
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
Add icon for amenity=bench and amenity=waste_basket #1249
Add icon for amenity=bench and amenity=waste_basket #1249
Conversation
At least the bench should be included direct as SVG. |
👍, I've opened pull request matthijsmelissen#5 for this. |
As with #1240 for picnic table I'm not sure if the bench should be slightly thicker? With 1.5px thickness it would look like that (more blurry of course): |
I prefer 1.5 pixels |
This would be my proposal for waste basket (waste-basket-16.svg): |
I merged in the svg bench icon. I don't particularly like the bin icon - not sure what to do about it, maybe it would look better with a straight bottom? |
@nebulon42 - I like this icon. |
Yes, looks better to me. Thanks! |
I have updated the corresponding icon: waste-basket-16.svg |
43a4ad6
to
5d71669
Compare
I updated the PR with both svg icons. |
To me bench.16.svg is still displayed as 1px version. Did you mean to keep that? |
No I forgot that one, it should be updated now. I also updated the preview. |
A bench is larger than a basket. They are often near. It looks strange if the basket has more visual weight. |
The new waste basket looks good to me. |
Yes. I would show the Icon for benches as ways, too. But showing the dimension seems not easy possible, so drop this information. |
How should we do that? Does it make sense to add an extra query specifically for linear benches? |
f4ad8f8
to
cec9e91
Compare
Added new bin icon and updated preview. |
Maybe bin icon can be even smaller? |
I'd prefer to leave the icon as is. |
I propose moving these two icons to .amenity-low-priority to avoid blocking more important icons and labels. |
The guidance #1282 (comment) about icon size and weight is IMO very applicable here. As mentioned earlier #1249 (comment) weight of bin vs bench is strange. We do not have practical guidelines yet, just general guidance. This is very related to @mkoniecz last comment
EDIT: +1 to avoid blocking more important icons and labels |
|
Rebased. |
cec9e91
to
2acc943
Compare
@math1985 Can you move these two icons to .amenity-low-priority to avoid blocking more important icons and labels? |
Sorry I forgot, done now. |
It is hard to find place with fully mapped benches and waste baskets, but in such places rendering of these objects is too prominent. I would propose pushing benches to z19 and waste baskets to z20 (or at least - waste baskets to z20). http://www.openstreetmap.org/#map=19/50.07081/19.91222 Also, is it possible to ensure than benches have priority over waste baskets? |
tile.osm.org doesn't render z20. I'd be against any changes for zoom level 20, except if continuing a size progression. These changes would only be visible to those running their own deployment of osm-carto. We've used z17 for the cutoff at which everything renders for other features, and I'd prefer to do so for benches and bins. If we feel benches/bins are too strong, perhaps reducing the size? Saying z19 only feels like a cartographic cop-out. |
Right, I forgot. Than I repeat my
|
I made a smaller version of waste basket: https://github.com/nebulon42/osmic/blob/master/outdoor/waste-basket-10.svg |
* Add icon for amenity=bench from z18 (resolves gravitystorm#194) * Add icon for amenity=waste_basket from z19 (resolves gravitystorm#418)
Rebased, and made the following changes:
This is again ready for review. |
9c54f7f
to
55a6404
Compare
* Render waste_basket smaller * Render bench only from z19 * Give bench priority over waste_basket
55a6404
to
0ea7622
Compare
I would support merging. @math1985 - would it be OK for me to squash 3 commits from this PR into one before merging? It is case of "improve previous commit" rather than separation of independent changes. |
Yes, please squash. In general I think it's clearer if the reviewer squashes rather than the author of the PR, as it makes it easier for the reviewer to see what has changed. |
Merge done via cli in 376a1b4 |
These icons are taken from the SJJB repository, so they are still not aligned to the pixels.
Preview:
![screenshot from 2015-01-30 21 30 06](https://cloud.githubusercontent.com/assets/5251909/5983650/2f62bb5c-a8c7-11e4-8f45-5d263f83a227.png)