Skip to contentSkip to conversation

ui/curses: Fix color attribute of monitor for curses

PATCH Superseded OGAWA Hirofumi 2 participants 5 messages Nov 29, 2015 – Dec 15, 2015

lore
Share & tools
lore

OGAWA Hirofumi

author
#0 lore raw

PATCH 1/1 ui/curses: Fix color attribute of monitor for curses

Current text_console_update() writes totally broken color attributes to console_write_ch(). The format now is writing,

[WRONG]

	bold << 21 | fg << 12 | bg << 8 | char
	fg == 3bits curses color number
	bg == 3bits curses color number

I can't see this format is where come from. Anyway, this doesn't work at all.

What curses expects is actually (and vga.c is using),

[RIGHT]

	bold << 21 | bg << 11 | fg << 8 | char
	fg == 3bits vga color number
	bg == 3bits vga color number

And curses set COLOR_PAIR() up to match this format, and curses's chtype. I.e,

	bold | color_pair | char
	color_pair == (bg << 3 | fg)

To fix, this simply uses VGA color number everywhere except curses.c internal. Then, convert it to above [RIGHT] format to write by console_write_ch(). And as bonus, this reduces to expose curses define to other parts (removes COLOR_* from console.c).

[Tested the first line is displayed as white on blue back for monitor in curses console]

Signed-off-by OGAWA Hirofumi <[email protected]>

---

 hw/display/jazz_led.c |    6 +-
 hw/display/vga.c      |    3 -
 include/ui/console.h  |   15 +++++++
 ui/console.c          |  101 +++++++++++++++++++++----------------------------
 ui/curses.c           |   13 ++++--
 5 files changed, 76 insertions(+), 62 deletions(-)
include/ui/console.h renamed from include/ui/console.h~fix-curses-colors-for-monitor +15 -0
diff -puN include/ui/console.h~fix-curses-colors-for-monitor include/ui/console.h
@@ -30,6 +30,21 @@
 #define GUI_REFRESH_INTERVAL_DEFAULT    30
 #define GUI_REFRESH_INTERVAL_IDLE     3000
 
+/* Color number is match to standard vga palette */
+enum qemu_color_names {
+    QEMU_COLOR_BLACK   = 0,
+    QEMU_COLOR_BLUE    = 1,
+    QEMU_COLOR_GREEN   = 2,
+    QEMU_COLOR_CYAN    = 3,
+    QEMU_COLOR_RED     = 4,
+    QEMU_COLOR_MAGENTA = 5,
+    QEMU_COLOR_YELLOW  = 6,
+    QEMU_COLOR_WHITE   = 7
+};
+/* Convert to curses char attributes */
+#define ATTR2CHTYPE(c, fg, bg, bold) \
+    ((bold) << 21 | (bg) << 11 | (fg) << 8 | (c))
+
 typedef void QEMUPutKBDEvent(void *opaque, int keycode);
 typedef void QEMUPutLEDEvent(void *opaque, int ledstate);
 typedef void QEMUPutMouseEvent(void *opaque, int dx, int dy, int dz, int buttons_state);
ui/console.c renamed from ui/console.c~fix-curses-colors-for-monitor +45 -56

181 lines not on this page · raw message

