Make WordPress Core

Opened 5 weeks ago

Last modified 8 days ago

#63448 reopened defect (bug)

Image quality significantly degrades for resized PNGs with transparency in WordPress 6.8.1

Reported by: elvismdev's profile elvismdev Owned by: adamsilverstein's profile adamsilverstein
Milestone: 6.8.2 Priority: normal
Severity: critical Version: 6.8
Component: Media Keywords: has-patch has-test-info has-unit-tests needs-dev-note dev-feedback
Focuses: Cc:

Description

In WordPress 6.8, there's a reproducible regression where transparent PNG images experience noticeable quality degradation when resized by WordPress into additional image sizes (e.g., 768x768). This behavior does not occur in WordPress 6.7 under the same conditions.

Environment

  • Tested on WP Engine and locally with LocalWP
  • Active editor: WP_Image_Editor_Imagick
  • Imagick version: 3.7.0
  • ImageMagick version: 6.9.11-60 Q16 x86_64
  • GD version: 2.3.0
  • Theme: TwentyTwentyFive
  • No plugins installed

Reproduction steps

  1. Start a fresh install of WordPress 6.7.2 (vanilla install using default TwentyTwentyFive theme, no plugins active).
  2. Upload a high-detail PNG with transparency (example attached: color bird illustration, 2500×2500 px).
  3. Observe resized image versions (e.g., 768x768) — output is clear and color-accurate.
  4. Upgrade same install to WordPress 6.8.1.
  5. Upload the same original PNG again or regenerate image sizes.
  6. Observe the resized versions — visible degradation, color banding.

Visual comparison

I'm adding image samples showing a 768x768 output from WP 6.7.2 (clear output and color-accurate) and WP 6.8.1 (noticeable quality loss).

WP 6.7.2 (expected result - no quality lost):
no quality lost image example

WP 6.8.1 (visible degradation):
lost quality example

Original PNG uploaded to both installs:
Original PNG uploaded to both installs

Notes

This may relate to changes mentioned in the WordPress 6.8 Miscellaneous Developer Changes post, possibly affecting image quality logic for certain file types.

Expected

Image quality for resized PNGs should remain consistent across WordPress versions, assuming identical libraries and environment.

Actual

In WP 6.8.1, the resized image shows degradation not present in 6.7.2.

Attachments (10)

images-examples-and-original.zip (3.0 MB) - added by elvismdev 5 weeks ago.
images examples and original
vivid-green-bird-sample-768x768-no-quality-lost-wp67-local.png (298.1 KB) - added by elvismdev 5 weeks ago.
no quality lost image example
vivid-green-bird-sample-768x768-lost-quality-wp68-local.png (270.3 KB) - added by elvismdev 5 weeks ago.
lost quality example
vivid-green-bird-sample-original.png (2.5 MB) - added by elvismdev 5 weeks ago.
Original PNG uploaded to both installs
vivid-green-bird-sample-original-1024x1024-lost-quality.png (306.6 KB) - added by wildworks 5 weeks ago.
A portion of the degraded image, with the color loss especially noticeable in the bird's body.
image-quality-degradation-quantizeimage-63448.patch (968 bytes) - added by elvismdev 5 weeks ago.
oval-or8.png (25.1 KB) - added by nosilver4u 4 weeks ago.
indexed PNG image with transparency and gradient
oval-or8-grayscale-indexed.png (28.7 KB) - added by siliconforks 4 weeks ago.
Like oval-or8.png but grayscale
44-png8.png (75.1 KB) - added by siliconforks 10 days ago.
The image saved using png8 format
44-quantizeImage.png (181.7 KB) - added by siliconforks 10 days ago.
The same image as the previous one, saved after using quantizeImage()

Change History (100)

@elvismdev
5 weeks ago

images examples and original

@elvismdev
5 weeks ago

Original PNG uploaded to both installs

#1 follow-up: @wildworks
5 weeks ago

  • Milestone changed from Awaiting Review to 6.8.2

I tested the following local environments:

  • Studio by WordPRess.com (WP_Image_Editor_GD)
  • Playground (WP_Image_Editor_GD)
  • LocalWP (WP_Image_Editor_Imagick)
  • @wordpress/env (WP_Image_Editor_Imagick)

The only one I could reproduce the problem with was wp-env. Also, this problem may be related to #63338.

#2 in reply to: ↑ 1 @SirLouen
5 weeks ago

Replying to wildworks:

I tested the following local environments:

  • Studio by WordPRess.com (WP_Image_Editor_GD)
  • Playground (WP_Image_Editor_GD)
  • LocalWP (WP_Image_Editor_Imagick)
  • @wordpress/env (WP_Image_Editor_Imagick)

The only one I could reproduce the problem with was wp-env. Also, this problem may be related to #63338.

Maybe the interesting thing here is to see which GD/Imagick version are using each of these environments?

Last edited 5 weeks ago by SirLouen (previous) (diff)

#3 follow-up: @wildworks
5 weeks ago

I also added the wordpress-develop environment and checked the version.

Environment Image library version number Is it reproducible?
Studio by WordPress.com WP_Image_Editor_GD bundled (2.1.0 compatible) No
Playground WP_Image_Editor_GD bundled (2.1.0 compatible) No
LocalWP WP_Image_Editor_Imagick 1808 No
@wordpress/env WP_Image_Editor_Imagick 1691 Yes
wordpress-develop WP_Image_Editor_Imagick 1691 Yes

Perhaps the Imagick version is related, but I'm not sure yet.

Fortunately, however, I was able to reproduce the problem in wordpress-develop, so it should be easy to identify the commit that caused the problem.

#4 in reply to: ↑ 3 @SirLouen
5 weeks ago

Replying to wildworks:

Fortunately, however, I was able to reproduce the problem in wordpress-develop, so it should be easy to identify the commit that caused the problem.

A good test could be simply updating imagemagick library in wordpress-develop to match LocalWP and retest. If you cannot reproduce it, then it's a problem of imagemagick for certain versions, not sure if you could do much in this regard. I bet it has something to do with 6.9 version of Imagick.

The difficulty is that upgrading this library in wordpress-develop/wp-env is not trivial at all. I think its easier to set up a random php image with a Dockerfile modulating the imagick verison with pecl for this test.

The only cue we have is that the OP also reports the same version

ImageMagick version: 6.9.11-60 Q16 x86_64 == 1691

I'm going to ask the reporter of #63338

Last edited 5 weeks ago by SirLouen (previous) (diff)

#5 @elvismdev
5 weeks ago

Thanks all for the continued investigation and context!

I can confirm that in both environments I've tested, I'm still able to consistently reproduce the issue with PNG image quality degradation in WP 6.8. Below is the full output from Site Health > Media Handling for both:

WP Engine (Production Hosting)

  • Active editor: WP_Image_Editor_Imagick
  • ImageMagick version string: 6.9.11-60 Q16 x86_64
  • Imagick version: 3.7.0
  • ImageMagick version number: 1691
  • GD version: 2.3.0
  • Ghostscript version: Not available

LocalWP (Local Testing)

  • Active editor: WP_Image_Editor_Imagick
  • ImageMagick version string: 7.1.0-10 Q16 x86_64
  • Imagick version: 3.7.0
  • ImageMagick version number: 1808
  • GD version: bundled (2.1.0 compatible)
  • Ghostscript version: 9.56.1

Even with the newer ImageMagick version (1808) on LocalWP, I’m still able to reproduce the issue there as well.

#6 @wildworks
5 weeks ago

There are two consecutive changesets related to media:

I was able to identify the latter one as the first to cause this problem.

#7 @wildworks
5 weeks ago

I found that this line causes the image degradation. If I comment out this line, the image degradation is not observed. This might be a problem with older versions of Imagick.

The following steps should allow you to reproduce the problem.

@adamsilverstein Do you no anything about this problem?

@wildworks
5 weeks ago

A portion of the degraded image, with the color loss especially noticeable in the bird's body.

This ticket was mentioned in PR #8810 on WordPress/wordpress-develop by @elvismdev.


5 weeks ago
#8

  • Keywords has-patch added

This PR addresses a regression introduced in WordPress 6.8, where use of Imagick::quantizeImage() during image resizing causes visible quality loss, especially for:

  • PNGs with transparency
  • Images with soft gradients or color transitions

Observed Effects

  • Banding and posterization in gradients
  • Loss of clarity in high-fidelity illustrations
  • Artifacts in resized images generated via upload or wp media regenerate

This fix only applies quantizeImage() when $max_colors < 256, which is common when intentional palette reduction is desired.

Trac ticket: https://core.trac.wordpress.org/ticket/63448

#9 @elvismdev
5 weeks ago

Thanks @wildworks for digging deep into this and pinpointing the regression to the quantizeImage() line added in [59589].

From my testing and what others have also reported, this function seems to reduce image quality drastically, particularly for:

  • PNGs with transparency or soft gradients
  • Images with smooth color transitions, like illustrations or UI graphics

From my research this seems to be because quantizeImage() limits the color palette used in the resized image. While useful for reducing file size, it's too aggressive by default in these cases, leading to color banding and a loss of visual fidelity.

I'm adding a patch via this GitHub PR #8810 that skips quantization unless $max_colors < 256, which is common when intentional palette reduction is desired.

Would love feedback from others affected by this, and happy to iterate further if needed.

#10 @wildworks
5 weeks ago

@elvismdev Thanks for submitting a patch. I'll ping the contributors involved in [59589].

