[Qemu-devel] [RFC 0/3] improve/fix -curses mode

[Qemu-devel] [RFC 0/3] improve/fix -curses mode

[OGAWA Hirofumi]

Hi,

This patchset is to improve/fix -curses mode. But this patchset is
tested only on linux, so RFC. (BTW, tested with xterm/gnome-terminal on
linux)

    ui/curses: Fix monitor color with -curses when 256 colors
    ui/curses: Support line graphics chars on -curses mode
    ui/curses: Fix pageup/pagedown on -curses

 include/ui/console.h |   11 ++++++++++-
 ui/curses.c          |   45 +++++++++++++++++++++++++++++++++++++++++++++
 ui/curses_keys.h     |    8 ++++++--
 3 files changed, 61 insertions(+), 3 deletions(-)

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

[Qemu-devel] [PATCH 1/3] ui/curses: Fix monitor color with -curses when 256 colors

[OGAWA Hirofumi]

If TERM=xterm-256color, COLOR_PAIRS==256 and monitor passes chtype
like 0x74xx. Then, the code uses uninitialized color pair. As result,
monitor uses black for both of fg and bg color, i.e. terminal is
filled by black.

To fix, this initialize above than 64 with default color (fg=white,bg=black).

FIXME: on 256 color, curses may be possible better vga color emulation.

Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---

 ui/curses.c |    3 +++
 1 file changed, 3 insertions(+)

diff -puN ui/curses.c~support-curses-256color ui/curses.c
--- qemu/ui/curses.c~support-curses-256color	2015-10-14 20:12:06.311051365 +0900
+++ qemu-hirofumi/ui/curses.c	2015-10-14 20:27:39.417674271 +0900
@@ -343,6 +343,9 @@ static void curses_setup(void)
 
     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) */
+    for (i = 64; i < COLOR_PAIRS; i ++)
+        init_pair(i, COLOR_WHITE, COLOR_BLACK);
 }
 
 static void curses_keyboard_setup(void)
_

[Qemu-devel] [PATCH 2/3] ui/curses: Support line graphics chars on -curses mode

[OGAWA Hirofumi]

This converts vga code to curses code in console_write_bh().

With this changes, we can see line graphics (for example, dialog uses)
correctly.

Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---

 include/ui/console.h |   11 ++++++++++-
 ui/curses.c          |   42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 1 deletion(-)

diff -puN include/ui/console.h~support-curses-border include/ui/console.h
--- qemu/include/ui/console.h~support-curses-border	2015-10-14 20:25:56.681826861 +0900
+++ qemu-hirofumi/include/ui/console.h	2015-10-14 20:25:56.685826856 +0900
@@ -284,13 +284,22 @@ static inline pixman_format_code_t surfa
 #ifdef CONFIG_CURSES
 #include <curses.h>
 typedef chtype console_ch_t;
+extern chtype vga_to_curses[];
 #else
 typedef unsigned long console_ch_t;
 #endif
 static inline void console_write_ch(console_ch_t *dest, uint32_t ch)
 {
-    if (!(ch & 0xff))
+    uint8_t c = ch;
+#ifdef CONFIG_CURSES
+    if (vga_to_curses[c]) {
+        ch &= ~(console_ch_t)0xff;
+        ch |= vga_to_curses[c];
+    }
+#else
+    if (c == '\0')
         ch |= ' ';
+#endif
     *dest = ch;
 }
 
diff -puN ui/curses.c~support-curses-border ui/curses.c
--- qemu/ui/curses.c~support-curses-border	2015-10-14 20:25:56.681826861 +0900
+++ qemu-hirofumi/ui/curses.c	2015-10-14 20:25:56.685826856 +0900
@@ -42,6 +42,8 @@ static WINDOW *screenpad = NULL;
 static int width, height, gwidth, gheight, invalidate;
 static int px, py, sminx, sminy, smaxx, smaxy;
 
+chtype vga_to_curses[256];
+
 static void curses_update(DisplayChangeListener *dcl,
                           int x, int y, int w, int h)
 {
@@ -346,6 +348,46 @@ static void curses_setup(void)
     /* Set default color for more than 64. (monitor uses 0x74xx for example) */
     for (i = 64; i < COLOR_PAIRS; i ++)
         init_pair(i, COLOR_WHITE, COLOR_BLACK);
+
+    /* Setup mapping for vga to curses line graphics */
+    memset(vga_to_curses, 0, sizeof(vga_to_curses));
+    /* FIXME: for better font, have to use ncursesw and setlocale() */
+#if 0
+    /* FIXME: map from where? */
+    ACS_S1;
+    ACS_S3;
+    ACS_S7;
+    ACS_S9;
+#endif
+    vga_to_curses['\0'] = ' ';
+    vga_to_curses[0x04] = ACS_DIAMOND;
+    vga_to_curses[0x0a] = ACS_RARROW;
+    vga_to_curses[0x0b] = ACS_LARROW;
+    vga_to_curses[0x18] = ACS_UARROW;
+    vga_to_curses[0x19] = ACS_DARROW;
+    vga_to_curses[0x9c] = ACS_STERLING;
+    vga_to_curses[0xb0] = ACS_BOARD;
+    vga_to_curses[0xb1] = ACS_CKBOARD;
+    vga_to_curses[0xb3] = ACS_VLINE;
+    vga_to_curses[0xb4] = ACS_RTEE;
+    vga_to_curses[0xbf] = ACS_URCORNER;
+    vga_to_curses[0xc0] = ACS_LLCORNER;
+    vga_to_curses[0xc1] = ACS_BTEE;
+    vga_to_curses[0xc2] = ACS_TTEE;
+    vga_to_curses[0xc3] = ACS_LTEE;
+    vga_to_curses[0xc4] = ACS_HLINE;
+    vga_to_curses[0xc5] = ACS_PLUS;
+    vga_to_curses[0xce] = ACS_LANTERN;
+    vga_to_curses[0xd8] = ACS_NEQUAL;
+    vga_to_curses[0xd9] = ACS_LRCORNER;
+    vga_to_curses[0xda] = ACS_ULCORNER;
+    vga_to_curses[0xdb] = ACS_BLOCK;
+    vga_to_curses[0xe3] = ACS_PI;
+    vga_to_curses[0xf1] = ACS_PLMINUS;
+    vga_to_curses[0xf2] = ACS_GEQUAL;
+    vga_to_curses[0xf3] = ACS_LEQUAL;
+    vga_to_curses[0xf8] = ACS_DEGREE;
+    vga_to_curses[0xfe] = ACS_BULLET;
 }
 
 static void curses_keyboard_setup(void)
_

[Qemu-devel] [PATCH 3/3] ui/curses: Fix pageup/pagedown on -curses

[OGAWA Hirofumi]

Current KEY_NPAGE/KEY_PPAGE handling is broken on -curses. Those uses
"GREY", but "KEY_MASK" masked out "GREY".

To fix, we have to use correct mask value - SCANCODE_KEYMASK.

Then, this adds support of "shift + pageup/pagedown". With this,
-curses mode can use scroll-up/down as usual like other display modes.

Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---

 ui/curses_keys.h |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff -puN ui/curses_keys.h~support-curses-pageup ui/curses_keys.h
--- qemu/ui/curses_keys.h~support-curses-pageup	2015-10-14 20:25:59.781822261 +0900
+++ qemu-hirofumi/ui/curses_keys.h	2015-10-14 20:25:59.785822255 +0900
@@ -29,8 +29,7 @@
 #include "keymaps.h"
 
 
-#define KEY_RELEASE         0x80
-#define KEY_MASK            0x7f
+#define KEY_MASK            SCANCODE_KEYMASK
 #define GREY_CODE           0xe0
 #define GREY                SCANCODE_GREY
 #define SHIFT_CODE          0x2a