ui/curses.c renamed from ui/curses.c~fix-curses-colors-for-monitor +10 -3
diff -puN ui/curses.c~fix-curses-colors-for-monitor ui/curses.c
@@ -333,8 +333,14 @@ static void curses_atexit(void)
 static void curses_setup(void)
 {
     int i, colour_default[8] = {
-        COLOR_BLACK, COLOR_BLUE, COLOR_GREEN, COLOR_CYAN,
-        COLOR_RED, COLOR_MAGENTA, COLOR_YELLOW, COLOR_WHITE,
+        [QEMU_COLOR_BLACK]   = COLOR_BLACK,
+        [QEMU_COLOR_BLUE]    = COLOR_BLUE,
+        [QEMU_COLOR_GREEN]   = COLOR_GREEN,
+        [QEMU_COLOR_CYAN]    = COLOR_CYAN,
+        [QEMU_COLOR_RED]     = COLOR_RED,
+        [QEMU_COLOR_MAGENTA] = COLOR_MAGENTA,
+        [QEMU_COLOR_YELLOW]  = COLOR_YELLOW,
+        [QEMU_COLOR_WHITE]   = COLOR_WHITE,
     };
 
     /* input as raw as possible, let everything be interpreted

@@ -343,10 +349,11 @@ static void curses_setup(void)
     nodelay(stdscr, TRUE); nonl(); keypad(stdscr, TRUE);
     start_color(); raw(); scrollok(stdscr, FALSE);
 
+    /* Make color pair to match color format (3bits bg:3bits fg) */
     for (i = 0; i < 64; i++) {
         init_pair(i, colour_default[i & 7], colour_default[i >> 3]);
     }
-    /* Set default color for more than 64. (monitor uses 0x74xx for example) */
+    /* Set default color for more than 64 for safety. */
     for (i = 64; i < COLOR_PAIRS; i++) {
         init_pair(i, COLOR_WHITE, COLOR_BLACK);
     }
hw/display/jazz_led.c renamed from hw/display/jazz_led.c~fix-curses-colors-for-monitor +4 -2
diff -puN hw/display/jazz_led.c~fix-curses-colors-for-monitor hw/display/jazz_led.c
@@ -233,8 +233,10 @@ static void jazz_led_text_update(void *o
 
     /* TODO: draw the segments */
     snprintf(buf, 2, "%02hhx\n", s->segments);
-    console_write_ch(chardata++, 0x00200100 | buf[0]);
-    console_write_ch(chardata++, 0x00200100 | buf[1]);
+    console_write_ch(chardata++, ATTR2CHTYPE(buf[0], QEMU_COLOR_BLUE,
+                                             QEMU_COLOR_BLACK, 1));
+    console_write_ch(chardata++, ATTR2CHTYPE(buf[1], QEMU_COLOR_BLUE,
+                                             QEMU_COLOR_BLACK, 1));
 
     dpy_text_update(s->con, 0, 0, 2, 1);
 }
hw/display/vga.c renamed from hw/display/vga.c~fix-curses-colors-for-monitor +2 -1
diff -puN hw/display/vga.c~fix-curses-colors-for-monitor hw/display/vga.c
@@ -1979,7 +1979,8 @@ static void vga_update_text(void *opaque
     width = (s->last_width - size) / 2;
     dst = chardata + s->last_width + width;
     for (i = 0; i < size; i ++)
-        console_write_ch(dst ++, 0x00200100 | msg_buffer[i]);
+        console_write_ch(dst ++, ATTR2CHTYPE(msg_buffer[i], QEMU_COLOR_BLUE,
+                                             QEMU_COLOR_BLACK, 1));
 
     dpy_text_update(s->con, 0, 0, s->last_width, height);
 }

_

#1 lore raw
Quoted 24 lines · OGAWA Hirofumi

Current text_console_update() writes totally broken color attributes to console_write_ch(). The format now is writing,

[WRONG]

	bold << 21 | fg << 12 | bg << 8 | char
	fg == 3bits curses color number
	bg == 3bits curses color number

I can't see this format is where come from. Anyway, this doesn't work at all.

What curses expects is actually (and vga.c is using),

[RIGHT]

	bold << 21 | bg << 11 | fg << 8 | char
	fg == 3bits vga color number
	bg == 3bits vga color number

And curses set COLOR_PAIR() up to match this format, and curses's chtype. I.e,

	bold | color_pair | char
	color_pair == (bg << 3 | fg)
Quoted 7 lines

To fix, this simply uses VGA color number everywhere except curses.c internal. Then, convert it to above [RIGHT] format to write by console_write_ch(). And as bonus, this reduces to expose curses define to other parts (removes COLOR_* from console.c).

[Tested the first line is displayed as white on blue back for monitor in curses console]

Nice cleanup.

Quoted 8 lines
 static const pixman_color_t color_table_rgb[2][8] = {
     {   /* dark */
ui/console.c renamed from ui/console.c~fix-curses-colors-for-monitor +0 -6
-        QEMU_RGB(0x00, 0x00, 0x00),  /* black */
-        QEMU_RGB(0xaa, 0x00, 0x00),  /* red */
-        QEMU_RGB(0x00, 0xaa, 0x00),  /* green */
-        QEMU_RGB(0xaa, 0xaa, 0x00),  /* yellow */
-        QEMU_RGB(0x00, 0x00, 0xaa),  /* blue */
-        QEMU_RGB(0xaa, 0x00, 0xaa),  /* magenta */

ui/console.c static void vga_bitblt(QemuConsole *con, · line 390

Quoted 10 lines
ui/console.c renamed from ui/console.c~fix-curses-colors-for-monitor +8 -2
-        QEMU_RGB(0x00, 0xaa, 0xaa),  /* cyan */
-        QEMU_RGB(0xaa, 0xaa, 0xaa),  /* white */
+        [QEMU_COLOR_BLACK]   = QEMU_RGB(0x00, 0x00, 0x00),  /* black */
+        [QEMU_COLOR_BLUE]    = QEMU_RGB(0x00, 0x00, 0xaa),  /* blue */
+        [QEMU_COLOR_GREEN]   = QEMU_RGB(0x00, 0xaa, 0x00),  /* green */
+        [QEMU_COLOR_CYAN]    = QEMU_RGB(0x00, 0xaa, 0xaa),  /* cyan */
+        [QEMU_COLOR_RED]     = QEMU_RGB(0xaa, 0x00, 0x00),  /* red */
+        [QEMU_COLOR_MAGENTA] = QEMU_RGB(0xaa, 0x00, 0xaa),  /* magenta */
+        [QEMU_COLOR_YELLOW]  = QEMU_RGB(0xaa, 0xaa, 0x00),  /* yellow */
+        [QEMU_COLOR_WHITE]   = QEMU_RGB(0xaa, 0xaa, 0xaa),  /* white */

The color comments are kind of redundant now ...

cheers,
Gerd

author
#2 lore raw
Quoted 19 lines · Gerd Hoffmann
Quoted 8 lines
 static const pixman_color_t color_table_rgb[2][8] = {
     {   /* dark */
ui/console.c renamed from ui/console.c~fix-curses-colors-for-monitor +0 -6
-        QEMU_RGB(0x00, 0x00, 0x00),  /* black */
-        QEMU_RGB(0xaa, 0x00, 0x00),  /* red */
-        QEMU_RGB(0x00, 0xaa, 0x00),  /* green */
-        QEMU_RGB(0xaa, 0xaa, 0x00),  /* yellow */
-        QEMU_RGB(0x00, 0x00, 0xaa),  /* blue */
-        QEMU_RGB(0xaa, 0x00, 0xaa),  /* magenta */

ui/console.c static void vga_bitblt(QemuConsole *con, · line 390

Quoted 10 lines
ui/console.c renamed from ui/console.c~fix-curses-colors-for-monitor +8 -2
-        QEMU_RGB(0x00, 0xaa, 0xaa),  /* cyan */
-        QEMU_RGB(0xaa, 0xaa, 0xaa),  /* white */
+        [QEMU_COLOR_BLACK]   = QEMU_RGB(0x00, 0x00, 0x00),  /* black */
+        [QEMU_COLOR_BLUE]    = QEMU_RGB(0x00, 0x00, 0xaa),  /* blue */
+        [QEMU_COLOR_GREEN]   = QEMU_RGB(0x00, 0xaa, 0x00),  /* green */
+        [QEMU_COLOR_CYAN]    = QEMU_RGB(0x00, 0xaa, 0xaa),  /* cyan */
+        [QEMU_COLOR_RED]     = QEMU_RGB(0xaa, 0x00, 0x00),  /* red */
+        [QEMU_COLOR_MAGENTA] = QEMU_RGB(0xaa, 0x00, 0xaa),  /* magenta */
+        [QEMU_COLOR_YELLOW]  = QEMU_RGB(0xaa, 0xaa, 0x00),  /* yellow */
+        [QEMU_COLOR_WHITE]   = QEMU_RGB(0xaa, 0xaa, 0xaa),  /* white */
Quoted 1 line

The color comments are kind of redundant now ...

OK.

Signature · 1 line
OGAWA Hirofumi <[email protected]>
author
#3 lore raw

PATCH v2 1/1 ui/curses: Fix color attribute of monitor for curses

Current text_console_update() writes totally broken color attributes to console_write_ch(). The format now is writing,

[WRONG]

	bold << 21 | fg << 12 | bg << 8 | char
	fg == 3bits curses color number
	bg == 3bits curses color number

I can't see this format is where come from. Anyway, this doesn't work at all.

What curses expects is actually (and vga.c is using),

[RIGHT]

	bold << 21 | bg << 11 | fg << 8 | char
	fg == 3bits vga color number
	bg == 3bits vga color number

And curses set COLOR_PAIR() up to match this format, and curses's chtype. I.e,

	bold | color_pair | char
	color_pair == (bg << 3 | fg)

To fix, this simply uses VGA color number everywhere except curses.c internal. Then, convert it to above [RIGHT] format to write by console_write_ch(). And as bonus, this reduces to expose curses define to other parts (removes COLOR_* from console.c).

[Tested the first line is displayed as white on blue back for monitor in curses console]

Signed-off-by OGAWA Hirofumi <[email protected]>

---

 hw/display/jazz_led.c |    6 +-
 hw/display/vga.c      |    3 -
 include/ui/console.h  |   15 +++++++
 ui/console.c          |  101 +++++++++++++++++++++----------------------------
 ui/curses.c           |   13 ++++--
 5 files changed, 76 insertions(+), 62 deletions(-)
include/ui/console.h renamed from include/ui/console.h~fix-curses-colors-for-monitor +15 -0
diff -puN include/ui/console.h~fix-curses-colors-for-monitor include/ui/console.h
@@ -30,6 +30,21 @@
 #define GUI_REFRESH_INTERVAL_DEFAULT    30
 #define GUI_REFRESH_INTERVAL_IDLE     3000
 
+/* Color number is match to standard vga palette */
+enum qemu_color_names {
+    QEMU_COLOR_BLACK   = 0,
+    QEMU_COLOR_BLUE    = 1,
+    QEMU_COLOR_GREEN   = 2,
+    QEMU_COLOR_CYAN    = 3,
+    QEMU_COLOR_RED     = 4,
+    QEMU_COLOR_MAGENTA = 5,
+    QEMU_COLOR_YELLOW  = 6,
+    QEMU_COLOR_WHITE   = 7
+};
+/* Convert to curses char attributes */
+#define ATTR2CHTYPE(c, fg, bg, bold) \
+    ((bold) << 21 | (bg) << 11 | (fg) << 8 | (c))
+
 typedef void QEMUPutKBDEvent(void *opaque, int keycode);
 typedef void QEMUPutLEDEvent(void *opaque, int ledstate);
 typedef void QEMUPutMouseEvent(void *opaque, int dx, int dy, int dz, int buttons_state);
ui/console.c renamed from ui/console.c~fix-curses-colors-for-monitor +45 -56

181 lines not on this page · raw message

ui/curses.c renamed from ui/curses.c~fix-curses-colors-for-monitor +10 -3
diff -puN ui/curses.c~fix-curses-colors-for-monitor ui/curses.c
@@ -333,8 +333,14 @@ static void curses_atexit(void)
 static void curses_setup(void)
 {
     int i, colour_default[8] = {
-        COLOR_BLACK, COLOR_BLUE, COLOR_GREEN, COLOR_CYAN,
-        COLOR_RED, COLOR_MAGENTA, COLOR_YELLOW, COLOR_WHITE,
+        [QEMU_COLOR_BLACK]   = COLOR_BLACK,
+        [QEMU_COLOR_BLUE]    = COLOR_BLUE,
+        [QEMU_COLOR_GREEN]   = COLOR_GREEN,
+        [QEMU_COLOR_CYAN]    = COLOR_CYAN,
+        [QEMU_COLOR_RED]     = COLOR_RED,
+        [QEMU_COLOR_MAGENTA] = COLOR_MAGENTA,
+        [QEMU_COLOR_YELLOW]  = COLOR_YELLOW,
+        [QEMU_COLOR_WHITE]   = COLOR_WHITE,
     };
 
     /* input as raw as possible, let everything be interpreted

@@ -343,10 +349,11 @@ static void curses_setup(void)
     nodelay(stdscr, TRUE); nonl(); keypad(stdscr, TRUE);
     start_color(); raw(); scrollok(stdscr, FALSE);
 
+    /* Make color pair to match color format (3bits bg:3bits fg) */
     for (i = 0; i < 64; i++) {
         init_pair(i, colour_default[i & 7], colour_default[i >> 3]);
     }
-    /* Set default color for more than 64. (monitor uses 0x74xx for example) */
+    /* Set default color for more than 64 for safety. */
     for (i = 64; i < COLOR_PAIRS; i++) {
         init_pair(i, COLOR_WHITE, COLOR_BLACK);
     }
hw/display/jazz_led.c renamed from hw/display/jazz_led.c~fix-curses-colors-for-monitor +4 -2
diff -puN hw/display/jazz_led.c~fix-curses-colors-for-monitor hw/display/jazz_led.c
@@ -233,8 +233,10 @@ static void jazz_led_text_update(void *o
 
     /* TODO: draw the segments */
     snprintf(buf, 2, "%02hhx\n", s->segments);
-    console_write_ch(chardata++, 0x00200100 | buf[0]);
-    console_write_ch(chardata++, 0x00200100 | buf[1]);
+    console_write_ch(chardata++, ATTR2CHTYPE(buf[0], QEMU_COLOR_BLUE,
+                                             QEMU_COLOR_BLACK, 1));
+    console_write_ch(chardata++, ATTR2CHTYPE(buf[1], QEMU_COLOR_BLUE,
+                                             QEMU_COLOR_BLACK, 1));
 
     dpy_text_update(s->con, 0, 0, 2, 1);
 }
hw/display/vga.c renamed from hw/display/vga.c~fix-curses-colors-for-monitor +2 -1
diff -puN hw/display/vga.c~fix-curses-colors-for-monitor hw/display/vga.c
@@ -1979,7 +1979,8 @@ static void vga_update_text(void *opaque
     width = (s->last_width - size) / 2;
     dst = chardata + s->last_width + width;
     for (i = 0; i < size; i ++)
-        console_write_ch(dst ++, 0x00200100 | msg_buffer[i]);
+        console_write_ch(dst ++, ATTR2CHTYPE(msg_buffer[i], QEMU_COLOR_BLUE,
+                                             QEMU_COLOR_BLACK, 1));
 
     dpy_text_update(s->con, 0, 0, s->last_width, height);
 }

_

#4 lore raw
Quoted 24 lines · OGAWA Hirofumi

Current text_console_update() writes totally broken color attributes to console_write_ch(). The format now is writing,

[WRONG]

	bold << 21 | fg << 12 | bg << 8 | char
	fg == 3bits curses color number
	bg == 3bits curses color number

I can't see this format is where come from. Anyway, this doesn't work at all.

What curses expects is actually (and vga.c is using),

[RIGHT]

	bold << 21 | bg << 11 | fg << 8 | char
	fg == 3bits vga color number
	bg == 3bits vga color number

And curses set COLOR_PAIR() up to match this format, and curses's chtype. I.e,

	bold | color_pair | char
	color_pair == (bg << 3 | fg)
Quoted 7 lines

To fix, this simply uses VGA color number everywhere except curses.c internal. Then, convert it to above [RIGHT] format to write by console_write_ch(). And as bonus, this reduces to expose curses define to other parts (removes COLOR_* from console.c).

[Tested the first line is displayed as white on blue back for monitor in curses console]

Added to ui patch queue.

thanks,
Gerd