Make WordPress Core

Opened 9 years ago

Closed 4 years ago

Last modified 3 years ago

#35725 closed enhancement (fixed)

Add WebP support.

Reported by: markoheijnen's profile markoheijnen Owned by: adamsilverstein's profile adamsilverstein
Milestone: 5.8 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch needs-testing has-unit-tests has-dev-note
Focuses: Cc:

Description

Would be great that at least WP_Image_Editor (if supported) could be used to transform images to the .webp format. Tickets #24718 and #27789 focus on upload which is something which could be done later.

Attachments (28)

35725.patch (1.5 KB) - added by markoheijnen 9 years ago.
35725.diff (28.1 KB) - added by blobfolio 7 years ago.
Patch with upload_mimes and image editor integration.
phpunit.zip (671.5 KB) - added by blobfolio 7 years ago.
Files for unit testing.
35725.2.diff (28.1 KB) - added by blobfolio 7 years ago.
Just a refresh :)
35725.3.diff (25.8 KB) - added by adamsilverstein 4 years ago.
35725.4.diff (25.9 KB) - added by adamsilverstein 4 years ago.
35725.5.diff (20.7 KB) - added by adamsilverstein 4 years ago.
35725.6.diff (21.1 KB) - added by adamsilverstein 4 years ago.
35725.7.diff (21.6 KB) - added by adamsilverstein 4 years ago.
35725.8.diff (22.0 KB) - added by adamsilverstein 4 years ago.
35725.9.diff (23.7 KB) - added by adamsilverstein 4 years ago.
35725.10.diff (29.4 KB) - added by adamsilverstein 4 years ago.
35725.11.diff (30.0 KB) - added by adamsilverstein 4 years ago.
35725.12.diff (21.4 KB) - added by adamsilverstein 4 years ago.
35725.13.diff (21.5 KB) - added by adamsilverstein 4 years ago.
35725.14.diff (21.7 KB) - added by adamsilverstein 4 years ago.
35725.15.diff (22.2 KB) - added by kirasong 4 years ago.
Patch refresh after [50586] and addition of WebP resize automated test. (not passing yet)
35725.16.diff (23.3 KB) - added by adamsilverstein 4 years ago.
35725.17.diff (22.9 KB) - added by adamsilverstein 4 years ago.
35725.18.diff (23.4 KB) - added by adamsilverstein 4 years ago.
35725.19.diff (25.7 KB) - added by adamsilverstein 4 years ago.
35725.20.diff (25.8 KB) - added by adamsilverstein 4 years ago.
35725.21.diff (28.0 KB) - added by adamsilverstein 4 years ago.
35725.22.diff (28.8 KB) - added by adamsilverstein 4 years ago.
35725.23.diff (28.7 KB) - added by adamsilverstein 4 years ago.
35725.24.diff (29.2 KB) - added by adamsilverstein 4 years ago.
35725.25.diff (29.1 KB) - added by adamsilverstein 4 years ago.
35725.26.diff (28.9 KB) - added by adamsilverstein 4 years ago.

Download all attachments as: .zip

Change History (227)

@markoheijnen
9 years ago

#1 @markoheijnen
9 years ago

  • Keywords has-patch added

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


9 years ago

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


9 years ago

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


7 years ago

#6 @blobfolio
7 years ago

PHP support for WebP is starting to catch up. I'd like to investigate whether or not it is feasible to expand this enhancement to include thumbnail generation support as well.

I'll dive deeper in the next week or so and update the patch accordingly, but in the meantime, here are a few references:

GD and EXIF support:

The output from gd_info() should include ['WebP Support']=>true under environments that support WebP.

Imagick can be compiled with WebP support (e.g. with libwebp-dev, etc.). This does not seem to be very common yet, but we could conditionally check the response of Imagick::queryFormats('WEBP'); a non-empty array() being Yay!, an empty one being Boo!

@blobfolio
7 years ago

Patch with upload_mimes and image editor integration.

@blobfolio
7 years ago

Files for unit testing.

#7 @blobfolio
7 years ago

  • Keywords 2nd-opinion needs-testing has-unit-tests added

The 35725.diff patch adds full WebP support to WordPress:

  • Whitelists webp files for upload;
  • Adds webp to the list of image formats for admin display, thumbnail generation, etc.;
  • Provides numerous workarounds for environments where WebP support exists in a partial state;
  • Supports VP8, VP8L, and VP8X container formats;
  • GD handling (PHP5.6+ for VP8, PHP7.0+ for the rest);
  • ImageMagick handling (works as long as the system's binary was compiled with WebP support);


This patch introduces one new function:

/**
 * Wrapper for getimagesize()
 *
 * The native PHP function might not support newer formats.
 *
 * @since 5.0
 *
 * @see {https://core.trac.wordpress.org/ticket/35725}
 *
 * @param string $file Full path to the file.
 * @param array $info Additional metadata to pull.
 * @return array|false An array containing width, height, type constant, attributes, and media type.
 */
function wp_get_image_size( $file, &$info = array() ) { ... }

All calls to the native getimagesize() function have been redirected to this wrapper to ensure that WebP dimensions are properly reported. This also fixes the inconsistent use of error suppression that existed previously.

This wrapper can later be expanded to account for other new formats, such as #42775.


A few other things to note:

  • Hosts that do not support WebP through GD or ImageMagick will still allow files to be uploaded, they just won't get intermediate sizes.
  • There was once a bug in GD that confused ARGB with RGBA, causing colors to shift. This has been fixed upstream, but some unsupported distributions (like Debian Squeeze) might still be using it.
  • ImageMagick on Arch Linux is currently causing a segfault when dumping a WebP via the getImageBlob() method. As a workaround, stream previews (when e.g. editing an image) output png instead. This doesn't affect the saved results, so seems safe to keep.


One final note:

WebP browser support (https://caniuse.com/#search=webp) is not universal.

This patch restricts its focus to backend handling, but there is opportunity for a future frontend-focused patch to provide jpg/png fallbacks. The use of the HTML5 <picture> element seems like the best approach for such work. As it happens, @desrosj is currently working on general <picture> support for WP, so if/when that lands, a frontend WebP enhancement should be easy. :)

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


7 years ago

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


7 years ago

@blobfolio
7 years ago

Just a refresh :)

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


7 years ago

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


7 years ago

#12 @micasuh
6 years ago

With Firefox 65's WebP support and Edge adopting Chromium and Blink, the end of 2019 could be close to 80% WebP support unless Webkit surprises us. Browsers that don't support WebP will be outliers.

@blobfolio's patches (which I assume can easily be updated again) and @desrosj's work on <picture> would give WP devs an excellent avenue to use WebP in production today and help reduce load times in non-trivial ways.

I think 2019 is the right time for this to move to core.

Last edited 6 years ago by micasuh (previous) (diff)

#13 @SergeyBiryukov
6 years ago

  • Milestone changed from Awaiting Review to 5.3

#14 @desrosj
6 years ago

  • Keywords needs-refresh added

Looks like 35725.2.diff is no longer applying cleanly to trunk.

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


5 years ago

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


5 years ago

#17 follow-up: @desrosj
5 years ago

  • Milestone changed from 5.3 to Future Release

This ticket was discussed briefly in the weekly media office hours this morning.

Because Safari still does not support WebP, this seems like plugin territory until there is a way to specify multiple sources for an image in core.

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


5 years ago

#19 in reply to: ↑ 17 @pmsanz
5 years ago

Replying to desrosj:

This ticket was discussed briefly in the weekly media office hours this morning.

Because Safari still does not support WebP, this seems like plugin territory until there is a way to specify multiple sources for an image in core.

I'm sorry, but 13 browsers does support it, and only 4 does not (Apple is always late). Anyway, there is a solution for displaying WebP in any browser with a piece of Js: https://webpjs.appspot.com/

It would be interesting to offer a config option in WordPress to convert uploaded files to .jpg or .webp, so user can decide. In my case, media is 80% of the site, so .webp would be great to trim disk storage.

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

#20 @themefour
5 years ago

After more than 8 months, this format is still not supported! I don't know, is this a difficult task? Or politics behind the scenes!

https://wordpress.org/support/topic/add-support-webp-format-imge/

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


4 years ago

#22 @spacedmonkey
4 years ago

As of safari 14.0, safari on iOS, iPadOS, and MacOS will support WEBP natively. See the dev notes of first beta.

These new versions of the OS will not ship until September, but we can start working on patch that could land as soon as WordPress 5.6.

#24 @atjn
4 years ago

Because Safari still does not support WebP, this seems like plugin territory until there is a way to specify multiple sources for an image in core.

@desrosj WebP is now supported in all browsers, including Safari. Time to reconsider?

#25 @audrasjb
4 years ago

#51766 was marked as a duplicate.

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


4 years ago

#27 @anonymized_18391075
4 years ago

I found a code to add a plugin which enables webp. Think that is the base, but there is much more needed to handle webp. Especially in Gutenberg there need to be block settings for it. The question for me is if it will be handled within an image block or as an own block. An own block would offer more possibilities.

<?php
/**
* Plugin Name: WEBP Enabling
* Plugin URI: 
* Description: WEBP Enabling
* Version: 1.0
* Author: Sven Esser
* Author URI: 
* Functions Included: webp_upload_mimes, webp_is_displayable
**/

//** *Enable upload for webp image files.*/
function webp_upload_mimes($existing_mimes) {
$existing_mimes['webp'] = 'image/webp';
return $existing_mimes;
}
add_filter('mime_types', 'webp_upload_mimes');

//** * Enable preview for webp image files.*/
function webp_is_displayable($result, $path) {
if ($result === false) {
$displayable_image_types = array( IMAGETYPE_WEBP );
$info = @getimagesize( $path );

if (empty($info)) {
$result = false;
} elseif (!in_array($info[2], $displayable_image_types)) {
$result = false;
} else {
$result = true;
}
}
return $result;

}
add_filter('file_is_displayable_image', 'webp_is_displayable', 10, 2);
?>
Last edited 4 years ago by dd32 (previous) (diff)

#28 @anonymized_18391075
4 years ago

Just wondering why this didn't made it into WP 5.6. Now it's a must for WP 5.7 I think.

Last edited 4 years ago by dd32 (previous) (diff)

#29 @anonymized_18391075
4 years ago

How can we make sure webp is making it into WP 5.7?

Last edited 4 years ago by dd32 (previous) (diff)

#30 @anonymized_18391075
4 years ago

We discussed in slack the way to go. It comes to the point of image sizes and what is supported:

GD vs imagick vs JS library

  • GD is supporting webp since PHP 4.3
  • Imageick isn't 100% sure since when it supports webp, but looks like since 4 years minimum
  • There is also a possibility to use a JS library

So it looks like GD should be prefered use with JS as fallback
1st GD, 2nd JS

Or is it better o go for this?
1st GD, 2nd Imagick, 3rd JS

Last edited 4 years ago by dd32 (previous) (diff)

#31 @anonymized_18391075
4 years ago

Also it is iimportant that someone takes over ownership and push it into 5.7. I can't do so.

Last edited 4 years ago by dd32 (previous) (diff)

#32 @Clorith
4 years ago

Some more research should also be put into the underlying dependencies here, as both the GD and Imagick library can be compiled without WebP support (at least in the case of GD, it needs to be explicitly told to compile with the support).

An indication of availability of such features would go a long way as an initial step before looking at possible implementations.

Other considerations brought up is availability from a user perspective, the support for WebP in the image libraries is for either converting other formats towards WebP, but also to manipulate WebP images to generate image sizes for example. But we should also consider the formats availability to end users, can they make the image format them selves?

And finally, compatibility, as noted, the latest OSX just launched (Big Sur) which brings support for WebP to the Safari browser, but other clients are still lagging behind. the ever present IE 11 is still out there in the wild, and Microsoft Edge (pre-chromium update) is also the latest Edge version available to many users due to the staggered rollout from Microsoft, both of which do not support the WebP format and a fallback would need to be implemented for sites trying to use WebP images that their visitors may not be able to access, this introduces wrapper classes which may not be compatible with existing styling of sites, and is likely going to need a theme support entry for WebP, much like there is for HTML5-variations of some components.

#33 @anonymized_18391075
4 years ago

Thanks @Clorith

Regarding "other clients are still lagging behind"...

https://caniuse.com/webp

So it was just Safari which hasn't supported it. IE is replaced by Edge and I don't think we should skip webp because of IE. IE isn't supporting many many things right now and is outdated... a bit like Netscape :-)

Last edited 4 years ago by dd32 (previous) (diff)

#34 @joemcgill
4 years ago

I would love to see this happen, but there are a few distinct challenges that we'd need to solve to move this forward.

First, we need to be clear by what we meant when we say "add WebP support". Allowing people to upload WebP images to their media library is insufficient, in my opinion, as long as there are are a number of devices/browsers that still don't support rendering WebP images. As of today (Nov 13, 2020) WebP support is sitting at ~86% based on https://caniuse.com/webp. This has gotten much better, but not enough to add it as a default format yet.

In order to truly support WebP images (and other next-generation formats, like AVIF) we need to be able to convert uploaded files to these alternate formats and create sub-sizes. We also will need to decide on a way of serving users the best available image format to people based on the capabilities of their browsers. There are a number of strategies we could pursue, including markup based solutions (i.e. <picture>) or server-side solutions that serves alternative file formats based on the accept header from a request. The latter is admittedly more difficult since WP currently serves files directly from the filesystem, rather than hitting any code where this logic could run before serving an image, so it may be a nonstarter to pursue something like this for core.

#35 @anonymized_18391075
4 years ago

AVIF is no option based on this
https://caniuse.com/avif

webp is working in Safari since yesterday. The updates will take fast and the WP 5.7 takes some month to take place.
So with this, the current stats on https://caniuse.com/webp is not relevant asny longer. We need to add webp support now! Please don't switch back the discussion to this. With in the slack chat we all agreed that we need to implement webp support now (WP 5.7)

Just enabling upload and visibility in the media library is by far not enough. As explained in #30 it is about generating different image sizes and also transform other image formats into webp.

Last edited 4 years ago by dd32 (previous) (diff)

#36 @anonymized_18391075
4 years ago

Sorry if I got it wrong. Think the difference is between this 2 points:

  1. Support of webp
  2. Using webp as default image format

Support should be a clear yes!
Default format should be a no or better there should be a switch in settings if yes or no.

On a settings page it is easy to check if the hosting provider support everything which is needed(like GD / Imagick or other thigs) and display a red / green icon. If it is green the option to switch to webp as the default format should be available. If the icon is red it means that the server doesn't support everything which is needed and therefore this option is grayed out.

Last edited 4 years ago by dd32 (previous) (diff)

#37 @anonymized_18391075
4 years ago

Short Reminder:

1) Googles Web Vitals are measuring performance and will be part of the ranking factor starting May 2021
2) webp is bosting website performance
3) WP 5.7 is sheduled for March 2021

So the timing for webp is perfect! :-)

Last edited 4 years ago by dd32 (previous) (diff)

#38 follow-up: @jorbin
4 years ago

Please don't switch back the discussion to "if or not".

It's not switching it back, no decision has been made. In fact, before an if this should happen decision is made, a decision on what we want to do is needed.

There are a number of strategies we could pursue, including markup based solutions (i.e. <picture>) or server-side solutions that serves alternative file formats based on the accept header from a request. The latter is admittedly more difficult since WP currently serves files directly from the filesystem, rather than hitting any code where this logic could run before serving an image, so it may be a nonstarter to pursue something like this for core.

I think the picture based solution is much better, but sadly it still means IE is left out, so it doesn't give us much more adoption. To me, running WP on every image request is a non-starter. That's going to beat up a lot of servers.

#39 @anonymized_18391075
4 years ago

Deleted because of the respectles answer from @desrosj in comment # 52

Version 1, edited 4 years ago by anonymized_18391075 (previous) (next) (diff)

#40 @anonymized_18391075
4 years ago

So here are the key things to do at the moment:

  1. Image Manipulation

Details in #30 and https://github.com/WordPress/phpunit-test-runner/issues/131

  1. Detection enviorenment and switch

Details in #36

Last edited 4 years ago by dd32 (previous) (diff)

#41 in reply to: ↑ 38 @atjn
4 years ago

Replying to jorbin:

There are a number of strategies we could pursue, including markup based solutions (i.e. <picture>) or [...]

I think the picture based solution is much better, but sadly it still means IE is left out, so it doesn't give us much more adoption. To me, running WP on every image request is a non-starter. That's going to beat up a lot of servers.

@jorbin the <picture> element is specifically designed with older browsers in mind. It lets you define a fallback image that older browsers can read and understand, while newer browsers will recognize the newer markup around the fallback, and use that instead.
This allows you to support the latest fancy image types like AVIF, while falling back to still-performant WEBP for other browsers, and then falling back again to JPEG for legacy browsers, all in a few lines of simple markup. I highly recommend going with the <picture> solution, and apart from triage considerations, I see no reason that it can't be implemented and shipped immediately.

#42 follow-up: @anonymized_18391075
4 years ago

@atjn thanks for describing the <picture> element... now it is clear for me. 100% with you to use it as a fallback solution, even if it's just afew lines of simple markup.

Also 100% with you, there is no reason it can't be implementd and shipped immediately. We don't need a triage.

Last edited 4 years ago by dd32 (previous) (diff)

#43 in reply to: ↑ 42 @blobfolio
4 years ago

Replying to svenwordpress:

Also 100% with you, there is no reason it can't be implementd and shipped immediately. We don't need a triage.

I mean, there are a quarter of a billion reasons not to alter the HTML structure generated by existing WordPress image functions. Themes styling images based on e.g. p > img will break if images are suddenly shoved into <picture> elements. There will also be untold consequences for all the sites using plugins or custom workarounds to produce their own <picture>-type solutions currently. (Lots of tools, and even the Core itself, rely on dirty regex parsing, for example.)

Performance-wise, many servers already struggle to generate all the different JPG and PNG thumbnails needed for display. Auto-generating WebP conversions would double that burden on upload, and require a runtime performance hit to check for file_exists() on older media to either create the missing WebPs or avoid suggesting dead links. (Retroactive generation would be a disaster even on speedy servers; a typical archive page calls a dozen or more images, and when you factor in the srcset variants, that could easily reach triple digits.)

But that said, there's no reason WordPress can't support the WebP format in its own right, the same way it supports JPG and PNG formats. I did all the hard work for that years ago. Someone just needs to go through those patches to update them for a Gutenberg world. ;)

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


4 years ago

#45 follow-up: @anonymized_18391075
4 years ago

There are billions of reasons to close the web today! Let's stop seeing issues and start seeing possibilities!!!

If there are more to come with problems asking for stoping webp support I switch of and change from WordPress to another system!!!!

Sorry, but meanwhile I get angry!!!!!!

Last edited 4 years ago by dd32 (previous) (diff)

#46 @anonymized_18391075
4 years ago

So, here's the attended to-do list

  1. Support of webp

1.1. Upload of webp
1.2. Display in media gallery
1.3. Image Manipulation for resizing webp and transforming other image formats into webp
1.3.1. GD vs Imagick vs JS library
1.3.2. …
1.4. …

  1. Using webp as default image format

2.1. Add a new settings page which
2.1.1. Adds a check of the server environment and displays a red or green signal
2.1.2. Add a switch to decide if webp should be the default format or not. Fixed to not if the check in 3.1.1 failed

  1. Decision if handled within Gutenberg through an image block or an own block

3.1. Do we need additional block settings for webp?
3.2. ...

Last edited 4 years ago by dd32 (previous) (diff)

#47 @atjn
4 years ago

@blobfolio I see, I don't know too much about the implementation in WordPress.

#48 @anonymized_18391075
4 years ago

@atjn Implementation isn't the issue at the moment. It is more about the what to do exactly. I also don't know how to implement it.... I'm not a programmer. WHat I can do is defining the tasks and sum things up. The pure coding is the last step where I'm out... I step back in if it goes to testing.

@all Please see my list in comment # 46. Anything to add?

Last edited 4 years ago by dd32 (previous) (diff)

#49 in reply to: ↑ 45 ; follow-up: @blobfolio
4 years ago

Replying to svenwordpress:

Sorry, but meanwhile I get angry!!!!!!

Why are you angry? You can already do all of this with plugins. WordPress is used by hundreds of millions of people in hundreds of millions of different ways. Features like this are great if they can be implemented as *optional enhancements*, but there are very real consequences when decisions are made to abruptly force changes in behavior for everyone.

Replying to svenwordpress:

Using webp as default image format

It should be noted that WebP is primarily *lossy* in nature. It isn't a magic bullet that can be used to represent all images all the time. It is great sometimes, and terrible others. It should not serve as a default; the format the user actually uploaded should serve as its own default. :)

Under the hood, WebP supports only RGBA and greyscale color tables, leading to immediate distortion of images saved with different color profiles, gamma corrections, etc. Beyond that, it struggles to cleanly represent hard color breaks of the variety seen in web comics, designer logos, (artful) product photography, etc; it will add blocky artifacts around the edges of such images.

Don't get me wrong. I love WebP and use it on probably 90% of my projects. But it requires intention. The aesthetic losses aren't always worth the arbitrary pagespeed score boost. Haha.

Last edited 4 years ago by blobfolio (previous) (diff)

#51 @anonymized_18391075
4 years ago

Happy to see things to add to the to-do list

Last edited 4 years ago by dd32 (previous) (diff)

#52 @desrosj
4 years ago

@svenwordpress I truly appreciate that you are so passionate about adding support for the WebP image format to WordPress Core.

However, I want to ask that everyone please step back and consider several things.

  • Hopping into a ticket and commenting 17 times in just over 10 hours will not move anything forward, no matter how urgent the need for a fix is. In fact, it will most likely have the opposite affect, as many people will most likely just unsubscribe to updates because their email is being flooded.
  • This project is largely maintained by volunteers. Very few people are fortunate enough to be paid full-time to work on WordPress. And even the ones who are have a limited amount of time and resources they are able to contribute.
  • One of the most important elements to having a "successful" contribution experience is to have a constructive attitude. Yelling, getting angry, making passive aggressive comments, and attacking other contributors is not tolerated.
  • Everyone here wants to make WordPress better. It's perfectly acceptable (and actually preferred) for people to disagree on how to reach the same finish line. It helps ensure every aspect and potential outcome of a change is considered.
  • There many millions of WordPress sites, and many more people who use WordPress in some capacity. Before such wide-reaching changes can be made, due diligence needs to be performed to ensure the changes would not make WordPress considerably "worse" for any sets of users.
  • Changes take time, especially ones that touch so many areas of WordPress, such as this one. See bullet point 2.
  • Many of the people chiming in on this ticket have been responsible for shepherding in large changes like this in the past. Their questions come from past experiences, and no one is trying to prevent this ticket from moving forward.
  • Likewise, everyone's thoughts, questions, and ideas should be considered, regardless of experience level. I have not seen anything here that makes me think this has happened, but if someone feels dismissed or ignored, they are welcome to DM me on Slack and we can discuss. Everyone is welcome, and should feel that way.

If anyone feels that they are unable to partake in respectful conversations to help move this initiative forward, it is well documented that full WebP support can be added to a site very easily using a plugin, or various filters, and you are free to leave this ticket and continue using those methods instead. You're also free to fork WordPress and add Webp support to your own fork. Such is the beauty of open source.

Last edited 4 years ago by desrosj (previous) (diff)

#53 @anonymized_18391075
4 years ago

@desrosj I hate it when people see allways the bad thing and are blocking things. Your lines now are the reason why I'm out. Don't want to waste my time with fighting. BYE

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


4 years ago

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


4 years ago

This ticket was mentioned in Slack in #meta-tracdev by clorith. View the logs.


4 years ago

This ticket was mentioned in Slack in #meta by schlessera. View the logs.


4 years ago

#59 in reply to: ↑ 49 @atjn
4 years ago

Replying to blobfolio:

It should be noted that WebP is primarily *lossy* in nature. It isn't a magic bullet that can be used to represent all images all the time. [...]

[...] it struggles to cleanly represent hard color breaks of the variety seen in web comics, designer logos, (artful) product photography, etc; it will add blocky artifacts around the edges of such images. [...]

WebP was specifically designed to be a magic bullet that can represent any image. It has two different modes: WebP Lossy (which you are describing) and WebP Lossless which solves most of the issues you are bringing up.

I'd guess that your editor has a section somewhere that lets you choose between the two modes.

WebP definitely has certain issues, but not the ones you are mentioning here.

Last edited 4 years ago by atjn (previous) (diff)

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


4 years ago

This ticket was mentioned in PR #1024 on WordPress/wordpress-develop by adamsilverstein.


4 years ago
#61

  • Keywords needs-refresh removed

#62 @adamsilverstein
4 years ago

In 35725.3.diff I took a pass at refreshing against trunk.

#63 @adamsilverstein
4 years ago

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

#64 @blobfolio
4 years ago

Hey @adamsilverstein

Looks like there's a small typo in the diff for src/wp-admin/includes/media.php:

$allowed_extensions = array( 'jpg', 'jpeg', 'jpe', 'png', 'gif', 'png' );

("png" was added to the end instead of "webp".)

#65 follow-up: @adamsilverstein
4 years ago

Hey, great - thanks for catching that @blobfolio!

I also left off one block of code I wasn't sure about:

// Render WebP previews as PNG to workaround possible segfault issues.
if ( 'image/webp' === $mime_type ) {
		$extension = 'PNG';
		$mime_type = 'image/png';
}

Do you know if this is still required? Do you have any additional context?

#66 @adamsilverstein
4 years ago

35725.4.diff fixes a typo in 35725.3.diff as well as resolving errors reported by phpcs.

#67 in reply to: ↑ 65 @blobfolio
4 years ago

Replying to adamsilverstein:

// Render WebP previews as PNG to workaround possible segfault issues.
if ( 'image/webp' === $mime_type ) {
		$extension = 'PNG';
		$mime_type = 'image/png';
}

Do you know if this is still required? Do you have any additional context?

Most of the weird GD/Imagick workarounds in the earlier patches were added to account for bugs and shortcomings in some of the PHP 5.x modules, where WebP support was only partially or inconsistently implemented.

Since WordPress now requires PHP 7+, that PNG stream workaround is probably not needed any longer.

#68 @spacedmonkey
4 years ago

Since WordPress now requires PHP 7+, that PNG stream workaround is probably not needed any longer.

Support is still PHP 5.6+ min requirement. PHP 7.2 is recommended but not required.

#69 follow-up: @adamsilverstein
4 years ago

@blobfolio I'm digging into this a bit further and appreciate any feedback you have, leaving some notes here.

All calls to the native getimagesize() function have been redirected to this wrapper to ensure that WebP dimensions are properly reported.

I wonder if we can update wp_getimagesize instead of adding the wp_get_image_size function.

This also fixes the inconsistent use of error suppression that existed previously.

The error suppression is that way for a reason as explained in the doc block, we need to keep that (see See https://core.trac.wordpress.org/ticket/42480).

#70 @adamsilverstein
4 years ago

35725.5.diff simplifies the changes to wp_getimagesize

#71 in reply to: ↑ 69 @blobfolio
4 years ago

Replying to adamsilverstein:

I wonder if we can update wp_getimagesize instead of adding the wp_get_image_size function.

With the new debug mode condition, I think all you really need to do is strip all the @ out of your rewrite of my sizing method, give it a more appropriate name (like wp_getimagesize_debug), and update wp_getimagesize to call either wp_getimagesize_debug or @wp_getimagesize_debug instead of getimagesize/@getimagesize as it does now.

(You could do it all inline, but that would require duplicating a lot of code.)

#72 follow-up: @adamsilverstein
4 years ago

  • Summary changed from Add mime-type for Webp to Add WebP support.

Updated the ticket title slightly to include the full scope of adding WebP support.

35725.6.diff is a Work in Progress, appreciate any review & testing.

To test:

  • What works
    • Uploading WebP images to media library - UI works as expected.
    • Block editor - inserting WebP images from media library or via drag and drop works as expected.
  • What doesn't work
    • Editing a WebP image in the media library does not work as expected (I get a blank image icon).
    • The image is not resized into multiple sizes the way jpg images are.
    • As a consequence, only one image size is listed in the markup, a srcset is not provide - so uplike jpg the image will not be responsive.
  • Room for improvement
    • In addition to creating four WebP sizes when a WebP is uploaded, we should do the same when users upload a jpg, automatically substituting the WebP version for output. For the vast majority of users this will enhance site performance without sacrificing image quality. Sites/users or hosts can opt out of the default behavior with a filter if they don't want to take on the overhead of additional image compression, or can't use WebP images for some other reason.

#73 in reply to: ↑ 72 @atjn
4 years ago

Replying to adamsilverstein:

In addition to creating four WebP sizes when a WebP is uploaded, we should do the same when users upload a jpg, automatically substituting the WebP version for output.

Before you create an auto-convert module like this, please consider the following:

  • Because WebP-compression is fundamentally different from JPEG, auto-conversion is very hard without introducing weird artefacts or ending up with WebP files that are larger than the original JPEG. JPEG also has support for different image encodings and metadata that would be stripped when converted to WebP.
  • JPEG XL is likely to be supported in all browsers within this year, and it has none of these issues. The underlying compression is the same as JPEG, it has support for all of the extra features in JPEG and it has better compression than WebP.
  • JPEG XL also has strong support for lossless images, so it could essentially be implemented as the de-facto standard for images in WP that could accept any input image without compromising on quality or performance.

If it was up to me, I would only use WebP when the user or a converter plugin uploads it, and then I would invest the time I saved in building support for JPEG XL.

An early official decoder is already available: https://gitlab.com/wg1/jpeg-xl
You can follow browser implementation here: https://caniuse.com/jpegxl

#74 @adamsilverstein
4 years ago

35725.7.diff Adds resizing for WebP images.

#75 follow-ups: @adamsilverstein
4 years ago

@atjn Thanks for the feedback. I was hoping someone would bring this up so we can discuss in more detail.

Because WebP-compression is fundamentally different from JPEG, auto-conversion is very hard without introducing weird artefacts or ending up with WebP files that are larger than the original JPEG. JPEG also has support for different image encodings and metadata that would be stripped when converted to WebP.

Is this true even in the likely common scenario where someone uploads a very high quality, very large jpg image and we are downsizing to display the image at medium to large sizes? Are there cases when we can be confident the WebP images will be smaller and still high quality?

JPEG XL is likely to be supported in all browsers within this year, and it has none of these issues. The underlying compression is the same as JPEG, it has support for all of the extra features in JPEG and it has better compression than WebP.

That is a great idea, however it is out of scope for this ticket which aims to add support for WebP images. If you feel this is valuable, can you please create a new trac ticket requesting the feature?

#76 @adamsilverstein
4 years ago

  • Milestone changed from Future Release to 5.8

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


4 years ago

#78 in reply to: ↑ 75 @marylauc
4 years ago

Replying to adamsilverstein:

@atjn Thanks for the feedback. I was hoping someone would bring this up so we can discuss in more detail.

Because WebP-compression is fundamentally different from JPEG, auto-conversion is very hard without introducing weird artefacts or ending up with WebP files that are larger than the original JPEG. JPEG also has support for different image encodings and metadata that would be stripped when converted to WebP.

Is this true even in the likely common scenario where someone uploads a very high quality, very large jpg image and we are downsizing to display the image at medium to large sizes? Are there cases when we can be confident the WebP images will be smaller and still high quality?

Hi, I'm from the webp team. If you are both resizing and converting to webp, there is no reason not to use webp. Webp should definitely provide a benefit over jpeg (smaller size for similar visual quality).

If you are only converting to webp without resizing, there might be some cases where webp will be worse, such as with a low quality/noisy source jpeg. However, for most practical use cases, we would still expect webp to be beneficial.

Regarding metadata, some tools like cwebp strip it by default, but it's of course possible to copy over exif/xmp/icc if you choose to (using "-metadata all" in the case of cwebp).

#79 in reply to: ↑ 75 @atjn
4 years ago

Replying to adamsilverstein:
Hmmm. As I understood it, you wanted to replace the original JPEG source with a WebP image. That was why i felt it was necessary to remind you of the issues with WebP. If your plan is to keep and use the source image for normal use, but replace smaller preview files with WebP versions, I don't see any problems.

The reason I mentioned JPEG XL, is if you replace the source with a WebP image now, and then release a new version in a year that replaces the source with JPEG XL, you will have to make a conversion between two different encoding styles twice, which is bad for retaining quality. That could be avoided if you design it with JPEG XL in mind from the start.

I agree with @marylauc, although I'd like to point out that one of the extra "features" I alluded to in JPEG is the ability to progressively decode the image. You can have an image appear instantly on screen, and slowly become higher quality as it is downloaded. This feature is not supported in WebP. It is supported in JPEG XL. There are other similar issues, which again won't be a problem for preview files, but could be very frustrating for end-users if you blindly force it on every image uploaded to WordPress.

#80 follow-up: @adamsilverstein
4 years ago

35725.8.diff Cleans things up a bit and fixes phpcs errors.

Hmmm. As I understood it, you wanted to replace the original JPEG source with a WebP image. That was why i felt it was necessary to remind you of the issues with WebP. If your plan is to keep and use the source image for normal use, but replace smaller preview files with WebP versions, I don't see any problems.

@atjn - Right, we would use this approach only for the "downsized" image formats WordPress creates when you upload an image (we would keep the original source image). I'm trying to explore what the best approach for most users is here, of course WordPress will let users opt out of any new behavior, and we will be sure to document how to do that.

The reason I mentioned JPEG XL, is if you replace the source with a WebP image now, and then release a new version in a year that replaces the source with JPEG XL, you will have to make a conversion between two different encoding styles twice, which is bad for retaining quality. That could be avoided if you design it with JPEG XL in mind from the start.

WordPress will always keep the source uploaded image, then resize the image to the sizes we need for display.

This feature is not supported in WebP. It is supported in JPEG XL. There are other similar issues,

I'm curious to hear more about any specific issues I should watch out for.

Can you open a separate ticket to consider JPEG XL support? I'm also interesting in adding support for AVIF!

Hi, I'm from the webp team. If you are both resizing and converting to webp, there is no reason not to use webp. Webp should definitely provide a benefit over jpeg (smaller size for similar visual quality).

@marylauc thank you for lending your expertise here. WordPress always saves the original uploaded image and the conversion from jpg to WebP would only occur during the downsizing of the large image to smaller sizes. Given the compression size benefits of WebP this seems like a promising approach.

One follow up WebP question I have relates to the resources (CPU/memory) required to compress WebP images compared to jpeg images. The default jpeg quality settings WordPress uses have been tuned to meet the needs of most hosts and I want to better understand the impact launching this feature would have on hosts. Do you have any data on that?

#81 in reply to: ↑ 80 @atjn
4 years ago

Replying to adamsilverstein:

Right, we would use this approach only for the "downsized" image formats [ .. ]

I'm all for it! I see no issues, only performance improvements across the board. It should also be easy to replace the WebP encoder with JPEG XL/Whatever when a new, better format is widely supported.

I'm curious to hear more about any specific issues I should watch out for.

I don't have a comprehensive list but I also know that WebP doesn't support 4:4:4 chroma subsampling, and doesn't support very large images. I don't think any of those would be problematic for generating previews.

Can you open a separate ticket to consider JPEG XL support? I'm also interesting in adding support for AVIF!

Sure, here you go: Add JPEG XL support.

I would love to see AVIF support, although I'm unsure how much benefit it would bring, and I know how resource intensive it is which could be a problem for you.

#82 follow-up: @marylauc
4 years ago

Regarding webp format limitations:

  • Indeed, webp does not support progressive decoding. However, it does support incremental decoding, which means that browsers can decode data as soon as they arrive and start showing pixels. I don't want to start a debate about progressive decoding, but I'll just point out that it's unclear if it provides a better user experience at all. Moreover, is WordPress currently encoding thumbnails using progressive jpegs? If not, the question is irrelevant.
  • Webp only supports 4:2:0 chroma subsampling. As you pointed out, this should not be an issue in most cases. For some images, especially artificial images with sharp, bright color transitions, this can create some artifacts. These images would typically be encoded losslessly anyway (using png or webp lossless). If using webp lossy, the sharp_yuv option reduces those artifacts a lot. This option is disabled by default because it's slower.
  • The maximum pixel dimensions of a WebP image is 16383 x 16383.

Regarding CPU/memory requirements, webp generally uses about 40% less memory than jpeg encoding, but is 2x to 3x more cpu intensive.

#83 in reply to: ↑ 82 @atjn
4 years ago

Replying to marylauc:

I don't want to start a debate about progressive decoding, but I'll just point out that it's unclear if it provides a better user experience at all.

And I'd like to point out that I disagree with that statement in general. But for the specific purpose of generating thumbnails in a widely supported format today, WebP is definitely the right choice, so there's no reason to start a long discussion, agreed.

#84 @spacedmonkey
4 years ago

@adamsilverstein Can you convert your patch to a github pull request? That way I can easier give you feedback.

#85 @adamsilverstein
4 years ago

Hey @spacedmonkey that would be great, really appreciate a review!

The PR is at https://github.com/WordPress/wordpress-develop/pull/1024 (they get auto-linked now, see top of this ticket).

#86 follow-up: @adamsilverstein
4 years ago

for the specific purpose of generating thumbnails in a widely supported format today, WebP is definitely the right choice, so there's no reason to start a long discussion, agreed.

Thanks for the feedback and raising the specific limitations- and for the JPEG XL ticket @atjn!

Regarding CPU/memory requirements, webp generally uses about 40% less memory than jpeg encoding, but is 2x to 3x more cpu intensive.

Very interesting @marylauc! Is the tradeoff intentional, eg. has WebP been optimized to reduce memory usage at the expense of CPU use?

I wonder how hosts would feel about this potential impact? cc: @mikeschroder.

These images would typically be encoded losslessly anyway (using png or webp lossless).

This brings up a good point, so far we have thought mostly about jpgs. In addition, when users upload a lossless format image, we should take advantage of WebP lossless.

#87 in reply to: ↑ 86 @marylauc
4 years ago

Replying to adamsilverstein:

Regarding CPU/memory requirements, webp generally uses about 40% less memory than jpeg encoding, but is 2x to 3x more cpu intensive.

Very interesting @marylauc! Is the tradeoff intentional, eg. has WebP been optimized to reduce memory usage at the expense of CPU use?

The increased CPU usage is a consequence of how webp works. Modern image codecs work by having multiple ways of encoding an area of the image, and the encoder has to try many different methods to find the best one. This is in large part how a better compression is achieved, so webp will always be slower than jpeg no matter how much we optimize it.

Regarding memory, it's the other way around, jpeg encoders are usually optimized for speed and use more memory as a trade off.

These images would typically be encoded losslessly anyway (using png or webp lossless).

This brings up a good point, so far we have thought mostly about jpgs. In addition, when users upload a lossless format image, we should take advantage of WebP lossless.

For lossless, CPU and memory usage are roughly on par with a normal png encoder, maybe a little bit higher. And webp is much faster than png optimizers, and offers a larger compression gain.

#88 @adamsilverstein
4 years ago

@marylauc Excellent, thank you for the detailed description of resource usage.

#89 @adamsilverstein
4 years ago

  • Keywords needs-dev-note added; 2nd-opinion removed

In 35725.9.diff:

  • Create a mime type mapping object that maps image formats, so far jpegs are mapped to webp, meaning when you upload a jpeg image, the downsampled copies will be created as webp.
  • Introduce a new filter image_editor_mime_mapping which controls image format mapping; returning false will prevent remapping.

In my testing, my resampled images where ~25% smaller than the jpeg versions. Other than the smaller file size, everything worked as expected. I am able to crop images, edit them in the media library and when I delete a jpeg image in the media library , the associated webp versions are also removed.

This ticket was mentioned in Slack in #hosting-community by adamsilverstein. View the logs.


4 years ago

#91 @adamsilverstein
4 years ago

In 35725.10.diff

  • Ensure existing media tests pass by removing new media type conversion for legacy tests (wiith add_filter( 'image_editor_mime_mapping', '__return_false' ); in test setup).
  • Add a test validating conversion of images to WebP.

#92 @adamsilverstein
4 years ago

  • Keywords dev-feedback added

This ticket could use more testing and feedback. It would be good to test these scenarios:

  • Various sized images.
  • Various image formats: jpg, webp, gif, png; lossy, lossless.
  • All supported PHP versions.
    • with and without libGD, Imagick configured
      • with and without webp support

Still todo:

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

This ticket was mentioned in Slack in #hosting-community by javier. View the logs.


4 years ago

This ticket was mentioned in Slack in #hosting-community by jadonn. View the logs.


4 years ago

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


4 years ago

#96 @adamsilverstein
4 years ago

Noting that we can also test replace all gif/png images in wp-admin/images with webp equivalents to see what kind of savings in our bundle size we can get.

#97 @adamsilverstein
4 years ago

35725.11.diff

  • Map gif and png images to webp as well.
  • Add a new filter for webp save image quality webp_quality.

#98 @SergeyBiryukov
4 years ago

Looking at 35725.11.diff, I think the changes to wp-includes/ID3/ should probably be left out for now, since it's an external library and not a WordPress project. We might want to suggest these changes upstream if necessary.

#99 follow-up: @spacedmonkey
4 years ago

In my testing, the Github didn't work. The image size could not generated. I believe it because of this line https://github.com/WordPress/wordpress-develop/pull/1024/files#r596403477

I am also not sure, about using generating different image sizes using webp. This is not the behavior of other file types, so jpg are not convert to png for example. I also think that forcing users to use webp is not the best idea.

I still think that numbers are not high enough to force it's use. https://www.caniuse.com/webp

Safari 13, Edge 12-17 jump out as a lack of support.

I think it is a bit different if a user uploads a webp, they have opted in. Maybe we could add an option to opt out.

Outside the existing PR is looking good and almost code ready IMO.

#100 in reply to: ↑ 99 @atjn
4 years ago

Replying to spacedmonkey:

I still think that numbers are not high enough to force it's use. https://www.caniuse.com/webp
Safari 13, Edge 12-17 jump out as a lack of support.

Edge 17 was replaced by Edge 18 more than two years ago, it no longer receives security updates and it only accounts for 0.07% of worldwide traffic. I don't see it as a problem.

Safari is a slightly bigger issue. It only received WebP support about half a year ago and approximately 6% of worldwide traffic is still coming from Safari versions that don't support WebP. That number is steadily going down, it was around 7% a month ago. Most (>80%) of that traffic is coming from browsers that could easily be upgraded to support WebP.

(All numbers are rough estimates from Statcounter)

#101 @kirasong
4 years ago

Thanks for working on this @adamsilverstein!

A few questions and comments:

  • It looks like this, right now, does not fall back to JPEG resizing on servers where WebP is not supported. Is that accurate? If so, I think it should do this before it lands.
  • What do the DSSIM differences look like between the existing JPEG resizing, both with Imagick and GD? I'm interested in making sure quality doesn't suffer compared to existing methods.
  • Are color profiles respected with this method? I.e. Will users end up with the same colors with the original JPEG as with the other sizes?
  • What does the resource use look like when profiled compared with existing resizing methods for images of the same dimensions? (this is re: your question -- I think host response depends on what the difference is)
  • I saw you mention that Responsive Images support doesn't work right now -- agreed that is important to have working as well.

Thanks again!

Last edited 4 years ago by kirasong (previous) (diff)

#102 @spacedmonkey
4 years ago

All I am saying is, I did not expect a file to be converted from one type to another when I started reviewing this ticket. Allowing WordPress to upload WEBP ( and it generate correct metadata / sub sizes ) is one task, and changing all sub-sizes to webp is another. Personally, I think these two tickets.

For what is worth, I am all for using webp for sub sizes, but I am just worried that change this bahaviour could be considered as a breaking changing. If we change this bahaviour and be bold, I would strongly recommand some easy way of changing it back to the old ( current ) bahaviour. Maybe a filter.

I did a lot of testing last night with different webp and this worked great, can't wait for this to be in.

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


4 years ago

#104 @adamsilverstein
4 years ago

Looking at 35725.11.diff​, I think the changes to wp-includes/ID3/ should probably be left out for now, since it's an external library and not a WordPress project. We might want to suggest these changes upstream if necessary.

Thanks for the feedback @SergeyBiryukov I was wondering about these changes and agree we can remove them. Since they are about supporting older PHP versions, they probably don't belong in the upstream library. Will remove.

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


4 years ago

#106 follow-up: @adamsilverstein
4 years ago

I still think that numbers are not high enough to force it's use. https://www.caniuse.com/webp

@spacedmonkey For unsupported browsers, one possibility is to use an async capabilities check then lazy load a shim if required. We may want to consider the caniuse numbers with a little salt, they may not be accurate.

#107 follow-up: @adamsilverstein
4 years ago

Thanks for the feedback @mikeschroder - replies inline:

It looks like this, right now, does not fall back to JPEG resizing on servers where WebP is not supported. Is that accurate? If so, I think it should do this before it lands.

It should for sure, that is the intended behavior. I may have the logic wrong now and haven't carefully tested this. I've been doing all my testing with PHP v8 so far, and need to test with earlier versions, imagick & gd, with and without webp support to validate the correct behavior. As you stated, in any case where the platform does not support WebP behavior should be unchanged (the original format should be used).

What do the DSSIM differences look like between the existing JPEG resizing, both with Imagick and GD?

Great question. I don't know enough about the format to answer that question, I did find this article: https://developers.google.com/speed/webp/docs/webp_study and @marylauc may be able to add some details.

Would you expect much difference between Imagick and GD, wouldn't they both rely on the same underlying code?

I'm interested in making sure quality doesn't suffer compared to existing methods.

Absolutely, this is critical! I think WebP quality is well established, let's validate that.

Are color profiles respected with this method? I.e. Will users end up with the same colors with the original JPEG as with the other sizes?

There is a fundamental difference here you rightly flag - "Webp only supports 4:2:0 chroma subsampling.". This was discussed a bit above https://core.trac.wordpress.org/ticket/35725#comment:82 and I tend to agree this should not affect the vast majority of use cases. The existing filter could be used to fine tune the behavior, for example opting out for certain image types.

What does the resource use look like when profiled compared with existing resizing methods for images of the same dimensions? (this is re: your question -- I think host response depends on what the difference is)

In my initial testing with twentytwenty which has nine total image sizes, switching to WebP reduced my files sizes by ~25%.

I saw you mention that Responsive Images support doesn't work right now -- agreed that is important to have working as well.

Right, this is working as expected now. Here is what it looks like when I upload and insert a WebP or a jpeg image:

<img loading="lazy" width="1024" height="683" 
src="https://wpdev.localhost/wp-content/uploads/2021/03/testimage-1024x683.webp" alt="" 
class="wp-image-2081" 
srcset="https://wpdev.localhost/wp-content/uploads/2021/03/testimage-1024x683.webp 1024w, 
https://wpdev.localhost/wp-content/uploads/2021/03/testimage-300x200.webp 300w, 
https://wpdev.localhost/wp-content/uploads/2021/03/testimage-768x512.webp 768w, 
https://wpdev.localhost/wp-content/uploads/2021/03/testimage-1536x1024.webp 1536w, 
https://wpdev.localhost/wp-content/uploads/2021/03/testimage-2048x1365.webp 2048w, 
https://wpdev.localhost/wp-content/uploads/2021/03/testimage-1200x800.webp 1200w, 
https://wpdev.localhost/wp-content/uploads/2021/03/testimage-1980x1320.webp 1980w" 
sizes="(max-width: 1024px) 100vw, 1024px">

#108 @adamsilverstein
4 years ago

All I am saying is, I did not expect a file to be converted from one type to another when I started reviewing this ticket. Allowing WordPress to upload WEBP ( and it generate correct metadata / sub sizes ) is one task, and changing all sub-sizes to webp is another. Personally, I think these two tickets.

@spacedmonkey very true, adding mime type support for WebP is a much smaller change that using WebP as the default format for downsampled images. As I have continued to work on this issue I have become a huge fan of the later - it will have a much bigger impact for the web and for site owners, who will benefit from faster site performance with no change required on their part.

That said, the changes that remap images from one format to another are all isolated within the save functionality (plus test changes) and could be split off if we don't feel we are ready for the bigger change.

For what is worth, I am all for using webp for sub sizes, but I am just worried that change this bahaviour could be considered as a breaking changing. If we change this bahaviour and be bold, I would strongly recommand some easy way of changing it back to the old ( current ) bahaviour. Maybe a filter.

Great! What is breaking? From a user perspective everything works as it did before, no changes or broken features.

Right, removing the behavior is as simple as add_filter( 'image_editor_mime_mapping', '__return_false' ); (we can make a mini plugin) and regenerating images should undo any already generated WebPs; we should validate that is true.

#109 in reply to: ↑ 107 @kirasong
4 years ago

Sure! Replies inline as well:

Replying to adamsilverstein:

It should for sure, that is the intended behavior. I may have the logic wrong now and haven't carefully tested this. I've been doing all my testing with PHP v8 so far, and need to test with earlier versions, imagick & gd, with and without webp support to validate the correct behavior. As you stated, in any case where the platform does not support WebP behavior should be unchanged (the original format should be used).

Okay, cool, thanks! I must have misunderstood when reading. Testing sounds good!

What do the DSSIM differences look like between the existing JPEG resizing, both with Imagick and GD?

Great question. I don't know enough about the format to answer that question, I did find this article: https://developers.google.com/speed/webp/docs/webp_study and @marylauc may be able to add some details.

Would you expect much difference between Imagick and GD, wouldn't they both rely on the same underlying code?

Good question! I don't know if they have any code in common, but they both act in different ways, and the settings used change the outcome.

This make post from when WP compression settings were changed last and the linked research talk a bit about some of the concerns, ways to test, and research re: acceptable DSSIM scores.

Given that, one way to test would be to take the same full size images (or a set of them), and use DSSIM to compare the originals to the resizes of the JPEGs generated (GD+Imagick), then the same with WebP implementations (GD+Imagick). That should give a good idea of the starting point, and how much optimization of settings are or aren't necessary to match.

Absolutely, this is critical! I think WebP quality is well established, let's validate that.

Sounds good! To be clear, I don't mean to question whether the format can generate quality images -- it's validating that the methods and resize/compression settings we're using give the same quality output as users currently get with JPEGs, if that makes sense?

Are color profiles respected with this method? I.e. Will users end up with the same colors with the original JPEG as with the other sizes?

There is a fundamental difference here you rightly flag - "Webp only supports 4:2:0 chroma subsampling.". This was discussed a bit above https://core.trac.wordpress.org/ticket/35725#comment:82 and I tend to agree this should not affect the vast majority of use cases. The existing filter could be used to fine tune the behavior, for example opting out for certain image types.

I think that might be something different, if I understand properly.
I did a search and it looks like color profiles are supported: https://developers.google.com/speed/webp/docs/riff_container

So might be a matter of checking to see whether they're passed through with the same methods we use for Imagick, or it'd need extra handling?

What does the resource use look like when profiled compared with existing resizing methods for images of the same dimensions? (this is re: your question -- I think host response depends on what the difference is)

In my initial testing with twentytwenty which has nine total image sizes, switching to WebP reduced my files sizes by ~25%.

Sorry, I should have been more specific -- I meant profiling the server side, in terms of CPU time and memory used to do the resizing to WebP, compared to current JPEG resizing.

I see this higher up (thank you @marylauc!):

Regarding CPU/memory requirements, webp generally uses about 40% less memory than jpeg encoding, but is 2x to 3x more cpu intensive."

So essentially, interested in specifically what this looks like for WordPress (both GD and Imagick):
How much memory is used/saved, and how much time it takes compared to JPEG to resize -- especially on shared hosting.

This isn't as big of a problem as it used to be now that WordPress can resume/retry generating sizes, but it would be relevant if it has a large enough time increase that it either can't generate a size in a single PHP request's time, or it adds enough time that WordPress is likely to run out of retry requests.

CPU time and memory usage is also dependent on settings, for JPEG+Imagick resizing, at least. I don't know how much it changes based on WebP settings.

Right, this is working as expected now. Here is what it looks like when I upload and insert a WebP or a jpeg image:

Great, thanks!

#110 in reply to: ↑ 106 ; follow-up: @atjn
4 years ago

Replying to adamsilverstein:

I still think that numbers are not high enough to force it's use. https://www.caniuse.com/webp

@spacedmonkey For unsupported browsers, one possibility is to use an async capabilities check then lazy load a shim if required. We may want to consider the caniuse numbers with a little salt, they may not be accurate.

Usage statistics should definitely be taken with some skepticism, but the criticism you're linking to is objectively wrong (see the comments from Statcounter). I think Jason misunderstood how the service works fundamentally. In my experience, Statcounter is pretty solid.

Having a fallback to a browser shim for a few years sounds like a great idea to me.

#111 follow-up: @adamsilverstein
4 years ago

In my experience, Statcounter is pretty solid.

Great! I missed the follow up conversation.

Having a fallback to a browser shim for a few years sounds like a great idea to me.

The shim works well in my testing, my biggest concern is making sure the impact is minimal for supporting browsers, ie. not loading the shim unless we need it.

We could also (eventually) offer the shim as a plugin site owners could install if they know their audience needs it - similar to the classic editor plugin - then support that for a limited time frame.

#112 in reply to: ↑ 110 ; follow-up: @blobfolio
4 years ago

Replying to atjn:

Replying to adamsilverstein:

I still think that numbers are not high enough to force it's use. https://www.caniuse.com/webp

@spacedmonkey For unsupported browsers, one possibility is to use an async capabilities check then lazy load a shim if required. We may want to consider the caniuse numbers with a little salt, they may not be accurate.

Usage statistics should definitely be taken with some skepticism, but the criticism you're linking to is objectively wrong (see the comments from Statcounter). I think Jason misunderstood how the service works fundamentally. In my experience, Statcounter is pretty solid.

Don't forget that iOS doesn't actually support rendering engines other than Safari's. Everyone with an old iPhone will be stuck without WebP support until they buy a new device regardless of the "browser" they use.

Global usage stats are a good place to start, but don't really mean much when it comes to predicting the kinds of traffic any individual site might receive. There are still entire industries (insurance, government, non-profit, medical, etc.) requiring Internet Explorer compatibility.

Having a fallback to a browser shim for a few years sounds like a great idea to me.

Testing for WebP compatibility (a la Modernizr) requires the ability to inject a data-uri image into the DOM. Sites with CSP restrictions prohibiting script-generated data: sources won't be able to execute the tests.

Using <picture> is a lot more performant than any runtime-based decoding fallback, but would require sites still generate thumbs in the original format (and WebP thumbs secondarily). That's the main reason the original ticket avoided conversion. The original patches were limited to adding support for WebP images in their own right, not trying to take away steam from other formats.

Whatever approach ends up being taken, it is important to allow sites to opt-out of any fallback/compatibility-testing functionality (in addition to being able to opt out of conversion).

#113 in reply to: ↑ 111 @atjn
4 years ago

Replying to adamsilverstein:

[ .. ] my biggest concern is making sure the impact is minimal for supporting browsers, ie. not loading the shim unless we need it.

Agreed. I am not very familiar with Wordpress code, but I am very comfortable with browser testing. Let me know if you want help finding the best way to test for WebP support.

We could also (eventually) offer the shim as a plugin site owners could install if they know their audience needs it - similar to the classic editor plugin - then support that for a limited time frame

I'm hoping JPEG XL and a new shim has taken over before the plugin becomes necessary.

Last edited 4 years ago by atjn (previous) (diff)

#114 in reply to: ↑ 112 ; follow-up: @atjn
4 years ago

Replying to blobfolio:

Don't forget that iOS doesn't actually support rendering engines other than Safari's. Everyone with an old iPhone will be stuck without WebP support until they buy a new device regardless of the "browser" they use.

I did account for this in comment 100. "Safari" covers all browsers installed on iOS (and Safari on MacOS). The latest iPhone that can't update is about 5 years old, and the vast majority of users have upgraded since then. That is why it almost isn't represented in the usage stats anymore.

Global usage stats are a good place to start, but don't really mean much when it comes to predicting the kinds of traffic any individual site might receive. There are still entire industries (insurance, government, non-profit, medical, etc.) requiring Internet Explorer compatibility.

True, I think a good shim will solve that issue.

Testing for WebP compatibility (a la Modernizr) requires the ability to inject a data-uri image into the DOM. Sites with CSP restrictions prohibiting script-generated data: sources won't be able to execute the tests.

Great point, although this seems like a very specific issue. Wouldn't a filter to disable WebP be good enough for this? Or everything could just be a try:catch block and then load the shim if the code fails.

Using <picture> is a lot more performant than any runtime-based decoding fallback, but would require sites still generate thumbs in the original format (and WebP thumbs secondarily).

Yeah, that probably wouldn't work. You also had a few other great reasons in comment 43.

#115 @adamsilverstein
4 years ago

Okay, cool, thanks! I must have misunderstood when reading. Testing sounds good!

Working on it!

Looking more carefully at this bit: https://github.com/WordPress/wordpress-develop/blob/679ccc35e63e62a90c768afcc16f46bd154dfb65/src/wp-includes/class-wp-image-editor.php#L319-L320 - the way the code works, it checks if the editor (could be gd or imagick) has support for the mime type after the new mapping happens. If the type is not available, it switches to the editor's default type. I can see an improvement though: we shouldn't do the remapping at all if the new type will not be supported.

#116 @adamsilverstein
4 years ago

There are still entire industries (insurance, government, non-profit, medical, etc.) requiring Internet Explorer compatibility.

Good point @blobfolio - and although we are planning to drop IE11 support in WordPress 5.8 it is important to consider these cases.

Whatever approach ends up being taken, it is important to allow sites to opt-out of any fallback/compatibility-testing functionality (in addition to being able to opt out of conversion).

Absolutely, we will provide a way to opt out of the shim as well as the mapping to a new format. In addition to code level filters I plan to provide a plugin that will disable the shim/mapping.

Global usage stats are a good place to start, but don't really mean much when it comes to predicting the kinds of traffic any individual site might receive.

Right! that is why sites can opt out - while the vast majority of sites - and users - still benefit. In the case of WebP, we are fortunate to both broad browser support and a solid shim.

#117 @adamsilverstein
4 years ago

@mikeschroder thank you for the reference to the previous work and research - this is super helpful. Also, thanks for describing a testing approach, I've never done this type of testing and I'm excited to give it a try. Also cc: @joemcgill who published that post in case he wants to weigh in on this ticket again.

It was interesting to see how we arrived at our current default of 82 for the jpeg quality setting. I am guessing that the compression level we select for WebP will be slightly different!

#118 in reply to: ↑ 114 @blobfolio
4 years ago

Replying to atjn:

Great point, although this seems like a very specific issue. Wouldn't a filter to disable WebP be good enough for this? Or everything could just be a try:catch block and then load the shim if the code fails.

As I see it, this ticket is currently covering at least three distinct features, any combination of which might be good or bad for a given site:

  1. WebP support in the Media Library (allow uploads of WebP files and generate thumbnails for them);
  2. Convert JPEG/PNG/GIF images into WebP instead of or in addition to preserving their original format;
  3. Test/polyfill WebP support in users' browsers;

The first and third parts are presumably already covered by existing hooks, like upload_mimes, but it would be beneficial to be able to disable #2 explicitly and separately from the rest.

For example, I've tried auto-converting images to WebP and other formats for hosted clients in the past, but I received enough complaints from people who noticed the 5% of cases where such images wound up worse (rather than being overjoyed by the 95% of cases where images wound up as good or better...) to make it not worth it.

But once these features roll out, particularly if they're enabled by default even on existing installs, it will only be a matter of time before clients start uploading WebP source images without realizing it, and it would be very handy to have #3 as a fallback to ensure their visitors are actually able to load those images.

People... Haha.

#119 @marylauc
4 years ago

Replying to mikeschroder:

Replying to adamsilverstein:

Would you expect much difference between Imagick and GD, wouldn't they both rely on the same underlying code?  

Good question! I don't know if they have any code in common, but they both act in different ways, and the settings used change the outcome.
This make post from when WP compression settings were changed last and the linked research talk a bit about some of the concerns, ways to test, and research re: acceptable DSSIM scores.

Both imagemagick and GD use the libwebp encoder, so they should give very similar results. At most they might be using slightly different versions of the library or use different default settings.

The default quality setting (75) is the value we usually recommend for webp, but testing on a dataset and choosing a quality value that gives the DSSIM you want sounds like a great idea.

The cwebp encoder has a -print_ssim option, from which you can compute DSSIM = (1-SSIM)/2, but note that it doesn't take into account the loss from the 420 chroma subsampling. That means it's slightly inaccurate for images with bright color transitions. Otherwise, you can use webp's get_disto tool (https://github.com/webmproject/libwebp/blob/master/extras/get_disto.c), or the the compare tool from imagemagick 7, and I'm sure there are other ones.

Replying to atjn:

Replying to blobfolio:

Testing for WebP compatibility (a la Modernizr) requires the ability to inject a data-uri image into the DOM. Sites with CSP restrictions prohibiting script-generated data: sources won't be able to execute the tests.

Great point, although this seems like a very specific issue. Wouldn't a filter to disable WebP be good enough for this? Or everything could just be a try:catch block and then load the shim if the code fails.

The recommended way to test for webp  support is to use the "accept" request header. Browsers that support webp should list "image/webp", even for the initial page request (not just when requesting images). Chrome, Firefox and Safari definitely do this.  https://developer.mozilla.org/en-US/docs/Web/HTTP/Content_negotiation/List_of_default_Accept_values says that Edge does not, but I checked Edge on macOS and it does send image/webp. I'm not sure about the windows version. So using only this method, you might in theory get some false negatives, as you rely on the browser sending "image/webp" for html requests. But if "image/webp" is indeed present, you can be sure the browser supports it and you don't need to include any polyfill.

See also https://developers.google.com/speed/webp/faq#how_can_i_detect_browser_support_for_webp if you haven't already.

#120 follow-up: @adamsilverstein
4 years ago

@marylauc thank you for your feedback!

The recommended way to test for webp support is to use the "accept" request header.

Good to know this is a reliable approach. I was mainly considering the client side solution because page level caching is commonly used in WordPress, so all users get served the same (cached) page. Client based solutions would still work in these cases, server side would not.

Both imagemagick and GD use the libwebp encoder, so they should give very similar results. At most they might be using slightly different versions of the library or use different default settings.

Good to know!

One follow up question on Imagick vs LibGD: When researching animated image support (eg. animated gifs/pngs -> WebP), I found this ticket indicating webp.animated format is only supported in Imagick, not in GD - https://github.com/libgd/libgd/issues/648. Any feedback on this or how to detect library animated support?

The default quality setting (75) is the value we usually recommend for webp, but testing on a dataset and choosing a quality value that gives the DSSIM you want sounds like a great idea.

Thanks for all the testing details. This will help us find the best values to match what we have in WordPress already although I don't want to redo testing others have already completed. I found this existing research - https://developers.google.com/speed/webp/docs/webp_study - are you aware of any others?

#121 @adamsilverstein
4 years ago

The first and third parts are presumably already covered by existing hooks, like upload_mimes, but it would be beneficial to be able to disable #2 explicitly and separately from the rest.

@blobfolio - Right! the patch as proposed adds a new filter developers can use to opt out of the remapping behavior - either entirely, or on a per image basis (the filter passes some image context info). For non developers, we will provide a tiny plugin users can install.

#122 @spacedmonkey
4 years ago

  1. WebP support in the Media Library (allow uploads of WebP files and generate thumbnails for them);
  2. Convert JPEG/PNG/GIF images into WebP instead of or in addition to preserving their original format;
  3. Test/polyfill WebP support in users' browsers;

I believe this ticket should cover 1 and 3. Coverted images to webp, could have unknown side effects. Also until we get wider testing, we will not know issues with webp processing until a wider audience using it.

Why not add, support for uploads of webp in this ticket and in the next release. Then wait until the release after, to automatically convert files to webp until the release after that. This will do a couple of things.

  1. Making the code change smaller and easy to review / revert.
  2. Give a whole release cycle for the WordPress community to use and test webp uploads and processing.
  3. Gives more time for browser updates to roll out.
  4. Makes this feature, effectly opt in for the first release ( if you don't upload a webp then you will not see this change ).

As we are changing how images are crunched, I am just worried the converting to webp will effect quality, coloring or scaling. Going slowly here, will make it is easier to spot bugs and give them before this goes wider. There is no functionality in core to regenerate images, throught there are plugins / wp cli to do. My worry is the different crunch sizes will be broken and will hard for users to spot / fix these issues.

#123 @adamsilverstein
4 years ago

I believe this ticket should cover 1 and 3.

Thanks for the feedback, I can sense the resistance here to #2 which admittedly is a much bigger change.

Why not add, support for uploads of webp in this ticket and in the next release. Then wait until the release after, to automatically convert files to webp

I am definitely open to that approach, especially with a plan to move towards WebP as the default in the next release. Although I do feel we are ready for WebP, and our users would have the biggest benefit from #2, I understand the need to move slowly and deliberately here.

We can at least consider them separately. I will split the code up and change this patch so it includes only the 1 & 3 changes, then open a follow up ticket for #2.

#124 in reply to: ↑ 120 @marylauc
4 years ago

Replying to adamsilverstein:

@marylauc thank you for your feedback!

The recommended way to test for webp support is to use the "accept" request header.

Good to know this is a reliable approach. I was mainly considering the client side solution because page level caching is commonly used in WordPress, so all users get served the same (cached) page. Client based solutions would still work in these cases, server side would not.

That's a good point. In that case you need a client side solution indeed.

#comment:112 points out that "Testing for WebP compatibility (a la Modernizr) requires the ability to inject a data-uri image into the DOM. Sites with CSP restrictions prohibiting script-generated data: sources won't be able to execute the tests". This seems true, and not only that, but a shim won't work either on such a host, as it replaces webp image with data urls. I don't know how common it is to have such a tight CSP policy.

Both imagemagick and GD use the libwebp encoder, so they should give very similar results. At most they might be using slightly different versions of the library or use different default settings.

Good to know!

One follow up question on Imagick vs LibGD: When researching animated image support (eg. animated gifs/pngs -> WebP), I found this ticket indicating webp.animated format is only supported in Imagick, not in GD - https://github.com/libgd/libgd/issues/648. Any feedback on this or how to detect library animated support?

Sorry this sounds like a PHP question more than a webp one and I'm not familiar with this at all.

The default quality setting (75) is the value we usually recommend for webp, but testing on a dataset and choosing a quality value that gives the DSSIM you want sounds like a great idea.

Thanks for all the testing details. This will help us find the best values to match what we have in WordPress already although I don't want to redo testing others have already completed. I found this existing research - https://developers.google.com/speed/webp/docs/webp_study - are you aware of any others?

This is our latest data, I'm sure there are more unofficial comparisons on the web. But this kind of study compares the visual quality (PSNR/SSIM) of compressed images for a given bitrate (or conversely compare the size for a given visual quality), which tells you which format performs better in general, but doesn't tell you which quality setting to use for a desired SSIM. cwebp does have a "-psnr" option which allows you to specify a target psnr, but it's much slower than just providing a quality setting.

I think running a small experiment on the same dataset used for the switch to quality 81 would make sense to find the right value for you.

#125 @adamsilverstein
4 years ago

This seems true, and not only that, but a shim won't work either on such a host,

Right, such hosts would need to opt out of the feature.

I think running a small experiment on the same dataset used for the switch to quality 81 would make sense to find the right value for you.

Excellent, I'll give this a try.

#126 @adamsilverstein
4 years ago

@spacedmonkey can you please review 35725.13.diff (also in GitHub) which now only includes support for uploading/using WebP images (ie # 1 above) - I reverted other changes. I will tackle parts 2 and 3 in separate follow up tickets, let's get the initial support in here to start.

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

#127 @adamsilverstein
4 years ago

Follow up ticket to use WebP as the default format for sub-sizes: https://core.trac.wordpress.org/ticket/52867 we can consider that separately there, if we feel it is ready to land in 5.8 or wait for 5.9.

#128 @adamsilverstein
4 years ago

35725.14.diff includes a few minor adjustments to ensure all tests pass. PR also updated.

This should be ready, I'm going to do some final testing and get this committed ASAP to ensure wide testing.

#129 follow-up: @kirasong
4 years ago

@adamsilverstein Hey!

I'm going through the code, and it looks good so far -- thank you!

Just to make sure I'm not missing anything -- it looks like the current patch doesn't have support for the Imagick image editor. Is that correct?

I think it'd be good to support both GD and Imagick.

I was able to get a bit of time approved to work on this; I'm happy to help with this, and other things like testing for ideal default quality.

I'm glad to see the new tests you added, thanks! I haven't done this yet, but thinking it might be helpful to go through the existing tests that exist for the other image formats and create equivalent ones for WebP.

On a quick look, found one for resizing that might be helpful in verifying the operation:
https://github.com/WordPress/wordpress-develop/blob/master/tests/phpunit/tests/image/resize.php#L23

#130 in reply to: ↑ 129 @kirasong
4 years ago

As a quick follow-up to myself (and for folks here), it's possible it's just that there weren't any changes necessary for the Imagick editor to support it.

If so, apologies. I'm going to do some testing, see if I can get the resize automated test working as well, and report back.

@kirasong
4 years ago

Patch refresh after [50586] and addition of WebP resize automated test. (not passing yet)

This ticket was mentioned in PR #1137 on WordPress/wordpress-develop by adamsilverstein.


4 years ago
#131

Trac ticket:

#132 @adamsilverstein
4 years ago

@mikeschroder

Patch refresh after [50586] and addition of WebP resize automated test. (not passing yet)

Excellent, thanks for the refreshed patch.

it's possible it's just that there weren't any changes necessary for the Imagick editor to support it.

I finally got around to testing this locally and as you suspected, no other changes are required for Imagick to work correctly. In particular, WP_Image_Editor_Imagick::supports_mime_type works as expected when passed 'image/webp' and _save also works as expected.

I tested by enabling only WP_Image_Editor_Imagick for wp_image_editors then uploaded a large WebP image. The image sub-sizes were generated and the source code correctly contained the srcset referencing the various image sizes (as WebPs).

I also did some testing using an older PHP with GD lacking WebP support. In this case I was still able to upload WebP images, while sub-size resizing failed. In source view, only the uploaded image was referenced.

#133 @adamsilverstein
4 years ago

35725.17.diff Fixes the resizing test; added a new test file and fixed naming in comparison; check out https://github.com/WordPress/wordpress-develop/pull/1137 to get the new binary files.

@mikeschroder This looks ready to me and I'd like to get this in for wider testing, have you been able to test?

#134 @adamsilverstein
4 years ago

ps. note that work on automatically creating WebP images as the default sub-size type (when supported) continues in https://core.trac.wordpress.org/ticket/52867

#135 follow-up: @spacedmonkey
4 years ago

Nit pic.

This confuses me

This line is followed by

return imagewebp( $image, null, 90 );
 return imagewebp( $image, null, apply_filters( 'webp_quality', 75 ) );

Why does one have quality of 90 and no filter and other 75 with a filter?

#136 @spacedmonkey
4 years ago

There are also 3 of these

// phpcs:ignore PHPCompatibility.Constants.NewConstants.imagetype_webpFound

We could just add these to phpcs.xml.dist file. So

<rule ref="PHPCompatibility.Constants.NewConstants.imagetype_webpFound">
   <exclude-pattern>/src/wp-admin/includes/image\.php</exclude-pattern>
   <exclude-pattern>/src/wp-includes/compat\.php</exclude-pattern>
   <exclude-pattern>/src/wp-includes/media\.php</exclude-pattern>
</rule>

#137 @adamsilverstein
4 years ago

Why does one have quality of 90 and no filter and other 75 with a filter?

In the 90 instance the user is resizing the image. In the 75, wp is saving sub-sizes.

My guess is 90 is used because after resizing a save then has to happen, so you want to avoid double compression artifacts. Note these quality settings match what is used for jpeg as well (not in the diff but nearby in the code).

We could just add these to phpcs.xml.dist file. So

Ah, great suggestion thanks for the tip - I will change that.

#138 in reply to: ↑ 135 @kirasong
4 years ago

Replying to spacedmonkey:
<snip>

Why does one have quality of 90 and no filter and other 75 with a filter?

This was confusing to me as well when I checked out the patch.

It looks like 90 is the quality for the streamed image preview while resizing (not for saved images), and only when a WP_Image_Editor object is not passed to wp_stream_image():
https://github.com/WordPress/wordpress-develop/blob/2382765afa36e10bf3c74420024ad4e85763a47c/src/wp-admin/includes/image-edit.php#L267

As far as I'm aware, this should only happen as backcompat for plugins/themes.

Last edited 4 years ago by kirasong (previous) (diff)

#139 @spacedmonkey
4 years ago

In the 90 instance the user is resizing the image. In the 75, wp is saving sub-sizes.

Just checking that these are the right settings. I know they were used for jpg, but they also valid for webp. How these files is generated is going to be different, so quality here could be changed. I will go with wiser heads here to make that judgement call, just flagging.

I would also note that this patch should be tested against plugins like S3 uploads. Anything that touches files like this, may break this integrations.

#140 @desrosj
4 years ago

I spent a good chunk of time today looking at our Docker images with @mikeschroder and improving WebP support there to ensure we're able to develop and test this properly across supported versions of PHP.

https://github.com/WordPress/wpdev-docker-images/pull/62 is the PR and is ready for testing. A quick summary:

  • ImageMagick/Imagick already supported WebP on the PHP 7.1-7.4 images.
  • ImageMagick/Imagick is currently not installed/configured on our 8.0 image as no official release currently supports PHP 8. However, GD is installed and used.
  • The PR adds full support for WebP when using GD on PHP 7.0-8.0.
  • WebP support cannot easily be added to the 7.0 image because the base image used for our image has been EOL'ed, and is stuck compiling with a version of ImageMagick that did not contain support for WebP.

If you'd like to give those images a test spin, the extra eyes would be appreciated! There is a docker-compose.override.yml sample file in the PR description you can use to easily test the images. If there's no suggestions or feedback, I'll plan on merging this before end of week, and

Last edited 4 years ago by desrosj (previous) (diff)

#141 @adamsilverstein
4 years ago

Thanks for the updates @desrosj - this will make testing easier! For the record I have been using MAMP (Pro) which supports WebP in most PHP versions (except really old ones) and lets you toggle Imagick on/off.

We could just add these to phpcs.xml.dist file. So

@spacedmonkey I tried that and got an error:
"PHP Fatal error: Uncaught Error: Interface 'PHP_CodeSniffer_Sniff' not found..."

If you can get this working, I welcome the improvement, leaving off for now though.

#143 @kirasong
4 years ago

Thanks @adamsilverstein!

I tested 35725.17.diff on a GoDaddy cPanel test site, and things seem to be working as expected with both GD and Imagick.

I noticed that when uploading the lossless test image on this ticket, perhaps as expected, the quality of the resizes was reduced significantly, as the sizes generated are lossy. I'm wondering if WordPress should detect when a lossless image is uploaded, and generate lossless resizes?

@spacedmonkey I agree that testing to choose an appropriate quality level makes sense. I'll do some initial DSSIM testing to help find a level that's reasonably close to what users are used to with JPEG uploads. I think deep testing (with a wide variety of images) would be warranted before #52867. Personal opinion, for adding WebP support by itself, I think getting close is fine for an initial commit, and we can iterate/improve from there.

I agree it'd be good to test with S3 plugins and/or others that use streams. I don't have a good setup, outside of the existing automated tests that are in core (perhaps we can make sure/change them so they are also testing with WebP?). If anyone has a real-world setup for this, testing would be very appreciated.

@desrosj Thanks so much! Was great working through that with you last night, and it makes it much easier to test+improve this.

#144 follow-up: @spacedmonkey
4 years ago

@desrosj To provide some extra context. The docker hub image for WordPress and PHP 8
https://github.com/docker-library/wordpress/pull/551. That image does not include imagick as PHP 8 is not officially supported. See https://github.com/Imagick/imagick/issues/358 . It is unknown when the official support will come back you can build manually inside the container and it does work, or so I am told.

@mikeschroder To test S3 locally I would recommend something like fake-s3 or s3mock docker images. Then it should just be a matter of spinning up a WordPress docker image, install the plugin and point it to the container. It is easier than setting up a real S3 bucket with permissions etc.

Personal opinion, for adding WebP support by itself, I think getting close is fine for an initial commit, and we can iterate/improve from there.

I also think, that adding WebP that is opt in, as in a user opts to upload the file is low risk. We are always tweak these settings later, if we get feedback. There is also a filter...

#145 in reply to: ↑ 144 @desrosj
4 years ago

Replying to spacedmonkey:

The docker hub image for WordPress and PHP 8 https://github.com/docker-library/wordpress/pull/551. That image does not include imagick as PHP 8 is not officially supported. See https://github.com/Imagick/imagick/issues/358 . It is unknown when the official support will come back you can build manually inside the container and it does work, or so I am told.

The image linked here is the Docker (company) "official" WordPress image. We maintain separate WordPress images for use within the local development environment in wordpress-develop. Those are maintained here: https://github.com/WordPress/wpdev-docker-images and published here: https://hub.docker.com/u/wordpressdevelop.

I've also read that many have had success building Imagick manually on PHP 8, but haven't had a chance to open a PR for it on our repo. If anyone is looking to tackle that, feel free to DM me on Slack and I can share some more helpful info!

Last edited 4 years ago by desrosj (previous) (diff)

#146 @adamsilverstein
4 years ago

I'm wondering if WordPress should detect when a lossless image is uploaded, and generate lossless resizes?

Good idea, I agree the expectation when uploading a lossless image is the subsizes should also be lossless.

Imagick will only use lossless if we set the quality to 100 or above then call $img->setOption('webp:lossless', 'true');, so we need to do that manually. With LibGD we need to set the quality to gdWebpLossless.

For WebP uploads, the patch already includes code that reads a WebP header and determines the file type, which we could use. For other formats, we can tell lossy/lossless by the file type.

#147 follow-up: @adamsilverstein
4 years ago

Actually, as far as I can tell, LibGD does not currently support lossless WebP images, unless I am missing something the library makes a single call to save images using the lossy function:

https://github.com/libgd/libgd/blob/1e47a89a65d49a9003d8365da4e26a7c1a32aa51/src/gd_webp.c#L220

This abandoned PR added lossless support: https://github.com/libgd/libgd/pull/327 - it calls WebPEncodeLosslessRGBA when a lossless quality is specified.

#148 in reply to: ↑ 147 @blobfolio
4 years ago

Replying to adamsilverstein:

Actually, as far as I can tell, LibGD does not currently support lossless WebP images, unless I am missing something the library makes a single call to save images using the lossy function

Yeah, GD only supported lossy compression when I wrote the original patches, and I don't think that situation has changed.

#149 @adamsilverstein
4 years ago

@blobfolio Thanks for confirming. We can work on getting that support added upstream; in the meantime I'll limit the lossless support to Imagick.

#150 @adamsilverstein
4 years ago

35725.19.diff (and also on the PR):

  • Add wp_get_webp_info which gets a webp image width, height and type by reading the file headers.
  • Add _wp_webp_is_lossy which returns true if the file is a lossy WebP
  • In src/wp-includes/class-wp-image-editor-imagick.php:set_quality if the file is a WebP file, check if it is a lossless image (!lossy) and if so, set Imagick to use lossless output.

In addition to testing this manually it would be good to add an automated test that verifies it works as expected. Note: this only works for Imagick, with GD all uploaded WebP images will be converted to lossy image subsizes.

#151 follow-up: @adamsilverstein
4 years ago

I'm wondering if WordPress should detect when a lossless image is uploaded, and generate lossless resizes?

@mikeschroder - The latest patch should do this when Imagick is present; GD currently cannot generate lossless WebP images.

#152 in reply to: ↑ 151 @kirasong
4 years ago

Replying to adamsilverstein:

@mikeschroder - The latest patch should do this when Imagick is present; GD currently cannot generate lossless WebP images.

That's awesome; thank you! I'll give that a test.

As an update, I'm still figuring out the best way to do DSSIM testing, because the tool used in the previous tests does not support WebP. It looks like this can be done with a user script + ImageMagick, but I've got to find out if the numbers generated can be compared to the previous ones, or whether that'll have to start from scratch.

If anyone has thoughts / previous testing with DSSIM and WebP, insight and recs would be appreciated!

One note on the new patch -- the WebP specific functions/sub-functions for gathering size/meta are starting to get a little confusing for me to follow. I'm wondering if it might be helpful to consolidate a little bit? I'm guessing it's this way for a reason, but figured I'd pass on the feedback.

#153 @adamsilverstein
4 years ago

This article has some methodology using SSIM, not sure how that compares to DSSIM: https://developers.google.com/speed/webp/docs/webp_study#methodology

#155 @adamsilverstein
4 years ago

35725.21.diff incorporates additional feedback from @spacedmonkey on the GitHub PR (thanks!) and adds a test for wp_get_webp_info explicitly, as well as an additional mime type check in that function so it only operates on WebP images.

I'm going to give this some final testing and get it committed! I'd like to get this in so it can get wider testing (and continue working on using WebP as the default format), we can fine tune things like the compression quality default before launch.

Note: WebP is spelled with a capital P which I kind of like :)

Hey @SergeyBiryukov if you have a chance I would appreciate a code review before I commit this!

#156 @spacedmonkey
4 years ago

Doing some more testing, it seems like image cropping doesn't work. I believe it because of this line.

Should this version of the patch support image editing like this?

#157 @adamsilverstein
4 years ago

@spacedmonkey good catch, I will update- we should enable cropping WebP images.

I've been busy with a few other things, planning to finish this up soon!

#158 @adamsilverstein
4 years ago

In 35725.22.diff:

  • Include WebP support in WP_REST_Attachments_Controller::edit_media_item.

#159 @spacedmonkey
4 years ago

@adamsilverstein

Add a couple more nitpicy feedback to the PR.

Might be worth review This Check for if file is png and Do not scale (large) PNG images. #48736

#160 @adamsilverstein
4 years ago

@spacedmonkey Excellent, thanks for the feedback. Will review.

Reviewed the large image PNG check, thanks. I don't think WebP suffers from the same issue as PNG issues in this regard, so we can leave that logic in place unchanged.

#161 @adamsilverstein
4 years ago

35725.23.diff includes updates based on feedback on the PR. Thanks to @spacedmonkey and @hellofromtonya for the additional feedback.

#162 follow-up: @spacedmonkey
4 years ago

One question, in my testing, it seems like thumbnails are not generated on animated webp. Is this an expected behaviour? What is the reasoning behind this? Can we document this well in release notes.

Note for the future, I would solve to solve generating thumbnails for animated webp. I would love to convert animated gifs to animated webp and generate thumbnails for this webp.

#163 in reply to: ↑ 162 @kirasong
4 years ago

Replying to spacedmonkey:

One question, in my testing, it seems like thumbnails are not generated on animated webp. Is this an expected behaviour? What is the reasoning behind this? Can we document this well in release notes.

Note for the future, I would solve to solve generating thumbnails for animated webp. I would love to convert animated gifs to animated webp and generate thumbnails for this webp.

I agree! I'll note that animated GIFs are currently not supported by WP for resizing either, unfortunately (See: #28474).

I'm not sure about GD support (I saw in another comment that Adam noted GD doesn't support resizing animated files) -- but with Imagick for GIFs, this is because WP grabs the first frame, and resizes that, rather than iterating through and resizing all of the frames. I don't know if WebP handles this in a different fashion that might make it easier.

Regardless, I think it'd be a nice feature to support.

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


4 years ago

#165 @adamsilverstein
4 years ago

One question, in my testing, it seems like thumbnails are not generated on animated webp. Is this an expected behaviour?

Yea, I discovered this as well in testing (see https://core.trac.wordpress.org/ticket/35725#comment:120). I agree this should be improved if possible.

Based on my understanding GD will not supported animated GIF/WebP, and likely never will because it's architecture was never built for multi-frame images. Imagick on the other hand should support resizing these images. Thanks @mikeschroder for pointing to #28474 which I had not seen.

adamsilverstein commented on PR #1137:


4 years ago
#166

@felixarntz thanks for the review and feedback.

This also seems a bit odd to me. If we are using different quality values here, why don't we do the same for JPEG? I agree more research here would be worth doing.

We do use different quality settings for JPEG as well (for saving vs. edits) so not sure what you are referring to. l will double check, but this should match the jpeg behavior.

#167 follow-up: @adamsilverstein
4 years ago

@blobfolio - @flixos90 on the PR asked about the two sections of code that extract WebP info from the file directly (https://github.com/WordPress/wordpress-develop/pull/1137/files#r617747316 & https://github.com/WordPress/wordpress-develop/pull/1137/files#r617745292). I'd like to add a doc block explaining/referencing the approach. Can you provide some more details? Otherwise I'll go looking, thought I'd ask first though.

#168 @adamsilverstein
4 years ago

In 35725.24.diff:

  • Address feedback from PR.
  • Add documentation/references for WebP file detection & parsing.

#169 in reply to: ↑ 167 @blobfolio
4 years ago

Replying to adamsilverstein:

@blobfolio - @flixos90 on the PR asked about the two sections of code that extract WebP info from the file directly (https://github.com/WordPress/wordpress-develop/pull/1137/files#r617747316 & https://github.com/WordPress/wordpress-develop/pull/1137/files#r617745292). I'd like to add a doc block explaining/referencing the approach. Can you provide some more details? Otherwise I'll go looking, thought I'd ask first though.

Hey @adamsilverstein,

This page lays out the RIFF container format specs: https://developers.google.com/speed/webp/docs/riff_container

Of particular relevance are the https://developers.google.com/speed/webp/docs/riff_container#webp_file_header (file signature and subtype differentiation) and https://developers.google.com/speed/webp/docs/riff_container#extended_file_format sections (width, height, etc.).

#170 @adamsilverstein
4 years ago

Hey @blobfolio - great, thanks for the links which are a little more detailed than what I linked in the doc block. Overall this feels ready for merge, excited to get it committed and shared for wider testing.

#171 @adamsilverstein
4 years ago

I don't know if WebP handles this in a different fashion that might make it easier.

@mikeschroder looks like the handling would have to be similar for WebP, ie splitting into frames, going thru each frame, resizing, then combining and output. Leaving that off for now since it is complicated and not part of the original scope.

I had one other question for you: the patch adds quite a bit of image handling code that only kicks in when the image object passed in not an instance of WP_Image_Editor. Do you know if this is all legacy code, or are their situations/user flows where these code paths would still be used?

ie. this check in stream_image: https://github.com/WordPress/wordpress-develop/blob/2b7cd8d4f3ae296b24089fbd53d360a4dbe30cee/src/wp-admin/includes/image-edit.php#L267

and this one in save_image: https://github.com/WordPress/wordpress-develop/blob/2b7cd8d4f3ae296b24089fbd53d360a4dbe30cee/src/wp-admin/includes/image-edit.php#L333

#172 @adamsilverstein
4 years ago

35725.25.diff improves the doc blocks.

#173 @adamsilverstein
4 years ago

In 35725.26.diff

  • remove webp_quality filter from legacy code block. image_editor_save_pre can be used to adjust the quality setting for the image save.

#174 @adamsilverstein
4 years ago

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

In 50810:

Images: enable WebP support.

Add support for uploading, editing and saving WebP images when supported by the server.

Add 'image/webp' to supported mime types. Correctly identify WebP images and sizes even when PHP doesn't support WebP. Resize uploaded WebP files (when supported) and use for front end markup.

Props markoheijne, blobfolio, Clorith, joemcgill, atjn, desrosj, spacedmonkey, marylauc, mikeschroder, hellofromtonya, flixos90.
Fixes #35725.

#175 @adamsilverstein
4 years ago

  • Version 3.5 deleted

### Testing Instructions

  • Upload a lossy WebP format image in the media library, in on the upload screen, or in the post editor.
  • Some webP test images can be downloaded from https://developers.google.com/speed/webp/gallery1.
  • Note that while most servers now support lossy WebP, they may still lack support for lossless, animated or alpha features, so those are not expected to work.
  • Test cropping the image in the media library, and in the post editor.
  • Test rotating the image in the media library.
  • Test using WebP images anywhere you might normally use a jpeg image: the header image, a widget, a featured image, etc.

In all cases, a WebP image should work just like a jpeg image works (as long as the server image engine supports the format you upload!). Note that you will need to use a web browser that supports WebP to view the images correctly (all modern browsers support lossy WebP - https://caniuse.com/webp).

  • Test with and without WebP support on server if possible
    • the server image engine (Imagick or GD) must support WebP images, otherwise images will be rejected with an error.
    • If images were somehow transferred to a system lacking native WebP support (for example if WebP support is turned off after the images are uploaded), WordPress will still determine the image mime type and dimensions correctly by examining the file headers.

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


4 years ago

#177 follow-up: @johnjamesjacoby
4 years ago

Neat! Some initial thoughts:

  • Should webp be added to the upload_filetypes multisite setting?
  • Should/can the new functions in function_exists() checks be added to compat.php to shim WebP functionality entirely when GD does not support it?
  • It seems weird to hardcode support for webp into all of the mime arrays, knowing the environment may not offer support for it. I understand fully that this gripe is out-of-scope here, but this initiative reminds me how frustrating it is as a user to have WordPress let me attempt something when it knows it will fail 100% of the time.
  • Constants in compat.php (IMAGETYPE_WEBP, IMG_WEBP) are weird. Maybe default-constants.php?
  • Should wp_get_webp_info() be public? Seems WebP specific?
  • Should _wp_get_image_size() be private? Seems generally useful.
  • wp_getimagesize() was introduced in WordPress 5.7, which competes with _wp_get_image_size()
  • Should _wp_webp_is_lossy() be WebP specific? Are other mimes' lossy'ness similarly detectable?
  • Is the @ in front of the _wp_get_image_size() usages necessary?
  • _wp_get_image_size() has some whitespace inconsistencies

#178 @SergeyBiryukov
4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for some cleanup per comment:177.

Replying to johnjamesjacoby:

  • Should _wp_get_image_size() be private? Seems generally useful.
  • wp_getimagesize() was introduced in WordPress 5.7, which competes with _wp_get_image_size()
  • Is the @ in front of the _wp_get_image_size() usages necessary?

Yeah, it seems a bit weird to have two functions with a similar name but a slightly different purpose. There is now unnecessary error suppression for _wp_get_image_size() and missing error suppression for getimagesize(), which goes against the purpose of [50146] / #49889.

I think _wp_get_image_size() can just be a part of wp_getimagesize() instead.

#179 @SergeyBiryukov
4 years ago

In 50814:

Media: Some documentation and test improvements for WebP support:

  • Document that WebP constants are only defined in PHP 7.1+.
  • Correct the $filename parameter type in wp_get_webp_info().
  • Use a consistent message when skipping tests due to the lack of WebP support.
  • Remove unnecessary else branches after markTestSkipped().
  • Replace assertEquals() with more appropriate assertions.

Follow-up to [50810].

See #35725.

#180 @SergeyBiryukov
4 years ago

In 50815:

Media: Move retrieving WebP image size information into wp_getimagesize().

Remove _wp_get_image_size().

Follow-up to [50146], [50810], [50814].

Props johnjamesjacoby.
See #35725.

#181 @adamsilverstein
4 years ago

Thanks for the follow up review and commits @johnjamesjacoby & @SergeyBiryukov, appreciate the extra attention and fine tuning.

#182 in reply to: ↑ 177 @SergeyBiryukov
4 years ago

Replying to johnjamesjacoby:

  • Should _wp_webp_is_lossy() be WebP specific? Are other mimes' lossy'ness similarly detectable?

It looks like the function is only used in a single place in core, in WP_Image_Editor_Imagick::set_quality(), so I think we can just move the condition there and probably don't need a dedicated function for that at this time.

#183 @SergeyBiryukov
4 years ago

Found an endless loop in wp_getimagesize():

  1. Use a system that does not have native WebP support in PHP and does not have the exif PHP extension.
  2. Run core tests for the image group: composer test -- --group image.
  3. Enter the endless loop :)

This happens because wp_getimagesize() now calls wp_get_image_mime(), which in turn calls wp_getimagesize() again if exif_imagetype() is not available.

#184 @SergeyBiryukov
4 years ago

In 50818:

Media: Correct an early return condition in wp_get_webp_info().

Previously, this would only have evaluated to true if wp_get_image_mime() returned false.

Follow-up to [50810], [50814], [50815].

See #35725.

#185 @SergeyBiryukov
4 years ago

In 50819:

Media: Remove _wp_webp_is_lossy() for now.

The function was only used in a single place in core.

Follow-up to [50810], [50814], [50815], [50818].

Props johnjamesjacoby.
See #35725.

#186 @SergeyBiryukov
4 years ago

In 50820:

Docs: Correct documentation for wp_get_webp_info() return results.

Follow-up to [50810], [50814], [50815], [50818], [50819].

See #35725.

#187 @spacedmonkey
4 years ago

@johnjamesjacoby webp was added to new sites. See this line. Not sure if we should add it for existing sites. We have done this before.

#188 @spacedmonkey
4 years ago

I have created a follow up ticket for allowed file exts in multisite. See #53167

#189 @SergeyBiryukov
4 years ago

In 50821:

Media: Remove an extra variable and a redundant check in WP_Image_Editor_Imagick::set_quality().

wp_get_webp_info() returns either a string or false for the type key, so we can just check for the string directly.

Follow-up to [50810], [50814], [50815], [50818-50820].

See #35725.

#190 @SergeyBiryukov
4 years ago

In 50822:

Media: Avoid an infinite loop between wp_getimagesize() and wp_get_image_mime().

As a result of the recent changes, both functions were calling each other if the exif PHP extension is not available.

The issue is now resolved by calling the getimagesize() PHP function directly, instead of the wp_getimagesize() wrapper.

Follow-up to [50146], [50810], [50814], [50815], [50818-50821].

See #35725.

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


4 years ago

#192 @desrosj
4 years ago

  • Keywords dev-feedback removed
  • Resolution set to fixed
  • Status changed from reopened to closed

Thanks for all the amazing work here, everyone!

Today is 5.8 feature freeze. I believe that all of the feedback here so far has been addressed. Refinements can continue to be made referencing this ticket, and bugs should be opened for new issues.

If there's anything I missed remaining on this overarching ticket, feel free to reopen.

#193 @adamsilverstein
3 years ago

  • Keywords has-dev-note added; needs-dev-note removed

I posted about this change along some dev-note style details: https://make.wordpress.org/core/2021/06/07/wordpress-5-8-adds-webp-support/. Marking as 'has-dev-note'.

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


3 years ago

#195 @GaryJ
3 years ago

The // WebP constants are only defined in PHP 7.1+. comment in the code suggests that both IMAGETYPE_WEBP and IMG_WEBP were defined in PHP 7.1.

According to https://www.php.net/manual/en/function.imagetypes.php and https://www.php.net/manual/en/image.constants.php, the IMG_WEBP constant was available at 7.0.10, so the current comment is misleading.

#196 @desrosj
3 years ago

In 51442:

Media: Document edge cases with the new image_editor_output_format filter.

More testing has revealed that the image_editor_output_format filter has some interesting edge cases that developers should be aware of when electing to use this filter (see #53667 and #53668).

Because this is a new filter that was intended to be used for experimenting with different ways to handle generating image sizes and has not yet been adopted in the wild, expanding the inline documentation is an acceptable temporary solution while these edge cases are explored further and addressed.

Props mikeschroder, antpb, desrosj, adamsilverstein, ianmjones.
See #5366, #53668, #35725.

#197 @desrosj
3 years ago

In 51444:

Media: Document edge cases with the new image_editor_output_format filter.

More testing has revealed that the image_editor_output_format filter has some interesting edge cases that developers should be aware of when electing to use this filter (see #53667 and #53668).

Because this is a new filter that was intended to be used for experimenting with different ways to handle generating image sizes and has not yet been adopted in the wild, expanding the inline documentation is an acceptable temporary solution while these edge cases are explored further and addressed.

Props mikeschroder, antpb, desrosj, adamsilverstein, ianmjones.
Merges [51442] to the 5.8 branch.
See #53667, #53668, #35725.

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


3 years ago

#199 @desrosj
3 years ago

@GaryJ Sorry, but looks like ticket:35725#comment:195 was missed!

I've opened #53680 to address this in 5.8.1.

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


3 years ago

#201 @desrosj
3 years ago

#35726 was marked as a duplicate.

Note: See TracTickets for help on using tickets.