Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#48736 closed defect (bug) (fixed)

Exclude PNG images from scaling after upload

Reported by: azaozz's profile azaozz Owned by: azaozz's profile azaozz
Milestone: 5.3.1 Priority: normal
Severity: normal Version:
Component: Upload Keywords: has-patch fixed-major
Focuses: Cc:

Description

Seems that large PNG images are not optimized well after scaling. In some cases the scaled image can be quite larger (by file size) than the original.

Perhaps best to consider disabling the "big image" handling for PNGs by default.

Attachments (1)

48736.diff (6.8 KB) - added by azaozz 5 years ago.

Download all attachments as: .zip

Change History (21)

#1 @azaozz
5 years ago

  • Keywords 2nd-opinion needs-patch added
  • Milestone changed from Awaiting Review to 5.3.1

Moving this to 5.3.1 for consideration. Reported in user feedback on https://make.wordpress.org/core/2019/10/09/introducing-handling-of-big-images-in-wordpress-5-3/

More testing/more use cases/more reports would be great :)

Also looking at/confirming handling of large GIFs on different servers/configurations would be good (do such 2560px GIFs even exist?).

This ticket was mentioned in Slack in #core-media by azaozz. View the logs.


5 years ago

#3 @shaharia.azam
5 years ago

I think PNG is a hugely used format. If we disable scaling then it will ignore massive amount of images.

And the disabling stuff just got introduce. I will check some of my sites to see how and exactly where PNG is losing it's quality noticeably.

#4 follow-up: @rmens
5 years ago

The handling of PNG images needs more checking. The width en height doesn't say it all. Overlays, sprites and patterns saved as PNG and used in for example site builders should not be touched, while photos uploaded as PNG could be touched.

In general this feature leaves a bad taste in my mouth. We run a large managed hosting platform for publishers with 60+ editors working daily with press and stock photo's and saw our average CPU load increase by 9 percent. Bandwidth is cheap, CPU and inodes are not. Images are mostly already served in a way that doesn't hurt the reader, for example with responsive images. With that already baked in WordPress original images never get served.

Last edited 5 years ago by rmens (previous) (diff)

#5 in reply to: ↑ 4 ; follow-up: @azaozz
5 years ago

Replying to rmens:

The handling of PNG images needs more checking.

Yep, I agree, opened this ticket for that.

We run a large managed hosting platform for publishers with 60+ editors working daily with press and stock photo's and saw our average CPU load increase by 9 percent.

Could you provide some more info about this, if possible? In particular, was that increase seen only when post-processing uploaded images or was it an overall increase after updating to WP 5.3?

Bandwidth is cheap, CPU and inodes are not.

Bandwidth may be "cheap" in some cases, but may also be "quite expensive". Overall it is more expensive that "disk storage".

Images are mostly already served in a way that doesn't hurt the reader, for example with responsive images. With that already baked in WordPress original images never get served.

Right. The changes in WP 5.3 expand and enhance WordPress' ability to serve images better. Generally the change was to add up to three larger image sub-sizes. This was done to support more devices with larger screens and/or high-density displays. Inside WordPress the largest size is also set as the "full" size. This ensures that original images are never used when they are "too big" for web use. There is a filter to control what the "too big" threshold is.

However it seems that in some cases PHP (GD and ImageMagick) do not handle resizing/scaling of very large PNG images well. So this ticket is to (temporarily?) turn off replacing the "full" size image with a scaled down version, and use the originally uploaded image as before.

I'm also thinking that more testing/consideration is needed on whether to create large sub-sizes for PNGs at all. Seems in some cases even a 1024px PNG can have very large file size, 2MB and over, when resized from PHP.

It would be great if you can provide some examples/more info/more testing. :)

Last edited 5 years ago by azaozz (previous) (diff)

@azaozz
5 years ago

#6 @azaozz
5 years ago

  • Keywords has-patch added; needs-patch removed

In 48736.diff: Exclude PNGs when scaling big images after upload. May create sub-sizes with smaller dimensions but greater file size.

Thinking this is "safe" for 5.3.1. Ideally both GD and ImageMagick would optimize PNGs. Also would be good to test the file size when creating large sub-sizes.

#7 in reply to: ↑ 5 ; follow-ups: @rmens
5 years ago

Replying to azaozz:

Could you provide some more info about this, if possible? In particular, was that increase seen only when post-processing uploaded images or was it an overall increase after updating to WP 5.3?

