Changeset 82297 in webkit

Timestamp:

Mar 29, 2011 12:00:21 PM (13 years ago)

Author:

eric@webkit.org

Message:

2011-03-29 Eric Seidel <​eric@webkit.org>

Reviewed by Ryosuke Niwa.

Start to clean up BidiResolver::createBidiRunsForLine so that mere mortals can understand it
​https://bugs.webkit.org/show_bug.cgi?id=57338

I'm attempting to break createBidiRunsForLine into understandable pieces
so that we can tell what it's actually doing. Our implementation of the
unicode bidi algorithm is slightly different from the spec in that we
run it per-line (instead of over the entire paragraph at once). This is
great for performance (our implementation is resumable), but it makes
things a bit tricky to understand. Splitting createBidiRunsForLine into
pieces should help make our UBA implementation more readable.

• platform/text/BidiResolver.h: (WebCore::::updateStatusLastFromCurrentDirection): (WebCore::::createBidiRunsForLine):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/text/BidiResolver.h (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-03-29  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by Ryosuke Niwa.
+
+        Start to clean up BidiResolver::createBidiRunsForLine so that mere mortals can understand it
+        https://bugs.webkit.org/show_bug.cgi?id=57338
+
+        I'm attempting to break createBidiRunsForLine into understandable pieces
+        so that we can tell what it's actually doing.  Our implementation of the
+        unicode bidi algorithm is slightly different from the spec in that we
+        run it per-line (instead of over the entire paragraph at once).  This is
+        great for performance (our implementation is resumable), but it makes
+        things a bit tricky to understand.  Splitting createBidiRunsForLine into
+        pieces should help make our UBA implementation more readable.
+
+        * platform/text/BidiResolver.h:
+        (WebCore::::updateStatusLastFromCurrentDirection):
+        (WebCore::::createBidiRunsForLine):
+
 2011-03-29  Mario Sanchez Prada  <msanchez@igalia.com>
 

trunk/Source/WebCore/platform/text/BidiResolver.h

@@ -211 +211 @@
     void checkDirectionInLowerRaiseEmbeddingLevel();
 
+    void updateStatusLastFromCurrentDirection(WTF::Unicode::Direction);
+
     Vector<WTF::Unicode::Direction, 8> m_currentExplicitEmbeddingSequence;
 };
@@ -518 +520 @@
     if (!afterEnd)
         m_lastRun = startRun;
+}
+
+template <class Iterator, class Run>
+inline void BidiResolver<Iterator, Run>::updateStatusLastFromCurrentDirection(WTF::Unicode::Direction dirCurrent)
+{
+    using namespace WTF::Unicode;
+    switch (dirCurrent) {
+    case EuropeanNumberTerminator:
+        if (m_status.last != EuropeanNumber)
+            m_status.last = EuropeanNumberTerminator;
+        break;
+    case EuropeanNumberSeparator:
+    case CommonNumberSeparator:
+    case SegmentSeparator:
+    case WhiteSpaceNeutral:
+    case OtherNeutral:
+        switch (m_status.last) {
+        case LeftToRight:
+        case RightToLeft:
+        case RightToLeftArabic:
+        case EuropeanNumber:
+        case ArabicNumber:
+            m_status.last = dirCurrent;
+            break;
+        default:
+            m_status.last = OtherNeutral;
+        }
+        break;
+    case NonSpacingMark:
+    case BoundaryNeutral:
+    case RightToLeftEmbedding:
+    case LeftToRightEmbedding:
+    case RightToLeftOverride:
+    case LeftToRightOverride:
+    case PopDirectionalFormat:
+        // ignore these
+        break;
+    case EuropeanNumber:
+        // fall through
+    default:
+        m_status.last = dirCurrent;
+    }
 }
 
@@ -859 +903 @@
         }
 
-        // set m_status.last as needed.
-        switch (dirCurrent) {
-            case EuropeanNumberTerminator:
-                if (m_status.last != EuropeanNumber)
-                    m_status.last = EuropeanNumberTerminator;
-                break;
-            case EuropeanNumberSeparator:
-            case CommonNumberSeparator:
-            case SegmentSeparator:
-            case WhiteSpaceNeutral:
-            case OtherNeutral:
-                switch(m_status.last) {
-                    case LeftToRight:
-                    case RightToLeft:
-                    case RightToLeftArabic:
-                    case EuropeanNumber:
-                    case ArabicNumber:
-                        m_status.last = dirCurrent;
-                        break;
-                    default:
-                        m_status.last = OtherNeutral;
-                    }
-                break;
-            case NonSpacingMark:
-            case BoundaryNeutral:
-            case RightToLeftEmbedding: 
-            case LeftToRightEmbedding: 
-            case RightToLeftOverride: 
-            case LeftToRightOverride: 
-            case PopDirectionalFormat:
-                // ignore these
-                break;
-            case EuropeanNumber:
-                // fall through
-            default:
-                m_status.last = dirCurrent;
-        }
-
+        updateStatusLastFromCurrentDirection(dirCurrent);
         last = current;
 
@@ -942 +949 @@
     unsigned char levelLow = 128;
     unsigned char levelHigh = 0;
-    Run* r = firstRun();
-    while (r) {
-        if (r->m_level > levelHigh)
-            levelHigh = r->m_level;
-        if (r->m_level < levelLow)
-            levelLow = r->m_level;
-        r = r->next();
+    for (Run* r = firstRun(); r; r = r->next()) {
+        levelHigh = max(r->m_level, levelHigh);
+        levelLow = min(r->m_level, levelLow);
     }