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

Optimize various SVGs (second try) #2318

Merged
merged 2 commits into from
Jan 25, 2022
Merged

Optimize various SVGs (second try) #2318

merged 2 commits into from
Jan 25, 2022

Conversation

RiedleroD
Copy link
Contributor

So last time didn't work - for more context see #2130 and #2179.

I only fixed the circlecount image for now, to see if this hypothetical fix even works. If this doesn't work then your SVG engine is severely outdated or just plain shit.

Anyway, @borgmanJeremy could you please check if the circlecount image displays correctly now? Compat. commit is 038f869

@borgmanJeremy
Copy link
Contributor

I've only tested on MacOs but the circle counter looks correct now.

@RiedleroD
Copy link
Contributor Author

that should be all, I think. Everything should work now.

@borgmanJeremy
Copy link
Contributor

Can you squash and rebase on master? As an aside please don't have obscenities in the final commit message, I try to keep it family friendly 😁

@RiedleroD
Copy link
Contributor Author

Can you squash and rebase on master

wish me luck :/ git usually doesn't like me

please don't have obscenities in the final commit message

got it! Wouldn't have added any, since that one is actually important, but in the future I'll try to keep them out of the regular ones as well 👍🏻

@borgmanJeremy
Copy link
Contributor

If git gives you a problem let me know I can fix it

decreased file size, complexity, fixed spec compliancy and fixed one case where the text rendered weirdly sometimes
@RiedleroD
Copy link
Contributor Author

ayy it worked!

@RiedleroD RiedleroD marked this pull request as ready for review January 24, 2022 22:22
@borgmanJeremy
Copy link
Contributor

Thanks! Just tested this on MacOS and its 99% correct. The only issue is with the "upload to imgur icon". The massive keyboard looked good too.
2022-01-24_19-43

I'll test on Windows tomorrow.

@RiedleroD
Copy link
Contributor Author

should be fixed now, will test on windows later today as well

@RiedleroD
Copy link
Contributor Author

tested x64 portable on windows, works without visible problems.

@borgmanJeremy
Copy link
Contributor

All looks good here. Thanks!!

@borgmanJeremy borgmanJeremy merged commit 5ab76e2 into flameshot-org:master Jan 25, 2022
@borgmanJeremy
Copy link
Contributor

Do you have directions for how you generated these reduced SVGs? If i make more icons in the future I can minimize them.

@RiedleroD
Copy link
Contributor Author

generated

If you could see the bags under my eyes you wouldn't ask that question.

On a more serious note, most of it is just knowing what stuff you can remove without problems, various tricks to make paths shorter and a few handy websites: yqqn's path editor, ellipse/circle to path converter, and inkscape if there's some path transform you can't do otherwise. There's also various websites and applets that try to minimize SVGs automagically, but I haven't had much success with them in the past.

TL;DR: it's mostly experience.

@borgmanJeremy
Copy link
Contributor

Haha that shows my ignorance for graphic design. I usually draw it in GIMP and then auto trace it in Inkscape....

@RiedleroD
Copy link
Contributor Author

you can send me a message when you need a new icon → [email protected]

borgmanJeremy pushed a commit that referenced this pull request Jan 28, 2022
* Optimized various SVGs

decreased file size, complexity, fixed spec compliancy and fixed one case where the text rendered weirdly sometimes

* fixed a single space too little 🤦
panpuchkov pushed a commit to namecheap/flameshot that referenced this pull request May 13, 2022
* Optimized various SVGs

decreased file size, complexity, fixed spec compliancy and fixed one case where the text rendered weirdly sometimes

* fixed a single space too little 🤦

(cherry picked from commit 5ab76e2)
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.

2 participants