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

added onnx icon #5472

Merged
merged 7 commits into from May 6, 2021
Merged

added onnx icon #5472

merged 7 commits into from May 6, 2021

Conversation

lukesalamone
Copy link
Contributor

@lukesalamone lukesalamone commented Apr 17, 2021

onnx preview

Brand Name: ONNX
Website: onnx.ai
Alexa rank: 480,199

Official resources for icon and color:
https://github.com/onnx/onnx

Issue:
#5391

Alexa rank: 480,199
However their github repository has over 10k stars.

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

  • hex value selected from onnx.ai website
  • resized logo with inkscape, cleared out some internal paths

@github-actions github-actions bot added the new icon Issues or pull requests for adding a new icon label Apr 17, 2021
@lukesalamone lukesalamone mentioned this pull request Apr 17, 2021
@adamrusted adamrusted linked an issue Apr 18, 2021 that may be closed by this pull request
@PeterShaggyNoble
Copy link
Member

Welcome to Simple Icons, @lukesalamone and thanks for taking this one on 🙂

Note that the SVG you mentioned in the original issue isn't quite square, it's wider than it is high, so when resizing the width to 24 the relative width should be 23.397.

I'm not sure what we should be using for the colour; from their website I'd personally expect a blue but they use many different shades without any clear preference for one over the others.

Also, for GitHub source URLs, we use the commit URL which, in this case, would be https://github.com/onnx/onnx.github.io/blob/382e7036b616ce1555499ac41730245a2478513c/images/ONNX-ICON.svg

@PeterShaggyNoble
Copy link
Member

Thanks for the update, @lukesalamone. However, I'm seeing a few small differences such as this one in the upper right between your version (in red) and the source SVG. Would you mind having another look at it to see if you can fix those up?

@lukesalamone
Copy link
Contributor Author

lukesalamone commented Apr 21, 2021

There may be a misalignment due to the size and centering requirements of the linter. If there are additional requirements please add them to the contribution guidelines. I am not an expert on SVGs so I don't know how to resolve this issue.

The icon is sized such that its largest dimension (width) is exactly 24. To meet the centering requirements of the linter, I shifted it left by 0.001.

@PeterShaggyNoble
Copy link
Member

That comparison was done after resizing and centring the original, @lukesalamone so that wouldn't account for the differences.

I think it may be an issue with the precision during the optimisation process as, after recreating the icon myself and optimising with a precision of 3, there was a marked difference in the dimensions - the width dropped to 23.969 - and, after resizing that path to 24 wide and re-centring it, I was seeing similar differences to the ones I was seeing with your path. Trying again with a precision of 4, everything lined up perfectly. So, could you try again with a precision of 4, too.

Alternatively, if you want to use it, here's my final path:

M23.0325 11.2963c-.0503 0-.1006 0-.1508.0126l-4.021-7.4387c.0754-.1383.1131-.289.1131-.4524 0-.5403-.4398-.9675-.9675-.9675-.2765 0-.5278.113-.7037.3015L9.286 1.156C9.2357.6785 8.821.3016 8.3184.3016c-.5277 0-.9675.4398-.9675.9675 0 .1634.0377.3141.113.4524l-6.245 8.9591c-.0753-.0251-.1633-.0377-.2513-.0377-.5403 0-.9675.4398-.9675.9676 0 .5403.4398.9675.9675.9675h.0377l3.3676 8.3309c-.0503.1257-.088.2639-.088.402 0 .5404.4398.9676.9676.9676.2764 0 .5277-.113.7036-.3015l10.1152.9926c.1005.4273.49.7288.9424.7288.5403 0 .9676-.4398.9676-.9675 0-.2388-.088-.465-.2262-.6283l5.1141-8.8712c.0503.0126.1005.0126.1634.0126.5403 0 .9675-.4398.9675-.9676 0-.5403-.4272-.98-.9675-.98zM17.2272 4.021c.1131.1508.2765.264.4524.3267l-1.533 11.5728c-.1005.0252-.1885.0503-.2764.1005L7.4513 8.708c.0251-.0754.0377-.1634.0377-.2514 0-.0628-.0126-.1256-.0126-.1884zm4.8754 8.5068l-5.177 3.556a1.105 1.105 0 0 0-.1256-.0753L18.3455 4.335h.0126l3.9456 7.288c-.1508.1759-.2388.3895-.2388.6408 0 .1005.0126.1885.0377.2638zM6.3832 7.5016c-.4649.0754-.8293.4775-.8293.955v.0628l-3.4555 2.0481 5.378-7.7026zm.3519 1.91c.1256-.0252.2513-.088.3518-.1634l8.356 7.2628c-.0377.113-.0628.2262-.0628.3518v.0503l-9.311 3.845c-.1382-.201-.3518-.3518-.6031-.402zm8.8963 8.1172c.1257.1382.3016.2513.5026.289l.465 4.046c-.201.1006-.3519.264-.4524.4524l-9.8136-.955zm1.1435.2136c.3267-.1633.5403-.49.5403-.867 0-.088-.0126-.1634-.0377-.2513l4.7372-3.2545-4.8 8.331zm.2513-14.3497l-9.889 4.31-.1131-.0755 1.2565-5.3906h.0377c.3393 0 .6409-.1759.8168-.4397l7.891 1.5706zM1.935 11.6105c0-.0629-.0126-.1257-.0126-.1885l3.9079-2.2995c.0754.0754.1633.1508.2638.201L4.8252 20.243l-3.2043-7.9036c.1885-.176.3142-.4398.3142-.7288Z

@PeterShaggyNoble
Copy link
Member

PeterShaggyNoble commented Apr 23, 2021

Thanks for updating, @lukesalamone.

As you're using my path now, though, we'll need a review from one of the other @simple-icons/maintainers before merging.

Copy link
Member

@adamrusted adamrusted left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comparing directly to the source (black) in Illustrator, I'm seeing quite a few inconsistencies in both the PR, in red and @PeterShaggyNoble's path, in green.

The below path is passing the linter with 4 decimals, and matches up to source - though it's probably worth another @simple-icons/maintainers checking that before you put the work into updating the PR @lukesalamone 😅

<svg role="img" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"><title>ONNX icon</title><path d="M23.0841 11.2929h-.1369L18.887 3.9023c.0912-.1369.0912-.2737.0912-.4562 0-.5474-.4562-.958-.958-.958-.2737 0-.5474.1369-.6843.3193L9.3066 1.1651C9.261.6632 8.8504.2983 8.3486.2983c-.5474 0-.958.4562-.958.958 0 .1825.0456.3193.0912.4562l-6.25 8.9873c-.0912 0-.1825-.0456-.2737-.0456-.5474 0-.958.4562-.958.958 0 .5475.4562.958.958.958h.0456l3.3759 8.3486c-.0456.1369-.0912.2737-.0912.4106 0 .5475.4562.958.958.958.2737 0 .5474-.1369.6843-.3193l10.1278 1.0037c.0912.4106.5018.7299.958.7299.5475 0 .958-.4562.958-.958 0-.2281-.0912-.4562-.2281-.6387l5.1095-8.8961h.1825c.5474 0 .958-.4562.958-.958.0458-.5018-.3648-.958-.9123-.958zm-5.8394-7.2537c.1369.1369.2737.2737.4562.3193l-1.5511 11.5877c-.0912 0-.1825.0456-.2737.0912L7.4362 8.7381c0-.0912.0456-.1825.0456-.2737v-.1825l9.7629-4.2427zm4.8814 8.5311-5.2008 3.5584c-.0456-.0456-.0912-.0456-.1369-.0912l1.5967-11.6789 3.969 7.2993c-.1369.1825-.2281.4106-.2281.6387-.0455.0912.0001.1824.0001.2737zM6.3869 7.5064c-.4562.0912-.8212.4562-.8212.958V8.51l-3.4672 2.0529L7.4818 2.853 6.3869 7.5064zm.365 1.916c.1369-.0456.2737-.0912.365-.1369l8.3486 7.2537c-.0456.0912-.0456.2281-.0456.365v.0456L6.0676 20.782c-.1369-.1825-.365-.365-.5931-.4106l1.2774-10.949zm8.896 8.1205c.1369.1369.3193.2281.5018.2737l.4562 4.0603c-.1825.0912-.365.2737-.4562.4562l-9.8085-.958 9.3067-3.8322zm1.1862.2281c.3193-.1369.5474-.5018.5474-.8668 0-.0912 0-.1825-.0456-.2281l4.7446-3.2391-4.7902 8.303-.4562-3.969zm.2281-14.3705-9.8997 4.334c-.0457-.0457-.0913-.0457-.0913-.0913L8.3486 2.26h.0456c.365 0 .6387-.1825.8212-.4562l7.8468 1.5967zM1.9617 11.6122v-.1825L5.8851 9.103c.0912.0912.1825.1369.2737.1825l-1.2774 10.949-3.1934-7.8923c.1368-.1825.2737-.4106.2737-.73z"/></svg>

@PeterShaggyNoble
Copy link
Member

OK, there is definitely something very weird going on here 🤔 I'm seeing these differences when comparing @adamrusted's version (in red) to the original but, after quarduple checking, I'm not seeing the differences in my version. Think we're going to need another pair of eyes here!

Copy link
Member

@service-paradis service-paradis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot find any significant differences using the current path, so LGTM 💯

@PeterShaggyNoble
Copy link
Member

Did you intend to merge this, @service-paradis?

@service-paradis
Copy link
Member

Did you intend to merge this, @service-paradis?

Oops 😅

@service-paradis service-paradis merged commit 9a05648 into simple-icons:develop May 6, 2021
ericcornelissen added a commit that referenced this pull request May 9, 2021
# New Icons

- Adafruit (#5592)
- Alpine.js (#5607)
- BookBub (#5435)
- FFmpeg (#5537)
- Google Fonts (#5207)
- IOTA (#5520)
- Lit (#5571)
- Odysee (#5577)
- ONNX (#5472)
- OpenWrt (#5576)
- Purism (#5462)
- Pusher (#5568)
- Telegraph (#5501)
- Thingiverse (#5591)
- UFC (#5569)

# Updated Icons

- Ford (#5564)
- Hack The Box (#5555)
- Hackaday (#5555)
- HackHands (#5555)
- Hackster (#5555)
- Handshake (#5555)
- Handshake (#5555)
- Harbor (#5555)
- Hashnode (#5555)
- Haskell (#5555)
- Hasura (#5555)
- Haxe (#5555)
- HBO (#5555)
- HCL (#5555)
- Headspace (#5555)
- HelpDesk (#5555)
- HERE (#5555)
- Heroku (#5555)
- Hexo (#5555)
- Hilton (#5555)
- HipChat (#5555)
- HockeyApp (#5555)
- Home Assistant (#5555)
- Home Assistant Community Store (#5555)
- HomeAdvisor (#5555)
- Homebrew (#5555)
- Hootsuite (#5555)
- Hoppscotch (#5555)
- Houzz (#5555)
- HP (#5555)
- Huawei (#5555)
- HubSpot (#5555)
- Hugo (#5555)
- Hulu (#5555)
- Humble Bundle (#5555)
- Hypothesis (#5555)
- Hyundai (#5555)
- ICON (#5519)
- Transport for Ireland (#5621)
- Transport for London (#5621)
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.

Please add ONNX
4 participants