@@ -60,6 +59,8 @@ static const int curses2keysym[CURSES_KE
     ['\n'] = KEY_ENTER,
     [27] = 27,
     [KEY_BTAB] = '\t' | KEYSYM_SHIFT,
+    [KEY_SPREVIOUS] = KEY_PPAGE | KEYSYM_SHIFT,
+    [KEY_SNEXT] = KEY_NPAGE | KEYSYM_SHIFT,
 };
 
 static const int curses2keycode[CURSES_KEYS] = {
@@ -149,6 +150,9 @@ static const int curses2keycode[CURSES_K
     [KEY_IC] = 82 | GREY, /* Insert */
     [KEY_DC] = 83 | GREY, /* Delete */
 
+    [KEY_SPREVIOUS] = 73 | GREY | SHIFT, /* Shift + Page Up */
+    [KEY_SNEXT] = 81 | GREY | SHIFT, /* Shift + Page Down */
+
     ['!'] = 2 | SHIFT,
     ['@'] = 3 | SHIFT,
     ['#'] = 4 | SHIFT,
_

Re: [Qemu-devel] [PATCH 2/3] ui/curses: Support line graphics chars on -curses mode

[Eric Blake]

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

[meta-comment]

On 10/14/2015 05:37 AM, OGAWA Hirofumi wrote:
> This converts vga code to curses code in console_write_bh().
> 

You sent your message with deep threading, with each message a reply to
the previous, resulting in:

0/3
+ 1/3
  + 2/3
    + 3/3

and when someone replies, a mail client that sorts by references before
age sees:

0/3
+ 1/3
| + 2/3
| | + 3/3
| | | + Re: 3/3
| | + Re: 2/3
| + Re: 1/3
+ Re: 0/3

which is rather confusing.  Better is to use shallow threading, with
each message in reply ONLY to the cover letter:

0/3
+ 1/3
+ 2/3
+ 3/3

and where replies are easier to follow in mail clients:

0/3
+ 1/3
| + Re: 1/3
+ 2/3
| + Re: 2/3
+ 3/3
| + Re: 3/3
+ Re: 0/3

You may want to double-check your git settings, as there are some knobs
to tweak for shallow vs. default threading (I think shallow is default
in newer git, but it was not always the case in really old git - but I
don't remember the knob names off-hand).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

Re: [Qemu-devel] [PATCH 2/3] ui/curses: Support line graphics chars on -curses mode

[Gerd Hoffmann]

On Mi, 2015-10-14 at 20:37 +0900, OGAWA Hirofumi wrote:
> This converts vga code to curses code in console_write_bh().
> 
> With this changes, we can see line graphics (for example, dialog uses)
> correctly.

But it breaks line graphics in vga text mode on gtk I suspect?

cheers,
  Gerd

Re: [Qemu-devel] [RFC 0/3] improve/fix -curses mode

[Markus Armbruster]

Copying maintainer.  You can use scripts/get_maintainer.pl to find
maintainers yourself.

OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> writes:

> Hi,
>
> This patchset is to improve/fix -curses mode. But this patchset is
> tested only on linux, so RFC. (BTW, tested with xterm/gnome-terminal on
> linux)
>
>     ui/curses: Fix monitor color with -curses when 256 colors
>     ui/curses: Support line graphics chars on -curses mode
>     ui/curses: Fix pageup/pagedown on -curses
>
>  include/ui/console.h |   11 ++++++++++-
>  ui/curses.c          |   45 +++++++++++++++++++++++++++++++++++++++++++++
>  ui/curses_keys.h     |    8 ++++++--
>  3 files changed, 61 insertions(+), 3 deletions(-)
>
> Thanks.

Re: [Qemu-devel] [PATCH 2/3] ui/curses: Support line graphics chars on -curses mode

[OGAWA Hirofumi]

Gerd Hoffmann <kraxel@redhat.com> writes:

> On Mi, 2015-10-14 at 20:37 +0900, OGAWA Hirofumi wrote:
>> This converts vga code to curses code in console_write_bh().
>> 
>> With this changes, we can see line graphics (for example, dialog uses)
>> correctly.
>
> But it breaks line graphics in vga text mode on gtk I suspect?

vga_to_curses[] is filled by curses_setup(). So if no -curses option, I
think it should have no change to behavior. (BTW, there is no common
section like kernel in qemu build, right?  i.e. no need initializer to
global value.)

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [Qemu-devel] [RFC 0/3] improve/fix -curses mode

[OGAWA Hirofumi]

Markus Armbruster <armbru@redhat.com> writes:

> Copying maintainer.  You can use scripts/get_maintainer.pl to find
> maintainers yourself.

Ah, I was missing to use it. Thanks.

> OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> writes:
>
>> Hi,
>>
>> This patchset is to improve/fix -curses mode. But this patchset is
>> tested only on linux, so RFC. (BTW, tested with xterm/gnome-terminal on
>> linux)
>>
>>     ui/curses: Fix monitor color with -curses when 256 colors
>>     ui/curses: Support line graphics chars on -curses mode
>>     ui/curses: Fix pageup/pagedown on -curses
>>
>>  include/ui/console.h |   11 ++++++++++-
>>  ui/curses.c          |   45 +++++++++++++++++++++++++++++++++++++++++++++
>>  ui/curses_keys.h     |    8 ++++++--
>>  3 files changed, 61 insertions(+), 3 deletions(-)
>>
>> Thanks.

-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [Qemu-devel] [PATCH 2/3] ui/curses: Support line graphics chars on -curses mode

[OGAWA Hirofumi]

OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> writes:

> Gerd Hoffmann <kraxel@redhat.com> writes:
>
>> On Mi, 2015-10-14 at 20:37 +0900, OGAWA Hirofumi wrote:
>>> This converts vga code to curses code in console_write_bh().
>>> 
>>> With this changes, we can see line graphics (for example, dialog uses)
>>> correctly.
>>
>> But it breaks line graphics in vga text mode on gtk I suspect?
>
> vga_to_curses[] is filled by curses_setup(). So if no -curses option, I
> think it should have no change to behavior. (BTW, there is no common
> section like kernel in qemu build, right?  i.e. no need initializer to
> global value.)

The patch is following reply removes unnecessary

	memset(vga_to_curses, 0, sizeof(vga_to_curses));

in curses_setup(). And tested line graphics works on both of gtk and curses.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

[Qemu-devel] [PATCH v2 1/3] ui/curses: Support line graphics chars on -curses mode

[OGAWA Hirofumi]

This converts vga code to curses code in console_write_bh().

With this changes, we can see line graphics (for example, dialog uses)
correctly.

Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---

 include/ui/console.h |   11 ++++++++++-
 ui/curses.c          |   43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletion(-)

diff -puN include/ui/console.h~support-curses-border include/ui/console.h
--- qemu/include/ui/console.h~support-curses-border	2015-10-14 20:27:42.653669461 +0900
+++ qemu-hirofumi/include/ui/console.h	2015-10-14 20:27:42.657669455 +0900
@@ -284,13 +284,22 @@ static inline pixman_format_code_t surfa
 #ifdef CONFIG_CURSES
 #include <curses.h>
 typedef chtype console_ch_t;
+extern chtype vga_to_curses[];
 #else
 typedef unsigned long console_ch_t;
 #endif
 static inline void console_write_ch(console_ch_t *dest, uint32_t ch)
 {
-    if (!(ch & 0xff))
+    uint8_t c = ch;
+#ifdef CONFIG_CURSES
+    if (vga_to_curses[c]) {
+        ch &= ~(console_ch_t)0xff;
+        ch |= vga_to_curses[c];
+    }
+#else
+    if (c == '\0')
         ch |= ' ';
+#endif
     *dest = ch;
 }
 
diff -puN ui/curses.c~support-curses-border ui/curses.c
--- qemu/ui/curses.c~support-curses-border	2015-10-14 20:27:42.653669461 +0900
+++ qemu-hirofumi/ui/curses.c	2015-10-15 09:00:09.639887509 +0900
@@ -42,6 +42,8 @@ static WINDOW *screenpad = NULL;
 static int width, height, gwidth, gheight, invalidate;
 static int px, py, sminx, sminy, smaxx, smaxy;
 
+chtype vga_to_curses[256];
+
 static void curses_update(DisplayChangeListener *dcl,
                           int x, int y, int w, int h)
 {
@@ -346,6 +348,47 @@ static void curses_setup(void)
     /* Set default color for more than 64. (monitor uses 0x74xx for example) */
     for (i = 64; i < COLOR_PAIRS; i ++)
         init_pair(i, COLOR_WHITE, COLOR_BLACK);
+
+    /*
+     * Setup mapping for vga to curses line graphics.
+     * FIXME: for better font, have to use ncursesw and setlocale()
+     */
+#if 0
+    /* FIXME: map from where? */
+    ACS_S1;
+    ACS_S3;
+    ACS_S7;
+    ACS_S9;
+#endif
+    vga_to_curses['\0'] = ' ';
+    vga_to_curses[0x04] = ACS_DIAMOND;
+    vga_to_curses[0x0a] = ACS_RARROW;
+    vga_to_curses[0x0b] = ACS_LARROW;
+    vga_to_curses[0x18] = ACS_UARROW;
+    vga_to_curses[0x19] = ACS_DARROW;
+    vga_to_curses[0x9c] = ACS_STERLING;
+    vga_to_curses[0xb0] = ACS_BOARD;
+    vga_to_curses[0xb1] = ACS_CKBOARD;
+    vga_to_curses[0xb3] = ACS_VLINE;
+    vga_to_curses[0xb4] = ACS_RTEE;
+    vga_to_curses[0xbf] = ACS_URCORNER;
+    vga_to_curses[0xc0] = ACS_LLCORNER;
+    vga_to_curses[0xc1] = ACS_BTEE;
+    vga_to_curses[0xc2] = ACS_TTEE;
+    vga_to_curses[0xc3] = ACS_LTEE;
+    vga_to_curses[0xc4] = ACS_HLINE;
+    vga_to_curses[0xc5] = ACS_PLUS;
+    vga_to_curses[0xce] = ACS_LANTERN;
+    vga_to_curses[0xd8] = ACS_NEQUAL;
+    vga_to_curses[0xd9] = ACS_LRCORNER;
+    vga_to_curses[0xda] = ACS_ULCORNER;
+    vga_to_curses[0xdb] = ACS_BLOCK;
+    vga_to_curses[0xe3] = ACS_PI;
+    vga_to_curses[0xf1] = ACS_PLMINUS;
+    vga_to_curses[0xf2] = ACS_GEQUAL;
+    vga_to_curses[0xf3] = ACS_LEQUAL;
+    vga_to_curses[0xf8] = ACS_DEGREE;
+    vga_to_curses[0xfe] = ACS_BULLET;
 }
 
 static void curses_keyboard_setup(void)
_

Re: [Qemu-devel] [PATCH 2/3] ui/curses: Support line graphics chars on -curses mode

[Gerd Hoffmann]

On Do, 2015-10-15 at 08:58 +0900, OGAWA Hirofumi wrote:
> Gerd Hoffmann <kraxel@redhat.com> writes:
> 
> > On Mi, 2015-10-14 at 20:37 +0900, OGAWA Hirofumi wrote:
> >> This converts vga code to curses code in console_write_bh().
> >> 
> >> With this changes, we can see line graphics (for example, dialog uses)
> >> correctly.
> >
> > But it breaks line graphics in vga text mode on gtk I suspect?
> 
> vga_to_curses[] is filled by curses_setup(). So if no -curses option, I
> think it should have no change to behavior. (BTW, there is no common
> section like kernel in qemu build, right?  i.e. no need initializer to
> global value.)

Ok, curses and gtk can't be active at the same time.  But for curses and
vnc it is possible (qemu -curses -vnc :$display), and you probably get a
dissorted vga text mode over vnc then.

I think the mapping should happen in the curses code, when curses
updates the screen.  curses_update() function is probably the best place
for it.

Also you don't need a init function, you can simply init the array with
c99 initializers:

chtype vga_to_curses[256] = {
    [0x04] = ACS_DIAMOND,
    [ ... ]
};

Re: [Qemu-devel] [PATCH 2/3] ui/curses: Support line graphics chars on -curses mode

[OGAWA Hirofumi]

Gerd Hoffmann <kraxel@redhat.com> writes:

>> vga_to_curses[] is filled by curses_setup(). So if no -curses option, I
>> think it should have no change to behavior. (BTW, there is no common
>> section like kernel in qemu build, right?  i.e. no need initializer to
>> global value.)
>
> Ok, curses and gtk can't be active at the same time.  But for curses and
> vnc it is possible (qemu -curses -vnc :$display), and you probably get a
> dissorted vga text mode over vnc then.
>
> I think the mapping should happen in the curses code, when curses
> updates the screen.  curses_update() function is probably the best place
> for it.

OK. It would be possible though, but parsing at curses_update() is
slower. It is why, I did at that. Well, I'll see.

> Also you don't need a init function, you can simply init the array with
> c99 initializers:
>
> chtype vga_to_curses[256] = {
>     [0x04] = ACS_DIAMOND,
>     [ ... ]
> };

This is not possible, because ACS_* is not constant (initialized by
reading from termcap or such).

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [Qemu-devel] [PATCH 2/3] ui/curses: Support line graphics chars on -curses mode

[OGAWA Hirofumi]

OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> writes:

>> Ok, curses and gtk can't be active at the same time.  But for curses and
>> vnc it is possible (qemu -curses -vnc :$display), and you probably get a
>> dissorted vga text mode over vnc then.
>>
>> I think the mapping should happen in the curses code, when curses
>> updates the screen.  curses_update() function is probably the best place
>> for it.
>
> OK. It would be possible though, but parsing at curses_update() is
> slower. It is why, I did at that. Well, I'll see.

I've checked callers and usage of console_write_ch(). Call path is
following paths.

curses_refresh()
    graphic_hw_text_update(NULL, screen);
        ->text_update()
            vga_update_text()
                console_write_ch()

        ->text_update()
            jazz_led_text_update()
                console_write_ch()

        ->text_update()
            text_console_update()
                console_write_ch()

>From this call paths, graphic_hw_text_update() => console_write_ch() =>
->text_update() path is only used for -curses mode. What do you think?
(E.g. qemu is going to extend to use this for other purpose?)

I guess, rather qemu wants to remove the call of ->text_update() for
micro optimization if curses display is not used. (I'm not sure how
optimizes it right now though.)

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [Qemu-devel] [PATCH 2/3] ui/curses: Support line graphics chars on -curses mode

[Gerd Hoffmann]

  Hi,

> From this call paths, graphic_hw_text_update() => console_write_ch() =>
> ->text_update() path is only used for -curses mode. What do you think?

Oh, right, text mode curses update and text mode framebuffer rendering
take completely separate code paths.  Completely forgot that, I rarely
look into the curses code.  Sorry for the extra trouble.

> (E.g. qemu is going to extend to use this for other purpose?)

Unlikely.  In theory other ui frontends could support vga text mode that
way.  But text mode is a dying relic, people use either graphics or a
serial console these days, so I don't expect that to happen.

So, I still think it would be cleaner to implement the curses special
char mapping in ui/curses.c.  I can't imagine there is a noticable
performance difference.  But doing it like you did shouldn't break
things either.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 2/3] ui/curses: Support line graphics chars on -curses mode

[Gerd Hoffmann]

> > Also you don't need a init function, you can simply init the array with
> > c99 initializers:
> >
> > chtype vga_to_curses[256] = {
> >     [0x04] = ACS_DIAMOND,
> >     [ ... ]
> > };
> 
> This is not possible, because ACS_* is not constant (initialized by
> reading from termcap or such).

Ah, ok.  That isn't obvious from reading the code.  Can you add a
comment please?

thanks,
  Gerd

Re: [Qemu-devel] [PATCH 2/3] ui/curses: Support line graphics chars on -curses mode

[OGAWA Hirofumi]

Gerd Hoffmann <kraxel@redhat.com> writes:

>> (E.g. qemu is going to extend to use this for other purpose?)
>
> Unlikely.  In theory other ui frontends could support vga text mode that
> way.  But text mode is a dying relic, people use either graphics or a
> serial console these days, so I don't expect that to happen.
>
> So, I still think it would be cleaner to implement the curses special
> char mapping in ui/curses.c.  I can't imagine there is a noticable
> performance difference.  But doing it like you did shouldn't break
> things either.

OK. For cleaner (purpose is separation of curses code), I introduced
hw_text_update_cb() interface (looks like possible to use text data for
other than curses interface too). But you may feel this is overkill.

Well, IMO, the patch makes clear purpose of this path in following reply.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

[Qemu-devel] [PATCH v3] ui/curses: Support line graphics chars on -curses mode

[OGAWA Hirofumi]

This converts vga code to curses code in console_write_bh().

With this changes, we can see line graphics (for example, dialog uses)
correctly.

Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---

 include/ui/console.h |    9 ++------
 ui/console.c         |   13 ++++++++++++
 ui/curses.c          |   53 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+), 6 deletions(-)

diff -puN include/ui/console.h~support-curses-border include/ui/console.h
--- qemu/include/ui/console.h~support-curses-border	2015-10-15 18:18:18.702781174 +0900
+++ qemu-hirofumi/include/ui/console.h	2015-10-15 18:43:06.927991391 +0900
@@ -287,12 +287,9 @@ typedef chtype console_ch_t;
 #else
 typedef unsigned long console_ch_t;
 #endif
-static inline void console_write_ch(console_ch_t *dest, uint32_t ch)
-{
-    if (!(ch & 0xff))
-        ch |= ' ';
-    *dest = ch;
-}
+typedef void (hw_text_update_t)(console_ch_t *, uint32_t);
+void register_hw_text_update_cb(hw_text_update_t *cb);
+void console_write_ch(console_ch_t *dest, uint32_t ch);
 
 typedef struct GraphicHwOps {
     void (*invalidate)(void *opaque);
diff -puN ui/curses.c~support-curses-border ui/curses.c
--- qemu/ui/curses.c~support-curses-border	2015-10-15 18:18:18.702781174 +0900
+++ qemu-hirofumi/ui/curses.c	2015-10-15 18:44:56.511788663 +0900
@@ -41,6 +41,17 @@ static console_ch_t screen[160 * 100];
 static WINDOW *screenpad = NULL;
 static int width, height, gwidth, gheight, invalidate;
 static int px, py, sminx, sminy, smaxx, smaxy;
+static chtype vga_to_curses[256];
+
+static void curses_write_ch(console_ch_t *dest, uint32_t ch)
+{
+    uint8_t c = ch;
+    if (vga_to_curses[c]) {
+        ch &= ~(console_ch_t)0xff;
+        ch |= vga_to_curses[c];
+    }
+    *dest = ch;
+}
 
 static void curses_update(DisplayChangeListener *dcl,
                           int x, int y, int w, int h)
@@ -346,6 +357,47 @@ static void curses_setup(void)
     /* Set default color for more than 64. (monitor uses 0x74xx for example) */
     for (i = 64; i < COLOR_PAIRS; i ++)
         init_pair(i, COLOR_WHITE, COLOR_BLACK);
+
+    /*
+     * Setup mapping for vga to curses line graphics.
+     * FIXME: for better font, have to use ncursesw and setlocale()
+     */
+#if 0
+    /* FIXME: map from where? */
+    ACS_S1;
+    ACS_S3;
+    ACS_S7;
+    ACS_S9;
+#endif
+    vga_to_curses['\0'] = ' ';
+    vga_to_curses[0x04] = ACS_DIAMOND;
+    vga_to_curses[0x0a] = ACS_RARROW;
+    vga_to_curses[0x0b] = ACS_LARROW;
+    vga_to_curses[0x18] = ACS_UARROW;
+    vga_to_curses[0x19] = ACS_DARROW;
+    vga_to_curses[0x9c] = ACS_STERLING;
+    vga_to_curses[0xb0] = ACS_BOARD;
+    vga_to_curses[0xb1] = ACS_CKBOARD;
+    vga_to_curses[0xb3] = ACS_VLINE;
+    vga_to_curses[0xb4] = ACS_RTEE;
+    vga_to_curses[0xbf] = ACS_URCORNER;
+    vga_to_curses[0xc0] = ACS_LLCORNER;
+    vga_to_curses[0xc1] = ACS_BTEE;
+    vga_to_curses[0xc2] = ACS_TTEE;
+    vga_to_curses[0xc3] = ACS_LTEE;
+    vga_to_curses[0xc4] = ACS_HLINE;
+    vga_to_curses[0xc5] = ACS_PLUS;
+    vga_to_curses[0xce] = ACS_LANTERN;
+    vga_to_curses[0xd8] = ACS_NEQUAL;
+    vga_to_curses[0xd9] = ACS_LRCORNER;
+    vga_to_curses[0xda] = ACS_ULCORNER;
+    vga_to_curses[0xdb] = ACS_BLOCK;
+    vga_to_curses[0xe3] = ACS_PI;
+    vga_to_curses[0xf1] = ACS_PLMINUS;
+    vga_to_curses[0xf2] = ACS_GEQUAL;
+    vga_to_curses[0xf3] = ACS_LEQUAL;
+    vga_to_curses[0xf8] = ACS_DEGREE;
+    vga_to_curses[0xfe] = ACS_BULLET;
 }
 
 static void curses_keyboard_setup(void)
@@ -379,6 +431,7 @@ void curses_display_init(DisplayState *d
     }
 #endif
 
+    register_hw_text_update_cb(curses_write_ch);
     curses_setup();
     curses_keyboard_setup();
     atexit(curses_atexit);
diff -puN ui/console.c~support-curses-border ui/console.c
--- qemu/ui/console.c~support-curses-border	2015-10-15 18:18:32.482754986 +0900
+++ qemu-hirofumi/ui/console.c	2015-10-15 18:43:22.191963132 +0900
@@ -333,6 +333,19 @@ void qmp_screendump(const char *filename
     ppm_save(filename, surface, errp);
 }
 
+static hw_text_update_t *hw_text_update_cb;
+
+void register_hw_text_update_cb(hw_text_update_t *cb)
+{
+    hw_text_update_cb = cb;
+}
+
+void console_write_ch(console_ch_t *dest, uint32_t ch)
+{
+    if (hw_text_update_cb)
+        hw_text_update_cb(dest, ch);
+}
+
 void graphic_hw_text_update(QemuConsole *con, console_ch_t *chardata)
 {
     if (!con) {
_

Re: [Qemu-devel] [PATCH 2/3] ui/curses: Support line graphics chars on -curses mode

[OGAWA Hirofumi]

Gerd Hoffmann <kraxel@redhat.com> writes:

>> > Also you don't need a init function, you can simply init the array with
>> > c99 initializers:
>> >
>> > chtype vga_to_curses[256] = {
>> >     [0x04] = ACS_DIAMOND,
>> >     [ ... ]
>> > };
>> 
>> This is not possible, because ACS_* is not constant (initialized by
>> reading from termcap or such).
>
> Ah, ok.  That isn't obvious from reading the code.  Can you add a
> comment please?

Oh, sent updated patch already. I will resend a updated patch to this
reply.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

[Qemu-devel] [PATCH v4] ui/curses: Support line graphics chars on -curses mode

[OGAWA Hirofumi]

This converts vga code to curses code in console_write_bh().

With this changes, we can see line graphics (for example, dialog uses)
correctly.

Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---

 include/ui/console.h |    9 ++------
 ui/console.c         |   13 ++++++++++++
 ui/curses.c          |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 70 insertions(+), 6 deletions(-)

diff -puN include/ui/console.h~support-curses-border include/ui/console.h
--- qemu/include/ui/console.h~support-curses-border	2015-10-15 18:58:35.454187841 +0900
+++ qemu-hirofumi/include/ui/console.h	2015-10-15 18:58:35.462187825 +0900
@@ -287,12 +287,9 @@ typedef chtype console_ch_t;
 #else
 typedef unsigned long console_ch_t;
 #endif
-static inline void console_write_ch(console_ch_t *dest, uint32_t ch)
-{
-    if (!(ch & 0xff))
-        ch |= ' ';
-    *dest = ch;
-}
+typedef void (hw_text_update_t)(console_ch_t *, uint32_t);
+void register_hw_text_update_cb(hw_text_update_t *cb);
+void console_write_ch(console_ch_t *dest, uint32_t ch);
 
 typedef struct GraphicHwOps {
     void (*invalidate)(void *opaque);
diff -puN ui/curses.c~support-curses-border ui/curses.c
--- qemu/ui/curses.c~support-curses-border	2015-10-15 18:58:35.458187833 +0900
+++ qemu-hirofumi/ui/curses.c	2015-10-15 19:00:06.182005139 +0900
@@ -41,6 +41,17 @@ static console_ch_t screen[160 * 100];
 static WINDOW *screenpad = NULL;
 static int width, height, gwidth, gheight, invalidate;
 static int px, py, sminx, sminy, smaxx, smaxy;
+static chtype vga_to_curses[256];
+
+static void curses_write_ch(console_ch_t *dest, uint32_t ch)
+{
+    uint8_t c = ch;
+    if (vga_to_curses[c]) {
+        ch &= ~(console_ch_t)0xff;
+        ch |= vga_to_curses[c];
+    }
+    *dest = ch;
+}
 
 static void curses_update(DisplayChangeListener *dcl,
                           int x, int y, int w, int h)
@@ -346,6 +357,48 @@ static void curses_setup(void)
     /* Set default color for more than 64. (monitor uses 0x74xx for example) */
     for (i = 64; i < COLOR_PAIRS; i ++)
         init_pair(i, COLOR_WHITE, COLOR_BLACK);
+
+    /*
+     * Setup mapping for vga to curses line graphics.
+     * FIXME: for better font, have to use ncursesw and setlocale()
+     */
+#if 0
+    /* FIXME: map from where? */
+    ACS_S1;
+    ACS_S3;
+    ACS_S7;
+    ACS_S9;
+#endif
+    /* ACS_* is not constant. So, we can't initialize statically. */
+    vga_to_curses['\0'] = ' ';
+    vga_to_curses[0x04] = ACS_DIAMOND;
+    vga_to_curses[0x0a] = ACS_RARROW;
+    vga_to_curses[0x0b] = ACS_LARROW;
+    vga_to_curses[0x18] = ACS_UARROW;
+    vga_to_curses[0x19] = ACS_DARROW;
+    vga_to_curses[0x9c] = ACS_STERLING;
+    vga_to_curses[0xb0] = ACS_BOARD;
+    vga_to_curses[0xb1] = ACS_CKBOARD;
+    vga_to_curses[0xb3] = ACS_VLINE;
+    vga_to_curses[0xb4] = ACS_RTEE;
+    vga_to_curses[0xbf] = ACS_URCORNER;
+    vga_to_curses[0xc0] = ACS_LLCORNER;
+    vga_to_curses[0xc1] = ACS_BTEE;
+    vga_to_curses[0xc2] = ACS_TTEE;
+    vga_to_curses[0xc3] = ACS_LTEE;
+    vga_to_curses[0xc4] = ACS_HLINE;
+    vga_to_curses[0xc5] = ACS_PLUS;
+    vga_to_curses[0xce] = ACS_LANTERN;
+    vga_to_curses[0xd8] = ACS_NEQUAL;
+    vga_to_curses[0xd9] = ACS_LRCORNER;
+    vga_to_curses[0xda] = ACS_ULCORNER;
+    vga_to_curses[0xdb] = ACS_BLOCK;
+    vga_to_curses[0xe3] = ACS_PI;
+    vga_to_curses[0xf1] = ACS_PLMINUS;
+    vga_to_curses[0xf2] = ACS_GEQUAL;
+    vga_to_curses[0xf3] = ACS_LEQUAL;
+    vga_to_curses[0xf8] = ACS_DEGREE;
+    vga_to_curses[0xfe] = ACS_BULLET;
 }
 
 static void curses_keyboard_setup(void)
@@ -379,6 +432,7 @@ void curses_display_init(DisplayState *d
     }
 #endif
 
+    register_hw_text_update_cb(curses_write_ch);
     curses_setup();
     curses_keyboard_setup();
     atexit(curses_atexit);
diff -puN ui/console.c~support-curses-border ui/console.c
--- qemu/ui/console.c~support-curses-border	2015-10-15 18:58:35.458187833 +0900
+++ qemu-hirofumi/ui/console.c	2015-10-15 18:58:35.466187817 +0900
@@ -333,6 +333,19 @@ void qmp_screendump(const char *filename
     ppm_save(filename, surface, errp);
 }
 
+static hw_text_update_t *hw_text_update_cb;
+
+void register_hw_text_update_cb(hw_text_update_t *cb)
+{
+    hw_text_update_cb = cb;
+}
+
+void console_write_ch(console_ch_t *dest, uint32_t ch)
+{
+    if (hw_text_update_cb)
+        hw_text_update_cb(dest, ch);
+}
+
 void graphic_hw_text_update(QemuConsole *con, console_ch_t *chardata)
 {
     if (!con) {
_

Re: [Qemu-devel] [PATCH 2/3] ui/curses: Support line graphics chars on -curses mode

[Gerd Hoffmann]

  Hi,

> OK. For cleaner (purpose is separation of curses code), I introduced
> hw_text_update_cb() interface (looks like possible to use text data for
> other than curses interface too). But you may feel this is overkill.

Waded through the code a bit.  The whole text interface isn't cleanly
abstracted.  There are curses-specific assumptions all over the place
and thats why there is a curses include in console.h (ideally there
shouldn't be any outside ui/curses.c).

So I've changed my mind and I think adding this interface doesn't help
much.  It pretends we have a clean text mode interface, which we clearly
don't have.  Tex mode support needs some major work anyway should we add
a second text mode interface at some point in the future.  Using
vga_to_curses in console.c makes at least pretty clear what is going on
here.

Lets go with v2 of this patch.

There are some codestyle issues in the patch (and the others too),
please fix them them (scripts/checkpatch.pl helps), then resend the
whole series.

thanks,
  Gerd

Re: [Qemu-devel] [PATCH 2/3] ui/curses: Support line graphics chars on -curses mode

[OGAWA Hirofumi]

Gerd Hoffmann <kraxel@redhat.com> writes:

> Lets go with v2 of this patch.
>
> There are some codestyle issues in the patch (and the others too),
> please fix them them (scripts/checkpatch.pl helps), then resend the
> whole series.

OK. I'll send with new thread, not this thread.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>