[PATCH v2 1/3] Handle CJK wide glyph rendering

[David Herrmann]

Hi

On Tue, Jul 23, 2013 at 9:49 AM, Peng Wu <peng.e.wu at gmail.com> wrote:
> Hi,
>
> On Tue, 2013-07-23 at 09:22 +0200, David Herrmann wrote:
>> Please first address the issues I mentioned in v1. You cannot just
>> redraw characters and be fine. That's not how it works. You need to
>> adjust handle_char() to move the cursor two columns for
>> wide-characters.
>>
> Actually I also adjusted the cursor positioning and drawing in v2 patch
> set.
>
> [PATCH v2 2/3] Fixes cursor rendering for wide glyphs
>
> Show the cursor at the right position, and use wide cursor under wide
> glyph.

That's not what I am talking about. This is about _drawing_ the cursor
at the right position. I am talking about positioning the cursor
_internally_. That is, the x/y cursor position. Your patch just moves
the visual cursor according to your wide-characters. See the two
examples below about internal cursor positions.

>> Imagine MODE_AUTOWRAP is set and you draw 10 wide-characters on a 10
>> cell-line. We expect a vt320-compat terminal to draw 5 chars on the
>> first line and the other 5 chars on the next line.
>>
>> Assume MODE_AUTOWRAP not set and you draw 10 wide-characters on a 10
>> cell-line. We expect the first 4 chars on the line and the last char
>> to be the last character written.
>>
> I plan to skip to support MODE_AUTOWRAP feature.

Ugh? I gave two examples, one with and one without MODE_AUTOWRAP. So
it doesn't matter whether you support it or not, in both cases it will
fail.

If you don't implement it properly, you will soon notice that stuff
like vim will fail. They rely on these features.

>> Really, multi-width characters isn't about drawing. It's about
>> correctly interpreting the incoming characters in your vt100-like
>> state-machine. Why do you want to support that in terminal.c anyway?
>> It's an example, it misses support for a lot of things and I recommend
>> avoiding adding complex features.
>>
> Because we want weston terminal has i18n support, such as CJK glyph
> display.

I recommend keeping it simple. But it's not me to judge. If you refuse
to implement it properly, I have no reason to review that. You need to
convince krh to apply it.

>> Anyhow, here is an example how to move the cursor for multi-width
>> characters:
>>   https://github.com/dvdhrm/kmscon/blob/master/src/tsm_screen.c#L1000

Cheers
David