#21668 closed enhancement (fixed)
WordPress does not respect jpeg’s progressive feature of original image
Reported by: | _ck_ | Owned by: | adamsilverstein |
---|---|---|---|
Milestone: | 6.5 | Priority: | normal |
Severity: | normal | Version: | 3.4.1 |
Component: | Media | Keywords: | has-patch commit add-to-field-guide |
Focuses: | performance | Cc: |
Description
Every time WordPress does imagejpeg
it should be doing imageinterlace
beforehand to set progressive mode, but it's not.
http://php.net/manual/en/function.imageinterlace.php
Progressive jpeg has been supported by ALL browsers since 2001 or so.
progressive demo: http://bbshowcase.org/progressive/
Note that even if an ancient browser is listed somewhere as not rendering progressive, progressive jpeg still will be displayed, simply showing it all at once rather than gradual.
IE6 for example WILL load progressive jpeg, it will just not render it progressively, instead it will load the entire image and then render all at once at the end.
Progressive algorithm will also make slightly smaller jpeg as a side effect in most cases.
ie. in media.php
`
imageinterlace($newimage,1);
imagejpeg( $newimage, ...
`
This update is years overdue.
It might be possible to futureproof this by adding a filter to $newimage
before the final imagejpeg
. I am uncertain if that would work since filters would not pass $newimage
by reference and instead make a copy - while newimage is only a pointer to php/gd memory. Can apply_filters be forced to work somehow by reference instead of a copy? Maybe that should be another feature idea.
Attachments (11)
Change History (86)
#2
in reply to:
↑ 1
@
12 years ago
Replying to scribu:
Many servers do not have imagemagick installed, which given the WP install footprint could mean hundreds of thousands of websites.
Why not fix the default mode of WP, considering only 20 characters are needed to be added in three files and do the responsible thing to make the web faster for millions of visitors on all those pages.
Without a filter, there is no way to affect the output image, so changing the core is required. What's interesting is if they added a filter for this or after it, people could even intercept the jpeg output for watermarking, etc.
#4
@
12 years ago
I don't think progressive images should be the default. I also don't really believe the images will get smaller or there is an extra lost of quality.
About being responsible and that this update is overdue is not really helpful for this ticket. I would love to get some more insight in progressive images and if/how this can also effect PNGs and GIFs.
That said a filter is indeed needed and hopefully it is possible with a reference instead of copying.
#6
in reply to:
↑ 5
@
12 years ago
Replying to SergeyBiryukov:
Thanks for making a patch - I would disagree with the patch for wp-admin/includes/image.php
because interlace should only be set specifically before imagejpeg and nothing else as gif and png progressive support is spotty.
Basically anywhere imagejpeg
is found in WP code, either a filter-by-reference should be done beforehand or imageinterlace
set.
I am uncertain if an apply_filters will obey a php function that is declared as function foo(&$var)
and really pass it by reference or first make a copy of the variable it's passed and then try to pass that copy by reference to the function. $newimage
almost certainly has to be passed by reference and not a copy (but I could be wrong).
We could test this theory easily:
ie. in media.php
add_filter('imagejpeg','setprogressive'); function setprogressive(&$handle) { if (function_exists('imageinterlace')) {imageinterlace($handle,1);} } ... apply_filters('imagejpeg',$newimage); // no need to return variable if it's by reference? imagejpeg( $newimage, ...
If the image produced is progressive, then it works.
#9
@
12 years ago
Re-did the patch for 3.5 taking into account the wp-image-editor for both GD and Imagick, JPG and PNG.
@
11 years ago
Revised to only apply progressive encoding to JPEG images and to handle both the GD and Imagemagick paths.
#14
@
11 years ago
I updated the patch to only encode JPEGs progressively.
Progressive JPEGs are nearly always smaller than baseline in addition to the improved user experience and since the largest images are usually photos we will get most of the bang for our buck there anyway.
PNG's and GIF's tend to inflate the size so I don't recommend touching them.
There are quite a few studies on the benefits of progressive JPEGs from well respected developers and companies:
Yahoo's image optimization post from 2008 (Stoyan is now at Facebook)
Ann Roboson's article for the 2012 web performance calendar
My work at Google and WebPagetest on it
Akamai's image optimization presentation from Velocity 2013
Other than the patch itself, let me know if there is anything I can do to help make this happen (more data, research, testing, whatever).
#16
@
11 years ago
When testing, you can validate that progressive JPEGs are created here.
I tested the patch on an Ubuntu 12.04 system with php 5.3.10 and imagemagick 6.6.9-7 (both the latest from the standard apt sources) and validated both the GD and imagemagick code paths. The GD code path was tested before installing imagemagick and logging was added to both output paths to validate that the writes were done through the expected paths. Progressive JPEGs were correctly created in both cases.
These were photos that I uploaded to my test Wordpress site with the patch (and logging) applied.
#17
@
11 years ago
- Keywords 3.7-early added
- Milestone changed from Awaiting Review to Future Release
#18
follow-up:
↓ 21
@
11 years ago
@pmeenan: Thanks for the patch!
@markoheijnen, @derekspringer and I we chatted about this a bit at the Contributor Day at WCSF.
Notes from the conversation:
- This is still something we want to do.
- Initially it's something we'd like to introduce as an optional feature via a filter, due to the increase in file sizes, but we could it consider later becoming a default for JPG alone.
- We'd like to see support for at least PNG as well, since there's no reason not to support it via the framework when it's optional, and PNG is a commonly used format. As you noted, this would not be something done by default either, due to the increase in file size.
@
11 years ago
Restored interlacing for .gif and .png images, added 'image_save_progressive' filter.
#19
@
11 years ago
Big thanks to @DH-Shredder for giving me some pointers today while I worked out the latest patch. I was able to successfully test the progressive .jpg creation using the links that @pmeenan provided using gd, but haven't found a reliable way to test successful creation of interlaced .gif & .png or any of the Imagick options.
If someone with access to an install with Imagick would like to test the images, that would be pretty cool :)
#21
in reply to:
↑ 18
@
11 years ago
Replying to DH-Shredder:
@pmeenan: Thanks for the patch!
@markoheijnen, @derekspringer and I we chatted about this a bit at the Contributor Day at WCSF.
Notes from the conversation:
- This is still something we want to do.
- Initially it's something we'd like to introduce as an optional feature via a filter, due to the increase in file sizes, but we could it consider later becoming a default for JPG alone.
- We'd like to see support for at least PNG as well, since there's no reason not to support it via the framework when it's optional, and PNG is a commonly used format. As you noted, this would not be something done by default either, due to the increase in file size.
FWIW, the progressive JPEGs should almost always be smaller than baseline (unlike PNG and GIF). In Stoyan's testing above, 75% of JPEG's that were under 10KB were smaller as progressive and 94% of JPEG's over 10KB were smaller as progressive. That's largely why I killed the PNG and GIF support in my version of the patch and had it turned on by default for JPEG.
#22
follow-ups:
↓ 23
↓ 43
@
11 years ago
While progressive JPEGs can be smaller, they are not always desired from an aesthetics standpoint.
I am not inclined to support 21668.5.patch as it appears to change functionality, and/or increase the requirements for GD or Imagick to be used. Please correct me if I am mistaken on these points.
#23
in reply to:
↑ 22
;
follow-up:
↓ 26
@
11 years ago
Replying to nacin:
While progressive JPEGs can be smaller, they are not always desired from an aesthetics standpoint.
I am not inclined to support 21668.5.patch as it appears to change functionality, and/or increase the requirements for GD or Imagick to be used. Please correct me if I am mistaken on these points.
It doesn't change the requirements for GD or Imagick. The only change is to set the progressive flag in cases where GD or Imagick were already being used to save JPEGs (resize or edit).
As far as the asthetics go, I can't really say because I'm not aware of any studies that have been done to evaluate end-user's perception of either baseline (top-down rendering) or progressive. I do know lots of individual developers have strong opinions one way or another and there are plenty in both camps but I'm not aware of any data either way.
I do have data backing the faster visual experience (not just of the blurry initial passes) but that doesn't really address user perceptions and asthetics.
#24
@
11 years ago
- Milestone changed from 3.7 to Future Release
I am a serious news junkie. Some of my favorite sites do not use WordPress, I can't remember the last time I have seen a progressive JPEG used.
I'm fine with adding filters in place (if they don't already exist) to make it easy for a plugin to flip the interlace bit. But I don't see enough evidence here to change the default.
#25
@
11 years ago
It's been 15 months since this improvement was proposed and still no inclusion into WP core? Come on, it's so simple, it doesn't break anything, but it makes JPGs load faster and it improves Speed Index. Isn't it what should everybody care about in 2013? Provided that only ca. 7% JPGs are progressive, it's high time to change that. Thanks in advance!
#26
in reply to:
↑ 23
@
11 years ago
Replying to pmeenan:
Replying to nacin:
While progressive JPEGs can be smaller, they are not always desired from an aesthetics standpoint.
I am not inclined to support 21668.5.patch as it appears to change functionality, and/or increase the requirements for GD or Imagick to be used. Please correct me if I am mistaken on these points.
It doesn't change the requirements for GD or Imagick. The only change is to set the progressive flag in cases where GD or Imagick were already being used to save JPEGs (resize or edit).
As far as the asthetics go, I can't really say because I'm not aware of any studies that have been done to evaluate end-user's perception of either baseline (top-down rendering) or progressive. I do know lots of individual developers have strong opinions one way or another and there are plenty in both camps but I'm not aware of any data either way.
I do have data backing the faster visual experience (not just of the blurry initial passes) but that doesn't really address user perceptions and asthetics.
Nacin is right about the possibility to raise the requirements. In case of Imagick the check happens on load. I change that logic now. Not sure what you think about this way.
Since Imagick supports I wonder if we should limit the possibilities in Imagick to only allow it on png, gif and jpg.
#29
@
11 years ago
For the sake of a more (apparently) performant Internet, saving images as progressive when possible is a default option that I hope WordPress adopts.
At parade.com we used 21668.4.patch for several months on the WP3.5 branch (https://core.trac.wordpress.org/attachment/ticket/21668/21668.4.patch). The improvements in 21668.5.patch (https://core.trac.wordpress.org/attachment/ticket/21668/21668.5.patch ) are nice - allowing filter-based control - but I think the default false
option is the wrong one for the Web. The changes in 21668.6 (https://core.trac.wordpress.org/attachment/ticket/21668/21668.6.patch ) seem more complex than required and are to me confusing, especially since it appears to be wrongly checking for a "PNG" mimetype when the $mimetype (correctly) is "image/png".
Unfortunately I was not able to validate any of these patches as working on WordPress 3.8 with both ImageMagick and GD installed using the identify
command and the verbose
flag set (in my understanding, identify -verbose $path_to_file | grep Interlace
should yield "Interlace: JPEG" and not "Interlace: None").
#30
@
11 years ago
You are right about the mime type. I didn't check 21668.5.patch good enough for that. It's a bit more complex but it can be required. The solution can be better if it means that we support more ImageMagick/Imagick versions.
I agree with nacin here that having progressive images by default would change the functionality.
#31
@
11 years ago
- Keywords needs-patch added; has-patch needs-testing removed
attachment:21668.5.patch sounds like the closest thing to what would be a desirable change here, but needs massaging. Comments from @buley and @nacin need to be considered.
#33
@
9 years ago
- Summary changed from WordPress still does not save jpeg as progressive jpeg to WordPress does not save jpeg as progressive jpeg
This ticket was mentioned in Slack in #core-images by markoheijnen. View the logs.
9 years ago
#37
follow-up:
↓ 38
@
9 years ago
Let's add this early in for 4.6. I will do some more testing on the current patch in the meanwhile.
#38
in reply to:
↑ 37
;
follow-up:
↓ 40
@
8 years ago
Replying to markoheijnen:
Let's add this early in for 4.6. I will do some more testing on the current patch in the meanwhile.
This will be very interesting and helpful feature. I didn't find this in 4.6. As WordPress 4.7 RC is already out, could someone please confirm tentatively when this would be included in core?
Thanks in advance!
#39
@
8 years ago
Currently this ticket is lacking a finalized patch, so it is not being considered for any release. If one was made, the earliest it could go in would be 4.8 as 4.7 is well beyond enhancement window.
#40
in reply to:
↑ 38
@
8 years ago
I think the final patch will not be submitted, because it's required Imagick & GD work. But on in cheap hosting Imagick can not be installed.
This is my opinion
#41
follow-up:
↓ 42
@
8 years ago
Patch needs a refresh.
@kuzvac It only requires one of the two. So if someone doesn't has Imagick, GD will be used. So we are fine.
#42
in reply to:
↑ 41
@
8 years ago
Replying to markoheijnen:
Patch needs a refresh.
@kuzvac It only requires one of the two. So if someone doesn't has Imagick, GD will be used. So we are fine.
Ok, thanks for clarifying.
#43
in reply to:
↑ 22
;
follow-up:
↓ 44
@
8 years ago
Replying to nacin:
While progressive JPEGs can be smaller, they are not always desired from an aesthetics standpoint.
Yes! FOUT anyone? Now FOUP. Flash of Unstyled Picture...
Progressive JPGs alter the aesthetic experience of a site. And while it may be something for a site owner to consider, it should be their choice on whether to utilize or not.
Similarly, lazy loaders are not baked into Core.
The psychology of how people interact with progressive jpegs can be read here:
http://www.webperformancetoday.com/2014/09/17/progressive-image-rendering-good-evil/
TL;DR: the brain has to figure out what they're loooking at when they see a blurry image, then acknowledge what the object actually is when it's finally rendered. (This is very much in contrast with the Don't Make Me Think philosophy).
I could see this potentially as an option in the Media settings, but then you're dealing with regenerating all the jpegs (and doing the same when/if switching back).
This is something a site owner should be responsible for IF they want to pursue it. And when the average Progressive JPG only saves a few percentage points, there are other areas to focus on. Lossless compression saves way more (25-60% in cases) than Progressive JPGs, without the aesthetic issues.
Progressive JPEGs have been around forever, but we have better technology that allows for a much superior user experience. You can't trade usability/experience for a small performance improvement, when there are better options now.
As a photographer, and as a internet user, I'm inclined to agree with Nacin, and not support this.
#44
in reply to:
↑ 43
@
8 years ago
Granted a better image format does exist in the form on WebP.
https://developers.google.com/speed/webp/
Replying to bahia0019:
Replying to nacin:
While progressive JPEGs can be smaller, they are not always desired from an aesthetics standpoint.
Yes! FOUT anyone? Now FOUP. Flash of Unstyled Picture...
Progressive JPGs alter the aesthetic experience of a site. And while it may be something for a site owner to consider, it should be their choice on whether to utilize or not.
Similarly, lazy loaders are not baked into Core.
The psychology of how people interact with progressive jpegs can be read here:
http://www.webperformancetoday.com/2014/09/17/progressive-image-rendering-good-evil/
TL;DR: the brain has to figure out what they're loooking at when they see a blurry image, then acknowledge what the object actually is when it's finally rendered. (This is very much in contrast with the Don't Make Me Think philosophy).
I could see this potentially as an option in the Media settings, but then you're dealing with regenerating all the jpegs (and doing the same when/if switching back).
This is something a site owner should be responsible for IF they want to pursue it. And when the average Progressive JPG only saves a few percentage points, there are other areas to focus on. Lossless compression saves way more (25-60% in cases) than Progressive JPGs, without the aesthetic issues.
Progressive JPEGs have been around forever, but we have better technology that allows for a much superior user experience. You can't trade usability/experience for a small performance improvement, when there are better options now.
As a photographer, and as a internet user, I'm inclined to agree with Nacin, and not support this.
This ticket was mentioned in Slack in #core-media by mike. View the logs.
6 years ago
#46
@
4 years ago
- Resolution set to invalid
- Status changed from assigned to closed
- Summary changed from WordPress does not save jpeg as progressive jpeg to WordPress does not respect jpeg’s progressive feature of original image
As the savings aren’t huge, and people don’t agree on the UX/aesthetics arguments—shouldn’t it be the author’s call? I.e. just generate all image sizes with the same ‘progressiveness’ as the originally uploaded image?
FWIW, on a slow connection I find it very irritating to watch a header image download in baseline form—I don’t have a clue what is to come; while with a progressive image, I have some idea of what to expect, much earlier.
(Looking forward to AV1IF!)
This ticket was mentioned in Slack in #core-media by mike. View the logs.
4 years ago
This ticket was mentioned in Slack in #core-media by joyously. View the logs.
4 years ago
#50
@
4 years ago
- Milestone changed from Future Release to 5.8
Mentioned in the recent Media Component meeting, this looks like a good ticket to investigate as part of 5.8. Adding to the milestone for visibility. @mikeschroder I'd really appreciate your thoughts on this one :)
#51
@
4 years ago
Hi @antpb!
I left a brief of a note here that I'll copy:
I haven’t researched what is currently being talked about re: interlaced images and best practices (which might change how I think about a default), but either way, think it’s a good idea to give plugins an easy way to enable it via a filter.
And certainly, if the original image was uploaded interlaced, if we can, I agree it’d be good to keep that original intent.
This ticket was mentioned in Slack in #core by lukecarbis. View the logs.
4 years ago
#53
@
4 years ago
- Milestone changed from 5.8 to Future Release
I'm moving this out of 5.8 given the rhythm of this release cycle, general priorities for the release, and the landing of webp support. It can be brought back if there are strong feelings and rapid work happening, noting that bugs-only comes into play in 2 weeks.
I'd also be curious if this might be worth declining to address and closing this ticket at this point. It's not to say that the ticket is wrong or anything like that, just that the balance of work and benefit seems like it doesn't quite work out.
#54
@
3 years ago
- Owner set to adamsilverstein
- Status changed from reopened to assigned
Revisiting this in light of https://github.com/w3c/largest-contentful-paint/issues/68
This ticket was mentioned in PR #5684 on WordPress/wordpress-develop by @adamsilverstein.
13 months ago
#55
- Keywords has-patch added; needs-patch removed
Testing:
add_filter( 'image_save_progressive', function ( $progressive, $mime_type ) { if ( 'image/jpeg' === $mime_type ) { return true; } return $progressive; }, 10, 2 );
Trac ticket: https://core.trac.wordpress.org/ticket/21668
#57
@
10 months ago
Testing:
add_filter( 'image_save_progressive', function ( $progressive, $mime_type ) { if ( 'image/jpeg' === $mime_type ) { return true; } return $progressive; }, 10, 2 );
#58
@
10 months ago
21668.5.diff includes the latest changes from the PR.
- Add a new filter to control the output of progressive images (when available)
This builds on previous patches, thanks for all the input here over the years!
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
10 months ago
#61
@
10 months ago
- Keywords needs-dev-note added
As per today's bug scrub:
The logic of the last patch looks good!
This has to be committed before tomorrow's beta 1.
Adding needs-dev-note
workflow keyword.
#62
@
10 months ago
- Keywords needs-dev-note removed
And certainly, if the original image was uploaded interlaced, if we can, I agree it’d be good to keep that original intent.
I did not implement this part yet - although it makes sense to me I wanted to limit the scope of the initial change to not changing current behavior. We can consider this in a follow up change.
#63
@
10 months ago
This has to be committed before tomorrow's beta 1.
Thanks for the review, working on this now!
@adamsilverstein commented on PR #5684:
10 months ago
#65
This ticket was mentioned in Slack in #core-performance by adamsilverstein. View the logs.
10 months ago
This ticket was mentioned in Slack in #core-media by adamsilverstein. View the logs.
10 months ago
This ticket was mentioned in Slack in #core-performance by adamsilverstein. View the logs.
10 months ago
#74
@
10 months ago
Not sure this needs a dev note, maybe sufficient to add a note in the field guide.
It would be good to document the new image_save_progressive
filter - how to use it and the fact that it is disabled by default to avoid changing the current behavior.
We are currently in the process of adding support for ImageMagick, so not sure if this is relevant or not: #6821