Detach Imagick frame when converting to image

[nlemoine]

On the Imagick driver, taking a frame off an animation and calling toImage() gives back an image that still holds every frame. Reading a pixel off it then answers frame 0, whatever index you asked for.

Repro on a 2 frame GIF that is red then blue (magick -delay 50 -loop 0 -size 100x100 xc:red xc:blue animated.gif):

for ($i = 0; $i < $image->count(); $i++) {
    $sub = $image->core()->frame($i)->toImage($image->driver());
    printf("frame %d: %s (count=%d)\n", $i, $sub->colorAt(1, 1)->toHex(), $sub->count());
}

Imagick:

frame 0: ff0000 (count=2)
frame 1: ff0000 (count=2)

GD, same bytes:

frame 0: ff0000 (count=1)
frame 1: 0000ff (count=1)

GD matches how the file was made, so the two drivers disagree.

Imagick::current() returns the wand itself, not a copy, so the Frame that Core::frame() builds wraps the multi frame wand the core holds. toImage() then did new Core($this->native()) on that same wand, which is where both the count=2 and the frame 0 pixels come from.

The fix copies the current frame out with getImage(). Copying alone is not quite enough, it only lands on the right frame while the shared wand still points at it, and taking any other frame in between moves it:

$frame = $core->frame(1);
$core->frame(0);
$frame->toImage($driver);   // was red, should be blue

So the frame also keeps the position it was taken from and seeks back before copying. I checked that the copy keeps the delay and the disposal method, and the page geometry too, so nothing the frame API exposes gets lost. toImage() has one caller in the library, the GD GifEncoder, and that one is untouched.

Three tests added to FrameTest, one on the colors and the frame count, one asserting the result is detached from the core and does not change when the derived image is modified, one on the deferred case above. About the detach test: I first wrote it as "read frame 0's image after touching frame 1" and it passed on develop even with the bug there, because PixelColorAnalyzer defaults to $frame = 0 and seeks the shared wand back before reading. So that phrasing tests nothing, worth knowing if you write your own.

One thing I left out. Two frame handles taken from the same core still alias each other for everything other than toImage():

$f0 = $image->core()->frame(0);
$f1 = $image->core()->frame(1);
var_dump($f0->native() === $f1->native());   // bool(true), $f0->delay() now reads frame 1's

My guess is this one should stay as it is. Modifiers mutate through foreach ($image as $frame) { $frame->native()->... }, so frame() and current() have to hand back a live handle on the wand, a copy there would silently drop every modifier. It would also copy a frame on every colorAt() call. Happy to look at it separately if you disagree.