Conversation
| the jpeg library is /jpegLib/ &\ | ||
| the png library is /pngLib/ { | ||
| the png library is /pngLib/ &\ | ||
| the gif library is /gifLib/ { |
There was a problem hiding this comment.
Not sure how much I like adding yet another dependency to image.folk. There isn't a good story for graceful degradation here.
There was a problem hiding this comment.
We could split it into independent handlers for gif/png/jpeg that all check if the path applies to them and return if not?
ppkn
left a comment
ppkn
left a comment
There was a problem hiding this comment.
This code is a lot shorter and cleaner than I expected. I left a couple of comments on things that may need improvement.
Right now there are two Wishes, one for still gifs and one for animated gifs. Wish $this displays gif "example.gif" actually delegates to Wish $this displays image "example.gif" when there is only 1 frame. I wonder if instead we invert that by checking the number of frames in the image.folk When and delegating to animated-gif.folk when the llength of frames is > 1. That way we have a consistent image user api.
| // Box the Image struct into a Tcl object | ||
| Jim_Obj *imObj; | ||
| $[$cc ret Image imObj im] | ||
| Jim_ListAppendElement(interp, framesList, imObj); |
There was a problem hiding this comment.
I am sort of uncomfortable with how little I understand this (especially line 31). It works but I'd like to understand the mechanisms behind $cc ret a bit better.
There was a problem hiding this comment.
This is a pretty deep cut -- I think I originally made ret and argtype purely for internal use in the C compiler library (to generate function prologues/epilogues). ret should generate Tcl code to turn Image into Jim_Obj (and assign it into imObj). It depends on the rtype already being defined in $cc, which happens when you extend imageLib.
Makefile
Outdated
| $< -I./vendor/jimtcl -I./vendor/tracy/public | ||
|
|
||
| vendor/gifdec/gifdec.o: vendor/gifdec/gifdec.c vendor/gifdec/gifdec.h | ||
| cc -c -O2 -g -fPIC -fno-omit-frame-pointer $(if $(ASAN_ENABLE),-fsanitize=address -fsanitize-recover=address,) -o$@ \ |
There was a problem hiding this comment.
I know for sure we need this fPIC flag, but I don't know how much Gemini cargo culted the rest of this compilation step.
|
Blinking probably a larger Folk issue. Maybe addressable with some use of |
|
(I don't think it's a blocker for this) |
|
I think you forgot to check in gifdec? I also think mentioning it in the Makefile along with core modules of Folk is not good practice -- would be better to build from source in gif-lib.folk and not mention it anywhere else. |
| uint16_t h = gif->height; | ||
| gd_close_gif(gif); | ||
|
|
||
| Jim_Obj *result = Jim_NewDictObj(interp, NULL, 0); |
There was a problem hiding this comment.
It would be more elegant to define this as a struct Gif and return that on the stack instead of having the Tcl encoding inline here, I think
i.e., just pass gifdec.c as a cflags into |
2 participants
I'm often asked if we support animated GIFs. This was a hurried attempt at adding GIF support.
Demos
Both of these demos show some blinking. I haven't diagnosed what is causing that.
Animated GIF
PXL_20260304_185939177.mp4
Remote Animated GIF
PXL_20260304_185958257.mp4