FIX: Do not downsize GIF images (PR #10989)

This is problematic because during this operation only the first frame is kept.

GitHub

@nbianca Should the same logic be applied to downsize_uploads/ShrinkUploadedImage?

Do we need both tests? Wouldn’t FastImage.animated? be enough?

I believe scale is still being used.

Is it a GIF or a JPG? :thinking:

Can we get away without those stubs?

    context "GIF image" do
      it "is not downsized" do

Yes, it is. FastImage.animated? returns nil for any non-GIF image.

I removed the remaining use of scale.

I did not want to add a GIF image to fixtures, but eventually did to get rid of the other stubs too.

oh @jbrw made this change https://github.com/discourse/discourse/pull/11031 do we need to redo this?

I think the bottom line was that animated? still needs a bit of work in fastimage

I am converting this back to a draft because it requires a new version of FastImage.

1 Like

My PR was merged and I wait for a new version of the FastImage gem to be released.

This breaks all animated webp files that will be converted to animation-less jpgs now.

FastImage can’t detect if a webp is animated, while the old ImageMagick based method was able to do it. LibVips also can detect it just fine.

Maybe we fallback to ImageMagick detection only when FastImage returns nil to animated?

1 Like

Ah, that is correct. FastImage should support detection for animated WEBPs, but until that happens I will readd the old detection method.

1 Like