Re: [Qemu-devel] [PATCH 2/2] curses: add option to specify VGA font encoding

2019-03-04 Thread Eric Blake

On 3/4/19 10:56 AM, Eddie Kohler wrote:

Another difference is that Samuel's patch supports platforms on which
wchar_t is not Unicode.

I don't know if there are many/any such platforms but it does support them!


I know Cygwin is such a platform, and suspect that mingw is too 
(although I didn't actually check) - in general, Windows picked wchar_t 
to be 2 bytes, which is incompatible with full Unicode support requiring 
4-byte wchar_t, and hence explaining why C11 introduced char16_t and 
char32_t to work around the legacy mess that wchar_t causes.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



Re: [Qemu-devel] [PATCH 2/2] curses: add option to specify VGA font encoding

2019-03-04 Thread Eddie Kohler
Another difference is that Samuel's patch supports platforms on which
wchar_t is not Unicode.

I don't know if there are many/any such platforms but it does support them!


On Mon, Mar 4, 2019 at 3:14 AM Samuel Thibault 
wrote:

> Gerd Hoffmann, le lun. 04 mars 2019 09:03:38 +0100, a ecrit:
> > On Sun, Mar 03, 2019 at 11:44:30AM +0100, Samuel Thibault wrote:
> > > This uses iconv to convert glyphs from the specified VGA font encoding
> to
> > > unicode, and makes use of cchar_t instead of chtype when using
> ncursesw,
> > > which allows to store all wide char as well as the WACS values.
> > >
> > > Signed-off-by: Samuel Thibault 
> > > Cc: Eddie Kohler 
> >
> > So the difference to the patch from Eddie is that charset
> > is configurable instead of being hard-coded to CP437?
>
> Yes.
>
> Samuel
>


Re: [Qemu-devel] [PATCH 2/2] curses: add option to specify VGA font encoding

2019-03-04 Thread Markus Armbruster
Samuel Thibault  writes:

> Markus Armbruster, le lun. 04 mars 2019 07:44:34 +0100, a ecrit:
>> Samuel Thibault  writes:
>> > --- a/qapi/ui.json
>> > +++ b/qapi/ui.json
>> > @@ -1131,6 +1131,7 @@
>> >  # @full-screen:   Start user interface in fullscreen mode (default: off).
>> >  # @window-close:  Allow to quit qemu with window close button (default: 
>> > on).
>> >  # @gl:Enable OpenGL support (default: off).
>> > +# @charset:   Font charset used by guest (default: CP437).
>> 
>> Can you give brief rationale for defaulting to CP437?
>
> I have added to the commit message:
>
> “
> The default charset is made CP437 since that is the charset of the
> hardware default VGA font.
> ”
>
>> > @@ -492,6 +709,10 @@ static void curses_display_init(DisplayState *ds, 
>> > DisplayOptions *opts)
>> >  }
>> >  #endif
>> >  
>> > +setlocale(LC_CTYPE, "");
>> 
>> General principles: any change to locale deserves prominent mention in
>> the commit message.
>
> I have added to the commit message:
>
> “
> This also makes the curses backend set the LC_CTYPE locale to "" to
> allow curses to emit wide characters.
> ”

Your commit message amendments work for me.  Thanks!



Re: [Qemu-devel] [PATCH 2/2] curses: add option to specify VGA font encoding

2019-03-04 Thread Gerd Hoffmann
On Sun, Mar 03, 2019 at 11:44:30AM +0100, Samuel Thibault wrote:
> This uses iconv to convert glyphs from the specified VGA font encoding to
> unicode, and makes use of cchar_t instead of chtype when using ncursesw,
> which allows to store all wide char as well as the WACS values.
> 
> Signed-off-by: Samuel Thibault 
> Cc: Eddie Kohler 

So the difference to the patch from Eddie is that charset
is configurable instead of being hard-coded to CP437?

> +# @charset:   Font charset used by guest (default: CP437).

> @@ -1139,7 +1140,8 @@
>'base': { 'type'   : 'DisplayType',
>  '*full-screen'   : 'bool',
>  '*window-close'  : 'bool',
> -'*gl': 'DisplayGLMode' },
> +'*gl': 'DisplayGLMode',
> +'*charset'   : 'str' },

No.  Please add a new DisplayCurses struct for the charset option.  It
will only be used by the curses UI and it is highly unlikely that this
will ever change as only curses needs this (the graphical UIs will just
use whatever font the guest loaded into the vga).

cheers,
  Gerd




Re: [Qemu-devel] [PATCH 2/2] curses: add option to specify VGA font encoding

2019-03-04 Thread Samuel Thibault
Gerd Hoffmann, le lun. 04 mars 2019 09:03:38 +0100, a ecrit:
> On Sun, Mar 03, 2019 at 11:44:30AM +0100, Samuel Thibault wrote:
> > This uses iconv to convert glyphs from the specified VGA font encoding to
> > unicode, and makes use of cchar_t instead of chtype when using ncursesw,
> > which allows to store all wide char as well as the WACS values.
> > 
> > Signed-off-by: Samuel Thibault 
> > Cc: Eddie Kohler 
> 
> So the difference to the patch from Eddie is that charset
> is configurable instead of being hard-coded to CP437?

Yes.

Samuel



