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

When generating WebPs, ensure full size is generated(-scaled for large images) #174

Closed
Tracked by #22
adamsilverstein opened this issue Feb 15, 2022 · 6 comments · Fixed by #194 or #195
Closed
Tracked by #22
Assignees
Labels
[Plugin] Modern Image Formats Issues for the Modern Image Formats plugin (formerly WebP Uploads) [Type] Bug An existing feature is broken
Milestone

Comments

@adamsilverstein
Copy link
Member

adamsilverstein commented Feb 15, 2022

Bug Description

After #152 "Full"" sized images inserted by users will still use the original jpeg image or (-scaled version for larger images) because the WebP version is not being generated. See #152 (comment).

Proposal to fix:

  • Always generate a full sized WebP version when WebP output is active
  • For larger images (over big_image_size_threshold), generate a -scaled.webp file

Steps to reproduce

  1. Enable webp-uploads
  2. Upload and insert an image, select full size in the image properties sidebar
  3. view the post and check the image source

Expected:

  • All image references are to .webp files

Actual

  • some references are to a .jpg file.

Screenshots

image
image
image

@adamsilverstein adamsilverstein added [Type] Bug An existing feature is broken [Focus] Images Needs Dev Anything that requires development (e.g. a pull request) [Plugin] Modern Image Formats Issues for the Modern Image Formats plugin (formerly WebP Uploads) Needs Discussion Anything that needs a discussion/agreement labels Feb 15, 2022
@mitogh
Copy link
Member

mitogh commented Feb 16, 2022

I agree with this behavior, however, the opposite of this behavior was introduced here:

Closing #122

Just want to make sure we are consistent with those behaviors moving forward.

@felixarntz
Copy link
Member

@mitogh #122 was opened based on the current implementation where all sub-sizes are only created in one version (e.g. WebP), while the original image is remaining in the original format (e.g. JPEG).

But now, given we're working towards keeping JPEG available for all sizes and using WebP in addition, this may still be a relevant issue, it doesn't conflict with #122.

@adamsilverstein I wonder though whether we should rephrase the purpose of this issue. It wouldn't make sense to me to create a WebP version of the -scaled image if we don't also create a WebP version for the full image if it is small enough. We should either make the decision to only create WebP versions of the sub-sizes or we should make the decision to create WebP versions of the full size as well, no matter whether that full size is a downscaled version (i.e. if the original is too large, as mentioned here) or small enough to not need a downscaled version (i.e. not a "big" image).

@adamsilverstein
Copy link
Member Author

We should either make the decision to only create WebP versions of the sub-sizes or we should make the decision to create WebP versions of the full size as well, no matter whether that full size is a downscaled version (i.e. if the original is too large, as mentioned here) or small enough to not need a downscaled version (i.e. not a "big" image).

We need the full sized web- to generate correct markup when the user selects a large image, see #152 (comment)

@adamsilverstein adamsilverstein changed the title When generating only WebP sub sizes, ensure WebP -scaled is generated and used When generating WebPs, ensure full size is generated(-scaled for large images) Feb 25, 2022
@adamsilverstein
Copy link
Member Author

I updated this ticket's title and description to more accurately describe the issue: we need to generate the "Full" sized image as a WebP in all cases (small or large image) when we are generating WebPs. For larger images this is the -scaled image, otherwise its just the original image we need a WebP version of.

@felixarntz
Copy link
Member

Marking this as Needs Dev - a PR for this can be started anytime, since the dependency of #142 is now completely implemented.

@felixarntz felixarntz removed the Needs Discussion Anything that needs a discussion/agreement label Feb 25, 2022
@mitogh
Copy link
Member

mitogh commented Feb 26, 2022

Taking over this one as I think I have a good idea of how we can implement it on top of what we already have in place. :)

@mitogh mitogh self-assigned this Feb 26, 2022
@eclarke1 eclarke1 removed the Needs Dev Anything that requires development (e.g. a pull request) label Feb 28, 2022
mitogh added a commit that referenced this issue Feb 28, 2022
If the full size image is present as part of the metadata
information, make sure the image is replaced in the content
with the new reference to the expected image.

This fixes #174
@mitogh mitogh added this to the 1.0.0-beta.2 milestone Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Plugin] Modern Image Formats Issues for the Modern Image Formats plugin (formerly WebP Uploads) [Type] Bug An existing feature is broken
Projects
None yet
4 participants