Re: Total bidi regression

2007-09-26 Thread Shachar Shemesh
Maarten Lankhorst wrote:
>
> I may have slightly misunderstood those flags then. I was under the
> impression that the FORCE flags would be similar to LRO/RLO.
The only thing that behaves like LRO and RLO are LRO and RLO. Believe
you me, no one was more surprised than me when I found out that Windows
actually respected them - they are as far away from an end user's
experience as you might get (though useful, if you know about them).

The Unicode algorithm keeps talking about setting the paragraph
direction based on the first strong direction character in the paragraph
(P2). As nice as that may have sound to the people drafting it, it only
sort of works in real life. Either way, Window's BiDi doesn't work that
way. The paragraph direction is set explicitly by the calling program
(or, failing that, by the HDC, or, failing that, by an EXT_STYLE in the
Window, you get the picture). This means the end of section 3.3.1,
instructing implementors to ignore P2 and P3 in case of "higher-level
protocol" apply here. Instead, the paragraph direction is explicitly
passed to BIDI_Reorder through the "dwWineGCP_Flags" argument.
>  Instead
> they are probably more like LRE/RLE.
It's better to say that LRE and RLE allows one to switch paragraph
direction for a specific part of the paragraph. In other words, LRE/RLE
are like the paragraph direction, not vice versa.
>  If that is a real problem I will
> send in a patch.
If?
>  I would still rather prefer a real bidi implementation,
> so that selecting and deleting characters would work properly.
Lost you there completely. What do you mean by "selecting and deleting"?
If you mean a BiDi editor, I think you have the tasks confused.
Reordering characters in order to get them out on screen for printing
has very little to do with string editing. It's one minor small step to
start with. Bidi editing is a lot more complex.
>  To my
> defense, there was no real clarification for them in the source.
>   
The comments in the code used the same terminology used by Annex 9. The
parameters were passed almost directly from the inputs in BIDI_Reorder
to ICU's input functions, where they are documented in the ICU
documentation. These parameters were also received, pretty directly,
from ExtTextOut, again, where they are documented in MSDN. To my eyes,
this is the level of documentation any Wine hacker should need. I don't
believe code comments should start explaining algorithms, particularly
algorithms implemented by a library and documented in an international
standard.

And I should warn you that edit control is several magnitudes more
complicated. Questions such as cursor position when the cursor is
between two letters that are, after reordering (i.e. - on display) not
adjacent, what happens if a given cursor location is clicked which has
two possible logical locations, and what to do if the user then clicks
"del" or types a letter in an RTL or an LTR language. Editing is
COMPLEX, and the road is not paved and documented. Matti Alluche wrote a
document once that gives specifications for BiDi editing, but after
Mozilla implemented it, I whole heartedly recommend that you avoid it.
Your best course of action is to find out what Windows does for its edit
control and copy that.
> Cheers,
> Maarten
>   

Shachar




Re: Total bidi regression

2007-09-26 Thread Maarten Lankhorst
Shachar Shemesh schreef:
> Maarten Lankhorst wrote:
>   
>> Shachar Shemesh schreef:
>>   
>> 
>>> Maarten Lankhorst wrote:
>>>   
>>> 
>>>   
 If you want it back try replacing this in font.c:
 WINE_GCPW_FORCE_RTL:WINE_GCPW_FORCE_LTR
 change FORCE to LOOSE, it should work then.
 
   
 
>>> I'm not sure what you are suggesting.
>>>
>>> WINE_GCPW_FORCE_RTL only appear on line 1089 of bidi.c, which reads
>>> 
>>>   
case WINE_GCPW_FORCE_RTL: forcedir = R; baselevel = 1; break;
 
   
 
