Make WordPress Core

Opened 7 years ago

Last modified 12 months ago

#36477 accepted defect (bug)

Responsive images (srcset) can include images larger than the full size

Reported by: peterdavehello's profile peterdavehello Owned by: joemcgill's profile joemcgill
Milestone: Future Release Priority: normal
Severity: normal Version: 4.4.2
Component: Media Keywords: has-patch needs-testing dev-feedback needs-unit-tests
Focuses: performance Cc:

Description

In many cases, I saw the resized and smaller images are much larger than the origin image, especially for the optimized images, it will make no sense to do that resize in this case, the worst case I've seen is about 13x larger than the origin and bigger image.

If an example can help to explain the problem, please take this picture:
https://cdn2.peterdavehello.org/wp-content/uploads/2016/04/status.png

Many thanks!

Attachments (2)

36477_1.diff (1.0 KB) - added by codex-m 7 years ago.
Retain number of unique colors when resizing, save file size
36477_2.diff (2.0 KB) - added by codex-m 7 years ago.
Use getImageType() to check if the image is indexed color encoded

Download all attachments as: .zip

Change History (46)

#1 @Presskopp
7 years ago

The owner of this website (cdn2.peterdavehello.org) does not allow hotlinking to that resource (/wp-content/uploads/2016/04/status.png).

with http instead of https it works

Last edited 7 years ago by Presskopp (previous) (diff)

#2 @joemcgill
7 years ago

  • Owner set to joemcgill
  • Status changed from new to reviewing

Hi @peterdavehello,

Welcome to the Trac, and thank you for the report.

