Skip to content

Media: Add a libvips image editor (WP_Image_Editor_Vips) - #13762

Open
nickchomey wants to merge 84 commits into
WordPress:trunkfrom
nickchomey:image-editor-vips
Open

nickchomey wants to merge 84 commits into
WordPress:trunkfrom
nickchomey:image-editor-vips

Conversation

@nickchomey

@nickchomey nickchomey commented Sep 26, 2026 •

Copy link
Copy Markdown

Adds WP_Image_Editor_Vips, an image editor backend backed by libvips reached through PHP's FFI extension.

It is listed first in wp_image_editors, ahead of WP_Image_Editor_Imagick and WP_Image_Editor_GD. It reports itself as unavailable when libvips, the FFI extension, or ffi.enable is missing, so wp_get_image_editor() falls through to Imagick or GD and nothing changes on a server that cannot run it.

Trac ticket: https://core.trac.wordpress.org/ticket/65473

Inspired by the vips-image-editor-ffi plugin, but heavily reworked to follow the patterns of the GD and Imagick editors and to implement equivalent test coverage.

What this adds

  • A complete WP_Image_Editor implementation: load, save, resize, crop, rotate, flip, stream, multi_resize(), and make_subsize().
  • JPEG, PNG, GIF, WebP (including lossless), TIFF, AVIF and HEIC/HEIF support. Support for each format is probed at runtime, so a build without the relevant encoder reports the format as unsupported instead of failing on save.
  • Reporting in Site Health and the debug data, alongside the existing Imagick and GD entries, including whether the FFI extension and libvips are available.
  • Local Docker environment support so the test suite can be run against libvips.
  • A minimum libvips version of 8.13, plus guards on the decoded pixel count and on which libvips loaders may be used. Each is explained under the design notes.

Design notes

