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 load_animation and AnimatedSurface #2218

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

yunline
Copy link
Contributor

@yunline yunline commented Jun 1, 2023

Implementd image.load_animation() and some basic functions of pygame.AnimatedSurface
Issues: #1054 #2181

This PR requires a SDL_image 2.6.0+
You can build it by your self or use my pre-built version.

Pre-built SDL_image

SDL2_image-2.7.0-pygame-ce.zip

You may also need to use this customized Setup file

#This Setup file is used by the setup.py script to configure the
#python extensions. You will likely use the "config.py" which will
#build a correct Setup file for you based on your system settings.
#If not, the format is simple enough to edit by hand. First change
#the needed commandline flags for each dependency, then comment out
#any unavailable optional modules in the first optional section.


SDL = -Iprebuilt-x64/SDL2-2.26.4/include -Lprebuilt-x64/SDL2-2.26.4/lib/x64  -lSDL2
FONT = -Iprebuilt-x64/SDL2_ttf-2.20.2/include -Lprebuilt-x64/SDL2_ttf-2.20.2/lib/x64  -lSDL2_ttf
IMAGE = -Iprebuilt-x64/SDL2_image-2.7.0-pygame-ce/include -Lprebuilt-x64/SDL2_image-2.7.0-pygame-ce/lib/x64  -lSDL2_image
MIXER = -Iprebuilt-x64/SDL2_mixer-2.6.2/include -Lprebuilt-x64/SDL2_mixer-2.6.2/lib/x64  -lSDL2_mixer
PORTMIDI = -Iprebuilt-x64/include -Lprebuilt-x64/lib  -lportmidi
PORTTIME = 
FREETYPE = -Iprebuilt-x64/include -Lprebuilt-x64/lib  -lfreetype
PNG = -Iprebuilt-x64/include -Lprebuilt-x64/SDL2_image-2.7.0-pygame-ce/lib/x64  -llibpng16
SCRAP = -luser32 -lgdi32
COPYLIB_SDL2 -Lprebuilt-x64/SDL2-2.26.4/lib/x64/SDL2.dll
COPYLIB_SDL2_ttf -lSDL -Lprebuilt-x64/SDL2_ttf-2.20.2/lib/x64/SDL2_ttf.dll
COPYLIB_SDL2_image -lSDL -ljpeg -lpng -ltiff -Lprebuilt-x64/SDL2_image-2.7.0-pygame-ce/lib/x64/SDL2_image.dll
COPYLIB_SDL2_mixer -lSDL -Lprebuilt-x64/SDL2_mixer-2.6.2/lib/x64/SDL2_mixer.dll
COPYLIB_portmidi -Lprebuilt-x64/lib/portmidi.dll
COPYLIB_freetype -Lprebuilt-x64/lib/freetype.dll
COPYLIB_png -lz -Lprebuilt-x64/SDL2_image-2.7.0-pygame-ce/lib/x64/libpng16.dll
COPYLIB_jpeg = -I.
COPYLIB_ogg -Lprebuilt-x64/SDL2_mixer-2.6.2/lib/x64/optional/libogg-0.dll
COPYLIB_modplug -Lprebuilt-x64/SDL2_mixer-2.6.2/lib/x64/optional/libmodplug-1.dll
COPYLIB_opus -Lprebuilt-x64/SDL2_mixer-2.6.2/lib/x64/optional/libopus-0.dll
COPYLIB_opusfile -Lprebuilt-x64/SDL2_mixer-2.6.2/lib/x64/optional/libopusfile-0.dll
COPYLIB_tiff -ljpeg -lz -Lprebuilt-x64/SDL2_image-2.7.0-pygame-ce/lib/x64/tiff.dll
COPYLIB_z -Lprebuilt-x64/SDL2_image-2.7.0-pygame-ce/lib/x64/zlib1.dll
COPYLIB_webp = -I.

DEBUG =

#the following modules are optional. you will want to compile
#everything you can, but you can ignore ones you don't have
#dependencies for, just comment them out

imageext src_c/imageext.c $(SDL) $(IMAGE) $(PNG) $(DEBUG)
font src_c/font.c $(SDL) $(FONT) $(DEBUG)
mixer src_c/mixer.c $(SDL) $(MIXER) $(DEBUG)
mixer_music src_c/music.c $(SDL) $(MIXER) $(DEBUG)
scrap src_c/scrap.c $(SDL) $(SCRAP) $(DEBUG)
pypm src_c/pypm.c $(SDL) $(PORTMIDI) $(PORTTIME) $(DEBUG)

_sdl2.sdl2 src_c/_sdl2/sdl2.c $(SDL) $(DEBUG) -Isrc_c
_sdl2.audio src_c/_sdl2/audio.c $(SDL) $(DEBUG) -Isrc_c
_sdl2.video src_c/_sdl2/video.c src_c/pgcompat.c $(SDL) $(DEBUG) -Isrc_c
_sdl2.mixer src_c/_sdl2/mixer.c $(SDL) $(MIXER) $(DEBUG) -Isrc_c
_sdl2.touch src_c/_sdl2/touch.c $(SDL) $(DEBUG) -Isrc_c
_sdl2.controller_old src_c/_sdl2/controller_old.c $(SDL) $(DEBUG) -Isrc_c

GFX = src_c/SDL_gfx/SDL_gfxPrimitives.c
#GFX = src_c/SDL_gfx/SDL_gfxBlitFunc.c src_c/SDL_gfx/SDL_gfxPrimitives.c
gfxdraw src_c/gfxdraw.c $(SDL) $(GFX) $(DEBUG)

#optional freetype module (do not break in multiple lines
#or the configuration script will choke!)
_freetype src_c/freetype/ft_cache.c src_c/freetype/ft_wrap.c src_c/freetype/ft_render.c  src_c/freetype/ft_render_cb.c src_c/freetype/ft_layout.c src_c/freetype/ft_unicode.c src_c/_freetype.c $(SDL) $(FREETYPE) $(DEBUG)

_sprite src_c/_sprite.c $(SDL) $(DEBUG)

#these modules are required for pygame to run. they only require
#SDL as a dependency. these should not be altered

base src_c/base.c $(SDL) $(DEBUG)
color src_c/color.c $(SDL) $(DEBUG)
constants src_c/constants.c $(SDL) $(DEBUG)
display src_c/display.c $(SDL) $(DEBUG)
event src_c/event.c $(SDL) $(DEBUG)
key src_c/key.c $(SDL) $(DEBUG)
mouse src_c/mouse.c $(SDL) $(DEBUG)
rect src_c/rect.c src_c/pgcompat_rect.c $(SDL) $(DEBUG)
rwobject src_c/rwobject.c $(SDL) $(DEBUG)
surface src_c/simd_blitters_sse2.c src_c/simd_blitters_avx2.c src_c/surface.c src_c/alphablit.c src_c/surface_fill.c src_c/surface_animation.c $(SDL) $(DEBUG)
surflock src_c/surflock.c $(SDL) $(DEBUG)
time src_c/time.c $(SDL) $(DEBUG)
joystick src_c/joystick.c $(SDL) $(DEBUG)
draw src_c/draw.c $(SDL) $(DEBUG)
image src_c/image.c $(SDL) $(DEBUG)
transform src_c/transform.c src_c/rotozoom.c src_c/scale2x.c src_c/scale_mmx.c $(SDL) $(DEBUG)
mask src_c/mask.c src_c/bitmask.c $(SDL) $(DEBUG)
bufferproxy src_c/bufferproxy.c $(SDL) $(DEBUG)
pixelarray src_c/pixelarray.c $(SDL) $(DEBUG)
math src_c/math.c $(SDL) $(DEBUG)
pixelcopy src_c/pixelcopy.c $(SDL) $(DEBUG)
newbuffer src_c/newbuffer.c $(SDL) $(DEBUG)
system src_c/system.c $(SDL) $(DEBUG)
_camera src_c/_camera.c src_c/camera_windows.c -lMfplat -lMf -lMfuuid -lMfreadwrite -lOle32 $(SDL) $(DEBUG)

Example Code:

import pygame
import sys

pygame.init()

anim=pygame.image.load_animation("cat.gif")
anim.loop_mode="loop"

sf=pygame.display.set_mode((640,480))

anim.start_play()
while 1:
    sf.fill((255,255,255))
    sf.blit(anim, (0,0))

    pygame.display.update()
    for event in pygame.event.get():
        if (event.type == pygame.KEYDOWN 
            and event.unicode==" "):
            anim.toggle_pause()
        if event.type==pygame.QUIT:
            sys.exit()