Had time to check this today. We have a separate cluster for the wp-admin and the CPU load increase is only noticeable there. On the front-end cluster the CPU load did not increase. This seems to be directly related to post-processing uploaded images. I wonder about the impact on large shared hosting platforms. We mostly use JPG images, not PNG. So this might be a 'separate' thing.

However it seems that in some cases PHP (GD and ImageMagick) do not handle resizing/scaling of very large PNG images well. So this ticket is to (temporarily?) turn off replacing the "full" size image with a scaled down version, and use the originally uploaded image as before.

PNG images are quite a beast to deal with. Some of our editors use a tool called 'ImageOptim' to reduce the size of PNG images a lot. This is mostly the case with images for infographics, quizzes and patterns. I did some tests with a bunch of iOS screenshots. The scaled version ended up being larger than the original PNG that was optimized by ImageOptim.

Disabling this for PNG seems like the right move. I however don't see a path ahead to ever make this work reliably for PNG.

Last edited 5 years ago by rmens (previous) (diff)

#8 in reply to: ↑ 7 @azaozz
5 years ago

Replying to rmens:

Disabling this for PNG seems like the right move. I however don't see a path ahead to ever make this work reliably for PNG.

Right. Disabling the scaling of big PNGs is about all we can do in 5.3.1.

Apart from trying to enhance the actual post-processing, a possible enhancement in the future may be to further extend the context of how an image is going to be used when the user uploads it. This is currently done for "custom backgrounds" and "custom header", but perhaps a more general UI would be better.

#9 in reply to: ↑ 7 @azaozz
5 years ago

Replying to rmens:

On the front-end cluster the CPU load did not increase. This seems to be directly related to post-processing uploaded images. I wonder about the impact on large shared hosting platforms.

IMHO it is a good idea to open another ticket with these concerns. True, in 5.3 there are two or three new large image sub-sizes being generated when the user uploads a (very) large image. This is done to prepare the image for "web use", and to better support large and high-density screens. Frankly I don't see a good workaround for now, the images need to be generated if we want the websites to look good.

It may be possible to use other command-line tools (on the server) to post-process images that are somewhat more efficient, but image processing is always very CPU and memory intense. It may eventually be possible to use js in the browser to "pre-process" images. That still needs better support and is in the planning stages.

In any case, having a place to explore more options and "compare notes" is a good idea :)

Last edited 5 years ago by azaozz (previous) (diff)

#10 @azaozz
5 years ago

  • Owner set to azaozz
  • Resolution set to fixed
  • Status changed from new to closed

In 46809:

Upload: Exclude PNG images from scaling after uploading. Fixes a case where resizing a very large PNG may create a scaled image that has smaller dimensions but larger file size than the original.

Fixes #48736.

#11 @azaozz
5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopen for 5.3.1.

#12 follow-up: @justlevine
5 years ago

To clarify: does this only affect the new scaled full-size image, or all generated pngs? #36477

Last edited 5 years ago by SergeyBiryukov (previous) (diff)

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


5 years ago

#14 @SergeyBiryukov
5 years ago

  • Keywords fixed-major added; 2nd-opinion removed

#15 in reply to: ↑ 12 @azaozz
5 years ago

Replying to justlevine:

To clarify: does this only affect the new scaled full-size image

Correct, this only affects scaling of large PNGs as that was added in 5.3 and the patch is targeted at 5.3.1.

Ideally the patch on #36477 (that affects all PNG sub-sizes) should be tested and added in 5.4. Then, perhaps, this can be revisited.

#16 @SergeyBiryukov
5 years ago

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

In 46834:

Upload: Exclude PNG images from scaling after uploading. Fixes a case where resizing a very large PNG may create a scaled image that has smaller dimensions but larger file size than the original.

Props azaozz.
Merges [46809] to the 5.3 branch.
Fixes #48736.

#17 @rogeriodec
5 years ago

Still creating larger size images:
https://i.imgur.com/eExLuTa.png

#18 @knutsp
5 years ago

@rogeriodec

This ticket was closed on a completed milestone.
If you have a bug or enhancement to report, please open a new ticket. Be sure to mention this ticket, #48736.

This ticket was mentioned in Slack in #core-media by pbiron. View the logs.


5 years ago

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


5 years ago

Note: See TracTickets for help on using tickets.