[PATCH] Fix color problem the grub-emu

[PATCH] Fix color problem the grub-emu

[Bean]

[-- Attachment #1: Type: text/plain, Size: 842 bytes --]

Hi,

Currently, the color handling in grub-emu is broken, sometimes you see
nothing on screen. This patch fix it, now variable menu_color_normal
and menu_color_highlight works properly in grub-emu.

2008-08-02  Bean  <bean123ch@gmail.com>

	* util/console.c (grub_console_cur_color): New variable.
	(grub_console_standard_color): Likewise.
	(grub_console_normal_color): Likewise.
	(grub_console_highlight_color): Likewise.
	(color_map): Likewise.
	(use_color): Likewise.
	(NUM_COLORS): New macro.
	(grub_ncurses_setcolorstate): Handle color properly.
	(grub_ncurses_setcolor): Don't change color here, just remember the
	settings, color will be set in grub_ncurses_setcolorstate.
	(grub_ncurses_getcolor): New function.
	(grub_ncurses_init): Initialize color pairs.
	(grub_term grub_ncurses_term): New member grub_ncurses_getcolor.

-- 
Bean

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: color.diff --]
[-- Type: text/x-diff; name=color.diff, Size: 2926 bytes --]

diff --git a/util/console.c b/util/console.c
index 8c9401c..53fc5d0 100644
--- a/util/console.c
+++ b/util/console.c
@@ -41,6 +41,28 @@
 
 static int grub_console_attr = A_NORMAL;
 
+grub_uint8_t grub_console_cur_color = 7;
+
+static grub_uint8_t grub_console_standard_color = 0x7;
+static grub_uint8_t grub_console_normal_color = 0x7;
+static grub_uint8_t grub_console_highlight_color = 0x70;
+
+#define NUM_COLORS	8
+
+static grub_uint8_t color_map[NUM_COLORS] =
+{
+  COLOR_BLACK,
+  COLOR_BLUE,
+  COLOR_GREEN,
+  COLOR_CYAN,
+  COLOR_RED,
+  COLOR_MAGENTA,
+  COLOR_YELLOW,
+  COLOR_WHITE
+};
+
+static int use_color;
+
 static void
 grub_ncurses_putchar (grub_uint32_t c)
 {
@@ -100,24 +122,46 @@ grub_ncurses_setcolorstate (grub_term_color_state state)
   switch (state) 
     {
     case GRUB_TERM_COLOR_STANDARD:
+      grub_console_cur_color = grub_console_standard_color;
       grub_console_attr = A_NORMAL;
       break;
     case GRUB_TERM_COLOR_NORMAL:
+      grub_console_cur_color = grub_console_normal_color;
       grub_console_attr = A_NORMAL;
       break;
     case GRUB_TERM_COLOR_HIGHLIGHT:
+      grub_console_cur_color = grub_console_highlight_color;
       grub_console_attr = A_STANDOUT;
       break;
     default:
       break;
     }
+
+  if (use_color)
+    {
+      grub_uint8_t fg, bg;
+
+      fg = (grub_console_cur_color & 7);
+      bg = (grub_console_cur_color >> 4) & 7;
+
+      grub_console_attr = (grub_console_cur_color & 8) ? A_BOLD : A_NORMAL;
+      color_set ((bg << 3) + fg, 0);
+    }
 }
 
 /* XXX: This function is never called.  */
 static void
 grub_ncurses_setcolor (grub_uint8_t normal_color, grub_uint8_t highlight_color)
 {
-  color_set (normal_color << 8 | highlight_color, 0);
+  grub_console_normal_color = normal_color;
+  grub_console_highlight_color = highlight_color;
+}
+
+static void
+grub_ncurses_getcolor (grub_uint8_t *normal_color, grub_uint8_t *highlight_color)
+{
+  *normal_color = grub_console_normal_color;
+  *highlight_color = grub_console_highlight_color;
 }
 
 static int saved_char = ERR;
@@ -272,7 +316,23 @@ grub_ncurses_init (void)
   nonl ();
   intrflush (stdscr, FALSE);
   keypad (stdscr, TRUE);
-  start_color ();
+
+  if (has_colors ())
+    {
+      start_color ();
+
+      if ((COLORS >= NUM_COLORS) && (COLOR_PAIRS >= NUM_COLORS * NUM_COLORS))
+        {
+          int i, j, n;
+
+          n = 0;
+          for (i = 0; i < NUM_COLORS; i++)
+            for (j = 0; j < NUM_COLORS; j++)
+              init_pair(n++, color_map[j], color_map[i]);
+
+          use_color = 1;
+        }
+    }
 
   return 0;
 }
@@ -300,6 +360,7 @@ static struct grub_term grub_ncurses_term =
     .cls = grub_ncurses_cls,
     .setcolorstate = grub_ncurses_setcolorstate,
     .setcolor = grub_ncurses_setcolor,
+    .getcolor = grub_ncurses_getcolor,
     .setcursor = grub_ncurses_setcursor,
     .refresh = grub_ncurses_refresh,
     .flags = 0,

Re: [PATCH] Fix color problem the grub-emu

[Vesa Jääskeläinen]

Bean wrote:
> Hi,
> 
> Currently, the color handling in grub-emu is broken, sometimes you see
> nothing on screen. This patch fix it, now variable menu_color_normal
> and menu_color_highlight works properly in grub-emu.

> +
> +static grub_uint8_t color_map[NUM_COLORS] =
> +{
> +  COLOR_BLACK,
> +  COLOR_BLUE,
> +  COLOR_GREEN,
> +  COLOR_CYAN,
> +  COLOR_RED,
> +  COLOR_MAGENTA,
> +  COLOR_YELLOW,
> +  COLOR_WHITE
> +};
> +

Doesn't also ncurses have 16 different colors for foreground and 8 for 
background? This is at least quite common for terminals.

On some terminals there is one bit controlling background highlight 
colors or text blinking.

Re: [PATCH] Fix color problem the grub-emu

[Bean]

On Tue, Aug 5, 2008 at 3:54 PM, Vesa Jääskeläinen <chaac@nic.fi> wrote:
> Bean wrote:
>>
>> Hi,
>>
>> Currently, the color handling in grub-emu is broken, sometimes you see
>> nothing on screen. This patch fix it, now variable menu_color_normal
>> and menu_color_highlight works properly in grub-emu.
>
>> +
>> +static grub_uint8_t color_map[NUM_COLORS] =
>> +{
>> +  COLOR_BLACK,
>> +  COLOR_BLUE,
>> +  COLOR_GREEN,
>> +  COLOR_CYAN,
>> +  COLOR_RED,
>> +  COLOR_MAGENTA,
>> +  COLOR_YELLOW,
>> +  COLOR_WHITE
>> +};
>> +
>
> Doesn't also ncurses have 16 different colors for foreground and 8 for
> background? This is at least quite common for terminals.
>
> On some terminals there is one bit controlling background highlight colors
> or text blinking.

Hi,

I check my system, it only has 8 foreground and 8 background, the
highlight color can be enabled with the A_BOLD flag.

-- 
Bean

Re: [PATCH] Fix color problem the grub-emu

[Marco Gerards]

Hi,

Bean <bean123ch@gmail.com> writes:

> Currently, the color handling in grub-emu is broken, sometimes you see
> nothing on screen. This patch fix it, now variable menu_color_normal
> and menu_color_highlight works properly in grub-emu.
>
> 2008-08-02  Bean  <bean123ch@gmail.com>
>
> 	* util/console.c (grub_console_cur_color): New variable.
> 	(grub_console_standard_color): Likewise.
> 	(grub_console_normal_color): Likewise.
> 	(grub_console_highlight_color): Likewise.
> 	(color_map): Likewise.
> 	(use_color): Likewise.
> 	(NUM_COLORS): New macro.
> 	(grub_ncurses_setcolorstate): Handle color properly.
> 	(grub_ncurses_setcolor): Don't change color here, just remember the
> 	settings, color will be set in grub_ncurses_setcolorstate.
> 	(grub_ncurses_getcolor): New function.
> 	(grub_ncurses_init): Initialize color pairs.
> 	(grub_term grub_ncurses_term): New member grub_ncurses_getcolor.

