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

resize big images and compress them #120

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

AurelienRichez
Copy link

@AurelienRichez AurelienRichez commented Sep 27, 2023

I resized the backgrounds that were bigger than 2000px wide with imagemagick (exact command is convert -thumbnail '2000>' $image $image, there was a bit of trial and error until I found that this gives decent results) and then reduce their size with pngcrush (pngcrush -ow -brute $image $image.tmp). A few images were bigger after this process, so I just git reset'd them.

On top of that, I added a github workflow that checks the size of the image and add an annotation if the size is bigger than 1 MB (this is a bit arbitrary). I don't intend for it to be blocking, just to help us pause and think when we are about to push a 15 MB image.

I wanted the action to annotate the files with a warning. Unfortunately, I don't think it's possible to annotate a file (it's only possible to annotate a line in a file) so the warning does not show up in the PR. Instead I make the workflow fail and the author will then receive a notification and can check out the warnings.

rationale:

The "background" image is displayed as ~1000px wide. I doubled it to account for some high DPI screens. We could be smarter and provide several resolutions and format but I don't think the engine handles that anyway (and we would need some automation to do it properly). So this is already an improvement.

I did not check every single image but for the one I looked at I could not find any big difference.

@AurelienRichez AurelienRichez force-pushed the reduce-image-size branch 5 times, most recently from 08c0dd0 to 3b37908 Compare September 27, 2023 07:44
@AurelienRichez AurelienRichez marked this pull request as ready for review September 27, 2023 07:51
@VincentBrule
Copy link
Contributor

What do you think about using WEBP/Jpeg instead of PNG so we can save even more space?

@AurelienRichez
Copy link
Author

What do you think about using WEBP/Jpeg instead of PNG so we can save even more space?

I thought about it actually 😅. As far as I know, they are supported enough to work everywhere so it would work. But I think that the background.png file is hardcoded in the engine right ? So I did not want to change everything.

It's also what I was hinting at when I talked about automation. Some CDN or other tools will probably do all that work for us (resizing, providing several resolutions depending on the screen DPI, providing the format most suited for the browser). So in the end I did not want to change too many things and just improve the current state without modifying the format.

@AurelienRichez
Copy link
Author

AurelienRichez commented Oct 20, 2023

Note for myself:

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