[PATCH 3/5] add simple one time animation helper

Ray Strode halfline at gmail.com
Wed Mar 4 12:42:46 PST 2009


Hi,

On Wed, Mar 4, 2009 at 3:22 PM, William Jon McCann
<william.jon.mccann at gmail.com> wrote:
> From: William Jon McCann <jmccann at redhat.com>
>
> This will play a sequence of images from beginning to end at
> a fixed frame rate.  It differs from the throbber in that it starts
> at the 0th image and does not repeat.

Sounds reasonable.

> ---
>  src/libplybootsplash/Makefile.am    |    5 +-
>  src/libplybootsplash/ply-animator.c |  396 +++++++++++++++++++++++++++++++++++
>  src/libplybootsplash/ply-animator.h |   56 +++++
Should probably be called ply-animation.c no? (It's not something that
creates animations, it *is* the animation).


> + * Copyright (C) 2007, 2008 Red Hat, Inc.
It's 2009!

> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2, or (at your option)
> + * any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
> + * 02111-1307, USA.
> + *
> + * Written by: Ray Strode <rstrode at redhat.com>
I didn't write this, you did (well it's somewhat copy and paste, but
your name should be there).

[... snip implementation which looks okay ...]

> +bool ply_animator_start (ply_animator_t     *animator,
> +                         ply_event_loop_t   *loop,
> +                         ply_window_t       *window,
> +                         long                x,
> +                         long                y);
> +void ply_animator_stop (ply_animator_t *animator,
> +                        ply_trigger_t  *stop_trigger);
So, this doesn't quite seem right to me.  If the animation goes
through all the frames and then stops, it seems like you'll always
want to get notified when it finishes.  I think the trigger should be
put in ply_animation_start, not _stop, and _stop should either get
removed or be what stop_now is.

With the throbber it's a little different, since it loops
indefinitely.  We pass the trigger in at stop time so we can let it
finish out the current iteration of the frames before stopping.  If
there's only ever one interation with this new class it doesn't make
sense to do it that way.

--Ray


More information about the plymouth mailing list