Skip to content

Fix imagick stale layers#879

Merged
mlocati merged 2 commits into
php-imagine:developfrom
nlemoine:fix-imagick-stale-layers
Jun 4, 2026
Merged

Fix imagick stale layers#879
mlocati merged 2 commits into
php-imagine:developfrom
nlemoine:fix-imagick-stale-layers

Conversation

@nlemoine

@nlemoine nlemoine commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Resize an animated GIF on the Imagick driver, then ask for its layers: you get the pre-resize frames back.

$image = $imagine->open('animated.gif'); // 2 frames, 8x8
$image->resize(new Box(4, 4));

$image->getSize()->getWidth();                                // 4
$image->layers()->coalesce()->get(0)->getSize()->getWidth();  // 8, expected 4

Layers grabs the \Imagick instance in its constructor, and Image::layers() caches the object forever. That was harmless until the multi-frame paths started swapping the instance: coalesceImages()/deconstructImages() return a new \Imagick, which resize(), crop(), flatten() and the optimize save path assign to $this->imagick, so the cached Layers keeps reading frames nothing else uses anymore. One concrete casualty: animated.delay/animated.loops are applied through layers() in prepareOutput(), land on the orphaned frames, and the saved file keeps its original timing.

Reassignments now go through a small setter that drops the cached Layers when the instance changes (__clone() already rebuilds it the same way), so the next layers() call reads the live resource.

nlemoine added 2 commits June 4, 2026 10:51
Image::layers() lazily creates a Layers object that captures the current
\Imagick resource. Multi-frame resize()/crop() (and flatten(), and the
optimize output path) replace the resource via coalesceImages() /
deconstructImages() without invalidating that cache, so any subsequent
layers() access (get, coalesce, iteration, animate, merge) operated on
the orphaned pre-operation frames. Among other effects, this caused the
animated.delay and animated.loops save options to be silently lost after
resizing an animated image.

Replace direct resource reassignments with a setter that invalidates the
cached layers when the \Imagick instance changes.
@mlocati

mlocati commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

Nice catch!

@mlocati mlocati merged commit 62aad20 into php-imagine:develop Jun 4, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants