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.

Sign up for GitHub

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

Jump to bottom

Icon cleanup #2451

Merged
merged 4 commits into from
Nov 24, 2016
Merged

Icon cleanup #2451

merged 4 commits into from
Nov 24, 2016

Conversation

nebulon42
Copy link
Contributor

@nebulon42 nebulon42 commented Nov 20, 2016 •
edited
Loading

Fixes #2430
Fixes #2062

Various cleanup tasks regarding icons.

  • remove standard size (14) from icon name
  • set explicit width and height on icons (fixes Width and height attributes on SVG symbols #2430)
  • clean up 16px icons, remove groups in some icons and bring some to 14px, add missing canvas to some icons as that makes it easier to customise the width/height downstream
  • move shop icons in shop subfolder

Just for the record, we are currently rendering 49 different shop icons (yes, quite some are from me).

@HolgerJeromin
Copy link
Contributor

Really nice and important work!

@kocio-pl
Copy link
Collaborator

It also makes #2062 obsolete.

Why some icons are renamed to include pixel size (like helipad.16.svg or post_box.12.svg)? This is not consistent with the rest (like pub.16.svg -> pub.svg).

@nebulon42
Copy link
Contributor Author

I have removed the standard size (14px) from the name, but left the non-standard sizes, so that it is easy to see which icons deviate from the common canvas size.

@matthijsmelissen
Copy link
Collaborator

+1

@kocio-pl
Copy link
Collaborator

Oh, so I suspect pub.16.svg -> pub.svg name transition was done because the code was also fixed? If yes, I have no other issues with this PR.

@math1985 Technical question - is it ready to be merged as it is or the commits should be joined into just one?

@nebulon42
Copy link
Contributor Author

Oh, so I suspect pub.16.svg -> pub.svg name transition was done because the code was also fixed?

Yes.

or the commits should be joined into just one?

From my side I placed the commits deliberately into these chunks, so that changes could be left out, reverted and so on. But I guess it is fine either way. If we later want to change something it is easier if a part and not the whole bunch has to be changed.

@matthijsmelissen
Copy link
Collaborator

To me either is fine.

@kocio-pl kocio-pl merged commit 26ffa3b into gravitystorm:master Nov 24, 2016
Ircama added a commit to Ircama/openstreetmap-carto that referenced this pull request Nov 26, 2016
@Ircama
Test commit without rebasing, just to allow revision
e2faadf 
amenity-points.mss and symbols/wilderness_hut.svg are ready for revision.

I have now changed the file naming of wilderness_hut.svg (with icon produced by @nebulon42) following @nebulon42's convention in gravitystorm#2451.

This commit also would like to help @jojo4u in testing his last update at gravitystorm#2459. I generated project.mml with Windows; I temporarily added the new yaml2mml.py, just to allow testing. Due to the inclusion of the new project.mml and new yaml2mml.py with this commit, please, do not merge by now.
Ircama added a commit to Ircama/openstreetmap-carto that referenced this pull request Nov 26, 2016
@Ircama
Test commit (rebased), just to allow revision
a312fb7 
amenity-points.mss and symbols/wilderness_hut.svg are ready for revision.

I have now changed the file naming of wilderness_hut.svg (with icon produced by @nebulon42) following @nebulon42's convention in gravitystorm#2451.

This commit also would like to help @jojo4u in testing his last update at gravitystorm#2459. I generated project.mml with Windows; I temporarily added the new yaml2mml.py, just to allow testing. Due to the inclusion of the new project.mml and new yaml2mml.py with this commit, please, do not merge by now.

______________________________________________________________________

**Rendering of tag tourism=wilderness_hut**

Latest changes:

- Use wilderness-hut.svg icon by @nebulon42
- Rebased to master

______________________________________________________________________

To summarize, the following different kind of huts have specific symbols now:

- amenity=shelter (only shown at zoom >=16)
- tourism=alpine_hut  (not modified within this drop)
- tourism=wilderness_hut (zoom>=13)

Symbols:
- wilderness_hut: symbols/wilderness_hut.svg (new)
- alpine_hut: symbols/alpinehut.p.16.png (unmodified)
- generic shelter: symbols/shelter.svg (unmodified)
Ircama added a commit to Ircama/openstreetmap-carto that referenced this pull request Nov 29, 2016
@Ircama
**Rendering of tag tourism=wilderness_hut**
063a9eb 
Code completed

amenity-points.mss and symbols/wilderness_hut.svg are ready for merge.

Latest changes:
- I have now changed the file naming of wilderness_hut.svg (with icon produced by @nebulon42) following @nebulon42's convention in gravitystorm#2451.
- Rebased to master

______________________________________________________________________

To summarize, the following different kind of huts have specific symbols now:

- amenity=shelter (only shown at zoom >=16)
- tourism=alpine_hut  (not modified within this drop)
- tourism=wilderness_hut (zoom>=13)

Symbols:
- wilderness_hut: symbols/wilderness_hut.svg (new)
- alpine_hut: symbols/alpinehut.p.16.png (unmodified)
- generic shelter: symbols/shelter.svg (unmodified)
@HolgerJeromin HolgerJeromin mentioned this pull request Dec 1, 2016
Closed
nebulon42 pushed a commit to nebulon42/openstreetmap-carto that referenced this pull request Dec 1, 2016
@Ircama @nebulon42
**Rendering of tag tourism=wilderness_hut**
f27b17f 
Code completed

amenity-points.mss and symbols/wilderness_hut.svg are ready for merge.

Latest changes:
- I have now changed the file naming of wilderness_hut.svg (with icon produced by @nebulon42) following @nebulon42's convention in gravitystorm#2451.
- Rebased to master

______________________________________________________________________

To summarize, the following different kind of huts have specific symbols now:

- amenity=shelter (only shown at zoom >=16)
- tourism=alpine_hut  (not modified within this drop)
- tourism=wilderness_hut (zoom>=13)

Symbols:
- wilderness_hut: symbols/wilderness_hut.svg (new)
- alpine_hut: symbols/alpinehut.p.16.png (unmodified)
- generic shelter: symbols/shelter.svg (unmodified)
@kocio-pl kocio-pl mentioned this pull request Dec 15, 2016
Closed
sommerluk pushed a commit to sommerluk/openstreetmap-carto that referenced this pull request Dec 17, 2016
@Ircama @sommerluk
**Rendering of tag tourism=wilderness_hut**
7bf4113 
Code completed

amenity-points.mss and symbols/wilderness_hut.svg are ready for merge.

Latest changes:
- I have now changed the file naming of wilderness_hut.svg (with icon produced by @nebulon42) following @nebulon42's convention in gravitystorm#2451.
- Rebased to master

______________________________________________________________________

To summarize, the following different kind of huts have specific symbols now:

- amenity=shelter (only shown at zoom >=16)
- tourism=alpine_hut  (not modified within this drop)
- tourism=wilderness_hut (zoom>=13)

Symbols:
- wilderness_hut: symbols/wilderness_hut.svg (new)
- alpine_hut: symbols/alpinehut.p.16.png (unmodified)
- generic shelter: symbols/shelter.svg (unmodified)
@nebulon42 nebulon42 deleted the icon-cleanup branch March 22, 2017 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Width and height attributes on SVG symbols use either name.size.ext or name-size.ext for all symbol files
4 participants
@nebulon42 @HolgerJeromin @kocio-pl @matthijsmelissen