Cleanup IFX_BidiChar

core/src/fxcrt/fx_arabic.cpp

 // Copyright 2014 PDFium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "../../include/fxcrt/fx_ucd.h"
 #include "fx_arabic.h"
 
 extern const FX_DWORD gs_FX_TextLayout_CodeProperties[65536];

[Tom Sepez, 2015/06/19 20:30:32]

nit: This needs to be in a .h file. Does it have to be a dword, or can we save
like 128kb or 192kb with a uint16 or uint8?

[Tom Sepez, 2015/06/19 21:36:10]

Answer is that its a DWORD.  I was counting the bits as defined in fx_ucd.h and
came up with 16, but there's some stuff in the high bytes that's just
manipulated with magic numbers in the .cpp

Still, needs to be in a header though.

[Lei Zhang, 2015/08/17 17:41:35]

Done.

+
 IFX_BidiChar* IFX_BidiChar::Create()
 {
     return new CFX_BidiChar;
 }
+
 CFX_BidiChar::CFX_BidiChar()
-    : m_bSeparateNeutral(TRUE)
-    , m_iCurStart(0)
-    , m_iCurCount(0)
-    , m_iCurBidi(0)
-    , m_iLastBidi(0)
-    , m_iLastStart(0)
-    , m_iLastCount(0)
+    : m_iCurStart(0),
+      m_iCurCount(0),
+      m_CurBidi(NEUTRAL),
+      m_iLastStart(0),
+      m_iLastCount(0),
+      m_LastBidi(NEUTRAL)
 {
 }
-void CFX_BidiChar::SetPolicy(FX_BOOL bSeparateNeutral)
+
+CFX_BidiChar::~CFX_BidiChar()
 {
-    m_bSeparateNeutral = bSeparateNeutral;
 }
 
-FX_BOOL CFX_BidiChar::AppendChar(FX_WCHAR wch)
+bool CFX_BidiChar::AppendChar(FX_WCHAR wch)
 {
     FX_DWORD dwProps = gs_FX_TextLayout_CodeProperties[(FX_WORD)wch];
     int32_t iBidiCls = (dwProps & FX_BIDICLASSBITSMASK) >> FX_BIDICLASSBITS;
-    int32_t iContext = 0;
+    Direction bidi = NEUTRAL;
     switch (iBidiCls) {
         case FX_BIDICLASS_L:
         case FX_BIDICLASS_AN:
         case FX_BIDICLASS_EN:
-            iContext = 1;
+            bidi = LEFT;
             break;
         case FX_BIDICLASS_R:
         case FX_BIDICLASS_AL:
-            iContext = 2;
+            bidi = RIGHT;
             break;
     }
-    FX_BOOL bRet = FALSE;
-    if (iContext != m_iCurBidi) {
-        if (m_bSeparateNeutral) {
-            bRet = TRUE;
-        } else {
-            if (m_iCurBidi == 0) {
-                bRet = (m_iCurCount > 0);
-            } else {
-                bRet = (iContext != 0);
-            }
-        }
-        if (bRet) {
-            m_iLastBidi = m_iCurBidi;
-            m_iLastStart = m_iCurStart;
-            m_iCurStart = m_iCurCount;
-            m_iLastCount = m_iCurCount - m_iLastStart;
-        }
-        if (m_bSeparateNeutral || iContext != 0) {
-            m_iCurBidi = iContext;
-        }
+
+    bool bRet = (bidi != m_CurBidi);
+    if (bRet) {
+        SaveCurrentStateToLastState();
+        m_iCurStart = m_iCurCount;
+        m_CurBidi = bidi;
     }
-    m_iCurCount ++;
+    m_iCurCount++;
     return bRet;
 }
-FX_BOOL CFX_BidiChar::EndChar()
+
+bool CFX_BidiChar::EndChar()
 {
-    m_iLastBidi = m_iCurBidi;
-    m_iLastStart = m_iCurStart;
-    m_iCurStart = m_iCurCount;

[Tom Sepez, 2015/06/19 20:30:32]

Did we lose this assignment?

[Lei Zhang, 2015/08/17 17:41:35]

Yep, don't remember why I did that.

-    m_iLastCount = m_iCurCount - m_iLastStart;
+    SaveCurrentStateToLastState();
     return m_iLastCount > 0;
 }
-int32_t CFX_BidiChar::GetBidiInfo(int32_t &iStart, int32_t &iCount)
+
+IFX_BidiChar::Direction CFX_BidiChar::GetBidiInfo(int32_t* iStart,
+                                                  int32_t* iCount)
 {
-    iStart = m_iLastStart;
-    iCount = m_iLastCount;
-    return m_iLastBidi;
+    if (iStart)
+        *iStart = m_iLastStart;
+    if (iCount)
+        *iCount = m_iLastCount;
+    return m_LastBidi;
 }
-void CFX_BidiChar::Reset()
+
+void CFX_BidiChar::SaveCurrentStateToLastState()
 {
-    m_iCurStart = 0;
-    m_iCurCount = 0;
-    m_iCurBidi = 0;
-    m_iLastBidi = 0;
-    m_iLastStart = 0;
-    m_iLastCount = 0;
+    m_iLastStart = m_iCurStart;
+    m_iLastCount = m_iCurCount - m_iLastStart;
+    m_LastBidi = m_CurBidi;
 }