Add EncoderParams to make predictor transform optional

[w-flo]

Hi,

Thank you for your work on this!

I noticed map tiles rendered from SVG compress much better with two tweaks: disable the predictor transform, and use a color cache (almost 40% smaller files with 11 bit color cache).

Now I guess that only works for some types of image, and I can't come up with good heuristics for this. And testing all kinds of encoding parameters every time the encoder runs to pick the best combination seems expensive – I want these map tiles to compress fast, which they currently do.

So I think the API needs some way for the user to specify encoding parameters. I'm not sure if my PR offers a good design for this. It's just something that works for me, but I guess with possible "automatic parameter choice heuristics" to be added in the future by someone else, maybe a better way to design the API can be found, that allows users to set params that they care about, while leaving other params to be determined automatically (probably offering different speed settings).

I'm marking this as a draft for this reason, since I'm very open to suggestions on how to change the API.

Anyway, this PR contains a new "EncoderParams" struct that currently has only one member for disabling or enabling the predictor transform, and there's a second commit that adds a image buffer comparison to the libwebp roundtrip test to hopefully notice if the encoding is buggy. The color cache is not included, to focus on API first.

[w-flo]

I wasn't sure if I should mark this #[non_exhaustive].

[kornelski]

Yes, it's a good idea to have non-exhaustive in public APIs.

[w-flo]

I agree, but it makes using this struct a bit less ergonomic, because

EncoderParams {
    use_predictor_transform: false,
    ..Default::default()
}

(like in the test in my second commit) is forbidden outside of the image-webp crate with non-exhaustive structs. It needs to be

let mut p = EncoderParams::default();
p.use_predictor_transform = false;

instead.

[kornelski]

You can make EncoderParams::with_predictor_transform(false)

[fintelia]

I like this approach.

libwebp uses heuristics to select one of six combinations of transforms (They call the predictor transform "spatial" in that list). Except for the lowest compression level, where libwebp just has a single hard-code pick, and the highest level, which literally does try all of them.

There's separate logic to select whether (and how large) of a color cache to include. I haven't looked through precisely how it works yet, though.

[w-flo]

I like this approach.

I had considered some more, like using a Builder pattern, or only offering a params_mut() method like encoder.params_mut().use_predictor_pattern = false;. I wasn't sure which would work best so I just picked one randomly and thought I'd leave it up to you to figure out what the best way forward would be, and then update my PR as needed. :-) So let me know if you want me to change anything.

libwebp uses heuristics to select one of six combinations of transforms (They call the predictor transform "spatial" in that list). Except for the lowest compression level, where libwebp just has a single hard-code pick, and the highest level, which literally does try all of them.

There's separate logic to select whether (and how large) of a color cache to include. I haven't looked through precisely how it works yet, though.

Well, that does seem like a lot of work to get right, i.e. picking good trade-offs for the various speed settings. I'm glad the "only subtract green transform + big color cache" seems to work pretty decently for all the files I want to compress! Even at -z 9 (slowest), cwebp only shrinks the file size of one test file by 6% more and is really slow. So going forward, I hope image-webp still supports a fast way to say "don't do heuristics, just use these params, trust me!".

[w-flo]

Marking as "ready for review" now.

One thing about the params struct that may not be very future-proof is that users may want to specify what kind of predictor they want to use (Top, left, or one of the other predictors, or maybe even different predictors for different image regions). The boolean is not a good way to express this, so maybe a non-exhaustive enum like { Disabled, Top } for the current options might be better.