Fix incorrect missing subsizes being reported during rotated image upload#4681
Fix incorrect missing subsizes being reported during rotated image upload#4681ianmjones wants to merge 2 commits intoWordPress:trunkfrom ianmjones:58611-fix-incorrect-missing-subsizes-for-rotated-images
Conversation
| // Use the originally uploaded image dimensions as full_width and full_height. | ||
| if ( ! empty( $image_meta['original_image'] ) ) { | ||
| // Use the originally uploaded image dimensions as full_width and full_height, unless rotated. | ||
| if ( ! empty( $image_meta['original_image'] ) && empty( $image_meta['image_meta']['orientation'] ) ) { |
There was a problem hiding this comment.
| if ( ! empty( $image_meta['original_image'] ) && empty( $image_meta['image_meta']['orientation'] ) ) { | |
| if ( | |
| ! empty( $image_meta['original_image'] ) && | |
| ( empty( $image_meta['image_meta']['orientation'] ) || '1' === (string) $image_meta['image_meta']['orientation'] ) | |
| ) { |
Think that the EXIF Orientation may be empty (missing) or (string) '1' which means the image is oriented properly.
There was a problem hiding this comment.
Hey @azaozz,
Sorry, I'm failing to follow why this change is needed?
To me, your suggestion negates the purpose of this PR, namely to not use the original image's dimensions when the image has been rotated. We need to use the rotated image's dimensions rather than the original image, which was stored before the rotate happened.
Doesn't your suggestion say that if the image is not rotated, or is rotated, then use the original image's dimensions?
Sorry if I misunderstood the suggestion.
There was a problem hiding this comment.
I'm failing to follow why...
Sure, let me try to explain. This code: empty( $image_meta['image_meta']['orientation'] ) in the original checks whether the image may have to be rotated, right? If the EXIF "Orientation" is missing/empty, the image will not be rotated (cannot be rotated) and is presumed to be oriented properly.
The same happens then "Orientation" is equal to 1. I.e. the EXIF "Orientation" value indicates that the image is oriented properly and should not be rotated.
So in both cases the original image dimensions can be used as it's not going to be rotated (or has already been rotated and the EXIF value has been changed).
I've checked some of the image meta on my test site and most images (that haven't been rotated) have orientation set to 1. This comes from importing the EXIF values I think.
There was a problem hiding this comment.
Hmm, ok, then I think using the orientation that WP has isn't going to help solve the issue here.
What we'd seen previously is that during upload of an image that gets rotated, orientation was set to 1, thumbnails were generated from the newly rotated "full size" image, and therefore using the non-rotated original image to determine missing thumbnails gave incorrect results.
I've done a bunch more testing this morning, and sure enough, orientation is set to 1 for both rotated and non-rotated images, but also 0 for non-rotated images.
Therefore, it doesn't look like we can use WP's orientation metadata to determine whether an image was rotated or not, it's not the indicator that we thought it was.
To properly determine whether the original image can be used to generate missing thumbnail sizes, we probably need to grab its width vs. height ratio and compare with the full size image's ration. If they're the same, then yay, we can use the original and maybe pick up a couple extra sizes, but otherwise, stick with the full size image.
As such, I'm going to close this PR as it's not the solution we were looking for.
There was a problem hiding this comment.
I've done a bunch more testing this morning, and sure enough, orientation is set to 1 for both rotated and non-rotated images, but also 0 for non-rotated images.
I see. Good call on closing this PR.
BTW 0 is an illegal value there. EXIF Orientation can only be 1 to 8, with 1 meaning the image is oriented properly and 2 to 8 meaning it needs to be flipped or rotated. Not sure where the 0 comes from.
During the upload of a rotated image, the
wp_get_missing_image_subsizes()function currently incorrectly uses the dimensions of the non-rotated original image to determine which subsizes should exist.This changeset adds a condition that the original image should only be used for setting the image dimensions if the orientation of the Media Library item has not been changed.
Trac ticket: https://core.trac.wordpress.org/ticket/58611