Combine palette reduction and sorting

[andrews05]

This PR resolves the regression issue in #553.
When a palette reduction occurs, we no longer force it to be the baseline. Instead we combine it with the sorting step and only make it the baseline if sorting fails. This way we can retain the original image as the baseline in these situations without changing the number of evaluations (therefore performance remains unchanged).

The mystery of how #553's palette is weirdly optimised remains unsolved.

[AlexTMjugador]

This PR looks reasonable, but I guess it's a draft because it's a specific hotfix for the mentioned issue that may cause regressions or unwanted behavior changes in some edge cases?

[andrews05]

Yeah, I marked it as draft just because I want to consider my logic a little more. And indeed, it seems almost hacky for one specific image.

[Winterhuman]

I have a good number of images that show weird output size variations when OxiPNG is combined with pngquant if you need them for analysis (I think the differences are due to the palette optimisations like what this pull tackles), however, I suspect the new release might fix this discrepancy so I might hold off until then.

Let me know if you'd like the images anyways, @andrews05

[andrews05]

@Winterhuman You can try out development builds from the workflow runs if you like. Find them on the Actions page, e.g. https://github.com/shssoichiro/oxipng/actions/runs/6305282807

[Winterhuman]

Just tried out that build with some test images. Unfortunately, the output size (just oxipng on its own) varies wildly in comparison to the current version of oxipng; it's hard for me to draw any conclusions on if it's better or worst than the current version.

I'll make sure to open a separate issue with what I've found. EDIT: Now done

[andrews05]

@AlexTMjugador I've updated the logic here. It all comes down what we do when a palette reduction alters the image data (as opposed to just truncating the palette, for instance). In these cases, the logic goes like this:

1. We want to keep the original image as the baseline, not assume the "reduced" version is necessarily better.
2. The palette is effectively in a random order right now, neither matching the original order nor having been explicitly sorted yet. Therefore, it's unlikely to be "good" and we shouldn't waste time evaluating it until after we sort it.

It's similar to what I first posted but more correct in edge cases.
Not fussed about whether this makes the cut for v9 or not.

[AlexTMjugador]

Neat! I think your suggested changes are better, but I would hate to rush them into the v9 release and not think them through. For now, let's focus on releasing v9, which already has a lot of changes. A minor release can be pushed relatively quickly with this patch if we need to.

[andrews05]

Sounds good! Might help if we can get more reports of similar issues after release.

[Winterhuman]

Ran the newest force-push out of curiosity, and it's looking good! (left is the current push, right was the push I ran in #558)

Filename:		'section-tags.png'
Original Filesize:	240 bytes
PNGQuant Filesize:	186 bytes
OxiPNG Filesize:	185 bytes	<-	188 bytes
OxiPNG-ZF Filesize:	185 bytes	<-	185 bytes
Oxi-Quant Filesize:	185 bytes	<-	186 bytes
Oxi-ZF-Quant Filesize:	185 bytes	<-	185 bytes

Most of the results are a couple bytes of improvement, them staying the exact same, or a few bytes regressions for the non-Zopfli versions. The only notable result being this one which was one of the images included in #558 (though it's still an improvement over v8.0.0):

Filename:		'section-series.png'
Original Filesize:	197 bytes
PNGQuant Filesize:	160 bytes
OxiPNG Filesize:	160 bytes	<-	163 bytes
OxiPNG-ZF Filesize:	160 bytes	<-	157 bytes
Oxi-Quant Filesize:	160 bytes	<-	160 bytes
Oxi-ZF-Quant Filesize:	160 bytes	<-	157 bytes

[andrews05]

@Winterhuman The changes you're seeing will likely be due to the libdeflate update rather than anything in this PR. (Libdeflate is always used for evaluations, so can affect the outcome even when using Zopfli).
Good to see more consistency though.

[ace-dent]

@andrews05 @AlexTMjugador - sorry to disagree... I think this fix should be in v9 release, as it corrects a 'regression' introduced with the new palette optim code. For User's not backing up their images, they may lose an optimal palette, that may be difficult / impossible to regenerate with future fixes...
I think the original input palette should always be evaluated against potential sorted palettes.

[AlexTMjugador]

Right, fair point @ace-dent. Some weeks have passed by since this PR was made, and looking at it again with fresh eyes I think its logic is sound now. I vote we merge it! (My main reason for not merging this for v9 was that it'd delay its release, but that's kind of a moot point now.)