Several parts of this take a route that is not the obvious one, because the obvious libvips call does not work. Flagging them here so they do not read as mistakes.

  • EXIF orientation is applied with autorot(), not by rotating and flipping. libvips applies all eight EXIF orientations itself and clears the orientation field as it goes. Using the parent class's rotate/flip switch would leave the field in place, and the saved file would then be oriented a second time when it is loaded again. This is the "EXIF Orientation can be reset afterwards" requirement maybe_exif_rotate() documents.

  • Rotate and flip reset the orientation tag by setting the orientation field to 1 and then calling autorot(). This mirrors Imagick's setImageOrientation( ORIENTATION_TOPLEFT ). Setting the field alone does not reach the file: libvips writes JPEG EXIF from the EXIF block rather than from the metadata field. autorot() is the operation that rewrites that block, and declaring the image upright first means it has nothing to apply, so the pixels are untouched and only the tag is corrected. Verified: pixdiff = 0 against the un-reset image, and no change to PNG files that have no EXIF at all.

  • _resize() is a separate method because libvips is immutable — every operation returns a new image — so resize() and make_subsize() both need a fresh image without disturbing $this->image. This is GD's shape rather than Imagick's, which mutates in place and works around it by snapshotting and restoring.

  • Encoder support is probed by writing to a buffer, not a temp file, which avoids a temp file per format check.

  • Paletted PNGs are saved with palette => true. libvips expands a palette on load, so the original colour type is read from the PNG IHDR chunk at load time and a palette is requested back on save. Without it a resized indexed PNG is written as true colour; the same fixture that is 25 KB on disk comes back at 342 KB.

  • image_max_bit_depth is applied as an encoder option, not by downcasting. cast() to a lower band format fails on write with pngsave: invalid buffer size for any 16-bit source (libvips 8.16.1), so the filter is passed to the encoders that accept a bit depth instead.

  • Metadata is reduced to the set Imagick keeps rather than stripped with libvips' strip. strip is a boolean that drops every field, including the colour profile, which changes how the image renders once nothing tells the browser how to interpret the samples. libvips instead offers a keep mask, set to the same set WP_Image_Editor_Imagick::strip_meta() protects: ICC, EXIF, XMP and IPTC. image_strip_meta still decides whether any of this happens. The mask only exists from libvips 8.15, so get_metadata_strategy() is the one place that decides what a given version can do:

    libvips keep mask Fields removed by hand
    8.13 – 8.14 unavailable (passing it fails the save) yes
    8.15 used yes, for what the mask misses
    8.16+ used no
  • The minimum libvips version is 8.13, which is the first release that can write a palette back to a PNG — the same feature the paletted-PNG note above depends on. Below it the editor reports itself unsupported rather than running without it.

  • The resampling kernel is passed explicitly and is filterable (wp_image_editor_vips_resampling_kernel, default lanczos3), because libvips would otherwise choose silently. Measured over 28 cases against core's Imagick output and the original, lanczos3 led SSIMULACRA2, SSIM, MS-SSIM and IW-SSIM. It did not lead Butteraugli (lanczos2 was marginally better) or CIEDE2000 (Imagick's output did, with lanczos3 second). It costs roughly 10% more bytes and about 20% more CPU than linear. Those are full-reference metrics against a chosen reference, so they measure distance rather than quality; the default is lanczos3 and linear is one filter away.

  • Truncated files are rejected through libvips' fail_on loader option, with JPEG set to none so a damaged JPEG is filled rather than rejected, matching the other editors. The option is load-bearing only for some formats: a truncated GIF or JPEG reads cleanly without it, while PNG and WebP fail in the decoder either way. The test therefore uses a GIF, which is the case the option actually decides.

  • Encoding options are set for parity, not by default. JPEG gets optimize_coding (measured 4.6% smaller at equal quality on a 33.8 MP source). image_save_progressive is mapped to libvips' interlace for JPEG, PNG and GIF, with the GIF case guarded because gifsave only accepts it from libvips 8.14. AVIF gets an effort of 3 (wp_image_editor_vips_avif_effort); libvips defaults it to 4, verified by encoding with and without the option, and on a 33.8 MP image that is 73 CPU-seconds against 26 at effort 3, for a 0.35% size cost. Effort 3 is still close to a typical 30-second PHP-FPM limit at that size, and effort above 4 produces a slightly larger file.

  • WebP quality is mapped the way Imagick maps it. ImageMagick's WebP coder reserves quality 100 for lossless, so the same number would otherwise mean very different things in the two editors — libvips' Q100 is a lossy maximum, measured at 90,262 bytes where Imagick's Q100 is 311,954 and a lossless write of the same image is 309,444. Quality 100 is therefore written losslessly here.

  • libvips chooses its loader from the file's magic bytes, not the extension, so the format an upload claims is not necessarily the format decoded. Measured: an SVG renamed to .jpg still decodes through svgload. The decoder is therefore checked against an allowlist (wp_image_editor_vips_allowed_loaders), and a loader outside it is refused, which reports the format as unsupported and lets wp_get_image_editor() fall through to Imagick.

  • A pixel cap is enforced at load (wp_image_editor_vips_max_pixels, default 16383², 268,402,689 px). libvips reads pixels lazily, so a small file can announce enormous dimensions and only cost that memory once something touches the pixels; the dimensions are known first, so the refusal comes first too. Two things about the default are worth stating plainly, because the obvious reading is wrong: it is not a format limit — a JPEG can describe 65500 pixels per side, verified by encoding 17000- and 40000-pixel-wide JPEGs and failing only at 70000 — and it is not a limit the other editors share, since neither GD nor Imagick restricts the size it will attempt. It is a deliberately conservative guard, and because _wp_image_editor_choose() selects an editor before the file is opened and does not retry, an over-size image fails to load rather than falling through to them. A site handling images that large has to raise the filter.

Vendoring

php-vips is vendored rather than required through Composer, so that a site running this needs no Composer step and no network access at runtime. The tree is scoped with PHP-Scoper into WordPress\VipsDependencies\... with Jcupitt\Vips deliberately left unscoped, so the public API of the library is unchanged. The generated autoload.php records the upstream repository, ref and commit it was built from, and regeneration is deterministic.

tools/php-vips/installer.sh performs the vendoring. With no arguments it resolves the latest upstream release; --version= and --branch= select a specific ref. It hard-fails if the scoper step does not actually rewrite files, which it will do silently on PHP 8.5 with older scoper releases.

Testing instructions

The local Docker environment builds a PHP image with libvips and the FFI extension. Add to .env (which is not tracked):

LOCAL_PHP_VIPS=true

Then:

npm run env:start

# The editor's own tests.
npm run test:php -- --group wp-image-editor-vips

# The rest of the image surface, which now runs against VIPS.
npm run test:php -- --group image
npm run test:php -- --filter 'Tests_Media|WP_Test_REST_Attachments_Controller'

