oxipng

oxipng copied to clipboard

Fixes #579. In lack of a proper understanding of what's going on here and why the issue is happening, this workaround will do for now.

I'd like to fix this in the Zopfli crate itself instead, but let's keep this PR open in case I'm unable to do so in a timely fashion.

Sounds good. I'm thinking we should do a point release once this is fixed.

@andrews05 could you please rebase the branch so that I can grab the latest CI build for testing? Thanks!

@XhmikosR Done

Is the artefact build good enough for some regression testing? Or should I wait for the point release?

Yeah, it should be perfectly good for testing, feel free. I’ve just rebased again so you should see a new build in a moment.

Note you may see some cases where the “fix” makes it worse, but on average it’s an improvement in my observations.

Oh, looks like there are some new clippy lints I need to fix. I’ll try and sort that soon…

Success!

@AlexTMjugador What do you think, will you have time to investigate or shall we just use this for now?

@andrews05 just my 2 cents. The clippy fixes could go into master separately, but this makes me wonder how come you guys are hitting this inconsistency. I believe you need to enable the repo option for branches to always be up to date so that you catch such issues before landing something in master.

I think the Clippy lint fixes should have been pushed to the main branch separately, so that this and other PRs could be rebased on top of those changes and benefit them all. But I'm doing the cherry-picking and rebasing now, so no worries here.

What do you think, will you have time to investigate or shall we just use this for now?

Next week I finally have some days off which I could use to dedicate some time to open source work like this, but no promises :slightly_smiling_face:

Hey gang... any chance of rebasing to main, to regenerate the CI artefacts? 🙏🏻

@ace-dent done. Let me know your observations with and without this patch.

@andrews05 - nice! Thanks!

@andrews05 could you please rebase the branch? TIA!

@XhmikosR Done. Are you finding any improvement from this change?

Thanks!

I need to test again with new files because I've already had a run with this patch in the past. I remember there were some small savings, but I'll try to get back to you with numbers.

Maybe @ace-dent has a recent benchmark.

@XhmikosR I won't be in position to do thorough testing for a few weeks. If you want a highly optimised test set, you can try re-running on all *.png images in this repo. This directory of pixel art is also highly optimised. It'd be interesting to see if any further (lossless) improvements are possible.

I tested @ace-dent's repos and there was no difference between 9.1.2 and this PR, although there were improvements:

Before: 3,31 MB (3.475.236 bytes)
After:  3,25 MB (3.409.474 bytes)

But I tested a few images created with Firefox's built-in screenshot tool and this PR resulted in consistently better compression.

OS: Windows 11 (Pro) x86_64 / WIN32_NT 10.0.22631.3880 (23H2)

9.1.2:
E:\Downloads\Internet Downloads>oxipng -o max -Z --preserve -s --fix --alpha "Screenshot 2024-07-14 at 14-38-25 Tor Project Success.png"
Processing: Screenshot 2024-07-14 at 14-38-25 Tor Project Success.png
110394 bytes (62.10% smaller): Screenshot 2024-07-14 at 14-38-25 Tor Project Success.png

PR:
E:\Downloads\Internet Downloads>oxipng -o max -Z --preserve -s --fix --alpha "Screenshot 2024-07-14 at 14-38-25 Tor Project Success.png"
Processing: Screenshot 2024-07-14 at 14-38-25 Tor Project Success.png
108665 bytes (62.69% smaller): Screenshot 2024-07-14 at 14-38-25 Tor Project Success.png

---
9.1.2:
E:\Downloads\Internet Downloads>oxipng -o max -Z --preserve -s --fix --alpha "Screenshot 2024-07-14 at 17-57-00 Workaround weird regression with & u8 and zopfli by andrews05 · Pull Request #595 · shssoichiro_oxipng.png"
Processing: Screenshot 2024-07-14 at 17-57-00 Workaround weird regression with & u8 and zopfli by andrews05 · Pull Request #595 · shssoichiro_oxipng.png
127783 bytes (49.18% smaller): Screenshot 2024-07-14 at 17-57-00 Workaround weird regression with & u8 and zopfli by andrews05 · Pull Request #595 · shssoichiro_oxipng.png

PR:
E:\Downloads\Internet Downloads>oxipng -o max -Z --preserve -s --fix --alpha "Screenshot 2024-07-14 at 17-57-00 Workaround weird regression with & u8 and zopfli by andrews05 · Pull Request #595 · shssoichiro_oxipng.png"
Processing: Screenshot 2024-07-14 at 17-57-00 Workaround weird regression with & u8 and zopfli by andrews05 · Pull Request #595 · shssoichiro_oxipng.png
123321 bytes (50.95% smaller): Screenshot 2024-07-14 at 17-57-00 Workaround weird regression with & u8 and zopfli by andrews05 · Pull Request #595 · shssoichiro_oxipng.png

---
9.1.2:
E:\Downloads\Internet Downloads>oxipng912 -o max -Z --preserve -s --fix --alpha "Screenshot 2024-07-14 at 18-11-25 Release v9.1.2 · shssoichiro_oxipng.png"
Processing: Screenshot 2024-07-14 at 18-11-25 Release v9.1.2 · shssoichiro_oxipng.png
117701 bytes (55.77% smaller): Screenshot 2024-07-14 at 18-11-25 Release v9.1.2 · shssoichiro_oxipng.png

PR:
E:\Downloads\Internet Downloads>oxipng -o max -Z --preserve -s --fix --alpha "Screenshot 2024-07-14 at 18-11-25 Release v9.1.2 · shssoichiro_oxipng.png"
Processing: Screenshot 2024-07-14 at 18-11-25 Release v9.1.2 · shssoichiro_oxipng.png
116135 bytes (56.36% smaller): Screenshot 2024-07-14 at 18-11-25 Release v9.1.2 · shssoichiro_oxipng.png

And a screenshot taken with the Windows snipping tool:

9.1.2:
E:\Downloads\Internet Downloads>oxipng912 -o max -Z --preserve -s --fix --alpha "Screenshot 2024-07-14 182953.png"
Processing: Screenshot 2024-07-14 182953.png
194877 bytes (37.68% smaller): Screenshot 2024-07-14 182953.png

PR:
E:\Downloads\Internet Downloads>oxipng -o max -Z --preserve -s --fix --alpha "Screenshot 2024-07-14 182953.png"
Processing: Screenshot 2024-07-14 182953.png
191109 bytes (38.88% smaller): Screenshot 2024-07-14 182953.png

@XhmikosR Nice, thanks for the info!

@andrews05 could you please rebase the branch one more time?

BTW, any reason why this hasn't landed yet? I mean, I know it doesn't fix the root cause but it does yield better compression.

Thanks!

@XhmikosR Done. I guess we're still hoping @AlexTMjugador can investigate the root cause sometime 😅

I guess we're still hoping @AlexTMjugador can investigate the root cause sometime 😅

I clearly didn't have the chance to delve into the matter that week I planned to :sweat_smile:

Researching a more proper fix for this behavior should be relatively straightforward though, nothing too complicated for an afternoon or so. The problem being that almost always another thing to do appears in my afternoons... But I'd like for me or someone else to get around to it, eventually.