cat.gif

cat

@yunline yunline added New API This pull request may need extra debate as it adds a new class or function to pygame image pygame.image labels Jun 1, 2023
@yunline yunline requested a review from a team as a code owner June 1, 2023 09:30
@oddbookworm
Copy link
Member

image

@MyreMylar
Copy link
Member

I tried to review this, but for whatever reason pygame-ce didn't seem to want to build properly with the downloaded SDL_Image 2.7.0 dll. I must be missing a step somewhere (and yes, I tried using the edited Setup file above).

It looks like it has built and compiled - but actually it hasn't included SDL_image at all and pygame.image.get_sdl_image_version() returns None.

@MyreMylar
Copy link
Member

Requires this PR:
#2596

Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

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

I'm really interested in moving this PR forward, but the exported API may need discussion. Here's what I'm thinking

  • We keep the first level API very barebones, so I'm thinking we add something like an animation: bool kwarg to the existing load functions, and the return type could be list[tuple[Surface, float]] (basically something like [(surface1, delay1), (surface2, delay2), ...]
  • Later we may decide to add a higher level abstraction like the AnimatedSurface class

Thoughts on this, folks?

@yunline yunline marked this pull request as draft December 16, 2023 07:12
@yunline yunline marked this pull request as ready for review December 16, 2023 07:24
@MyreMylar
Copy link
Member

MyreMylar commented Dec 16, 2023

This works now for me which is great.

API wise:

  • The looping mode is a little confusing. To play an animation through once I have to set .loop_mode = "simple" which does not seem straight-forward to me. I think I would probably prefer a bool on animation creation that was just looping=True by default and then delegate the lesser-used "ping-pong" option to an attribute, or another creation parameter - maybe .loop_mode = "continous' or `.loop_mode = "ping-pong" with "continous" being the default.
  • At the minute it doesn't feel like the user has any control over the speed of the animations they just play at the default speed as best they can. It would be nice to be able to set a playback speed multiplier or an FPS for an animation, either seems fine to me.
  • Should we have native support for doing flip transforms on all the frames of an animation when loading it in? This seems like a pretty common operation to get left and right facing animations from the same simple anim. We definitely don't want to to a flip on each frame every loop while the animation is playing.
  • We should probably provide direct access to the currently playing frame number of an animated surface and set a simple .finished condition for single play through animations - as users will likely want to tie other effects (particles/audio) and events (do damage now) to specific frames of a longer animation and being able to say something like:
if (not self.attacked and 
    self.current_anim in [self.anims["sword_attack_right"], self.anims["sword_attack_left"]] and 
    self.current_anim.frame = 12):
    # maximum sword extension frame so check collision and play sound
    self.attacked = True

and later...

if (self.attacked and 
    self.current_anim in [self.anims["sword_attack_right"], self.anims["sword_attack_left"]] and 
    self.current_anim.finished):
    # sword anim finished, reset attack state and transtion back to idle animation 
    self.attacked = False
    if self.facing = "right":
        self.current_anim = self.anims["idle_right"]
    else:
        self.current_anim = self.anims["idle_left"]
  • We should probably also provide direct user access to the underlying frames of the AnimatedSurface as Surface objects.

Those are my initial ideas.

Perhaps Ankith is right and we should start with getting the loading part right into a list of basic Surfaces and move the AnimatedSurface option to a second PR, but would that then mean supporting two APIs if we end up having it possible to load into both a list of surfaces and an AnimatedSurface class?

I think I'm broadly in favour of us having a basic AnimatedSurface class. Would there be cases where you want to load an animated source image as a single frame Surface? If not we should be auto-determining what type of thing that we are loading based on the file type. I can also see wanting it to be clear to the user that they are attempting to load a static image versus an animation if they are going to return very different objects. The submodule name could get a bit confusing here as we are used to thinking of an image as a static image.

It should also be possible to construct an AnimatedSurface object from a sequence of Surfaces.

@MyreMylar MyreMylar added this to the 2.5.0 milestone Dec 16, 2023
@joereynolds
Copy link

joereynolds commented Jan 12, 2024

Hello!