# The static analysis gates.
npm run env:composer -- lint
npm run typecheck:php -- -- src/wp-includes/class-wp-image-editor-vips.php

To confirm the fallback path, run the same group with FFI disabled. npm run test:php cannot pass -d through to PHP, so invoke the runner directly:

node ./tools/local-env/scripts/docker.js run -T --rm php php -d ffi.enable=0 ./vendor/bin/phpunit --group image

ffi.enable is what makes it work rather than unloading the extension: the extension stays loaded, and libvips fails to initialise, so WP_Image_Editor_Vips::test() returns false and wp_get_image_editor() selects Imagick or GD instead.

Test results

With VIPS enabled, on the branch rebased onto current trunk:

Suite Result
wp-image-editor-vips 77 tests, 243 assertions, 1 skip, 0 failures
image 376 tests, 980 assertions, 7 skips, 0 failures
Tests_Media + WP_Test_REST_Attachments_Controller 701 tests, 2,636 assertions, 3 skips, 0 failures

Rebasing onto current trunk exposed two VIPS-only failures, both since fixed and both worth recording because each was a real divergence from Imagick rather than a difference of taste:

  • A failed stream fetch reported invalid_image rather than error_loading_image. The read sat inside the same try as the decode, so a stream that throws was indistinguishable from a file that would not decode. Imagick reads a stream first and maps only that failure to error_loading_image; this editor now does the same.
  • WebP at quality 100 was written lossily. ImageMagick's WebP coder reserves quality 100 for lossless, so Imagick's 99 corresponds to libvips' 100 and its 100 is a jump. Measured on one 768×432 image: Imagick 88,964 bytes at quality 99 against 311,954 at 100; libvips 90,262 at Q100 against 309,444 written losslessly. Quality 100 is now written losslessly, which is what the test_quality_with_image_conversion_file_sizes assertion depends on. GD behaves like libvips here (85,330 bytes at Q100); it passes that test only because GD strips metadata, so its JPEG intermediate is much smaller.

Static analysis: PHPStan reports no errors on the changed files, and PHPCS is clean.

All of these were re-measured after the rebase onto current trunk. wp_generate_attachment_metadata: 7 tests, 18 assertions, 0 failures. Full suite: 26,505 tests, 4,563,120 assertions, 45 skips, 86 warnings, and one failure that is not related to this PR — Tests_Theme_ThemeDir::test_broken_themes asserts the exact contents of get_broken_themes(), so it fails in any environment that has extra theme directories installed. It fails identically with ffi.enable=0, i.e. with this PR's editor out of the picture entirely, and the PR touches no theme files. The warnings are the suite's own PHPUnit 10 deprecation notices for expectDeprecated(), not failures, and none of them come from the image tests.

Known limitations

  • image_max_bit_depth has no effect on JPEG, WebP or GIF. libvips stores samples in a fixed set of band formats rather than at an arbitrary bit depth, and only the PNG and HEIF encoders accept a bit depth. Imagick applies the filter to every format because it lowers the depth of the in-memory image. This is documented in get_save_bit_depth().

  • A resized paletted PNG can come out slightly larger than its source. palette => true re-quantises at libvips' default quality, which lands close to the original but can overshoot: of five paletted fixtures, two shrink and three grow by up to a third (test8.png, 45,231 → 60,310 bytes). Passing a Q value makes all five smaller but controls how aggressively colours are reduced, so it trades colour fidelity for bytes. Left alone rather than decided unilaterally.

  • Encoding to AVIF or HEIC depends on the host's encoder plugins. Reading HEIC and HEIF works with the standard libheif decoder plugins, but writing needs an encoder plugin, and libvips loads those as plugins at runtime, so the list of formats the build was compiled with says nothing about whether they can actually encode. Those formats are therefore always verified by encoding rather than by that list, and a host without the plugin reports the format as unsupported. Core only ever writes one of them: wp_get_image_editor_output_format() maps HEIC and HEIF to JPEG, so the HEIC probe matters for reading and for wp_getimagesize(), and only comes into play for writing if a site filters that mapping.

  • No CI job runs these tests. The suite runs without libvips in CI, so the editor is not yet exercised automatically. Guidance on how core would like that set up is welcome. On Ubuntu 24.04 the runtime package is libvips42t64 rather than libvips42, and the packaged 8.15.1 is new enough for everything used here, so a stock runner should work without a custom image.

