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

AVIF images and quality settings #123

Merged
merged 16 commits into from May 23, 2022
Merged

AVIF images and quality settings #123

merged 16 commits into from May 23, 2022

Conversation

ncla
Copy link
Collaborator

@ncla ncla commented Apr 22, 2022

Just putting this out there, still working on this, just in case someone has same idea (or differing thoughts), so there isn't duplicate effort. 😅

TODO:

  • Figure out best values for quality setting
  • Write minimal GraphQL tests for AVIF/WEBP srcset inclusion
  • README updates

Intro

In PHP 8.1, GD and Imagick PHP extensions received AVIF support. Statamic 3.3 has upgraded to newer version of Glide too. AVIF offers even more file size savings, more than WEBP does! AVIF browser support is improving as time goes, seems like it is the next WEBP. https://caniuse.com/avif

AVIF disabled by default

As we have now three types of image file formats, I have disabled AVIF by default in the config, just in case those who upgrade to newer version don't stumble upon even longer page load times due to large amount of image generations required. And because AVIF is not supported as widely yet.

Quality setting

Quality setting goes from 0 to 100 for JPG and WEBP images. AVIF however does 0 to 62. And because quality defaults to 90 in Glide, we end up with AVIF image that is larger than JPEG or WEBP in file size. We have to figure out good optimal, default quality setting values for JPG, WEBP and AVIF. 90 is in my opinion too high for JPEG and WEBP.

Can use tools like this to determine good values:
https://www.industrialempathy.com/posts/avif-webp-quality-settings/
https://squoosh.app/editor

Low level PHP functions/methods reference that set the quality:
https://www.php.net/manual/en/imagick.setimagecompressionquality
https://www.php.net/manual/en/function.imageavif

This quality adjustement could be considered a breaking change.

Future ideas

I also would like to add option to disable JPEG images too, as support for WEBP is pretty good now, and the browser can always use the default tag src image to fall back on. This would help reduce amount of generated images too. But I am leaving this for the future as it is out of scope for this PR in my opinion. Let me know what you think.

Another future PR idea is to have adjustable quality setting per file format. AFAIK developers have no control over this when using this addon.

@ncla
Copy link
Collaborator Author

ncla commented Apr 23, 2022

I fixed the zero width test, even though in my opinion this addon should not account for such edge cases and just let Intervention or Glide fail with it's own exceptions. Or check upon image upload if all is correct.

@riasvdv
Copy link
Member

riasvdv commented Apr 25, 2022

This is looking very good so far! I'll check it out once you mark it ready for review

even though in my opinion this addon should not account for such edge cases and just let Intervention or Glide fail with it's own exceptions.

I'd rather it display no image than breaking the full page though.

Or check upon image upload if all is correct.

Checking upon upload is hard, because then you're checking all files, not just the ones people might use with responsive images

I also would like to add option to disable JPEG images too, as support for WEBP is pretty good now, and the browser can always use the default tag src image to fall back on. This would help reduce amount of generated images too. But I am leaving this for the future as it is out of scope for this PR in my opinion. Let me know what you think.

I'd leave them enabled by default but having the option is a good idea!

Figure out best values for quality setting

We could make it configurable in the config or have it passed to the tag

@ncla
Copy link
Collaborator Author

ncla commented Apr 29, 2022

I stumbled upon one issue for quality settings per output format which got me thinking of complications. When editing README, I read that you can also use PNG as source image, and PNG URLs will be generated for the default srcSet. Up until this point I thought users mostly would use JPGs as source image (how naive of me) or it would be forced JPG output. But from my understanding the only limit to input file formats is what Glide/Intervention/Statamic/PHP supports/allows, except for GIFs and SVGs which do not generate responsive variants in this addon. The default srcSet seems to be intentionally designed not to be specific (including not specifying mimetype in type property).

So to pass quality setting for the default original source image file format, I guess we can just use param like this quality="90". For AVIF and WEBP it would be specific: quality:webp="90" quality:avif="90".

PNGs seem to not be interested in the quality setting, according to the Intervention docs (a bit outdated cus no mention of WEBP/AVIF but I think it still applies);

Quality is only applied if you're encoding JPG format since PNG compression is lossless and does not affect image quality.

Do users of this addon expect transparent images to work?

You can also upload a WEBP image and you will end up with two srcsets that both output WEBP URLs, with the latter one having no source type defined. Though admittedly this is a bit of an edge case.

As such, the previously mentioned idea of "disabling JPG srcset" should be "disabling default srcset" instead. But I think for now I will maybe leave this part for another PR, as this has gotten a bit out of hand than I would like to, ha.

Thoughts?

@riasvdv
Copy link
Member

riasvdv commented Apr 29, 2022

Quality per tag seems a bit overkill though, would you really want this to be set per image? Maybe we'd just have something like:

'quality' => [
    'webp' => 90
    'avif' => 90,
    'jpg' => 85,
]

In the config file, and use those instead? Per image seems like more of a "nice to have" that could be added in the future.

@ncla
Copy link
Collaborator Author

ncla commented Apr 29, 2022

I agree, I don't think there's lots of use cases to be honest in one project to have differing quality settings. Will go for just config then, which is what I currently have implemented, just have to remove the params then.

I think I will change it so that you can specify any format quality in config, and if a format is there in the config array, the addon will use it.

@ncla
Copy link
Collaborator Author

ncla commented Apr 29, 2022

On another hand, one useful use case I forgot about is when you utilize breakpoints, and for example you would set lower quality for mobile viewport to improve loading times for mobile. And as I already had coded this I will leave this in. Apologies for back and forth. Anyway..


The PR is ready for review.

For quality values I just used the default 90 for backwards compatibility and familiarity, this is for JPG and WEBP. For AVIF I used 45. I roughly deducted -20% of the WEBP filesize to get that number. This is according to this comparison article which used DSSIM to compare format quality and size. If somebody doesn't like AVIF default value, they can just change it. Or if they want to rely on Glide defaults, you can put null in config and it will not generate quality URL parameter.

Keep in mind that for some reason AVIF does not adjust quality from 0 to 100 as JPEG and WEBP does, from my testing with Imagick at least. It seems to be going from 0 to 62 instead. That's why there is such a low quality value for AVIF.

I added the AVIF stuff to GraphQL and wrote very minimal tests for it, there was none before for GraphQL. Writing more tests to cover other stuff in GraphQL would be out of scope of this PR I feel like.

If you are testing locally you might have difficulties getting AVIF working with PHP GD, you might have to use Imagick instead. See this issue.

For other new features/ideas I mentioned in this PR comments, another PR will have to do.

Lemme know if anything needs changing. 👍

@ncla ncla marked this pull request as ready for review April 29, 2022 15:50
@ncla ncla changed the title Add AVIF images AVIF images and quality settings Apr 29, 2022
@ncla
Copy link
Collaborator Author

ncla commented May 19, 2022

@riasvdv Hi. Did this slip through the cracks? Pinging you just in case. Let me know if stuff needs changing. Thanks.

@riasvdv
Copy link
Member

riasvdv commented May 19, 2022

It did! Thanks for reminding me, I'll take a look at this today or tomorrow 👍

@riasvdv riasvdv merged commit 9dd4e53 into spatie:main May 23, 2022
@riasvdv
Copy link
Member

riasvdv commented May 23, 2022

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants