Make Imagick and Gmagick adapters work with CMYK images

[crishoj]

Patch for #36

[crishoj]

Thank you for the feedback. I've pushed patches to address the points you raised.

[ksubileau]

Well done, it only remains to fix the two test failures. It seems we still don't get the right color with Imagick. GD returns the right color. Maybe it's because we have to use the RGB color space, and not SRGB.

[crishoj]

That's odd.

Both ImagickImageAdapterTest::testGetPixelColorFromCmykJpg and ColorThiefTest::testDominantColor pass in my development environments:

• PHP 7.2.7 on macOS 10.13.6 with ImageMagick 7.0.8-6 and GraphicsMagick 1.3.30
• PHP 7.2.7 on Ubuntu 18.04 LTS with ImageMagick 6.9.7-4 and GraphicsMagick 1.3.28

Changing ImagickImageAdapter to use RGB rather than SRGB seems to break other tests.

[crishoj]

The problem is reproducible on Ubuntu Trusty with PHP 7.2.7 and ImageMagick 6.7.7.

Specifically, on this stack, getting the pixel color from the CMYK image works when transforming to RGB (whereas only SRGB works on the newer stacks).

It seems there is an inconsistency between ImageMagick 6.7.7 and 6.9.7 in how this image is treated.

[crishoj]

This workaround isn't pretty, but perhaps a pragmatic solution in lieu of delving deeper into the version history of ImageMagick.

One issue remains: The ImageMagick bindings for PHP 5.3 on the CI box (PECL imagick version 2.3.0) lack the transformImageColorspace method (which was introduced in version 3.0.0). Without this, color sampling a CMYK image will not work.

Options:

• [ ] Conditionally skip the test case (namely for imagick version < 3.0.0)
• [ ] Require imagick version >= 3.0.0
• [x] Bump imagick from version 2.3.0 to version 3.0.0 on the CI box for PHP 5.3

Given that PECL imagick 3.0.0 was released in 2010, I think the latter seems reasonable.

[crishoj]

All green now ✅

[ksubileau]

Thank you for your analysis and your amazing work! My development environment for this project is a quite outdated and would need a refresh (Debian 7.8, PHP 5.5, Imagick 6.7.7). I can confirm that I had the same problem on this configuration as on your Ubuntu Trusty box, which is now fixed. I just added some comments on the code on some points that can still be discussed, but overall it seems ready for merging. Also maybe we should add a line in the requirements paragraph of the readme file about the minimum version of Imagick required for CMYK images. Just a reminder for later, or maybe for another PR if you have some extra time : since you have found differences in behavior depending on the version of the extensions, I wonder if it could be interesting to improve the CI to test the code against several versions of Imagick and Gmagick.

[crishoj]

Expanding the CI to test specific versions of both the imagick extension and the backing ImageMagick library would be good indeed:

• [ ] Ensure exception is thrown with for imagick <3.0 and a CMYK image
• [ ] Test RGB/SRGB quirk with a CMYK image for specific ImageMagick versions:
  • 6.7.7
  • some versions in between
  • 6.9.7
  • 7.*

[crishoj]

I suggest the CI expansion be done in a separate pull request.