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

Add Ardour #2918

Merged
merged 5 commits into from Apr 29, 2020
Merged

Add Ardour #2918

merged 5 commits into from Apr 29, 2020

Conversation

mondeja
Copy link
Member

@mondeja mondeja commented Apr 9, 2020

ardour-preview

Issue: Closes #2909

Checklist

  • I updated the JSON data in _data/simple-icons.json
  • I optimized the icon with SVGO or SVGOMG
  • The SVG viewbox is 0 0 24 24

Description

Source SVG file from here.

@ericcornelissen ericcornelissen added the new icon Issues or pull requests for adding a new icon label Apr 11, 2020
@PeterShaggyNoble
Copy link
Member

Thanks for the PR, @mondeja.

I think we can go with the icon without the text here.

@PeterShaggyNoble
Copy link
Member

Also, the source URL should be the URL of the SVG used on GitHub, including the commit hash. See our contributing guidelines for more details.

@mondeja
Copy link
Member Author

mondeja commented Apr 14, 2020

Updated, here is the new preview:

ardour-commit-2-preview

@PeterShaggyNoble
Copy link
Member

Thanks, @mondeja 👍

Firstly, one minor issue: your path is coming in at 24.001 wide. This can usually be fixed by increasing the precision.

Secondly, to help us compare like with like, could you let us know which specific version of their icon you based yours on? There are some terrible inconsistencies between files and even within individual files 🙄 The comparison below, for example, shows the top-left icon in this file overlaid in red with the top-right one. I've compared your version with 3 different versions from their repo and all have different inconsistencies so I suspect I'm not using the one you used.

Finally, for the source URL, if you used the app-icon_tango.svg file linked above, for example, it should be https://github.com/Ardour/ardour/blob/9fac6139ea40ee2ba5af1415c302c61fee6a13aa/tools/misc_resources/app-icon_tango.svg - to get the file URL with the latest commit hash included hit y on your keyboard while viewing the source file in your browser.

Ardour

@PeterShaggyNoble PeterShaggyNoble added the awaiting reply Issues or pull requests awaiting reply from an individual before it may be addressed label Apr 15, 2020
@mondeja
Copy link
Member Author

mondeja commented Apr 15, 2020

I've used the file ardour_bw.svg. I'm not sure of what icon to use. Maybe we need to ping Ardour developers.

@PeterShaggyNoble
Copy link
Member

I've used the file ardour_bw.svg.

That was one of the ones I tried in my comparisons, too. There are a couple of minor discrepancies that will need to cleaned up before we can merge this (see screenshots below for examples, with your version in red).

I'm not sure of what icon to use.

As they're all from an official source, it's fine to use any of them. My first preference would be to ardour_bw.ai, followed by ardour_bw.svg.

Maybe we need to ping Ardour developers.

Certainly can't hurt 👍 Pinging @pauldavisthefirst as the top contributor to that repo for feedback. If we don't hear back in a few days.


@thorwil
Copy link

thorwil commented Apr 17, 2020

Hi, I’m the creator of Ardour’s logo. There has been an update several years ago. The old version has irregular “teeth” and happens to be in the first image of this thread. The new version has evenly spaced “teeth”. Since I never had commit access, as it would have been silly for one file here, one file there, and since years have passed, I don’t know what is what in there, except that all originals are SVG created with Inkscape.

ardour_for_simple-icons.zip: current version, including low-resolution versions derived from the original Tango-themed icon set.

@mondeja
Copy link
Member Author

mondeja commented Apr 17, 2020

Thanks @thorwil. I will update the logo soon.

@mondeja
Copy link
Member Author

mondeja commented Apr 17, 2020

I've used the best resolution version at the top of the file provided at the comment above and setted the source of the icon pointing to Ardour miscelaneous resources repository directory.

ardour-preview

@PeterShaggyNoble
Copy link
Member

Thanks for taking the time to reply and provide us with that file, @thorwil - we very much appreciate it.

The height of your SVG is a little off, @mondeja (20.933 vs. the expected 20.787), leading to the sort of differences seen below, with your version in red. With a vector of the newest logo seemingly not being available anywhere I'm not sure about the source URL but I think what you've used is a good compromise; hopefully by the time they next update their logo, that repo will be up to date.

@PeterShaggyNoble PeterShaggyNoble removed the awaiting reply Issues or pull requests awaiting reply from an individual before it may be addressed label Apr 20, 2020
@mondeja
Copy link
Member Author

mondeja commented Apr 20, 2020

Resized to 20.787. Let me know if you catch another inconvenient.

@PeterShaggyNoble
Copy link
Member

You've gone the other way now, @mondeja - this is now coming in at only 23.832 in width.

@PeterShaggyNoble
Copy link
Member

Took me a second to realise that you based your latest version on the more simplified second icon in the provided zip! 🤦‍♂️ Yours lines up perfectly with that and, for our purposes, I'm OK with using that second icon.

I'll let @thorwil have the final, official say, though; if they give the 👍 on us using the second icon over the first then this is ready to merge.

@PeterShaggyNoble PeterShaggyNoble added awaiting reply Issues or pull requests awaiting reply from an individual before it may be addressed and removed changes requested labels Apr 27, 2020
@thorwil
Copy link

thorwil commented Apr 27, 2020

That version is really only meant for 48px and maybe below. For anything larger,
ardour.zip would be better. So it depends on what kind of use you expect.

@PeterShaggyNoble
Copy link
Member

As our native size is 24*24, I think that suits us perfectly, so.

Thanks for your efforts on this one, @mondeja and for your valuable inpu & feedback, @thorwil.

@PeterShaggyNoble PeterShaggyNoble merged commit c9cc0b2 into simple-icons:develop Apr 29, 2020
@github-actions github-actions bot mentioned this pull request May 3, 2020
ericcornelissen added a commit that referenced this pull request May 3, 2020
# New Icons

- NBB (#2915)
- Ardour (#2918)
- Facebook Live (#2990)
- Azure Functions (#2991)
- Autodesk (#2954)
@PeterShaggyNoble PeterShaggyNoble removed the awaiting reply Issues or pull requests awaiting reply from an individual before it may be addressed label May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new icon Issues or pull requests for adding a new icon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Request: Ardour
4 participants