Automattic / regenerate-thumbnails

WordPress plugin for regenerating thumbnails of uploaded images. Over 1 million active users and counting.
https://alex.blog/wordpress-plugins/regenerate-thumbnails/
GNU General Public License v2.0
133 stars 54 forks source link

Existing images are not skipped #136

Closed gikaragia closed 1 year ago

gikaragia commented 1 year ago

Overview

Skip regenerating existing correctly sized thumbnails (faster). option is ignored. The images are regenerated regardless its value.

How to reproduce

What I expected to happen

What happened instead

renatho commented 1 year ago

Also reported in https://wordpress.org/support/topic/option-skip-existing-doesnt-work/

Just adding here that I checked it, and it seems the filter intermediate_image_sizes_advanced is not working as expected in Regenerate Thumbnails code. The data is already empty when arriving on the method RegenerateThumbnails_Regenerator::filter_image_sizes_to_only_missing_thumbnails.

Checking a little the core code, it seems there were some recent changes around that filter. I imagine it's impacting that behavior in Regenerate Thumbnails.

ddur commented 1 year ago

Tested on plugin free site?

renatho commented 1 year ago

Tested on plugin free site?

I tested in a development environment, but it's probably happening in the latest versions of WordPress.

ddur commented 1 year ago

Tested on plugin free site?

I tested in a development environment, but it's probably happening in the latest versions of WordPress.

I mean: Filters may be intercepted by other plugins. Does your "development environment" include any other plugin except tested plugin?

renatho commented 1 year ago

I mean: Filters may be intercepted by other plugins. Does your "development environment" include any other plugin except tested plugin?

I tested in a new setup only with Regenerate Thumbnails activated.

ddur commented 1 year ago

Also reported in https://wordpress.org/support/topic/option-skip-existing-doesnt-work/

Just adding here that I checked it, and it seems the filter intermediate_image_sizes_advanced is not working as expected in Regenerate Thumbnails code. The data is already empty when arriving on the method RegenerateThumbnails_Regenerator::filter_image_sizes_to_only_missing_thumbnails.

Checking a little the core code, it seems there were some recent changes around that filter. I imagine it's impacting that behavior in Regenerate Thumbnails.

Actually, that data is always empty since WP version 5.3. Because image metadata is cleared upon entry into new WP 5.3 function wp_create_image_subsizes( and saved over existing image/thumbnails metadata.

Anyways, always regenerating existing images is not bad at all. Because some thumbnail settings may be changed, like compression quality, sharpening, color sampling and so forth.

codemonkeynorth commented 1 year ago

confirm issue with intermediate_image_sizes_advanced see here: https://github.com/Automattic/regenerate-thumbnails/issues/142#issuecomment-1642296304

MarcGuay commented 1 year ago

Bug probably created by this core commit which saves the meta data with an empty sizes array before running the intermediate_image_sizes_advanced filter.

https://github.com/WordPress/WordPress/commit/43ed67661a9398939a210de7fdf67382e7061c1b#diff-44594c5e0f87468956393bee5dcf1ea1eaf3addbaccec88b0cf91e5d42a8fe28L294

See wp-admin/includes/image.php lines 326-330 in case it's not obvious with the link.

ddur commented 1 year ago

Bug probably created by this core commit which saves the meta data with an empty sizes array before running the intermediate_image_sizes_advanced filter.

WordPress/WordPress@43ed676#diff-44594c5e0f87468956393bee5dcf1ea1eaf3addbaccec88b0cf91e5d42a8fe28L294

See wp-admin/includes/image.php lines 326-330 in case it's not obvious with the link.

@MarcGuay

That is by design, not a bug. It forced me to partially rewrite my Warp iMagick plugin at the time (Nov 4, 2019).

MarcGuay commented 1 year ago

That is by design, not a bug.

I was referring to the bug in regenerate-thumbnails.

ddur commented 1 year ago

That is by design, not a bug.

I was referring to the bug in regenerate-thumbnails.

I was referring to the Core commit code your linked at. Metadata is cleared by design. It is not a bug.

WordPress/WordPress@43ed676#diff-44594c5e0f87468956393bee5dcf1ea1eaf3addbaccec88b0cf91e5d42a8fe28L294

ddur commented 1 year ago

Because metadata is cleared, it is not easy to find out if specific thumbnail already exist or not.

I guess there is no bug in regenerate-thumbnails (or it requires substantial redesign?).

aaronfc commented 1 year ago

Hey, I have been looking into this and looks like the issue is that wp_generate_attachment_metadata resets the 'size' property in metadata to an empty array before calling the intermediate_image_sizes_advanced filter.

I have a fix here that keeps the behaviour the same as the original. But I am a bit tempted to just remove the check for metadata sizes, that I think might not be needed. I think we can discuss that in the review.