-
Notifications
You must be signed in to change notification settings - Fork 29
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
Conversation
Could tweak the values further, they are not final in my opinion
This reverts commit 7543403.
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. |
This is looking very good so far! I'll check it out once you mark it ready for review
I'd rather it display no image than breaking the full page though.
Checking upon upload is hard, because then you're checking all files, not just the ones people might use with responsive images
I'd leave them enabled by default but having the option is a good idea!
We could make it configurable in the config or have it passed to the tag |
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 So to pass quality setting for the default original source image file format, I guess we can just use param like this 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);
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? |
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:
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. |
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. |
…ed AVIF fallback quality value
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 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. 👍 |
@riasvdv Hi. Did this slip through the cracks? Pinging you just in case. Let me know if stuff needs changing. Thanks. |
It did! Thanks for reminding me, I'll take a look at this today or tomorrow 👍 |
Thanks! |
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:
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.