-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Fix image quality degradation caused by quantizeImage on resized PNG assets (Trac #63448) #8810
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix image quality degradation caused by quantizeImage on resized PNG assets (Trac #63448) #8810
Conversation
…ized images WordPress 6.8 introduced a call to Imagick's quantizeImage() function, which reduces color depth during image resizing. This has been observed to degrade image quality, particularly for PNG files with gradients, soft transitions, or transparency. This change conditionally applies quantization only when max_colors is below 256. Props elvismdev. See Trac #63448.
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN:
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Hi @elvismdev! 👋 Thank you for your contribution to WordPress! 💖 It looks like this is your first pull request to No one monitors this repository for new pull requests. Pull requests must be attached to a Trac ticket to be considered for inclusion in WordPress Core. To attach a pull request to a Trac ticket, please include the ticket's full URL in your pull request description. Pull requests are never merged on GitHub. The WordPress codebase continues to be managed through the SVN repository that this GitHub repository mirrors. Please feel free to open pull requests to work on any contribution you are making. More information about how GitHub pull requests can be used to contribute to WordPress can be found in the Core Handbook. Please include automated tests. Including tests in your pull request is one way to help your patch be considered faster. To learn about WordPress' test suites, visit the Automated Testing page in the handbook. If you have not had a chance, please review the Contribute with Code page in the WordPress Core Handbook. The Developer Hub also documents the various coding standards that are followed:
Thank you, |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
…on on resized PNGs This update replaces the previous depth-based quantization check with a more accurate method: - Uses getImageColors() to count unique colors. - Uses getImageProperty('png:IHDR.color_type') to confirm the image is indexed (palette-based). Quantization is now only applied to true indexed PNGs with 256 colors or fewer, preventing degradation in full-color or gradient-rich images. Also retains grayscale and alpha channel logic for PNG optimization. See: https://core.trac.wordpress.org/ticket/63448
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You almost got this, definitely this "3" in color_type
is a great discovery
But there is a sense in all the original convoluted logic with color depth Imagick methods and the reason of why I did not remove it in my patch #8813
The original bug reported that the Indexed images were increasing in size after the thumbnail_image
processing.
This is why, the original code, actually get both original colors and minimized colors (256 at most) to determine which is the max colors to get a resulting quantized image
$current_colors = $this->image->getImageColors();
$max_colors = min( $max_colors, $current_colors );
This $max_colors
is what you have to check against the 256 in the condition.
But to get first $max_colors you should get the bit depth and then do the exponential of 2 pow( 2, $max_colors)
as the original code did. This way, If the depth was, say 6, then the max colors for the indexed image would be 64 (and not always, say 256, optimizing always therefore getting the minimum size possible for the final resulting image)
is_callable( array( $this->image, 'getImageProperty' ) ) | ||
) { | ||
$current_colors = $this->image->getImageColors(); | ||
$color_type = $this->image->getImageProperty( 'png:IHDR.color_type' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The right property (at least in my Imagick version, is png:IHDR.color-type-orig
$color_type = $this->image->getImageProperty( 'png:IHDR.color-type-orig' );
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SirLouen I’ve updated the PR to use png:IHDR.color-type-orig
instead, which provides more reliable results across Imagick versions. Appreciate the heads-up!
…lors This refines the logic introduced in [59589] to prevent image quality degradation on resized PNGs. Quantization is now only applied when: - The image is indexed (color-type-orig == 3) - The actual color count (getImageColors) is ≤ the max allowed by bit depth This preserves the original optimization goals (palette size reduction) while preventing unintended degradation on full-color or gradient-rich images. Also maintains grayscale (png8) support and alpha channel chunk handling. Props @SirLouen @siliconforks @wildworks Fixes #63448
Thank you @SirLouen for the feedback! I have incorporated Why this approach
Benefits of this logic
In short, both of our patches solve the degradation issue - but this one aims to be a middle-ground that:
Happy to align with all on the preferred path forward, just wanted to share context on why this direction was taken. Thanks again everyone! |
The problem is that much of the logic from [59589] seems to be unnecessary and/or outright wrong and buggy. I'm not sure why you would want to keep it - even if you can somehow get it to work, it seems like that's not really fixing the problem, it's just covering up the issues. I think the approach in the latest version of PR #8813 (which simplifies the code considerably) is a better way to go.
As far as I can tell, this doesn't actually work. I tested it with Are there any particular issues you see with PR #8813? It seems to me like that is the best approach to take:
|
Just FYI, know that these PRs are not being commited into WordPress github repostory, they are just here for informative purposes. They will be downloaded by the WordPress commiters and then commited into the SVN repo. So all this is just hinting content and all PR proposed will be closed at some point and lost in oblivion. I tell you this because I noticed you are taking too much time unnecessarily.. All this will be done by a commiter at some point (and props will include a large list of guys (including those who only dropped to say "Hi" in the report). This is how WP dev works ATM. |
Thanks @siliconforks and @SirLouen! I really appreciate both your insights in this. @siliconforks you're right: the logic around My previous goal in this PR was to fix the regression while cautiously maintaining what seemed to be the spirit of [#59589], in case the quantization logic had tangible value beyond indexed type detection. I’ve also tested #8813, and it does successfully fix the issue I reported in Trac https://core.trac.wordpress.org/ticket/63448, since it avoids using With that said, I’m going to close this PR in favor of #8813 , which cleanly addresses that regression. Thanks again! |
This PR addresses a regression introduced in WordPress 6.8 via [59589], where use of
Imagick::quantizeImage()
during image resizing causes visible quality loss, especially for:Observed Effects
wp media regenerate
Updated Fix
Instead of relying on
getImageDepth()
(which inaccurately represents image color depth), this updated approach:getImageColors()
to check if the image has 256 colors or fewergetImageProperty( 'png:IHDR.color_type' )
to verify if it's an indexed PNG (color_type = 3)Only when both conditions are met does
quantizeImage()
run. This prevents quantization from being applied to high-color or truecolor images, preserving fidelity.Grayscale PNGs still receive the
png8
treatment, and alpha chunk logic is retained.Trac ticket: https://core.trac.wordpress.org/ticket/63448
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.