Use Imagick class

[distantnative]

Description

• Help needed: Cannot figure out why ICC profile isn't preserved
• Unit tests methods that change the Imagick object
• Set CLI back to use latest ubuntu version

Summary of changes

• New Imagick class for imagick darkroom driver
• Old CLI version still available as im ( ImageMagick class)

Changelog

Enhancements

• New imagick thumbs driver, using the Imagick class instead of command line
  • imagick thumbs driver does not respect bin option

Deprecated

• im driver option, use imagick instead

Ready?

• In-code documentation (wherever needed)
• Unit tests for fixed bug/feature
• Tests and CI checks all pass

For review team

• Add lab and/or sandbox examples (wherever helpful)
• Add changes & docs to release notes draft in Notion

[afbora]

I don't have any idea about IM. I've run unit tests. Getting following error for all provider for ::testKeepColorProfileStripMeta()

identify: unknown image property "%[profile:icc]" @ warning/property.c/InterpretImageProperties/4238.

[bastianallgeier]

It's no help with the todos (sorry about that) but I was just thinking if we maybe could change the driver name to "imagick" for this and keep the old "im" driver? Then it wouldn't be a breaking change, but more important: the preferred name would be a lot better.

[rasteiner]

• Help needed: Cannot figure out why ICC profile isn't preserved

I have some doubts about calling stripImage() together with profileImage().

A short test for demonstration (not a solution):

function doTest(string $file, string $ext) {
    $image = new Imagick($file);

    // get the ICC profile before stripping
    $profiles = $image->getImageProfiles('icc', true);

    // strip all metadata
    $image->stripImage();

    // re-apply the ICC profile
    if ($icc = $profiles['icc'] ?? null) {
        $image->profileImage('icc', $icc);
    }

    // save the image
    file_put_contents('./output-wrong.' . $ext, $image);
    // destroy the image
    $image->destroy();


    // re-open the output image and apply the ICC profile without calling stripImage
    $image = new Imagick('./output-wrong.' . $ext);
    if ($icc = $profiles['icc'] ?? null) {
        $image->profileImage('icc', $icc);
    }

    // save the image
    file_put_contents('./output-correct.' . $ext, $image);
    // destroy the image
    $image->destroy();
}

doTest('./png-adobe-rgb-gps.png', 'png');
doTest('./onigiri-adobe-rgb-gps.jpg', 'jpg');

Execute this in a folder with the png-adobe-rgb-gps.png and onigiri-adobe-rgb-gps.jpg images. You should end up with two sets of output-wrong.* a output-correct.* files. The "output-wrong" files do what you're doing in Kirby, the "output-correct" files are made by opening the "output-wrong" files and only re-adding the ICC profile.

The ICC profiles being present in:

	output-wrong	output-correct
png	❌	✅
jpg	✅	✅

I know the top comment in the PHP guide about stripImage() suggests doing exactly this, but it doesn't seem to work, at least not for PNG files.

Imho this looks like a bug in either Imagick or ImageMagick, because other file formats seem to work ootb.

[distantnative]

@rasteiner Thanks for sharing your insights, much appreciated.

So to make our use case work (keep color profile but strip all other metadata) we would have to write the thumb basically twice - at least for PNG.

[rasteiner]

So to make our use case work (keep color profile but strip all other metadata) we would have to write the thumb basically twice - at least for PNG.

I don't know if that can be a valid workaround, encoding the image twice surely isn't very optimal for a CMS...

FYI this seems to be an issue in ImageMagick, not the imagick extension. The same behaviour issue also shows with the command line:

identify -format %[profile:icc]\\n png-adobe-rgb-gps.png # --> Adobe RGB (1998)
convert png-adobe-rgb-gps.png extracted.icc
convert png-adobe-rgb-gps.png -strip -profile extracted.icc clean.png
identify -format %[profile:icc]\\n clean.png # --> unknown image property "%[profile:icc]"

I've also posted this here:
https://github.com/ImageMagick/ImageMagick/discussions/7944

Anyway, if it's really a bug in imagemagick, I don't know if it can be fixed in a useful timespan since it's 2 steps upstream (Kirby -> Imagick -> Magick C API) which is then compiled who knows when by unknown hosting providers :/

So unless we find a better workaround (maybe the discussion in ImageMagick leads to something), maybe encoding it twice might actually be the only solution, in that case we should only do it when writing a png file with a profile: as converting a PNG with profile to JPG/WEBP seems to work fine. Saving to avif also seems to skip the profile, but I'm not sure if that's the same problem

[distantnative]

@rasteiner yea I guess we cannot expect a fix there but have to work around it ourselves. @afbora has pushed a working version that avoids writing to the filesystem but making use of getImageBloc/readImageBlob.