#11 @siliconforks
5 weeks ago

The patch in PR #8810 doesn't really seem to fix the fundamental issue here...

I think fundamental problem is that [59589] seems to be assuming that if getImageDepth() returns <= 8, the image must be using <= 256 colors (so it is either using indexed color or it can be converted to indexed color) - but I don't think this is correct.

ImageMagick appears to use the term "depth" to refer to the depth of an image channel rather than the depth of the entire image. See for example https://github.com/ImageMagick/ImageMagick/blob/main/MagickCore/attribute.c where it states that "GetImageDepth() returns the depth of a particular image channel." (Note that the C function GetImageDepth is not the same thing as the PHP function getImageDepth but I suspect they are both using the same definition of "depth".)

Similarly the ImageMagick identify command considers the file vivid-green-bird-sample-original.png to have "Depth: 8-bit":

$ identify -verbose -unique vivid-green-bird-sample-original.png
Image:
  Filename: vivid-green-bird-sample-original.png
  Format: PNG (Portable Network Graphics)
  Mime type: image/png
...
  Depth: 8-bit
  Channel depth:
    red: 8-bit
    green: 8-bit
    blue: 8-bit
    alpha: 8-bit
...
  Colors: 830043

So the image has more than 800,000 colors but ImageMagick still considers the "depth" to be 8 bits (because the depth of the red channel, the green channel, the blue channel, and the alpha channel are each 8 bits).

You can get the same result with a simple PHP program:

<?php
$image = new Imagick('vivid-green-bird-sample-original.png');
$depth = $image->getImageDepth();
echo 'depth = ' . $depth . "\n";
$colors = $image->getImageColors();
echo 'colors = ' . $colors . "\n";

This will display:

depth = 8
colors = 830043

So I think [59589] really needs to be reworked to not use getImageDepth.

This ticket was mentioned in PR #8813 on WordPress/wordpress-develop by @SirLouen.


5 weeks ago
#12

@siliconforks great analysis, you have saved me a good time of research

But if we look at the original Bug, the problem is that the Indexed types images were being converted into Truecolor type. So the @adamsilverstein idea was great but was not taking in consideration, that there are images within the 8-bit depth color range that are actually Truecolor.

AFAIK, the issue is that ImageMagick doesn't provide a method to identify if an image is Indexed or if it's Truecolor, so the sole hint we can find within the Imagick object is the fact that the image has 256 colors or less.

So I think that the @elvismdev idea was also on the right direction, but knowing this, I believe that it could be slightly optimized and overall, the whole algorithm, to apply Quantization for images with lower than 256 colors and still get a result on compression, and to ignore any image that is not Indexed but still have a color depth of 8+ bits.

Trac ticket: https://core.trac.wordpress.org/ticket/63448

#13 @SirLouen
5 weeks ago

  • Keywords needs-testing needs-unit-tests added
  • Severity changed from major to critical

Ideally some units tests for this part

**
 * If the colorspace is 'gray', use the png8 format to ensure it stays indexed.
 */
if ( Imagick::COLORSPACE_GRAY === $this->image->getImageColorspace() ) {
	$this->image->setOption( 'png:format', 'png8' );
}

Will also be great.

#14 @joemcgill
5 weeks ago

Great looking everyone. If more time is needed to iterate on the right solution, I think it would be fine to revert [59589] and the follow-up commit for an upcoming minor release and reopen the original ticket.

@siliconforks commented on PR #8813:


5 weeks ago
#15

AFAIK, the issue is that ImageMagick doesn't provide a method to identify if an image is Indexed or if it's Truecolor, so the sole hint we can find within the Imagick object is the fact that the image has 256 colors or less.

There probably is a method to check whether an image is indexed or not. It looks like identifyImage might work:

https://www.php.net/manual/en/imagick.identifyimage.php

But I'm not sure if that's really what we want here. Possibly just checking the number of colors is sufficient?

So I think that the @elvismdev idea was also on the right direction, but knowing this, I believe that it could be slightly optimized and overall, the whole algorithm, to apply Quantization for images with lower than 256 colors and still get a result on compression, and to ignore any image that is not Indexed but still have a color depth of 8+ bits.

That sounds basically right - but isn't this PR sort of doing the opposite of that? Is this check for 256 <= $max_colors supposed to be $max_colors <= 256?

https://github.com/WordPress/wordpress-develop/blob/e8862701ccfc7ed045855ecd1e8c8bccdbac843b/src/wp-includes/class-wp-image-editor-imagick.php#L500

Even if that is fixed I'm still not sure the logic entirely makes sense here. Possibly what we want to look at is the number of colors in the image before resizing, rather than after?

It may be simpler to just revert https://core.trac.wordpress.org/changeset/59589 for now and then try to get a revised version of it in a future release because I think it's going to need more extensive changes.

#16 follow-up: @nosilver4u
5 weeks ago

I think you're correct that there is no method in IM to detect the PNG color type, which is why my original pull request added a method to do so: https://github.com/WordPress/wordpress-develop/pull/6686

If you use that, then you can accurately determine the color type, and apply the quantization only if the color type is 3. The bit depth merely serves to help determine how many colors are allowed when the color type is 3. Do be sure to keep the PNG8 reduction for grayscale also.

The identifyImage method may work sometimes, but is inaccurate similar to getImageType (Adam & I discussed this in the above PR). The only reliable method I was able to find was to parse the file directly, as I had it in my pull request.

@SirLouen commented on PR #8813:


5 weeks ago
#17

