[PATCH] colored menu

[PATCH] colored menu

[Robert Millan]

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


This patch adds a nice looking colored menu like the one you'd obtain on GRUB
Legacy with "color cyan/blue white/blue" command.  Screenshot is attached as
well.

A pair of notes:

  - The *_setcolor function stubs are being removed from some terminals
  because grub_setcolor() from kernel already skips undefined *_setcolor
  implementations so there's no need for an empty stub.  In fact, not needed
  for this patch but adds confusion (I had to inspect all them to see if
  *_getcolor() had to be added).

  - I know that hardcoding colors is not so nice, but I was unsure how would
  the selection interface have to look like (and lacking time to implement it),
  and in comparison with defaulting to hardcoded 0x07 (grey on black) I see it
  as an improvement.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

[-- Attachment #2: colored_menu.diff --]
[-- Type: text/x-diff, Size: 11144 bytes --]


	* include/grub/term.h (struct grub_term): Add `getcolor' function.
	(grub_getcolor): New function.

	* kern/term.c (grub_getcolor): New function.
	* normal/menu.c (GRUB_COLOR_MENU_NORMAL): New macro.
	(GRUB_COLOR_MENU_HIGHLIGHT): New macro.
	(print_entry): Set normal and highlight colors to
	`GRUB_COLOR_MENU_NORMAL' and `GRUB_COLOR_MENU_HIGHLIGHT',
	respectively, before printing and restore them to old
	values afterwards.
	(run_menu): Likewise.

	* term/efi/console.c (grub_console_getcolor): New function.
	(struct grub_console_term.getcolor): New variable.
	* term/gfxterm.c (grub_virtual_screen_getcolor): New function.
	(struct grub_console_term.getcolor): New variable.
	* term/i386/pc/console.c (grub_console_getcolor): New function.
	(struct grub_console_term.getcolor): New variable.
	* term/ieee1275/ofconsole.c (grub_ofconsole_getcolor): New function.
	(struct grub_console_term.getcolor): New variable.

	* term/i386/pc/serial.c (grub_serial_setcolor): Remove function.
	(struct grub_console_term.setcolor): Remove variable.
	* term/i386/pc/vesafb.c (grub_virtual_screen_setcolor): Remove function.
	(struct grub_console_term.setcolor): Remove variable.
	* term/i386/pc/vga.c (grub_vga_setcolor): Remove function.
	(struct grub_console_term.setcolor): Remove variable.

diff -x '*~' -x configure -Nurp ../grub2/include/grub/term.h ./include/grub/term.h
--- ../grub2/include/grub/term.h	2007-07-22 01:32:22.000000000 +0200
+++ ./include/grub/term.h	2007-12-23 21:20:42.000000000 +0100
@@ -164,6 +164,10 @@ struct grub_term
      color is VGA's.  */
   void (*setcolor) (grub_uint8_t normal_color, grub_uint8_t highlight_color);
   
+  /* Get the normal color and the highlight color. The format of each
+     color is VGA's.  */
+  void (*getcolor) (grub_uint8_t *normal_color, grub_uint8_t *highlight_color);
+  
   /* Turn on/off the cursor.  */
   void (*setcursor) (int on);
 
@@ -197,6 +201,8 @@ void EXPORT_FUNC(grub_cls) (void);
 void EXPORT_FUNC(grub_setcolorstate) (grub_term_color_state state);
 void EXPORT_FUNC(grub_setcolor) (grub_uint8_t normal_color,
 				 grub_uint8_t highlight_color);
+void EXPORT_FUNC(grub_getcolor) (grub_uint8_t *normal_color,
+				 grub_uint8_t *highlight_color);
 int EXPORT_FUNC(grub_setcursor) (int on);
 int EXPORT_FUNC(grub_getcursor) (void);
 void EXPORT_FUNC(grub_refresh) (void);
diff -x '*~' -x configure -Nurp ../grub2/kern/term.c ./kern/term.c
--- ../grub2/kern/term.c	2007-07-22 01:32:26.000000000 +0200
+++ ./kern/term.c	2007-12-23 21:08:10.000000000 +0100
@@ -230,6 +230,13 @@ grub_setcolor (grub_uint8_t normal_color
     (grub_cur_term->setcolor) (normal_color, highlight_color);
 }
 
+void
+grub_getcolor (grub_uint8_t *normal_color, grub_uint8_t *highlight_color)
+{
+  if (grub_cur_term->getcolor)
+    (grub_cur_term->getcolor) (normal_color, highlight_color);
+}
+
 int
 grub_setcursor (int on)
 {
diff -x '*~' -x configure -Nurp ../grub2/normal/menu.c ./normal/menu.c
--- ../grub2/normal/menu.c	2007-11-10 21:32:32.000000000 +0100
+++ ./normal/menu.c	2007-12-23 21:35:23.000000000 +0100
@@ -25,6 +25,9 @@
 #include <grub/env.h>
 #include <grub/script.h>
 
+#define GRUB_COLOR_MENU_NORMAL		0x13
+#define GRUB_COLOR_MENU_HIGHLIGHT	0x1f
+
 static void
 draw_border (void)
 {
@@ -105,7 +108,8 @@ print_entry (int y, int highlight, grub_
   grub_ssize_t len;
   grub_uint32_t *unicode_title;
   grub_ssize_t i;
-  
+  grub_uint8_t normal_code, highlight_code;
+
   title = entry ? entry->title : "";
   unicode_title = grub_malloc (grub_strlen (title) * sizeof (*unicode_title));
   if (! unicode_title)
@@ -121,6 +125,8 @@ print_entry (int y, int highlight, grub_
       return;
     }
   
+  grub_getcolor (&normal_code, &highlight_code);
+  grub_setcolor (GRUB_COLOR_MENU_NORMAL, GRUB_COLOR_MENU_HIGHLIGHT);
   grub_setcolorstate (highlight
 		      ? GRUB_TERM_COLOR_HIGHLIGHT
 		      : GRUB_TERM_COLOR_NORMAL);
@@ -128,7 +134,7 @@ print_entry (int y, int highlight, grub_
   grub_gotoxy (GRUB_TERM_LEFT_BORDER_X + GRUB_TERM_MARGIN, y);
 
   for (x = GRUB_TERM_LEFT_BORDER_X + GRUB_TERM_MARGIN + 1, i = 0;
-       x < GRUB_TERM_LEFT_BORDER_X + GRUB_TERM_BORDER_WIDTH - GRUB_TERM_MARGIN;
+       x <= GRUB_TERM_LEFT_BORDER_X + GRUB_TERM_BORDER_WIDTH - GRUB_TERM_MARGIN;
        i++)
     {
       if (i < len
@@ -155,6 +161,7 @@ print_entry (int y, int highlight, grub_
     }
   grub_gotoxy (GRUB_TERM_CURSOR_X, y);
 
+  grub_setcolor (normal_code, highlight_code);
   grub_setcolorstate (GRUB_TERM_COLOR_STANDARD);
   grub_free (unicode_title);
 }
@@ -279,6 +286,7 @@ run_menu (grub_menu_t menu, int nested)
   int first, offset;
   unsigned long saved_time;
   int default_entry;
+  grub_uint8_t normal_code, highlight_code;
   
   first = 0;
   
@@ -301,8 +309,11 @@ run_menu (grub_menu_t menu, int nested)
 
  refresh:
   grub_setcursor (0);
+  grub_getcolor (&normal_code, &highlight_code);
+  grub_setcolor (GRUB_COLOR_MENU_NORMAL, GRUB_COLOR_MENU_HIGHLIGHT);
   grub_menu_init_page (nested, 0);
   print_entries (menu, first, offset);
+  grub_setcolor (normal_code, highlight_code);
   grub_refresh ();
 
   while (1)
diff -x '*~' -x configure -Nurp ../grub2/term/efi/console.c ./term/efi/console.c
--- ../grub2/term/efi/console.c	2007-07-22 01:32:30.000000000 +0200
+++ ./term/efi/console.c	2007-12-23 21:42:35.000000000 +0100
@@ -260,6 +260,13 @@ grub_console_setcolor (grub_uint8_t norm
 }
 
 static void
+grub_console_getcolor (grub_uint8_t *normal_color, grub_uint8_t *highlight_color)
+{
+  *normal_color = grub_console_normal_color;
+  *highlight_color = grub_console_highlight_color;
+}
+
+static void
 grub_console_setcursor (int on)
 {
   grub_efi_simple_text_output_interface_t *o;
@@ -283,6 +290,7 @@ static struct grub_term grub_console_ter
     .cls = grub_console_cls,
     .setcolorstate = grub_console_setcolorstate,
     .setcolor = grub_console_setcolor,
+    .getcolor = grub_console_getcolor,
     .setcursor = grub_console_setcursor,
     .flags = 0,
     .next = 0
diff -x '*~' -x configure -Nurp ../grub2/term/gfxterm.c ./term/gfxterm.c
--- ../grub2/term/gfxterm.c	2007-07-22 01:32:30.000000000 +0200
+++ ./term/gfxterm.c	2007-12-23 21:43:40.000000000 +0100
@@ -848,6 +848,13 @@ grub_virtual_screen_setcolor (grub_uint8
 }
 
 static void
+grub_virtual_screen_getcolor (grub_uint8_t *normal_color, grub_uint8_t *highlight_color)
+{
+  *normal_color = virtual_screen.fg_color_setting;
+  *highlight_color = virtual_screen.bg_color_setting;
+}
+
+static void
 grub_gfxterm_setcursor (int on)
 {
   if (virtual_screen.cursor_state != on)
@@ -883,6 +890,7 @@ static struct grub_term grub_video_term 
     .cls = grub_gfxterm_cls,
     .setcolorstate = grub_virtual_screen_setcolorstate,
     .setcolor = grub_virtual_screen_setcolor,
+    .getcolor = grub_virtual_screen_getcolor,
     .setcursor = grub_gfxterm_setcursor,
     .refresh = grub_gfxterm_refresh,
     .flags = 0,
diff -x '*~' -x configure -Nurp ../grub2/term/i386/pc/console.c ./term/i386/pc/console.c
--- ../grub2/term/i386/pc/console.c	2007-07-22 01:32:30.000000000 +0200
+++ ./term/i386/pc/console.c	2007-12-23 21:09:59.000000000 +0100
@@ -117,6 +117,13 @@ grub_console_setcolor (grub_uint8_t norm
   grub_console_highlight_color = highlight_color;
 }
 
+static void
+grub_console_getcolor (grub_uint8_t *normal_color, grub_uint8_t *highlight_color)
+{
+  *normal_color = grub_console_normal_color;
+  *highlight_color = grub_console_highlight_color;
+}
+
 static struct grub_term grub_console_term =
   {
     .name = "console",
@@ -132,6 +139,7 @@ static struct grub_term grub_console_ter
     .cls = grub_console_cls,
     .setcolorstate = grub_console_setcolorstate,
     .setcolor = grub_console_setcolor,
+    .getcolor = grub_console_getcolor,
     .setcursor = grub_console_setcursor,
     .flags = 0,
     .next = 0
diff -x '*~' -x configure -Nurp ../grub2/term/i386/pc/serial.c ./term/i386/pc/serial.c
--- ../grub2/term/i386/pc/serial.c	2007-11-10 21:23:14.000000000 +0100
+++ ./term/i386/pc/serial.c	2007-12-23 21:41:22.000000000 +0100
@@ -454,13 +454,6 @@ grub_serial_setcolorstate (const grub_te
 }
 
 static void
-grub_serial_setcolor (grub_uint8_t normal_color __attribute__ ((unused)),
-                      grub_uint8_t highlight_color __attribute__ ((unused)))
-{
-  /* FIXME */
-}
-
-static void
 grub_serial_setcursor (const int on)
 {
   if (on)
@@ -483,7 +476,6 @@ static struct grub_term grub_serial_term
   .gotoxy = grub_serial_gotoxy,
   .cls = grub_serial_cls,
   .setcolorstate = grub_serial_setcolorstate,
-  .setcolor = grub_serial_setcolor,
   .setcursor = grub_serial_setcursor,
   .flags = 0,
   .next = 0
diff -x '*~' -x configure -Nurp ../grub2/term/i386/pc/vesafb.c ./term/i386/pc/vesafb.c
--- ../grub2/term/i386/pc/vesafb.c	2007-07-22 01:32:31.000000000 +0200
+++ ./term/i386/pc/vesafb.c	2007-12-23 21:41:09.000000000 +0100
@@ -565,13 +565,6 @@ grub_virtual_screen_setcolorstate (grub_
 }
 
 static void
-grub_virtual_screen_setcolor (grub_uint8_t normal_color __attribute__ ((unused)),
-			      grub_uint8_t highlight_color __attribute__ ((unused)))
-{
-  /* FIXME */
-}
-
-static void
 grub_vesafb_setcursor (int on)
 {
   if (virtual_screen.cursor_state != on)
@@ -599,7 +592,6 @@ static struct grub_term grub_vesafb_term
     .gotoxy = grub_vesafb_gotoxy,
     .cls = grub_vesafb_cls,
     .setcolorstate = grub_virtual_screen_setcolorstate,
-    .setcolor = grub_virtual_screen_setcolor,
     .setcursor = grub_vesafb_setcursor,
     .flags = 0,
     .next = 0
diff -x '*~' -x configure -Nurp ../grub2/term/i386/pc/vga.c ./term/i386/pc/vga.c
--- ../grub2/term/i386/pc/vga.c	2007-11-10 19:34:48.000000000 +0100
+++ ./term/i386/pc/vga.c	2007-12-23 21:40:50.000000000 +0100
@@ -460,13 +460,6 @@ grub_vga_setcolorstate (grub_term_color_
 }
 
 static void
-grub_vga_setcolor (grub_uint8_t normal_color __attribute__ ((unused)),
-		   grub_uint8_t highlight_color __attribute__ ((unused)))
-{
-  /* FIXME */
-}
-
-static void
 grub_vga_setcursor (int on)
 {
   if (cursor_state != on)
@@ -494,7 +487,6 @@ static struct grub_term grub_vga_term =
     .gotoxy = grub_vga_gotoxy,
     .cls = grub_vga_cls,
     .setcolorstate = grub_vga_setcolorstate,
-    .setcolor = grub_vga_setcolor,
     .setcursor = grub_vga_setcursor,
     .flags = 0,
     .next = 0
diff -x '*~' -x configure -Nurp ../grub2/term/ieee1275/ofconsole.c ./term/ieee1275/ofconsole.c
--- ../grub2/term/ieee1275/ofconsole.c	2007-07-22 11:05:11.000000000 +0200
+++ ./term/ieee1275/ofconsole.c	2007-12-23 21:45:11.000000000 +0100
@@ -129,6 +129,13 @@ grub_ofconsole_setcolor (grub_uint8_t no
   bgcolor = highlight_color;
 }
 
+static void
+grub_ofconsole_getcolor (grub_uint8_t *normal_color, grub_uint8_t *highlight_color)
+{
+  *normal_color = fgcolor;
+  *highlight_color = bgcolor;
+}
+
 static int
 grub_ofconsole_readkey (int *key)
 {
@@ -364,6 +371,7 @@ static struct grub_term grub_ofconsole_t
     .cls = grub_ofconsole_cls,
     .setcolorstate = grub_ofconsole_setcolorstate,
     .setcolor = grub_ofconsole_setcolor,
+    .getcolor = grub_ofconsole_getcolor,
     .setcursor = grub_ofconsole_setcursor,
     .refresh = grub_ofconsole_refresh,
     .flags = 0,

[-- Attachment #3: colored_menu.png --]
[-- Type: image/png, Size: 7525 bytes --]

Re: [PATCH] colored menu

[Robert Millan]

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


New patch.  Corrects a minor mistake when filling the last colum in the
menu.  Also, it doesn't change the colors on gfxterm since some reasjustments
were needed for that, and there's also a bug that makes it impossible to
change colors on the fly with gfxterm/vbe (I'll write more on that later).

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

[-- Attachment #2: colored_menu.diff --]
[-- Type: text/x-diff, Size: 9979 bytes --]


	* include/grub/term.h (struct grub_term): Add `getcolor' function.
	(grub_getcolor): New function.

	* kern/term.c (grub_getcolor): New function.
	* normal/menu.c (GRUB_COLOR_MENU_NORMAL): New macro.
	(GRUB_COLOR_MENU_HIGHLIGHT): New macro.
	(print_entry): Set normal and highlight colors to
	`GRUB_COLOR_MENU_NORMAL' and `GRUB_COLOR_MENU_HIGHLIGHT',
	respectively, before printing and restore them to old
	values afterwards.
	(grub_menu_init_page): Likewise.  Fill an additional colored space
	that would otherwise be left blank.

	* term/efi/console.c (grub_console_getcolor): New function.
	(struct grub_console_term.getcolor): New variable.
	* term/i386/pc/console.c (grub_console_getcolor): New function.
	(struct grub_console_term.getcolor): New variable.
	* term/ieee1275/ofconsole.c (grub_ofconsole_getcolor): New function.
	(struct grub_console_term.getcolor): New variable.

	* term/i386/pc/serial.c (grub_serial_setcolor): Remove function.
	(struct grub_console_term.setcolor): Remove variable.
	* term/i386/pc/vesafb.c (grub_virtual_screen_setcolor): Remove function.
	(struct grub_console_term.setcolor): Remove variable.
	* term/i386/pc/vga.c (grub_vga_setcolor): Remove function.
	(struct grub_console_term.setcolor): Remove variable.
	* term/gfxterm.c (grub_virtual_screen_setcolor): Remove function.
	(struct grub_console_term.setcolor): Remove variable.

diff -ur grub2/include/grub/term.h grub2.color/include/grub/term.h
--- grub2/include/grub/term.h	2007-07-22 01:32:22.000000000 +0200
+++ grub2.color/include/grub/term.h	2007-12-24 02:21:30.000000000 +0100
@@ -164,6 +164,10 @@
      color is VGA's.  */
   void (*setcolor) (grub_uint8_t normal_color, grub_uint8_t highlight_color);
   
+  /* Get the normal color and the highlight color. The format of each
+     color is VGA's.  */
+  void (*getcolor) (grub_uint8_t *normal_color, grub_uint8_t *highlight_color);
+  
   /* Turn on/off the cursor.  */
   void (*setcursor) (int on);
 
@@ -197,6 +201,8 @@
 void EXPORT_FUNC(grub_setcolorstate) (grub_term_color_state state);
 void EXPORT_FUNC(grub_setcolor) (grub_uint8_t normal_color,
 				 grub_uint8_t highlight_color);
+void EXPORT_FUNC(grub_getcolor) (grub_uint8_t *normal_color,
+				 grub_uint8_t *highlight_color);
 int EXPORT_FUNC(grub_setcursor) (int on);
 int EXPORT_FUNC(grub_getcursor) (void);
 void EXPORT_FUNC(grub_refresh) (void);
diff -ur grub2/kern/term.c grub2.color/kern/term.c
--- grub2/kern/term.c	2007-07-22 01:32:26.000000000 +0200
+++ grub2.color/kern/term.c	2007-12-24 02:21:30.000000000 +0100
@@ -230,6 +230,13 @@
     (grub_cur_term->setcolor) (normal_color, highlight_color);
 }
 
+void
+grub_getcolor (grub_uint8_t *normal_color, grub_uint8_t *highlight_color)
+{
+  if (grub_cur_term->getcolor)
+    (grub_cur_term->getcolor) (normal_color, highlight_color);
+}
+
 int
 grub_setcursor (int on)
 {
diff -ur grub2/normal/menu.c grub2.color/normal/menu.c
--- grub2/normal/menu.c	2007-11-10 21:32:32.000000000 +0100
+++ grub2.color/normal/menu.c	2007-12-24 02:21:30.000000000 +0100
@@ -25,6 +25,9 @@
 #include <grub/env.h>
 #include <grub/script.h>
 
+#define GRUB_COLOR_MENU_NORMAL		0x13
+#define GRUB_COLOR_MENU_HIGHLIGHT	0x1f
+
 static void
 draw_border (void)
 {
@@ -105,7 +108,8 @@
   grub_ssize_t len;
   grub_uint32_t *unicode_title;
   grub_ssize_t i;
-  
+  grub_uint8_t normal_code, highlight_code;
+
   title = entry ? entry->title : "";
   unicode_title = grub_malloc (grub_strlen (title) * sizeof (*unicode_title));
   if (! unicode_title)
@@ -121,6 +125,8 @@
       return;
     }
   
+  grub_getcolor (&normal_code, &highlight_code);
+  grub_setcolor (GRUB_COLOR_MENU_NORMAL, GRUB_COLOR_MENU_HIGHLIGHT);
   grub_setcolorstate (highlight
 		      ? GRUB_TERM_COLOR_HIGHLIGHT
 		      : GRUB_TERM_COLOR_NORMAL);
@@ -153,8 +159,12 @@
 	  x++;
 	}
     }
+  grub_setcolorstate (GRUB_TERM_COLOR_NORMAL);
+  grub_putchar (' ');
+
   grub_gotoxy (GRUB_TERM_CURSOR_X, y);
 
+  grub_setcolor (normal_code, highlight_code);
   grub_setcolorstate (GRUB_TERM_COLOR_STANDARD);
   grub_free (unicode_title);
 }
@@ -199,9 +209,15 @@
 void
 grub_menu_init_page (int nested, int edit)
 {
+  grub_uint8_t normal_code, highlight_code;
+  grub_getcolor (&normal_code, &highlight_code);
+  grub_setcolor (GRUB_COLOR_MENU_NORMAL, GRUB_COLOR_MENU_HIGHLIGHT);
+
   grub_normal_init_page ();
   draw_border ();
   print_message (nested, edit);
+
+  grub_setcolor (normal_code, highlight_code);
 }
 
 /* Return the current timeout. If the variable "timeout" is not set or
diff -ur grub2/term/efi/console.c grub2.color/term/efi/console.c
--- grub2/term/efi/console.c	2007-07-22 01:32:30.000000000 +0200
+++ grub2.color/term/efi/console.c	2007-12-24 02:21:30.000000000 +0100
@@ -260,6 +260,13 @@
 }
 
 static void
+grub_console_getcolor (grub_uint8_t *normal_color, grub_uint8_t *highlight_color)
+{
+  *normal_color = grub_console_normal_color;
+  *highlight_color = grub_console_highlight_color;
+}
+
+static void
 grub_console_setcursor (int on)
 {
   grub_efi_simple_text_output_interface_t *o;
@@ -283,6 +290,7 @@
     .cls = grub_console_cls,
     .setcolorstate = grub_console_setcolorstate,
     .setcolor = grub_console_setcolor,
+    .getcolor = grub_console_getcolor,
     .setcursor = grub_console_setcursor,
     .flags = 0,
     .next = 0
diff -ur grub2/term/gfxterm.c grub2.color/term/gfxterm.c
--- grub2/term/gfxterm.c	2007-07-22 01:32:30.000000000 +0200
+++ grub2.color/term/gfxterm.c	2007-12-24 02:21:30.000000000 +0100
@@ -840,14 +840,6 @@
 }
 
 static void
-grub_virtual_screen_setcolor (grub_uint8_t normal_color,
-                              grub_uint8_t highlight_color)
-{
-  virtual_screen.fg_color_setting = grub_video_map_color (normal_color);
-  virtual_screen.bg_color_setting = grub_video_map_color (highlight_color);
-}
-
-static void
 grub_gfxterm_setcursor (int on)
 {
   if (virtual_screen.cursor_state != on)
@@ -882,7 +874,6 @@
     .gotoxy = grub_gfxterm_gotoxy,
     .cls = grub_gfxterm_cls,
     .setcolorstate = grub_virtual_screen_setcolorstate,
-    .setcolor = grub_virtual_screen_setcolor,
     .setcursor = grub_gfxterm_setcursor,
     .refresh = grub_gfxterm_refresh,
     .flags = 0,
diff -ur grub2/term/i386/pc/console.c grub2.color/term/i386/pc/console.c
--- grub2/term/i386/pc/console.c	2007-07-22 01:32:30.000000000 +0200
+++ grub2.color/term/i386/pc/console.c	2007-12-24 02:21:30.000000000 +0100
@@ -117,6 +117,13 @@
   grub_console_highlight_color = highlight_color;
 }
 
+static void
+grub_console_getcolor (grub_uint8_t *normal_color, grub_uint8_t *highlight_color)
+{
+  *normal_color = grub_console_normal_color;
+  *highlight_color = grub_console_highlight_color;
+}
+
 static struct grub_term grub_console_term =
   {
     .name = "console",
@@ -132,6 +139,7 @@
     .cls = grub_console_cls,
     .setcolorstate = grub_console_setcolorstate,
     .setcolor = grub_console_setcolor,
+    .getcolor = grub_console_getcolor,
     .setcursor = grub_console_setcursor,
     .flags = 0,
     .next = 0
diff -ur grub2/term/i386/pc/serial.c grub2.color/term/i386/pc/serial.c
--- grub2/term/i386/pc/serial.c	2007-11-10 21:23:14.000000000 +0100
+++ grub2.color/term/i386/pc/serial.c	2007-12-24 02:21:30.000000000 +0100
@@ -454,13 +454,6 @@
 }
 
 static void
-grub_serial_setcolor (grub_uint8_t normal_color __attribute__ ((unused)),
-                      grub_uint8_t highlight_color __attribute__ ((unused)))
-{
-  /* FIXME */
-}
-
-static void
 grub_serial_setcursor (const int on)
 {
   if (on)
@@ -483,7 +476,6 @@
   .gotoxy = grub_serial_gotoxy,
   .cls = grub_serial_cls,
   .setcolorstate = grub_serial_setcolorstate,
-  .setcolor = grub_serial_setcolor,
   .setcursor = grub_serial_setcursor,
   .flags = 0,
   .next = 0
diff -ur grub2/term/i386/pc/vesafb.c grub2.color/term/i386/pc/vesafb.c
--- grub2/term/i386/pc/vesafb.c	2007-07-22 01:32:31.000000000 +0200
+++ grub2.color/term/i386/pc/vesafb.c	2007-12-24 02:21:30.000000000 +0100
@@ -565,13 +565,6 @@
 }
 
 static void
-grub_virtual_screen_setcolor (grub_uint8_t normal_color __attribute__ ((unused)),
-			      grub_uint8_t highlight_color __attribute__ ((unused)))
-{
-  /* FIXME */
-}
-
-static void
 grub_vesafb_setcursor (int on)
 {
   if (virtual_screen.cursor_state != on)
@@ -599,7 +592,6 @@
     .gotoxy = grub_vesafb_gotoxy,
     .cls = grub_vesafb_cls,
     .setcolorstate = grub_virtual_screen_setcolorstate,
-    .setcolor = grub_virtual_screen_setcolor,
     .setcursor = grub_vesafb_setcursor,
     .flags = 0,
     .next = 0
diff -ur grub2/term/i386/pc/vga.c grub2.color/term/i386/pc/vga.c
--- grub2/term/i386/pc/vga.c	2007-11-10 19:34:48.000000000 +0100
+++ grub2.color/term/i386/pc/vga.c	2007-12-24 02:21:30.000000000 +0100
@@ -460,13 +460,6 @@
 }
 
 static void
-grub_vga_setcolor (grub_uint8_t normal_color __attribute__ ((unused)),
-		   grub_uint8_t highlight_color __attribute__ ((unused)))
-{
-  /* FIXME */
-}
-
-static void
 grub_vga_setcursor (int on)
 {
   if (cursor_state != on)
@@ -494,7 +487,6 @@
     .gotoxy = grub_vga_gotoxy,
     .cls = grub_vga_cls,
     .setcolorstate = grub_vga_setcolorstate,
-    .setcolor = grub_vga_setcolor,
     .setcursor = grub_vga_setcursor,
     .flags = 0,
     .next = 0
diff -ur grub2/term/ieee1275/ofconsole.c grub2.color/term/ieee1275/ofconsole.c
--- grub2/term/ieee1275/ofconsole.c	2007-07-22 11:05:11.000000000 +0200
+++ grub2.color/term/ieee1275/ofconsole.c	2007-12-24 02:21:30.000000000 +0100
@@ -129,6 +129,13 @@
   bgcolor = highlight_color;
 }
 
+static void
+grub_ofconsole_getcolor (grub_uint8_t *normal_color, grub_uint8_t *highlight_color)
+{
+  *normal_color = fgcolor;
+  *highlight_color = bgcolor;
+}
+
 static int
 grub_ofconsole_readkey (int *key)
 {
@@ -364,6 +371,7 @@
     .cls = grub_ofconsole_cls,
     .setcolorstate = grub_ofconsole_setcolorstate,
     .setcolor = grub_ofconsole_setcolor,
+    .getcolor = grub_ofconsole_getcolor,
     .setcursor = grub_ofconsole_setcursor,
     .refresh = grub_ofconsole_refresh,
     .flags = 0,

gfxterm / grub_virtual_screen_setcolorstate()

[Robert Millan]

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

On Mon, Dec 24, 2007 at 02:28:18AM +0100, Robert Millan wrote:
> 
> New patch.  Corrects a minor mistake when filling the last colum in the
> menu.  Also, it doesn't change the colors on gfxterm since some reasjustments
> were needed for that, and there's also a bug that makes it impossible to
> change colors on the fly with gfxterm/vbe (I'll write more on that later).

This patch (relative to previous one) basicaly makes it work with a big
ugly kludge (see "| 0xff000000" line).  The problem seems to be that alpha
channel is set to 0 for all return values of grub_video_map_color() after
gfxterm initialization.  I checked the few places in the code where those
variables are zeroed, but to no avail.  I assume the problem is just they
haven't been initialised, but I just have no idea where they're supposed to.

Any clues?

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

[-- Attachment #2: gfxterm.diff --]
[-- Type: text/x-diff, Size: 4482 bytes --]

diff -ur grub2.color/term/gfxterm.c grub2.gfxterm/term/gfxterm.c
--- grub2.color/term/gfxterm.c	2007-12-24 02:21:30.000000000 +0100
+++ grub2.gfxterm/term/gfxterm.c	2007-12-24 02:07:31.000000000 +0100
@@ -38,10 +38,13 @@
 
 #define DEFAULT_BORDER_WIDTH	10
 
-#define DEFAULT_FG_COLOR    	0x0a
-#define DEFAULT_BG_COLOR    	0x00
 #define DEFAULT_CURSOR_COLOR	0x0f
 
+static grub_uint8_t grub_gfxterm_cur_color = 0x7;
+static grub_uint8_t grub_gfxterm_standard_color = 0x7;
+static grub_uint8_t grub_gfxterm_normal_color = 0x7;
+static grub_uint8_t grub_gfxterm_highlight_color = 0x70;
+
 struct grub_dirty_region
 {
   int top_left_x;
@@ -90,8 +93,6 @@
   int cursor_state;
 
   /* Color settings.  */
-  grub_video_color_t fg_color_setting;
-  grub_video_color_t bg_color_setting;
   grub_video_color_t fg_color;
   grub_video_color_t bg_color;
   grub_video_color_t cursor_color;
@@ -175,10 +176,8 @@
      we can only have those after mode is initialized.  */
   grub_video_set_active_render_target (text_layer);
 
-  virtual_screen.fg_color_setting = grub_video_map_color (DEFAULT_FG_COLOR);
-  virtual_screen.bg_color_setting = grub_video_map_color (DEFAULT_BG_COLOR);
-  virtual_screen.fg_color = virtual_screen.fg_color_setting;
-  virtual_screen.bg_color = virtual_screen.bg_color_setting;
+  virtual_screen.fg_color = grub_video_map_color (grub_gfxterm_cur_color & 0x0f);
+  virtual_screen.bg_color = grub_video_map_color (grub_gfxterm_cur_color >> 4);
   virtual_screen.cursor_color = grub_video_map_color (DEFAULT_CURSOR_COLOR);
 
   grub_video_set_active_render_target (GRUB_VIDEO_RENDER_TARGET_DISPLAY);
@@ -805,15 +804,12 @@
 static void
 grub_gfxterm_cls (void)
 {
-  grub_video_color_t color;
-
   /* Clear virtual screen.  */
   grub_virtual_screen_cls ();
 
   /* Clear text layer.  */
   grub_video_set_active_render_target (text_layer);
-  color = virtual_screen.bg_color_setting;
-  grub_video_fill_rect (color, 0, 0, mode_info.width, mode_info.height);
+  grub_video_fill_rect (virtual_screen.bg_color, 0, 0, mode_info.width, mode_info.height);
   grub_video_set_active_render_target (GRUB_VIDEO_RENDER_TARGET_DISPLAY);
 
   /* Mark virtual screen to be redrawn.  */
@@ -823,20 +819,39 @@
 static void
 grub_virtual_screen_setcolorstate (grub_term_color_state state)
 {
-  switch (state)
-    {
+  grub_video_color_t tmp;
+
+  switch (state) {
     case GRUB_TERM_COLOR_STANDARD:
+      grub_gfxterm_cur_color = grub_gfxterm_standard_color;
+      break;
     case GRUB_TERM_COLOR_NORMAL:
-      virtual_screen.fg_color = virtual_screen.fg_color_setting;
-      virtual_screen.bg_color = virtual_screen.bg_color_setting;
+      grub_gfxterm_cur_color = grub_gfxterm_normal_color;
       break;
     case GRUB_TERM_COLOR_HIGHLIGHT:
-      virtual_screen.fg_color = virtual_screen.bg_color_setting;
-      virtual_screen.bg_color = virtual_screen.fg_color_setting;
+      grub_gfxterm_cur_color = grub_gfxterm_highlight_color;
       break;
     default:
       break;
-    }
+  }
+
+  virtual_screen.fg_color = grub_video_map_color (grub_gfxterm_cur_color & 0x0f) | 0xff000000;
+  virtual_screen.bg_color = grub_video_map_color (grub_gfxterm_cur_color >> 4);
+}
+
+static void
+grub_virtual_screen_setcolor (grub_uint8_t normal_color,
+                              grub_uint8_t highlight_color)
+{
+  grub_gfxterm_normal_color = normal_color;
+  grub_gfxterm_highlight_color = highlight_color;
+}
+
+static void
+grub_virtual_screen_getcolor (grub_uint8_t *normal_color, grub_uint8_t *highlight_color)
+{
+  *normal_color = grub_gfxterm_normal_color;
+  *highlight_color = grub_gfxterm_highlight_color;
 }
 
 static void
@@ -874,6 +889,8 @@
     .gotoxy = grub_gfxterm_gotoxy,
     .cls = grub_gfxterm_cls,
     .setcolorstate = grub_virtual_screen_setcolorstate,
+    .setcolor = grub_virtual_screen_setcolor,
+    .getcolor = grub_virtual_screen_getcolor,
     .setcursor = grub_gfxterm_setcursor,
     .refresh = grub_gfxterm_refresh,
     .flags = 0,
diff -ur grub2.color/video/i386/pc/vbe.c grub2.gfxterm/video/i386/pc/vbe.c
--- grub2.color/video/i386/pc/vbe.c	2007-07-22 01:32:32.000000000 +0200
+++ grub2.gfxterm/video/i386/pc/vbe.c	2007-12-24 02:29:48.000000000 +0100
@@ -722,6 +722,8 @@
       value |= blue << render_target->mode_info.blue_field_pos;
       value |= alpha << render_target->mode_info.reserved_field_pos;
 
+      grub_printf ("alpha=%x, bits=%x\n", alpha, render_target->mode_info.reserved_field_pos);
+
       return value;
     }
 

Re: [PATCH] colored menu

[Yoshinori K. Okuji]

On Sunday 23 December 2007 22:09, Robert Millan wrote:
> This patch adds a nice looking colored menu like the one you'd obtain on
> GRUB Legacy with "color cyan/blue white/blue" command.  Screenshot is
> attached as well.
>
> A pair of notes:
>
>   - The *_setcolor function stubs are being removed from some terminals
>   because grub_setcolor() from kernel already skips undefined *_setcolor
>   implementations so there's no need for an empty stub.  In fact, not
> needed for this patch but adds confusion (I had to inspect all them to see
> if *_getcolor() had to be added).
>
>   - I know that hardcoding colors is not so nice, but I was unsure how
> would the selection interface have to look like (and lacking time to
> implement it), and in comparison with defaulting to hardcoded 0x07 (grey on
> black) I see it as an improvement.

I still prefer to keep the default as it is. I like that a user (or a 
distributor) would modify the colors in grub.cfg.

Anyway, this will be a temporary hack, right? I'd love to see a fancier 
interface built in. Mmh, I really need to allocate time to implement one...

Okuji

Re: [PATCH] colored menu

[Robert Millan]

On Mon, Dec 24, 2007 at 08:46:29PM +0100, Yoshinori K. Okuji wrote:
> 
> I still prefer to keep the default as it is. I like that a user (or a 
> distributor) would modify the colors in grub.cfg.

Distributors (/me puts Debian hat on) can easily patch it.

Anyway, which interface to use?  It seems to me that env variables would
be more fitting than the "color" command we had in GRUB Legacy.  I suggest
using two: E.g. "menu_normal_color=fg/bg" and "menu_highlight_color=fg/bg",
with the same *color_list[16] from GRUB Legacy.

> Anyway, this will be a temporary hack, right?

No, not really.  That patch will be necessary when colors can be modified in
grub.cfg, too.  The only difference will be that it'll only act based on user
input.

I plan to implement user input too, just not right now.  In the meantime it'd
be nice to have this feature in CVS.  If you like 0x07 / 0x70 more than
0x13 / 0x1f, we can have that as default so that distributors (/me puts debian
hat) can change it easily?

> I'd love to see a fancier 
> interface built in.

Will we have that anytime soon?

> Mmh, I really need to allocate time to implement one...

Would be very nice to see you back on the playground! :-)

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

Re: [PATCH] colored menu

[Yoshinori K. Okuji]

On Tuesday 25 December 2007 10:26, Robert Millan wrote:
> On Mon, Dec 24, 2007 at 08:46:29PM +0100, Yoshinori K. Okuji wrote:
> > I still prefer to keep the default as it is. I like that a user (or a
> > distributor) would modify the colors in grub.cfg.
>
> Distributors (/me puts Debian hat on) can easily patch it.
>
> Anyway, which interface to use?  It seems to me that env variables would
> be more fitting than the "color" command we had in GRUB Legacy.  I suggest
> using two: E.g. "menu_normal_color=fg/bg" and "menu_highlight_color=fg/bg",
> with the same *color_list[16] from GRUB Legacy.

As you might know, my preference is to implement a style sheet and apply it to 
a text-based interface as well as a graphic-based interface (which is still 
vaporware). But this will need time, so I can accept whatever a short-term 
hack.

> > Anyway, this will be a temporary hack, right?
>
> No, not really.  That patch will be necessary when colors can be modified
> in grub.cfg, too.  The only difference will be that it'll only act based on
> user input.
>
> I plan to implement user input too, just not right now.  In the meantime
> it'd be nice to have this feature in CVS.  If you like 0x07 / 0x70 more
> than 0x13 / 0x1f, we can have that as default so that distributors (/me
> puts debian hat) can change it easily?

It is okay for me.

> > I'd love to see a fancier
> > interface built in.
>
> Will we have that anytime soon?

I hope so. ;)

> > Mmh, I really need to allocate time to implement one...
>
> Would be very nice to see you back on the playground! :-)

Thanks.

Okuji

Re: [PATCH] colored menu

[Robert Millan]

On Tue, Dec 25, 2007 at 11:27:56AM +0100, Yoshinori K. Okuji wrote:
> > > Anyway, this will be a temporary hack, right?
> >
> > No, not really.  That patch will be necessary when colors can be modified
> > in grub.cfg, too.  The only difference will be that it'll only act based on
> > user input.
> >
> > I plan to implement user input too, just not right now.  In the meantime
> > it'd be nice to have this feature in CVS.  If you like 0x07 / 0x70 more
> > than 0x13 / 0x1f, we can have that as default so that distributors (/me
> > puts debian hat) can change it easily?
> 
> It is okay for me.

Ok, committed.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

Re: gfxterm / grub_virtual_screen_setcolorstate()

[Vesa Jääskeläinen]

Robert Millan wrote:
> On Mon, Dec 24, 2007 at 02:28:18AM +0100, Robert Millan wrote:
>> New patch.  Corrects a minor mistake when filling the last colum in the
>> menu.  Also, it doesn't change the colors on gfxterm since some reasjustments
>> were needed for that, and there's also a bug that makes it impossible to
>> change colors on the fly with gfxterm/vbe (I'll write more on that later).
> 
> This patch (relative to previous one) basicaly makes it work with a big
> ugly kludge (see "| 0xff000000" line).  The problem seems to be that alpha
> channel is set to 0 for all return values of grub_video_map_color() after
> gfxterm initialization.  I checked the few places in the code where those
> variables are zeroed, but to no avail.  I assume the problem is just they
> haven't been initialised, but I just have no idea where they're supposed to.
> 
> Any clues?

Ok... Your "fix" with OR'ing is not a good one as that makes an
assumption about display mode settings. Idea of mapping colors is to get
most optimal color settings for desired video mode. Nevertheless I think
I know the problem and I get back to you shortly... now I sleep :)

That zero alpha setting most likely comes from color table being
specified. (or from video mode settings currently in use, eg. no
alpha/reserved bits)

Re: gfxterm / grub_virtual_screen_setcolorstate()

[Marco Gerards]

Vesa Jääskeläinen <chaac@nic.fi> writes:

> Robert Millan wrote:
>> On Mon, Dec 24, 2007 at 02:28:18AM +0100, Robert Millan wrote:
>>> New patch.  Corrects a minor mistake when filling the last colum in the
>>> menu.  Also, it doesn't change the colors on gfxterm since some reasjustments
>>> were needed for that, and there's also a bug that makes it impossible to
>>> change colors on the fly with gfxterm/vbe (I'll write more on that later).
>> 
>> This patch (relative to previous one) basicaly makes it work with a big
>> ugly kludge (see "| 0xff000000" line).  The problem seems to be that alpha
>> channel is set to 0 for all return values of grub_video_map_color() after
>> gfxterm initialization.  I checked the few places in the code where those
>> variables are zeroed, but to no avail.  I assume the problem is just they
>> haven't been initialised, but I just have no idea where they're supposed to.
>> 
>> Any clues?
>
> Ok... Your "fix" with OR'ing is not a good one as that makes an
> assumption about display mode settings. Idea of mapping colors is to get
> most optimal color settings for desired video mode. Nevertheless I think
> I know the problem and I get back to you shortly... now I sleep :)
>
> That zero alpha setting most likely comes from color table being
> specified. (or from video mode settings currently in use, eg. no
> alpha/reserved bits)

This was fixed already?

--
Marco

Re: gfxterm / grub_virtual_screen_setcolorstate()

[Robert Millan]

On Wed, Jan 23, 2008 at 01:23:52PM +0100, Marco Gerards wrote:
> Vesa Jääskeläinen <chaac@nic.fi> writes:
> 
> > Robert Millan wrote:
> >> On Mon, Dec 24, 2007 at 02:28:18AM +0100, Robert Millan wrote:
> >>> New patch.  Corrects a minor mistake when filling the last colum in the
> >>> menu.  Also, it doesn't change the colors on gfxterm since some reasjustments
> >>> were needed for that, and there's also a bug that makes it impossible to
> >>> change colors on the fly with gfxterm/vbe (I'll write more on that later).
> >> 
> >> This patch (relative to previous one) basicaly makes it work with a big
> >> ugly kludge (see "| 0xff000000" line).  The problem seems to be that alpha
> >> channel is set to 0 for all return values of grub_video_map_color() after
> >> gfxterm initialization.  I checked the few places in the code where those
> >> variables are zeroed, but to no avail.  I assume the problem is just they
> >> haven't been initialised, but I just have no idea where they're supposed to.
> >> 
> >> Any clues?
> >
> > Ok... Your "fix" with OR'ing is not a good one as that makes an
> > assumption about display mode settings. Idea of mapping colors is to get
> > most optimal color settings for desired video mode. Nevertheless I think
> > I know the problem and I get back to you shortly... now I sleep :)
> >
> > That zero alpha setting most likely comes from color table being
> > specified. (or from video mode settings currently in use, eg. no
> > alpha/reserved bits)
> 
> This was fixed already?

Vesa fixed it.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)