No way of preventing image_get_intermediate_size from returning cropped image

[pputzer]

I'm using image_get_intermediate_size to get a suitable small image for use in a tiled gallery. Sometimes, dimensions for these are quite tiny (< 150 pixels on one dimension). While image_get_intermediate_size normally doesn't return image sizes with variant aspect ratios, these checks are ignored for the thumbnail size (default: 150 x 150 pixels, hard cropped).

While the recent commit resulting from #34124 will allow us to override this behavior ex post via filter, this won't be an efficient solution. I therefore propose to either remove the thumbnail check completely or add a new parameter (or filter, but I think that could be overkill) to override the check.

I'm not sure why @markjaquith implemented things this way in [13145], but I assume changing them completely will break something somewhere, so the new parameter might be the better solution for backwards compatibility.

[flixos90]

use a new parameter to disable forcing the thumbnail

[flixos90]

Replying to pputzer:

I'm not sure why @markjaquith implemented things this way in [13145], but I assume changing them completely will break something somewhere, so the new parameter might be the better solution for backwards compatibility.

34384.diff​ is a take on the approach you suggested, using a new parameter to disable the behavior. Not sure about the naming conventions there, but it should basically allow to bypass the thumbnail image in the case you described.

However, there is something else in that function that struck me: The image_resize_dimensions() call is returning a non-cropped version of the original image which fits into the size we're iterating through in the array. Why is that? I feel like it should take the resized version of the image and resize it into the $size we're requesting to get.

To be more precise, in my opinion this

<?php
$maybe_cropped = image_resize_dimensions($imagedata['width'], $imagedata['height'], $data['width'], $data['height'], false );

should become

<?php
$maybe_cropped = image_resize_dimensions($data['width'], $data['height'], $size[0], $size[1], false );

and we would also need to adjust the comparison afterwards accordingly.

The problem the current version produces for the user is the following:
Let's say, for example, you upload an image of 1200x800 (with ID 2) and you have a 300x500 image size with hard crop registered. And now you call image_get_intermediate_size( 2, array( 60, 100 ) ). First of all, this would be another example of a case where we would need to disable the thumbnail workaround (the cause of the original issue). But even if we did that, we would get a wrong image back - it would probably return the medium size since it has the same aspect ratio as the original image (but that depends on the image sizes we have on the site). In this case however, this wouldn't make any sense since we actually have an image that is the aspect ratio we want.

Correct me if I'm wrong about what the function does, but if I'm not, we should probably deal with the above in a separate issue. I first wanted to make sure it's valid before opening one.

[pputzer]

I missed the Slack chat, but some comments on the issues raised by @joemcgill:

• $force_thumb enables the current behavior, so having it set to true by default ensures backward compatibility. It is probably true that a filter would be needed as well for certain situations.
• However, I think a parameter should also stay so that direct callers don't have to create filter functions for this (filters make the code much less obvious IMHO).

[joemcgill]

I took some time to refactor the internals of image_get_intermediate_size() in 34384.2.diff​ to make the logic much cleaner and address a few concerns at once.

First, this moves the test for aspect ratios into a new helper function, wp_image_matches_ratio(), which is the same logic used in wp_calculate_image_srcset(). Now, when we're evaluating available sizes against an array of dimensions, we only add a size to our $candidates array if the ratio matches. A side affect of this optimization, is that we also fix #34980 since we wouldn't have two different image sizes with different dimensions but the same aspect ratio and surface area.

If we end up with no sizes matching the requested aspect ratio, then we fall back to the thumbnail size. This still doesn't explicitly keep a divergent ratio from being returned, as requested by @pputzer, but the filter should suffice if we end up falling back to the 'thumbnail' size, which is required to maintain backwards compatibility.

Another side affect of this cleanup is that since we no longer return $data at different places in the logic tree, we are always able to return the $path and $url instead of only when a named size is passed. The downside would be for anyone who was passing $sizes as an array in order to avoid this calculation for performance reasons.

We already have decent test coverage for image_get_intermediate_size() and this adds an additional test case to make sure an array of dimensions doesn't return the thumbnail size when another size matches the same aspect ratio. If this breaks other edge cases, we may need to add additional unit tests.

[joemcgill]

34384.9.diff​ cleans up some style/typo issues in 34384.2.diff​. In the process, I noticed that I had a typo in the thumbnail fallback check which had the effect of always bypassing the fallback. However, it turns out that the only time the fallback ‘thumbnail’ gets returned is when the size array requested is smaller than the dimensions of the thumbnail size, otherwise, we return false when no images matching the requested ratio are found.

This makes me wonder if we should remove the fallback altogether and let people who are relying on this functionality (which I suspect is a very small number of people) fix it themselves using available filters. However, [attachment 34384.9.diff] keeps the current fallback behavior and adds an additional unit test to make sure we are retaining the fallback behavior.

[joemcgill]

34384.10.5.diff​ is identical to 34384.9.diff​ except I've changed the variable names in the new wp_image_matches_ratio() function to be $source_width, $source_height, $target_width, $target_height instead of $img_a_width, $img_a_height, $img_b_width, $img_b_height. I also simplified some inline docs in the same function.

[joemcgill]

In 38086:

Media: Prevent image_get_intermediate_size() from returning cropped images.

When $size is passed to image_get_intermediate_size() as an array of width
and height values and an exact image size matching those values isn't available,
the function loops through the available attachment sizes and returns the
smallest image larger than the requested dimensions with the same aspect ratio.

The aspect ratio check is skipped for the 'thumbnail' size to provide a fallback
for small sizes when no other image option is available. This resulted in a poor
selection when the size requested was smaller than the 'thumbnail' dimensions
but a larger size matching the requested ratio existed.

This refactors the internals of image_get_intermediate_size() to ensure the
'thumbnail' size is only returned as a fallback to small sizes once all other
options have been considered, and makes the control flow easier to follow.

This also introduces a new helper function, wp_image_matches_ratio() for
testing whether the aspect ratios of two sets of dimensions match. This function
is also now used in wp_calculate_image_srcset() during the selection process.

Props flixos, joemcgill.
Fixes #34384, #34980.