Preserve transparency in the Imagick grayscale effect#880
Merged
mlocati merged 2 commits intoJun 4, 2026
Conversation
effects()->grayscale() on the Imagick driver switches the image to IMGTYPE_GRAYSCALE, an alpha-less type: ImageMagick deactivates the alpha channel as part of the type switch, so every transparent pixel is encoded as opaque gray. The test saves and reloads the image because the defect only shows at encode time: in-memory pixel reads still expose the stored alpha values.
effects()->grayscale() switched the image to IMGTYPE_GRAYSCALE, an alpha-less image type: ImageMagick deactivates the alpha channel as part of the type switch, so transparent pixels were encoded as opaque gray in the saved file, whatever the output format. Switch to the alpha-preserving grayscale type instead, picked through the same constant fallback chain the driver already uses in Image::setColorspace() (IMGTYPE_GRAYSCALEALPHA since ImageMagick 7 / Imagick 3.4.3, previously IMGTYPE_GRAYSCALEMATTE, hard-coded 3 when neither is defined). The gray rendition is unchanged: both types go through the same colorspace transform, only the alpha trait differs. The GD driver is unaffected (IMG_FILTER_GRAYSCALE keeps alpha); the Gmagick driver does not support transparency at all, so the shared test is skipped there.
Collaborator
|
Thanks! |
Contributor
Author
|
Thanks @mlocati :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
$image->effects()->grayscale()on the Imagick driver turns transparent pixels into opaque gray.Imagick::IMGTYPE_GRAYSCALEis one of ImageMagick's alpha-less image types, and switching to it deactivates the image's alpha channel (getImageAlphaChannel()flips fromtruetofalse). The pixels keep their stored alpha values in memory — only the encoders ignore them — so the breakage only surfaces in the saved file, whatever the format. That's also why the test asserts through an encode/decode roundtrip instead of reading pixels in place.The driver already solves this exact problem in
Image::setColorspace(), whereusePalette()with a grayscale palette deliberately lands onIMGTYPE_GRAYSCALEALPHAto keep alpha;effects()->grayscale()just never got the same treatment. It now picks its type through the same fallback chain (IMGTYPE_GRAYSCALEALPHAon ImageMagick 7 / Imagick 3.4.3+,IMGTYPE_GRAYSCALEMATTEbefore, hard-coded3when neither constant exists). The gray rendition is unchanged — both types go through the same colorspace transform, only the alpha trait differs — so the existingtestGrayscaleexpectations still hold.GD is unaffected (
IMG_FILTER_GRAYSCALEpreserves alpha), and Gmagick doesn't support transparency at all, so the shared test skips there.