This should be:

 	(grub_ncurses_term): New member grub_ncurses_getcolor.

> -- 
> Bean
>
> diff --git a/util/console.c b/util/console.c
> index 8c9401c..53fc5d0 100644
> --- a/util/console.c
> +++ b/util/console.c
> @@ -41,6 +41,28 @@
>  
>  static int grub_console_attr = A_NORMAL;
>  
> +grub_uint8_t grub_console_cur_color = 7;

Any reason why this is not static?



Otherwise, like usual, your patch looks fine at first sight.  If it
works and no one else has objections just commit it after making these
two changes :-)

--
Marco

Re: [PATCH] Fix color problem the grub-emu

[Bean]

On Tue, Aug 5, 2008 at 6:15 PM, Marco Gerards <mgerards@xs4all.nl> wrote:
> Hi,
>
> Bean <bean123ch@gmail.com> writes:
>
>> Currently, the color handling in grub-emu is broken, sometimes you see
>> nothing on screen. This patch fix it, now variable menu_color_normal
>> and menu_color_highlight works properly in grub-emu.
>>
>> 2008-08-02  Bean  <bean123ch@gmail.com>
>>
>>       * util/console.c (grub_console_cur_color): New variable.
>>       (grub_console_standard_color): Likewise.
>>       (grub_console_normal_color): Likewise.
>>       (grub_console_highlight_color): Likewise.
>>       (color_map): Likewise.
>>       (use_color): Likewise.
>>       (NUM_COLORS): New macro.
>>       (grub_ncurses_setcolorstate): Handle color properly.
>>       (grub_ncurses_setcolor): Don't change color here, just remember the
>>       settings, color will be set in grub_ncurses_setcolorstate.
>>       (grub_ncurses_getcolor): New function.
>>       (grub_ncurses_init): Initialize color pairs.
>>       (grub_term grub_ncurses_term): New member grub_ncurses_getcolor.
>
> This should be:
>
>        (grub_ncurses_term): New member grub_ncurses_getcolor.
>
>> --
>> Bean
>>
>> diff --git a/util/console.c b/util/console.c
>> index 8c9401c..53fc5d0 100644
>> --- a/util/console.c
>> +++ b/util/console.c
>> @@ -41,6 +41,28 @@
>>
>>  static int grub_console_attr = A_NORMAL;
>>
>> +grub_uint8_t grub_console_cur_color = 7;
>
> Any reason why this is not static?

Hi,

This files include <grub/i386/pc/console.h>, which define
grub_console_cur_color as global variable. Although grub-emu doesn't
use this variable itself, but if I define it as static, it will
generate a warning message.

-- 
Bean

Re: [PATCH] Fix color problem the grub-emu

[Marco Gerards]

Bean <bean123ch@gmail.com> writes:

