Add WebP. #1137
Add WebP. #1137
Conversation
This reverts commit 1a0f931.
…th, height and type.
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
…develop-fork into ticket/35725b
…values for width, height and type
src/wp-admin/includes/image-edit.php
Outdated
| * | ||
| * @param int $quality Quality level between 0 (low) and 100 (high) of the WebP. | ||
| */ | ||
| return imagewebp( $image, null, apply_filters( 'webp_quality', 75 ) ); |
There was a problem hiding this comment.
I'm curious why the quality is 75 here, but 90 above. Also, is there any quality comparison data to show that this is a good number? When the quality level for JPEGs was adjusted in 4.5, there as some pretty extensive research that we were able to point to justifying the number that was chosen.
There was a problem hiding this comment.
Thanks for reviewing!
We discussed this a bit previously - 90 is for transforms, 75 for saves - see this and following comment - https://core.trac.wordpress.org/ticket/35725#comment:137
More research is probably warranted to further fine tune the default WebP quality (and can continue after the initial commit).
There was a problem hiding this comment.
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.
One other point of feedback that's more code-related: The jpeg_quality filter also passes additional context of edit_image. I'm thinking we should probably architect the new webp_quality filter in the same way and pass edit_image here as well.
There was a problem hiding this comment.
Added the 'edit_image' context. Odd that is the context because this is actually the 'save_image' function, but we can keep that way to match jpeg.
As for the quality, although it is shown as 90 here for jpeg, the quality for saves is actually set to 82 here ->
I will double check that the quality settings are used as I expect for jpeg and WebP.
There was a problem hiding this comment.
Turns out these sections are legacy code and only apply when the passed image isn't an instance of WP_Image_Editor. I'm actually not sure that can happen, in my testing the image is always a WP_Image_Editor and the quality used is the default: 82. @getsource - any idea how an image could get passed that isn't an instance of WP_Image_Editor? If this is purely legacy code, we could remove the WebP support parts.
If we want to use a different value for WebP quality we could add this via the wp_editor_set_quality filter which passed the mime type as context. For now, feels fine to leave at the default value since we haven't studied the best level to use yet.
Co-authored-by: Tonya Mork <hello@hellofromtonya.com>
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
src/wp-admin/includes/image-edit.php
Outdated
| * | ||
| * @param int $quality Quality level between 0 (low) and 100 (high) of the WebP. | ||
| */ | ||
| return imagewebp( $image, null, apply_filters( 'webp_quality', 75 ) ); |
There was a problem hiding this comment.
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.
One other point of feedback that's more code-related: The jpeg_quality filter also passes additional context of edit_image. I'm thinking we should probably architect the new webp_quality filter in the same way and pass edit_image here as well.
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
|
@felixarntz thanks for the review and feedback.
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. |
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
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.