Re: [Qemu-devel] [PATCH 2/2] curses: add option to specify VGA font encoding

2019-03-03 Thread Samuel Thibault
Markus Armbruster, le lun. 04 mars 2019 07:44:34 +0100, a ecrit:
> Samuel Thibault  writes:
> > --- a/qapi/ui.json
> > +++ b/qapi/ui.json
> > @@ -1131,6 +1131,7 @@
> >  # @full-screen:   Start user interface in fullscreen mode (default: off).
> >  # @window-close:  Allow to quit qemu with window close button (default: 
> > on).
> >  # @gl:Enable OpenGL support (default: off).
> > +# @charset:   Font charset used by guest (default: CP437).
> 
> Can you give brief rationale for defaulting to CP437?

I have added to the commit message:

“
The default charset is made CP437 since that is the charset of the
hardware default VGA font.
”

> > @@ -492,6 +709,10 @@ static void curses_display_init(DisplayState *ds, 
> > DisplayOptions *opts)
> >  }
> >  #endif
> >  
> > +setlocale(LC_CTYPE, "");
> 
> General principles: any change to locale deserves prominent mention in
> the commit message.

I have added to the commit message:

“
This also makes the curses backend set the LC_CTYPE locale to "" to
allow curses to emit wide characters.
”

Samuel



Re: [Qemu-devel] [PATCH 2/2] curses: add option to specify VGA font encoding

2019-03-03 Thread Markus Armbruster
Samuel Thibault  writes:

> This uses iconv to convert glyphs from the specified VGA font encoding to
> unicode, and makes use of cchar_t instead of chtype when using ncursesw,
> which allows to store all wide char as well as the WACS values.
>
> Signed-off-by: Samuel Thibault 
> Cc: Eddie Kohler 
> ---
>  configure   |   5 +-
>  qapi/ui.json|   4 +-
>  qemu-options.hx |   5 +-
>  ui/curses.c | 315 
>  4 files changed, 279 insertions(+), 50 deletions(-)
>
> diff --git a/configure b/configure
> index 9979ca708d..1270dc8dc0 100755
> --- a/configure
> +++ b/configure
> @@ -3449,14 +3449,17 @@ if test "$curses" != "no" ; then
>  #include 
>  #include 
>  #include 
> +#include 
>  int main(void) {
> +  const char *codeset;
>wchar_t wch = L'w';
>setlocale(LC_ALL, "");
>resize_term(0, 0);
>addwstr(L"wide chars\n");
>addnwstr(&wch, 1);
>add_wch(WACS_DEGREE);
> -  return 0;
> +  codeset = nl_langinfo(CODESET);
> +  return codeset != 0;
>  }
>  EOF
>IFS=:
> diff --git a/qapi/ui.json b/qapi/ui.json
> index c5d1d7f099..12d3a2c751 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1131,6 +1131,7 @@
>  # @full-screen:   Start user interface in fullscreen mode (default: off).
>  # @window-close:  Allow to quit qemu with window close button (default: on).
>  # @gl:Enable OpenGL support (default: off).
> +# @charset:   Font charset used by guest (default: CP437).

Can you give brief rationale for defaulting to CP437?

>  #
>  # Since: 2.12
>  #
> @@ -1139,7 +1140,8 @@
>'base': { 'type'   : 'DisplayType',
>  '*full-screen'   : 'bool',
>  '*window-close'  : 'bool',
> -'*gl': 'DisplayGLMode' },
> +'*gl': 'DisplayGLMode',
> +'*charset'   : 'str' },
>'discriminator' : 'type',
>'data': { 'gtk': 'DisplayGTK',
>  'egl-headless'   : 'DisplayEGLHeadless'} }
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 1cf9aac1fe..4bc4e736bb 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1216,7 +1216,7 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
>  "[,window_close=on|off][,gl=on|core|es|off]\n"
>  "-display gtk[,grab_on_hover=on|off][,gl=on|off]|\n"
>  "-display vnc=[,]\n"
> -"-display curses\n"
> +"-display curses[,charset=]\n"
>  "-display none\n"
>  "-display egl-headless[,rendernode=]"
>  "select display type\n"
> @@ -1248,6 +1248,9 @@ support a text mode, QEMU can display this output using 
> a
>  curses/ncurses interface. Nothing is displayed when the graphics
>  device is in graphical mode or if the graphics device does not support
>  a text mode. Generally only the VGA device models support text mode.
> +The font charset used by the guest can be specified with the
> +@code{charset} option, for example @code{charset=CP850} for IBM CP850
> +encoding. The default is @code{CP437}.
>  @item none
>  Do not display video output. The guest will still see an emulated
>  graphics card, but its output will not be displayed to the QEMU
> diff --git a/ui/curses.c b/ui/curses.c
> index 1724dd57d4..203cc075d0 100644
> --- a/ui/curses.c
> +++ b/ui/curses.c
[...]
> @@ -492,6 +709,10 @@ static void curses_display_init(DisplayState *ds, 
> DisplayOptions *opts)
>  }
>  #endif
>  
> +setlocale(LC_CTYPE, "");

General principles: any change to locale deserves prominent mention in
the commit message.

> +if (opts->charset) {
> +font_charset = opts->charset;
> +}
>  curses_setup();
>  curses_keyboard_setup();
>  atexit(curses_atexit);