add Image#TransformICCProfileWithFallback

[toaster]

This fixes #314.

Motivation:

As described in #314 the Image#TransformICCProfile does not give control over the fallback ICC profile. But there might be better options than an SRGB profile. For CMYK images the cmyk profile of vips might be used.

With this method, the caller can decide how to determine the fallback profile.

I’d love to provide tests but I am not sure how these would look like. Any guidance is appreciated.

Notes:

embedded is always set to true. As the vips documentation states this means that the embedded profile should be used as input profile iff it is present (https://www.libvips.org/API/current/libvips-colour.html#vips-icc-transform).

I think that Image#OptimizeICCProfile should be adjusted to call the new function, too. But this would change its behaviour (to the better, I guess).

[tonimelisma]

Hey @toaster! Thank you so much for this. I apologize sincerely - I must have missed this while I was travelling on vacation or something, I do try to merge PRs as soon as possible. This delay is embarrassing.

Can you look at some other recent PRs, many provide golden tests? If you have an Ubuntu 22.04 machine yourself you can try running the test suite to generate the golden reference images and submit them with your PR.

Would that be cool, if you added that? We've had some issues with ICC profiles before because there's some automagic behavior and I'd really love to have a test against this.

[toaster]

Hi @tonimelisma, sorry for the delay, I don’t read email regularly :).

I’ll try to find some time to add the tests within the next days.

[toaster]

If I understand the golden tests correctly, they perform some case specific assertions on the output image and also expect that the output image matches the golden master.
But AFAICS the latter does not actually work on the CI runs currently because the ubuntu-latest of GitHub is “jammy” while all the golden masters are “mantic”.

I suggest to let the tests fail if the master is missing (besides writing it), so that missing masters make the CI run fail.

Also, I think that golden masters should not be OS specific. I understand that there might be cases where they actually are because of some dependency of some OS behaviour. But these cases should be addressed by a different approach where the test explicitly performs the check with an OS specific master.

[tonimelisma]

Hey, that's not how the tests are run. All of the image processing library versions cause different variation in the exact image data even if for the sake of the test they would be passing, which is why we need the OS version. The OS version is in the file names exactly so the tests would know which image to test against.

If there's no reference golden image for that specific OS and library version the tests pass automatically.

[toaster]

Well, I still think that makes the golden images quite useless. Currently, your CI runs don’t check a single image. I don’t know how many contributors test on an Ubuntu Mantic with a specific libvips version but I assume its a minority.

I am aware of the problem of slightly different image bytes (even if the pixels are the same). In another project, I used to load the golden and the output into an image.Image and compare those. That still fails on slightly pixel diffs because of some lossy encoder optimization. These you can avoid by using comparison with a certain epsilon which allows for small divergences. After a couple of years these changes might pile up and finally fail a test but then you can simply have a look at the image and update the golden file.

Automatic tests are only useful if their outcome is reliable. A test which promises to check images against masters and doesn’t do so has no value.

[tonimelisma]

Thank you for the insightful observation. I've gone ahead and added the missing jammy images.

A more fuzzy matching algorithm would be awesome, thanks for the feature request! Would you have time to do a pull request?

[toaster]

Not in the short term.

[toaster]

I started working on the fuzzy differ. It slows down the golden image tests significantly.
Is this okay for you?

[toaster]

The CI run takes around ten minutes instead of three with the fuzzy differ applied.
I guess, this can be reduced by using smaller images where possible.

[toaster]

@tonimelisma I added tests for the new function and also fixed a bug with the new implementation which was revealed by the old tests.

I think, this is ready to review/merge now.

[coveralls]

coverage: 73.303% (-0.02%) from 73.322%
when pulling cead4f9 on toaster:improve_icc_handling
into 7f3b594 on davidbyttow:master.