Intervention / image

PHP Image Processing
https://image.intervention.io
MIT License
13.85k stars 1.5k forks source link

Imagick driver writes white image after resize #1261

Open storeman opened 8 months ago

storeman commented 8 months ago

Describe the bug I have a large JPEG file which results in a white output file (with correct dimensions) after resizing. If I use Imagick directly from within PHP it works correctly (at least with the thumbnailImage method). Also when using the GD-driver, the results are as expected.

After more fiddling, even leaving the resize call out of it, results in a white image, see code sample below.

Code Example

Expected behavior I expect the st-john2.jpg to have the same file size and content. Instead the original is ~25MB and the new version ~180kB

Images I can provide the image using PB, due to copyright.

EDIT
This image has the same behaviour: http://pictures.4ever.eu/data/download/buildings/bridges/st-johns-bridge,-forest,-hill-245439.jpg?no-logo

\Intervention\Image\ImageManager::imagick()->read('st-john-xl.jpg')->save('st-john2.jpg');

Environment (please complete the following information):

storeman commented 8 months ago

After some more digging, this problem doesn't occur when removing line Intervention\Image\Drivers\Imagick\Decoders\BinaryImageDecoder::32

The coalesceImages() call causes the image to become empty/white.

The question now is, is this a InterventionImage bug, or an Imagick bug. Or is the image corrupt...

A possible fix could be to replace line 32, but I cannot oversee if this would break stuff. I did a quick test resizing an animated GIF and the image mentioned in the start post.

        $imagickCoalesced = $imagick->coalesceImages();
        if ($imagickCoalesced->count() !== $imagick->count()) {
            $imagick = $imagickCoalesced;
        }
olivervogel commented 8 months ago

That's strange. I just checked it with the image from the URL in my environment and could not reproduce the problem described.

Everything ok

ImageManager::imagick()
    ->read('images/st-john-xl.jpg')
    ->resize(300, 200)
    ->save('images/st-john-xl-2.jpg');

PHP: 8.2.14 Host OS: Mac OS 13 Intervention Image Version: 3.2.3 Imagick 3.7.0 / ImageMagick 7.1.1-24 Q16-HDRI aarch64 21856

olivervogel commented 8 months ago

I checked it again on Debian. Here I can see the effect with the white background. Unfortunately, that doesn't get me any further at the moment.

Bug reproducable

PHP: 8.1.26 Host OS: Debian Intervention Image Version: 3.2.3 Imagick 3.7.0 / ImageMagick 6.9.11-60 Q16 x86_64 2021-01-25

Bug reproducable

PHP: 8.2.14 Host OS: Debian Intervention Image Version: 3.2.3 Imagick 3.7.0 / ImageMagick 6.9.11-60 Q16 x86_64 2021-01-25

storeman commented 8 months ago

Thanks Oliver for checking.

I updated my previous message, because I missed your reply:

The coalesceImages() call causes the image to become empty/white.

The question now is, is this a InterventionImage bug, or an Imagick bug. Or is the image corrupt...

A possible fix could be to replace line 32, but I cannot oversee if this would break stuff. I did a quick test resizing an animated GIF and the image mentioned in the start post.

    $imagickCoalesced = $imagick->coalesceImages();
    if ($imagickCoalesced->count() !== $imagick->count()) {
        $imagick = $imagickCoalesced;
    }

Basically this checks if coalesceImages() does anything. If not, continue working with the original.

olivervogel commented 8 months ago

After further tests, I currently see no other possibility than to call coalesceImages() only for animated images all image formats except Jpeg. Because Jpeg has no transparency and can not be animated.

Although I cannot understand the logic of this error at all.

It could very well be a bug in ImageMagick. This could even be fixed with ImageMagick >= 7.1.1-24, as no error occurred with this version in my tests. However, this is just speculation and would have to be checked.

olivervogel commented 8 months ago

This issue should be kept in mind. The described bug was only fixed with a workaround and a more permanent solution should be sought.