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 animated image data support as a built-in Image feature #1448

Closed
Xrayez opened this issue Sep 2, 2020 · 1 comment
Closed

Add animated image data support as a built-in Image feature #1448

Xrayez opened this issue Sep 2, 2020 · 1 comment

Comments

@Xrayez
Copy link
Contributor

Xrayez commented Sep 2, 2020

Describe the project you are working on:
Godot Engine. 🙂

This proposal is largely inspired by the discussion in my previous proposal regarding loading and importing animated images #1433, so this proposal can be considered as a possible alternative or a dependency to #1433, and exists for the entire purpose of discussion. Because this proposal goes against Solutions must be local, I don't expect this to be approved, but worth a shot.

Describe the problem or limitation you are having in your project:
To repeat #1433, Godot doesn't provide any kind of core data structures which would be useful as a container (and most importantly, a loader) for animated images, or rather image frames to make it agnostic to the purpose of being animated. Neither AnimatedTexture nor SpriteFrames is suitable for this task either because both have either number of frames limitation or lack of ability to set custom delay between frames, not to mention that adding various methods such as load and load_gif_from_buffer is out of scope for those classes, because they're usually imported rather than loaded.

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
While being able to load images via script is still a common denominator and motivation behind #1433 and this proposal, a concern was raised by @groud that adding a dedicated class (namely ImageFrames, previously referred to as AnimatedImage) just for loading animated image data is not necessary and could potentially bloat Godot, make it more confusing for beginners, introduce maintenance costs due to excess flexibility etc.

I propose that we add a way to deal with those issues by actually adding built-in support for a sequence of frames (with delay), directly in the Image class. This has the following benefits:

  • No new class is added to Godot (?).
  • Makes the API more intuitive (you just want to load an image, but it may be just animated one, like gif).
  • No need to create a dedicated ImageFramesLoader to replicate the functionality of ImageLoader, which is already quite specific to loading images compared to other format/resource loaders.
  • It does simplify implementing various resource importers which can reuse image loaders with animated formats for AnimatedTexture, SpriteFrames etc.
  • You could also import as Image as you currently can, but if the underlying format is animated image (gif, apng, webp), then the Image instance shall have those frames data already in-place.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
To make it less intrusive, you'd just add a property which would hold a dictionary/array of frames alongside regular image data. Image.get_data() and alike could return only the first frame (if an image does have those).

This is where we can add those loading methods:

var image = Image.new()
# Note that we already have specific methods for png, jpg etc.
image.load_gif_from_buffer(bytes)
image.load("res://godot.gif") # Would also handle png, jpg etc just fine.

Perhaps adding Image.is_animated() method would also be nice, which would just check whether the image has frames data associated with it, but again it's not necessary to add.

Then implement needed animated image loaders as ImageFormatLoader, in theory even without changing the API.

Similarly to #1433, we could then add AnimatedTexture.create_from_image() just like in ImageTexture. It would still work if the animated image has only 1 frame.

I believe no other changes would have to be done, except for some minor refactor of existing methods. Note that I do not propose providing a way to do image processing on a bulk of frames with the same Image API, I think this could be overkill (for Godot).

If this enhancement will not be used often, can it be worked around with a few lines of script?:
It's about extending the existing core Image class and various C++ loaders which are/can be implemented in the future, not sure if possible to implement via script and it would just defeat the purpose of this proposal.

Is there a reason why this should be core and not an add-on in the asset library?:
It's definitely not necessary to have this in core, consider approving #1433 which is easier to maintain and which has a higher chance of being backwards compatible, I just don't see other ways to expose animated image loading methods otherwise.

You may recall a pull request of mine which aimed at adding indexed image support: godotengine/godot#28013 (which is now part of Goost, by the way), and GIFs could be imported as ImageIndexed there, so perhaps it's better if this is maintained outside of Godot core by now. What do you think?

@Xrayez Xrayez changed the title Adding animated image data support as a built-in Image feature Add animated image data support as a built-in Image feature Sep 2, 2020
@Xrayez
Copy link
Contributor Author

Xrayez commented Apr 26, 2021

Closing due to the same reasons as in, #1433 (comment), since this proposal was created as an alternative as well.

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

No branches or pull requests

2 participants