From de66c370602266a11480b53d1eb5a0cd1e288d69 Mon Sep 17 00:00:00 2001 From: Oliver Vogel Date: Fri, 10 Jan 2025 16:43:46 +0100 Subject: [PATCH 01/10] Refactor imagick driver CropModifier::class --- .../Imagick/Modifiers/CropModifier.php | 81 +++++-------------- 1 file changed, 18 insertions(+), 63 deletions(-) diff --git a/src/Drivers/Imagick/Modifiers/CropModifier.php b/src/Drivers/Imagick/Modifiers/CropModifier.php index 067c1fd44..dd2815465 100644 --- a/src/Drivers/Imagick/Modifiers/CropModifier.php +++ b/src/Drivers/Imagick/Modifiers/CropModifier.php @@ -4,8 +4,7 @@ namespace Intervention\Image\Drivers\Imagick\Modifiers; -use ImagickDraw; -use ImagickPixel; +use Imagick; use Intervention\Image\Interfaces\ImageInterface; use Intervention\Image\Interfaces\SpecializedInterface; use Intervention\Image\Modifiers\CropModifier as GenericCropModifier; @@ -14,80 +13,36 @@ class CropModifier extends GenericCropModifier implements SpecializedInterface { public function apply(ImageInterface $image): ImageInterface { - $originalSize = $image->size(); $crop = $this->crop($image); $background = $this->driver()->colorProcessor($image->colorspace())->colorToNative( $this->driver()->handleInput($this->background) ); - $transparent = new ImagickPixel('transparent'); - - $draw = new ImagickDraw(); - $draw->setFillColor($background); + $imagick = new Imagick(); foreach ($image as $frame) { - $frame->native()->setBackgroundColor($transparent); - $frame->native()->setImageBackgroundColor($transparent); - - // crop image - $frame->native()->extentImage( - $crop->width(), - $crop->height(), - $crop->pivot()->x() + $this->offset_x, - $crop->pivot()->y() + $this->offset_y + $canvas = new Imagick(); + $canvas->newImage($crop->width(), $crop->height(), $background, 'png'); + + $canvas->compositeImage( + $frame->native(), + Imagick::COMPOSITE_OVER, + ($crop->pivot()->x() + $this->offset_x) * -1, + ($crop->pivot()->y() + $this->offset_y) * -1, ); - // repage - $frame->native()->setImagePage( - $crop->width(), - $crop->height(), - 0, - 0, + $canvas->compositeImage( + $frame->native(), + Imagick::COMPOSITE_COPYOPACITY, + ($crop->pivot()->x() + $this->offset_x) * -1, + ($crop->pivot()->y() + $this->offset_y) * -1, ); - // cover the possible newly created areas with background color - if ($crop->width() > $originalSize->width() || $this->offset_x > 0) { - $draw->rectangle( - $originalSize->width() + ($this->offset_x * -1) - $crop->pivot()->x(), - 0, - $crop->width(), - $crop->height() - ); - } - - // cover the possible newly created areas with background color - if ($crop->height() > $originalSize->height() || $this->offset_y > 0) { - $draw->rectangle( - ($this->offset_x * -1) - $crop->pivot()->x(), - $originalSize->height() + ($this->offset_y * -1) - $crop->pivot()->y(), - ($this->offset_x * -1) + $originalSize->width() - 1 - $crop->pivot()->x(), - $crop->height() - ); - } - - // cover the possible newly created areas with background color - if ((($this->offset_x * -1) - $crop->pivot()->x() - 1) > 0) { - $draw->rectangle( - 0, - 0, - ($this->offset_x * -1) - $crop->pivot()->x() - 1, - $crop->height() - ); - } - - // cover the possible newly created areas with background color - if ((($this->offset_y * -1) - $crop->pivot()->y() - 1) > 0) { - $draw->rectangle( - ($this->offset_x * -1) - $crop->pivot()->x(), - 0, - ($this->offset_x * -1) + $originalSize->width() - $crop->pivot()->x() - 1, - ($this->offset_y * -1) - $crop->pivot()->y() - 1, - ); - } - - $frame->native()->drawImage($draw); + $imagick->addImage($canvas); } + $image->core()->setNative($imagick); + return $image; } } From a0b27ef806163a713e1ee749453c3bf76a80f994 Mon Sep 17 00:00:00 2001 From: Oliver Vogel Date: Fri, 10 Jan 2025 16:54:47 +0100 Subject: [PATCH 02/10] Fix copying of alpha channel for different Imagick versions --- src/Drivers/Imagick/Modifiers/CropModifier.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Drivers/Imagick/Modifiers/CropModifier.php b/src/Drivers/Imagick/Modifiers/CropModifier.php index dd2815465..7fb29ac7b 100644 --- a/src/Drivers/Imagick/Modifiers/CropModifier.php +++ b/src/Drivers/Imagick/Modifiers/CropModifier.php @@ -33,7 +33,7 @@ public function apply(ImageInterface $image): ImageInterface $canvas->compositeImage( $frame->native(), - Imagick::COMPOSITE_COPYOPACITY, + $this->imagemagickMajorVersion() <= 6 ? Imagick::COMPOSITE_DSTIN : Imagick::COMPOSITE_COPYOPACITY, ($crop->pivot()->x() + $this->offset_x) * -1, ($crop->pivot()->y() + $this->offset_y) * -1, ); @@ -45,4 +45,13 @@ public function apply(ImageInterface $image): ImageInterface return $image; } + + private function imagemagickMajorVersion(): int + { + if (preg_match('/^ImageMagick (?P[0-9]+)\./', Imagick::getVersion()['versionString'], $matches) != 1) { + return 0; + } + + return intval($matches['major'] ?? 0); + } } From 3ac8c6725c5bdc2b92cf436e728b92309d880da9 Mon Sep 17 00:00:00 2001 From: Oliver Vogel Date: Fri, 10 Jan 2025 17:01:34 +0100 Subject: [PATCH 03/10] Fix analyzer --- src/Drivers/Imagick/Modifiers/CropModifier.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Drivers/Imagick/Modifiers/CropModifier.php b/src/Drivers/Imagick/Modifiers/CropModifier.php index 7fb29ac7b..d5b673c46 100644 --- a/src/Drivers/Imagick/Modifiers/CropModifier.php +++ b/src/Drivers/Imagick/Modifiers/CropModifier.php @@ -52,6 +52,6 @@ private function imagemagickMajorVersion(): int return 0; } - return intval($matches['major'] ?? 0); + return intval($matches['major']); } } From f5027b2da535d8e56a1998c32d92b8c2a81c4644 Mon Sep 17 00:00:00 2001 From: Oliver Vogel Date: Fri, 10 Jan 2025 17:18:45 +0100 Subject: [PATCH 04/10] Keep image resolutionin CropModifier::class --- src/Drivers/Imagick/Modifiers/CropModifier.php | 2 ++ .../Imagick/Modifiers/CropModifierTest.php | 17 +++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/src/Drivers/Imagick/Modifiers/CropModifier.php b/src/Drivers/Imagick/Modifiers/CropModifier.php index d5b673c46..18fef3256 100644 --- a/src/Drivers/Imagick/Modifiers/CropModifier.php +++ b/src/Drivers/Imagick/Modifiers/CropModifier.php @@ -19,10 +19,12 @@ public function apply(ImageInterface $image): ImageInterface ); $imagick = new Imagick(); + $resolution = $image->resolution()->perInch(); foreach ($image as $frame) { $canvas = new Imagick(); $canvas->newImage($crop->width(), $crop->height(), $background, 'png'); + $canvas->setImageResolution($resolution->x(), $resolution->y()); $canvas->compositeImage( $frame->native(), diff --git a/tests/Unit/Drivers/Imagick/Modifiers/CropModifierTest.php b/tests/Unit/Drivers/Imagick/Modifiers/CropModifierTest.php index 2b3691f7c..73d74550f 100644 --- a/tests/Unit/Drivers/Imagick/Modifiers/CropModifierTest.php +++ b/tests/Unit/Drivers/Imagick/Modifiers/CropModifierTest.php @@ -4,6 +4,7 @@ namespace Intervention\Image\Tests\Unit\Drivers\Imagick\Modifiers; +use Intervention\Image\Colors\Cmyk\Colorspace; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\RequiresPhpExtension; use Intervention\Image\Modifiers\CropModifier; @@ -36,4 +37,20 @@ public function testModifyExtend(): void $this->assertColor(0, 0, 255, 255, $image->pickColor(445, 16)); $this->assertTransparency($image->pickColor(460, 16)); } + + public function testModifyKeepsColorspace(): void + { + $image = $this->readTestImage('cmyk.jpg'); + $this->assertInstanceOf(Colorspace::class, $image->colorspace()); + $image = $image->modify(new CropModifier(800, 100, -10, -10, 'ff0000')); + $this->assertInstanceOf(Colorspace::class, $image->colorspace()); + } + + public function testModifyKeepsResolution(): void + { + $image = $this->readTestImage('300dpi.png'); + $this->assertEquals(300, round($image->resolution()->perInch()->x())); + $image = $image->modify(new CropModifier(800, 100, -10, -10, 'ff0000')); + $this->assertEquals(300, round($image->resolution()->perInch()->x())); + } } From 955563a9a0ee50846989d58160fb889ccb4d74cb Mon Sep 17 00:00:00 2001 From: Oliver Vogel Date: Fri, 10 Jan 2025 17:26:06 +0100 Subject: [PATCH 05/10] Add doc block --- tests/ImagickTestCase.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/ImagickTestCase.php b/tests/ImagickTestCase.php index bef7b6745..e88bbadc8 100644 --- a/tests/ImagickTestCase.php +++ b/tests/ImagickTestCase.php @@ -5,6 +5,7 @@ namespace Intervention\Image\Tests; use Imagick; +use ImagickException; use ImagickPixel; use Intervention\Image\Decoders\FilePathImageDecoder; use Intervention\Image\Drivers\Imagick\Core; @@ -20,6 +21,14 @@ public static function readTestImage($filename = 'test.jpg'): Image ); } + /** + * Create test image with red (#ff0000) background + * + * @param int $width + * @param int $height + * @return Image + * @throws ImagickException + */ public static function createTestImage(int $width, int $height): Image { $background = new ImagickPixel('rgb(255, 0, 0)'); From e0cc42c7c487d24eedeeb73cfa1f469add187a9e Mon Sep 17 00:00:00 2001 From: Oliver Vogel Date: Fri, 10 Jan 2025 17:26:13 +0100 Subject: [PATCH 06/10] Add test for CropModifier::class --- .../Drivers/Imagick/Modifiers/CropModifierTest.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/Unit/Drivers/Imagick/Modifiers/CropModifierTest.php b/tests/Unit/Drivers/Imagick/Modifiers/CropModifierTest.php index 73d74550f..d43324a19 100644 --- a/tests/Unit/Drivers/Imagick/Modifiers/CropModifierTest.php +++ b/tests/Unit/Drivers/Imagick/Modifiers/CropModifierTest.php @@ -38,6 +38,19 @@ public function testModifyExtend(): void $this->assertTransparency($image->pickColor(460, 16)); } + public function testModifySinglePixel(): void + { + $image = $this->createTestImage(1, 1); + $this->assertEquals(1, $image->width()); + $this->assertEquals(1, $image->height()); + $image->modify(new CropModifier(3, 3, 0, 0, 'ff0', 'center')); + $this->assertEquals(3, $image->width()); + $this->assertEquals(3, $image->height()); + $this->assertColor(255, 255, 0, 255, $image->pickColor(0, 0)); + $this->assertColor(255, 0, 0, 255, $image->pickColor(1, 1)); + $this->assertColor(255, 255, 0, 255, $image->pickColor(2, 2)); + } + public function testModifyKeepsColorspace(): void { $image = $this->readTestImage('cmyk.jpg'); From 9921ab86d9e2dd66cf73324eabcde2510b193b17 Mon Sep 17 00:00:00 2001 From: Oliver Vogel Date: Fri, 10 Jan 2025 17:42:27 +0100 Subject: [PATCH 07/10] Fix bug --- src/Drivers/Imagick/Modifiers/CropModifier.php | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/Drivers/Imagick/Modifiers/CropModifier.php b/src/Drivers/Imagick/Modifiers/CropModifier.php index 18fef3256..a6744f127 100644 --- a/src/Drivers/Imagick/Modifiers/CropModifier.php +++ b/src/Drivers/Imagick/Modifiers/CropModifier.php @@ -33,12 +33,14 @@ public function apply(ImageInterface $image): ImageInterface ($crop->pivot()->y() + $this->offset_y) * -1, ); - $canvas->compositeImage( - $frame->native(), - $this->imagemagickMajorVersion() <= 6 ? Imagick::COMPOSITE_DSTIN : Imagick::COMPOSITE_COPYOPACITY, - ($crop->pivot()->x() + $this->offset_x) * -1, - ($crop->pivot()->y() + $this->offset_y) * -1, - ); + if ($frame->native()->getImageAlphaChannel()) { + $canvas->compositeImage( + $frame->native(), + $this->imagemagickMajorVersion() <= 6 ? Imagick::COMPOSITE_DSTIN : Imagick::COMPOSITE_COPYOPACITY, + ($crop->pivot()->x() + $this->offset_x) * -1, + ($crop->pivot()->y() + $this->offset_y) * -1, + ); + } $imagick->addImage($canvas); } From d40fdd1aeb62acbc1719cb07e19073c0cbf715d7 Mon Sep 17 00:00:00 2001 From: Oliver Vogel Date: Fri, 10 Jan 2025 17:57:31 +0100 Subject: [PATCH 08/10] Re-apply animation details --- src/Drivers/Imagick/Modifiers/CropModifier.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/Drivers/Imagick/Modifiers/CropModifier.php b/src/Drivers/Imagick/Modifiers/CropModifier.php index a6744f127..e5e1421cd 100644 --- a/src/Drivers/Imagick/Modifiers/CropModifier.php +++ b/src/Drivers/Imagick/Modifiers/CropModifier.php @@ -18,14 +18,24 @@ public function apply(ImageInterface $image): ImageInterface $this->driver()->handleInput($this->background) ); + // create empty container imagick to rebuild core $imagick = new Imagick(); $resolution = $image->resolution()->perInch(); foreach ($image as $frame) { + // create new frame canvas with modifiers background $canvas = new Imagick(); $canvas->newImage($crop->width(), $crop->height(), $background, 'png'); $canvas->setImageResolution($resolution->x(), $resolution->y()); + // set animation details + if ($image->isAnimated()) { + $canvas->setImageDelay($frame->native()->getImageDelay()); + $canvas->setImageIterations($frame->native()->getImageIterations()); + $canvas->setImageDispose($frame->native()->getImageDispose()); + } + + // place original frame content onto the empty colored frame canvas $canvas->compositeImage( $frame->native(), Imagick::COMPOSITE_OVER, @@ -33,6 +43,7 @@ public function apply(ImageInterface $image): ImageInterface ($crop->pivot()->y() + $this->offset_y) * -1, ); + // copy alpha channel if available if ($frame->native()->getImageAlphaChannel()) { $canvas->compositeImage( $frame->native(), @@ -42,9 +53,11 @@ public function apply(ImageInterface $image): ImageInterface ); } + // add newly built frame to container imagick $imagick->addImage($canvas); } + // replace imagick $image->core()->setNative($imagick); return $image; From cc0b332b03eb563a63fb357dcb28f6aa66df621e Mon Sep 17 00:00:00 2001 From: Oliver Vogel Date: Sat, 11 Jan 2025 09:04:25 +0100 Subject: [PATCH 09/10] Refactor driver version detection --- src/Drivers/Imagick/Modifiers/CropModifier.php | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/Drivers/Imagick/Modifiers/CropModifier.php b/src/Drivers/Imagick/Modifiers/CropModifier.php index e5e1421cd..0b9df68d9 100644 --- a/src/Drivers/Imagick/Modifiers/CropModifier.php +++ b/src/Drivers/Imagick/Modifiers/CropModifier.php @@ -5,6 +5,7 @@ namespace Intervention\Image\Drivers\Imagick\Modifiers; use Imagick; +use Intervention\Image\Drivers\Imagick\Driver; use Intervention\Image\Interfaces\ImageInterface; use Intervention\Image\Interfaces\SpecializedInterface; use Intervention\Image\Modifiers\CropModifier as GenericCropModifier; @@ -47,7 +48,9 @@ public function apply(ImageInterface $image): ImageInterface if ($frame->native()->getImageAlphaChannel()) { $canvas->compositeImage( $frame->native(), - $this->imagemagickMajorVersion() <= 6 ? Imagick::COMPOSITE_DSTIN : Imagick::COMPOSITE_COPYOPACITY, + version_compare(Driver::version(), '7.0.0', '>=') ? + Imagick::COMPOSITE_COPYOPACITY : + Imagick::COMPOSITE_DSTIN, ($crop->pivot()->x() + $this->offset_x) * -1, ($crop->pivot()->y() + $this->offset_y) * -1, ); @@ -62,13 +65,4 @@ public function apply(ImageInterface $image): ImageInterface return $image; } - - private function imagemagickMajorVersion(): int - { - if (preg_match('/^ImageMagick (?P[0-9]+)\./', Imagick::getVersion()['versionString'], $matches) != 1) { - return 0; - } - - return intval($matches['major']); - } } From bcc055fdcb43ee9f00f689666d982a31f9a311d4 Mon Sep 17 00:00:00 2001 From: Oliver Vogel Date: Sat, 11 Jan 2025 09:27:06 +0100 Subject: [PATCH 10/10] Simplify GD's CropModifier::class --- src/Drivers/Gd/Modifiers/CropModifier.php | 63 +++---------------- .../Drivers/Gd/Modifiers/CropModifierTest.php | 21 +++++++ 2 files changed, 28 insertions(+), 56 deletions(-) diff --git a/src/Drivers/Gd/Modifiers/CropModifier.php b/src/Drivers/Gd/Modifiers/CropModifier.php index fabdf7f77..c1ea48252 100644 --- a/src/Drivers/Gd/Modifiers/CropModifier.php +++ b/src/Drivers/Gd/Modifiers/CropModifier.php @@ -6,6 +6,7 @@ use Intervention\Image\Drivers\Gd\Cloner; use Intervention\Image\Exceptions\ColorException; +use Intervention\Image\Interfaces\ColorInterface; use Intervention\Image\Interfaces\FrameInterface; use Intervention\Image\Interfaces\ImageInterface; use Intervention\Image\Interfaces\SizeInterface; @@ -23,9 +24,7 @@ public function apply(ImageInterface $image): ImageInterface { $originalSize = $image->size(); $crop = $this->crop($image); - $background = $this->driver()->colorProcessor($image->colorspace())->colorToNative( - $this->driver()->handleInput($this->background) - ); + $background = $this->driver()->handleInput($this->background); foreach ($image as $frame) { $this->cropFrame($frame, $originalSize, $crop, $background); @@ -41,10 +40,10 @@ protected function cropFrame( FrameInterface $frame, SizeInterface $originalSize, SizeInterface $resizeTo, - int $background + ColorInterface $background ): void { // create new image with transparent background - $modified = Cloner::cloneEmpty($frame->native(), $resizeTo); + $modified = Cloner::cloneEmpty($frame->native(), $resizeTo, $background); // define offset $offset_x = $resizeTo->pivot()->x() + $this->offset_x; @@ -56,6 +55,9 @@ protected function cropFrame( $targetWidth = $targetWidth < $originalSize->width() ? $targetWidth + $offset_x : $targetWidth; $targetHeight = $targetHeight < $originalSize->height() ? $targetHeight + $offset_y : $targetHeight; + // don't alpha blend for copy operation to keep transparent areas of original image + imagealphablending($modified, false); + // copy content from resource imagecopyresampled( $modified, @@ -70,57 +72,6 @@ protected function cropFrame( $targetHeight ); - // don't alpha blend for covering areas - imagealphablending($modified, false); - - // cover the possible newly created areas with background color - if ($resizeTo->width() > $originalSize->width() || $this->offset_x > 0) { - imagefilledrectangle( - $modified, - $originalSize->width() + ($this->offset_x * -1) - $resizeTo->pivot()->x(), - 0, - $resizeTo->width(), - $resizeTo->height(), - $background - ); - } - - // cover the possible newly created areas with background color - if ($resizeTo->height() > $originalSize->height() || $this->offset_y > 0) { - imagefilledrectangle( - $modified, - ($this->offset_x * -1) - $resizeTo->pivot()->x(), - $originalSize->height() + ($this->offset_y * -1) - $resizeTo->pivot()->y(), - ($this->offset_x * -1) + $originalSize->width() - 1 - $resizeTo->pivot()->x(), - $resizeTo->height(), - $background - ); - } - - // cover the possible newly created areas with background color - if ((($this->offset_x * -1) - $resizeTo->pivot()->x() - 1) > 0) { - imagefilledrectangle( - $modified, - 0, - 0, - ($this->offset_x * -1) - $resizeTo->pivot()->x() - 1, - $resizeTo->height(), - $background - ); - } - - // cover the possible newly created areas with background color - if ((($this->offset_y * -1) - $resizeTo->pivot()->y() - 1) > 0) { - imagefilledrectangle( - $modified, - ($this->offset_x * -1) - $resizeTo->pivot()->x(), - 0, - ($this->offset_x * -1) + $originalSize->width() - $resizeTo->pivot()->x() - 1, - ($this->offset_y * -1) - $resizeTo->pivot()->y() - 1, - $background - ); - } - // set new content as resource $frame->setNative($modified); } diff --git a/tests/Unit/Drivers/Gd/Modifiers/CropModifierTest.php b/tests/Unit/Drivers/Gd/Modifiers/CropModifierTest.php index 4a5e268db..673405644 100644 --- a/tests/Unit/Drivers/Gd/Modifiers/CropModifierTest.php +++ b/tests/Unit/Drivers/Gd/Modifiers/CropModifierTest.php @@ -36,4 +36,25 @@ public function testModifyExtend(): void $this->assertColor(0, 0, 255, 255, $image->pickColor(445, 16)); $this->assertTransparency($image->pickColor(460, 16)); } + + public function testModifySinglePixel(): void + { + $image = $this->createTestImage(1, 1); + $this->assertEquals(1, $image->width()); + $this->assertEquals(1, $image->height()); + $image->modify(new CropModifier(3, 3, 0, 0, 'ff0', 'center')); + $this->assertEquals(3, $image->width()); + $this->assertEquals(3, $image->height()); + $this->assertColor(255, 255, 0, 255, $image->pickColor(0, 0)); + $this->assertColor(255, 0, 0, 255, $image->pickColor(1, 1)); + $this->assertColor(255, 255, 0, 255, $image->pickColor(2, 2)); + } + + public function testModifyKeepsResolution(): void + { + $image = $this->readTestImage('300dpi.png'); + $this->assertEquals(300, round($image->resolution()->perInch()->x())); + $image = $image->modify(new CropModifier(800, 100, -10, -10, 'ff0000')); + $this->assertEquals(300, round($image->resolution()->perInch()->x())); + } }