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

Add GIF support for loading image frames and import #8

Merged
merged 11 commits into from
Sep 7, 2020
Merged

Add GIF support for loading image frames and import #8

merged 11 commits into from
Sep 7, 2020

Conversation

Xrayez
Copy link
Contributor

@Xrayez Xrayez commented Aug 30, 2020

Co-Authored-By: @Daw11, as originally implemented for Godot in godotengine/godot#31831.

Notable changes:

Note that some changes are currently governed by Godot development requirements. This PR is created in hopes that it can be reviewed (and hopefully approved), so that these features can have a chance to be merged directly in Godot for the master branch (and perhaps backported to 3.2), else these features are already quite functional and can be tested, and eventually merged to Goost.

You can test this out by compiling this PR with the plain scons command.

Godot proposal: godotengine/godot-proposals#1433.

Co-Authored-By: Davide Busterna <davidebusterna@gmail.com>
This makes the API to reflect `ImageLoader` class by introducing `ImageFramesLoader`.

`AnimatedTexture` is implemented as a dedicated importer
handling `ImageFrames` loaded from `ImageFramesLoader`,
similarly to `Image`.
@Xrayez
Copy link
Contributor Author

Xrayez commented Sep 6, 2020

See godotengine/godot-proposals#1433 (comment).

This is mostly completed, image frames loading works and successfully importing as AnimatedTexture now. Note that current implementation is a self-contained module. The classes needed to cover other animated images formats demand more complete solution: implemented classes like ImageFrames, ImageFramesLoader, ImageFramesFormatLoader etc. are mostly agnostic to the animated image format, so these classes can be considered core and can be moved to core/ if someone wants to implement other animated image loaders, but for now it's better that those classes remain closer to a module so other people can compile this module without any dependencies.

Xrayez and others added 4 commits September 7, 2020 15:52
Co-Authored-By: Davide Busterna <davidebusterna@gmail.com>
`GifSaver` can be implemented in the future, if needed.

Also renamed `_load_gif` to `load_gif_func` for consistency with `Image`.
For consistency with `AnimatedTexture` API.
Sligthly different implementation in relation to refactored API.
Mostly the imported data is already compatible for it to be loaded
as `SpriteFrames` from the `ImageFramesLoader`.

This can be used in `AnimatedSprite` nodes (drag-n-drop conversion from
`SpriteFrames` to `AnimatedSprite` is still not implemented).

Co-Authored-By: Davide Busterna <davidebusterna@gmail.com>
@Xrayez
Copy link
Contributor Author

Xrayez commented Sep 7, 2020

Re-implemented SpriteFrames importer, so should be more up-to-notch now compared to godotengine/godot#31831.

The lack of decision making regarding godotengine/godot-proposals#1433 shouldn't block this PR, I cannot possibly know for sure how long it would take for the proposal to get reviewed/approved/rejected, there are no deadlines so this could take infinitely long to wait for, so lets just test the implementation "in production" and see.

Credits: I've added @Daw11 as a co-author (means that changes will be reflected in the actual git history) and hope that it's alright for these features to be used by other people considering the amount of 👍 the original PR accumulated over a year (!), so I'm going to merge this now rather than wait another year to pass. I'm inviting @Daw11 as a collaborator in case he'd like to maintain the existing and/or implement other animated image loaders, thanks for working on this! I hope my feedback was also useful in godotengine/godot#31831 the time you worked on that PR. Also added to AUTHORS.md, cheers! ❤️

Context: @Daw11 hasn't responded to any messages from the moment he has completed all the planned changes for godotengine/godot#31831 (comment), so the PR considered abandoned.

@sairam4123
Copy link

Implement APNG too, #193

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants