Automattic / Automattic/regenerate-thumbnails
Check for existing images is off by 1px and always regenerates correct image
- 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