>>> I'm not sure what you are suggesting I do with it.
>>>
>>> Either way, the change you are suggesting will only affect (if I
>>> understand the code correctly) the paragraph direction setting, where as
>>> I'm experiencing total lack of BiDi reordering of any kind.
>>>   
>>> 
>>>   
>> Before arguing, you should really give it a try, it helps. ;-)
>> 
> Sure thing. Just what, exactly, is "it"? What do you suggest I change,
> and what to? I am really asking you to be less ambiguous.
>
> If you mean to change the "FORCE" to "LOOSE", then things are slightly
> better in that same direction runs are at least now being reordered, but
> things are still at a huge regression. Neutrals (at least some neutrals,
> like space) seem to be incorrectly handled in mixed paragraphs (I'm not
> sure, as this could be a font problem as well). Also, and this is
> confirmed, there is now no way to request a right to left paragraph, at
> all. Numbers I haven't checked.
>
> Again, I may have misunderstood what change you meant for me to try. If
> you don't want to be misunderstood, try just sending a patch. I'm more
> than willing to help, but not if it means trying to decrypt what code
> changes you are suggesting, or where in the 1200 lines file they are
> meant to be.
>   
>> forcedir means basically force every not-control character to that
>> direction.
>> 
> Which doesn't ring a bell as far as the Annex 9. I don't recall any such
> thing there. They had a notion of "paragraph direction", which did
> affect NEUTRALS, and only if they were placed on the border between
> conflicting direction runs (and also the initial nesting level, of
> course). The ONLY thing I recall that could cause a hard RTL or hard LTR
> character to be rendered in the opposite direction was an LRO/RLO, which
> was never used here. Thus, I say again, the change you offer seem out of
> place in relation to the place you offer it, and it seems to me that
> there is an error in your implementation of Annex 9.
>   
I may have slightly misunderstood those flags then. I was under the
impression that the FORCE flags would be similar to LRO/RLO. Instead
they are probably more like LRE/RLE. If that is a real problem I will
send in a patch. I would still rather prefer a real bidi implementation,
so that selecting and deleting characters would work properly. To my
defense, there was no real clarification for them in the source.

Cheers,
Maarten




Re: Total bidi regression

2007-09-26 Thread Shachar Shemesh
Maarten Lankhorst wrote:
> Shachar Shemesh schreef:
>   
>> Maarten Lankhorst wrote:
>>   
>> 
>>> If you want it back try replacing this in font.c:
>>> WINE_GCPW_FORCE_RTL:WINE_GCPW_FORCE_LTR
>>> change FORCE to LOOSE, it should work then.
>>> 
>>>   
>> I'm not sure what you are suggesting.
>>
>> WINE_GCPW_FORCE_RTL only appear on line 1089 of bidi.c, which reads
>> 
>>>case WINE_GCPW_FORCE_RTL: forcedir = R; baselevel = 1; break;
>>> 
>>>   
>> I'm not sure what you are suggesting I do with it.
>>
>> Either way, the change you are suggesting will only affect (if I
>> understand the code correctly) the paragraph direction setting, where as
>> I'm experiencing total lack of BiDi reordering of any kind.
>>   
>> 
> Before arguing, you should really give it a try, it helps. ;-)
>   
Sure thing. Just what, exactly, is "it"? What do you suggest I change,
and what to? I am really asking you to be less ambiguous.

If you mean to change the "FORCE" to "LOOSE", then things are slightly
better in that same direction runs are at least now being reordered, but
things are still at a huge regression. Neutrals (at least some neutrals,
like space) seem to be incorrectly handled in mixed paragraphs (I'm not
sure, as this could be a font problem as well). Also, and this is
confirmed, there is now no way to request a right to left paragraph, at
all. Numbers I haven't checked.

