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: Plymouth question (G. Michael Carter)
2. Input rework (Ray Strode)
----------------------------------------------------------------------
Message: 1
Date: Wed, 03 Dec 2008 14:23:08 -0500
From: "G. Michael Carter" <[EMAIL PROTECTED]>
Subject: Re: Plymouth question
To: [email protected]
Message-ID: <[EMAIL PROTECTED]>
Content-Type: text/plain; charset="utf-8"
Ray thanks for your help. I'm pretty safe posting to the list since
she would be in very different areas of the internet.
The debug help and manual run helped me figure out what's going on. (I
think) When doing the init 3/show splash thing I see the changes to
the star.png file no problem. But a reboot I don't. There is a "no
file found" type message on a reboot. I'm assuming the graphic files
need to be in the initrd file, but the
/usr/libexec/plymouth/plymouth-update-initrd doesn't appear to be
putting them in. I added the -v flag to the command inside and don't
see any reference to the /usr/share/plymouth directory.
Or did I really miss the mark on this one? Was there something I
needed to do before hand?
Thanks
Michael
Ray Strode wrote:
> Hi,
>> Sorry to contact you out of the blue like this.
> No problem! There is a plymouth mailing list here:
>
> http://lists.freedesktop.org/mailman/listinfo/plymouth
>
> but given it's a public list (with public archives) and you want this
> to be a surprise, contacting me directly was probably more appropriate.
>> I have a few ideas. The simplest is to change the star.png and
>> possibly the progress_bar.png That way I can put up a custom screen
>> for her without learning how to program it. (I know c/c++ but it's
>> been a while)
> That should work fine. I think the solar plugin may hard code some of
> the sizes, so it's probably best to make sure the images are the same
> size. Alternatively, switching out the spinfinity plugin's images may
> work better for you.
>>
>> When I tried however, the graphic boot no longer displays solar. It
>> falls back to the text version.
> I'm not sure why off hand. A few questions
>
> 1) did you rebuild the initrd after making your changes (with
> /usr/libexec/plymouth/plymouth-update-initrd) ? You may want to back
> up your existing initrd as a separate entry in grub.conf while testing
> changes.
> 2) did you replace the images with png images or some other image
> format? plymouth only supports pngs (and i think only rgb and rgba
> pngs at that) not say jpegs, gifs, mngs, or apngs.
> 3) if you wait for about 15 seconds into boot (so / is mounted) and
> hit escape a couple of times does your splash show up then? If so,
> then the files are fine on the root filesystem, but some how aren't
> getting copied to the initrd right.
>>
>> So my questions:
>>
>> 1. Is there some specifics I need to look for when making the custom
>> png files?
> Just what I mentioned above, I think. Of course, there may be bugs...
>> 2. Does it keep a checksum or something?
> No. It does copy the files to the initrd, though (since it shows up
> before / is mounted). Make sure you rebuild your initrd after making
> changes
>> 3. Why would it dump back to text version just adding the works "I
>> was here" to the png?
> It falls back to text if it couldn't load the graphical plugin for
> some reason. likely it was trying to open the png and couldn't.
>> 4. Is there a log somewhere that will tell me if plymouth fails and
>> why?
> If you add plymouth:debug to your kernel command line it may give a
> useful error message. alternatively, pressing ctrl-v while it's
> running turns on verbose messages.
>> 5. Does the rendering engine for the support animated png files?
>> (another way to cheep out of programming it)
> No. But spinfinity does a simple animation in the center of the
> screen based on multiple images. Using it as a basis may work okay
> for you. To switch which plugin you're using, run
>
> /usr/sbin/plymouth-set-default-plugin spinfinity
>
> (and then rebuild your initrd with
> /usr/libexec/plymouth/plymouth-update-initrd)
>
> If you're going to be making a lot changes, you can run plymouth
> manually without rebooting by doing:
>
> /sbin/init 3
>
> /sbin/plymouthd
>
> and then
>
> plymouth --show-splash
>
> (all as root)
>
> that should display it on screen. Then to get rid of it, press escape
> to jump back to text mode and use ctrl-alt-f3 (or whatever) to get to
> a tty and run
>
> plymouth --quit
>
> to quit it.
>
> --Ray
>
--
*G. Michael Carter*
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
http://lists.freedesktop.org/archives/plymouth/attachments/20081203/0b7ce815/attachment-0001.html
------------------------------
Message: 2
Date: Wed, 3 Dec 2008 15:50:53 -0500
From: "Ray Strode" <[EMAIL PROTECTED]>
Subject: Input rework
To: [email protected]
Message-ID:
<[EMAIL PROTECTED]>
Content-Type: text/plain; charset=UTF-8
Hi Charlie,
On IRC (irc.freenode.net, #plymouth) you asked me to review some patches
(which are here: http://cgit.freedesktop.org/plymouth/log/?h=input-rework)
that you're doing to allow for more flexibility with input.
Comments inline below (mostly nits that don't really matter):
> ============================================================
> Keystroke sensitifity
>
Should probably use better commit messages than this.
The usual convention is make the first line a summary saying
what you're doing, add a blank line, and then add a short
paragraph saying why you're doing it.
A good example here might be:
Add hooks to watch for keystrokes during boot up
There are cases during boot up where it's useful to check
for certain user input. We currently only handle asking
the user for a password. But we'd also like to handle, e.g.,
interactive startup scenarios. This commit puts the machinery
in place to accomodate those types of use cases.
>
> diff --git a/src/client/ply-boot-client.c b/src/client/ply-boot-client.c
> --- a/src/client/ply-boot-client.c
> +++ b/src/client/ply-boot-client.c
> @@ -570,6 +570,34 @@ ply_boot_client_ask_daemon_for_cached_passwords
> (ply_boot_client_t
> }
>
> void
> +ply_boot_client_ask_daemon_to_watch_for_keystroke (ply_boot_client_t
> *client,
> + const char
> *keys,
> +
> ply_boot_client_key_answer_handler_t handler,
> + ply_boot_client_response_handler_t
> failed_handler,
> + void
> *user_data)
> +{
Spacing here and in a few other places isn't right. things should line up.
"keys" should probably be called "keystroke" since the function name
is watch_for_keystroke
> diff --git a/src/client/ply-boot-client.h b/src/client/ply-boot-client.h
> --- a/src/client/ply-boot-client.h
> +++ b/src/client/ply-boot-client.h
> @@ -36,6 +36,10 @@ typedef void (* ply_boot_client_answer_handler_t) (void
> *user_data,
> const char *answer,
> ply_boot_client_t
> *client);
>
> +typedef void (* ply_boot_client_key_answer_handler_t) (void
> *user_data,
> + const char *answer,
> + ply_boot_client_t
> *client);
> +
Any particular reason you can't use the existing
ply_boot_client_answer_handler_t
type?
> @@ -147,20 +156,22 @@ answer_via_command (answer_state_t *answer_state,
> out:
> if (answer != NULL)
> close (command_input_sender_fd);
> - waitpid (pid, exit_status, 0);
> + if (exit_status) waitpid (pid, exit_status, 0);
> + else waitpid (pid, NULL, 0);
This change looks unneeded.
> static void
> -on_answer_failure (answer_state_t *answer_state)
> +on_answer_failure (answer_state_t *answer_state, ply_boot_client_t *client)
> {
> ply_event_loop_exit (answer_state->state->loop, 1);
> }
>
> static void
> on_answer (answer_state_t *answer_state,
> - const char *answer)
> + const char *answer,
> + ply_boot_client_t *client)
> {
> int exit_status;
Also, these changes seem unneeded (and below too for some other callbacks).
Were you getting a compiler warning?
If so, change is probably okay, or doing a cast at the caller side
would work, too.
Might be better to have this kind of clean up stuff as a separate commit though.
>
> @@ -199,6 +210,28 @@ on_answer (answer_state_t *answer_state,
> ply_event_loop_exit (answer_state->state->loop, WEXITSTATUS (exit_status));
> }
>
> +
> +static void
> +on_key_reply (answer_state_t *answer_state,
This should be a key_answer_state *, I think.
> + const char *answer,
> + ply_boot_client_t *client)
> +{
> +
> + if (answer_state->command != NULL)
> + {
> + bool command_started = false;
This variable isn't used, right?
> +
> + command_started = answer_via_command (answer_state, answer, NULL);
> + }
> + else
> + {
> + if (answer) write (STDOUT_FILENO, answer, strlen (answer));
> + }
> +
> + if (answer) ply_event_loop_exit (answer_state->state->loop, 0);
> + ply_event_loop_exit (answer_state->state->loop, 1);
> +}
The if statements above should probably have the condition on a separate line
from the meat. I don't want to harp on spacing issues though. Just
keep it in mind
when you're working, so the code looks consistent. Just makes it easier for new
comers to get acclumated to the code.
> exit_code = 0;
> @@ -379,6 +443,7 @@ main (int argc,
> "show-splash", "Show splash screen",
> PLY_COMMAND_OPTION_TYPE_FLAG,
> "hide-splash", "Hide splash screen",
> PLY_COMMAND_OPTION_TYPE_FLAG,
> "ask-for-password", "Ask user for
> password", PLY_COMMAND_OPTION_TYPE_FLAG,
> + "ignore-keystroke", "Remove sensitivity to
> a keystroke", PLY_COMMAND_OPTION_TYPE_STRING,
> "update", "Tell boot daemon an update
> about boot progress", PLY_COMMAND_OPTION_TYPE_STRING,
> "details", "Tell boot daemon there were
> errors during boot", PLY_COMMAND_OPTION_TYPE_FLAG,
> "wait", "Wait for boot daemon to quit",
> PLY_COMMAND_OPTION_TYPE_FLAG,
So, there is a bit of a mess here and it's my fault.
The plymouth client currently takes two types of commands
plymouth --command
and
plymouth subcommand --option1 --option2
the former method is deprecated. I'd like to move everything over to
the new format, since it's more flexible.
The only reason I haven't made the change over completely yet is
laziness. We should definitely not add new commands
of the first type though.
> --- a/src/libplybootsplash/ply-window.c
> +++ b/src/libplybootsplash/ply-window.c
> @@ -42,6 +42,7 @@
> #include "ply-buffer.h"
> #include "ply-event-loop.h"
> #include "ply-frame-buffer.h"
> +#include "ply-list.h"
> #include "ply-logger.h"
> #include "ply-utils.h"
>
> @@ -95,6 +96,12 @@
> #define TEXT_PALETTE_SIZE 48
> #endif
>
> +typedef struct
> +{
> + void *function;
I wouldn't use void * here for a function pointer. Instead,
create a typedef void (* ply_window_handler_t) (void *); and
use it.
> + void *user_data;
> +} ply_window_callback_t;
> +
I'd call this a ply_window_closure_t instead of
ply_window_callback_t (since it has both a callback and
data associated with the callback)
> @@ -257,8 +267,9 @@ process_keyboard_input (ply_window_t *window,
> size_t character_size)
> {
...
> - if (mbrtowc (&key, keyboard_input, 1, NULL) > 0)
> + if (mbrtowc (&key, keyboard_input, character_size, NULL) > 0)
Grr, that's embarassing. Should probably be fixed as a separate
commit though. In fact, looking at this again. I think it's
still wrong. mbrtowc returns size_t which is unsigned, so we need
to cast it to ssize_t I guess.
> +
> +void
> +ply_window_remove_keyboard_input_handler (ply_window_t *window,
> + ply_window_keyboard_input_handler_t
> input_handler)
> +{
> + ply_list_node_t *node;
> + assert (window != NULL);
> + for (node = ply_list_get_first_node(window->keyboard_input_handler_list);
> node; node = ply_list_get_next_node(window->keyboard_input_handler_list,
> node))
> + {
> + ply_window_callback_t *callback = ply_list_node_get_data (node);
> + if (callback->function == input_handler)
> + {
> + free(callback);
I'd probably have a ply_window_callback_free (well
ply_window_closure_free) function
here just for symmetry
> diff --git a/src/main.c b/src/main.c
> --- a/src/main.c
> +++ b/src/main.c
> @@ -48,6 +48,12 @@
> #define PLY_MAX_COMMAND_LINE_SIZE 512
> #endif
>
> +typedef struct
> +{
> + const char *keys;
> + ply_trigger_t *trigger;
> +} ply_keystroke_trigger_t;
Since this has-a trigger rather than is-a trigger, i'd probably call it
ply_keystroke_watch_t instead of ply_keystroke_trigger_t
>
> - ply_window_set_keyboard_input_handler (plugin->window, NULL, NULL);
> - ply_window_set_backspace_handler (plugin->window, NULL, NULL);
> - ply_window_set_enter_handler (plugin->window, NULL, NULL);
> + (plugin->window, NULL, NULL);
This looks fishy ^
> --- a/src/ply-boot-server.c
> +++ b/src/ply-boot-server.c
...
> +ply_boot_connection_on_keystroke_answer (ply_boot_connection_t *connection,
> + const char *key)
> +{
> +
> + uint8_t size;
> +
> + /* splash plugin isn't able to ask for password,
> + * punt to client
> + */
> + if (key == NULL)
> + {
> + if (!ply_write (connection->fd,
> + PLY_BOOT_PROTOCOL_RESPONSE_TYPE_NO_ANSWER,
> + strlen (PLY_BOOT_PROTOCOL_RESPONSE_TYPE_NO_ANSWER)))
> + ply_error ("could not write bytes: %m");
> + }
> + else
> + {
> + /* FIXME: support up to 4 billion (probably fine)
> + */
> + if (strlen (key) > 255)
> + ply_error ("password to long to fit in buffer");
> +
> + size = (uint8_t) strlen (key);
> +
> + if (!ply_write (connection->fd,
> + PLY_BOOT_PROTOCOL_RESPONSE_TYPE_ANSWER,
> + strlen (PLY_BOOT_PROTOCOL_RESPONSE_TYPE_ANSWER)) ||
> + !ply_write (connection->fd,
> + &size, sizeof (uint8_t)) ||
> + !ply_write (connection->fd,
> + key, size))
> + ply_error ("could not write bytes: %m");
> + }
> +
> +}
This looks cut-and-paste and still mentions things like "password" in it.
Should probably factor the code out into one function so the lame comments,
FIXME's don't get repeated.
> ============================================================
> Clearword entry and moving password entry to a higher level.
I don't like the name "clearword" should probably be
"plaintext entry", "text entry" or just "entry"
>
> diff --git a/src/client/ply-boot-client.c b/src/client/ply-boot-client.c
> --- a/src/client/ply-boot-client.c
> +++ b/src/client/ply-boot-client.c
> @@ -570,6 +570,21 @@ ply_boot_client_ask_daemon_for_cached_passwords
> (ply_boot_client_t
> }
>
> void
> +ply_boot_client_ask_daemon_for_clearword (ply_boot_client_t
> *client,
> +
> + const char *prompt,
> + ply_boot_client_answer_handler_t
> handler,
> + ply_boot_client_response_handler_t
> failed_handler,
> + void
> *user_data)
> +{
> + assert (client != NULL);
> +
> + ply_boot_client_queue_request (client,
> PLY_BOOT_PROTOCOL_REQUEST_TYPE_CLEARWORD,
maybe REQUEST_TYPE_QUESTION ?
> + prompt, (ply_boot_client_response_handler_t)
> + handler, failed_handler, user_data);
> +}
> +
> +void
> ply_boot_client_ask_daemon_to_watch_for_keystroke (ply_boot_client_t
> *client,
> const char
> *keys,
>
> ply_boot_client_key_answer_handler_t handler,
> diff --git a/src/client/ply-boot-client.h b/src/client/ply-boot-client.h
> --- a/src/client/ply-boot-client.h
> +++ b/src/client/ply-boot-client.h
> @@ -76,6 +76,11 @@ void ply_boot_client_ask_daemon_for_cached_passwords
> (ply_boot_client_t
>
> ply_boot_client_multiple_answers_handler_t handler,
>
> ply_boot_client_response_handler_t failed_handler,
> void
> *user_data);
> +void ply_boot_client_ask_daemon_for_clearword (ply_boot_client_t
> *client,
> + const char *prompt,
> +
> ply_boot_client_answer_handler_t handler,
> +
> ply_boot_client_response_handler_t failed_handler,
> + void
> *user_data);
> void ply_boot_client_ask_daemon_to_watch_for_keystroke (ply_boot_client_t
> *client,
> const char *keys,
>
> ply_boot_client_key_answer_handler_t handler,
> diff --git a/src/client/plymouth.c b/src/client/plymouth.c
> --- a/src/client/plymouth.c
> +++ b/src/client/plymouth.c
...
> +static void
> +on_clearword_reply (clearword_answer_state_t *answer_state,
> + const char *answer,
> + ply_boot_client_t *client)
> +{
> + if (answer_state->command != NULL)
> + {
> + bool command_started = false;
> +
> + command_started = answer_via_command (answer_state->command, answer,
> NULL);
not used variable
> static void
> +on_clearword_request (state_t *state,
> + const char *command)
> +{
> + char *prompt;
> + char *program;
> + int number_of_tries;
> + answer_state_t *answer_state;
should be clearword_answer_state_t. or maybe it should be the other way
around. keep this answer_state_t and make the other one password_state_t.
> + ply_command_parser_add_command (state.command_parser,
> + "ask-for-clearword", "Ask user for
> password",
maybe "ask-question", "Ask user a question" ?
> + (ply_command_handler_t)
> + on_clearword_request, &state,
> + "command", "Command to send clearword to
> via standard input",
> + PLY_COMMAND_OPTION_TYPE_STRING,
> + "prompt", "Message to display when asking
> for clearword",
> + PLY_COMMAND_OPTION_TYPE_STRING,
> + NULL);
> +
...
> diff --git a/src/libply/ply-buffer.c b/src/libply/ply-buffer.c
> --- a/src/libply/ply-buffer.c
> +++ b/src/libply/ply-buffer.c
> @@ -83,6 +83,7 @@ ply_buffer_remove_bytes (ply_buffer_t *buffer,
> buffer->size - bytes_to_remove);
> buffer->size -= bytes_to_remove;
> }
> + buffer->data[buffer->size] = '\0';
ugh, another embarrassing bug.
> }
>
> void
> @@ -94,6 +95,7 @@ ply_buffer_remove_bytes_at_end (ply_buffer_t *buffer,
> bytes_to_remove = MIN (buffer->size, bytes_to_remove);
>
> buffer->size -= bytes_to_remove;
> + buffer->data[buffer->size] = '\0';
and again
> @@ -182,16 +184,17 @@ ply_buffer_append_bytes (ply_buffer_t *buffer,
> assert (buffer != NULL);
> assert (bytes != NULL);
> assert (length != 0);
> -
> - if ((buffer->size + length) >= buffer->capacity)
> +
> + if (length >PLY_BUFFER_MAX_BUFFER_CAPACITY){
> + bytes += length - (PLY_BUFFER_MAX_BUFFER_CAPACITY-1);
bytes is void * here, probably better to cast to uint8_t *
before doing the pointer arithmetic
> + length = (PLY_BUFFER_MAX_BUFFER_CAPACITY-1);
> + }
> +
> + while ((buffer->size + length) >= buffer->capacity)
> {
> if (!ply_buffer_increase_capacity (buffer))
> {
> ply_buffer_remove_bytes (buffer, length);
> -
> - if ((buffer->size + length) >= buffer->capacity)
> - if (!ply_buffer_increase_capacity (buffer))
> - return;
> }
> }
>
> @@ -201,6 +204,7 @@ ply_buffer_append_bytes (ply_buffer_t *buffer,
> bytes, length);
>
> buffer->size += length;
> + buffer->data[buffer->size] = '\0';
> }
looks good. clearly, I should have added unit tests with this code.
> diff --git a/src/libplybootsplash/ply-boot-splash-plugin.h
> b/src/libplybootsplash/ply-boot-splash-plugin.h
> --- a/src/libplybootsplash/ply-boot-splash-plugin.h
> +++ b/src/libplybootsplash/ply-boot-splash-plugin.h
> @@ -33,7 +33,16 @@
>
> typedef struct _ply_boot_splash_plugin ply_boot_splash_plugin_t;
>
> -typedef void (* ply_boot_splash_password_answer_handler_t) (void
> *answer_data, const char *password);
> +typedef enum {
> + PLY_BOOT_SPLASH_DISPLAY_NORMAL,
> + PLY_BOOT_SPLASH_DISPLAY_CLEARWORD_ENTRY,
> + PLY_BOOT_SPLASH_DISPLAY_PASSWORD_ENTRY
> +} ply_boot_splash_display_type_t;
> +
> +typedef union {
> + struct {const char *prompt; const char *entry_text;} clearword;
> + struct {const char *prompt; int bullets;} password;
> +} ply_boot_splash_display_info_t;
>
> typedef struct
> {
> @@ -59,10 +68,9 @@ typedef struct
> void (* on_root_mounted) (ply_boot_splash_plugin_t *plugin);
> void (* hide_splash_screen) (ply_boot_splash_plugin_t *plugin,
> ply_event_loop_t *loop);
> -
> - void (* ask_for_password) (ply_boot_splash_plugin_t *plugin,
> - const char *prompt,
> - ply_trigger_t *answer);
> + void (* update_display) (ply_boot_splash_plugin_t *plugin,
> + ply_boot_splash_display_type_t type,
> + ply_boot_splash_display_info_t *info);
> void (* become_idle) (ply_boot_splash_plugin_t *plugin,
> ply_trigger_t *idle_trigger);
> } ply_boot_splash_plugin_interface_t;
I think I'd rather have ask_for_password and ask_question as separate
functions rather than using a union. There's no real reason for them
to be smooshed together.
> diff --git a/src/libplybootsplash/ply-entry.c
> b/src/libplybootsplash/ply-entry.c
> --- a/src/libplybootsplash/ply-entry.c
> +++ b/src/libplybootsplash/ply-entry.c
...
> @@ -162,59 +173,95 @@ ply_entry_draw (ply_entry_t *entry)
> ply_frame_buffer_fill_with_argb32_data (entry->frame_buffer,
> &entry->area, 0, 0,
> text_field_data);
> -
> - bullet_data = ply_image_get_data (entry->bullet_image);
> - bullet_area.width = ply_image_get_width (entry->bullet_image);
> - bullet_area.height = ply_image_get_height (entry->bullet_image);
> -
> - if (entry->number_of_bullets < entry->max_number_of_visible_bullets)
> - number_of_visible_bullets = entry->number_of_bullets;
> +
> + if (entry->is_password)
> + {
> + bullet_data = ply_image_get_data (entry->bullet_image);
> + bullet_area.width = ply_image_get_width (entry->bullet_image);
> + bullet_area.height = ply_image_get_height (entry->bullet_image);
> +
> + if (entry->number_of_bullets < entry->max_number_of_visible_bullets)
> + number_of_visible_bullets = entry->number_of_bullets;
> + else
> + {
> + number_of_visible_bullets = entry->max_number_of_visible_bullets;
> +
> + /* We've got more bullets than we can show in the available space,
> so
> + * draw a little half bullet to indicate some bullets are offscreen
> + */
> + bullet_area.x = entry->area.x;
> + bullet_area.y = entry->area.y + entry->area.height / 2.0 -
> bullet_area.height / 2.0;
> +
> + ply_frame_buffer_fill_with_argb32_data (entry->frame_buffer,
> + &bullet_area,
> bullet_area.width / 2.0, 0,
> + bullet_data);
> + }
> +
> + for (i = 0; i < number_of_visible_bullets; i++)
> + {
> + bullet_area.x = entry->area.x + i * bullet_area.width +
> bullet_area.width / 2.0;
> + bullet_area.y = entry->area.y + entry->area.height / 2.0 -
> bullet_area.height / 2.0;
> +
> + ply_frame_buffer_fill_with_argb32_data (entry->frame_buffer,
> + &bullet_area, 0, 0,
> + bullet_data);
> + }
> + }
> else
> {
> - number_of_visible_bullets = entry->max_number_of_visible_bullets;
> -
> - /* We've got more bullets than we can show in the available space, so
> - * draw a little half bullet to indicate some bullets are offscreen
> - */
> - bullet_area.x = entry->area.x;
> - bullet_area.y = entry->area.y + entry->area.height / 2.0 -
> bullet_area.height / 2.0;
> -
> - ply_frame_buffer_fill_with_argb32_data (entry->frame_buffer,
> - &bullet_area,
> bullet_area.width / 2.0, 0,
> - bullet_data);
This part shouldn't be needed here right?
> + ply_label_set_text (entry->label, entry->text);
> + ply_label_show (entry->label, entry->window, entry->area.x,
> entry->area.y);
> +
> }
> + ply_frame_buffer_unpause_updates (entry->frame_buffer);
> +}
Overall ply_entry_draw is getting too big for itself. I think i'd
split it out into two
helper functions, ply_entry_draw_text and ply_entry_draw_bullets
>
> - for (i = 0; i < number_of_visible_bullets; i++)
> +void
> +ply_entry_set_bullets (ply_entry_t *entry, int count)
call it set_number_of_bullets or set_bullet_count
> +{
> + count = MAX(0, count);
> + if (!entry->is_password || entry->number_of_bullets != count)
> {
> - bullet_area.x = entry->area.x + i * bullet_area.width +
> bullet_area.width / 2.0;
> - bullet_area.y = entry->area.y + entry->area.height / 2.0 -
> bullet_area.height / 2.0;
> -
> - ply_frame_buffer_fill_with_argb32_data (entry->frame_buffer,
> - &bullet_area, 0, 0,
> - bullet_data);
> + entry->is_password = true;
> + entry->number_of_bullets = count;
> + ply_entry_draw (entry);
> }
> - ply_frame_buffer_unpause_updates (entry->frame_buffer);
> +}
> +
> +int
> +ply_entry_get_bullets (ply_entry_t *entry)
get_number_of_bullets or get_bullet_count
> +++ b/src/libplybootsplash/ply-window.c
> --- a/src/main.c
> +++ b/src/main.c
> @@ -54,6 +54,15 @@ typedef struct
> ply_trigger_t *trigger;
> } ply_keystroke_trigger_t;
>
> +typedef struct
> +{
> + enum {PLY_ENTRY_TRIGGER_TYPE_PASSWORD,
> + PLY_ENTRY_TRIGGER_TYPE_CLEARWORD}
> + type;
> + const char *prompt;
> + ply_trigger_t *trigger;
> +} ply_entry_trigger_t;
> +
> typedef struct
> {
> ply_event_loop_t *loop;
> @@ -64,6 +73,8 @@ typedef struct
> ply_buffer_t *boot_buffer;
> ply_progress_t *progress;
> ply_list_t *keystroke_triggers;
> + ply_list_t *entry_triggers;
> + ply_buffer_t *entry_buffer;
> long ptmx;
I think input buffers should be per-window, since different windows
have different input devices associated with them. On the other hand,
we only need an answer from one given window before continuing...
It's sort of academic, since the only time we ever have more than one
window right now is if the user has a serial console.
> @@ -155,18 +167,28 @@ on_ask_for_password (state_t *state,
> const char *prompt,
> ply_trigger_t *answer)
> {
> - if (state->boot_splash == NULL)
> - {
> - show_detailed_splash (state);
> - if (state->boot_splash == NULL)
> - ply_trigger_pull (answer, "");
> - return;
> - }
> -
> - ply_boot_splash_ask_for_password (state->boot_splash,
> - prompt, answer);
> + ply_entry_trigger_t *entry_trigger =
> + calloc (1, sizeof (ply_entry_trigger_t));
> + entry_trigger->type = PLY_ENTRY_TRIGGER_TYPE_PASSWORD;
> + entry_trigger->prompt = prompt;
> + entry_trigger->trigger = answer;
> + ply_list_append_data (state->entry_triggers, entry_trigger);
> + update_display (state);
> }
Ahh, nice fix.
> dump_details_and_quit_splash (state_t *state)
> {
> state->showing_details = false;
> - on_escape_pressed (state);
> +/* on_escape_pressed (state); FIXME_brejc8 Check with halfline*/
So the reason we do:
show_details = false;
on_escape_pressed
is just to get it to show the details before quitting. I probably should have
factored out on_escape_pressed into common code between on_escape_pressed and
this function instead of munging with the state variable. That was laziness on
my part.
We need to see the details when quitting so, for example, if the root filesystem
doesn't get mounted we can show the error message from nash.
> --- a/src/plugins/splash/details/plugin.c
> +++ b/src/plugins/splash/details/plugin.c
> @@ -53,20 +53,17 @@
>
> #include <linux/kd.h>
>
> -void ask_for_password (ply_boot_splash_plugin_t *plugin,
> - const char *prompt,
> - ply_trigger_t *answer);
> +#define CLEAR_LINE_SEQUENCE "\033[2K\r"
> +
Shouldn't need this. call ply_window_clear_text_line instead.
Overall, looks good.
--Ray
------------------------------
_______________________________________________
plymouth mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/plymouth
End of plymouth Digest, Vol 3, Issue 2
**************************************
