[PATCH v3] Fixes CJK wide character display

[Kristian Høgsberg]

On Thu, Aug 01, 2013 at 01:49:03PM +0800, Peng Wu wrote:
> By jumping two columns when wide character prints,
> and draw wide cursor under wide character.

Hi Peng,

I do want this feature in weston-terminal, but there's still a few
issues to address - see below.

> ---
>  clients/Makefile.am |  2 +-
>  clients/terminal.c  | 26 ++++++++++++++++++++++++--
>  2 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/clients/Makefile.am b/clients/Makefile.am
> index 09963cc..e7c0ece 100644
> --- a/clients/Makefile.am
> +++ b/clients/Makefile.am
> @@ -113,7 +113,7 @@ weston_screenshooter_SOURCES =			\
>  weston_screenshooter_LDADD = libtoytoolkit.la
>  
>  weston_terminal_SOURCES = terminal.c
> -weston_terminal_LDADD = libtoytoolkit.la -lutil
> +weston_terminal_LDADD = libtoytoolkit.la -lutil $(PANGO_LIBS)

Again, I don't want to link to pango or glib... remember, the only
reason for weston-terminal to exist is that it doesn't have any
dependencies except cairo and the wayland libraries.

>  weston_image_SOURCES = image.c
>  weston_image_LDADD = libtoytoolkit.la
> diff --git a/clients/terminal.c b/clients/terminal.c
> index 0d4f726..dc56745 100644
> --- a/clients/terminal.c
> +++ b/clients/terminal.c
> @@ -33,6 +33,10 @@
>  #include <ctype.h>
>  #include <cairo.h>
>  #include <sys/epoll.h>
> +#include <glib.h>
> +#define __USE_XOPEN
> +#include <wchar.h>
> +#include <locale.h>
>  
>  #include <wayland-client.h>
>  
> @@ -93,6 +97,11 @@ union utf8_char {
>  	uint32_t ch;
>  };
>  
> +static bool is_wide(union utf8_char utf8){
> +	gunichar unichar = g_utf8_get_char((const char*) utf8.byte);
> +	return wcwidth(unichar) > 1;
> +}

... but I just committed a patch to make the utf-8 state machine
assemble the unicode value from the utf-8, so you can now use
get_unicode() instead of g_utf8_get_char().

Please follow the coding conventions and leave 'static int' on it's
own line and put the opening '{' of the function on it's own line.

>  enum utf8_state {
>  	utf8state_start,
>  	utf8state_accept,
> @@ -942,6 +951,7 @@ redraw_handler(struct widget *widget, void *data)
>  	struct glyph_run run;
>  	cairo_font_extents_t extents;
>  	double average_width;
> +	double unichar_width;
>  
>  	surface = window_get_surface(terminal->window);
>  	widget_get_allocation(terminal->widget, &allocation);
> @@ -967,6 +977,7 @@ redraw_handler(struct widget *widget, void *data)
>  			allocation.y + top_margin);
>  	/* paint the background */
>  	for (row = 0; row < terminal->height; row++) {
> +		p_row = terminal_get_row(terminal, row);
>  		for (col = 0; col < terminal->width; col++) {
>  			/* get the attributes for this character cell */
>  			terminal_decode_attr(terminal, row, col, &attr);
> @@ -974,12 +985,17 @@ redraw_handler(struct widget *widget, void *data)
>  			if (attr.attr.bg == terminal->color_scheme->border)
>  				continue;
>  
> +			if(is_wide(p_row[col]))
> +				unichar_width = 2 * average_width;
> +			else
> +				unichar_width = average_width;
> +
>  			terminal_set_color(terminal, cr, attr.attr.bg);
>  			cairo_move_to(cr, col * average_width,
>  				      row * extents.height);
> -			cairo_rel_line_to(cr, average_width, 0);
> +			cairo_rel_line_to(cr, unichar_width, 0);
>  			cairo_rel_line_to(cr, 0, extents.height);
> -			cairo_rel_line_to(cr, -average_width, 0);
> +			cairo_rel_line_to(cr, -unichar_width, 0);
>  			cairo_close_path(cr);
>  			cairo_fill(cr);
>  		}
> @@ -1871,6 +1887,10 @@ handle_char(struct terminal *terminal, union utf8_char utf8)
>  	row[terminal->column] = utf8;
>  	attr_row[terminal->column++] = terminal->curr_attr;
>  
> +	/* cursor jump for wide character. */
> +	if (is_wide(utf8))
> +		row[terminal->column++].ch = 0;

Inserting this 0 here confuses the selection logic which thinks 0
means "end of line".  I think we can use an invalid utf-8 character
instead to indicate that this cell is a wide-char filler.  We then
also need to handle that in terminal_send_selection(), where we must
ignore those filler cells when writing the selection to the fd.


>  	if (utf8.ch != terminal->last_char.ch)
>  		terminal->last_char = utf8;
>  }
> @@ -2711,6 +2731,8 @@ int main(int argc, char *argv[])
>  	struct terminal *terminal;
>  	int config_fd;
>  
> +	setlocale(LC_ALL, "");

What is this for?

Otherwise the patch looks good.

Kristian

>  	option_shell = getenv("SHELL");
>  	if (!option_shell)
>  		option_shell = "/bin/bash";
> -- 
> 1.8.3.1
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel