Navigation Menu

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

Optimized Images by the GitHub [ImgBot] (2.89kb -> 1.86kb (35.49%)) #6768

Closed
wants to merge 5 commits into from
Closed

Conversation

IDouble
Copy link
Contributor

@IDouble IDouble commented Sep 22, 2018

[ImgBot] optimizes images
/src/compiler/crystal/tools/playground/public/icon.png -- 2.89kb -> 1.86kb (35.49%)

ImgBotApp and others added 2 commits September 22, 2018 18:15
/src/compiler/crystal/tools/playground/public/icon.png -- 2.89kb -> 1.86kb (35.49%)
[ImgBot] optimizes images
@IDouble
Copy link
Contributor Author

IDouble commented Sep 22, 2018

It's a official tool from the GitHub Marketplace: https://github.com/marketplace/imgbot
"" ImgBot will crawl through all of your image files in GitHub and losslessly compress them. This will make the file size go down, but leave the dimensions and quality untouched. ""

@j8r
Copy link
Contributor

j8r commented Sep 22, 2018

Binaries shouldn't be versioned, I don't know why there are images lying in the code... except for convenience.

@Sija
Copy link
Contributor

Sija commented Sep 22, 2018

Binaries shouldn't be versioned [...]

Who said so? It's a common practice to have versioned images, many git tools (along with gh itself) supports diffing images - and yes, they are binary. Also, there's git-lfs made especially for versioning binary files.

I don't know why there are images lying in the code... except for convenience.

Because they're used by crystal playground?
I mean, have you looked at the code at all before posting?

@j8r
Copy link
Contributor

j8r commented Sep 22, 2018

That's fine to put them in a different location, like an orphaned branch or better a dedicated repository like assets or media. For Crystal I haven't found a repository for https://crystal-lang.org/media/.

I mean, there shouldn't have images in a language compiler itself.
Here it's the playground, that's related to move it out of the compiler, which won't happen any time soon (and that's OK)

@wdhwg001
Copy link

I haven't tried but I think the PNG file could be even smaller using tools like ECT:
https://github.com/fhanau/Efficient-Compression-Tool

And I think binary bitmap logos can be vectorized and stored as SVGs. They will look better and sharper on monitors with various of DPI.

@vlazar
Copy link
Contributor

vlazar commented Sep 26, 2018

ImageOptim gives better results :)
2,956 bytes -> 1,697 bytes

icon

@proyb6
Copy link

proyb6 commented Sep 26, 2018

We need Crystal in SVG artwork for showcase, video, presentations.

<500 bytes if pre-compressed gzipped and further reduction with pre-compressed br.

@j8r
Copy link
Contributor

j8r commented Sep 26, 2018

Yes, better to use the high quality SVG https://crystal-lang.org/assets/media/crystal_logo.svg, which weights 2.6KB.

@wdhwg001
Copy link

BTW, that SVG isn't perfect, there are some weird round angles if it's zoomed to 600% or larger.
But it's another issue though.

@bcardiff
Copy link
Member

This conversation is longer than the file size probably 😹

I would merge the change to the svg. The current file probably came directly from the designer.

@IDouble
Copy link
Contributor Author

IDouble commented Sep 30, 2018

Thank you all for your replies, I removed now the png and added the svg.
When I saw it right, the path to the icon only occurs on the layout page.
I changed the src path in the layout.html.ecr

@straight-shoota
Copy link
Member

straight-shoota commented Oct 2, 2018

The SVG file can be further reduced by 29%:

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 340.4 137.7"><path d="M138.5 88.9l.6 4.7c-2.8.9-6.3 1.3-10.6 1.3-5.2 0-8.8-1.3-10.9-4-2.1-2.7-3.2-7.3-3.2-13.8s1-11.2 3.2-13.8c2.1-2.7 5.7-4 10.9-4 3.8 0 7.1.4 9.9 1.1l-.6 4.8c-2.9-.2-6-.3-9.3-.3-2.9 0-4.8.9-5.9 2.6-1 1.8-1.6 5-1.6 9.7 0 4.7.5 8 1.6 9.7 1.1 1.8 3 2.6 5.9 2.6 4.2-.1 7.6-.3 10-.6zm28.7-5.4l3.1 10.8h-6.7l-2.8-10.7c-.6-2.1-2-3.1-4-3.1h-6.2v13.8h-6.2V59.8c3-.3 7.2-.4 12.6-.4 4.4 0 7.5.7 9.3 2.2 1.8 1.5 2.7 4.1 2.7 7.9 0 2.7-.5 4.7-1.5 6.1s-2.7 2.3-5 2.6v.3c1 .2 1.9.8 2.8 1.6 1 .7 1.6 1.9 1.9 3.4zm-5.7-9.4c.8-.8 1.2-2.2 1.2-4.3 0-2.1-.4-3.5-1.2-4.3-.8-.7-2.3-1.1-4.6-1.1h-6.2v10.9h6.2c2.3 0 3.8-.4 4.6-1.2zm96.2-14.3h-27.5v5.3h10.5v29.2h6.3V65.1h10.7v-5.3zm19.4 1.6l10.4 32.8H281l-2.8-9.8h-12.8l-2.8 9.8h-6.4l10.3-32.8c.3-1.1 1.1-1.7 2.3-1.7h6.1c1.1.1 1.9.6 2.2 1.7zm-.4 18l-3.5-11.9c-.5-1.8-.7-2.8-.8-2.9h-1.3l-.8 2.9-3.4 11.9h9.8zm35.9 9.7h-12c-1.1 0-1.9-.2-2.3-.6-.4-.4-.7-1.2-.7-2.2V59.8h-6.3v27.4c0 4.8 2.6 7.2 7.8 7.2 5.9 0 10.4-.2 13.7-.4l-.2-4.9zM187.8 73.2c-.2.5-.7 1.9-1.4 4.2h-.4c-.7-2.2-1.2-3.6-1.4-4.2l-6.3-13.4h-6.6L183 82.6v11.8h6.3V82.6l11.3-22.8h-6.5l-6.3 13.4zm32 2l-6.6-2.1c-1.5-.5-2.5-1-3-1.6-.6-.6-.8-1.6-.8-3 0-1.7.4-2.8 1.2-3.3.8-.4 2.3-.7 4.6-.7 3 0 6.5.1 10.3.2l.4-4.4c-3-.8-6.6-1.2-10.8-1.2-4.4 0-7.5.6-9.2 1.8-1.8 1.2-2.6 3.7-2.6 7.5 0 2.6.5 4.8 1.6 6.4s2.8 2.8 5.3 3.6l7 2.2c1.4.4 2.4.9 2.9 1.5s.8 1.6.8 3.1c0 1.7-.4 2.9-1.1 3.4-.8.5-2.3.8-4.6.8-1.5 0-5.1-.1-10.9-.2l-.4 4.6c3.7.7 7.4 1.1 11.2 1.1 4.6 0 7.8-.7 9.5-2 1.8-1.3 2.7-3.9 2.7-7.7 0-2.6-.5-4.8-1.6-6.4-1.4-1.6-3.2-2.8-5.9-3.6zM96.4 78l-9.1-34c0-.1-.1-.2-.3-.3l-34.1-9.1c-.1 0-.3 0-.4.1l-25 24.9c-.1.1-.1.2-.1.4l9.1 34c0 .1.1.2.3.3l34.1 9.1c.1 0 .3 0 .4-.1l25-24.9c.1-.2.2-.3.1-.4zM63 51.3l-9 33.4c0 .1-.1.1-.2 0L29.4 60.3c-.1-.1 0-.1 0-.2l33.5-9c0 .1.1.1.1.2z"/></svg>

I manually removed the empty rectangular and reduced to minimal SVG markup using svgo.

@wdhwg001
Copy link

wdhwg001 commented Oct 2, 2018

@straight-shoota I think we need to preserve the XML header for best compatibility.

@straight-shoota
Copy link
Member

@wdhwg001 SVG is based on XML 1.0 in which the XML declaration is optional.

It doesn't hurt adding it, but I'm not aware of any particular benefit. If there was any downside to removing it, I'm pretty sure svgo wouldn't do it by default.

@straight-shoota
Copy link
Member

@AYIDouble Would you mind applying the reduced svg?

@straight-shoota straight-shoota added the pr:needs-work A PR requires modifications by the author. label Feb 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:needs-work A PR requires modifications by the author. topic:tools:playground
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants