diff --git a/src/Drivers/Imagick/Modifiers/GrayscaleModifier.php b/src/Drivers/Imagick/Modifiers/GrayscaleModifier.php index 77112949c..dabcc8be2 100644 --- a/src/Drivers/Imagick/Modifiers/GrayscaleModifier.php +++ b/src/Drivers/Imagick/Modifiers/GrayscaleModifier.php @@ -4,6 +4,7 @@ namespace Intervention\Image\Drivers\Imagick\Modifiers; +use Imagick; use ImagickException; use Intervention\Image\Exceptions\ModifierException; use Intervention\Image\Interfaces\ImageInterface; @@ -19,15 +20,30 @@ public function apply(ImageInterface $image): ImageInterface { foreach ($image as $frame) { try { - $result = $frame->native()->modulateImage(100, 0, 100); + // read the colorspace of the frame to be able to restore it, + // getImageColorspace() only reports the frame the iterator is + // currently pointing at + $colorspace = $frame->native()->getImageColorspace(); + + // turn image to grayscale + $result = $frame->native()->transformImageColorspace(Imagick::COLORSPACE_GRAY); + if ($result === false) { + throw new ModifierException( + 'Failed to apply ' . self::class . ', unable to transform image to grayscale', + ); + } + + // return to the colorspace of the source, grayscale is a color + // operation and is not meant to move the image to another one + $result = $frame->native()->transformImageColorspace($colorspace); if ($result === false) { throw new ModifierException( - 'Failed to apply ' . self::class . ', unable to modulate image', + 'Failed to apply ' . self::class . ', unable to transform image to grayscale', ); } } catch (ImagickException $e) { throw new ModifierException( - 'Failed to apply ' . self::class . ', unable to modulate image', + 'Failed to apply ' . self::class . ', unable to transform image to grayscale', previous: $e, ); } diff --git a/tests/Unit/Drivers/Gd/Modifiers/GrayscaleModifierTest.php b/tests/Unit/Drivers/Gd/Modifiers/GrayscaleModifierTest.php index 7beaf7502..09fffa4a3 100644 --- a/tests/Unit/Drivers/Gd/Modifiers/GrayscaleModifierTest.php +++ b/tests/Unit/Drivers/Gd/Modifiers/GrayscaleModifierTest.php @@ -6,6 +6,7 @@ use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\RequiresPhpExtension; +use Intervention\Image\Colors\Rgb\Channels\Red; use Intervention\Image\Modifiers\GrayscaleModifier; use Intervention\Image\Tests\GdTestCase; @@ -21,4 +22,18 @@ public function testColorChange(): void $image->modify(new GrayscaleModifier()); $this->assertTrue($image->colorAt(0, 0)->isGrayscale()); } + + public function testSaturatedColorsKeepDistinctBrightness(): void + { + // blocks.png holds pure blue, green and red in three of its quadrants + $image = $this->readTestImage('blocks.png')->modify(new GrayscaleModifier()); + + $blue = $image->colorAt(160, 120)->channel(Red::class)->value(); + $green = $image->colorAt(160, 360)->channel(Red::class)->value(); + $red = $image->colorAt(480, 360)->channel(Red::class)->value(); + + // any luma based conversion keeps blue darkest and green brightest + $this->assertLessThan($red, $blue); + $this->assertLessThan($green, $red); + } } diff --git a/tests/Unit/Drivers/Imagick/Modifiers/GrayscaleModifierTest.php b/tests/Unit/Drivers/Imagick/Modifiers/GrayscaleModifierTest.php index 6ccf3e68e..ccdaf82bb 100644 --- a/tests/Unit/Drivers/Imagick/Modifiers/GrayscaleModifierTest.php +++ b/tests/Unit/Drivers/Imagick/Modifiers/GrayscaleModifierTest.php @@ -6,6 +6,17 @@ use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\RequiresPhpExtension; +use Imagick; +use ImagickPixel; +use Intervention\Image\Colors\Cmyk\Channels\Cyan; +use Intervention\Image\Colors\Cmyk\Channels\Key; +use Intervention\Image\Colors\Cmyk\Channels\Magenta; +use Intervention\Image\Colors\Cmyk\Channels\Yellow; +use Intervention\Image\Colors\Cmyk\Colorspace as CmykColorspace; +use Intervention\Image\Colors\Rgb\Channels\Red; +use Intervention\Image\Drivers\Imagick\Core; +use Intervention\Image\Drivers\Imagick\Driver; +use Intervention\Image\Image; use Intervention\Image\Modifiers\GrayscaleModifier; use Intervention\Image\Tests\ImagickTestCase; @@ -21,4 +32,71 @@ public function testColorChange(): void $image->modify(new GrayscaleModifier()); $this->assertTrue($image->colorAt(0, 0)->isGrayscale()); } + + public function testSaturatedColorsKeepDistinctBrightness(): void + { + // blocks.png holds pure blue, green and red in three of its quadrants + $image = $this->readTestImage('blocks.png')->modify(new GrayscaleModifier()); + + $blue = $image->colorAt(160, 120)->channel(Red::class)->value(); + $green = $image->colorAt(160, 360)->channel(Red::class)->value(); + $red = $image->colorAt(480, 360)->channel(Red::class)->value(); + + // any luma based conversion keeps blue darkest and green brightest + $this->assertLessThan($red, $blue); + $this->assertLessThan($green, $red); + } + + public function testColorspaceIsPreserved(): void + { + $image = $this->readTestImage('cmyk.jpg'); + $this->assertInstanceOf(CmykColorspace::class, $image->colorspace()); + + $image->modify(new GrayscaleModifier()); + + // grayscale is a color operation, it has no business moving a cmyk + // source to another colorspace + $this->assertEquals( + Imagick::COLORSPACE_CMYK, + $image->core()->frame(0)->native()->getImageColorspace(), + ); + + // the grey has to sit in the black channel, the three ink channels + // carry no color anymore + $color = $image->colorAt(0, 0); + $this->assertEquals(0, $color->channel(Cyan::class)->value()); + $this->assertEquals(0, $color->channel(Magenta::class)->value()); + $this->assertEquals(0, $color->channel(Yellow::class)->value()); + $this->assertEqualsWithDelta(43, $color->channel(Key::class)->value(), 2); + } + + public function testColorspaceIsPreservedOnEveryFrame(): void + { + // the frames deliberately do not share a colorspace, an implementation + // that reads the colorspace once outside the frame loop, or that + // hardcodes one, cannot satisfy this + $imagick = new Imagick(); + $imagick->setFormat('tiff'); + + foreach ([Imagick::COLORSPACE_SRGB, Imagick::COLORSPACE_CMYK] as $colorspace) { + $frame = new Imagick(); + $frame->newImage(4, 4, new ImagickPixel('red'), 'tiff'); + $frame->transformImageColorspace($colorspace); + $frame->setImageDelay(10); + $imagick->addImage($frame); + } + + $imagick->setFirstIterator(); + $image = new Image(new Driver(), new Core($imagick)); + + $image->modify(new GrayscaleModifier()); + + foreach ([Imagick::COLORSPACE_SRGB, Imagick::COLORSPACE_CMYK] as $key => $colorspace) { + $this->assertEquals( + $colorspace, + $image->core()->frame($key)->native()->getImageColorspace(), + 'Frame ' . $key . ' did not keep its colorspace', + ); + } + } }