Automattic / Automattic/regenerate-thumbnails

Check for existing images is off by 1px and always regenerates correct image

Open
#148 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
PHP
Stars
137
Forks
54
PR merge metrics
No merged PRs in 30d

Description

## Issue description
The plugin calculates the required thumbnail sizes from the `width` and `height` in the return array of `wp_get_attachment_metadata($attachment_id)`, which returns the dimension of the `-scaled` version if exists, weheras the actual generation of the thumbnails will calculate based on `wp_getimagesize($fullsizepath)` which returns the dimension of the original file, because it is called against that one, not the scaled version.

The dimensions from these two can agree most the times, but if the original image is much larger than the `-scaled` version, the dimensions calculated by the plugin can be slightly off compared to the final dimension that is generated.

When this happens the plugin will regenerate these thumbnails again and again, despite the `Skip regenerating existing correctly sized thumbnails (faster).` option being checked, because it is checking the presence of a thumbnail based on an incorrect filename.

Actually even on the top of the screen the plugin shows the scaled image size instead of the original, which hints it works from a source information that already lost the highest precision of spect ratio info available in the original file.

## Example:
- original size: `3650x4562`
- scaled size: `2048×2560`
- target size: `2048x2048` (default)
- expected size by plugin, calculated from `2048×2560`: `1638x2048` (2048 / 2560 * 2048 = 1638.4)
- actual size of generated image, calculated from `3650x4562`: `1639x2048` (3650 / 4562 * 2048 = 1638.57957)

So whenever the plugin checks if the image `*-1638x2048.jpg` is there, it do not find it, because the actual image is called `*-1639x2048.jpg`, so it regenerates again and again.

## Solution:
`$dims = image_resize_dimensions( $fullsize_width, $fullsize_height, $thumbnail_width, $thumbnail_height, $crop );` in [class-regeneratethumbnails-regenerator.php#L405](https://github.com/Automattic/regenerate-thumbnails/blob/master/includes/class-regeneratethumbnails-regenerator.php#L405) should be called with the actual `$fullsize_width` and `$fullsize_height` as it would be returned by `wp_getimagesize($this->fullsizepath)` instead of the scaled down size got from the metadata by `$metadata = wp_get_attachment_metadata( $this->attachment->ID );` in [class-regeneratethumbnails-regenerator.php#L581](https://github.com/Automattic/regenerate-thumbnails/blob/master/includes/class-regeneratethumbnails-regenerator.php#L581).

Let me know if PR is welcome!

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in includes/class-regeneratethumbnails-regenerator.php at the image_resize_dimensions call around line 405 and the metadata lookup around line 581. Compare the dimensions from wp_getimagesize($this->fullsizepath) with the scaled metadata dimensions, then verify that the existing-thumbnail check identifies the generated filename and does not regenerate it repeatedly when the skip option is enabled.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.