r3172 regresses background_color handling

r3172 regresses background_color handling

[Colin Watson]

This commit:

revno: 3172
committer: Vladimir 'phcoder' Serbinenko <phcoder@gmail.com>
branch nick: grub
timestamp: Wed 2011-04-06 13:00:18 +0200
message:
        Output errors if theme loading failed.

        * grub-core/gfxmenu/gfxmenu.c (grub_gfxmenu_try): Move the call to
        grub_gfxterm_fullscreen on error paths to ...
        * grub-core/normal/menu.c (menu_init): ...here. Wait after showing
        theme loading error.

... regresses the handling of background_color: grub_gfxterm_fullscreen
sets up the virtual screen again, including setting the background
colour back to black, and now it's called every time we re-enter the
menu.  Two simple ways to reproduce this are:

  1) Put 'background_color 44,0,30' before the menuentry commands in
     grub.cfg.  The menu should show up with a purple background, but in
     fact is black.

  2) From a menu, press 'c' for a command line, then enter
     'background_color 44,0,30', which should turn the screen purple.
     Now press Escape to go back to the menu.  The background should
     stay purple, but in fact turns black.

What's the best fix for this?  grub_gfxterm_fullscreen can run before
the virtual screen is set up, so I don't think I can just use
virtual_screen.bg_color_display.  Can we go back to having
grub_gfxterm_fullscreen called once on startup and then only on error
paths, which I think was the previous behaviour?

Thanks,

-- 
Colin Watson                                       [cjwatson@ubuntu.com]

Re: r3172 regresses background_color handling

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 27.06.2011 17:46, Colin Watson wrote:
> This commit:
>
> revno: 3172
> committer: Vladimir 'phcoder' Serbinenko <phcoder@gmail.com>
> branch nick: grub
> timestamp: Wed 2011-04-06 13:00:18 +0200
> message:
>         Output errors if theme loading failed.
>
>         * grub-core/gfxmenu/gfxmenu.c (grub_gfxmenu_try): Move the call to
>         grub_gfxterm_fullscreen on error paths to ...
>         * grub-core/normal/menu.c (menu_init): ...here. Wait after showing
>         theme loading error.
>
> ... regresses the handling of background_color: grub_gfxterm_fullscreen
> sets up the virtual screen again, including setting the background
> colour back to black, and now it's called every time we re-enter the
> menu.  Two simple ways to reproduce this are:
>
>   1) Put 'background_color 44,0,30' before the menuentry commands in
>      grub.cfg.  The menu should show up with a purple background, but in
>      fact is black.
>
>   2) From a menu, press 'c' for a command line, then enter
>      'background_color 44,0,30', which should turn the screen purple.
>      Now press Escape to go back to the menu.  The background should
>      stay purple, but in fact turns black.
>
> What's the best fix for this?  grub_gfxterm_fullscreen can run before
> the virtual screen is set up, so I don't think I can just use
> virtual_screen.bg_color_display.  Can we go back to having
> grub_gfxterm_fullscreen called once on startup and then only on error
> paths, which I think was the previous behaviour?
The reason to run it there is to avoid that gfxterm stays in some stuck
mode if theme loading fails midway.
> Thanks,
>


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

Re: r3172 regresses background_color handling

[Colin Watson]

On Mon, Jun 27, 2011 at 06:19:23PM +0200, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 27.06.2011 17:46, Colin Watson wrote:
> > What's the best fix for this?  grub_gfxterm_fullscreen can run before
> > the virtual screen is set up, so I don't think I can just use
> > virtual_screen.bg_color_display.  Can we go back to having
> > grub_gfxterm_fullscreen called once on startup and then only on error
> > paths, which I think was the previous behaviour?
> 
> The reason to run it there is to avoid that gfxterm stays in some stuck
> mode if theme loading fails midway.

OK.  How about this patch then?

2011-06-28  Colin Watson  <cjwatson@ubuntu.com>

	* grub-core/term/gfxterm.c (grub_virtual_screen_setup): Use
	default_bg_color rather than black.
	(grub_gfxterm_fullscreen): Likewise.
	(grub_gfxterm_background_color_cmd): Save new background color in
	default_bg_color.

=== modified file 'grub-core/term/gfxterm.c'
--- grub-core/term/gfxterm.c	2011-04-21 10:07:10 +0000
+++ grub-core/term/gfxterm.c	2011-06-28 09:33:26 +0000
@@ -131,6 +131,7 @@ static unsigned int bitmap_width;
 static unsigned int bitmap_height;
 static struct grub_video_bitmap *bitmap;
 static int blend_text_bg;
+static grub_video_rgba_color_t default_bg_color = { 0, 0, 0, 0 };
 
 static struct grub_dirty_region dirty_region;
 
@@ -266,7 +267,8 @@ grub_virtual_screen_setup (unsigned int
 
   grub_video_set_active_render_target (render_target);
 
-  virtual_screen.bg_color_display = grub_video_map_rgba(0, 0, 0, 0);
+  virtual_screen.bg_color_display =
+    grub_video_map_rgba_color (default_bg_color);
 
   /* Clear out text buffer. */
   for (i = 0; i < virtual_screen.columns * virtual_screen.rows; i++)
@@ -338,8 +340,8 @@ grub_gfxterm_fullscreen (void)
   double_redraw = mode_info.mode_type & GRUB_VIDEO_MODE_TYPE_DOUBLE_BUFFERED
     && !(mode_info.mode_type & GRUB_VIDEO_MODE_TYPE_UPDATING_SWAP);
 
-  /* Make sure screen is black.  */
-  color = grub_video_map_rgb (0, 0, 0);
+  /* Make sure screen is set to the default background color.  */
+  color = grub_video_map_rgba_color (default_bg_color);
   grub_video_fill_rect (color, 0, 0, mode_info.width, mode_info.height);
   if (double_redraw)
     {
@@ -1189,7 +1191,6 @@ static grub_err_t
 grub_gfxterm_background_color_cmd (grub_command_t cmd __attribute__ ((unused)),
                                    int argc, char **args)
 {
-  grub_video_rgba_color_t color;
   struct grub_video_render_target *old_target;
 
   if (argc != 1)
@@ -1199,7 +1200,7 @@ grub_gfxterm_background_color_cmd (grub_
   if (grub_video_get_info (NULL) != GRUB_ERR_NONE)
     return grub_errno;
 
-  if (grub_video_parse_color (args[0], &color) != GRUB_ERR_NONE)
+  if (grub_video_parse_color (args[0], &default_bg_color) != GRUB_ERR_NONE)
     return grub_errno;
 
   /* Destroy existing background bitmap if loaded.  */
@@ -1216,9 +1217,10 @@ grub_gfxterm_background_color_cmd (grub_
      compatible with the text layer.  */
   grub_video_get_active_render_target (&old_target);
   grub_video_set_active_render_target (text_layer);
-  virtual_screen.bg_color = grub_video_map_rgba_color (color);
+  virtual_screen.bg_color = grub_video_map_rgba_color (default_bg_color);
   grub_video_set_active_render_target (old_target);
-  virtual_screen.bg_color_display = grub_video_map_rgba_color (color);
+  virtual_screen.bg_color_display =
+    grub_video_map_rgba_color (default_bg_color);
   blend_text_bg = 1;
 
   /* Mark whole screen as dirty.  */

Thanks,

-- 
Colin Watson                                       [cjwatson@ubuntu.com]