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

Update oxipng to v9 [updated] #1396

Merged
merged 7 commits into from
Mar 18, 2024
Merged

Update oxipng to v9 [updated] #1396

merged 7 commits into from
Mar 18, 2024

Conversation

andrews05
Copy link
Contributor

@andrews05 andrews05 commented Jan 7, 2024

I've attempted to update to oxipng to v9, but the parallel build doesn't seem to work - the worker seems to hang when encoding. When I changed the encoder to always initST() it all worked fine.
Not sure how to debug it from here. I've spent hours just getting the thing to build correctly 😩. (Not currently possible on Apple Silicon, it seems).

There's also the new raw API in v9 which may be of use here, but I'll probably leave that for someone else 😆

Copy link

google-cla bot commented Jan 7, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@andrews05 andrews05 force-pushed the oxipng9 branch 2 times, most recently from 56d42b6 to a38577d Compare March 16, 2024 04:35
@andrews05 andrews05 marked this pull request as ready for review March 16, 2024 04:36
@andrews05
Copy link
Contributor Author

Fixed the parallel build by updating wasm-pack. It's all working now!

@andrews05 andrews05 changed the title Update oxipng to v9 Update oxipng to v9 [updated] Mar 16, 2024
@andrews05
Copy link
Contributor Author

Another update: I've reworked it to use the raw api, so it doesn't need to convert to png first.

@jakearchibald
Copy link
Collaborator

@surma this looks could. Could you give the rust bits a review?

Copy link
Collaborator

@surma surma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirm working. Great stuff!!


options.deflate = oxipng::Deflaters::Libdeflater;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I just saw that Oxipng now also offers Zopfli. Coo. We should expose that at some point.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We absolutely should

@jakearchibald
Copy link
Collaborator

The demo Squoosh logo, with 256 colors, no dithering, goes from 17.7kb to 17.2kb with this change. Nice.

@surma surma merged commit 19beb1a into GoogleChromeLabs:dev Mar 18, 2024
7 checks passed
@andrews05 andrews05 deleted the oxipng9 branch March 18, 2024 20:28
@andrews05
Copy link
Contributor Author

Awesome, thanks guys!

@jakearchibald
Copy link
Collaborator

Deployed! Thanks for the contribution. Excellent improvement.

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