I assume that your are referring to the file size (KB) of the images and not the dimensional size of the images (width x height). In which case, you are correct. Resizing an indexed color image like a PNG or GIF to a smaller size can oftentimes increase the overall file size because of the way the resampling process works (see: #30402). This is not an uncommon result among other image editors as well, but is most noticeable in use cases where the source image has gone through an optimization process before being uploaded.

The default image compression improvements that will be included in WordPress 4.5 (see: #33642) help with this problem, but the smaller images can still be larger than the full size images at times. However, for users who aren't optimizing their images before uploading (a majority of users, I suspect), using srcset and sizes often results in a significant reduction in file size, so we

Since WordPress has no awareness of the file size of each image when it is writing the image markup, I'm not sure what we can do to avoid this situation entirely, except to continue to make improvements on the default compression strategy for indexed color images.

#3 follow-up: @peterdavehello
7 years ago

Hi @joemcgill,

Thanks for your prompt reply in detail, yes, I'm talking about the file size, not the dimensional size, and my basic idea is try to get the size before and after the resize, once the resized images is larger, than we drop that size of image, what do you think?

#4 in reply to: ↑ 3 @joemcgill
7 years ago

  • Keywords needs-patch added

Replying to peterdavehello:

Hi @joemcgill,

Thanks for your prompt reply in detail, yes, I'm talking about the file size, not the dimensional size, and my basic idea is try to get the size before and after the resize, once the resized images is larger, than we drop that size of image, what do you think?

I would be careful about dropping images altogether, but it may be possible to save the file size as attachment metadata and factor that into our srcset considerations in wp_calculate_image_srcset().

#5 @peterdavehello
7 years ago

Hi @joemcgill,

Seems the problem is worse then I said originally, actually I found that some pictures didn't be manually optimized still have the same problem with this resize, for example, I use the PrintScreen key under Linuxmint 17.3 Cinnamon to catch the screen, the process never takes more than few seconds which means there is no additional process to optimize the image, but it's still become larger after the resize in WordPress, that makes no sense.

#6 follow-up: @joemcgill
7 years ago

  • Milestone changed from Awaiting Review to 4.6
  • Status changed from reviewing to accepted

The idea of trying to explicitly make file size a consideration when calculating srcset attributes. However, this will require us to add the file size of each image as attachment metadata. Perhaps we could use Imagick::getImageLength during the resize process to grab this info?

This ticket was mentioned in Slack in #core-images by joemcgill. View the logs.


7 years ago

#8 @azaozz
7 years ago

the resized and smaller images are much larger than the origin image, especially for the optimized images.

I'm thinking we need a better workflow for people that optimize their images before uploading them. Currently the steps are:

  1. Upload optimized image. WordPress creates non-optimized sub-sizes and the image meta to go with them.
  2. Create images with the exact dimensions like the WordPress sub-sizes.
  3. Rename them to match the file names generated by WordPress.
  4. Upload the optimized sub-size images with FTP replacing the non-optimized.

Of course setting GD and Imagick to create optimized images would be best, but I'm not sure if we can enhance that further.

Another possibility is to make a case with the hosting companies to install one of the jpeg optimizers (libjpeg/jpegtran?) and add cron job to scan the uploads directory and optimize the sub-sizes (if they don't do that already). This will reduce both bandwidth and space usage :)

In WordPress we could probably make the uploader "recognize" when the user is uploading a sub-size of an image and just replace the file instead of treating it as a new upload. That will let the users create optimized sub-sizes and upload them together.

#9 follow-up: @nosilver4u
7 years ago

Just curious, is the problem generally negated if you are "post-optimizing" your images? That is, using a plugin like EWWW Image Optimizer or one of the other I.O. plugins...

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

Replying to nosilver4u:

Of course. Post-upload optimizing is probably the best way to do this as long as the plugin optimizes the images well and doesn't make you over-use the resources if on a shared server.

This is also better for the hosting company, hence my suggestion to do it automatically by a script when server load is low.

#11 in reply to: ↑ 6 @nosilver4u
7 years ago

Replying to joemcgill:

The idea of trying to explicitly make file size a consideration when calculating srcset attributes.

The unfun side-effect of that is all the I.O. plugins need to make sure to update that information in the metadata after optimization as well. And I don't know that you need to use the Imagick method to get the file size, do you? Wouldn't a quick call to filesize() be sufficient after the resizes has been saved?
Also, filesize() would be "editor-agnostic" since you have the potential for GD, Imagick, maybe even Gmagick or others...

Last edited 7 years ago by nosilver4u (previous) (diff)

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


7 years ago

#13 @joemcgill
7 years ago

  • Component changed from Upload to Media
  • Milestone changed from 4.6 to Future Release

Punting due to a lack of patch and moving to the Media component so this can get more visibility in 4.7.

@codex-m
7 years ago

Retain number of unique colors when resizing, save file size

#14 @codex-m
7 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

@joemcgill I think we can handle this in coming 4.6. The trick is to check for unique colors before resizing. If its 256 or below, its indexed color type or already optimized. We implement a file size optimizing filter. In my test, FILTER_POINT works best.

I did several tests and it handles already compressed PNG images very well. So it means that after resizing, resulting file size is not very large. Since the number of unique colors will be preserved while not affecting quality ;)

Let me know what you think. Cheers!

Emerson

#15 @joemcgill
7 years ago

  • Milestone changed from Future Release to 4.7

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


7 years ago

#17 @desrosj
7 years ago

This will get some attention during the next media component meeting. A decision to land in 4.7 will be made.

This ticket was mentioned in Slack in #core-images by joemcgill. View the logs.


7 years ago

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


7 years ago

This ticket was mentioned in Slack in #core-images by mike. View the logs.


7 years ago

#21 @mikeschroder
7 years ago

  • Keywords needs-patch added; has-patch needs-testing removed
  • Milestone changed from 4.7 to Future Release
  • Summary changed from Responsive images(srcset) feature makes images fatter. to Responsive images (srcset) can include images larger than the full size

I tested 36477_1.diff today -- Thanks for the patch!

Unfortunately, it looks like running getImageColors is between 8% and 75% of image upload times, with only small images (~120x120) being nearer to that 8%.

In the tests I ran (on a standard shared server), this added up to an additional 19.4 seconds to resize time for iPhone 7-sized images.

This means that if we're going to go with a different method of resizing for different amounts of colors present, it'll have to have a different way to detect the amount of colors.

I'd be interested in seeing what a solution that includes saving the sizes of images on upload/resize would look like. This would make the data available to the srcset code, for better choices on which images are good to include.

I'm going to remove the 4.7 milestone, but if this ticket gets a patch that is ready for 4.7, am more than happy to take another look. Also updating the title to more accurately reflect the nature of the bug.

Edit: Fixed incorrect number of seconds

Last edited 7 years ago by mikeschroder (previous) (diff)

This ticket was mentioned in Slack in #core-images by mike. View the logs.


7 years ago

#23 follow-up: @codex-m
7 years ago

Thank you @mikeschroder for feedback and testing! I agree that getImageColors() will introduce a performance problem especially when re-sizing very large images. It is because processing time is directly proportional to the total number of pixels on the image.

I'm currently testing a new patch which does not use this function while still solving the main issue and not introducing any new issues. It's looking good so far and will post it here very soon ;) I'm hoping we could introduce this in 4.7. For your update. Cheers.

#24 in reply to: ↑ 23 @mikeschroder
7 years ago

Replying to codex-m:

Thank you @mikeschroder for feedback and testing! I agree that getImageColors() will introduce a performance problem especially when re-sizing very large images. It is because processing time is directly proportional to the total number of pixels on the image.

I'm currently testing a new patch which does not use this function while still solving the main issue and not introducing any new issues. It's looking good so far and will post it here very soon ;) I'm hoping we could introduce this in 4.7. For your update. Cheers.

No problem! Absolutely, if there is a patch before the deadline, it'll be considered for commit. Thanks for digging into this!

@codex-m
7 years ago

Use getImageType() to check if the image is indexed color encoded

#25 @codex-m
7 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

@mikeschroder Here it is. I attached 36477_2.diff. Basically I dig deeper into the issue and found out that this only affects indexed-color images. These images are limited to 256 colors. And when resized by WordPress, additional colors are added. And the image type is then converted from indexed color type to RGB true color. This increases the file size.

I also found out that to eliminate performance issue while checking for image colors. Its best to check for image type only using Imagick::getImageType. This will return if the image is indexed-color encoded or RGB.

The fix is simple, it then checks if the image type is indexed color. If yes, retains the image type after saving. This dramatically reduces the file size for affected image types while retaining our existing filters/quality settings ;)

In my test, this fix reduces the file size of indexed color images resizing to around 50%. This fix also resolves the following existing WordPress trac tickets like this: https://core.trac.wordpress.org/ticket/30402. It is because after resizing, the image type is now retained.

Let me know what you think. Thanks!

#26 @peterdavehello
7 years ago

AWESOME!!!!!!! Thanks @codex-m !!!!!!!!

#27 @codex-m
7 years ago

  • Keywords dev-feedback added

Any chance we can move this to 4.7 milestone? We are managing several big sites. It would be good to have this included in the coming release because it would lead to smaller backup files for uploads directory in the future :) Thanks for any feedback.

#28 @joemcgill
7 years ago

Hi @codex-m,

Thanks for following up on this and for continuing to push it forward. Unfortunately, we're a few weeks too late for this cycle, but definitely something we can evaluate for 4.8.

#29 @joemcgill
6 years ago

#39236 was marked as a duplicate.

#30 @peterdavehello
6 years ago

As this looks to be a strange bug which makes the size of the uploaded png images super fat and then wastes the bandwidth and disk space, would you guys consider to provide fix tool or plugin or something like that to scan the scaled png images and make them to use the indexed-color as the origin images to "fix" the size problem? Could be super helpful! Thanks a lot!

#31 @justlevine
4 years ago

Just wondering: does this address that WordPress upscales all 8-bit pngs to 16-bit, essentially doubling the size?

Because that's what's currently happening on all my sites.

While I saw mentioned how optimization on a resize might map new colors, but not sure if that translates to 'we need ImageMagick to change our pngs to 16-bit'.

If it's a separate issue, let me know and I'd be happy to open a new ticket.

#32 @cezza
4 years ago

What's happening to this ticket? Happy that @justlevine commented recently, after 2 years of inactivity. This is a very big issue that actually is preventing a lot of website to switch to the built-in solution wordpress provides.

