[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: PuTTY shapes - now what ?



Note: forwarded message attached.


__________________________________
Do you Yahoo!?
Yahoo! Mail SpamGuard - Read only the mail you want.
http://antispam.yahoo.com/tools
--- Begin Message ---
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 ---