[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kfm-devel
Subject:    Re: [PATCH] [Bug 91955] Bidi algorithm: incorrect implementation of
From:       Mitz Pettel <bugs.kde.org () mitzpettel ! com>
Date:       2004-11-03 23:46:18
Message-ID: 8E894E6B-2DF2-11D9-99B1-0003937C3DB0 () mitzpettel ! com
[Download RAW message or body]

On Nov 3, 2004, at 10:20 PM, Stephan Kulow wrote:

> Am Mittwoch 03 November 2004 17:21 schrieb Mitz Pettel:
>> I see. I think changing line 859 from
>>                       if( bidi.status.eor != QChar::DirL ) {
>> to
>>                       if( bidi.status.eor != QChar::DirL &&
>> bidi.status.eor!=QChar::DirEN ) {
>> should take care of most of these.
>
> I will try tomorrow. Thanks for the fix.

Forget it. I tried it myself and it failed one of the test cases. 
Here's the latest revision of the patch. This one looks good on 
bidi.html, and should not generate excessive runs (however I didn't 
test that part).


["bidi.cpp.diff" (bidi.cpp.diff)]

Index: bidi.cpp
===================================================================
RCS file: /home/kde/kdelibs/khtml/rendering/bidi.cpp,v
retrieving revision 1.198
diff -u -r1.198 bidi.cpp
--- bidi.cpp	26 Oct 2004 18:58:41 -0000	1.198
+++ bidi.cpp	3 Nov 2004 23:39:17 -0000
@@ -856,26 +856,25 @@
                 case QChar::DirS:
                 case QChar::DirWS:
                 case QChar::DirON:
-                    if(dir != QChar::DirL) {
+                     if( bidi.status.eor != QChar::DirL ) {
                         //last stuff takes embedding dir
-                        if( bidi.context->dir == QChar::DirR ) {
-                            if(!(bidi.status.eor == QChar::DirR)) {
-                                // AN or EN
+                        if(bidi.context->dir == QChar::DirL || bidi.status.lastStrong == QChar::DirL) {
+                            if ( bidi.status.eor != QChar::DirEN )
                                 appendRun( bidi );
-                                dir = QChar::DirR;
-                            }
-                            else
-                                bidi.eor = bidi.last;
-                            appendRun( bidi );
                             dir = QChar::DirL;
+                            bidi.eor = bidi.current;
                             bidi.status.eor = QChar::DirL;
                         } else {
-                            if(bidi.status.eor == QChar::DirR) {
+                            if ( bidi.status.eor == QChar::DirEN )
+                            {
+                                dir = QChar::DirEN;
                                 appendRun( bidi );
-                                dir = QChar::DirL;
-                            } else {
-                                bidi.eor = bidi.current; bidi.status.eor = QChar::DirL; break;
                             }
+                            dir = QChar::DirR; // stuff before neutrals
+                            bidi.eor = bidi.last;
+                            appendRun( bidi );
+                            dir = QChar::DirL;
+                            bidi.status.eor = QChar::DirL;
                         }
                     } else {
                         bidi.eor = bidi.current; bidi.status.eor = QChar::DirL;
@@ -911,16 +910,18 @@
                 case QChar::DirON:
                     if( !(bidi.status.eor == QChar::DirR) && !(bidi.status.eor == QChar::DirAL) ) {
                         //last stuff takes embedding dir
-                        if(bidi.context->dir == QChar::DirR || bidi.status.lastStrong == QChar::DirR) {
+                        if(bidi.context->dir == QChar::DirR || bidi.status.lastStrong == QChar::DirR
+                            || bidi.status.lastStrong == QChar::DirAL) {
                             appendRun( bidi );
                             dir = QChar::DirR;
                             bidi.eor = bidi.current;
-			    bidi.status.eor = QChar::DirR;
+                            bidi.status.eor = QChar::DirR;
                         } else {
+                            dir = QChar::DirL; // stuff before neutrals must have been L
                             bidi.eor = bidi.last;
                             appendRun( bidi );
                             dir = QChar::DirR;
-			    bidi.status.eor = QChar::DirR;
+                            bidi.status.eor = QChar::DirR;
                         }
                     } else {
                         bidi.eor = bidi.current; bidi.status.eor = QChar::DirR;
@@ -940,10 +941,7 @@
             if(!(bidi.status.lastStrong == QChar::DirAL)) {
                 // if last strong was AL change EN to AN
                 if(dir == QChar::DirON) {
-                    if(bidi.status.lastStrong == QChar::DirAL)
-                        dir = QChar::DirAN;
-                    else
-                        dir = QChar::DirL;
+                    dir = QChar::DirL;
                 }
                 switch(bidi.status.last)
                     {
@@ -1115,7 +1113,6 @@
                 break;
             case QChar::DirEN:
                 if ( bidi.status.last == QChar::DirL ) {
-                    bidi.status.last = QChar::DirL;
                     break;
                 }
                 // fall through


Best regards,
-- Mitz Pettel

[prev in list] [next in list] [prev in thread] [next in thread]