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

Render amenity=food_court #1040

Merged
merged 2 commits into from
Jan 13, 2015
Merged

Conversation

sommerluk
Copy link
Collaborator

Currently, there isn't any rendering for food courts (amenity=food_court). It would be nice to add this. It could be rendered the same way like amenity=restaurant.

Fixes #868.

@matthijsmelissen
Copy link
Collaborator

There is an alternative symbol on osm-icons: http://osm-icons.org/wiki/File:12050.svg

@sommerluk
Copy link
Collaborator Author

There is an alternative symbol on osm-icons: http://osm-icons.org/wiki/File:12050.svg

@math1985 Thanks for the hint. Current practice is using PNG versions, not SVG versions, right?

@matthijsmelissen
Copy link
Collaborator

@sommerluk The PNG versions are generated from SVG by the scripts here: https://github.com/twain47/Open-SVG-Map-Icons/tree/master/tools

There seems to be no documentation, if you can't figure out how to use the script(s) yourself I can have a look.

@matkoniecz
Copy link
Contributor

@sommerluk Can you rebase this PR into a single commit?

@sommerluk
Copy link
Collaborator Author

@math1985 The scripts work (with warnings, but they work). The SVG file itself needs some adoptions in order to work with the scripts. Result in size 16 and 20:

food_foodcourt p 734a08 16

food_foodcourt p 734a08 20

How is the policy about icon size? There seem to be some icons at size 16 and others at size 20.

@mkoniecz Could you give me a little hint how to do the rebase?

@matkoniecz
Copy link
Contributor

@mkoniecz Could you give me a little hint how to do the rebase?

http://git-scm.com/book/ch3-6.html (with goal of merging these two commits - "Render also a label" should not be a separate in master - https://github.com/gravitystorm/openstreetmap-carto/commits/master )

@matthijsmelissen
Copy link
Collaborator

Thank you for your pull request. We still have quite a lot inconsistencies in amenity-points, see #710. I think it's better if we solve these first, before adding new icons. I would therefore propose to defer this pull request until after #710 has been solved.

@matthijsmelissen matthijsmelissen added this to the New features milestone Oct 11, 2014
@matkoniecz
Copy link
Contributor

I think that current form (render it like amenity=restaurant) may be OK and it is not adding any icons. ("may" is caused by fact that I have no knowledge needed to decide whatever "render it like amenity=restaurant" is a good idea).

@matthijsmelissen
Copy link
Collaborator

The SVG file itself needs some adoptions in order to work with the scripts.

What are these adoptions? I only ran the script on SJJB's own icons, not on the derived osm-icons. It's a shame the osm-icons are not suitable to plug in the script directly.

How is the policy about icon size? There seem to be some icons at size 16 and others at size 20.

There is at the moment no policy, which is exactly the reason why I prefer solving #710, and indeed #974 as well, first. Adding new icons becomes much easier once there is some consistency in the current icons...

@matkoniecz
Copy link
Contributor

BTW, it would fix #868

@matthijsmelissen
Copy link
Collaborator

BTW, it would fix #868

Updated the first message so Github will close it automatically once this gets merged.

@sommerluk
Copy link
Collaborator Author

The SVG file itself needs some adoptions in order to work with the scripts.

What are these adoptions?

The helper script recolor.sh searches within the SVG file for “fill:#111111” and similar things and it replaces them by another color. This works fine in statements like <svg:path style="fill:#111111;stroke:#eeeeee;stroke-width:3.40799999" /> But our icon from osm-icons uses code like <path fill="#111111" stroke="#EEEEEE" stroke/> (capitals instead of minuscels, and no “:”) Probably the difference is there because the original icons where authered in Inkscape and our “new” icon was authered in Adobe Illustrator and these programs use different “styles” for producing their SVG code.

@sommerluk sommerluk closed this Oct 11, 2014
@matthijsmelissen
Copy link
Collaborator

Thanks! Shame that the guy from osm-icons didn't take the scripts into account.

Did you mean to close the issue?

@sommerluk sommerluk reopened this Oct 11, 2014
@sommerluk
Copy link
Collaborator Author

No, was just closed by mistake, sorry. Reopening.

@sommerluk
Copy link
Collaborator Author

@mkoniecz After various tries, I still could not unify the two commits. If there is interest in merging this “as is” (rendering like restaurant, without an own icon), I can provide a new PR with only one commit. If we want an own icon, we have to decide on the size …

matthijsmelissen added a commit that referenced this pull request Jan 13, 2015
@matthijsmelissen matthijsmelissen merged commit 81cd4a8 into gravitystorm:master Jan 13, 2015
@matthijsmelissen
Copy link
Collaborator

Thanks!

@sommerluk sommerluk deleted the foot_court branch January 17, 2015 16:34
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.

Add rendering for amenity=food_court
3 participants