CP-17510 image cursor

CHANGELOG-5.0.md

@@ -26,6 +26,10 @@ All notable changes are documented here. The format is based on [Keep a Changelo
 - Fixed `Phalcon\Filter\Validation\Validator\StringLength\Min` and `Phalcon\Filter\Validation\Validator\StringLength\Max` rejecting a string with a length exactly equal to `min` or `max` when the `included` option was not set, a regression introduced in 5.7.0 [#17503](https://github.com/phalcon/cphalcon/issues/17503) [[doc]](https://docs.phalcon.io/5.19/filter-validation/)
 - Fixed `Phalcon\Filter\Validation\Validator\StringLength` giving the `includedMinimum`/`includedMaximum` and `messageMinimum`/`messageMaximum` option of one boundary to the validator of the other boundary [#17503](https://github.com/phalcon/cphalcon/issues/17503) [[doc]](https://docs.phalcon.io/5.19/filter-validation/)
 - Fixed `Phalcon\Html\Helper\Input\Generic`, `Phalcon\Html\Helper\Input\Checkbox` and `Phalcon\Html\Helper\Input\Radio` throwing an error when you build them directly without a `Phalcon\Html\Helper\Doctype`. [#17507](https://github.com/phalcon/cphalcon/issues/17507) [[doc]](https://docs.phalcon.io/5.19/html-tagfactory/)
+- Fixed `Phalcon\Image\Adapter\Imagick::processReflection()` applying the reflection to the first frame only of a multi-frame image, because the first two loops changed the `reflection` clone but moved the frame cursor of `this->image` [#17510](https://github.com/phalcon/cphalcon/issues/17510) [[doc]](https://docs.phalcon.io/5.19/image/)
+- Fixed `Phalcon\Image\Adapter\Imagick::background()`, `reflection()`, `text()` and `watermark()` discarding any opacity below 100, and `sharpen()` rounding its amount down, because the division by 100 was stored back in the integer parameter; `watermark()` left the image unchanged for every opacity except 100 [#17510](https://github.com/phalcon/cphalcon/issues/17510) [[doc]](https://docs.phalcon.io/5.19/image/)
+- Fixed `Phalcon\Image\Adapter\Imagick` never coalescing the frames of a GIF, because the constructor compared `getImageType()`, an Imagick `IMGTYPE_*` value, against `IMAGETYPE_GIF`; the width and the height of an animated GIF now describe the canvas instead of whichever sub frame the cursor was on [#17510](https://github.com/phalcon/cphalcon/issues/17510) [[doc]](https://docs.phalcon.io/5.19/image/)
+- Fixed `Phalcon\Image\Adapter\Imagick::render()` returning the current frame alone for a GIF, and `save()` failing with `no encode delegate for this image format` after an operation that rebuilds the image; both now mark every frame with the format, which `setImageFormat()` applies to the current frame only [#17510](https://github.com/phalcon/cphalcon/issues/17510) [[doc]](https://docs.phalcon.io/5.19/image/)
 
 ### Removed
 

phalcon/Image/Adapter/Imagick.zep

@@ -103,9 +103,10 @@ class Imagick extends AbstractAdapter
             let this->type = this->image->getImageType();
 
             /**
-             * GIF
+             * GIF. The format, not the image type: getImageType() reports an
+             * Imagick IMGTYPE_* value, which never equals an IMAGETYPE_* one.
              */
-            if (this->image->getImageType() == IMAGETYPE_GIF) {
+            if ("GIF" === strtoupper(this->image->getImageFormat())) {
                 let image = this->image->coalesceImages();
 
                 this->image->clear();
@@ -248,12 +249,11 @@ class Imagick extends AbstractAdapter
     ) -> void {
         var background, color, image, localOpacity, pixel1, pixel2, result;
 
-        let localOpacity  = opacity;
-        let localOpacity /= 100;
-        let color         = sprintf("rgb(%d, %d, %d)", red, green, blue);
-        let pixel1        = new ImagickPixel(color);
-        let pixel2        = new ImagickPixel("transparent");
-        let background    = new ImagickNative();
+        let localOpacity = (float) opacity / 100;
+        let color        = sprintf("rgb(%d, %d, %d)", red, green, blue);
+        let pixel1       = new ImagickPixel(color);
+        let pixel2       = new ImagickPixel("transparent");
+        let background   = new ImagickNative();
 
         /** @var ImagickNative $image */
         let image = this->image;
@@ -465,7 +465,7 @@ class Imagick extends AbstractAdapter
         int opacity,
         bool fadeIn
     ) -> void {
-        var current, fade, image, pixel, pseudo, reflection, result;
+        var current, fade, fadeOpacity, image, pixel, pseudo, reflection, result;
 
         /** @var ImagickNative $current */
         let current = this->image;
@@ -495,7 +495,7 @@ class Imagick extends AbstractAdapter
                 0
             );
 
-            if (true !== current->nextImage()) {
+            if (true !== reflection->nextImage()) {
                 break;
             }
         }
@@ -509,7 +509,7 @@ class Imagick extends AbstractAdapter
             pseudo
         );
 
-        let opacity /= 100;
+        let fadeOpacity = (float) opacity / 100;
         reflection->setIteratorIndex(0);
 
         while (true) {
@@ -526,23 +526,24 @@ class Imagick extends AbstractAdapter
 
             reflection->evaluateImage(
                 constant("Imagick::EVALUATE_MULTIPLY"),
-                opacity,
+                fadeOpacity,
                 constant("Imagick::CHANNEL_ALPHA")
             );
 
-            if (true !== current->nextImage()) {
+            if (true !== reflection->nextImage()) {
                 break;
             }
         }
 
         fade->destroy();
 
-        let image  = new ImagickNative();
-        let pixel  = new ImagickPixel();
-        let height = current->getImageHeight() + height;
+        let image = new ImagickNative();
+        let pixel = new ImagickPixel();
 
         current->setIteratorIndex(0);
 
+        let height = current->getImageHeight() + height;
+
         while (true) {
             image->newImage(this->width, height, pixel);
 
@@ -620,8 +621,14 @@ class Imagick extends AbstractAdapter
         let extension = strtolower(extension);
         switch (extension) {
             case "gif":
+                this->setFramesFormat(image, extension);
+
                 image->optimizeImageLayers();
-                break;
+
+                /**
+                 * A blob of the current frame alone loses the animation
+                 */
+                return image->getImagesBlob();
             case "jpg":
             case "jpeg":
                 image->setImageCompression(
@@ -718,6 +725,8 @@ class Imagick extends AbstractAdapter
         let extension = strtolower(extension);
         switch (extension) {
             case "gif":
+                this->setFramesFormat(image, extension);
+
                 image->optimizeImageLayers();
 
                 /** @var resource $fp */
@@ -752,18 +761,18 @@ class Imagick extends AbstractAdapter
      */
     protected function processSharpen(int amount) -> void
     {
-        var image;
+        var image, sigma;
 
         let amount = (amount < 5) ? 5 : amount;
-        let amount = (amount * 3.0) / 100;
+        let sigma  = (float) amount * 3.0 / 100;
 
         /** @var ImagickNative $image */
         let image = this->image;
 
         image->setIteratorIndex(0);
 
         while (true) {
-            image->sharpenImage(0, amount);
+            image->sharpenImage(0, sigma);
 
             if (true !== image->nextImage()) {
                 break;
@@ -789,11 +798,11 @@ class Imagick extends AbstractAdapter
         int size,
         string fontFile = null
     ) -> void {
-        var color, draw, gravity, image, x, y;
+        var color, draw, gravity, image, textOpacity, x, y;
 
-        let opacity = opacity / 100;
-        let draw    = new ImagickDraw();
-        let color   = sprintf("rgb(%d, %d, %d)", red, green, blue);
+        let textOpacity = (float) opacity / 100;
+        let draw        = new ImagickDraw();
+        let color       = sprintf("rgb(%d, %d, %d)", red, green, blue);
 
         draw->setFillColor(new ImagickPixel(color));
 
@@ -805,8 +814,8 @@ class Imagick extends AbstractAdapter
             draw->setFontSize(size);
         }
 
-        if (opacity) {
-            draw->setfillopacity(opacity);
+        if (textOpacity) {
+            draw->setfillopacity(textOpacity);
         }
 
         let gravity = null;
@@ -889,15 +898,15 @@ class Imagick extends AbstractAdapter
         int offsetY,
         int opacity
     ) -> void {
-        var current, image, result;
+        var current, image, result, watermarkOpacity;
 
-        let opacity = opacity / 100;
-        let image   = new ImagickNative();
+        let watermarkOpacity = (float) opacity / 100;
+        let image            = new ImagickNative();
 
         image->readImageBlob(watermark->render());
         image->evaluateImage(
             constant("Imagick::EVALUATE_MULTIPLY"),
-            opacity,
+            watermarkOpacity,
             constant("Imagick::CHANNEL_ALPHA")
         );
 
@@ -943,4 +952,27 @@ class Imagick extends AbstractAdapter
             let this->version = constant("Imagick::IMAGICK_EXTNUM");
         }
     }
+
+    /**
+     * Marks every frame with the format.
+     *
+     * setImageFormat() marks the current frame only, and a wand built with
+     * newImage() carries no format at all, which stops a multi frame write.
+     *
+     * @throws ImagickException
+     */
+    private function setFramesFormat(var image, string extension) -> void
+    {
+        image->setIteratorIndex(0);
+
+        while (true) {
+            image->setImageFormat(extension);
+
+            if (true !== image->nextImage()) {
+                break;
+            }
+        }
+
+        image->setFormat(extension);
+    }
 }

tests/unit/Image/Adapter/Imagick/BackgroundTest.php

@@ -55,4 +55,31 @@ public function testImageAdapterImagickBackground(): void
 
         $this->safeDeleteFile('background.jpg');
     }
+
+    /**
+     * @author Phalcon Team <team@phalcon.io>
+     * @since  2026-08-19
+     */
+    public function testImageAdapterImagickBackgroundOpacity(): void
+    {
+        // The source needs an alpha channel; an opaque JPEG has nothing to dissolve
+        $file = Talon::settings()->supportPath('assets/images/example-png.png');
+
+        $half = new Imagick($file);
+        $full = new Imagick($file);
+
+        $half->background('#ff0000', 50);
+        $full->background('#ff0000', 100);
+
+        // A percentage below 100 must not collapse onto the opaque result
+        $this->assertNotSame(
+            $this->imageSignature($full),
+            $this->imageSignature($half)
+        );
+    }
+
+    private function imageSignature(Imagick $image): string
+    {
+        return $image->getImage()->getImageSignature();
+    }
 }

tests/unit/Image/Adapter/Imagick/ConstructTest.php

@@ -66,4 +66,33 @@ public function testImageAdapterImagickConstructNewImageNoDimensionsException():
 
         new Imagick('non-existing.png');
     }
+
+    /**
+     * @author Phalcon Team <team@phalcon.io>
+     * @since  2026-08-19
+     */
+    public function testImageAdapterImagickConstructCoalescesGif(): void
+    {
+        $image = new Imagick(
+            Talon::settings()->supportPath('assets/images/example-gif.gif')
+        );
+
+        $native     = $image->getImage();
+        $geometries = [];
+
+        $native->setIteratorIndex(0);
+
+        do {
+            $geometries[] = $native->getImageWidth() . 'x' . $native->getImageHeight();
+        } while ($native->nextImage());
+
+        // Coalescing lifts every sub frame back to the full canvas
+        $this->assertSame(
+            ['960x640'],
+            array_values(array_unique($geometries))
+        );
+
+        $this->assertSame(960, $image->getWidth());
+        $this->assertSame(640, $image->getHeight());
+    }
 }