Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 22 months ago

#52867 closed enhancement (fixed)

Add capability to set default format for image sub-sizes.

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

Description (last modified by adamsilverstein)

Introduces a new filter that enables setting a default sub-size image output format when uploading images.

This enables testing WebP (or other image formats) as the default output format.

Follow up to #35725 where we introduced support for WebP images.

By switching to WebP as their default output format, users won't need to do anything and will benefit from faster sites because images will be 25-40% smaller, with no loss of quality. Hosts will benefit from reduced memory usage for compression, and reduced storage and bandwidth requirements for images.

By using WebP as the default image format:

  • Uploaded images are converted to smaller sub-sizes - these images will now use the WebP format if the server supports it.
    • Note both GD and Imagick support WebP lossy and lossless, however only Imagick supports animated images.

Considerations:

  • We need to research the best compression level to use for WebP. In particular, we need to pick a compression level that results in images that are very close to the images our current compression defaults produce. Some details on testing methodology, plus a link to research with a set of test images we can use in this comment.
  • Some browsers still don't support WebP, so we should consider providing a https://github.com/chase-moskal/webp-hero shim] (behind feature detection)for sites that need to support these browsers, or whether letting these sites opt out is sufficient.
  • Disabling the feature is a one line code change, still we should release a plugin non-technical users can install to disable the feature.
  • Testing, testing, testing!

Attachments (8)

52867.diff (14.3 KB) - added by adamsilverstein 3 years ago.
52867.2.diff (1.5 KB) - added by adamsilverstein 3 years ago.
52867.3.diff (1.6 KB) - added by adamsilverstein 3 years ago.
52867.4.diff (2.5 KB) - added by adamsilverstein 3 years ago.
52867.5.diff (1.6 KB) - added by adamsilverstein 3 years ago.
52867.6.diff (2.7 KB) - added by adamsilverstein 3 years ago.
52867.7.diff (2.8 KB) - added by adamsilverstein 3 years ago.
webp_images.png (178.7 KB) - added by Boniu91 3 years ago.
It didn't work for me. I used wordpress-develop. The WebP is supported in GD (1st screen), patch applied (2nd), plugin installed and configured (3rd), images are still jpg/png (4th).

Download all attachments as: .zip

Change History (60)

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


3 years ago
#1

  • Keywords has-patch has-unit-tests added

#2 @adamsilverstein
3 years ago

  • Summary changed from Use WebP as default format for image subsizes (when available). to Use WebP as default format for image sub-sizes (when available).

#3 @SergeyBiryukov
3 years ago

  • Description modified (diff)

#4 @williampatton
3 years ago

Just noting that I discovered Safari supports .webp images as of MacOS 11 Big Sur and later only. https://caniuse.com/webp

#5 @adamsilverstein
3 years ago

Just noting that I discovered Safari supports .webp images as of MacOS 11 Big Sur and later only. https://caniuse.com/webp

Good point - users on older OS X would rely on the shim. Sites or hosts who expect a higher than usual percentage of non supported browsers can opt out with a single line of code (or a plugin).

#7 @adamsilverstein
3 years ago

52867.2.diff is the (renamed) filter part only of this patch. This filters lets developers set the default output format (mime type) and this would enable creating a simple plugin users could run to enable WebP (or any other format) as their default output format. Naming suggestions welcome!

Having a plugin available would enable much wider testing of the feature.

Testing:

To save uploaded jpeg images as WebP by default, add this to your codebase:

add_filter( 'wp_image_editor_output_format', function( $formats ) {
	$formats['image/jpg'] = 'image/webp';
	return $formats;
}

#8 @adamsilverstein
3 years ago

  • Keywords dev-feedback added

@mikeschroder @spacedmonkey how do you feel about 52867.2.diff which adds a new simplified wp_image_editor_output_format filter (naming suggestions welcome!) in WP_Image_Editor::get_output_format so we can build a feature plugin that enables wider testing of the "WebP as default output format" feature.

Version 0, edited 3 years ago by adamsilverstein (next)

#9 @spacedmonkey
3 years ago

Question, what happens if an animated gif is uploaded? Can PHP convert animated gifs into animated WEBPs?

#10 @spacedmonkey
3 years ago

So once merged, this would generates image sub sizes as webps right?

I personally believe that if we do this, we need an add an option on wp-admin/options-media.php
setting page to allow users to opt out of it.

#11 @adamsilverstein
3 years ago

Question, what happens if an animated gif is uploaded? Can PHP convert animated gifs into animated WEBPs?

If the server image engine supports animated WebP GIFs can get converted to WebP. In general GD does not support animation, only Imagick.

So once merged, this would generates image sub sizes as webps right?

No, for now this only adds a filter that gives use the capability to set the default output type (given a specific input type). In any case, the specified target is only used if the system supports it.

I created a simple plugin for testing: https://github.com/adamsilverstein/wordpress-modern-images/. With the patch here applied, install this plugin, then choose your preferred output format under Settings->Media (currently only alters JPEG uploads). Try setting this to WebP and uploading a JPEG image to a post. The image sub-sizes used should all be WebP if the system supports it.

I personally believe that if we do this, we need an add an option on wp-admin/options-media.php

setting page to allow users to opt out of it.

I'm proposing a feature plugin first, so users can test this feature. I agree users need a way to opt out, although I'm not certain about the UI.

#12 @adamsilverstein
3 years ago

  • Description modified (diff)
  • Summary changed from Use WebP as default format for image sub-sizes (when available). to Add capability to set default format for image sub-sizes.

#13 @adamsilverstein
3 years ago

@spacedmonkey I updated the ticket title/description to make it clear the goal here is introduce only the capability so far, not change output. I will move the final part about making WebP the default format into another ticket.

#14 @adamsilverstein
3 years ago

  • Milestone changed from Future Release to 5.8

@spacedmonkey, @desrosj, @mikeschroder - can you give this (filter only) patch a review (PR)? I'd love to land this in 5.8 as well as it enables building a plugin with the "WebP as default output format" feature.

I can work to add some basic tests for the filter.

#15 @kirasong
3 years ago

@adamsilverstein I like the idea of adding a filter, thank you!

I'll give it a review + test.
Agreed that a test for the filter is a good idea.

Wanted to also add -- I think it's fine and perhaps helpful to continue to have a ticket (whether this one or a different one) for the proposed feature so that discussion can continue, even if it ends up becoming a feature project / plugin.

#16 follow-up: @kirasong
3 years ago

Just tested, and this looks to work pretty well, thank you!

Two notes:

  • I noticed that the default image editor on my environment, Imagick, did not support WebP. GD on the same system did support WebP, but it wasn't used because the editor is chosen on whether it supports the format being loaded. This is an existing problem, really -- but figured it was worth mentioning.
  • I wonder if we should name this filter image_editor_output_format to line up with the existing image_editor_default_mime_type and image_editor_save_pre?

#17 @spacedmonkey
3 years ago

Wondering it is would be cleaner to have a filter like this.

return apply_filters( 'image_editor_output_format', array( $filename, $new_ext, $mime_type ) );

Just filter return value of get_output_format method. Feels a lot more flexible.

#18 in reply to: ↑ 16 @kirasong
3 years ago

Replying to mikeschroder:

  • I noticed that the default image editor on my environment, Imagick, did not support WebP. GD on the same system did support WebP, but it wasn't used because the editor is chosen on whether it supports the format being loaded.

As a follow-up on this, we could probably add support for checking more than one mime-type, and include the filter here as well, which would fix the problem:
https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/media.php#L3752

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


3 years ago

#20 @adamsilverstein
3 years ago

I wonder if we should name this filter image_editor_output_format to line up with the existing image_editor_default_mime_type and image_editor_save_pre?

Good suggestion, changed.

#21 @adamsilverstein
3 years ago

I noticed that the default image editor on my environment, Imagick, did not support WebP. GD on the same system did support WebP, but it wasn't used because the editor is chosen on whether it supports the format being loaded. This is an existing problem, really -- but figured it was worth mentioning.

@mikeschroder 52867.4.diff tries to address that, but I feel like it probably deserves a different name when used in that context. Also we loose the filename from context. I'm going to start with the single filter for now, then we can work on this as a follow up issue.

#22 @adamsilverstein
3 years ago

Wondering it is would be cleaner to have a filter like this.
return apply_filters( 'image_editor_output_format', array( $filename, $new_ext, $mime_type ) );
Just filter return value of get_output_format method. Feels a lot more flexible.

@spacedmonkey This might be a little cleaner or more flexible, however we loose some (existing) functionality.

  • If the requested type is not supported by the server, fall back to the default mime type.
  • Normalize the $filename to a wp path (although this would still run before the filter).

The current approach also feels a little easier to use from a developer perspective. Leaving as is for now!

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


3 years ago

#24 @adamsilverstein
3 years ago

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

#25 @adamsilverstein
3 years ago

52867.6.diff includes a unit test that verifies the functionality of the new image_editor_output_format filter.

#26 @adamsilverstein
3 years ago

This filter should be ready and could use some testing!

### Testing instructions

  • Apply the patch from the ticket (grunt patch:52867).
  • Install/activate the modern images plugin from https://github.com/adamsilverstein/wordpress-modern-images (download zip from the green button, or clone into plugins folder).
  • Test uploading images in a post and view the front end source code, check the uploads folder to see what happens!

### Some testing scenarios and what to expect:

  • Upload a jpeg image, by default the output format will now be WebP. Check the source code and images sub-sizes generated in uploads to verify.
  • Test on a system with GD that doesn't support WebP, images should be jpegs.
  • Test with Imagick with and without WebP support expecting similar behavior.
  • If your system supports AVIF, try outputting in AVIF - turn on under Settings->Media.
Last edited 3 years ago by adamsilverstein (previous) (diff)

#27 @adamsilverstein
3 years ago

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

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


3 years ago

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


3 years ago

#30 @hellofromTonya
3 years ago

  • Keywords needs-testing has-testing-info added

The manual testing instructions are specified in comment 26.

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


3 years ago

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


3 years ago

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


3 years ago

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


3 years ago

#35 @justinahinon
3 years ago

I test the first parts of instructions in comment 26 and it works as expected.

The images output webp, as well as the subsizes image the the upload folder.

@Boniu91
3 years ago

It didn't work for me. I used wordpress-develop. The WebP is supported in GD (1st screen), patch applied (2nd), plugin installed and configured (3rd), images are still jpg/png (4th).

#36 @Boniu91
3 years ago

@adamsilverstein Do you have any idea on what could go wrong? Thanks!

#37 @antpb
3 years ago

Hey there @Boniu91 ! I'm curious, are you seeing any sub sizes being created as WebP? I see you are using what looks like the originally uploaded image. The scope of this ticket and patch will cover all of the sub sizes but not the originally uploaded file. Try and edit the block you have to use the medium or thumbnail size and see if those are WebP.

I'll wait on merging this to confirm that it's performing as expected for you as well. In my testing it has worked well with WebP enabled configurations. The only thing pending on my end is verifying that not having WebP enabled works as expected with a fallback.

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


3 years ago

#39 @adamsilverstein
3 years ago

Hi @Boniu91 thanks for testing this out. A couple of things to check:

  • If you are running core from the build branch you need to run grunt build after applying the patch.
  • Make sure you are uploading a large JPEG image, you should see the img source code contain references to a bunch of sub-size WebP images sizes as srcset entries in the image html.

WordPress never changes your original uploaded source image, and I think your image in your test was small enough that is all it used. So if you upload a small JPEG, WordPress places that small JPEG. When you upload a large image though, the re-sizing routines kick in and this is where you will see the conversion to WebP take place.

#40 @antpb
3 years ago

Tested on a non-WebP enabled version of PHP with success! 🎉

Tested on PHP 5.6.39 with WebP not available. Uploaded large jpg with the modern image plugin setting the default to WebP. With WebP not available, the fallback of jpg is used with success.

The last test case needed is AVIF support.
If your system supports AVIF, try outputting in AVIF - turn on under Settings->Media.

Getting my non-WebP configuration working took longer than expected so I'd like to commit this on Monday when I'll have more time to watch it after merge.

Pending the above report of it not working, this will be ready to go!

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


3 years ago

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


3 years ago

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


3 years ago

This ticket was mentioned in PR #1271 on WordPress/wordpress-develop by anthonyburchell.


3 years ago
#44

Trac ticket:

---
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.
This PR is to test changes in the 52867 patch to verify tests will be safe in automated testing on merge.
https://core.trac.wordpress.org/ticket/52867

#45 @antpb
3 years ago

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

In 50943:

Media: Introduces image_editor_output_format filter for setting default MIME type of sub size image output.

This change introduces the image_editor_output_format filter, which fires as sub size images are generated allowing to define a default image MIME type for those items.

Props adamsilverstein, williampatton, spacedmonkey, mikeschroder, hellofromTonya, justinahinon, Boniu91, antpb, SergeyBiryukov.
Fixes #52867.

#46 @SergeyBiryukov
3 years ago

In 50951:

Media: Some documentation and test improvements for the image_editor_output_format filter:

  • Update the filter DocBlock per the documentation standards.
  • Use a shorter variable name for consistency with the surrounding code.
  • Delete the test file before performing assertions to avoid leftovers in case the test fails.

Follow-up to [50943].

See #52867.

#47 @milana_cap
3 years ago

  • Keywords has-dev-note needs-docs added

#48 @milana_cap
3 years ago

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

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


3 years ago

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


3 years ago

#51 @johnbillion
2 years ago

In 52034:

Docs: Fix some docblock syntax errors and add a missing canonical reference.

See #53399, #52867, #38942, #53668

#52 @GaryJ
22 months ago

Disabling the feature is a one line code change, still we should release a plugin non-technical users can install to disable the feature.

Was this plugin released? What would the one-line code change be?

Note: See TracTickets for help on using tickets.