@siliconforks this is the problem: identifyImage is identifying "Indexed" images as TrueColor and colorSpace as sRGB (check for example cloudflare-status.png from the unit-tests

About the code, you are right, these fricking Yoda Conditions mess me up constantly, it's the other way around. It's funny because the 5 testing images all happen to have exactly, 256 colors, hence the equals part was passing. In fact, I'm not sure, but I feel that all the 5 images are basically identical in properties, so basically just one of them would do the same stunt and the rest seem to be filler.

It may be simpler to just revert https://core.trac.wordpress.org/changeset/59589 for now and then try to get a revised version of it in a future release because I think it's going to need more extensive changes.

The thing is that Indexed images grow bigger (much bigger, like X2 in numerous instances). So imagine how playful can be to have 100K images occupying 2X when the issue was already solved. Not taking action now in any direction is harming websites.

Even if that is fixed I'm still not sure the logic entirely makes sense here. Possibly what we want to look at is the number of colors in the image before resizing, rather than after?

Resizing doesn't seem to be the problem. The problem is the Quantization. The number of colors is just the only trustworthy way I've researched to separate between Indexed and non Indexed images. And that is the exact moment where it can be checked.

So lets jump into the bull and take it by the horns: We could always get to a perfectly clean solution with maximum optimization parameters, logic and well-being, but while anyone practical decides to jump in the bull and give a better solution, my result solves the increased size on Indexed images and solves the trouble with degradation with colors in one shot.

@siliconforks commented on PR #8813:


5 weeks ago
#18

About the code, you are right, these fricking Yoda Conditions mess me up constantly, it's the other way around. It's funny because the 5 testing images all happen to have exactly, 256 colors, hence the equals part was passing. In fact, I'm not sure, but I feel that all the 5 images are basically identical in properties, so basically just one of them would do the same stunt and the rest seem to be filler.

I don't think there's any need to use a Yoda condition here. According to the WordPress PHP Coding Standards: "Yoda conditions for <, >, <= or >= are significantly more difficult to read and are best avoided." (Personally, I would prefer it to be written as $max_colors <= 256.)

However, I don't think that's sufficient to fix the PR - it's still calling getImageDepth() and that's being used in the calculation of $max_colors. The call to getImageDepth() is almost always going to return 8, even for an image like https://core.trac.wordpress.org/raw-attachment/ticket/63448/vivid-green-bird-sample-original.png (the only exception seems to be for PNG images with 16 bits per channel, which are rare). So $max_colors is almost always going to be <= 256.

I just don't think getImageDepth() is actually doing anything useful here. It appears it was originally used because of an incorrect assumption of what the return value was, and I think it probably needs to be removed entirely.

Resizing doesn't seem to be the problem. The problem is the Quantization. The number of colors is just the only trustworthy way I've researched to separate between Indexed and non Indexed images. And that is the exact moment where it can be checked.

Isn't the issue in https://core.trac.wordpress.org/ticket/36477 that the number of colors often increases when an image is resized? So you might start with an original image which has 256 colors, but then you resize it and the resized version of it has more than 256 colors, so it no longer can use a palette. Then the size of the file will increase (possibly enormously).

So lets jump into the bull and take it by the horns: We could always get to a perfectly clean solution with maximum optimization parameters, logic and well-being, but while anyone practical decides to jump in the bull and give a better solution, my result solves the increased size on Indexed images and solves the trouble with degradation with colors in one shot.

I'm happy to continue trying to fix this, but it's just that it took 9 years to try to fix https://core.trac.wordpress.org/ticket/36477 and it's still not working correctly, so I don't know how long this is going to take...

#19 in reply to: ↑ 16 ; follow-up: @siliconforks
5 weeks ago

Replying to nosilver4u:

I think you're correct that there is no method in IM to detect the PNG color type, which is why my original pull request added a method to do so: https://github.com/WordPress/wordpress-develop/pull/6686

If you use that, then you can accurately determine the color type, and apply the quantization only if the color type is 3. The bit depth merely serves to help determine how many colors are allowed when the color type is 3. Do be sure to keep the PNG8 reduction for grayscale also.

The identifyImage method may work sometimes, but is inaccurate similar to getImageType (Adam & I discussed this in the above PR). The only reliable method I was able to find was to parse the file directly, as I had it in my pull request.

Did anyone try using getImageProperty()?

I tried it with 'png:IHDR.color_type' and 'png:IHDR.color-type-orig' and they seemed to work...

#20 in reply to: ↑ 19 @nosilver4u
5 weeks ago

Replying to siliconforks:

Did anyone try using getImageProperty()?

I tried it with 'png:IHDR.color_type' and 'png:IHDR.color-type-orig' and they seemed to work...

I sure didn't, that's a good find! I ran it through my entire PNG test folder, and all matched on png:IHDR.color_type. Seems png:IHDR.color-type-orig is not set initially. It must get set once the image has been scaled. Interestingly, it seems the IHDR.color_type doesn't get overwritten after the image resized. At least not immediately, but it might be worth checking both values for a color type of 3 just to be safe.

@SirLouen commented on PR #8813:


5 weeks ago
#21

I don't think there's any need to use a Yoda condition here. According to the WordPress PHP Coding Standards: "Yoda conditions for <, >, <= or >= are significantly more difficult to read and are best avoided." (Personally, I would prefer it to be written as $max_colors <= 256.)

Everyday one learns something new. Happy to read this, I could not understand using Yoda conditions in these comparisons for ages because it broke my logic but in fact, in the Wikipedia article, they suggested that they could be used and were great (which in fact, for a combined and its not bad after all, like in this case, if the original comparisons were:

0 < $indexed_pixel_depth && $indexed_pixel_depth <= 8

Instead of full Yoda codntions, would make a lot of sense to me.

This said, I've also tested against the clean images directly in PHP test file and I've got this:
https://github.com/user-attachments/assets/1ed59062-116d-405a-8140-c5cb8de3b309

Type = Palette, which probably could be a great indicator that as you said, identifyImage could be an alternative used in the right place (I think, not 100% sure), but the problem is that once the conversion is done, it's converted into TrueColor, before any other compression filters are applied)

#22 @elvismdev
5 weeks ago

Thanks everyone for the great input and collaborative investigation. I’ve just updated my GitHub PR #8810 based on the recent insights shared here.

What changed:

  • I removed reliance on getImageDepth() since it's not a reliable indicator of whether a PNG is palette-based.
  • Instead, I now use:
    • getImageColors() to check if the image has 256 or fewer unique colors.
    • getImageProperty( 'png:IHDR.color_type' ) to verify it’s an indexed PNG (color_type == 3).
  • Quantization is only applied when both conditions are met, which ensures it won’t degrade quality on full-color PNGs or images with gradients/transparency.

This logic seems to solve the issue without regressing the optimization intent of [59589]. Grayscale (png8) and alpha chunk logic is preserved.

Let me know what you think, happy to iterate further if needed.

@SirLouen commented on PR #8813:


5 weeks ago
#23

Just for information purposes it seems there are more Indexed types
GrayscaleAlpha and PaletteAlpha

#24 @SirLouen
5 weeks ago

Just a picture of all the Imagick properties of an Indexed image, confirming that png:IHDR.color-type-orig is the right way to get the Indexed property (not sure if that orig part is always in all Imagick versions)

https://i.imgur.com/Cnsp6fB.png

@SirLouen commented on PR #8813:


5 weeks ago
#25

In my new final version, I took the idea commented by @siliconforks and @elvismdev of using getImageProperty instead of identifyImage Type because it was the most consistent of all to show the PNG Indexed property

But I also found that the option used for Greyscales, was actually the option that did the same as the whole Quantization process (checked the result size, and it's the same). So now, no additional unit-tests are required (because there is actually a Grayscale image among the set)

So with this last version, we have the best of both worlds, full performance, all test passing + non indexed images are not being affected.

Btw, for props don't forget to consider ticket #63338 for helping with testing

#26 @SirLouen
5 weeks ago

  • Keywords needs-unit-tests removed

@siliconforks commented on PR #8813:


5 weeks ago
#27

In my new final version, I took the idea commented by @siliconforks and @elvismdev of using getImageProperty instead of identifyImage Type because it was the most consistent of all to show the PNG Indexed property

OK, I know I suggested using getImageProperty in https://core.trac.wordpress.org/ticket/63448#comment:19 - but keep in mind that I have not extensively tested this and it's possible there might be issues with it (e.g., I'm not sure how well it is supported across different ImageMagick versions).

Other than that, this PR now looks much simpler and cleaner than the original code in https://core.trac.wordpress.org/changeset/59589

@elvismdev commented on PR #8810:


5 weeks ago
#28

Thank you @SirLouen for the feedback!

I have incorporated png:IHDR.color-type-orig in my latest update here, while also keeping parts of the original logic from [59589] in this PR.

Why this approach

  • It preserves the original intent of [59589], which was to optimize PNGs using palette reduction via quantization - particularly for indexed images - while avoiding bloat.
  • This version continues using getImageDepth() to calculate max_colors via pow( 2, bit_depth ), then compares that to getImageColors() to ensure quantization only runs when:
    • The image is truly indexed (color_type === 3)
    • And it has ≤ max_colors (based on bit depth)

Benefits of this logic

  • It gives better control over when quantization happens, reducing risk of visual degradation while still keeping file sizes low when palette reduction is appropriate.
  • It avoids assumptions about grayscale vs. palette PNGs and allows bit depths < 8 to quantize to smaller palettes (e.g., 4-bit = 16 colors), which helps optimize further in edge cases.
  • Maintains backward compatibility and flexibility for different Imagick builds.

In short, both of our patches solve the degradation issue - but this one aims to be a middle-ground that:

  • Fixes the regression
  • Keeps the size-saving benefit for real indexed PNGs
  • Preserves the bit-depth-based logic introduced in [59589]
  • It's not overriding too much or baking in hardcoded assumptions
  • Lets WordPress control compression behavior based on reliable and well-defined properties
    • getImageColors()
    • getImageDepth()
    • getImageProperty( 'png:IHDR.color-type-orig' ))

Happy to align with all on the preferred path forward, just wanted to share context on why this direction was taken.

Thanks again everyone!

@siliconforks commented on PR #8810:


5 weeks ago
#29

I have incorporated png:IHDR.color-type-orig in my latest update here, while also keeping parts of the original logic from [59589] in this PR.

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.

  • It preserves the original intent of [59589], which was to optimize PNGs using palette reduction via quantization - particularly for indexed images - while avoiding bloat.

As far as I can tell, this doesn't actually work. I tested it with `rabbit-time-paletted-or8.png` and it does not meet the condition $current_colors <= $max_colors so it doesn't reduce the number of colors in the resized images. $max_colors is always 256, while $current_colors is between 4350 and 171744 (your results may vary depending on what your site's configured image sizes are).

Are there any particular issues you see with PR #8813? It seems to me like that is the best approach to take:

  • It appears to work well with images like `vivid-green-bird-sample-original.png` as it does not attempt to reduce the number of colors and thereby degrade the image quality.
  • It appears to work well with images like rabbit-time-paletted-or8.png as it prevents the number of colors from blowing up and resulting in enormous file sizes.
  • Also - I haven't been able to verify this yet, but I suspect that it might fix https://core.trac.wordpress.org/ticket/63338 (I think I have a vague idea of what's going on there, and it looks like PR#8813 will probably fix that too)

@SirLouen commented on PR #8810:


5 weeks ago
#30

Thank you @SirLouen for the feedback!

I have incorporated png:IHDR.color-type-orig in my latest update here, while also keeping parts of the original logic from [59589] in this PR.

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.

#31 @SirLouen
5 weeks ago

  • Keywords has-test-info added

Testing Instructions

I'm going to recap here with full testing instructions to help testers.

  1. Download these 4 images:
  1. Take note of the size of each image
  2. Upload them to Media
  3. Check if they show correctly or if they are blurred
  4. Check if the size of the images is bigger or smaller than the original.

Add this patch and check if:

  1. The images are not getting blurred after uploading
  2. The sizes are always lower after uploading.

This ticket was mentioned in Slack in #core-test by sirlouen. View the logs.


5 weeks ago

@elvismdev commented on PR #8810:


5 weeks ago
#33

Thanks @siliconforks and @SirLouen! I really appreciate both your insights in this.

@siliconforks you're right: the logic around $max_colors isn’t achieving the palette reduction goal in practice (I've now tested with rabbit-time-paletted-or8.png), then it’s not truly preserving [#59589]’s intent.

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 quantizeImage() in the problematic way that was introduced in [#59589] that made it to WP 6.8.

With that said, I’m going to close this PR in favor of #8813 , which cleanly addresses that regression. Thanks again!

#34 follow-ups: @iamshashank
4 weeks ago

Test Report

Description

This report validates whether the indicated patch works as expected.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/8810.diff

Environment

  • WordPress: 6.9-alpha-20250517.041504
  • PHP: 7.4.31-dev
  • Server: PHP.wasm
  • Database: WP_SQLite_Translator (Server: 5.5 / Client: 3.40.1)
  • Browser: Chrome 135.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty-Five 1.2
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Actual Results

  1. ✅ Issue resolved with patch.

#35 in reply to: ↑ 34 @SirLouen
4 weeks ago

Replying to iamshashank:

Test Report

Description

This report validates whether the indicated patch works as expected.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/8810.diff

Environment

  • WordPress: 6.9-alpha-20250517.041504
  • PHP: 7.4.31-dev
  • Server: PHP.wasm
  • Database: WP_SQLite_Translator (Server: 5.5 / Client: 3.40.1)
  • Browser: Chrome 135.0.0.0
  • OS: macOS
  • Theme: Twenty Twenty-Five 1.2
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Actual Results

  1. ✅ Issue resolved with patch.

@iamshashank this is a completely invalid Test Report, for two reasons:

  1. You have not used my clear Test instructions
  2. You have not tested the right patch.

Please, guys, do a little effort and take some time to read thoroughly the full reports to understand what needs to be tested and how needs to be tested, don't be like robots testing the first patch you see with the test information from the OP text only.

Last edited 4 weeks ago by SirLouen (previous) (diff)

#36 in reply to: ↑ 34 @iamshashank
4 weeks ago

Test Report

Description

This report validates whether the indicated patch works as expected.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/8813.diff

Environment

  • WordPress: 6.8.1
  • PHP: 8.2.27
  • Server: nginx/1.26.1
  • Database: mysqli (Server: 8.0.35 / Client: mysqlnd 8.2.27)
  • Browser: Safari 18.3
  • OS: macOS
  • Theme: Twenty Twenty-Five 1.2
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Additional Notes

  • The sizes remains the same.
  • A minor difference is seen in the image after uploading; there is a little blur which is noticeable on zooming.

https://ibb.co/9mKwJM3T

This ticket was mentioned in Slack in #core-test by sirlouen. View the logs.


4 weeks ago

#38 @SirLouen
4 weeks ago

I notice a very little difference between the 6.7.2 compression and my last patch compression given a minor difference in code.

For all images chunks were being fully excluded

$this->image->setOption( 'png:exclude-chunk', 'all' );

But for Indexed images, the 6.8 patch was expecting to add tNRS.

The result in size was even greater with my previous commit.

But now I'm taking in consideration the full exclusion for a little 218 bytes decrement in the vivid-green-bird.png test. Nothing great, but something to be respected.

Maybe one more testing required to get sure that we are outputting the same results with the patch, as we had in 6.7.2 before the [59589] regression

Recommended Current Testing

Using original image

  1. Result in 6.7.2
  2. Result in 6.8.1 without patch
  3. Result in 6.8.1 with patch

Check the 3 versions quality differences.

Last edited 4 weeks ago by SirLouen (previous) (diff)

#39 @iamshashank
4 weeks ago

Test Report

Description

✅ This report validates that the indicated patch works as expected.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/8813.diff

Environment

  • WordPress: 6.8.1
  • PHP: 8.2.27
  • Server: nginx/1.26.1
  • Database: mysqli (Server: 8.0.35 / Client: mysqlnd 8.2.27)
  • Browser: Safari 18.3
  • OS: macOS
  • Theme: Twenty Twenty-Five 1.2
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.2.0

Actual Results

  1. ✅ Issue resolved with patch.

Additional Notes

  • I have tested on local and WordPress Playground both
  • Tested 3 varients

In 6.7.2 with no patch
In latest 6.8.1 with no patch
In latest 6.8.1 with the patch

#40 @SirLouen
4 weeks ago

  • Keywords dev-feedback added; needs-testing removed

#41 @SirLouen
4 weeks ago

#63338 was marked as a duplicate.

#42 follow-ups: @adamsilverstein
4 weeks ago

  • Keywords needs-unit-tests added

@elvismdev thanks for the detailed bug report, sample images and PR! I'm just catching up here, however it sounds like you all have tracked down the source of the bug and come up with a reasonable, reliable solution.

It would be good to include a new unit test with an image that currently gets handled incorrectly (such as the sample image on the ticket). After the patch, the output images should be full color.

Last edited 4 weeks ago by adamsilverstein (previous) (diff)

#43 @adamsilverstein
4 weeks ago

  • Owner set to adamsilverstein
  • Status changed from new to assigned

#44 in reply to: ↑ 42 @SirLouen
4 weeks ago

  • Keywords has-unit-tests added; needs-unit-tests removed

Replying to adamsilverstein:

@elvismdev thanks for the detailed bug report, sample images and PR! I'm just catching up here, however it sounds like you all have tracked down the source of the bug and come up with a reasonable, reliable solution.

It would be good to include a new unit test with an image that currently gets handled incorrectly (such as the sample image on the ticket). After the patch, the output images should be full color.

PR updated with Unit Tests.

#45 in reply to: ↑ 42 @elvismdev
4 weeks ago

Replying to adamsilverstein:

@elvismdev thanks for the detailed bug report, sample images and PR! I'm just catching up here, however it sounds like you all have tracked down the source of the bug and come up with a reasonable, reliable solution.

It would be good to include a new unit test with an image that currently gets handled incorrectly (such as the sample image on the ticket). After the patch, the output images should be full color.

Thanks @adamsilverstein , & @wildworks ! I really appreciate you jumping in and helping guide this forward - and thank you again to everyone who contributed to the analysis, testing, and improvements across PRs.

I'm glad to see this regression now has a reliable fix in place.

#46 @adamsilverstein
4 weeks ago

  • Keywords commit added

Thanks for the unit tests @SirLouen!

I validated both the issue and the fix manually - the degradation is quite apparent with the provided test image, especially when zooming in on the color gradients. This fix should be good to go!

#47 follow-up: @adamsilverstein
4 weeks ago

@SirLouen I reviewed the PR and left some small feedback. Looks great overall.

#48 in reply to: ↑ 47 @SirLouen
4 weeks ago

Replying to adamsilverstein:

@SirLouen I reviewed the PR and left some small feedback. Looks great overall.

👌

Strict PHPCS was a little angry with those unlink but since these are tests, I assume that it's fine to use PHP functions straightly. All reviews sorted. Ready to ship.

#49 @adamsilverstein
4 weeks ago

Thanks again @SirLouen - I'll get this committed.

@nosilver4u commented on PR #8813:


4 weeks ago
#50

I did some further tests on old versions of ImageMagick & PHP, as there had been concerns regarding the use of getImageProperty. I went all the way back to Ubuntu 16.04 with PHP 7 and ImageMagick 6.8.9, and the color type reported by getImageProperty( 'png:IHDR.color-type-orig' ) is consistent to that stored in the IHDR chunk.

On the other hand getImageDepth (on 6.8.9 and 6.9.7) always reports a bit depth of 8 for images with depths of 1, 2, and 4. So it'll be good to have this changed to be more accurate and consistent all around!

@nosilver4u commented on PR #8813:


4 weeks ago
#51

I just noticed that the quantizeImage() method was removed completely in favor of the 'png8' method of palette reduction. Though none of the methods in ImageMagick are perfect, quantizeImage() produces the best quality across various images.

Also, from what I have in the code for EWWW IO, I believe some versions of IM change gray indexed images to grayscale, which was why we had the conditional to also apply the png8 method for those. If needed, I can try to do some comparisons on some of my test images to verify that.

@siliconforks commented on PR #8813:


4 weeks ago
#52

I just noticed that the quantizeImage() method was removed completely in favor of the 'png8' method of palette reduction. Though none of the methods in ImageMagick are perfect, quantizeImage() produces the best quality across various images.

Also, from what I have in the code for EWWW IO, I believe some versions of IM change gray indexed images to grayscale, which was why we had the conditional to also apply the png8 method for those. If needed, I can try to do some comparisons on some of my test images to verify that.

The png8 format is still being applied to gray indexed images, it's just being applied unconditionally now (so it is applied to both gray and color indexed images).

For the quantizeImage method - I did not notice a difference between these two cases:

  1. Calling quantizeImage and then converting to png8
  2. Skipping the quantizeImage call and just converting to png8 directly

Are there cases where it makes a difference?

@nosilver4u commented on PR #8813:


4 weeks ago
#53

The png8 format is still being applied to gray indexed images, it's just being applied unconditionally now (so it is applied to both gray and color indexed images).

For the quantizeImage method - I did not notice a difference between these two cases:

  1. Calling quantizeImage and then converting to png8
  2. Skipping the quantizeImage call and just converting to png8 directly

Are there cases where it makes a difference?

No, I don't think you'd see a difference in those two scenarios. The trick is to NOT set the png8 option unless absolutely necessary, as that applies a sub-par quantization algorithm. I'm in the middle of updating our own implementation to use getImageProperty (instead of direct file access), so I'll go through my test images to see if I can find some good examples.

@nosilver4u commented on PR #8813:


4 weeks ago
#54

Okay, I found a couple examples where it stands out the most. Sometimes it isn't so noticeable unless you know what to look for. In particular, the png8 method tends to use harsher edges, so it's mostly problematic when there is transparency in the image.

With png:format = png8:
https://github.com/user-attachments/assets/6a9b2b8e-daf2-4027-8740-7d2e1e14f73c

Only quantizeImage:
https://github.com/user-attachments/assets/1191c35c-c7af-40a5-bf68-2d291e0a4982

This one is much more noticeable, and if I remember correctly, was something I created specifically to illustrate the issues I had seen other folks posting about.

With png:format = png8:
https://github.com/user-attachments/assets/34f0a9b2-bdb6-4648-b2cf-7aa1cf9fa2af

Only quantizeImage:
https://github.com/user-attachments/assets/ee4e59e7-7915-4e8f-8a02-a428e34f23e8

Here's the original for oval-or8.png in case you want to add it into the test suite (hoping GitHub doesn't muck with it, should be 25,707 bytes):
https://github.com/user-attachments/assets/e5dbfc2d-2a28-4fed-a3e8-b2616023987c

#55 @adamsilverstein
4 weeks ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 60246:

Media: fix degraded handling for 24 bit PNG uploads.

Add additional logic to correctly identify indexed PNG images so output PNGs correctly match the uploaded image depth.

Fix a regression introduced in WordPress 6.8 [59589] where uploaded Truecolor PNG images (meaning > 8 bit) were mis-identified as indexed (8 bit) images, causing output images to be indexed instead of Truecolor resulting in a noticeable visual degradation.

Props elvismdev, wildworks, SirLouen, siliconforks, joemcgill, iamshashank, nosilver4u.
Fixes #63448.

@adamsilverstein commented on PR #8813:


4 weeks ago
#56

Hey @nosilver4u - apologies for crossing paths, I committed this fix before seeing your recent comments. I think what we have is still an improvement, however I'd like to make it as robust as possible by adding your test images to the test suite as well as improving the logic further.

When you have a moment can you please upload these test images to a trac ticket (fine to re-open the existing one), I am concerned as well that GitHub may have messed with your uploads)

@nosilver4u
4 weeks ago

indexed PNG image with transparency and gradient

#57 follow-up: @nosilver4u
4 weeks ago

@adamsilverstein No problem, I should have put my comments directly here instead of on the pull request! Agreed that we already have an improved solution, but would like to see quantizeImage() added back in to avoid regressions on indexed PNG images.

#58 in reply to: ↑ 57 @SirLouen
4 weeks ago

Replying to nosilver4u:

@adamsilverstein No problem, I should have put my comments directly here instead of on the pull request! Agreed that we already have an improved solution, but would like to see quantizeImage() added back in to avoid regressions on indexed PNG images.

I'm going to test this now, thing is that I found the png8 operation thing completely by chance. I got the exact same results in one of the images of the unit-tests and then I inferred (maybe wrongly) that png8 was actually doing a quantization in the background. I have not been able to find the source code of this imagick function to know what is been done behind the scenes, ngl.

Btw, I recommend you to post in Trac for comments and use github exclusively for code reviews, because gh integration with Trac is sometimes buggy (messages don't show in Trac for some reason).

#59 @nosilver4u
4 weeks ago

@SirLouen The png8 option does indeed perform a quantization, I don't know whether it is immediate, or delayed until the image is written. Originally, I had attempted to do something like $image->writeImage( 'PNG8:' . $output_file );. Unfortunately, that tended to crash, but setOption works just fine. See https://github.com/Imagick/imagick/issues/672 for more context, including why we need the png8 operation for grayscale images.


#60 @peterwilsoncc
4 weeks ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening to consider r60246 for backport to the 6.8 branch on a second committers sign-off.

#61 follow-up: @SirLouen
4 weeks ago

I see the problem with this png8 method and the image provided:

The original has an 8 bit alpha
Channel depth:
    red: 8-bit
    green: 8-bit
    blue: 8-bit
    alpha: 8-bit

While the resultin has an 1 bit alpha

Channel depth:
    red: 8-bit
    green: 8-bit
    blue: 8-bit
    alpha: 1-bit

Hence the result is a massive degradation in the transparency gradients. This is what happens when you don't truly know whats going on behind the scenes with this poorly documented functions from Imagick.

Doing a little research I see this old SO topic
https://stackoverflow.com/questions/13327675/php-imagick-quantize-transparent-equivalent
Commenting that the best way to preserve transparency is using 257 colors.

I don't see much differences between 256 aned 257:

https://i.imgur.com/TToK7y1.png

Except for the fact that switchin png8 for

$this->image->quantizeImage( 257, Imagick::COLORSPACE_RGB, 0, false, false );

Makes cloudflare-status.png and test8.png also fail

While 256 makes only test8.png fail (so this is what @nosilver4u comments about greyscale images for png8 instead of quantization?)

One idea that comes to my mind is using quantization only for alpha channels over 1 bit alpha. The problem is that there is a bug or something in Imagick, and is that in Q16 builds (like the one I'm using), it will be reporting 16 bit alpha in images like test8.png, that in reality, has a 1 bit alpha, provoking the troubles that nosilve4u described in the Imagick GH Issue

Conclusion: Edge cases are all over the place and playing with depths in Imagick seems to be a little random depending on the Imagick version we are using, so the only solution here is reverting to the old idea but sticking to 257 colours in the Quantization for maximizing transparency colour quality.

The only problem is that I have not found a Imagick method that can identify the test8.png greyscale to set it to png8. I've tried with getImageColorspace and its always reporting 13 for sRGBColorspace in my tests (probably in the original code it was being quantized with low bit depht, I have not checked this).

Also tried with getImageType with IMGTYPE_GRAYSCALE but its not been catched it either :(

TL;TR: For this solution I would need to find a method that identifies the test8.png as a greyscale 1-bit alpha depth and use png8 for that edge case (or find some consistent method that identifies alpha channel depth and use quantization for anything over 1)

This ticket was mentioned in PR #8833 on WordPress/wordpress-develop by @mukesh27.


4 weeks ago
#62

Trac ticket: https://core.trac.wordpress.org/ticket/63448

The https://core.trac.wordpress.org/changeset/60246 introduce some docblock issue that this PR handles.

#63 in reply to: ↑ 61 ; follow-up: @nosilver4u
4 weeks ago

@SirLouen With quantizeImage, I'm getting 8-bit alpha after quantizing thumbs for oval-or8.png, so that's one win. For the grayscale issue, I think there are a few potential issues when doing as you said:

$this->image->quantizeImage( 257, Imagick::COLORSPACE_RGB, 0, false, false );

First, 257 is too high, as an indexed image can only have 256 palette entries. The SO thread is quite old, and I suspect the commenters were fighting old bugs in IM. Using 257 colors was their attempt to prevent IM from using indexed mode, but still keeping the number of colors down, which improves the PNG compression dramatically--just not as much as a properly indexed PNG.

Now, often the quantization doesn't use as many colors as you allow, so 257 will still work on some images, and not on others. Also, some folks claim that you might need to use 255 to allow 1 entry for alpha. I could never confirm or deny that, given quantizeImage frequently uses less colors than specified, so I stuck with 255 as the max. We'd then have this:

$this->image->quantizeImage( 255, Imagick::COLORSPACE_RGB, 0, false, false );

Second, I think you might get better results with getColorspace(). At least that's what we came up with last year, though I honestly couldn't tell you whether I tested that for sure, or if I picked it up from some other place online. We can supply the colorspace like this then:

$this->image->quantizeImage( 255, $this->image->getColorspace(), 0, false, false );

Third, the number of colors in the original PNG can be far less than 255. So for test8.png, using 255 when the original has only 9, you could get a 20x larger image than you want. Here the original core fix isn't great since it relied on the (flaky) getImageDepth to set the maximum colors. But it could have been sufficient, as it uses getImageColors and uses that number if it is less than the maximum.

But then again, this all happens after the image has been scaled, at which point the number of colors is far too high (50,000 or more on certain sizes), so then $max_colors isn't constrained properly at all.

In EWWW IO, we run getImageColors to get the current color count prior to scaling, and then constrain that similarly by the number of colors allowed for the bit depth: 8-bit=255 colors, 4-bit=16 colors, 2-bit=4 colors, 1-bit=2 colors. *255 instead of 256 due to the aforementioned possibility that alpha takes up 1 entry. I checked again back to Ubuntu 16, and getImageColors appears to work properly even on ancient versions of IM. So we can do something like this:

$max_colors = $max_colors = pow( 2, $indexed_pixel_depth ); // or use a switch/case statement...
if ( is_callable( array( $this->image, 'getImageColors' ) ) ) {
    $current_colors = $this->image->getImageColors();
    $max_colors     = min( $max_colors, $current_colors );
}
// do the scaling/resizing, and afterward...
$this->image->quantizeImage( $max_colors, $this->image->getColorspace(), 0, false, false );

As for the grayscale issue, once the above is done, this seems to work for me:

if ( Imagick::COLORSPACE_GRAY === $this->image->getImageColorspace() ) {
    $this->image->setOption( 'png:format', 'png8' );
}

I added some debugging into our code in EWWW IO, and it's definitely detecting Imagick::COLORSPACE_GRAY. If you're still not getting that, I'd be curious what version of IM you've got. My dev server with Ubuntu 24.04 is running IM 6.9.12-98 and seems to work. For reference, this is what I'm working with in EWWW IO after updating our code to use getImageProperty(): https://github.com/nosilver4u/ewww-image-optimizer/blob/ed85dd1de2c49bdfc690e1924737e626e4fc0c17/classes/class-ewwwio-imagick-editor.php#L350

#64 in reply to: ↑ 63 ; follow-up: @siliconforks
4 weeks ago

My thoughts on the matter:

I would be inclined to just change the line $this->image->setOption( 'png:format', 'png8' ) to use quantizeImage instead, and then simply not worry about the issue with the test8.png image (i.e., just remove that test from the test suite). I just think that it's a pretty marginal edge case issue:

  • It's not a quality issue - the quality of the resized test8.png seems fine.
  • Yes, there is a file size issue (so it is essentially #36477 again) - however, it seems like it would be a fairly rare edge case since as far as we can tell it will only happen for grayscale images?
  • Also - assuming that it happens only for grayscale images - the file size is likely to be fairly small anyway. The size blowup with grayscale images will probably not be anywhere near as bad as some of the cases we saw in #36477.

Replying to nosilver4u:

I added some debugging into our code in EWWW IO, and it's definitely detecting Imagick::COLORSPACE_GRAY. If you're still not getting that, I'd be curious what version of IM you've got.

This doesn't seem to be working for me, either. I tried testing some different things - I'm not actually certain what the difference between getColorspace and getImageColorspace is, but it doesn't seem to matter since neither one seems useful here. Here is a simple test program:

<?php
$filename = 'test8.png';
echo $filename . "\n";
$image = new Imagick( $filename );
echo 'getColorspace() = ' . $image->getColorspace() . "\n";
echo 'getImageColorspace() = ' . $image->getImageColorspace() . "\n";
echo 'COLORSPACE_UNDEFINED = ' . imagick::COLORSPACE_UNDEFINED . "\n";
echo 'COLORSPACE_RGB = ' . imagick::COLORSPACE_RGB . "\n";
echo 'COLORSPACE_GRAY = ' . imagick::COLORSPACE_GRAY . "\n";
echo 'COLORSPACE_SRGB = ' . imagick::COLORSPACE_SRGB . "\n";

This is what it displays on my system:

test8.png
getColorspace() = 0
getImageColorspace() = 13
COLORSPACE_UNDEFINED = 0
COLORSPACE_RGB = 1
COLORSPACE_GRAY = 2
COLORSPACE_SRGB = 13

My dev server with Ubuntu 24.04 is running IM 6.9.12-98 and seems to work.

I tested it on an older Ubuntu 22.04 system with ImageMagick 6.9.11-60. So possibly there is an issue here with older ImageMagick versions - but even aside from that, I'm still not certain the logic is correct. If the purpose of this is only to handle images like test8.png it may be simpler to just not worry about this particular edge case.

#65 @adamsilverstein
4 weeks ago

I would be inclined to just change the line $this->image->setOption( 'png:format', 'png8' ) to use quantizeImage instead, and then simply not worry about the issue with the test8.png image (i.e., just remove that test from the test suite). I just think that it's a pretty marginal edge case issue

That makes sense to me if test8.png is truly an edge case image and all other issues are resolved with this approach. Curious to hear what others on the threat feel about this.

#66 in reply to: ↑ 64 ; follow-ups: @nosilver4u
4 weeks ago

I think the disconnect here is when one checks the image colorspace. I just tested on 22.04 with IM 6.9.11-60 as well and get Imagick::COLORSPACE_GRAY after quantizing the image.

So the issue isn't that test8.png is grayscale, it is that IM thinks it ought to make it grayscale after quantizing and reducing the number of colors back to 9. The file size difference seems dramatic to me, though that's a matter of perspective I suppose.

Here is the test script I wrote way back to illustrate this issue:

<?php
$filename = 'test8.png';
$newname  = 'test8-1140x662-grayscale.png';
$width = 1140;
$height = 662;

get_png_color_depth( $filename );

$image = new Imagick( $filename );

$image->resizeImage( $width, $height, Imagick::FILTER_TRIANGLE, 1 );

$image->setOption( 'png:compression-filter', '5' );
$image->setOption( 'png:compression-level', '9' );
$image->setOption( 'png:compression-strategy', '1' );
$image->setOption( 'png:include-chunk', 'tRNS' );

$current_colors = $image->getImageColors();
$max_colors     = min( $current_colors + 8, 255 );

$image->quantizeImage( $max_colors, $image->getColorspace(), 0, false, false );

if ( Imagick::COLORSPACE_GRAY === $image->getImageColorspace() ) {
        echo "image changed to gray colorspace\n";
//      $image->setOption( 'png:format', 'png8' );
}

$write_image_result = $image->writeImage( $newname );

if ( $write_image_result ) {
        get_png_color_depth( $newname );
}

function get_png_color_depth( $filename ) {
        if ( ! is_file( $filename ) ) {
                return;
        }

        $size = filesize( $filename );

        echo "size of $filename is $size\n";

        $file_handle = fopen( $filename, 'rb' );

        if ( ! $file_handle ) {
                return;
        }

        $png_header = fread( $file_handle, 4 );
        if ( chr( 0x89 ) . 'PNG' !== $png_header ) {
                return;
        }

        // Move forward 8 bytes.
        fread( $file_handle, 8 );
        $png_ihdr = fread( $file_handle, 4 );

        // Make sure we have an IHDR.
        if ( 'IHDR' !== $png_ihdr ) {
                return;
        }

        // Skip past the dimensions.
        $dimensions = fread( $file_handle, 8 );

        // Bit depth: 1 byte
        // Bit depth is a single-byte integer giving the number of bits per sample or
        // per palette index (not per pixel).
        //
        // Valid values are 1, 2, 4, 8, and 16, although not all values are allowed for all color types.
        $pixel_depth = ord( (string) fread( $file_handle, 1 ) );

        echo "pixel depth of $filename is $pixel_depth\n";

        // Color type is a single-byte integer that describes the interpretation of the image data.
        // Color type codes represent sums of the following values:
        // 1 (palette used), 2 (color used), and 4 (alpha channel used).
        // The valid color types are:
        // 0 => Grayscale
        // 2 => Truecolor
        // 3 => Indexed
        // 4 => Greyscale with alpha
        // 6 => Truecolour with alpha
        $color_type = ord( (string) fread( $file_handle, 1 ) );

        echo "color type of $filename is $color_type\n";

        fclose( $file_handle );
}

Without the PNG8 fix, the resized image is 138kb. Otherwise, with the proper fix to keep it indexed, test8-1140x662-grayscale.png is 32kb. That's an image with 9 colors that increases by more than 4x.

Now, if this was another of my 'contrived' examples, I might be inclined to agree that it is unnecessary. But this is one of a number of actual images from a real site that was experiencing #36477. To revert the fix for test8.png would be a regression, and seems silly to me when we already have a fix.

#67 @nosilver4u
4 weeks ago

Oh, and by the way, I looked up the difference between getColorspace and getImageColorspace, as I was curious also. getColorspace returns the "global colorspace", whereas getImageColorspace returns the colorspace for the current image.

Since @siliconforks' testing seems to indicate the global colorspace is typically undefined, it makes me think the usage of getColorspace within quantizeImage is really just telling ImageMagick to use whatever colorspace the image already has, in order to avoid hard-coding a colorspace that might mess up the image.

#68 in reply to: ↑ 66 ; follow-up: @siliconforks
4 weeks ago

Replying to nosilver4u:

I think the disconnect here is when one checks the image colorspace. I just tested on 22.04 with IM 6.9.11-60 as well and get Imagick::COLORSPACE_GRAY after quantizing the image.

OK, I see that it works for me now if you run quantizeImage first, then call getImageColorspace - it returns Imagick::COLORSPACE_GRAY.

However, the question then becomes: is that actually the right thing to check? Suppose you have an image which is similar to the oval-or8.png image above, but grayscale. Then won't that also be identified as Imagick::COLORSPACE_GRAY? Then won't forcing it to png8 destroy the alpha values in the image?

I think what you probably want to check here is what @SirLouen alluded to above in https://core.trac.wordpress.org/ticket/63448#comment:61 - you want to check whether the image has 1-bit alpha transparency. If so (or if it has no transparency at all), then it should be safe to use png8 format; otherwise no. But I'm not sure if there's an easy way to do that, or if it's even worth doing considering that this is probably only useful for a fairly rare edge case.

@siliconforks
4 weeks ago

Like oval-or8.png but grayscale

#69 in reply to: ↑ 68 @SirLouen
4 weeks ago

Replying to siliconforks:

Replying to nosilver4u:

I think the disconnect here is when one checks the image colorspace. I just tested on 22.04 with IM 6.9.11-60 as well and get Imagick::COLORSPACE_GRAY after quantizing the image.

OK, I see that it works for me now if you run quantizeImage first, then call getImageColorspace - it returns Imagick::COLORSPACE_GRAY.

However, the question then becomes: is that actually the right thing to check? Suppose you have an image which is similar to the oval-or8.png image above, but grayscale. Then won't that also be identified as Imagick::COLORSPACE_GRAY? Then won't forcing it to png8 destroy the alpha values in the image?

I think what you probably want to check here is what @SirLouen alluded to above in https://core.trac.wordpress.org/ticket/63448#comment:61 - you want to check whether the image has 1-bit alpha transparency. If so (or if it has no transparency at all), then it should be safe to use png8 format; otherwise no. But I'm not sure if there's an easy way to do that, or if it's even worth doing considering that this is probably only useful for a fairly rare edge case.

I was about to check for this, because on my debug yesterday, I found that I could not get into the COLORSPACE_GRAY conditional and I could not understand why. Then I noticed that the images were not into that color space but RGB as I commented above (according to Imagick). But what I did not try was sequentually checking for this after a Quantization (and now makes all the sense to me, because the original tests before this last patch were passing for test8.png which happened to do this sequence.

As you say, things start to become extremely tricky when gray images, have an alpha of 8-bit. Then quantization can't handle the image, but png8 restores it and at the same time destroying all gradients.

I'm not sure if we should consider this as "an edge case". Let's say that you have a picture gallery with full greyscale pictures. The impact is going to be there.

But at the same time, it seems that Imagick cannot find (to what I've researched, but the last time IHDR.color-type-orig was found in the wild).

About the 257 colours, it was this tricky strategy to preserve more quality in gradients (in exchange for a less optimized result). It's interesting because I can only see this quality improvement in one of my screens.

#70 in reply to: ↑ 66 ; follow-up: @SirLouen
4 weeks ago

Replying to nosilver4u:

Here is the test script I wrote way back to illustrate this issue:

This is the result using this script for the grayscale alpha image that @siliconforks provided

https://i.imgur.com/1IPJdJs.png

(Not sure what's going on with the width)

Also for the rabbit example these are my results:

Initial file info:
Size of rabbit-time-paletted-or8.png is 200581 bytes
Pixel depth of rabbit-time-paletted-or8.png is 8
Color type of rabbit-time-paletted-or8.png is 3

Final file info:
Size of rabbit-time-paletted-or8.png-mod.png is 213153 bytes
Pixel depth of rabbit-time-paletted-or8.png-mod.png is 8
Color type of rabbit-time-paletted-or8.png-mod.png is 6

It grows in size, and the color type goes from 3 (indexed?) to 6 RGBA

Last edited 4 weeks ago by SirLouen (previous) (diff)

#71 in reply to: ↑ 70 @siliconforks
3 weeks ago

Replying to SirLouen:

(Not sure what's going on with the width)

The width and height are just hard-coded in the test script - so if you don't change those, some images might get stretched (but I don't think that really matters for our purposes).

Did you run the test script with or without this line commented out?

//      $image->setOption( 'png:format', 'png8' );

My point with the oval-or8-grayscale-indexed.png image is that may work differently depending on whether you have that line of code or not.

Also for the rabbit example these are my results:

Initial file info:
Size of rabbit-time-paletted-or8.png is 200581 bytes
Pixel depth of rabbit-time-paletted-or8.png is 8
Color type of rabbit-time-paletted-or8.png is 3

Final file info:
Size of rabbit-time-paletted-or8.png-mod.png is 213153 bytes
Pixel depth of rabbit-time-paletted-or8.png-mod.png is 8
Color type of rabbit-time-paletted-or8.png-mod.png is 6

That's odd ... that's not what I'm getting:

size of rabbit-time-paletted-or8.png is 200581
pixel depth of rabbit-time-paletted-or8.png is 8
color type of rabbit-time-paletted-or8.png is 3
size of rabbit-time-paletted-or8.png-mod.png is 68109
pixel depth of rabbit-time-paletted-or8.png-mod.png is 8
color type of rabbit-time-paletted-or8.png-mod.png is 3

I wonder if there may be some differences in the way different ImageMagick versions are handling this...

I still think that using quantizeImage is probably the best way to go here. Even if there are some versions of ImageMagick which fail to save the image as indexed, using quantizeImage will still probably help to keep the file size from blowing up too much (though it still won't be as small as it would be if it had used indexed format).

#72 @nosilver4u
3 weeks ago

Hmm, you've certainly got a point there, resizing the grayscale version of oval-or8.png either results in a non-indexed image that preserves the gradient, or an indexed image that utterly destroys the gradient. Ugh...

Well, at the very least, it would help to use a lower number of colors with quantizeImage, based on the original number of colors in the image. That will make the "damage" less, even if ImageMagick refuses to use Indexed mode for images that are in the gray colorspace.

#73 follow-up: @adamsilverstein
3 weeks ago

Here is the relevant bit of code that we had in place previously:

// Reduce colors in the images to maximum needed, using the global colorspace.
$max_colors = pow( 2, $indexed_pixel_depth );
if ( is_callable( array( $this->image, 'getImageColors' ) ) ) {
    $current_colors = $this->image->getImageColors();
    $max_colors     = min( $max_colors, $current_colors );
}
$this->image->quantizeImage( $max_colors, $this->image->getColorspace(), 0, false, false );
/**
 * If the colorspace is 'gray', use the png8 format to ensure it stays indexed.
 */
if ( Imagick::COLORSPACE_GRAY === $this->image->getImageColorspace() ) {
    $this->image->setOption( 'png:format', 'png8' );
}

#74 in reply to: ↑ 73 @SirLouen
3 weeks ago

Replying to adamsilverstein:

Here is the relevant bit of code that we had in place previously:

// Reduce colors in the images to maximum needed, using the global colorspace.
$max_colors = pow( 2, $indexed_pixel_depth );
if ( is_callable( array( $this->image, 'getImageColors' ) ) ) {
    $current_colors = $this->image->getImageColors();
    $max_colors     = min( $max_colors, $current_colors );
}
$this->image->quantizeImage( $max_colors, $this->image->getColorspace(), 0, false, false );
/**
 * If the colorspace is 'gray', use the png8 format to ensure it stays indexed.
 */
if ( Imagick::COLORSPACE_GRAY === $this->image->getImageColorspace() ) {
    $this->image->setOption( 'png:format', 'png8' );
}

And for the icing, we had to add this report #63481

#75 follow-up: @SirLouen
3 weeks ago

Following from the last comment from @siliconforks

This is the result I got:

Size: 505955
Elapsed time: 0.526
Size (quantizeImage): 185245
Elapsed time: 0.558
Size (getImageColors): 185245
Elapsed time: 10.328

As I commented, I'm wondering two things:

  1. If there is a way to ask getImageColors to stop once it finds a number of colors during the histogram creation. I doubt there is an alternative for this, but it would significantly reduce the operation time for most cases.
  1. If its not simply fine, to stick to 256 for quantization, as most Indexed images don't have more than 256 colors. How much impactful is to quantize an 8 color image with a 256 fixed-set parameter? (ngl, but if we were talking about edge-cases, this is definitely the ultimate edge-case)
Last edited 3 weeks ago by SirLouen (previous) (diff)

#76 in reply to: ↑ 75 ; follow-up: @siliconforks
3 weeks ago

Replying to SirLouen:

If its not simply fine, to stick to 256 for quantization, as most Indexed images don't have more than 256 colors.

I think an indexed PNG can't possibly contain more than 256 colors, because that number is defined in the PNG specification, isn't it?

https://libpng.org/pub/png/spec/1.2/PNG-Introduction.html
https://libpng.org/pub/png/spec/1.2/PNG-Chunks.html#C.PLTE

I would think it would be fine to call quantizeImage( 256, ... ) with the number 256 just a hard-coded fixed value, unless someone knows of an image where that doesn't work well for some reason...?

Keep in mind - even if you start with an image with only 8 colors, it will probably have more colors than that after you resize the image.

#77 in reply to: ↑ 76 @SirLouen
3 weeks ago

Replying to siliconforks:

I would think it would be fine to call quantizeImage( 256, ... ) with the number 256 just a hard-coded fixed value, unless someone knows of an image where that doesn't work well for some reason...?

So the last bit for me would be finding out a way to spot alpha channels confidently.

If alpha channels spotting work for example say after ImageMagick version 7.0+, we could always suggest a dev note with a warning like: "If you image after resizing is losing gradients, please update your ImageMagick version". Although from what I read, it was only a Q16 trouble that affected all versions across.

#78 follow-up: @nosilver4u
3 weeks ago

As @siliconforks said, an indexed PNG won't contain more than 256 colors. Thus, it should be acceptable running getImageColors, so long as you do it prior to resizing and only for indexed PNG images.

That said, the differences in file size are not very large if you just use 256 as the max. The only one that gets way out of line is test8.png, but that's mostly due to the conversion to grayscale+alpha. It does get significantly worse (117 kb vs. 175 kb) when you let it use 256 colors instead of 9, so that may be worth considering.

That is, if we can't sort out how to keep an indexed gray image (color type 3) from being converted to grayscale+alpha (color type 4), then it is at least beneficial to limit the colors to the same number as the original upload.

#79 in reply to: ↑ 78 ; follow-up: @SirLouen
12 days ago

Here is a recap of the current situation

We have 4 types of Indexed images

  • Indexed Color w/o Alpha
  • Indexed Color with Alpha
  • Greyscale Indexed w/o Alpha
  • Greyscale Indexed with Alpha
  1. png8 destroy the alpha channel
  2. We have not found a mechanism to identify alpha in images to separate them from non-alpha
  3. We consider that preserving alpha is important, but the increase in size is a pain point
  4. Moreover, the increase in processing time with getImageColors is a pain point

Current opinions

@nosilver4u considers that we should go full quantize again for indexed and aren't concerned about the processing time and the increase in size.

Personally, I agree that we should not destroy the alpha of indexed images, and if this means an increase in size because we don't have a solution to separate Indexed images with alpha and without alpha, this is a sacrifice we should take, but, contrarily to @nosilver4u, I would stick to 256 colours to at least, not sacrifice in processing time (despite relative larger images, in very edge cases)

Possible Solution to progress

I think that this report can get blocked until someone finds a solution to separate the Alpha (or maybe Imagick happens to release something). Perhaps we should report this upstream (Imagick) and see if we can find some additional help?

This ticket was mentioned in PR #8933 on WordPress/wordpress-develop by @siliconforks.


11 days ago
#80

Because ImageMagick cannot save images with true alpha transparency in indexed PNG format, we need to check whether an image has true alpha transparency before telling ImageMagick to use the png8 (indexed) format.

Trac ticket: https://core.trac.wordpress.org/ticket/63448

#81 in reply to: ↑ 79 ; follow-up: @siliconforks
11 days ago

Replying to SirLouen:

  1. We have not found a mechanism to identify alpha in images to separate them from non-alpha

Did you ever try using the Imagick getImageChannelDepth() method? I've been experimenting with it and it seems to work for me - see https://github.com/WordPress/wordpress-develop/pull/8933

(Honestly this PR is getting a bit more complicated than I would like, but it's mostly just rearranging things to work with the original image before resizing, rather than after.)

#82 in reply to: ↑ 81 @SirLouen
11 days ago

Replying to siliconforks:

Did you ever try using the Imagick getImageChannelDepth() method? I've been experimenting with it and it seems to work for me - see https://github.com/WordPress/wordpress-develop/pull/8933

Interesting, I think I have not seen this function before.

With this result, things change significantly:

If it's indexed, and it doesn't have what you call "true alpha transparency", then you can go straight with png8

For the rest, Quantization is required to avoid losing the Alpha.

If you can add this modification, I think it's ready to go (I've put the suggestions in the PR review)

Last edited 11 days ago by SirLouen (previous) (diff)

#83 follow-up: @SirLouen
11 days ago

  • Keywords needs-unit-tests added; has-unit-tests removed

Also some unit tests would be relevant. @siliconforks if you want me to add them to your PR, ping me.

#84 in reply to: ↑ 83 ; follow-up: @siliconforks
11 days ago

Replying to SirLouen:

Also some unit tests would be relevant. @siliconforks if you want me to add them to your PR, ping me.

What sort of test(s) did you want to add?

Note that the PR should work with all existing tests. I know I suggested earlier that we might need to remove a test, but that is no longer necessary - that test should pass with the current PR.

#85 in reply to: ↑ 84 @SirLouen
10 days ago

  • Keywords has-unit-tests changes-requested added; dev-feedback commit fixed-major needs-unit-tests removed

Test Report

Description

🟠 This report validates that the indicated patch works as expected but some changes are expected

Patch tested:

  1. https://github.com/WordPress/wordpress-develop/pull/8933.diff
  2. The version I suggest here

Environment

  • WordPress: 6.9-alpha-60093-src
  • PHP: 8.2.28
  • Server: nginx/1.27.5
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
  • Browser: Chrome 137.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-Five 1.2
  • MU Plugins: None activated
  • Plugins:
    • Image Resizing Performance Tester 1.0.0
    • Test Reports 1.2.0

Testing Instructions

  1. I've run a performance test with these 4 images:
    'tests/phpunit/data/images/png-tests/oval-or8-grayscale-indexed.png',
    'tests/phpunit/data/images/png-tests/oval-or8.png',
    'tests/phpunit/data/images/png-tests/test8.png',
    'tests/phpunit/data/images/png-tests/rabbit-time-paletted-or8.png'
    
  2. With a regular resize test (identical to unit test's one)
  3. Checking with the two versions of patches for the size difference and the processing time with microtime
  4. Results in additional notes.

Actual Results

  1. 🟠 Issue resolved with a patch but some changes are expected for improvement.

Additional Notes

Replying to siliconforks:

Note that the PR should work with all existing tests. I know I suggested earlier that we might need to remove a test, but that is no longer necessary - that test should pass with the current PR.

As I said, I would simply ignore all around the COLORSPACE_GRAY thing as you showed here is not trustworthy at all.

For the tests, AFAIK, they are correct: They are first checking that size is not increasing (first problem), second they are checking that the colour type doesn't change (second problem) and now they are checking that the alpha channel bits don't change (third problem).

I think we have now everything covered.

Here I'm going to provide some performance testing based on my solution and yours for multiple images. For this topic, I've run some performance tasks and here are the results:
@siliconforks version: https://gist.github.com/SirLouen/0187eddadd4a924cf20986e678baccfc
My version: https://gist.github.com/SirLouen/953278594c8db7daec33452894dc9802

As we can see, in terms of size reduction, both methods are totally identical

But in terms of processing, my method of using png8 for non-true alpha is way better, which can be seen in both test8.png and rabbit-time-paletted-or8.png

Again, I'm not 100% sure what algorithm is being used in png8 but it simply works, and it was the sole reason of why I removed the quantize function in my previous patch.

Last edited 10 days ago by SirLouen (previous) (diff)

#86 @siliconforks
10 days ago

I've been checking the results of resizing when using the png8 format and I've noticed some quality issues with it. quantizeImage seems to give better quality.

A good example of this is the image from #63481 - for some reason, ImageMagick seems to reduce the number of colors when resizing and saving in png8 format. This does not happen when using quantizeImage, and the final quality seems much better. (Admittedly, the png8 version has a smaller file size, presumably since it has fewer colors. But they are both saved in indexed format, and they are both smaller than the full-size image, which was what the original goal was in fixing #36477. I don't think any further reduction in file size is worthwhile if it means a big loss in quality.)

@siliconforks
10 days ago

The image saved using png8 format

@siliconforks
10 days ago

The same image as the previous one, saved after using quantizeImage()

#87 follow-up: @SirLouen
9 days ago

@siliconforks got your point

Now, I'm even more curious about how png8 is working behind the scenes.

Anyway, can you refresh to me the reason of quantizing and then forcing png8 afterwards, specifically for greyscale images? I remember there was a reason, but I've read through the thread, but I can't completely remind why this was useful.

#88 in reply to: ↑ 87 ; follow-up: @siliconforks
9 days ago

Replying to SirLouen:

@siliconforks got your point

Anyway, can you refresh to me the reason of quantizing and then forcing png8 afterwards, specifically for greyscale images? I remember there was a reason, but I've read through the thread, but I can't completely remind why this was useful.

The problem appears to be that when ImageMagick sees an image with only gray in it, it seems to think, "PNG has a format specifically for grayscale images - I'll use that." But it's usually a smaller file when the image is saved as an indexed PNG, not as a grayscale PNG. So that's what the original purpose of the setOption( 'png:format', 'png8' ) line was - to force ImageMagick to save it in indexed format rather than grayscale format.

For grayscale images, I'm not sure if it makes a difference if you call quantizeImage() and then call setOption( 'png:format', 'png8' ), or if you skip the quantizeImage() call and just call setOption( 'png:format', 'png8' ).

#89 in reply to: ↑ 88 @SirLouen
9 days ago

  • Keywords needs-dev-note added

Replying to siliconforks:

For grayscale images, I'm not sure if it makes a difference if you call quantizeImage() and then call setOption( 'png:format', 'png8' ), or if you skip the quantizeImage() call and just call setOption( 'png:format', 'png8' ).

I've been researching a bit on how png8 operates under the hood.
Basically, it's a quantization with certain very specific parameters.
Some (not all) info here
It doesn't histogram the colours as the original image (as we were doing before), but it samples, resulting in way faster processing speeds.
Also forces a 1-bit transparency (as we were seeing with the alpha issues we were facing).
It appears that it also adds some dithering
Resulting, mostly, in the smallest size possible but losing a ton of shading in the process.

This function, was designed for converting very simple images, into an indexed PNG. I feel, overall, we have been playing a bit with all this as trial an error. Identifying both images in 44.png, I can see that the colour difference is massive. Almost double the colours used.

So I can get one first conclusion: png8 is not ideal for already png8 indexed images. Quantizing is okish, will show a little improvement for most images (specially with the safe approach we are using)

Now for the grey images, I'm using test8.png for the test:

With only png8 = 13Kb result
With only quantize = 74kb results (expected, this was the original bug)
With both = 11Kb result

So, basically, running both isn't resulting in a massive improvement.

To put an ending for this, I was thinking of suggesting you, omitting quantization from greyscale images
But after a little last debugging, I remembered why we can't really do this: we cannot identify greyscale images without a quantization first. This was the first "bug" of Imagick we identified with @nosilver4u
And this is why we were doing two runs: one for quantization first (to identify those gray images as COLORSPACE_GRAY and then another for compressing into png8 (otherwise they end with an even larger size than the original image).

So let's consider this a hacky way to bypass the colorspace trouble with greyscale images, and not have the issue of the increase in size due to the quantizing troubles. Conclusion: You can leave it as-is now with one thing:
I think all this must be documented. All this thought process has been highly convoluted, with numerous edge cases all over the place.

Maybe you could add in L538 something a comment like:

"And we need to first quantize greyscale images, because we have not found a way to identify their Colorspace without quantizing first."

And we can call this a day. Furthermore, I'm adding needs-dev-note because this should be documented somehow. According to the Handbook, as we are solving all this in a less than ideal way.

#90 @SirLouen
8 days ago

  • Keywords dev-feedback added; changes-requested removed

@adamsilverstein

I think that finally we have got to a point that the last patch is deliverable, sorting all the extra scenarios we encountered in the past weeks.

It also includes unit tests.

I'm not sure if dev notes will be required for this

Just a recap of what has been going on up to now.

  1. Resizing indexed images significantly increases their size, this was the first problem you solved with quantization
  2. Quantizing non-indexed images significantly reduces their quality (specially in terms of colour gradients)
  3. Force-indexing (png8) is a form of aggressive quantization with extra stunts like dithering and alpha channel removal.
  4. Force-Indexing Images, limit alpha to 1-bit, therefore images with more alpha channel, lose transparency gradients, for this reason, we should not force-indexing both greyscale and coloured images with more than 1-bit alpha channel.
  5. Indexed Greyscale images need to be both quantized and indexed to preserve their moderate size.
  6. We cannot identify indexed Grayscale (standard grayscale, 1-bit alpha) images after resizing because we have not found a way to identify them without quantizing first (this is the main reason of the dev-note as I feel we are not solving this in an ideal way).
Note: See TracTickets for help on using tickets.