Skip to content

Allow developers to tweak which image formats should be used #187

Closed

Description

@felixarntz

#142 changes the default implementation of the webp-uploads module to generate WebP versions in addition to JPEG versions for all sub-sizes of an image. This was decided in #96, replacing the original implementation from #4 where only WebP versions would be generated.

It has already been established that we would still like to provide developers with the flexibility to make their own decision though: For example, if they want to only generate WebP versions, or even only JPEG versions, they should be free to do so.

This issue is about defining the developer API and integration to control that behavior.

Activity

  1. felixarntz commented on Feb 23, 2022

    @felixarntz
    MemberAuthor

    @adamsilverstein @mitogh I'd like to suggest the following here:

    • We already have a filter webp_uploads_supported_image_mime_transforms. That filter exists only to control in what additional MIME types image sub-sizes will be generated, based on the MIME type originally uploaded.
    • That filter allows us to generate JPEG and WebP sub-sizes for JPEG uploads, as well as to generate WebP and JPEG sub-sizes for WebP uploads. What it does not allow us yet is to only generate WebP sub-sizes even if a JPEG is uploaded, or only generate JPEG sub-sizes even if a WebP is uploaded (the latter is probably an edge-case, but worth considering).

    Suggested filter

    In order to keep the management of these formats simple while allowing for the additional flexibility of creating only one format at all time, I suggest we replace the above filter by something like this:

    • A new filter like webp_uploads_image_mime_transforms (in core that would become wp_image_mime_transforms).
    • The type of the value filtered would be similar to the above exiting filter, it would be a map of an image MIME type and the one or more image MIME types that other image versions can be created for.
    • However, to allow the flexibility we need it should also allow include the original MIME type itself in the list of MIME types to transform into. That sounds a bit complicated to explain, so I added some examples below to clarify what I mean.

    Examples for possible filter values

    The following would mean for every JPEG upload, sub-sizes are generated in JPEG and WebP, and for every WebP upload, sub-sizes are generated in WebP and JPEG. This is what #142 does, so it's essentially the default.

    $image_mime_transforms = array(
    	'image/jpeg' => array( 'image/jpeg', 'image/webp' ),
    	'image/webp' => array( 'image/webp', 'image/jpeg' ),
    );

    The following would mean regardless of whether JPEG or WebP is uploaded, only WebP sub-sizes are generated. This is what the original webp-uploads implementation from #4 did. Sites which don't need JPEG at all would likely go that approach.

    $image_mime_transforms = array(
    	'image/jpeg' => array( 'image/webp' ),
    	'image/webp' => array( 'image/webp' ),
    );

    The following would mean for every JPEG and WebP uploaded, sub-sizes are generated in the same MIME type. This is basically how WP core currently works.

    $image_mime_transforms = array(
    	'image/jpeg' => array( 'image/jpeg' ),
    	'image/webp' => array( 'image/webp' ),
    );

    The following would mean regardless of whether JPEG or WebP is uploaded, only JPEG sub-sizes are generated. This is a bit of an obscure use-case IMO, but I guess if a site really needs JPEG and doesn't care about WebP, it's possible.

    $image_mime_transforms = array(
    	'image/jpeg' => array( 'image/jpeg' ),
    	'image/webp' => array( 'image/jpeg' ),
    );

    Implementation

    We could easily implement this filter in its own function. It would tie in nicely with the existing implementation from #142, we would just need to make sure that in that filter we would ignore the formats that already exist for the image (e.g. if only JPEG sub-sizes should be generated for JPEG uploads, we don't need to do anything extra).

    We would then also need to re-introduce a filter for image_editor_output_format, similar to the one from the original implementation in #4, which based on the above new filter either changes the format to JPEG/WebP or keeps it as is.

    Whatever we decide on, we can start development once #147 has been merged. Curious to hear feedback on this proposal!

  2. adamsilverstein commented on Mar 1, 2022

    @adamsilverstein
    Member

    @felixarntz 👍🏼 your proposed filter sounds right, I like the flexibility of the mime->mime array mapping. This will also work well when we want to add support for an additional mime type like avif.

    The following would mean regardless of whether JPEG or WebP is uploaded, only JPEG sub-sizes are generated. This is a bit of an obscure use-case IMO, but I guess if a site really needs JPEG and doesn't care about WebP, it's possible.

    This could be applicable for sites that for some reason need to support clients that don't support WebP (eg. IE or older MacOS, or windows Outlook)

    The following would mean regardless of whether JPEG or WebP is uploaded, only WebP sub-sizes are generated.

    One caveat here is that if the server lacks WebP support we should output jpeg sub-sizes for jpeg uploads (for WebP uploads we won't be able to do resizing at all if WebP support is missing)

    We would then also need to re-introduce a filter for image_editor_output_format

    Right, this will let us control the default output format from the plugin. for the core merge, we may be able to introduce the output change more directly by calling the function directly in get_output_format.

  3. felixarntz commented on Mar 1, 2022

    @felixarntz
    MemberAuthor

    @adamsilverstein

    One caveat here is that if the server lacks WebP support we should output jpeg sub-sizes for jpeg uploads (for WebP uploads we won't be able to do resizing at all if WebP support is missing)

    Absolutely. I think that will need to be handled by the logic consuming the filter value. If the formats provided in the filter are not supported by the server, the implementation should always fall back to generating sub-sizes in the originally uploaded format, so that in any case there will be sub-sizes generated.

  4. mitogh commented on Mar 1, 2022

    @mitogh
    Member

    This approach looks good to me, just a couple of notes for some scenarios that would be great to define.

    The following would mean for every JPEG and WebP uploaded, sub-sizes are generated in the same MIME type. This is basically how WP core currently works.

    $image_mime_transforms = array(
    	'image/jpeg' => array( 'image/jpeg' ),
    	'image/webp' => array( 'image/webp' ),
    );

    This is great, however another way to make this work would be by just returning an empty array

    $image_mime_transforms = array();

    So by doing this we let WP core to handle the logic for the images which should be the default behavior, allowing using an empty array, simplifies the removal of this behavior by just adding the callback __return_empty_array to the filter.

    Another thing about this one is the fact that this approach does not handle valid transformations, meaning that a using this approach we could specify something like:

    $image_mime_transforms = array(
    	'image/jpeg' => array( 'image/gif', 'image/png' ),
    );

    While technically that would work in some cases, I don't think it would make sense, (mostly an edge case but something to take in mind) to open the possibility to define a safe list of transformations between formats likeget_allowed_mime_types so a function like get_allowed_mime_transformations that would verify or ensure that the provided list of transformations is valid can be an enchantment.

    cc @felixarntz @adamsilverstein 0

  5. felixarntz commented on Mar 1, 2022

    @felixarntz
    MemberAuthor

    @mitogh

    This is great, however another way to make this work would be by just returning an empty array

    That could make sense, however it makes for an inconsistent API if we don't include the original format itself in the transforms values as well:

    • Based on my proposal 'image/jpeg' => array( 'image/jpeg', 'image/webp' ) clearly says "I want JPEG and WebP for all JPEG". And 'image/jpeg' => array( 'image/webp' ) clearly says "I want only WebP for all JPEG".
    • If we omit the original format like you're suggesting, then both of the above scenarios would be 'image/jpeg' => array( 'image/webp' ), so it would be unknown whether the user wants JPEG and WebP or only WebP.

    So I think my original proposal is more clear here.

    Another thing about this one is the fact that this approach does not handle valid transformations, meaning that a using this approach we could specify something like:

    Agreed, but I don't think that's a problem with the API. We need to make sure that whatever developers provide in the filter is validated. We have to decide what we would allow there, and for example we could initially limit the filter to only support any permutations of JPEG and WebP. Still, the API makes sense to be flexible, we should build it in a future-proof way.

  6. mitogh commented on Mar 1, 2022

    @mitogh
    Member

    If we omit the original format like you're suggesting, then both of the above scenarios would be 'image/jpeg' => array( 'image/webp' ), so it would be unknown whether the user wants JPEG and WebP or only WebP.

    Correct, the main idea is defining that scenario, I think the behavior if a mime type is not defined basically that the source property won't be created for that mime type, which would be the case for a PNG or a GIF image for example. So when an empty array is returned from this filter no transformation should take place, or that would be the definition of unknown in this case.

    Agreed, but I don't think that's a problem with the API. We need to make sure that whatever developers provide in the filter is validated. We have to decide what we would allow there, and for example we could initially limit the filter to only support any permutations of JPEG and WebP. Still, the API makes sense to be flexible, we should build it in a future-proof way.

    💯

  7. mitogh commented on Mar 2, 2022

    @mitogh
    Member

    Hey @felixarntz @adamsilverstein ,

    I was looking at this filter again, and just to confirm and not to create a new issue, the main idea with this filter is to control the uploads right? Not so the rendering or the image delivery to the client (browser). Meaning that currently there's no way for a user to determine which format would be delivered to the users when opening a page, besides image/webp by default.

    Take a look at: #186 (comment) as well for another approach.

    Regardless of the suggested approach above, I feel there should be a filter that short-circuit all the logic to deliver a mime type regardless. Let me know if you are envisioning this filter to handle that use case or if you are in favor of a new filter instead so I can open a new ticket.

    My personal preference would be to have a single filter to specify a mime type that would be rendered if no mime-type is specified the logic from #186 would be used instead. If a mime is specified that mime type would be used regardless of the logic from #186

  8. adamsilverstein commented on Mar 2, 2022

    @adamsilverstein
    Member

    I was looking at this filter again, and just to confirm and not to create a new issue, the main idea with this filter is to control the uploads right?

    Right, I was expecting we would add a separate filter to specify the mime format to use for rendering.

  9. felixarntz commented on Mar 2, 2022

    @felixarntz
    MemberAuthor

    @mitogh

    I was looking at this filter again, and just to confirm and not to create a new issue, the main idea with this filter is to control the uploads right? Not so the rendering or the image delivery to the client (browser). Meaning that currently there's no way for a user to determine which format would be delivered to the users when opening a page, besides image/webp by default.

    Take a look at: #186 (comment) as well for another approach.

    [...]

    My personal preference would be to have a single filter to specify a mime type that would be rendered if no mime-type is specified the logic from #186 would be used instead. If a mime is specified that mime type would be used regardless of the logic from #186

    Good point raising this, and I mostly agree with your suggestion. One additional thing to consider there though is also which images formats should even be eligible to be used in the frontend unconditionally. For example, image/avif we wouldn't want to even consider as an option if it existed. I realize right now we only have JPEG and WebP both of which would work in the frontend without fallback, but we need to think through this in a future-proof way, where there may also be formats present that must not be used here under no circumstance.

    Therefore, I think maybe we should instead have that single filter to control a list of MIME types that are eligible. Maybe the filter shape could be exactly the same as what I suggested above in #187 (comment), a map of MIME type => possible MIME types to use in the frontend. And then, based on the possible MIME types, the logic from #186 would run, choosing the smallest format. That would still allow enforcing WebP or JPEG, by just putting that one MIME type in the array.

    So the above filter webp_uploads_image_mime_transforms would be for which formats are generated on upload. And then another filter like webp_uploads_valid_content_image_mimes would be for which formats can be used in the frontend content.

    To provide an example for both filters, let's imagine that in the future we also support AVIF, but it is still not widely supported by browsers, so we can't put that into the frontend without a fallback. We would need to do the following:

    webp_uploads_image_mime_transforms filter (with AVIF):

    $image_mime_transforms = array(
    	'image/jpeg' => array( 'image/jpeg', 'image/webp', 'image/avif' ),
    	'image/webp' => array( 'image/webp', 'image/jpeg', 'image/avif' ),
    );

    webp_uploads_valid_content_image_mimes filter (exclude AVIF):

    $valid_content_image_mimes = array(
    	'image/jpeg' => array( 'image/jpeg', 'image/webp' ),
    	'image/webp' => array( 'image/webp', 'image/jpeg' ),
    );

    What do you think?

    Regarding in which issue to deal with this, I think we should keep the discussion centralized in this issue, since the two filters would be closely related. We could still have two separate PRs to implement them.

  10. mitogh commented on Mar 2, 2022

    @mitogh
    Member

    Agree to have to control the mime types one for the uploads and 1 for the formats delivered to the client.

    Another consideration is how the filter for the content would affect when the <picture> element comes in place #21 , in that instance we could use the same filter to display all the mimes in the array for an image due when using <picture> the browser will use the format that can be displayed. In that scenario should we introduce ordering a mechanism to infer that, what I mean by that is let's say if we have:

    $valid_content_image_mimes = array(
    	'image/jpeg' => array( 'image/jpeg', 'image/webp' ),
    	'image/webp' => array( 'image/webp', 'image/jpeg' ),
    );

    When rendering an image image/jpeg the highest item in priority is image/webp due has a higher index number on the array or we could introduce something like:

    $valid_content_image_mimes = array(
      'image/jpeg' => array( 
        'image/jpeg' => 10,
        'image/webp' => 20,
      ),
    );

    Mainly to make sure this filter is compatible once #21 is ready to go.

    cc @felixarntz @adamsilverstein

  11. eclarke1 commented on Mar 4, 2022

    @eclarke1

    @felixarntz @mitogh is this issue agreed on now and can begin development? Trying to understand who can pick up development of this issue along with #186 and when please?

  12. 15 remaining items

  13. adamsilverstein commented on Mar 8, 2022

    @adamsilverstein
    Member

    The filter should have a default value of a single array array( 'image/jpeg', 'image/webp' ).
    Again, order matters in this array. The array should be traversed in reverse order, and the first MIME type that exists should be used in the frontend.

    @felixarntz what is the thinking behind having this array traversed in the reverse order? I was expecting the first element to be the preferred format if an image is available, otherwise the next one etc. (and I was expecting the default order to be array( 'image/webp', 'image/jpeg' );)

  14. felixarntz commented on Mar 8, 2022

    @felixarntz
    MemberAuthor

    @adamsilverstein I was thinking that between the two filters it would be easier to understand for developers if they need to use the same type of order for both. For the transformations filter the base format to use (typically the original format) comes first (e.g. image/jpeg when uploading a JPEG), so I think it makes sense to use the same order for the content filter. That also ties in with the thought that e.g. image/jpeg for a JPEG is also the "reliable" format for the frontend and therefore should come first - when I say "reliable", I mean that would be the last resort fallback format in the future when using the picture element.

    Because of the above, the array would then need to be traversed in reverse order, so that the prioritized formats for the content at the end are chosen first while the initial formats act as fallbacks, with the very first element being the final fallback, i.e. the format that should be used if nothing else works - that means it's the least preferable, but also the most reliable.

  15. adamsilverstein commented on Mar 8, 2022

    @adamsilverstein
    Member

    Ok, that helps me understand your thinking, thanks!

    I was thinking differently, that being a way for developers to filter the output format they would specify the formats in the preferred order they want them to appear. For now, this would mean the img tag would choose the first mime type available in the sources array, in the future though we could expand that to support the picture element and output sources elements for each specified mime type that was available. Again in this case I would expect the picture sources to appear in the same order they appear in the array, meaning the first would be used by the browser if supported, falling back to the next (or contained img tag).

    I was thinking that between the two filters it would be easier to understand for developers if they need to use the same type of order for both.

    I'm not sure I agree, to me having this one in reverse preference order would make it more confusing :)

    Developers should think about these as two distinct filters with different purposes:

    • one is used to specify the mime types to generate, and the order to generate them in with the first being the "primary" image
    • the other is used to filter the output formats to try to use for the content, with the order specifying the order to try them in.

    I'm curious what do other people think?

  16. felixarntz commented on Mar 8, 2022

    @felixarntz
    MemberAuthor

    @adamsilverstein These are fair points. I don't have a strong preference, but would love to hear what others think.

    I'd say we shouldn't block beginning of the implementation on this though. While we can get started with PRs based on the summary in #187 (comment), let's follow up on this discussion and adjust the PR as needed, which would technically be a small tweak to make.

    We definitely need to come to a conclusion before merging the PR of course.

  17. removed
    Needs DevAnything that requires development (e.g. a pull request)
    on Mar 9, 2022
  18. eugene-manuilov commented on Mar 11, 2022

    @eugene-manuilov
    Contributor

    @felixarntz @mitogh here is the PR for the first part: #227

  19. added this to the 1.0.0-beta.2 milestone on Mar 14, 2022
  20. eugene-manuilov commented on Mar 15, 2022

    @eugene-manuilov
    Contributor

    @felixarntz @mitogh this is the second PR: #230

  21. getsource commented on Mar 17, 2022

    @getsource
    Member

    Hey everyone! I read through this and based on my current understanding, if the filter is designed to allow developers to give which their preferred formats are, my preference would be to have them listed in priority order (most preferred first), similar to how the wp_image_editors filter functions.

    It would be confusing to me to be asked by WordPress core to ”List the image format you don’t prefer first”.

  22. felixarntz commented on Mar 17, 2022

    @felixarntz
    MemberAuthor

    @getsource That makes sense of course. I think the complexity here is that there are two competing priorities:

    1. For maximum backward compatibility / browser support we would prefer JPEG.
    2. For the most progressive image format we would prefer WebP, eventually AVIF even more, etc.

    So the question is: Based on which of these priorities do we sort the items in the filter? Right now it's based on 1., if we reverse it like you and @adamsilverstein are suggesting, it would be based on 2.

  23. felixarntz commented on Mar 18, 2022

    @felixarntz
    MemberAuthor

    Reopening since #230 is also a requirement for this to be completed and it's still in review.

  24. felixarntz commented on Mar 18, 2022

    @felixarntz
    MemberAuthor

    Fixed via #227 and #230.

    Follow-up issues #237 and #238 have been created.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions