gfxterm distinction between fg color and cursor color

gfxterm distinction between fg color and cursor color

[Robert Millan]

Why does gfxterm make a distinction between foreground color and cursor
color?  Do we really want this?

Currently, the variable-based hooks for changing colors have no effect in
cursor color because of this.

How about supressing separate handling of cursor color, and simply use the
foreground color to draw it?

Or maybe it'd be better to have setcolor() set cursor color implicitly?

-- 
Robert Millan

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

Re: gfxterm distinction between fg color and cursor color

[Colin D Bennett]

On Thu, 24 Jul 2008 18:18:40 +0200
Robert Millan <rmh@aybabtu.com> wrote:

> Why does gfxterm make a distinction between foreground color and
> cursor color?  Do we really want this?
> 
> Currently, the variable-based hooks for changing colors have no
> effect in cursor color because of this.
> 
> How about supressing separate handling of cursor color, and simply
> use the foreground color to draw it?
> 
> Or maybe it'd be better to have setcolor() set cursor color
> implicitly?

I think it would probably be fine to simply use the foreground color as
the cursor color.

Re: gfxterm distinction between fg color and cursor color

[Robert Millan]

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

On Thu, Jul 24, 2008 at 02:13:31PM -0700, Colin D Bennett wrote:
> On Thu, 24 Jul 2008 18:18:40 +0200
> Robert Millan <rmh@aybabtu.com> wrote:
> 
> > Why does gfxterm make a distinction between foreground color and
> > cursor color?  Do we really want this?
> > 
> > Currently, the variable-based hooks for changing colors have no
> > effect in cursor color because of this.
> > 
> > How about supressing separate handling of cursor color, and simply
> > use the foreground color to draw it?
> > 
> > Or maybe it'd be better to have setcolor() set cursor color
> > implicitly?
> 
> I think it would probably be fine to simply use the foreground color as
> the cursor color.

Well, it took me a while but I finally got a minute to do this.  Here's the
patch.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

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

2008-08-18  Robert Millan  <rmh@aybabtu.com>

	* term/gfxterm.c (DEFAULT_CURSOR_COLOR): Remove.
	(struct grub_virtual_screen): Remove `cursor_color'.
	(grub_virtual_screen_setup): Remove `virtual_screen.cursor_color'
	initialization.
	(write_cursor): Use `virtual_screen.fg_color' to draw cursor.

Index: term/gfxterm.c
===================================================================
--- term/gfxterm.c	(revision 1820)
+++ term/gfxterm.c	(working copy)
@@ -42,7 +42,6 @@
 #define DEFAULT_STANDARD_COLOR  0x07
 #define DEFAULT_NORMAL_COLOR    0x07
 #define DEFAULT_HIGHLIGHT_COLOR 0x70
-#define DEFAULT_CURSOR_COLOR	0x07
 
 struct grub_dirty_region
 {
@@ -100,7 +99,6 @@ struct grub_virtual_screen
   /* Color settings.  */
   grub_video_color_t fg_color;
   grub_video_color_t bg_color;
-  grub_video_color_t cursor_color;
 
   /* Text buffer for virtual screen.  Contains (columns * rows) number
      of entries.  */
@@ -219,8 +217,6 @@ grub_virtual_screen_setup (unsigned int 
   
   set_term_color (virtual_screen.term_color);
 
-  virtual_screen.cursor_color = grub_video_map_color (DEFAULT_CURSOR_COLOR);
-
   grub_video_set_active_render_target (GRUB_VIDEO_RENDER_TARGET_DISPLAY);
 
   return grub_errno;
@@ -709,7 +705,7 @@ write_cursor (void)
   width = virtual_screen.char_width;
   height = 2;
 
-  color = virtual_screen.cursor_color;
+  color = virtual_screen.fg_color;
 
   /* Render cursor to text layer.  */
   grub_video_set_active_render_target (text_layer);

Re: gfxterm distinction between fg color and cursor color

[Robert Millan]

Committed.

On Mon, Aug 18, 2008 at 04:48:43PM +0200, Robert Millan wrote:
> On Thu, Jul 24, 2008 at 02:13:31PM -0700, Colin D Bennett wrote:
> > On Thu, 24 Jul 2008 18:18:40 +0200
> > Robert Millan <rmh@aybabtu.com> wrote:
> > 
> > > Why does gfxterm make a distinction between foreground color and
> > > cursor color?  Do we really want this?
> > > 
> > > Currently, the variable-based hooks for changing colors have no
> > > effect in cursor color because of this.
> > > 
> > > How about supressing separate handling of cursor color, and simply
> > > use the foreground color to draw it?
> > > 
> > > Or maybe it'd be better to have setcolor() set cursor color
> > > implicitly?
> > 
> > I think it would probably be fine to simply use the foreground color as
> > the cursor color.
> 
> Well, it took me a while but I finally got a minute to do this.  Here's the
> patch.
> 
> -- 
> Robert Millan
> 
>   The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
>   how) you may access your data; but nobody's threatening your freedom: we
>   still allow you to remove your data and not access it at all."

> 2008-08-18  Robert Millan  <rmh@aybabtu.com>
> 
> 	* term/gfxterm.c (DEFAULT_CURSOR_COLOR): Remove.
> 	(struct grub_virtual_screen): Remove `cursor_color'.
> 	(grub_virtual_screen_setup): Remove `virtual_screen.cursor_color'
> 	initialization.
> 	(write_cursor): Use `virtual_screen.fg_color' to draw cursor.
> 
> Index: term/gfxterm.c
> ===================================================================
> --- term/gfxterm.c	(revision 1820)
> +++ term/gfxterm.c	(working copy)
> @@ -42,7 +42,6 @@
>  #define DEFAULT_STANDARD_COLOR  0x07
>  #define DEFAULT_NORMAL_COLOR    0x07
>  #define DEFAULT_HIGHLIGHT_COLOR 0x70
> -#define DEFAULT_CURSOR_COLOR	0x07
>  
>  struct grub_dirty_region
>  {
> @@ -100,7 +99,6 @@ struct grub_virtual_screen
>    /* Color settings.  */
>    grub_video_color_t fg_color;
>    grub_video_color_t bg_color;
> -  grub_video_color_t cursor_color;
>  
>    /* Text buffer for virtual screen.  Contains (columns * rows) number
>       of entries.  */
> @@ -219,8 +217,6 @@ grub_virtual_screen_setup (unsigned int 
>    
>    set_term_color (virtual_screen.term_color);
>  
> -  virtual_screen.cursor_color = grub_video_map_color (DEFAULT_CURSOR_COLOR);
> -
>    grub_video_set_active_render_target (GRUB_VIDEO_RENDER_TARGET_DISPLAY);
>  
>    return grub_errno;
> @@ -709,7 +705,7 @@ write_cursor (void)
>    width = virtual_screen.char_width;
>    height = 2;
>  
> -  color = virtual_screen.cursor_color;
> +  color = virtual_screen.fg_color;
>  
>    /* Render cursor to text layer.  */
>    grub_video_set_active_render_target (text_layer);

> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel


-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."