Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use original image to generate all additional image format sub-sizes #207

Merged
merged 13 commits into from Mar 14, 2022

Conversation

mitogh
Copy link
Member

@mitogh mitogh commented Mar 7, 2022

Summary

Fixes #204

Relevant technical choices

When creating all the sub-sizes for additional mime types, using the original image instead of the attached image would ensure that all additional sub-sizes have the same size as the original images.

Checklist

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

For all the generated sub-sizes use the original image instead
of the attached image in order to accurate replicate the same
process that WordPress uses to create each sub-size.
@mitogh mitogh added [Type] Bug An existing feature is broken [Focus] Images Issues related to the Images focus area [Plugin] Modern Image Formats Issues for the Modern Image Formats plugin (formerly WebP Uploads) labels Mar 7, 2022
@mitogh mitogh added this to the 1.0.0-beta.2 milestone Mar 7, 2022
@mitogh mitogh self-assigned this Mar 7, 2022
@mitogh mitogh added this to Backlog in [Focus] Images via automation Mar 7, 2022
@mitogh mitogh moved this from Backlog to Review in [Focus] Images Mar 7, 2022
The removal of the `preg_replace` as is not required due we are
using the original image, and the original image does not have
a custom suffix.
Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mitogh Mostly looks good, I left a few comments.

The one critical thing we need to add here is about considering the orientation and applying the rotation as needed, similar to how WP core does.

modules/images/webp-uploads/load.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/load.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/load.php Outdated Show resolved Hide resolved
modules/images/webp-uploads/load.php Outdated Show resolved Hide resolved
@mitogh mitogh changed the base branch from trunk to feature/174-full-image-size March 9, 2022 00:50
@mitogh mitogh requested a review from felixarntz March 9, 2022 18:47
Base automatically changed from feature/174-full-image-size to trunk March 10, 2022 01:18
@bethanylang bethanylang added the Needs Review Anything that requires code review label Mar 14, 2022
Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @mitogh!

@adamsilverstein adamsilverstein self-requested a review March 14, 2022 21:30
Copy link
Member

@adamsilverstein adamsilverstein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent! Left one tiny typo correction.

Co-authored-by: Adam Silverstein <adamjs@google.com>
@felixarntz felixarntz merged commit b082f7d into trunk Mar 14, 2022
[Focus] Images automation moved this from Review to Done Mar 14, 2022
@felixarntz felixarntz changed the title Use original image to generate all subsizes Use original image to generate all additional image format sub-sizes Mar 14, 2022
@mitogh mitogh deleted the feature/204-original-images-for-webp branch March 14, 2022 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Images Issues related to the Images focus area Needs Review Anything that requires code review [Plugin] Modern Image Formats Issues for the Modern Image Formats plugin (formerly WebP Uploads) [Type] Bug An existing feature is broken
Projects
Development

Successfully merging this pull request may close these issues.

When creating additional mime types use the original image instead of attached image.
4 participants