Reviewer notes

  • Two changes outside the new class are not self-explanatory from the diff. The HEIC branch of wp_getimagesize() no longer requires instanceof WP_Image_Editor_Imagick; any editor that claimed HEIC support and loaded the file can report the size, and GD never reaches it because it does not claim HEIC support. Separately, the image_max_bit_depth docblock in the Imagick class stated that the filter "only applies when resizing using the Imagick editor since GD does not support getting or setting bit depth", which stops being true once VIPS honours it.

  • The pixel cap changes behaviour, not just resource use, and it is the one guard here I would like a second opinion on. Because editor selection happens before the file is opened and never retries, an image larger than the default now fails where Imagick would have attempted it. That is the intended protection, but it is a decision rather than a side effect.

  • There is an @phpstan-ignore in maybe_exif_rotate(). The is_callable( 'exif_read_data' ) guard is reported as function.alreadyNarrowedType because PHPStan's environment always has the extension, but the extension is optional in practice and @ does not suppress a fatal Call to undefined function. class-wp-image-editor.php:489 carries the identical construct; it is suppressed through a baseline entry, and tests/phpstan/baselines/function.alreadyNarrowedType.neon has the same suppression for the same function in src/wp-admin/includes/image.php. The baselines are documented as being for pre-existing errors and meant to shrink, so an inline ignore was used for new code. Happy to switch to whatever the project prefers.

  • _resize() has no Imagick equivalent. If reviewers prefer the editors to share a shape, this can be dropped in favour of Imagick's snapshot-and-restore approach.

  • The two libvips behaviours in the design notes above (set( 'orientation', 1 ) not reaching the saved file, and cast() breaking pngsave) are the two places where reading the code without context will suggest a simpler implementation. Both were measured, not assumed.

Use of AI Tools

AI assistance: Yes
Tool(s): GitHub Copilot
Model(s): DeepSeek V4.1
Used for: Pair programming - design, code and test implementation. Final implementation and tests were reviewed and edited by me.

@github-actions

github-actions Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props nickchomey, init0.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions

Copy link
Copy Markdown

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@nickchomey

Copy link
Copy Markdown
Author

@notglossy this PR is inspired by your work. Do you have any feedback on it?

@kirsky

kirsky commented Oct 7, 2026

Copy link
Copy Markdown

Thanks for working on this! It's great to see libvips being considered for WordPress core. Faster image processing with lower memory usage would be a really useful improvement for WordPress. I maintain a plugin with the same functionality on wordpress.org, Indigit Imagination, so I tested revision (75a5a0f) against problems I had already met. Here are the things I ran into while testing, and I can elaborate on each of them if you want me to. Host: PHP 8.4, libvips 8.16.1, unless noted.

1. Minimum libvips version. WP_Image_Editor_Vips::test() accepts 8.7. On libvips 8.12.1 (Ubuntu 22.04, PHP 8.1) uploads in PHP-FPM hang with cannot register existing type 'VipsObject', and 4 of the 54 tests in --group wp-image-editor-vips also fail. I recommend 8.13.

2. AVIF is very CPU-heavy. A 33 MP JPEG converted to AVIF takes 42 CPU-seconds with this editor (Imagick: 9.6). With PHP's usual 30 s limit on typical Linux PHP-FPM, the upload is killed after about 5 s, because PHP counts the CPU time of all threads. The AVIF save options set no effort, so libvips uses its default of 4. I recommend 3, which took 7.4 CPU-seconds and finished.

3. Files are bigger than Imagick's. JPEG: +25% over the 7 files of one upload, at the same quality setting (82). There are two causes of about equal size. The JPEG save options do not optimise the Huffman tables (optimize_coding), and the resize call uses libvips' default lanczos3 where core's Imagick editor uses a triangle filter (kernel => linear). Fixing either one alone leaves +8% to +9%, and fixing both gives -6.6%. Lanczos is sharper, so it may be deliberate. It still differs from core's current behaviour, so sites that switch will get bigger files than core's Imagick editor gives them today.

4. Metadata. For JPEG and WebP the PR strips all metadata from resized images. Core's Imagick editor keeps these profiles there: icc, icm, iptc, exif and xmp (colour, copyright, orientation and rights data). Without the colour profile, wide-gamut images may change colour in a browser (expected, I did not measure it). A test asserts the stripping, so it may be intended. I recommend matching the metadata semantics of the existing editor. Changing what is kept would be a decision for every editor, not just this one.

5. Damaged files. A PNG cut off at 70% still gets a sub-size built from partial data, where Imagick and GD fail. libvips can make that an error with the loader option fail_on.

6. image_save_progressive. The editor does not apply this filter, and core's Imagick editor does, for every format it saves. A site that uses it to get progressive images would lose that once this editor comes first in the list. I read this in the code and did not test it. I recommend passing interlace to the save options for JPEG and PNG, and for GIF on libvips 8.14 and newer.

7. Untrusted loaders. libvips chooses a loader from the file's content, and 15 of its loaders are flagged untrusted. Through a normal WordPress upload only BMP reaches one (ImageMagick, the same exposure as in Imagick today). The editor has no loader allowlist: load() hands any file to libvips. When wp_get_image_editor() is called on a file nobody checked, the editor opened an SVG and a BMP for me, also when they were named .jpg. I recommend setting VIPS_BLOCK_UNTRUSTED=1.

8. No image size limit. A 1.5 MB grey PNG of 40,000 x 40,000 pixels costs 22 CPU-seconds in this editor, and Imagick refused it. Memory stayed low in my test, so this is about CPU only.

Tests. The tests I found missing, most important first:

  • A run on an older libvips. The 4 failures from item 1 only show on an older libvips, so CI needs one.
  • Several requests in one PHP-FPM worker. The hang from item 1 cannot show in a single PHPUnit process.
  • Damaged files. Nothing tests a truncated PNG, GIF or WebP.
  • The metadata policy, which item 4 is about.
  • AVIF encoding. The only AVIF test, test_image_max_bit_depth, only decodes it, so the CPU cost from item 2 can't show.
  • Rotate and flip. Rotate is only checked by size on a square image, so a wrong rotation would still pass. Flip is checked at a single pixel, in one direction.

I have more points that are not in this list, and I can explain them if you want. I'm willing to help with this feature, and I can open a pull request against your branch with fixes and tests if you would like that. My plugin also handles each of these points, so it may be useful as a reference.

@nickchomey

Copy link
Copy Markdown
Author

Thanks for the review! I started addressing all of it yesterday and will probably finish early next week. I'd appreciate another review after that.

nickchomey and others added 24 commits October 12, 2026 15:39
The keep saver option, used to preserve colour profiles when metadata is stripped, is only available from 8.15. Raise the floor to it and drop the version branches the floor makes unreachable: the 8.8 guard around vips_foreign_get_suffixes(), the 8.13 guard around the PNG palette option, and the 8.14 guard around gifsave interlace.
Pass libvips a keep mask of ICC, EXIF, XMP and IPTC instead of the strip boolean, which discarded every field including the colour profile. The protected set is the one the Imagick editor protects, so a resized image no longer loses how it is meant to be rendered. The strip_meta option is now the same for every format, so it moved out of the per-format switch.
The keep mask is only honoured in full from libvips 8.16. On 8.15, PNG text comments, the profiles ImageMagick writes and the image description survive it, so they are removed from the image before it is encoded. The list of fields the mask protects and the list it misses are derived from the same policy, so a field that is protected is never removed.
The filter is documented on the GD editor, which is where the Imagick editor points as well, and core's own static analysis requires each call site to say so.
The loader is asked to fail on truncation, but libvips reads pixels lazily, so a truncated PNG is rejected at load by some encoders and only when the pixels are first read by others. The test asserts the contract that holds either way: a truncated file produces no output. A truncated JPEG is still filled rather than rejected, matching the GD and Imagick editors.
The AVIF path was only covered by loading one and saving it as PNG. This saves as AVIF, checks the file really is one, and checks that the encoding effort defaults to 3 and that the filter is what the encoder is given.
The existing test rotated a square image 180 degrees and only checked that the size was unchanged, so a rotation that did nothing would have passed. This rotates a non-square image through every right angle and compares each corner of the result with the corner of the source it should have come from, which also covers the dimension swap a quarter turn has to make.
Only the horizontal axis was covered, and only by one corner. This checks both axes by comparing every corner with the corner it should have come from, on a fixture whose four corners are all different, so a flip on the wrong axis cannot pass.
The floor decides whether the editor is used at all, but reaching it through test() needs an older libvips than the one running the suite, so the comparison moves into its own method and is checked directly: 8.7, 8.12.1 and 8.14.5 are rejected, 8.15 and above are accepted.
The `keep` saver option that preserves the colour profile when metadata is
reduced only exists from libvips 8.15, and passing an unknown option fails the
save. Move the floor to 8.13 and pass `keep` only where it exists; below that the
fields are removed by hand, which was already happening below 8.16.

Restore the libvips 8.14 guard on the GIF interlace option, which the 8.15 floor
had made unnecessary.
Whether the `keep` mask can be passed, and whether the unprotected fields also
have to be removed by hand, were answered by two separate version checks: one in
_save(), one in get_save_options() while building the saver options.

get_metadata_strategy() now returns both answers for a version, so the policy
lives in one place. get_save_options() takes that strategy instead of a strip_meta
boolean, and _save() consults it for the removal.
@nickchomey

nickchomey commented Oct 12, 2026 •

Copy link
Copy Markdown
Author

@kirsky

Thanks for the detailed review. I've gone through all eight points; most are fixed now, and where I could I measured instead of taking the point on trust. A few I left out on purpose, and there are a couple of your claims I couldn't reproduce and would like your input on. Everything below was measured in Docker with libvips 8.16.1 unless I say otherwise.

What changed

# Point What I did
1 Min libvips version The floor is now 8.13. Below it the editor reports itself unsupported and wp_get_image_editor() falls back to Imagick or GD, so nothing regresses. 8.13 is the first version that can write a palette back to a PNG. I also tried to reproduce the 8.12.1 failure and couldn't; details below.
2 AVIF effort Set to 3, filterable with wp_image_editor_vips_avif_effort. I measured this rather than assumed it, see below.
3a JPEG optimize_coding Set to true. On a 33.8 MP source at Q82: 7,341,179 to 7,002,869 bytes, 4.6% smaller.
3b Resampling kernel Passed explicitly and filterable (wp_image_editor_vips_resampling_kernel). Measured, see below.
4 Metadata stripped on resize strip no longer drops everything. The editor keeps ICC, EXIF, XMP and IPTC, the same set Imagick keeps.
5 Damaged files accepted fail_on => truncated on load for every format except JPEG, which uses none so a damaged JPEG gets filled rather than rejected, matching Imagick.
6 image_save_progressive ignored Applied for JPEG, PNG and GIF. GIF interlace only exists from libvips 8.14, so that one is guarded on the runtime version.
7 Untrusted loaders An allowlist of raster image loaders, plus a format whose loader isn't allowed reporting as unsupported so selection falls through to Imagick.
8 No pixel limit A filterable cap on load, defaulting to 16383² (268,402,689 px). This one fails the image rather than falling back to Imagick, see below.

