Send plymouth mailing list submissions to
        [email protected]

To subscribe or unsubscribe via the World Wide Web, visit
        http://lists.freedesktop.org/mailman/listinfo/plymouth
or, via email, send a message with subject or body 'help' to
        [email protected]

You can reach the person managing the list at
        [email protected]

When replying, please edit your Subject line so it is more specific
than "Re: Contents of plymouth digest..."


Today's Topics:

   1. Re: [PATCH 1/5] use the configured colors for the gradient
      (Ray Strode)
   2. Re: [PATCH 2/5] use the height in the max height comparison
      (Ray Strode)
   3. Re: [PATCH 1/5] use the configured colors for the gradient
      (William Jon McCann)
   4. Re: [PATCH 3/5] add simple one time animation helper (Ray Strode)
   5. Re: [PATCH 4/5] add a simple progress sequence helper (Ray Strode)
   6. Re: [PATCH 5/5] add a glow plugin (Ray Strode)


----------------------------------------------------------------------

Message: 1
Date: Wed, 4 Mar 2009 15:29:53 -0500
From: Ray Strode <[email protected]>
Subject: Re: [PATCH 1/5] use the configured colors for the gradient
To: William Jon McCann <[email protected]>
Cc: [email protected], William Jon McCann
        <[email protected]>
Message-ID:
        <[email protected]>
Content-Type: text/plain; charset=UTF-8

> diff --git a/src/plugins/splash/fade-in/plugin.c 
> b/src/plugins/splash/fade-in/plugin.c
> index 73318c5..07ebfc5 100644
> --- a/src/plugins/splash/fade-in/plugin.c
> +++ b/src/plugins/splash/fade-in/plugin.c
> @@ -436,7 +436,8 @@ on_erase (ply_boot_splash_plugin_t *plugin,
> ? area.height = height;
>
> ? ply_frame_buffer_fill_with_gradient (plugin->frame_buffer, &area,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x807c71, 0x3a362f);
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? PLYMOUTH_BACKGROUND_START_COLOR,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? PLYMOUTH_BACKGROUND_END_COLOR);
> ?}
Looks good.  Are these numbers the default numbers in configure.ac?
If not, may want to change them to match, so the look doesn't change.

--Ray


------------------------------

Message: 2
Date: Wed, 4 Mar 2009 15:31:37 -0500
From: Ray Strode <[email protected]>
Subject: Re: [PATCH 2/5] use the height in the max height comparison
To: William Jon McCann <[email protected]>
Cc: [email protected], William Jon McCann
        <[email protected]>
Message-ID:
        <[email protected]>
Content-Type: text/plain; charset=UTF-8

Hi,

On Wed, Mar 4, 2009 at 3:22 PM, William Jon McCann
<[email protected]> wrote:
> From: William Jon McCann <[email protected]>
>
> Fixes what was likely a copy/paste bug.
> ---
> ?src/libplybootsplash/ply-throbber.c | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/libplybootsplash/ply-throbber.c 
> b/src/libplybootsplash/ply-throbber.c
> index 79ab5b2..a3bd2ea 100644
> --- a/src/libplybootsplash/ply-throbber.c
> +++ b/src/libplybootsplash/ply-throbber.c
> @@ -237,7 +237,7 @@ ply_throbber_add_frame (ply_throbber_t *throbber,
> ? ply_array_add_element (throbber->frames, image);
>
> ? throbber->width = MAX (throbber->width, ply_image_get_width (image));
> - ?throbber->height = MAX (throbber->width, ply_image_get_height (image));
> + ?throbber->height = MAX (throbber->height, ply_image_get_height (image));
Doh, thanks. Looks good.  I wonder if spinfinity will look squished
together now.

If so, I'll probably just edit the image frames to be square :-)

--Ray


------------------------------

Message: 3
Date: Wed, 4 Mar 2009 15:33:36 -0500
From: William Jon McCann <[email protected]>
Subject: Re: [PATCH 1/5] use the configured colors for the gradient
To: Ray Strode <[email protected]>
Cc: [email protected]
Message-ID:
        <[email protected]>
Content-Type: text/plain; charset=ISO-8859-1

On Wed, Mar 4, 2009 at 3:29 PM, Ray Strode <[email protected]> wrote:
>> diff --git a/src/plugins/splash/fade-in/plugin.c 
>> b/src/plugins/splash/fade-in/plugin.c
>> index 73318c5..07ebfc5 100644
>> --- a/src/plugins/splash/fade-in/plugin.c
>> +++ b/src/plugins/splash/fade-in/plugin.c
>> @@ -436,7 +436,8 @@ on_erase (ply_boot_splash_plugin_t *plugin,
>> ? area.height = height;
>>
>> ? ply_frame_buffer_fill_with_gradient (plugin->frame_buffer, &area,
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x807c71, 0x3a362f);
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? PLYMOUTH_BACKGROUND_START_COLOR,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? PLYMOUTH_BACKGROUND_END_COLOR);
>> ?}
> Looks good. ?Are these numbers the default numbers in configure.ac?
> If not, may want to change them to match, so the look doesn't change.

