ksubileau / color-thief-php

Grabs the dominant color or a representative color palette from an image. Uses PHP and GD, Imagick or Gmagick.
MIT License
633 stars 64 forks source link

Clearly dominant color is not extracted #36

Closed crishoj closed 6 years ago

crishoj commented 6 years ago

Consider the following image:

margrethe-hjort-hay-2018-margrethes-perler-indbundet-bog

A visual inspection leads me to believe that the dominant color is yellow:

image

However, this color is not among the dominant colors extracted by ColorThief:

image

At first I thought the bright yellow was misidentified as white-ish and thus discarded. However, in a test case, isNonWhite returns true for (253, 230, 44)...

crishoj commented 6 years ago

The problem seems to lie in color extraction:

There were 2 failures:

1) ColorThief\Image\Adapter\Test\GmagickImageAdapterTest::testGetPixelColorFromCmykJpg
Failed asserting that two objects are equal.
--- Expected
+++ Actual
@@ @@
 stdClass Object (
-    'red' => 255
-    'green' => 229
-    'blue' => 44
+    'red' => 0
+    'green' => 26
+    'blue' => 211
     'alpha' => 0
 )

/Users/christian/Code/color-thief-php/tests/Image/Adapter/BaseImageAdapterTest.php:186

2) ColorThief\Image\Adapter\Test\ImagickImageAdapterTest::testGetPixelColorFromCmykJpg
Failed asserting that two objects are equal.
--- Expected
+++ Actual
@@ @@
 stdClass Object (
-    'red' => 255
-    'green' => 229
-    'blue' => 44
-    'alpha' => 0
+    'red' => 0.0
+    'green' => 26.0
+    'blue' => 211.0
+    'alpha' => 0.0
 )

/Users/christian/Code/color-thief-php/tests/Image/Adapter/BaseImageAdapterTest.php:186
crishoj commented 6 years ago

GDImageAdapter passes, but GmagickImageAdapter and ImagickImageAdapter both fail:

$ vendor/bin/phpunit tests/Image/Adapter --testdox

ColorThief\Image\Adapter\Test\GDImageAdapter
 [x] Get pixel color
 [x] Get pixel color from cmyk jpg

ColorThief\Image\Adapter\Test\GmagickImageAdapter
 [x] Get pixel color
 [ ] Get pixel color from cmyk jpg

ColorThief\Image\Adapter\Test\ImagickImageAdapter
 [x] Get pixel color
 [ ] Get pixel color from cmyk jpg

At some point the following warning is issued: Unsupported marker type 0x79

ksubileau commented 6 years ago

Hi, Indeed, the isNonWhite function simply return true when all channel values are greater than a constant threshold. Nothing very magical, so it's not surprising that this yellow color is not considered as white-ish.

Looking at you last comment, this seems to be a problem/bug with the underlying library (Imagick/Gmagick), or a misuse of it, as the getPixelColor function is simply a wrapper around it.

Could you try to extract the pixel color outside of ColorThief, using only Imagick functions, and see if you get the same wrong pixel values ? The code should be something like this (pseudo-code) :

$resource = new Imagick("/path/to/image.jpg");
$pixel = $resource->getImagePixelColor(20, 20);
$colorArray = $pixel->getColor(true);
var_dump($colorArray);
crishoj commented 6 years ago

Hello there! I've added a test case specifically for the color extraction: https://github.com/crishoj/color-thief-php/commit/423a20904f942bf1ed049965159d6b634f7b81e2

It passes for GD, but fails for both Imagick and Gmagick. Something makes me think it has to do with CMYK, but it's pure speculation.

crishoj commented 6 years ago

Could you try to extract the pixel color outside of ColorThief, using only Imagick functions, and see if you get the same wrong pixel values ?

php > $resource = new Imagick("tests/images/bookcover.jpg");
php > $pixel = $resource->getImagePixelColor(20, 20);
php > $colorArray = $pixel->getColor(true);
php > var_dump($colorArray);
php shell code:1:
array(4) {
  'r' =>
  double(0)
  'g' =>
  double(0.10196078431373)
  'b' =>
  double(0.82745098039216)
  'a' =>
  double(1)
}
php > var_dump([$colorArray['r'] * 255, $colorArray['g'] * 255, $colorArray['b'] * 255]);
php shell code:1:
array(3) {
  [0] =>
  double(0)
  [1] =>
  double(26)
  [2] =>
  double(211)
}
ksubileau commented 6 years ago

Great, so indeed we get the same wrong values... But I don't see anything in the PHP doc saying that these functions don't work with CMYK images. And what if we convert the colorspace to RGB before extracting colors ?

$resource = new Imagick("/path/to/image.jpg");
$resource->transformimagecolorspace(imagick::COLORSPACE_RGB);
$pixel = $resource->getImagePixelColor(20, 20);
$colorArray = $pixel->getColor(true);
var_dump($colorArray);

Also can we make sure this is not an issue specific to this image but all CMYK images ?

crishoj commented 6 years ago

Transforming the colorspace to SGRB fixes the problem with Imagick, but seems to break other test cases with Gmagick.

ksubileau commented 6 years ago

Hmm ok, so we need to investigate more to find the right way to get pixel color of CMYK images with Imagick/Gmagick

crishoj commented 6 years ago

The trouble with Gmagick was caused by using SRGB rather than RGB. Tests now pass. I'll send a pull request.

ksubileau commented 6 years ago

Closed by PR #37