Repository navigation
Discard WebP image if it is larger than corresponding JPEG image #372
Description
Following some brainstorming and discussion around the WebP efforts at WordCamp Europe, one of the key takeaways was that we should include the following enhancement in the implementation: WebP versions of an image size should only be kept if they are in fact smaller than the corresponding JPEG version.
This will certainly be the case most of the time, but in the special cases where it isn't, there is in fact no point in keeping the WebP version around. WebP versions for each size are created for their performance benefit, and if in any scenario the WebP file is larger than the JPEG file, this benefit is no longer present.
While this only has a minor impact on storage usage, the enhancement is also slightly related to that, as it helps not using any storage space wastefully. All other considerations that add to the necessary storage space are tied to a particular benefit (either performance or backward compatibility), but keeping larger WebP files around is simply wasteful and should be avoided.
The goal here is the following:
- Files of the primary MIME type (i.e. in our focused scenario usually JPEG) should always be kept.
- Files of additional MIME types (i.e. in our focused scenario usually WebP) should only be kept if they are in fact smaller than the corresponding file of the primary MIME type.
Here's the proposed implementation:
- Modify
webp_uploads_create_sources_property():- In both places where an additional image source file is created, its
filesizeshould be immediately compared to thefilesizeof the primary MIME type file (i.e. the one that is stored already for each size and for the "full" attachment file). - If the created file has a larger
filesize, it should be deleted again immediately (and of course not be added to thesourcesmap).
- In both places where an additional image source file is created, its
- PHPUnit tests should be added to verify this behavior.
Activity
felixarntz commentedon Jun 14, 2022
Related to #7. cc @getsource
getsource commentedon Jun 23, 2022
Thanks for opening this @felixarntz!
I think this will help solve one of the primary concerns folks have, and work around some of the issues with quality.
mukeshpanchal27 commentedon Jun 28, 2022
Thank you for reviewing PR #1.
If we check filesize logic before updating meta for the full image, it will not store the WebP mime type in the source array.
When
the_contentreplaces the images usingwebp_uploads_img_tag_update_mime_type()it will not update the thumbnail WebP image because it was checking the source image mime type here for both full and thumbnail images and it does not return the WebP mime type because we didn't update mime type source for the full size image.In this case, we should allow mime type update for the full size image, then we can remove the WebP if it is bigger than the original image for the full image.
felixarntz commentedon Jun 28, 2022
@mukeshpanchal27 Ah, good catch! I'd say in that case we need to update the logic in https://github.com/WordPress/performance/blob/trunk/modules/images/webp-uploads/load.php#L460-L465 though since we shouldn't keep a WebP image even for the full size if it's larger than the JPEG. The logic referenced there only makes sense based on the assumption that every size has all MIME types available which with this issue will change - so we have to change this bit too.
Maybe we need to instead loop through all
$target_mimesand make a replacement for the first one that exists, per image size.mukeshpanchal27 commentedon Jun 30, 2022
@felixarntz I'm working on the changes.
If we implement this feature in the plugin, we no longer need the smaller file size filter and related code. The filter
$prefer_smaller_image_file = apply_filters('webp_uploads_prefer_smaller_image_file', false);and code for full size image https://github.com/WordPress/performance/blob/trunk/modules/images/webp-uploads/load.php#L477-L485 and thumbnail size image https://github.com/WordPress/performance/blob/trunk/modules/images/webp-uploads/load.php#L533-L541 because now images have only files that have a smaller size.felixarntz commentedon Jun 30, 2022
@mukeshpanchal27 Great point! We can probably remove that filter then.
mukeshpanchal27 commentedon Jul 1, 2022
If we insert a loop through all
$target_mimesand try to replace the image, then we also don't get a proper solution. because if we try to iterate the image source with or without it, it fails in any of the below size conditions. Let me explain it in detail.For testing purposes, we have six images.
.jpeg).jpg).jpe)Case 1: When we upload these images to WP.
1.01.jpg (Some thumbnails are smaller than the original)
In this case, we don't add a full-size image source for the additional mime type because of the file size comparison code. But we have WebP images for some thumbnails.
In this case, we add a full-size image source for an additional mime type. But we don't have any WebP images for any thumbnails.
In this case, we add a full-size image source for an additional mime type. and we have WebP images for all images.
.jpeg)In this case, we add a full-size image source for an additional mime type. and we have WebP images for all images.
.jpgformat).In this case, we don't add a full-size image source for the additional mime type. But we don't generate WebP images for full size and thumbnail size because we already have
04.webpin the directory..jpe)In this case, we don't add a full-size image source for the additional mime type. But we don't generate WebP images for full size and thumbnail size because we already have
04.webpin the directory.Case 2: When we regenerate all these six images using any free Real Thumbnail Generator plugin or the Regenerate Thumbnails plugin. Here I consider #395 issue as it is similar to this.
For some of the attachments, we don't have an additional mime type in the source, so if we use to loop through all
$target_mimesfor the full image and thumbnail image, we get proper data for the first three in the frontendwebp_uploads_create_sources_property()but we don't get a valid result for the same image name for the last three images.I used the six images attached for testing. testimage.zip
For this issue, @felixarntz, can you please give your input on this?
felixarntz commentedon Jul 1, 2022
@mukeshpanchal27 I'm not sure I understand your point. Maybe I didn't phrase my previous comment in #372 (comment) precisely. Let me try to clarify:
$target_mime, we need to attempt each possible target MIME for each different size individually. In other words, first of all, https://github.com/WordPress/performance/blob/trunk/modules/images/webp-uploads/load.php#L460-L465 should be completely removed.foreachloop. The first one where it actually makes a replacement should cause the loop to terminate. In other words, https://github.com/WordPress/performance/blob/trunk/modules/images/webp-uploads/load.php#L472-L517 should be wrapped in aforeachloop through all$target_mimesand once a replacement has been successfully made, it shouldbreakout of the loop.foreachloop similar as for the full image above. In other words, https://github.com/WordPress/performance/blob/trunk/modules/images/webp-uploads/load.php#L525-L555 should also be wrapped in aforeachloop through all$target_mimesand once a replacement has been successfully made, it shouldbreakout of the MIME type loop, i.e. proceed with the next size.Hopefully this makes more sense. Let me know what you think.