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

Putty Shapes. Testers?

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.



added Shaping according to Simon's recomendations...

New Patch available with an extra file... from the CVS
http://cvs.arabeyes.org/cvsweb/projects/external/putty/

can i make a patch file that adds a file..? i mean do patches have to
be between original and new files only, or can the patch add a file that
doesnt have an original... i.e in this situation do i have to update arabic.c
and the .patch or can i append arabic.c to the .patch ...?

will this reach Simon..?? nadim !

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. 



umm, i choose arabic.c if u dont mind, and the only additions to
terminal.c and window.c are only 1 function call each and
a few useless comments...

i included the arabic.c file in the MSVC project, did not do any #includes

btw, Nadim, i stole the header off VIM's arabic.c file, and added my name
and a link to Arabeyes... :))
and i also changed the README file..., would u mind reviewing the 2
files nadim, and telling me if anything is wrong or missing..??

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.

do_arabic_flip() only flips the letters in a word, so it benefits from the

division of the line into words according to their attributes... anyway as
soon as  Bidi is implemented, do_arabic_flip() is going out the window...

umm, u know what its actually pretty useless now because you have to
force a redraw on putty for it to work, so maybe i'll remove it now...


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.



ok, done..

(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



i dont see y we need to break the inner loop into 2..?

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.) 



exactly, i changed pwrite()... what do u think of the changes..?
and what other changes to plen() do u suggest..?


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? 



sorry, my mistake..., i used to Shape on term->disptext, according
to your irc session log... when i did that, term->disptext would contain
the shaped Data (Form-B FEXX) but the function do_text() was called
with the Unshaped data (06XX)... i figured it out now... because do_text()
is called with data from ldata not disptext....
nevermind that...

anyway, here are the remaining issues...

+ deletes 4 Arabic letters, (plen() needs modification)
+ i think maybe Windows manages to sneak in... i'll check that...
   because some letters are flipped, maybe an error in my flipping function
   (i dont think any1 will understand what im talking about... :)) )
+ LAA is not shaped.. (ligatures..)
+ need to recheck the shaping code again...
+ how do we turn Arabic On and OFF, or do we just leave it always on..?
+ do u want to add ant #defines and #ifdefs simon..???
+ typing LAA with the Keyboards own LAA key, generates two letters
  which are not arabic... !!! (i dont remember that)...
+ Bidi...
+ ask Simon if its OK doing the shaping on the unsigned long array...?
  or do we need to extract the letters and do the 2 loop thing...??!!
+ some spacing is too much...

My Schedule: ( i've seen posts about Scheduling, am i doing it right..?)
in the next 7 to 10 days i'll recheck the Shaping code, and see about adding
Bidi, if the shaping is stable, and efficient and accepted by Simon, i'll add
something to PuTTy's Blurb :)))

btw, Tripod kicked me, because i was using the account as storage, so i wont
be able to provide an .exe, any1 willing to Host it for Testers..?

btw2, Any1 willing to test PuTTy..?

OK, Your move Simon...

ak...

_________________________________________________________________
Help STOP SPAM with the new MSN 8 and get 2 months FREE* http://join.msn.com/?page=features/junkmail