[squeak-dev] [Discussion] GIF and Animated GIF Refactoring
Christoph.Thiede at student.hpi.uni-potsdam.de
Sat Nov 27 19:12:05 UTC 2021
I cannot speak for the community, but I am very glad that you are addressing this domain - GIFs, especially animated ones, are a great application and demonstration of the Morphic values, and there have been several issues with the old implementation mentioned on this list during the last years. I have been using AnimatedImageMorph myself recently for TelegramBot and was a bit sad about how many GIFs could not be processed properly. I have tried out your changes and they look very beautiful, most impressively #animatedImageMorphComparison!
Here is a naive list of things that need to be done to get your changes into the Trunk:
* compatibility checks - are there any breaking changes in terms of behavior, or has the public protocol been changed? Maybe it would be worth improving compatibility, adding a few "deprecated" methods, autc. For instance, probably we should re-add an empty subclass AnimatedGIFReadWriter in the 60Deprecated/61Deprecated package to not break existing code that relies on it. Another example, #openGIFInWindow:/#fromStream: are not understood by BetterAnimatedImageMorph class.
* release scheduling - this looks like a bigger (though very valuable) change. We should discuss whether we want to integrate the patch before or after the upcoming 6.0 release.
* code review - not sure whether there is anyone who would like to have a detailed look at your patch before it is integrated. But as you say it is "tested by the crowd" on two different Smalltalk systems, my personal opinion would be that this should not be a reason to block great contributions.
@all trunk maintainers: What do you think? Did I miss any other step? :-)
PS: Have you or someone else already tried to optimize the new GIFReadWriter for performance? I am having a 36 MB large GIF<https://github.com/LinqLover/TelegramSmalltalkBot/blob/master/img/screencast.gif> here which takes eternities (> 10 minutes) to file in on my machine. With a 9 MB small GIF, it still takes a few minutes. And the RAM consumption of my image is noticeably high. But after that, I can play the GIF with ca. 25 FPS. However, the old implementation was not faster either.
Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Eric Gade <eric.gade at gmail.com>
Gesendet: Samstag, 27. November 2021 19:02 Uhr
An: The general-purpose Squeak developers list
Betreff: [squeak-dev] [Discussion] GIF and Animated GIF Refactoring
I am responsible for the re-implementation of GIFReadWriter in Pharo from a few years ago (see this PR<https://github.com/pharo-project/pharo/pull/1666>). Earlier this week I was able to successfully port most of that work to Cuis, so I thought I'd take a stab at Squeak too.
Unlike Pharo and Cuis, however, Squeak already has a class `AnimatedImageMorph`. I wanted to discuss my proposed changes here for this reason, prior to submitting any formal changes to the inbox.
So here's what I've done so far:
First, as with Pharo and Cuis, `GIFReadWriter` has been completely refactored to have a more clear and more complete implementation of the GIF format. This includes separating out the compression activity into two helper classes (`LzwGifEncoder` and `LzwGifDecoder`), as well as completely removing the existing `AnimatedGIFReadWriter` class. I have also added the `AnimatedImageFrame` class for composing GIF frame information that can be used by other classes -- both for writing and for displaying.
Second, I have refactored the existing `AnimatedImageMorph` in Squeak to better deal with displaying real-life animated GIFs. Prior to this, many GIFs would display incorrectly due to the nature of GIF frame compositing and disposal. All of the test GIFs I've tried so far (and there have been a lot) seem to be working well with this class.
Third, I have created an alternative class -- presumptuously named `BetterAnimatedImageMorph` -- that has some optimizations. The regular `AnimatedImageMorph` does the GIF frame/form compositing "on the fly," which results in a lot of extra computation each time the animation loops. The upshot here is that we cannot achieve real framerates for some GIFs -- they always play slower in Squeak than they would in a browser regardless of how low one sets the stepTime/delay. `BetterAnimatedImageMorph` takes a different approach: it loops through all the GIFReadWriter's read-in frames and first creates a collection of fully composited Forms. It then simply changes the current form to be displayed at each step. In my tests this has resulted in much more performant playback, of which I have provided examples (see below).
One issue with `BetterAnimatedImageMorph` is that the original information about the constituent forms of the GIF is "lost". Attempting to write it back to a GIF file will necessarily result in a larger file than the original, because most animated GIFs are made of frames that are "diffed" in a sense. There are a couple of ways to deal with this though, such as caching the original frame data etc.
My concrete proposals, then, would be the following:
1. Remove AnimatedGIFReadWriter and replace GIFReadWriter with my updated version (along with helper classes)
2. Replace AnimatedImageMorph with my new BetterAnimatedImageMorph
If you've read this far and have the patience to try it out, I have my working code up online at https://gitlab.com/darth-cheney/squeak-animated-gif
Load this code in Squot and check it out. Try some of the examples in the FunGIFExamples class (on the class side). In particular, any class method starting with `example` should be good fun.
For a live comparison of the performance between my `BetterAnimatedImageMorph` and the current `AnimatedImageMorph`, do `FunGIFExamples animatedImageMorphComparison` and you should be able to see the issue right away.
Any feedback is greatly appreciated here, and hopefully we can get this or something like it into the main image. GIFs are too fun to leave out!
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Squeak-dev