Repository navigation
Add an opt-in filter to generate animated image sub-sizes - #80385
adamsilverstein wants to merge 40 commits into
Conversation
Match WordPress core's server-side behavior, where both GD and Imagick flatten animated images when resizing and wp_calculate_image_srcset() keeps flattened sub-sizes and the animated full-size image from mixing. Loading all frames ([n=-1]) re-encoded a full animated GIF per uncropped sub-size, which took 16-47 seconds per size for a 769-frame GIF and produced sub-sizes larger than the original file (5.5MB medium from a 2.2MB source). Cropped sizes already flattened to the first frame, so behavior was inconsistent, and Media Library uploads taking the server path already produced static sub-sizes. See #80266.
… output mediabunny's default 2-second key frame cadence roughly doubles the output size for long GIF conversions (2.2MB vs 1.14MB for a 769-frame GIF) with no encode-time benefit. These looping, autoplaying GIF replacements don't need fine seek granularity. See #80266.
…size-performance # Conflicts: # packages/vips/CHANGELOG.md # packages/vips/src/index.ts # test/e2e/specs/editor/various/gif-to-video.spec.js
Add the wp_generate_animated_image_subsizes filter (boolean, default false). When a site opts in, uncropped sub-sizes of animated GIFs keep their animation instead of flattening to the first frame, resolving the long-standing request in https://core.trac.wordpress.org/ticket/28474 without depending on server-side Imagick availability. The flag travels the same path as image_strip_meta: REST API root index field -> block editor setting -> upload-media store -> vips worker, where it restores the pre-#80268 [n=-1] load path for uncropped resizes. When writing an animated GIF, gifsave is tuned (effort 2, interframe_maxerror 8, interpalette_maxerror 16), measured 4-8x faster than the defaults and avoiding sub-sizes larger than the original. See #80383.
|
Size Change: +311 B (0%) Total Size: 7.78 MB 📦 View Changed
|
|
Core backport of the server-side changes: WordPress/wordpress-develop#12572 (draft; Trac ticket to follow). |
|
Thanks for looking into this.
Before reviewing, we're now past Beta 1 and this sounds like a new feature to me. Should we be pursuing this, or focusing on bug fixing / polishing tasks? I.e. #80268 definitely sounds good to fix for the release, but I'm wondering where we should draw the line with the available time we have left for the release (and since we're both also trying to polish other features, too). Just feeling mindful of the time and attention we have to polish these things, but don't want to be a blocker needlessly of course! |
🤖 PR meta 🤖🎉 PropsIf you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. Updated as activity occurs, without notifying anyone named here. Add the 📦 Bundle sizeSize Change: +645 B (+0.01%) Total Size: 8.29 MB 📦 View Changed
⚡ PerformanceShow the resultsClient side metrics exclude the server response time. front-end-block-theme
front-end-classic-theme
media-processing
media-upload
post-editor
site-editor
|
…mment The `wp_generate_animated_image_subsizes` filter was tagged `@since 23.9.0`, a version already released; 24.0.0 is at RC, so the filter lands in 24.1.0. The 7.1 preload still described its field list as complete and required to match entities.js exactly. It no longer is: `generate_animated_image_subsizes` is spliced in by the 7.2 filter that runs after it. Say where the rest of the list comes from so the next field is added in the right place. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xhje3QeZwbKAvNQQcDRnjW
A site only reaches this path by enabling `wp_generate_animated_image_subsizes`, so quietly producing a static sub-size looks like the filter had no effect. Warn with the frame count, frame size, and the budget the estimate was measured against, so the degradation is visible and the constant is tunable against real uploads. Correct the two constants' docblocks while here. `BYTES_PER_PIXEL` claimed to cover vips' working buffers, but four bytes is exactly RGBA with no headroom; all of the margin comes from the budget sitting well under the heap size. `ANIMATION_MEMORY_BUDGET` compares against the fully materialized frame stack, which is an upper bound rather than an estimate: libvips streams the load/resize/save pipeline, so the check deliberately errs toward flattening. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xhje3QeZwbKAvNQQcDRnjW
… GIFs Raising `interpalette_maxerror` to 16 makes frames reuse the previous frame's palette far more often than libvips' default of 3. That only pays off when successive palettes are already close. When the colours genuinely shift, the reused palette no longer fits and the frame has to be dithered harder to compensate, costing both time and accuracy. Measured on a 48-frame colour-cycling GIF resized to 300px, 16 was ~50% slower than the default and more than doubled the mean per-pixel colour error, to save 1.7% of the file size. The win the tuning was added for comes from `effort: 2` (~7x faster on its own) and, marginally, from `interframe_maxerror: 8`; both are kept. Assert the option is absent so the value is not reintroduced as a size tweak without re-measuring the fidelity cost. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xhje3QeZwbKAvNQQcDRnjW
t-hamano
left a comment
There was a problem hiding this comment.
Looks good from my end.
- Entries in the changelog file should link to the PR and describe the actual changes this PR. introduces to the package.
- It is probably worth publishing a dev note.
…changelogs The entries linked to the issue rather than this pull request, which is what the rest of the changelogs reference, and described the feature rather than the interface each package gains. Name the new `generateAnimatedImageSubsizes` setting and `preserveAnimation` option, and state the cropped and memory-budget cases that flatten anyway. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RsWVvdyjpHJYwAp2xiGJGw
|
Got it, thanks @t-hamano - dev note makes sense, will do. |
|
Hey 👋 I wanted to give you a heads-up since this pull request is affected by recent validation changes for changelog files. #83043 adds additional validation for changelog files. You'll note that this pull request is currently failing a "Required changes from trunk" check. What you'll need to do: You will need to either rebase or merge the latest code from |
Both entries sat under published version headings, so the changelog structure validator failed: an entry for an unmerged PR must appear under `## Unreleased`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bk4AjYTiZ35zVfzYXurfx5
|
I did some manual testing on this and it worked as expected. With the filter returning true, I uploaded a sample gif and was able to see the medium and large subsizes were also animated gifs, while the cropped thumbnail variation was not animated (as expected). I posted a simple test plugin that lets you toggle the setting as a gist: https://gist.github.com/adamsilverstein/16c35549d26024dd7266630fbd801442 |
…ubsizes-optin # Conflicts: # packages/upload-media/CHANGELOG.md # packages/vips/CHANGELOG.md
Trunk's Vitest setup now fails any test that lets console.warn through without an explicit expectation, so the memory-budget fallback test asserts the warning through `toHaveWarnedWith` instead of a manual spy. Gutenberg 24.1.0 shipped while this branch was open, so the filter's @SInCE moves to 24.2.0. Claude-Session: https://claude.ai/code/session_018uRsBsDb9t3piAjyWaycCo
|
Thanks @t-hamano. The changelog entries in For the dev note, I put a proposed draft up as a gist so it can be reviewed before it goes in the field guide: https://gist.github.com/adamsilverstein/409f5cba3e689b7977c7eb31042cc5a5 The draft came out of a session with Claude Code, the gist of it:
Does that cover what you had in mind? |
t-hamano
left a comment
There was a problem hiding this comment.
@adamsilverstein, thanks for the update!
I'm not very familiar with this feature, but it looks good to me overall. @swissspidy @andrewserong, could you take another look if you have the bandwidth?
| // @see lib/compat/wordpress-7.2/preload.php | ||
| _fields: [ | ||
| 'description', | ||
| 'generate_animated_image_subsizes', |
There was a problem hiding this comment.
Should this field also be added to the templates generated by wp-build?
There was a problem hiding this comment.
Good call, added to both page templates (plus a wp-build changelog entry) in 1a1654e.
andrewserong
left a comment
There was a problem hiding this comment.
This is testing great for me! Nothing to note other than what @t-hamano has already mentioned. I jotted down a few comments while I was reviewing, but they're all nits and non-blocking 🙂
In the happy path, with common GIFs the preservation of animation succeeds, and only the cropped thumbnail is a still image:
2026-10-06.12.19.09.mp4
With enormous GIFs (like those I record using Gifox for screen recordings) the conversion is correctly bailed and logged out to the console:
And, of course, without the filter active, the behaviour as on trunk is preserved.
It's a neat feature — one idea for beyond 7.2 could be, if we feel like it's been working nice and stable for a release, we could always switch the default to it being switched on and/or add a block editor UI setting to allow users to toggle the feature without needing to use a filter.
For now, though, I like that this is shipped behind the filter as it feels like a safe way to roll it out and do any tweaks that might be needed as folks start to use it.
LGTM! 🚀
| /** | ||
| * Adds the `generate_animated_image_subsizes` field to the preloaded root REST index. | ||
| * | ||
| * The preloaded `_fields` list has to match the one requested by | ||
| * `packages/core-data/src/entities.js` exactly, same fields in the same order, or the | ||
| * preloaded response is discarded and the editor requests the index again. | ||
| * | ||
| * @since 7.2.0 | ||
| * | ||
| * @param array $paths REST API paths to preload. | ||
| * @return array Filtered preload paths. | ||
| */ | ||
| function gutenberg_block_editor_preload_paths_7_2( $paths ) { |
There was a problem hiding this comment.
Would it be simpler to just remove the wordpress-7.1/preload.php file, copy and paste its contents to this file and make the updates required to add in the generate_animated_image_subsizes value?
There was a problem hiding this comment.
That approach makes sense to me 👍
There was a problem hiding this comment.
Done in 1a72935 - the 7.1 file is gone and the 7.2 filter now owns the complete field list.
| * `height` is the full "toilet roll" height of an image loaded with | ||
| * `[n=-1]`, so `height / pageHeight` is the frame count, which drives | ||
| * the animation memory estimate. |
There was a problem hiding this comment.
LOL at "toilet roll" height. Though I suppose it's an apt metaphor for it!
| /** | ||
| * Maximum inter-palette error for palette reuse. | ||
| * | ||
| * Frames whose palette is within this distance of the previous frame's | ||
| * reuse it, avoiding a costly palette recomputation per frame. | ||
| * Only used by gifsave; do not provide for any other type! | ||
| */ | ||
| interpalette_maxerror?: number; |
There was a problem hiding this comment.
From the code comments it seems we're just using the default value for now. Is it still worth including this type for the future, or is it redundant for now?
| // Not an exact match against the source: the sub-size is written | ||
| // by cgifsave with inter-frame and inter-palette error tolerances, |
There was a problem hiding this comment.
Such a tiny nit (feel free to ignore!) but it seems we're using the default for inter-palette, right?
| | Filter | Where the value is used | | ||
| | ---------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | `image_editor_output_format` | Builds the per-MIME output format map applied by the client. | | ||
| | `wp_editor_set_quality` / `jpeg_quality` | Resolved per registered size into the upload response's `image_quality` field. | | ||
| | `big_image_size_threshold` | Shipped to the client via the REST index; scaling happens in the browser. | | ||
| | `image_save_progressive` | Read at upload time; the client applies progressive/interlaced encoding accordingly. | | ||
| | `image_strip_meta` | Exported on the REST index; when it returns `false`, the client keeps all metadata on generated images. | | ||
| | `image_max_bit_depth` | Exported on the REST index; the client caps the output bit depth of generated images accordingly. | | ||
| | `wp_generate_animated_image_subsizes` | Exported on the REST index; when it returns `true`, uncropped sub-sizes of animated images keep their animation instead of flattening to the first frame. | | ||
| | `intermediate_image_sizes` / `intermediate_image_sizes_advanced` | Consumed when computing registered sizes and `missing_image_sizes`. | |
There was a problem hiding this comment.
Very nitpicky comment here for the two .md file changes, feel free to ignore!
This PR changes the whitespace for the tables in these markdown files so that everything lines up visually in a text editor. However, I find the readability in the diff a little awkward as it means we need to update the whitespace any time we make changes to the tables.
Is it worth preserving what we had in trunk instead, which is that each column is just the size of its text? (I haven't checked what we're doing elsewhere, just noticed that this PR's diffs for these files seems bigger than it needs to be).
There was a problem hiding this comment.
yes good point, the spacing shouldn't be needed.
There was a problem hiding this comment.
Restored trunk's table formatting in 5553886.
Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com>
The fallback for an animation whose later frame cannot be decoded was silent, so a site that opted in would see a static sub-size with no hint why. Log it like the memory-budget fallback, and cover both the retry and the rethrow when animation is not being preserved. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MqJy7qoG31Ms7amXYtNN5K
The tuned gifsave settings leave inter-palette error at the libvips default, so the type had no caller. Also correct the e2e comment that still described an inter-palette tolerance. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MqJy7qoG31Ms7amXYtNN5K
Padding every table column to align in a text editor turned a few added rows into a rewrite of each table. Keep trunk's compact cells so the diff shows only the new content. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MqJy7qoG31Ms7amXYtNN5K
Splicing the new field into the 7.1 list from a second filter needed ordering and anchor logic to keep the list matching entities.js. One filter that owns the complete list is simpler and keeps a single place to update. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MqJy7qoG31Ms7amXYtNN5K
core-data now requests generate_animated_image_subsizes on the root index, so the page templates' preload path has to list it too or the preload is never consumed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MqJy7qoG31Ms7amXYtNN5K
andrewserong
left a comment
There was a problem hiding this comment.
Thanks for the updates, just gave it another quick smoke test again, and still testing well for me with and without the filter! ![]()
What?
Adds a developer opt-in filter,
wp_generate_animated_image_subsizes, that re-enables animated (multi-frame) sub-size generation for animated GIFs in the client-side media processing pipeline:Fixes #80383.
Also fixes the 12-year-old core request Trac #28474 - "WordPress destroys animation in animated GIF when it resizes", as proposed in comment:58.
Core backport: Trac #65656 / WordPress/wordpress-develop#12572.
Note
Stacked on #80268, which makes static first-frame sub-sizes the default. This PR adds the opt-in path back on top of it. Only the last commit is new; review the diff from
fix/animated-gif-subsize-performance.Why?
#80268 switched sub-sizes of animated GIFs to static first-frame images, matching what WordPress core has always done server-side, because full animated re-encodes were extremely expensive (~88 s combined for a 769-frame GIF, with sub-sizes larger than the original - see #80266).
But there is long-standing, sustained demand for resized GIFs that keep their animation: Trac #28474 has been open since 2014 and stalled server-side because GD cannot do it and Imagick is only available on a subset of hosts. Client-side processing sidesteps both constraints - the cost is paid once in the uploading user's browser, and wasm-vips is available regardless of host configuration. That makes animated sub-sizes reasonable as an explicit developer opt-in while keeping the fast, core-consistent static behavior as the default.
How?
The flag follows the exact same path as
image_strip_meta/image_max_bit_depth(#80218):lib/media/load.php):wp_generate_animated_image_subsizes(boolean, defaultfalse) is applied ingutenberg_media_processing_filter_rest_index()and exposed asanimated_image_subsizeson the REST API root index. The field is also added to the preload/entities field lists (lib/compat/wordpress-7.1/preload.php,packages/core-data/src/entities.js), which must match exactly.generateAnimatedImageSubsizessetting inuse-block-editor-settings.jsand forwarded byuse-media-upload-settings.js.resizeCropItemreads the setting from the@wordpress/upload-mediastore and passes it to the vips worker as a newpreserveAnimationoption onresizeImage()(options object from Client Side Media: Consolidate optional positional params into options objects in vips / upload-media #80328).preserveAnimationis set and the resize is uncropped,resizeImage()restores the pre-Client-side media: generate animated image sub-sizes from the first frame only, matching core #80268[n=-1]load path so all frames are decoded and re-encoded. Cropped sizes (e.g.thumbnail) always flatten to the first frame, matching the pre-existing behavior - per-frame smart-cropping is out of scope.Tuned
gifsavesettingsThe profiling in #80266 showed ~85% of the animated resize cost is GIF re-encoding (per-frame palette quantization), and that default
gifsavesettings cause the output-larger-than-input bloat. When writing an animated GIF, this PR applies the settings benchmarked there:effort: 2,interframe_maxerror: 8,interpalette_maxerror: 16- measured 4-8x faster and eliminates the size bloat.That turns the opt-in cost from ~88 s into roughly 10-20 s for a very large GIF - still too slow to be the default, but a reasonable trade for a site that has explicitly chosen animated sub-sizes.
Notes
wp_calculate_image_srcset()still never mixes the full-size GIF and its sub-sizes in onesrcset. With animated sub-sizes that guard becomes overly conservative but harmless; relaxing it is a server-side follow-up.Testing Instructions
add_filter( 'wp_generate_animated_image_subsizes', '__return_true' );medium/largesub-sizes (e.g. via/wp-json/wp/v2/media/<id>): they should be animated GIFs (multiple frames), whilethumbnail(cropped) remains a static first frame.Documentation
The client-side media docs (#75895) are updated alongside: the how-to guide gains an "Animated image sub-sizes" section for the new filter, and the architecture reference adds it to the filter table and REST index field list (plus corrects the now-stale note that sub-sizes preserve all frames).
Automated tests
packages/vips/src/test/resize-image.ts: newpreserveAnimationsuite -[n=-1]+ tuned gifsave for uncropped animated resizes, first-frame flattening for crops, no effect on still formats.phpunit/media/media-processing-test.php: REST index exposesgenerate_animated_image_subsizes(defaultfalse, honors the filter, hidden withoutupload_files).test/e2e/specs/editor/various/gif-to-video.spec.js: new test uploads an animated GIF with the filter enabled (via a new e2e test plugin) and asserts themediumsub-size keeps all frames.