Fix inverted AVIF quality on imagick, and quality being ignored by base64() on gd and vips - #332
Merged
Merged
Conversation
Imagick reads the compression quality from two different setters depending on the output format. JPEG and WebP use the per-image setter, while PNG and AVIF read the global one. That global setter was unconditionally inverted with a "For PNGs" comment. PNG needs it, because there the value is a compression level where a higher number means a smaller file. AVIF reads the same setter but treats it as a regular quality, so it silently inherited PNG's inversion: quality(20) produced a quality 80 image and quality(80) produced a quality 20 one. The quality is now stored and only inverted for PNG, right before the image is written, when the target format is known.
The added webp and avif cases were failing on CI. The avif ones are skipped there anyway, and the webp output of the ImageMagick build on the runners does not change between quality 10 and 50. The base64 test now only covers the imagick driver, since the gd and vips drivers do not pass the quality on to every format when encoding to base64. Those are separate, pre-existing bugs.
freekmurze
marked this pull request as ready for review
August 18, 2026 07:31
GdDriver::base64() called imagewebp() and imageavif() without a quality argument, and did not convert a palette image to true color first, so webp and avif output ignored the requested quality. Its save() already did all of this. VipsDriver::base64() called writeToBuffer() without any options, so it ignored the quality for every format. The logic that builds those save properties is now shared with save(). The avif capability check no longer bails out on CI. It probes the driver by actually encoding an image, since a driver can report avif as available while being built without an encoder for it. That also lets the vips driver run the avif tests, which it never did before.
The imagick build on the runners returns the same webp bytes for quality 10 and 90, which is not something this package controls. The per-format base64 test now only asserts that the quality is never applied in reverse, which is what the avif bug did. To keep the base64 fixes covered, a separate test asserts strictly on jpeg, the one format every driver does vary. Verified against the pre-fix code: without the fixes in this branch, the avif inversion and the dropped quality both still fail the suite.
The baseline records 4 occurrences of the GdImage|false assignment, but the exif orientation handling has grown to 7 imagerotate() calls since, so the analysis failed on the count mismatch rather than on any new error. This has been failing on main since at least March 2026.
The workflow only watched phpstan.neon.dist, so a commit that touches just the baseline never triggered the analysis. That is also why the stale count in the baseline could go unnoticed for months.
Member
|
Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Started out as the AVIF quality inversion on the Imagick driver, and grew to cover two related quality bugs found while adding test coverage.
1. AVIF quality is inverted on the imagick driver
Asking for quality 20 gives you a quality 80 image, and asking for 80 gives you a quality 20 one.
Measured on
main, encoding the test JPEG to AVIF:quality(20)quality(80)Exactly swapped.
Why
Imagick reads the compression quality from two different setters depending on the output format:
setImageCompressionQuality()— read by JPEG and WebPsetCompressionQuality()— read by PNG and AVIFThe driver set the second one to
100 - $quality, with a// For PNGscomment:The inversion is correct for PNG. PNG is lossless, so its "quality" is really a zlib compression level where a higher number means a smaller file — inverting keeps the library's "higher quality = bigger file" contract. Verified: every PNG quality value produces byte-identical pixels, and raw Q=0 → 36182 bytes vs Q=90 → 35654 bytes.
AVIF reads that same setter but treats it as a normal quality, so it silently inherited PNG's inversion. JPEG and WebP were never affected, because they read the other setter.
Fix
Store the requested quality and only invert it for PNG, resolved right before the image is written, once the target format is known. Applied on both
save()andbase64(), since each resolves its own format — removing either call flips PNG back the wrong way.2.
GdDriver::base64()ignored the quality for webp and avifIt called
\imagewebp($this->image, null)and\imageavif($this->image, null)with no quality argument, and skipped theimagepalettetotruecolor()call thatsave()does. Onmain, GDbase64('webp')returns 10792 bytes at both quality 10 and 90.3.
VipsDriver::base64()ignored the quality entirelyIt called
writeToBuffer()with no options at all, whilesave()builds aQ/compressionsave property. Onmain, vipsbase64('webp')returns 10204 bytes andbase64('jpeg')27680 bytes at both quality 10 and 90. The property-building logic is now shared between the two paths.Verification
Every format, both write paths, imagick driver:
After the fix,
quality(20)on AVIF produces a file byte-identical to raw Imagick atQ=20.All three bugs were confirmed against the pre-fix code before being fixed, and the new tests were checked against that same pre-fix code to make sure they actually fail without these changes — the avif inversion and the dropped quality each fail the suite there.
Tests and CI
The existing quality test only covered
jpgandpngthroughsave(), which is why all of this went unnoticed. This PR adds a regression test for the AVIF inversion, andbase64()coverage across drivers and formats.avifIsSupported()previously returnedfalsewheneverGITHUB_ACTIONSwas set, so no AVIF test had ever run on CI. It also hard-codedfalsefor the vips driver, which does support AVIF. It now probes a driver by actually encoding a small image — reporting the format as available is not sufficient, since a driver can be built against a libheif without an AVIF encoder and only fail at encode time.The workflow now installs
libheif1+libheif-plugin-aomencand prints the detected AVIF support per driver. On the runners this reportsgd imageavif(): yesandimagick AVIF formats: AVIF, so the avif tests now genuinely run in CI.One caveat worth recording: the imagick build on the runners returns identical webp bytes for quality 10 and 90, which is outside this package's control. The per-format base64 test therefore only asserts that quality is never applied in reverse, which is what the avif bug did. A separate test asserts strictly on jpeg — the one format every driver reliably varies — so a driver dropping the quality entirely still gets caught.
PHPStan
Also fixed, since it would otherwise keep this PR red. It was failing on a stale baseline count, not on a new error: the baseline records 4 occurrences of the
GdImage|falseassignment, but the exif orientation handling has grown to 7imagerotate()calls since it was written. This has been failing onmainon every run since at least March 2026.