[squeak-dev] [Discussion] GIF and Animated GIF Refactoring

Marcel Taeumel marcel.taeumel at hpi.de
Tue Dec 7 13:23:24 UTC 2021


Hi Eric --

I think this is works. I tested a bunch of GIFs ranging from a few KiB up to 4 MiB. All of them used to also load and play fine before.

I noticed that the loading times are about 5 times slower than before and that the GC runs runs longer. Hmm...

For that 4 MiB file it compares as follows:

BEFORE: '0.867 per second. 1.15 seconds per run. 3.71012 % GC time.' 
AFTER: '0.181 per second. 5.52 seconds per run. 58.18083 % GC time.' 

One can notice this slow-down especially in the FileList tool. Maybe we can make it faster by avoiding some GC time?

Best,
Marcel
Am 07.12.2021 13:41:13 schrieb Marcel Taeumel <marcel.taeumel at hpi.de>:
Hi Eric --

Thanks for doing this. I am looking at it now. Could it be that you dropped all "deleted" requests from the change set? There seems to be a lot of obsolete methods remaining in GIFReadWriter:

...
GIFReadWriter >> readBitData
GIFReadWriter >> writeCodeAndCheckCodeSize:
...

Most of that seems to be extracted to LzwGif(Decoder|Encoder).

I am trying to dissect, what I see. Maybe you could just "file-out" your version of GIFReadWriter and AnimatedImageMorph from the image directly?

Best,
Marcel
Am 04.12.2021 20:46:56 schrieb Eric Gade <eric.gade at gmail.com>:
Whoops, sorry all! The changeset was incorrect. Please use the revised changeset attached to this message. The content of the previous message still applies.


On Sat, Dec 4, 2021 at 1:18 PM Eric Gade <eric.gade at gmail.com [mailto:eric.gade at gmail.com]> wrote:

Hi Marcel (and others)


On Mon, Nov 29, 2021 at 5:45 AM Marcel Taeumel <marcel.taeumel at hpi.de [mailto:marcel.taeumel at hpi.de]> wrote:


Well, for the sake of backwards compatibility, maybe you could do instead:

1*) Improve GIFReadWriter while ensuring that the obviously public protocol stays functional. Empty out AnimatedGIFReadWriter but leave it there.
2*) Improve AnimatedImageMorph while ensuring that the obviously public protocol stays functional. No new class BetterAnimatedImageMorph.

Regarding the process:
- If single package: Inbox
- If multiple packages: Change set via squeak-dev

After some wrangling between Squot and Changesets (yikes!), I was able to come up with the following changeset (attached).
 

Here is what I did:
1) Updated AnimatedImageMorph to have the new/updated functionality from BetterAnimatedImageMorph, while maintaining the same methods and deprecating as needed;
2) Added the classes needed for new GIF parsing
3) Updated GIFReadWriter to use the new parsing methods
4) Deprecated the AnimatedGIFReadWriter class using the technique of adding #deprecated: to #basicNew
5) I have added a GIFReadWriterTest class that tests both the regular and animated GIF reading/writing at the same time. There are issues (see below).
 

If you would like to quickly test that things are working, do the following in a workspace:
```
GIFReadWriter exampleAnim.
(AnimatedImageMorph new fromGIFFileNamed: 'anim.gif') openInHand.
```

You should see an animated, growing red circle on a while background. See the class method comment.

Note on tests: I copied the GIFReadWriterTest class over from my Pharo implementation with minimal (file api related) modification. The tests are not passing and the reason is a bit confusing. Here's what's up: in these tests I write out a Form that has different colors in each quadrant, read it back in, and make sure the color values are the same. But apparently in Squeak when you use `FormCanvas fillColor: Color black`, the color that is put into the Form is not, in fact, equivalent to Color black. It is instead something like rgb(0,0,0.04). Is this intentional? If so, do you recommend that I simply update the tests to match on this specific color?

This is my first submission via changeset so definitely let me know if I messed up. Thanks!

--

Eric


--

Eric
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20211207/e0caf21e/attachment.html>


More information about the Squeak-dev mailing list