swissspidy / media-experiments

WordPress media experiments
GNU General Public License v2.0
84 stars 1 forks source link

Keep animation of GIF thumbnails #285

Open swissspidy opened 9 months ago

swissspidy commented 9 months ago

If they are not converted to MP4s, ensure the animations are kept for client-side thumbnails.

Might be a non-issue if output format is unchanged, but needs some testing.

See https://core.trac.wordpress.org/ticket/28474

swissspidy commented 5 months ago

Turns out vips supports scaling down animated images just fine (if you configure it), but it doesn't support cropping.

See https://github.com/libvips/libvips/issues/2668

I think that actually makes sense for us.

A 150x150 thumb of a dancing banana doesn't look great if half of it is cut off. But for a simple resize (e.g. for srcset) we can and should retain the frames.

swissspidy commented 5 months ago

To-do:

swissspidy commented 5 months ago

There's a remaining bug in resizeImage in the vips package, where the GIF is loaded with n=-1 even when the image needs to be hard-cropped. The result is a crop in-between the pages/frames:

nyancat-256x256-1-150x150

Only need to add n=-1 to loadOptions and strOptions if it's not a hard crop.

Related: if doing a 150x150 hard crop of a 256x256 gif (which basically means it could just resize it down) — should it still crop (which means losing the animation) or just scale down and thus allow retaining the animation? Or aren't they doing the same anyway?

swissspidy commented 5 months ago

To-do:

swissspidy commented 5 months ago

if doing a 150x150 hard crop of a 256x256 gif (which basically means it could just resize it down) — should it still crop (explore)

Tricky to do because need to know the dimension, but need to load all frames beforehand.

Expand resizeImage unit tests to cover animation use case

There are more tests that can be added there, for example for the hasTransparency function which doesn't have any coverage yet (add e2e too?)

swissspidy commented 5 months ago

Fix crop issue with multiple pages

To-do: Add snapshot test