Yup, they're the same.

Jon


------------------------------

Message: 4
Date: Wed, 4 Mar 2009 15:42:46 -0500
From: Ray Strode <[email protected]>
Subject: Re: [PATCH 3/5] add simple one time animation helper
To: William Jon McCann <[email protected]>
Cc: [email protected], William Jon McCann
        <[email protected]>
Message-ID:
        <[email protected]>
Content-Type: text/plain; charset=UTF-8

Hi,

On Wed, Mar 4, 2009 at 3:22 PM, William Jon McCann
<[email protected]> wrote:
> From: William Jon McCann <[email protected]>
>
> 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 <[email protected]>
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


------------------------------

Message: 5
Date: Wed, 4 Mar 2009 15:50:27 -0500
From: Ray Strode <[email protected]>
Subject: Re: [PATCH 4/5] add a simple progress sequence helper
To: William Jon McCann <[email protected]>
Cc: [email protected], William Jon McCann
        <[email protected]>
Message-ID:
        <[email protected]>
Content-Type: text/plain; charset=UTF-8

Hi,

On Wed, Mar 4, 2009 at 3:22 PM, William Jon McCann
<[email protected]> wrote:
> From: William Jon McCann <[email protected]>
>
> This is similar to the progress bar except that it uses a
> sequence of images for the stages of the progress.
Seems fine.  I'd like the existing progress bar to be prettier at some
point (maybe by using images for the end caps and middle part), but
that's something for later.

Minor nits below.  They don't really matter.

> ---
> ?src/libplybootsplash/Makefile.am ? ? ?| ? ?3 +-
> ?src/libplybootsplash/ply-progressor.c | ?330 
> +++++++++++++++++++++++++++++++++
> ?src/libplybootsplash/ply-progressor.h | ? 58 ++++++

Not a huge fan of the name.  If the other thing is ply-progress-bar,
maybe this should be ply-progress-animation

> + * Copyright (C) 2008 Red Hat, Inc.
2009!
> +#include <linux/kd.h>
> +
> +#ifndef BAR_HEIGHT
> +#define BAR_HEIGHT 16
> +#endif
copy-and-paste cruft?

Rest seems okay.

--Ray


------------------------------

Message: 6
Date: Wed, 4 Mar 2009 16:06:54 -0500
From: Ray Strode <[email protected]>
Subject: Re: [PATCH 5/5] add a glow plugin
To: William Jon McCann <[email protected]>
Cc: [email protected], William Jon McCann
        <[email protected]>
Message-ID:
        <[email protected]>
Content-Type: text/plain; charset=UTF-8

Hi,

On Wed, Mar 4, 2009 at 3:22 PM, William Jon McCann
<[email protected]> wrote:
> From: William Jon McCann <[email protected]>
>
> This plugin will progress through a sequence of images until
> the progress reaches 90% complete.  At that point it will run
> an animation sequence.
Looks fine.  At some point we're going to grow enough plugins that
we're going to need to do something about it, but adding one more
doesn't hurt for now.


> diff --git a/configure.ac b/configure.ac
> index 63a4709..f5d4639 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -111,6 +111,8 @@ AM_CONDITIONAL(ADD_DEFAULT_PLUGIN_LINK,
> ? ? ? ? ? ? ? ? ? -o "$default_plugin_name" = "fade-in" ? ?\
> ? ? ? ? ? ? ? ? ? -o "$default_plugin_name" = "pulser" ? ? \
> ? ? ? ? ? ? ? ? ? -o "$default_plugin_name" = "text" ? ? ? \
> + ? ? ? ? ? ? ? ? ?-o "$default_plugin_name" = "glow" ? ? ? \
> + ? ? ? ? ? ? ? ? ?-o "$default_plugin_name" = "solar" ? ? ?\
Ah thanks for catching that solar was missing.

[...]
> diff --git a/src/libplybootsplash/ply-throbber.c 
> b/src/libplybootsplash/ply-throbber.c
> index a3bd2ea..e1bce90 100644
> --- a/src/libplybootsplash/ply-throbber.c
> +++ b/src/libplybootsplash/ply-throbber.c
> @@ -70,6 +70,7 @@ struct _ply_throbber
> ? long x, y;
> ? long width, height;
> ? double start_time, previous_time, now;
> + ?uint32_t is_stopped : 1;
> ?};
>
> ?ply_throbber_t *
> @@ -86,6 +87,7 @@ ply_throbber_new (const char *image_dir,
> ? throbber->frames = ply_array_new ();
> ? throbber->frames_prefix = strdup (frames_prefix);
> ? throbber->image_dir = strdup (image_dir);
> + ?throbber->is_stopped = true;
> ? throbber->width = 0;
> ? throbber->height = 0;
> ? throbber->frame_area.width = 0;
> @@ -323,6 +325,7 @@ ply_throbber_start (ply_throbber_t ? ? ? ? *throbber,
> ? throbber->loop = loop;
> ? throbber->window = window;
> ? throbber->frame_buffer = ply_window_get_frame_buffer (window);;
> + ?throbber->is_stopped = false;
>
> ? throbber->x = x;
> ? throbber->y = y;
> @@ -345,6 +348,7 @@ ply_throbber_stop_now (ply_throbber_t *throbber)
>
> ? throbber->frame_buffer = NULL;
> ? throbber->window = NULL;
> + ?throbber->is_stopped = true;
>
> ? if (throbber->loop != NULL)
> ? ? {
> @@ -369,6 +373,12 @@ ply_throbber_stop (ply_throbber_t *throbber,
> ? throbber->stop_trigger = stop_trigger;
> ?}
>
> +bool
> +ply_throbber_is_stopped (ply_throbber_t *throbber)
> +{
> + ?return throbber->is_stopped;
> +}
> +
> ?long
> ?ply_throbber_get_width (ply_throbber_t *throbber)
> ?{
> diff --git a/src/libplybootsplash/ply-throbber.h 
> b/src/libplybootsplash/ply-throbber.h
> index 83f6b43..647f409 100644
> --- a/src/libplybootsplash/ply-throbber.h
> +++ b/src/libplybootsplash/ply-throbber.h
> @@ -46,6 +46,7 @@ bool ply_throbber_start (ply_throbber_t ? ? ? ? *throbber,
> ? ? ? ? ? ? ? ? ? ? ? ? ?long ? ? ? ? ? ? ? ?y);
> ?void ply_throbber_stop (ply_throbber_t *throbber,
> ? ? ? ? ? ? ? ? ? ? ? ? ply_trigger_t ?*stop_trigger);
> +bool ply_throbber_is_stopped (ply_throbber_t *throbber);
>
> ?long ply_throbber_get_width (ply_throbber_t *throbber);
> ?long ply_throbber_get_height (ply_throbber_t *throbber);

Do you actually need these changes or is it just for continuity with
some of the other apis?

No real comments on the actual plugin.  Do whatever you want there,
it's your thing.

--Ray


------------------------------

_______________________________________________
plymouth mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/plymouth


End of plymouth Digest, Vol 6, Issue 3
**************************************

Reply via email to