On the kernel (#3b). I ran a 28-case comparison of linear, lanczos2, lanczos3 and cubic against core's Imagick output, scored with SSIMULACRA2, SSIM, MS-SSIM, IW-SSIM, Butteraugli and CIEDE2000. lanczos3 had the best mean SSIMULACRA2 (7.28, against 1.38 for linear, 5.87 for cubic and 3.20 for Imagick) and led SSIM, MS-SSIM and IW-SSIM as well. It didn't lead everything: lanczos2 was marginally better on mean Butteraugli (7.421 against 7.427), and Imagick was better on mean CIEDE2000 (3.224 against 3.270).

It isn't free either. About 10% more bytes, and on the small images I tested, 10% more wall time. So the default is lanczos3, and if you'd rather have your linear, it's one filter away. I think this is appropriate because maintaining image quality should carry more weight than size and speed - if you turned it into a blurry mess in half the time, that wouldnt be desirable.

On AVIF effort (#2). You were right, and I measured it to be sure. First I confirmed that libvips really does default to effort 4: encoding with no effort option is byte-identical to passing effort = 4. Then, on a 33.8 MP source at Q82:

effort CPU s bytes
0 2.6 9,029,056
2 15.7 6,731,864
3 26.3 6,699,232
4 (default) 72.9 6,676,142
5 105.7 6,700,370

So the default does blow past a 30 s PHP-FPM limit, as you said, though I measured 73 CPU-seconds rather than 42, so the figure depends on the content and the machine. Effort 3 cuts that to 26 s for a 0.35% size cost, and above 4 more effort buys nothing (effort 5 came out slightly larger).

I've kept 3 as the default for now, and there are two things I'd still like your view on. Effort 3 is still only just under 30 s at 33.8 MP, so a larger image can still exceed the limit, and 2 may be the safer choice. And it applies on every AVIF save and stream, not only on a resize.

On truncated files (#5). I couldn't reproduce the PNG case. With libvips 8.16.1 a truncated PNG fails when the pixels are read whether or not fail_on is passed; the decoder raises "end of stream" either way. So fail_on isn't what protects PNG, and WebP behaves the same way. It's what protects GIF and JPEG, which read cleanly without it and fail with it.

That also exposed a weak spot in our own tests. The original test used a PNG, so it passed without the change. It now covers a truncated GIF, which fails if the load options are removed.

If you did see a sub-size produced from a truncated PNG, I'd like the details: the file, its size, and your libvips version. It may be a version difference worth understanding.

On loaders (#7). Confirmed, exactly as you described. An SVG renamed to .jpg is still decoded by svgload. The allowlist handles it.

On metadata (#4). I agree this is a cross-editor policy, and that's how I treated it. The kept set is Imagick's, decided in one place (get_metadata_strategy()), with the keep mask plus compensating removal before libvips 8.16. image_strip_meta still gates it.

I also measured the colour shift you predicted, and it depends on the image. For an sRGB-tagged image, dropping the profile is a no-op in the strict sense: the mean per-channel difference is 0.00, max 0, since interpreting sRGB values as sRGB changes nothing. For a genuine wide-gamut image the shift is real: converted to Display P3 and then de-tagged, the mean difference is 2.54 / 3.68 / 0.71 with a maximum of 81 out of 255. So keeping the profile is clearly right. Our fixture is sRGB-tagged, so test_resizing_keeps_the_color_profile asserts only that the ICC_PROFILE segment survives the resize; it could not catch a colour change, and the numbers above are what back the decision.

On the floor and the 8.12.1 failure (#1). I tried to reproduce this one. In ubuntu:22.04 (PHP 8.1.2, libvips 8.12.1-1build1) with php-ffi and php-imagick both loaded, a real load, resize and save succeeded, and Imagick still worked alongside it, with no "cannot register existing type 'VipsObject'" error. So I couldn't reproduce it in a single CLI process. I want to be careful about that, though. You reported it on uploads, which means under PHP-FPM across requests, and a CLI test can't rule that out. The error looks like a per-process GLib type-registration conflict, exactly the kind of thing FPM's lifecycle can expose and a one-shot CLI run may not. If you still have the failing environment, the exact reproduction would be worth having. Either way the floor is above 8.12.1, so that version is excluded regardless.

On the pixel cap (#8). Two corrections to how I first described it, both now fixed in the docblock. It isn't a format limit: a JPEG can describe 65500 pixels per side (verified: libvips encodes 17000- and 40000-pixel-wide JPEGs fine and only fails at 70000). And it isn't a limit the other editors share, since neither GD nor Imagick caps size at all. More importantly, it's a behaviour change rather than a pure guard. _wp_image_editor_choose() selects an editor before opening the file and never retries, and VIPS comes first in the default list, so an over-size image now fails to load where Imagick would have attempted it. I've kept the value you proposed, but if you'd rather it not become a hard failure, that's worth deciding now.

Deliberately left out. You mentioned more points beyond the list. If any of these are among them, I'd rather discuss than have you find them missing:

  • Alpha visibility threshold. Snapping near-transparent pixels to fully transparent changes output pixels, and GD and Imagick have no equivalent step.
  • Preserving animation. Core deliberately flattens animated GIFs into one frame when generating intermediate sizes (wp-includes/media.php), and Imagick selects the first frame to match (setIteratorIndex( 0 )). libvips loads the first frame by default, so this already matches. Preserving animation would move away from core, so it feels like a separate proposal.

Testing that surfaced a real gap: both existing GIF fixtures are single-frame, so the flattening behaviour was untested. There's now a two-frame fixture (tests/phpunit/data/images/gif-animated.gif) and a test asserting the resize and save write exactly one frame back.

Test coverage is up to 77 tests / 243 assertions / 1 skip, and now includes truncated PNG and GIF tests, an AVIF encode test, rotation correctness on non-square pixels, flip on both axes, the min-version gate and the metadata strategy.

I welcome any further feedback on the PR!

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.

2 participants