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

Add support for Oxipng #190

Merged
merged 1 commit into from Sep 16, 2021
Merged

Add support for Oxipng #190

merged 1 commit into from Sep 16, 2021

Conversation

oblakeerickson
Copy link
Contributor

Based on #167

Oxipng is a multi-threaded rust implementation of Optipng.

https://github.com/shssoichiro/oxipng

oblakeerickson added a commit to oblakeerickson/image_optim that referenced this pull request Apr 21, 2021
This is needed for PR toy#190
@coding-horror
Copy link

any movement on this? 🙏

toy pushed a commit that referenced this pull request Jun 6, 2021
This is needed for PR #190
@oblakeerickson
Copy link
Contributor Author

@toy Thanks for the merge on #193 :) I was hoping it would fix the appveyor CI test, but it still failed. It doesn't appear to be from the missing oxipng binary any more though. It might just need to be re-ran?

I rebased this branch with master to get rid of the merge conflicts. Let me know if there is anything else I should update.

@toy
Copy link
Owner

toy commented Jun 7, 2021

@oblakeerickson I should've mention here the worker-analysis branch that I created to try to understand if optipng and oxipng should be both enabled by default, or if only one should be enabled depending on availability. One more reason is to try to understand if level 6 for optipng is needed (this default is there from the start). I also need to find time to try to make output of the worker-analysis more readable.
Any suggestions for a bunch of test images that I can throw at analysis script?

@oblakeerickson
Copy link
Contributor Author

oblakeerickson commented Jun 8, 2021

Ah okay, ya I didn't know you were working on that branch.

For test images I used some from https://ftp.arl.army.mil/ftp/historic-computers/ just because I know they are large (several MB) and it was easiest to see a performance improvement with oxipng.

Specifically I used this image http://ftp.arl.army.mil/ftp/historic-computers/png/brlescI2.png as one benchmark I ran initially when I made this PR on my 8 core computer:

✓ 5.529442969s oxipng --strip all -i0 -o 3 --quiet /tmp/3620b7d70423a01b5c0220210219-270942-4esp2o.png
3620b7d70423a01b5c02.png

Original Size: 6941148
New file size: 3790576
✓ 24.734980747s optipng -i0 -o 2 -quiet -- /tmp/0650117ed6b0e9b29f0d20210219-271187-1hj9m1p.png
0650117ed6b0e9b29f0d.png

Original Size: 6941148
New file size: 3790592

For the same 6.7mb image oxipng compressed it in 5.5s compared to 24.7s with optipng.

@oblakeerickson
Copy link
Contributor Author

Looks like switching to github actions is part of this work? I see you have been working on the worker-analysis branch, so thank you ❤️

@toy
Copy link
Owner

toy commented Jul 13, 2021

Currently not easy to dedicate enough to opensource.
But yes, switching to github actions became a requirement. And sad as I was using travis for 8 years and the way their change happened was not beautiful, but github actions are at least much snappier even though not yet very polished.
While working on worker-analysis branch I've also found some possible problem with pngquant, as it seems to be not lossless in some cases even with options requiring it.

Based on toy#167

Oxipng is a multi-threaded rust implementation of Optipng.

https://github.com/shssoichiro/oxipng
@toy
Copy link
Owner

toy commented Sep 15, 2021

Sorry for long wait, I was trying to understand if it makes a lot of sense to run both oxipng and optipng for same image, but for now I only can tell that for some images they both contribute to reducing size. Worker analysis script in current state was not very helpful to understand if running both is not mostly wasteful and I didn't find time to rework it, so just updated PR and I think it is better to merge and iterate later

@oblakeerickson
Copy link
Contributor Author

@toy no worries on the wait, totally understand. Thank you for updating the PR :)

@toy toy merged commit 10fca39 into toy:master Sep 16, 2021
oblakeerickson added a commit to discourse/discourse_docker that referenced this pull request Nov 6, 2021
image_optim, a ruby library we use, now has support for oxipng:

 toy/image_optim#190 (comment)

So I'm adding the oxipng binary to the base image so that we can
start using it. There currently isn't an apt package for it.
oblakeerickson added a commit to discourse/discourse_docker that referenced this pull request Nov 8, 2021
image_optim, a ruby library we use, now has support for oxipng:

 toy/image_optim#190 (comment)

So I'm adding the oxipng binary to the base image so that we can
start using it. There currently isn't an apt package for it.
@oblakeerickson oblakeerickson deleted the oxipng branch November 22, 2021 17:18
@oblakeerickson
Copy link
Contributor Author

@toy Just following up and letting you know this is now live in discourse. Thanks again for helping out with this :)

discourse/discourse@da9cd4f

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.

None yet

3 participants