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

update icon: llvm: Simplify plain icon #2264

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Finii
Copy link

@Finii Finii commented Sep 5, 2024

[why]
While the plain icon is plainer than the original icon, it is still quite complex with very thin strokes.

[how]
Just take the outermost outlines of the plain icon and crate a solid icon from it.

Note

This can be controversial.
For me the plain icon is not really plain, it has lots and lots of detail and consists of 2600 nodes:

image

The simple plain icon proposed here drops all the details and uses just the outline (260 nodes)

image

If one needs an icon with all the glory details the -original should be a good fit, the plain should be a simple(r) version, I believe. But anyhow, this is very opinionated and I do not want to mess with your ideas here. For a small text icon the current plain icon is far too complex and all the small detail hairlines can not be seen anyhow.

Double check these details before you open a PR

  • PR does not match another non-stale PR currently opened

Features

This PR closes NONE

Notes

@Snailedlt as per ryanoasis/nerd-fonts#1691 (comment)

[why]
While the plain icon is plainer than the original icon, it is still
quite complex with very thin strokes.

[how]
Just take the outermost outlines of the plain icon and crate a solid
icon from it.

Then hand-remove 'dent' in the tail.

Signed-off-by: Fini Jastrow <[email protected]>
@Finii
Copy link
Author

Finii commented Sep 5, 2024

Hmm, hand-remove the gaps in the tail, that is an obvious artifact of the fill related broken outline in the original svg.
Force push.

image

@Finii Finii changed the title llvm: Simplify plain icon update icon: llvm: Simplify plain icon Sep 5, 2024
Copy link
Collaborator

@Snailedlt Snailedlt left a comment

Choose a reason for hiding this comment

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

hmmm, I don't agree with this one. While I agree that the icon could benefit from some simplification I think going all black is too much simplification, making it less recognizable... but let's see what the other maintainers think too :)

Edit:

Good that you fixed the artifact though. That should probably be fixed in the original icon too.

Some context: The original icon is made by converting a png to svg with inkscape. The process is not ideal so there are a few artifacts. I think it could be improved upon now though, since I know there are better png to svg converters out there now than when we made this icon :)

@Snailedlt Snailedlt requested review from a team, ConX, weh, Snailedlt, canaleal and lunatic-fox and removed request for a team September 12, 2024 11:01
@Snailedlt Snailedlt changed the base branch from master to develop September 12, 2024 11:03
@Finii
Copy link
Author

Finii commented Sep 12, 2024

hmmm, I don't agree with this one

😆

As I said, controversial (see image below).

For me, as "in a font" the current plain one is just too intricate. Have a look at it sized down:

image

But that is just for ME and font usage, which might or might not be important for you.
Just wanted to share this icon version with you.

image

@Snailedlt
Copy link
Collaborator

Snailedlt commented Sep 12, 2024

I agree it looks better in the font, but in larger sizes it looks significantly worse. Perhaps we could find some middle ground?
One way would be to roughly inverse the current plain version, and remove some of the detailing lines.

I'm thinking something like this (just with transparent lines instead of white ones):
image

@Snailedlt
Copy link
Collaborator

Snailedlt commented Sep 12, 2024

@Finii
Here's a suggestion. Though it should be optimized more, because it's very large (80Kb).
I optimized it using SVGOMG, but it should probably be optimized more by reducing the amount of nodes in the path.
llvm-original

@Snailedlt
Copy link
Collaborator

Sidenote: The old plain version can still be used as a line version instead :)

Copy link
Contributor

@weh weh left a comment

Choose a reason for hiding this comment

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

great work 👍

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.

3 participants