Make fx_bidi sane.

core/include/fxcrt/fx_bidi.h

Index: core/include/fxcrt/fx_bidi.h
diff --git a/core/include/fxcrt/fx_bidi.h b/core/include/fxcrt/fx_bidi.h
index a55ce6cfd2035db3ac08e3e1751c4b7d02c6d878..871a25eec7201972cecb4f1072c03cc3aa3725c4 100644
--- a/core/include/fxcrt/fx_bidi.h
+++ b/core/include/fxcrt/fx_bidi.h
@@ -7,15 +7,24 @@
 #ifndef CORE_INCLUDE_FXCRT_FX_BIDI_H_
 #define CORE_INCLUDE_FXCRT_FX_BIDI_H_
 
+#include <vector>
+#include <memory>

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

memory before vector.

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

Done.

+
+#include "fx_string.h"
 #include "fx_system.h"
 
 // Processes characters and group them into segments based on text direction.
 class CFX_BidiChar {
  public:
   enum Direction { NEUTRAL, LEFT, RIGHT };
+  struct Segment {
+    int32_t start;        // Start position.
+    int32_t count;        // Character count.
+    Direction direction;  // Segment direction.
+  };
 
-  CFX_BidiChar();
-  ~CFX_BidiChar();
+  CFX_BidiChar()
+      : m_CurrentSegment({0, 0, NEUTRAL}), m_LastSegment({0, 0, NEUTRAL}) {}

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

Keep the ctor impl in the .cpp file?

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

Done.

 
   // Append a character and classify it as left, right, or neutral.
   // Returns true if the character has a different direction than the
@@ -27,33 +36,37 @@ class CFX_BidiChar {
   // Returns true if there is still a segment to process.
   bool EndChar();
 
-  // Get information about the segment to process.
-  // The segment's start position and character count is returned in |iStart|
-  // and |iCount|, respectively. Pass in null pointers if the information is
-  // not needed.
-  // Returns the segment direction.
-  Direction GetBidiInfo(int32_t* iStart, int32_t* iCount) const;
+  // Call after a change in direction is indicated by the above to get
+  // information about the segment to process.
+  Segment GetSegmentInfo() const { return m_LastSegment; }
 
  private:
-  void SaveCurrentStateToLastState();
-
-  // Position of the current segment.
-  int32_t m_iCurStart;
+  void StartNewSegment(CFX_BidiChar::Direction direction);
 
-  // Number of characters in the current segment.
-  int32_t m_iCurCount;
+  Segment m_CurrentSegment;
+  Segment m_LastSegment;
+};
 
-  // Direction of the current segment.
-  Direction m_CurBidi;
+class CFX_BidiString {
+ public:
+  using const_iterator = std::vector<CFX_BidiChar::Segment>::const_iterator;
+  CFX_BidiString(const CFX_WideString& str);

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

explicit

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

Done.

 
-  // Number of characters in the last segment.
-  int32_t m_iLastStart;
+  // Overall direction is always LEFT or RIGHT, never NEUTRAL.
+  void SetOverallDirection(CFX_BidiChar::Direction eDirection);

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

If you make this private and only expose wrappers with SetOverallDirectionLeft()
/ SetOverallDirectionRight(), callers can't get it wrong.

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

Actually, we only call this with an argument of RIGHT, so add that one only.

+  CFX_BidiChar::Direction OverallDirection() const {
+    return m_eOverallDirection;
+  }
 
-  // Number of characters in the last segment.
-  int32_t m_iLastCount;
+  FX_WCHAR CharAt(size_t x) const { return m_Str[x]; }
+  const_iterator begin() const { return m_Order.begin(); }
+  const_iterator end() const { return m_Order.end(); }
 
-  // Direction of the last segment.
-  Direction m_LastBidi;
+ private:
+  const CFX_WideString m_Str;
+  std::unique_ptr<CFX_BidiChar> m_pBidiChar;
+  std::vector<CFX_BidiChar::Segment> m_Order;
+  CFX_BidiChar::Direction m_eOverallDirection;
 };
 
 #endif  // CORE_INCLUDE_FXCRT_FX_BIDI_H_