I'm not a C programmer but I have some thoughts on the animation work. These are mainly things that I'm currently using that don't look like they'll be supported by this.

  • Personally, I'd emit an event when animations are finished so that user's can hook into it and do something on the end of an animation (kill a sprite, fire a bullet etc...). Without this, the only option is to provide a callback (adds coupling) or like MyreMylar suggested, provide access to the frame number (also feels tightly coupled to me)

  • As Myre mentioned above, we should be able to hook into the FPS of the animation. Not only that, but a common thing is for different frames to have different lengths. I.e. one frame could be 100ms and the next 2 could be 50ms

Perhaps I've misunderstood and this isn't meant to be a "kitchen sink" approach in which case feel free to ignore me. Thought I'd add my two cents.

I've uploaded what I'm currently working with as I've found it's fitting all use cases so far and has evolved several times over the course of a few years to a stable state

https://gist.github.com/joereynolds/1c20645bfa54bd2df62133a8fb9de7cb

Thanks all

Copy link
Member

@MyreMylar MyreMylar left a comment

Choose a reason for hiding this comment

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

Awaiting API discussion or changes.

@robertpfeiffer
Copy link
Contributor

API discussion: I don't think we should adopt this API, and I don't think this should be one PR.

GIF loading should return a list of surfaces, and maybe data about timings. Ideally, the animation loading API would be general enough to also cover loading animated PNG files, short video clips, JSON+PNG sprite sheets, and ASEprite files, that means multiple animations with different frame rates and looping meta-information.

The AnimatedSurface or "Clip" class should, if we adopt such a thing, be its own pull request, and it should be optional to use it. I see no reason why animation can't be implemented in pure Python, and I have done it myself multiple times.

The only reason to merge this as-is would be, in my opinion, because yunline has already put so much work into it.

@Starbuck5
Copy link
Member

I agree that any animation class would be optional to use, and generic enough to cover various use cases (animated images, videos, even streamed things from pygame.camera or network potentially?)

However, I think if we have a load_animation method and an Animation class, that load_animation method should return an Animation by default.

The Animation class would need a way to be unwrapped into frames / positions? / timings. Maybe it could be like Rect, a pygame class that also has a "canonical form" of a 4 element sequence. Or maybe it could get an explicit method to "unwrap" it into a frame sequence representation that can be used in any way.

@robertpfeiffer
Copy link
Contributor

However, I think if we have a load_animation method and an Animation class, that load_animation method should return an Animation by default.

That's the worst of both worlds!

We could just have the Animation class load a GIF from a static method or constructor...

Anyway, something I mentioned in Discord but forgot to add here: Loading as a sprite sheet/list of subsurfaces would also make it easier to use with OpenGL/Renderer/future new renderer. Or we'd have to add more special code for loading an animated image into modernGL and so on.

@Starbuck5
Copy link
Member

That's the worst of both worlds!

What's worst about it? If you want an animation, it's already there. If you want a list of frames, treat it as a list of frames. Are you worried about the runtime cost of constructing the class versus a bare list?

We could just have the Animation class load a GIF from a static method or constructor...

That's a good idea.

It wouldn't be consistent with Surface, but I've been thinking for a bit about how un-object-ey Surface is. For example, image.load -> Surface instead of Surface("filename") or Surface.load("filename"). Or image.tobytes(surface) instead of surface.tobytes(). Maybe we should have a Surface.load...

@robertpfeiffer
Copy link
Contributor

robertpfeiffer commented Mar 27, 2024

If the Animation is just a pure Python data container that holds a surface, subsurfaces and lists of frames, then I might be persuaded. It should probably be able to represent ASEprite animated sprites, JSON sprite sheeps, GIFs, and small WEBM clips. If Animation is a thing that wraps a GIF, talks to the clock and automagically changes the frame, I am against it.

Are you worried about the runtime cost of constructing the class versus a bare list?

No, purely an API complexity/API support/implementation complexity thing.

@Starbuck5 Starbuck5 removed this from the 2.5.0 milestone May 30, 2024
@ankith26 ankith26 marked this pull request as draft December 12, 2024 18:51
@LeWolfYT
Copy link

i'd really love to see the idea of gif loading move forward, since there are plenty of use cases for animation in general with pygame. i also think that gif saving would be a good idea if gif loading becomes a thing

@joereynolds
Copy link

Forgive my ignorance but aren't GIFs quite large in size, would be people want that if better alternatives exist?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
image pygame.image New API This pull request may need extra debate as it adds a new class or function to pygame SDL_Image 2.6.0+
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants