--- Begin Message ---
- To: Nadim Shaikli <shaikli at yahoo dot com>
- Subject: Re: [putty]PuTTY shapes - now what ?
- From: Simon Tatham <anakin at pobox dot com>
- Date: Tue, 24 Feb 2004 10:15:52 +0000
- Cc: putty at projects dot tartarus dot org
Nadim Shaikli <shaikli at yahoo dot com> wrote:
> Simon et al, Mr. Khalifa has added shaping and uploaded the patch,
>
> http://cvs.arabeyes.org/cvsweb/projects/external/putty/
>
> yet various issues abound possibly due to it being done in the
> improper location, etc.
OK; I do have some comments.
First (and easiest), I think I'd be happier if large pieces of new
code went into a separate source file (arabic.c? shaping.c? bidi.c?
Not sure what it should be called...) rather than being added to the
already oversized terminal.c.
I think that the Arabic shaping shouldn't be being called just
before do_text(). The redraw routine divides each line into
(potentially) small sequences of characters which have the same
attributes and can be redrawn in a single ExtTextOut call. Calling
do_arabic_shape() just before do_text() causes it to process each of
these sequences individually, whereas I'd have expected you to want
to process the line as a whole. Calling do_arabic_flip() inside the
ExtTextOut wrapper function has the same problem: it means that the
positions of characters will move around depending on what size the
redraw chunks are.
I would have thought that the sensible place to do _both_ shaping
and bidi was at the top of the `for' loop in do_paint(). (That for
loop begins on line 3395 of the current unpatched terminal.c.) The
loop body begins with
scrpos.y = i + term->disptop;
ldata = lineptr(scrpos.y);
so that `ldata' then contains an array of _all_ the characters on
the line, before they are split up into small chunks for redrawing.
My understanding is that what you want to do is to process `ldata'
immediately after this, to change the order of the characters and do
shaping.
(It's just possible that the inner for loop, starting on line 3414,
might need to be split into two, so that the first loop would
process selection highlights, blinking and the cursor position, and
the second loop would deal with breaking the line up into
subsections for redraw. Then shaping and bidi could be done in
between the two loops, and the cursor position would naturally
adapt.)
> http://lists.arabeyes.org/archives/developer/2004/February/msg00136.html
Your thread complains that backspacing deletes 4 characters. I would
imagine (though I don't have a Windows machine to test on) that this
is because you've modified pwrite() to print high-bit-set characters
literally, but you haven't modified plen() to return the right
number of backspaces needed when deleting them again.
(That stuff needs changing anyway for UTF-8; pressing backspace
during local line editing should delete one _character_, which isn't
necessarily one byte.)
Also I see this comment:
he said in his chat session that it should be done at the top of
the big FOR loop.. between that and the display function inside
the loop, all FEXX values are changed to 06XX
I don't understand this comment. Is it a problem you're having with
the existing PuTTY code?
Cheers,
Simon
--
Simon Tatham "A defensive weapon is one with my finger on the
<anakin at pobox dot com> trigger. An offensive weapon is one with yours."
--- End Message ---