Make fx_bidi sane.

core/src/fxcrt/fx_bidi.cpp

Index: core/src/fxcrt/fx_bidi.cpp
diff --git a/core/src/fxcrt/fx_bidi.cpp b/core/src/fxcrt/fx_bidi.cpp
index d5d87a345b60ec66b571d95f4c1d9392eecb5e32..81b6dbd2fdaff0915f399196d434311bf8c5f111 100644
--- a/core/src/fxcrt/fx_bidi.cpp
+++ b/core/src/fxcrt/fx_bidi.cpp
@@ -7,60 +7,74 @@
 #include "core/include/fxcrt/fx_bidi.h"
 #include "core/include/fxcrt/fx_ucd.h"
 
-CFX_BidiChar::CFX_BidiChar()
-    : m_iCurStart(0),
-      m_iCurCount(0),
-      m_CurBidi(NEUTRAL),
-      m_iLastStart(0),
-      m_iLastCount(0),
-      m_LastBidi(NEUTRAL) {
-}
-
-CFX_BidiChar::~CFX_BidiChar() {
-}
+#include <algorithm>
 
 bool CFX_BidiChar::AppendChar(FX_WCHAR wch) {
   FX_DWORD dwProps = FX_GetUnicodeProperties(wch);
   int32_t iBidiCls = (dwProps & FX_BIDICLASSBITSMASK) >> FX_BIDICLASSBITS;
-  Direction bidi = NEUTRAL;
+  Direction direction = NEUTRAL;
   switch (iBidiCls) {
     case FX_BIDICLASS_L:
     case FX_BIDICLASS_AN:
     case FX_BIDICLASS_EN:
-      bidi = LEFT;
+      direction = LEFT;
       break;
     case FX_BIDICLASS_R:
     case FX_BIDICLASS_AL:
-      bidi = RIGHT;
+      direction = RIGHT;
       break;
   }
 
-  bool bRet = (bidi != m_CurBidi);
-  if (bRet) {
-    SaveCurrentStateToLastState();
-    m_CurBidi = bidi;
-  }
-  m_iCurCount++;
-  return bRet;
+  bool bChangeDirection = (direction != m_CurrentSegment.direction);
+  if (bChangeDirection)
+    StartNewSegment(direction);
+
+  m_CurrentSegment.count++;
+  return bChangeDirection;
 }
 
 bool CFX_BidiChar::EndChar() {
-  SaveCurrentStateToLastState();
-  return m_iLastCount > 0;
+  StartNewSegment(NEUTRAL);
+  return m_LastSegment.count > 0;
 }
 
-CFX_BidiChar::Direction CFX_BidiChar::GetBidiInfo(int32_t* iStart,
-                                                  int32_t* iCount) const {
-  if (iStart)
-    *iStart = m_iLastStart;
-  if (iCount)
-    *iCount = m_iLastCount;
-  return m_LastBidi;
+void CFX_BidiChar::StartNewSegment(CFX_BidiChar::Direction direction) {
+  m_LastSegment = m_CurrentSegment;
+  m_CurrentSegment.start += m_CurrentSegment.count;
+  m_CurrentSegment.count = 0;
+  m_CurrentSegment.direction = direction;
 }
 
-void CFX_BidiChar::SaveCurrentStateToLastState() {
-  m_LastBidi = m_CurBidi;
-  m_iLastStart = m_iCurStart;
-  m_iCurStart = m_iCurCount;
-  m_iLastCount = m_iCurCount - m_iLastStart;
+CFX_BidiString::CFX_BidiString(const CFX_WideString& str)
+    : m_Str(str),
+      m_pBidiChar(new CFX_BidiChar),
+      m_eOverallDirection(CFX_BidiChar::LEFT) {
+  size_t nR2L = 0;
+  size_t nL2R = 0;
+  for (int i = 0; i < m_Str.GetLength(); ++i) {
+    if (m_pBidiChar->AppendChar(m_Str.GetAt(i))) {
+      m_Order.push_back(m_pBidiChar->GetSegmentInfo());
+      if (m_Order.back().direction == CFX_BidiChar::RIGHT)
+        nR2L++;
+      else if (m_Order.back().direction == CFX_BidiChar::LEFT)
+        nL2R++;
+    }
+  }
+  if (m_pBidiChar->EndChar()) {
+    m_Order.push_back(m_pBidiChar->GetSegmentInfo());
+    if (m_Order.back().direction == CFX_BidiChar::RIGHT)

[Lei Zhang, 2016/02/12 00:46:43]

Maybe just push into |m_Order| and calculate |nR2L| / |nL2R| with std::count() ?

[Tom Sepez, 2016/02/12 17:54:33]

That's really clean.

+      nR2L++;
+    else if (m_Order.back().direction == CFX_BidiChar::LEFT)
+      nL2R++;
+  }
+  if (nR2L > 0 && nR2L >= nL2R)
+    SetOverallDirection(CFX_BidiChar::RIGHT);
+}
+
+void CFX_BidiString::SetOverallDirection(CFX_BidiChar::Direction eDirection) {
+  ASSERT(eDirection != CFX_BidiChar::NEUTRAL);
+  if (eDirection != m_eOverallDirection) {
+    std::reverse(m_Order.begin(), m_Order.end());

[Tom Sepez, 2016/02/11 18:25:58]

Note: this is the key idea. Rather than forcing our callers to figure out front
to back vs. back to front, give them back in the correct order for the
prevailing direction.

[Lei Zhang, 2016/02/12 00:46:43]

SGTM

+    m_eOverallDirection = eDirection;
+  }
 }