Again, I may have misunderstood what change you meant for me to try. If
you don't want to be misunderstood, try just sending a patch. I'm more
than willing to help, but not if it means trying to decrypt what code
changes you are suggesting, or where in the 1200 lines file they are
meant to be.
> forcedir means basically force every not-control character to that
> direction.
>   
Which doesn't ring a bell as far as the Annex 9. I don't recall any such
thing there. They had a notion of "paragraph direction", which did
affect NEUTRALS, and only if they were placed on the border between
conflicting direction runs (and also the initial nesting level, of
course). The ONLY thing I recall that could cause a hard RTL or hard LTR
character to be rendered in the opposite direction was an LRO/RLO, which
was never used here. Thus, I say again, the change you offer seem out of
place in relation to the place you offer it, and it seems to me that
there is an error in your implementation of Annex 9.
> Cheers,
> Maarten
>   
Shachar




Re: Total bidi regression

2007-09-26 Thread Maarten Lankhorst
Shachar Shemesh schreef:
> Maarten Lankhorst wrote:
>   
>> If you want it back try replacing this in font.c:
>> WINE_GCPW_FORCE_RTL:WINE_GCPW_FORCE_LTR
>> change FORCE to LOOSE, it should work then.
>> 
> I'm not sure what you are suggesting.
>
> WINE_GCPW_FORCE_RTL only appear on line 1089 of bidi.c, which reads
>>case WINE_GCPW_FORCE_RTL: forcedir = R; baselevel = 1; break;
>> 
> I'm not sure what you are suggesting I do with it.
>
> Either way, the change you are suggesting will only affect (if I
> understand the code correctly) the paragraph direction setting, where as
> I'm experiencing total lack of BiDi reordering of any kind.
>   
Before arguing, you should really give it a try, it helps. ;-)
forcedir means basically force every not-control character to that
direction.

Cheers,
Maarten




Re: Total bidi regression

2007-09-26 Thread Shachar Shemesh
Maarten Lankhorst wrote:
>
> If you want it back try replacing this in font.c:
> WINE_GCPW_FORCE_RTL:WINE_GCPW_FORCE_LTR
> change FORCE to LOOSE, it should work then.
>   
I'm not sure what you are suggesting.

WINE_GCPW_FORCE_RTL only appear on line 1089 of bidi.c, which reads:

>case WINE_GCPW_FORCE_RTL: forcedir = R; baselevel = 1; break;

I'm not sure what you are suggesting I do with it.

Either way, the change you are suggesting will only affect (if I
understand the code correctly) the paragraph direction setting, where as
I'm experiencing total lack of BiDi reordering of any kind.

All codes taken from latest git.
> Cheers,
> Maarten
>   
Thanks,
Shachar




Re: Total bidi regression

2007-09-26 Thread Maarten Lankhorst
Shachar Shemesh schreef:
> Hi Maarten,
>
> It seems that since your last changes to the Bidi implementation, BiDi
> suffered total regression. At least on my system, no BiDi related text
> (neither Hebrew nor Arabic) gets reordered, at all. Placing breakpoints
> suggest that BIDI_Reorder is still getting called, so I can only assume
> that the problem is somewhere inside it (or in the classification)
>
> I haven't traced inside to see where things went wrong, nor do I know
> whether your work on that matter is done. I just wanted to point out
> that Wine, at the moment, performs no BiDi at all as far as the user is
> concerned.
>
> Shachar
>   
If you want it back try replacing this in font.c:
WINE_GCPW_FORCE_RTL:WINE_GCPW_FORCE_LTR
change FORCE to LOOSE, it should work then.

Cheers,
Maarten




Total bidi regression

2007-09-26 Thread Shachar Shemesh
Hi Maarten,

It seems that since your last changes to the Bidi implementation, BiDi
suffered total regression. At least on my system, no BiDi related text
(neither Hebrew nor Arabic) gets reordered, at all. Placing breakpoints
suggest that BIDI_Reorder is still getting called, so I can only assume
that the problem is somewhere inside it (or in the classification)

I haven't traced inside to see where things went wrong, nor do I know
whether your work on that matter is done. I just wanted to point out
that Wine, at the moment, performs no BiDi at all as far as the user is
concerned.

Shachar