> On Tue, Aug 5, 2008 at 6:15 PM, Marco Gerards <mgerards@xs4all.nl> wrote:
>> Hi,
>>
>> Bean <bean123ch@gmail.com> writes:
>>
>>> Currently, the color handling in grub-emu is broken, sometimes you see
>>> nothing on screen. This patch fix it, now variable menu_color_normal
>>> and menu_color_highlight works properly in grub-emu.
>>>
>>> 2008-08-02  Bean  <bean123ch@gmail.com>
>>>
>>>       * util/console.c (grub_console_cur_color): New variable.
>>>       (grub_console_standard_color): Likewise.
>>>       (grub_console_normal_color): Likewise.
>>>       (grub_console_highlight_color): Likewise.
>>>       (color_map): Likewise.
>>>       (use_color): Likewise.
>>>       (NUM_COLORS): New macro.
>>>       (grub_ncurses_setcolorstate): Handle color properly.
>>>       (grub_ncurses_setcolor): Don't change color here, just remember the
>>>       settings, color will be set in grub_ncurses_setcolorstate.
>>>       (grub_ncurses_getcolor): New function.
>>>       (grub_ncurses_init): Initialize color pairs.
>>>       (grub_term grub_ncurses_term): New member grub_ncurses_getcolor.
>>
>> This should be:
>>
>>        (grub_ncurses_term): New member grub_ncurses_getcolor.
>>
>>> --
>>> Bean
>>>
>>> diff --git a/util/console.c b/util/console.c
>>> index 8c9401c..53fc5d0 100644
>>> --- a/util/console.c
>>> +++ b/util/console.c
>>> @@ -41,6 +41,28 @@
>>>
>>>  static int grub_console_attr = A_NORMAL;
>>>
>>> +grub_uint8_t grub_console_cur_color = 7;
>>
>> Any reason why this is not static?
>
> Hi,
>
> This files include <grub/i386/pc/console.h>, which define
> grub_console_cur_color as global variable. Although grub-emu doesn't
> use this variable itself, but if I define it as static, it will
> generate a warning message.

Ah, ok :-)

--
Marco

Re: [PATCH] Fix color problem the grub-emu

[Bean]

On Tue, Aug 5, 2008 at 6:59 PM, Marco Gerards <mgerards@xs4all.nl> wrote:
> Bean <bean123ch@gmail.com> writes:
>
>> On Tue, Aug 5, 2008 at 6:15 PM, Marco Gerards <mgerards@xs4all.nl> wrote:
>>> Hi,
>>>
>>> Bean <bean123ch@gmail.com> writes:
>>>
>>>> Currently, the color handling in grub-emu is broken, sometimes you see
>>>> nothing on screen. This patch fix it, now variable menu_color_normal
>>>> and menu_color_highlight works properly in grub-emu.
>>>>
>>>> 2008-08-02  Bean  <bean123ch@gmail.com>
>>>>
>>>>       * util/console.c (grub_console_cur_color): New variable.
>>>>       (grub_console_standard_color): Likewise.
>>>>       (grub_console_normal_color): Likewise.
>>>>       (grub_console_highlight_color): Likewise.
>>>>       (color_map): Likewise.
>>>>       (use_color): Likewise.
>>>>       (NUM_COLORS): New macro.
>>>>       (grub_ncurses_setcolorstate): Handle color properly.
>>>>       (grub_ncurses_setcolor): Don't change color here, just remember the
>>>>       settings, color will be set in grub_ncurses_setcolorstate.
>>>>       (grub_ncurses_getcolor): New function.
>>>>       (grub_ncurses_init): Initialize color pairs.
>>>>       (grub_term grub_ncurses_term): New member grub_ncurses_getcolor.
>>>
>>> This should be:
>>>
>>>        (grub_ncurses_term): New member grub_ncurses_getcolor.
>>>
>>>> --
>>>> Bean
>>>>
>>>> diff --git a/util/console.c b/util/console.c
>>>> index 8c9401c..53fc5d0 100644
>>>> --- a/util/console.c
>>>> +++ b/util/console.c
>>>> @@ -41,6 +41,28 @@
>>>>
>>>>  static int grub_console_attr = A_NORMAL;
>>>>
>>>> +grub_uint8_t grub_console_cur_color = 7;
>>>
>>> Any reason why this is not static?
>>
>> Hi,
>>
>> This files include <grub/i386/pc/console.h>, which define
>> grub_console_cur_color as global variable. Although grub-emu doesn't
>> use this variable itself, but if I define it as static, it will
>> generate a warning message.
>
> Ah, ok :-)

Committed.

-- 
Bean