wp-cli / media-command

Imports files as attachments, regenerates thumbnails, or lists registered image sizes.
MIT License
43 stars 41 forks source link

Regenerating a single image size (re-)generates auto-scaled big images & auto-rotated images #196

Open kraftner opened 4 months ago

kraftner commented 4 months ago

Bug Report

Describe the current, buggy behavior

In WP 5.3 two new features for uploaded images got introduced:

Unfortunately for any image affected by the above feature when regenerating any thumbnail size that auto-generated image gets regenerated as well. So even if you use wp media regenerate --image_size=<image_size>.

Under most circumstances this will just recreate an already existing image and won't cause any serious harm.

But it's still not ideal for at least these reasons:

I've noticed these unexpected changes while desperately attempting to understand what happened during a completely botched media regeneration caused by #130, #136 and this very issue.

Describe how other contributors can replicate this bug

  1. upload a JPG image that is bigger than 2560 in any dimension
  2. ensure that there has been created an image with the postfix -scaled.jpg
  3. note the last modified date of this file in the filesystem
  4. regenerate just one image size for the image: wp media regenerate <attachment-id> --image_size=medium
  5. see that last modified date of the -scaled.jpg file in the filesystem has changed to the time you ran the above command

An alternative approach to test this in a visually noticeable way is to add this snippet in a plugin or themes functions.php between the initial upload and the regeneration. (It reduces the JPEG quality of generated images to "terrible")

add_filter('jpeg_quality', function(){ return 0; });

After the regeneration you should be able to notice that the intentionally regenerated "medium" size thumbnail has terrible compression artifacts, but the -scaled.jpg unexpectedly also has.

Describe what you would expect as the correct outcome

When limiting the media regeneration to a specific image size I expect any other images to be unaffected.

Let us know what environment you are running this on

WordPress 6.4.3

OS:     Linux 6.5.0-26-generic #26~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Tue Mar 12 10:22:43 UTC 2 x86_64
Shell:  
PHP binary:     /usr/local/bin/php
PHP version:    8.1.27
php.ini used:   /usr/local/etc/php/php.ini
MySQL binary:   /usr/bin/mysql
MySQL version:  mysql  Ver 15.1 Distrib 10.11.6-MariaDB, for debian-linux-gnu (x86_64) using  EditLine wrapper
SQL modes:      
WP-CLI root dir:        phar://wp-cli.phar/vendor/wp-cli/wp-cli
WP-CLI vendor dir:      phar://wp-cli.phar/vendor
WP_CLI phar path:       /var/www/html/wp-content/plugins/my-plugin
WP-CLI packages dir:    /home/wordpress/.wp-cli/packages/
WP-CLI cache dir:       /home/wordpress/.wp-cli/cache
WP-CLI global config:   
WP-CLI project config:  
WP-CLI version: 2.10.0
kraftner commented 4 months ago

The problem seems to be that when generating the attachment meta data

https://github.com/wp-cli/media-command/blob/210cccf80f4faa38c0c608768089edf7acf2b319/src/Media_Command.php#L632

this always results in the creation of sub sizes

https://github.com/WordPress/WordPress/blob/1dc3f878703ff788a5dc1b06efda6c665554dcb3/wp-admin/includes/image.php#L559-L560

Looking at the logic in wp_create_image_subsizes() it seems that this could be disabled by temporarily disabling auto-downscale and auto-rotate with something like this:

add_filter('big_image_size_threshold', '__return_false');
add_filter('wp_image_maybe_exif_rotate', '__return_false');
danielbachhuber commented 4 months ago

Thanks for the great report, @kraftner !

Feel free to submit a pull request, if you'd like. Here is some guidance on our pull request best practices.

kraftner commented 4 months ago

@danielbachhuber Due to my current workload you probably shouldn't hold your breath for that. That is why I did my best to at least document it in case someone else can beat me to it earlier.

In any case I think there needs to be some more thinking about when/how to disable it, how to handle the case that someone wants to regenerate those files etc.

kraftner commented 3 months ago

Today I had some minutes to look at this some more. This particular issue can be solved by adding this

// For auto-downscaled images
$image_size_filters['big_image_size_threshold'] = '__return_false';

// For auto-rotated images
$image_size_filters['wp_image_maybe_exif_rotate'] = '__return_false';

in add_image_size_filters.

But it made me realize that the regeneration of these images is a more general and bigger issue and also can be a problem when regenerating everything and not just one size because then one might run into #136.

So I am leaving this here, but the whole issue needs some more investigation that also considers #136 and has extensive test coverage for all edge cases so I don't deem this ready for a PR yet.