https://www.dropbox.com/s/j2t5yu0rhhwyfdy/Screenshot%202019-01-04%2010.20.13.png

Check this screenshot to see how bad this problem can be.

#33 @peterdavehello
4 years ago

Friendly ping @joemcgill as it's been a few years here, it was v4.4.2 and it's almost v5.2 now, WordPress still growths rapidly during this period, would be really great if this issue can be resolved, many thanks again!

#34 @peterwilsoncc
4 years ago

#47311 was marked as a duplicate.

#35 @azaozz
4 years ago

  • Milestone changed from Future Release to 5.4

Lets try to get 36477_2.diff in 5.4. Ideally it should be tested to confirm that it doesn't reduce image quality, and perhaps a unit test should be added.

#36 @jokanane
3 years ago

Is the patch 36477_2.diff still valid? I applied it in my local environment (WP 5.3.2), but I am still seeing the issue mentioned by @justlevine and #30402, where PNGs I've exported as 8-bit in Photoshop have generated pixel sizes that are much larger in kb than the original file.

346895 image-8bit-1024x520.png
23022  image-8bit-150x150.png
747603 image-8bit-1536x780.png
39277  image-8bit-300x152.png
214964 image-8bit-768x390.png
275028 image-8bit-768x432.png
303507 image-8bit-800x450.png
391111 image-8bit-800x600.png
163554 image-8bit.png

The original file in the test above is 1600x812px. The generated 1536x780px is 4.5 times larger.

I've tried both uploading an image file and running wp media regenerate, and see no difference with the patch.

I hope I'm wrong, and WP 5.4 with the patch will fix the issue...

#37 @audrasjb
3 years ago

  • Milestone changed from 5.4 to Future Release

Hi,

With 5.4 Beta 3 approaching and the Beta period reserved for bugs introduced during the cycle, this is being moved to Future Release. If any maintainer or committer feels this should be included or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

#38 @sallyruchman
3 years ago

Hello,
I have this issue with WordPress Version 5.3.2. On Dev Website all Plugins disabled, just Theme (Divi Theme 4.4.0) activated. File Optimization done before with Shortpixel, after Upload Images to WordPress, the created Responsive Image Sizes have bigger File Size than the Original File.

Filename: Filesize:
Image-shortpixel-lossy.jpg 58465
Image-shortpixel-lossy-980x735.jpg 144868
Image-shortpixel-lossy-768x576.jpg. 100274
Image-shortpixel-lossy-1280x960.jpg 215971
Image-shortpixel-lossy-1080x810.jpg. 168313
Image-shortpixel-lossy-1080x675.jpg 149977
Image-shortpixel-lossy-1024x768.jpg. 154929

Is there a solution / fix ?
Thx

Last edited 3 years ago by sallyruchman (previous) (diff)

#39 @jokanane
3 years ago

What is the process to flag this for a higher priority? The generated PNGs are just way too big. Thanks.

The original image:

  • 1600x812px
  • 8-bit / 256 colors / palette
  • 164kb

Generated files:

  • 1536x780px: 748kb
  • 768x432px: 275kb
  • etc.
Last edited 3 years ago by jokanane (previous) (diff)

#40 @JeffPaul
3 years ago

  • Keywords needs-unit-tests added

#41 @wpwebdevj
3 years ago

After 5 years this still hasn't been fixed? Is anyone working on it?

#42 @wpfed
2 years ago

I noticed that when uploading a 8bit png image WordPress would resize and change bit depth to 24 or 32 bit, which created file sizes multiple times bigger. Opened ticket https://core.trac.wordpress.org/ticket/53408#ticket

#43 @mukesh27
12 months ago

Hi there,

No progress in the last some time.

@azaozz is this ticket is in your radar for the upcoming release? Happy to test the new patch or another approach.

#44 @wpfed
12 months ago

@mukesh27 They're looking at it in the WP performance plugin https://github.com/WordPress/performance/issues/264

Note: See TracTickets for help on using tickets.