Detach Imagick frame when converting to image

src/Drivers/Imagick/Frame.php

@@ -21,6 +21,12 @@
 
 class Frame extends AbstractFrame implements FrameInterface
 {
+    /**
+     * Position this frame was taken from, or null if it is not bound to a
+     * position in a sequence.
+     */
+    protected ?int $position = null;
+
     /**
      * Create new frame.
      *
@@ -29,6 +35,11 @@ class Frame extends AbstractFrame implements FrameInterface
     public function __construct(protected Imagick $native)
     {
         try {
+            // Imagick::current() returns the wand itself, so a frame taken
+            // from an animation shares the whole sequence with its core. Keep
+            // the position it was taken from to be able to seek back to it.
+            $this->position = $this->native->getIteratorIndex();
+
             $background = new ImagickPixel('rgba(255, 255, 255, 0)');
             $this->native->setImageBackgroundColor($background);
             $this->native->setBackgroundColor($background);
@@ -41,10 +52,25 @@ public function __construct(protected Imagick $native)
      * {@inheritdoc}
      *
      * @see DriverInterface::toImage()
+     *
+     * @throws DriverException
      */
     public function toImage(DriverInterface $driver): ImageInterface
     {
-        return new Image($driver, new Core($this->native()));
+        try {
+            // The native of a frame taken from an animation still holds the
+            // whole sequence, and any other frame access in the meantime has
+            // moved its pointer. Seek back before copying the frame out,
+            // otherwise the resulting image would report every frame and hold
+            // whichever one the shared wand was left on.
+            if ($this->position !== null) {
+                $this->native->setIteratorIndex($this->position);
+            }
+
+            return new Image($driver, new Core($this->native->getImage()));
+        } catch (ImagickException $e) {
+            throw new DriverException('Failed to transform frame into image', previous: $e);
+        }
     }
 
     /**
@@ -64,6 +90,10 @@ public function setNative(mixed $native): FrameInterface
 
         $this->native = $native;
 
+        // the replacement carries its own sequence, so the position this
+        // frame was taken from does not apply to it anymore
+        $this->position = null;
+
         return $this;
     }
 

tests/Unit/Drivers/Imagick/FrameTest.php

@@ -8,9 +8,12 @@
 use PHPUnit\Framework\Attributes\RequiresPhpExtension;
 use Imagick;
 use ImagickPixel;
+use Intervention\Image\Drivers\Imagick\Core;
 use Intervention\Image\Drivers\Imagick\Driver;
 use Intervention\Image\Drivers\Imagick\Frame;
+use Intervention\Image\Direction;
 use Intervention\Image\Image;
+use Intervention\Image\Modifiers\FlipModifier;
 use Intervention\Image\Size;
 use Intervention\Image\Tests\BaseTestCase;
 
@@ -29,6 +32,24 @@ protected function getTestFrame(): Frame
         return new Frame($imagick);
     }
 
+    /**
+     * Create a two frame animation, the first frame red and the second blue.
+     */
+    protected function getTestAnimation(): Imagick
+    {
+        $imagick = new Imagick();
+        $imagick->setFormat('gif');
+
+        foreach (['red', 'blue'] as $color) {
+            $frame = new Imagick();
+            $frame->newImage(3, 2, new ImagickPixel($color), 'gif');
+            $frame->setImageDelay(10);
+            $imagick->addImage($frame);
+        }
+
+        return $imagick;
+    }
+
     public function testConstructor(): void
     {
         $frame = $this->getTestFrame();
@@ -100,6 +121,43 @@ public function testToImage(): void
         $this->assertInstanceOf(Image::class, $frame->toImage(new Driver()));
     }
 
+    public function testToImageOfAnimationFrame(): void
+    {
+        $core = new Core($this->getTestAnimation());
+
+        foreach (['ff0000', '0000ff'] as $position => $hex) {
+            $image = $core->frame($position)->toImage(new Driver());
+            $this->assertEquals(1, $image->count());
+            $this->assertEquals($hex, $image->colorAt(0, 0)->toHex());
+        }
+    }
+
+    public function testToImageOfAnimationFrameIsDetached(): void
+    {
+        $core = new Core($this->getTestAnimation());
+        $image = $core->frame(0)->toImage(new Driver());
+
+        // the resulting image must not share the sequence of the core, which
+        // still holds all frames and gets seeked around by every frame access
+        $this->assertNotSame($core->native(), $image->core()->native());
+
+        // mirroring the result must not reach back into the animation
+        $image->modify(new FlipModifier(Direction::HORIZONTAL));
+        $this->assertEquals('ff0000', $core->frame(0)->toImage(new Driver())->colorAt(0, 0)->toHex());
+    }
+
+    public function testToImageOfAnimationFrameAfterFurtherFrameAccess(): void
+    {
+        $core = new Core($this->getTestAnimation());
+
+        // taking another frame moves the shared iterator away, the frame that
+        // was taken first still has to convert to its own position
+        $frame = $core->frame(1);
+        $core->frame(0);
+
+        $this->assertEquals('0000ff', $frame->toImage(new Driver())->colorAt(0, 0)->toHex());
+    }
+
     public function testSetGetNative(): void
     {
         $frame = $this->getTestFrame();