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

new icon: delphi (original, plain) #2290

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

Bulani
Copy link

@Bulani Bulani commented Oct 25, 2024

Double check these details before you open a PR

  • PR does not match another non-stale PR currently opened
  • PR name matches the format new icon: Icon name (versions separated by comma). More details here
  • PR's base is the develop branch.
  • Your icons are inside a folder as seen here
  • SVG matches the standards laid out here
  • A new object is added in the devicon.json file at the correct alphabetic position as seen here

This PR closes NONE

Link to prove your SVG is correct and up-to-date.

https://www.embarcadero.com/br/news/logo

@Bulani Bulani changed the title new icon: delphi-12 (original) new icon: delphi (original) Oct 25, 2024
@canaleal canaleal added the feature:icon Use this label for pull requests when a new icon is ready to be added to the collection label Nov 17, 2024
@canaleal
Copy link
Member

canaleal commented Dec 7, 2024

Excellent work on the icon. If possible could you also create a plain version of the icon? Here is how a plain icon should look like.

@Bulani Bulani changed the title new icon: delphi (original) new icon: delphi (original, plain) Dec 10, 2024
@Bulani
Copy link
Author

Bulani commented Dec 10, 2024

Excellent work on the icon. If possible could you also create a plain version of the icon? Here is how a plain icon should look like.

Added logo in plain version

Copy link
Contributor

Hi!

I'm the check-bot and we have some issues with your PR:

devicon.json is not sorted correctly.
Please make sure that your icon is added in the `devicon.json` file at the correct alphabetic position
as seen here: https://github.com/devicons/devicon/wiki/Updating-%60devicon.json%60



SVG Error in 'delphi-original.svg':
- 'viewBox' is not '0 0 128 128' -> Set it or scale the file using https://www.iloveimg.com/resize-image/resize-svg.

SVG Error in 'delphi-plain.svg':
- 'viewBox' is not '0 0 128 128' -> Set it or scale the file using https://www.iloveimg.com/resize-image/resize-svg.

Check our CONTRIBUTING guide for more details regarding these errors.

Please address these issues. When you update this PR, I will check your SVGs again.

Thanks for your help,
SVG-Checker Bot 😄

Copy link
Contributor

Hi!

I'm the check-bot and we have some issues with your PR:

devicon.json is not sorted correctly.
Please make sure that your icon is added in the `devicon.json` file at the correct alphabetic position
as seen here: https://github.com/devicons/devicon/wiki/Updating-%60devicon.json%60


Check our CONTRIBUTING guide for more details regarding these errors.

Please address these issues. When you update this PR, I will check your SVGs again.

Thanks for your help,
SVG-Checker Bot 😄

@Bulani
Copy link
Author

Bulani commented Jan 21, 2025

@canaleal Can you review this PR, now everything is ok

@canaleal
Copy link
Member

@Snailedlt Opinions on the original.svg? It had a lot of paths and I don't think there's a way to optimize it.

@Snailedlt
Copy link
Collaborator

Hmm, if you're not able to optimize it I doubt I'll be able to do it.

Maybe the guy from nerdfonts would be able to help here? I know he is good at optimizing the number of nodes.

@Finii do you have any tips for us to optimize this icon? Would be nice if it was properly optimized here, so you don't have to re-optimize it for usage in nerdfonts

@Finii
Copy link

Finii commented Feb 10, 2025

Hej ;)

It had a lot of paths and I don't think there's a way to optimize it.

In Inkscape, select all, Path -> Exclusion combines all path into one. That is not always the way, but here it certainly is. You end up with one path.

Removed this tiny (probably unwanted) path first:

image

And then the fill was concrete colors for each path, different colors 🤔

image

In the simple version you probably want no concrete color at all, select 'filled with unknown color' and probably 'evenodd fill', and 'no stroke'. Fonts always work with even-odd nonzero fill, and when you save it here with nonzero even-odd fill that often is converted correctly but not always.

image
Arrows point at undefined and even-odd fill, you should probably use the other fill rule

And then I guess there are too many nodes, optimized them away. Here the Path -> Simplify consideres its accuracy based on all selected nodes area (lengths etc) (and only simplifies the selected nodes) relatively; so if you want to make smallest simplifications you can select just some few nodes, while selecting all nodes in one allows bigger positional errors in total.

Here I simplified all nodes except the outer circle which is already optimal with 4 nodes at the extrema.

image

Left side 131 nodes, right side 347 nodes

And finally ... Save As..., use 'Optimized svg`. Let it clean up everything and usually 4 digits is enough to avoid rounding errors.

$ ll delphi-plain*         
-rw-rw-r-- 1 fini fini 10770 Feb 10 10:46 delphi-plain_orig.svg
-rw-rw-r-- 1 fini fini  4886 Feb 10 12:12 delphi-plain.svg

Here's the file:

delphi-plain (evenodd fill)
delphi-plain (nonzero fill)


Edit:

Hmm, I was wrong with the fill rules. Maybe it depends on being a postscript based (otf) font or a truetype (ttf) font, but I know that sometimes I have to correct the path direction to get the correct fill in the resulting font file. And that only makes sense if the font uses nonzero. 🤔

Changed the advice given above and also changed the attached svg.

image

Origin of snippet: https://www.w3.org/TR/SVG/painting.html#FillProperties

@Snailedlt
Copy link
Collaborator

@Finii thanks a bunch! I'm glad we have such a knowledgeable developer we can reach out to for help!
I assume the original version can also be simplified by using some of these tricks. I'm guessing mainly by removing the unused nodes and bringing down the nodecount. Let me know if thee are any other things we could do to optimize the delphi-original.svg specifically.

@canaleal I think these should be good, just make sure you double check that the plain versions render correctly on icomoon :)

@Finii
Copy link

Finii commented Feb 10, 2025

have such a knowledgeable developer

Hobbyist 😬

I assume the original version can also be simplified by using some of these tricks.

image

2000 path with maybe 300 nodes each 😬

The reason is that the fill is with gradients, but the gradients are in fact just a lot of individual paths all filled with one solid color (red), slightly different from the path nearby.

image

Top of the icon zoomed way in

Note all the single colored areas, and how 'unclean' the darkred wedge comes in 😬

But here I have no clue how to optimize it while keeping the coloring true to the original. A 'regular' gradient is how it is and can not be tweaked with infinite precision. One would need to define which 'imperfections' or 'deviations from the original' or still ok.

🤔 I am not even sure what the aim of the different svgs here is, maybe to be consumed as svgs, and as such that is probably all good. Otoh, the more I look at icons, the more I wonder WHO create the original svgs, probably graphic designers that do not care and/or understand how that all 'really' works. Like me 1992 with Corel Draw 2 🤣

@Snailedlt
Copy link
Collaborator

Yeah, it was probably made with some workarounds in a tool that wasn't made for gradients I guess.

Perhaps we should ask the delphi developers if we can remake the icon with a simplified gradient. I bet they would also appreciate a more optimized version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature:icon Use this label for pull requests when a new icon is ready to be added to the collection
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants