Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support GIF image decoder and animation #40

Merged
merged 1 commit into from
Sep 11, 2024
Merged

Conversation

ndsl7109256
Copy link
Collaborator

@ndsl7109256 ndsl7109256 commented Aug 29, 2024

To enable support for GIF animations, I have extended the twin_pixmap_t
structure. The twin_pixmap_t now includes a flag to determine if the
image is animated, along with a new structure, twin_animation_t, to
manage the frames and timing information. I have also integrated an API
with gifdec [1] to load, manage and display the twin_animation_t object.

In addition, I have added a new application to demonstrate the
capabilities of twin_pixmap_t, supporting both GIF animations and static
images.

[1] https://github.com/lecram/gifdec

Config the new image app

$ make config
截圖 2024-09-03 晚上10 14 36

Build and run sample Mado program
demo

[1] https://github.com/lecram/gifdec

@jserv
Copy link
Contributor

jserv commented Aug 29, 2024

You need to ensure compatibility so that GIF images can be manipulated as ordinary twin_pixmap_t objects, rather than being limited to an animation-only rendering path.

Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that gifdec is in the public domain, its source code can be safely integrated into src/image-gif.c and modified to align with the style guidelines in the CONTRIBUTING.md file. Be sure to retain credit, acknowledging that the GIF decoder was developed by Marcel Rodrigues. Additionally, verify that src/image-gif.c is included in the group of src/image-*.c files in the Makefile.

apps/animation.c Outdated Show resolved Hide resolved
src/gif.c Outdated Show resolved Hide resolved
@jserv jserv changed the title Support gif animation Support GIF image decoder and animation Aug 31, 2024
apps/apps_image.h Outdated Show resolved Hide resolved
Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the content of src/gif.c into src/image-gif.c and enforce the consistent style.

@ndsl7109256
Copy link
Collaborator Author

Move the content of src/gif.c into src/image-gif.c and enforce the consistent style.

Does this mean that we need to modify all the naming style in gifdec to match that used in Mado, such as changing gd_GIF to twin_GIF?

@jserv
Copy link
Contributor

jserv commented Sep 4, 2024

Does this mean that we need to modify all the naming style in gifdec to match that used in Mado, such as changing gd_GIF to twin_GIF?

Since GIF decoding is quite straightforward, it should be integrated in the consistent manner.

apps/animation.c Show resolved Hide resolved
include/twin.h Show resolved Hide resolved
src/image-gif.c Outdated Show resolved Hide resolved
src/image-gif.c Outdated Show resolved Hide resolved
include/twin.h Outdated Show resolved Hide resolved
src/image-gif.c Outdated Show resolved Hide resolved
src/image-gif.c Outdated Show resolved Hide resolved
src/image-gif.c Outdated Show resolved Hide resolved
src/image-gif.c Outdated Show resolved Hide resolved
src/image-gif.c Outdated Show resolved Hide resolved
src/image-gif.c Outdated Show resolved Hide resolved
Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decouple the animation APIs and GIF animation provider.

The animation-specific functions, such as twin_animation_from_file and twin_animation_get_current_frame, should be implemented outside of image-gif.c, which acts as the provider of the animation. A dedicated C source file, such as src/image.c, should be used to handle the animation interface and the corresponding state machine.

src/image-gif.c Outdated Show resolved Hide resolved
void apps_animation_start(twin_screen_t *screen, const char *path, int x, int y)
{
twin_pixmap_t *pix = twin_pixmap_from_file(path, TWIN_ARGB32);
twin_toplevel_t *toplevel =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid specifying window title to the full path of given GIF file. Instead, use fixed names such as "Viewer."

apps/animation.c Outdated Show resolved Hide resolved
include/twin.h Outdated
@@ -692,6 +709,19 @@ void twin_icon_draw(twin_pixmap_t *pixmap,

twin_pixmap_t *twin_pixmap_from_file(const char *path, twin_format_t fmt);

/*
* image-gif.c
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refine this comment for dedicated image manipulation interface file.

You can follow the idea how GdkPixbuf : PixbufAnimation deals with the iterator.

In addition, you should give more comments for each function introduced for the animation.

apps/animation.c Outdated Show resolved Hide resolved
@ndsl7109256
Copy link
Collaborator Author

Decouple the animation APIs and GIF animation provider.

The animation-specific functions, such as twin_animation_from_file and twin_animation_get_current_frame, should be implemented outside of image-gif.c, which acts as the provider of the animation. A dedicated C source file, such as src/image.c, should be used to handle the animation interface and the corresponding state machine.

Maybe we could dispose of twin_animation_from_file, since we now use twin_pixmap_from_file to load animations, and for GIFs, we would call _twin_gif_to_pixmap and we could call specific gif helper _twin_animation_from_gif_file to convert gif to twin_animation_t instead of the interface twin_animation_from_file.

Makefile Outdated Show resolved Hide resolved
@jserv
Copy link
Contributor

jserv commented Sep 9, 2024

Maybe we could dispose of twin_animation_from_file, since we now use twin_pixmap_from_file to load animations, and for GIFs, we would call _twin_gif_to_pixmap and we could call specific gif helper _twin_animation_from_gif_file to convert gif to twin_animation_t instead of the interface twin_animation_from_file.

It makes sense. Attempt to align with GDK's manipulation in a much simpler way.

src/animation.c Outdated Show resolved Hide resolved
Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refine git commit messages. The subject should reflect what you have done along with the revised body for animation API.

To enable support for GIF animations, I have extended the twin_pixmap_t
structure. The twin_pixmap_t now includes a new pointer to the new
twin_animation_t structure, which manages multiple frames and their
timing information for animated image.

To manipulate twin_animation_t, the following APIs have been added.
- twin_animation_get_current_delay : Retrieves the display duration of
the current frame.
- twin_animation_get_current_frame : Obtains the current frame for
display.
- twin_animation_advance_frame : Advances the animation to the next
frame. If the animation is looping, it will return to the first frame
after the last one.
- twin_animation_destroy : Frees the memory allocated for the animation,
including all associated frames.

To show the new features, a demo application has been developed. This
application supports both animated images and static images, and serves
as a practical example of how to utilize the new animation capabilities.
@jserv jserv merged commit 9ba5b51 into sysprog21:main Sep 11, 2024
3 checks passed
@jserv
Copy link
Contributor

jserv commented Sep 11, 2024

Thank @ndsl7109256 for contributing!

@jserv jserv mentioned this pull request Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants