[core] Use ICU for bidirectional text layout and Arabic text shaping

CMakeLists.txt

@@ -52,6 +52,7 @@ mason_use(earcut VERSION 0.12.1 HEADER_ONLY)
 mason_use(protozero VERSION 1.4.2 HEADER_ONLY)
 mason_use(pixelmatch VERSION 0.10.0 HEADER_ONLY)
 mason_use(geojson VERSION 0.3.2 HEADER_ONLY)
+mason_use(icu VERSION 58.1)
 
 if(WITH_COVERAGE)
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} --coverage")

cmake/core-files.cmake

@@ -360,6 +360,8 @@ set(MBGL_CORE_FILES
     src/mbgl/text/quads.hpp
     src/mbgl/text/shaping.cpp
     src/mbgl/text/shaping.hpp
+    src/mbgl/text/bidi.cpp
+    src/mbgl/text/bidi.hpp
 
     # tile
     src/mbgl/tile/geojson_tile.cpp

cmake/core.cmake

@@ -45,6 +45,7 @@ target_add_mason_package(mbgl-core PRIVATE supercluster)
 target_add_mason_package(mbgl-core PRIVATE kdbush)
 target_add_mason_package(mbgl-core PRIVATE earcut)
 target_add_mason_package(mbgl-core PRIVATE protozero)
+target_add_mason_package(mbgl-core PRIVATE icu)
 
 mbgl_platform_core()
 

package.json

@@ -24,7 +24,7 @@
     "lodash": "^4.16.4",
     "mapbox-gl-shaders": "mapbox/mapbox-gl-shaders#597115a1e1bd982944b068f8accde34eada74fc2",
     "mapbox-gl-style-spec": "mapbox/mapbox-gl-style-spec#7f62a4fc9f21e619824d68abbc4b03cbc1685572",
-    "mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#87192085b3c1ebe668524511bfba28381e5eb627",
+    "mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#c32d0c5ac80e3b7393bc17b8944e64fa5cffd90a",
     "mkdirp": "^0.5.1",
     "node-cmake": "^1.2.1",
     "request": "^2.72.0",

platform/default/mbgl/storage/offline_download.cpp

@@ -184,7 +184,7 @@ void OfflineDownload::activateDownload() {
 
         if (!parser.glyphURL.empty()) {
             for (const auto& fontStack : parser.fontStacks()) {
-                for (uint32_t i = 0; i < GLYPH_RANGES_PER_FONT_STACK; i++) {
+                for (char16_t i = 0; i < GLYPH_RANGES_PER_FONT_STACK; i++) {
                     queueResource(Resource::glyphs(parser.glyphURL, fontStack, getGlyphRange(i * GLYPHS_PER_GLYPH_RANGE)));
                 }
             }

platform/default/string_stdlib.cpp

@@ -1,10 +1,10 @@
 #include <mbgl/platform/platform.hpp>
-#include <mbgl/util/utf.hpp>
 #define NU_WITH_TOUPPER
 #define NU_WITH_TOLOWER
 #define NU_WITH_UTF8_WRITER
 #include <libnu/libnu.h>
 #include <cstring>
+#include <sstream>
 
 namespace mbgl { namespace platform {
 

src/mbgl/layout/merge_lines.cpp

@@ -47,10 +47,10 @@ enum class Side {
 };
 
 size_t
-getKey(const std::u32string& text, const GeometryCollection& geom, Side side) {
+getKey(const std::u16string& text, const GeometryCollection& geom, Side side) {
     const GeometryCoordinate& coord = side == Side::Right ? geom[0].back() : geom[0].front();
 
-    auto hash = std::hash<std::u32string>()(text);
+    auto hash = std::hash<std::u16string>()(text);
     boost::hash_combine(hash, coord.x);
     boost::hash_combine(hash, coord.y);
     return hash;

src/mbgl/layout/symbol_feature.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <mbgl/text/bidi.hpp>
 #include <mbgl/tile/geometry_tile_data.hpp>
 #include <mbgl/util/optional.hpp>
 
@@ -10,7 +11,8 @@ namespace mbgl {
 class SymbolFeature {
 public:
     GeometryCollection geometry;
-    optional<std::u32string> text;
+    optional<std::u16string> text;
+    optional<WritingDirection> writingDirection;
     optional<std::string> icon;
     std::size_t index;
 };

src/mbgl/layout/symbol_layout.cpp

@@ -20,6 +20,8 @@
 #include <mbgl/platform/platform.hpp>
 #include <mbgl/platform/log.hpp>
 
+#include <mbgl/text/bidi.hpp>
+
 namespace mbgl {
 
 using namespace style;
@@ -90,10 +92,12 @@ SymbolLayout::SymbolLayout(std::string bucketName_,
                 u8string = platform::lowercase(u8string);
             }
 
-            ft.text = util::utf8_to_utf32::convert(u8string);
+            std::u16string u16string = util::utf8_to_utf16::convert(u8string);
+            ft.text = bidi.bidiTransform(u16string);
+            ft.writingDirection = bidi.baseWritingDirection(u16string);
 
             // Loop through all characters of this text and collect unique codepoints.
-            for (char32_t chr : *ft.text) {
+            for (char16_t chr : *ft.text) {
                 ranges.insert(getGlyphRange(chr));
             }
         }
@@ -194,6 +198,7 @@ void SymbolLayout::prepare(uintptr_t tileUID,
         if (feature.text) {
             shapedText = glyphSet->getShaping(
                 /* string */ *feature.text,
+                /* base direction of text */ *feature.writingDirection,
                 /* maxWidth: ems */ layout.symbolPlacement != SymbolPlacementType::Line ?
                     layout.textMaxWidth * 24 : 0,
                 /* lineHeight: ems */ layout.textLineHeight * 24,
@@ -309,7 +314,7 @@ void SymbolLayout::addFeature(const GeometryCollection &lines,
     }
 }
 
-bool SymbolLayout::anchorIsTooClose(const std::u32string &text, const float repeatDistance, Anchor &anchor) {
+bool SymbolLayout::anchorIsTooClose(const std::u16string &text, const float repeatDistance, Anchor &anchor) {
     if (compareText.find(text) == compareText.end()) {
         compareText.emplace(text, Anchors());
     } else {

src/mbgl/layout/symbol_layout.hpp

@@ -4,6 +4,7 @@
 #include <mbgl/style/layers/symbol_layer_properties.hpp>
 #include <mbgl/layout/symbol_feature.hpp>
 #include <mbgl/layout/symbol_instance.hpp>
+#include <mbgl/text/bidi.hpp>
 
 #include <memory>
 #include <map>
@@ -64,8 +65,8 @@ class SymbolLayout {
                     const GlyphPositions& face,
                     const size_t index);
 
-    bool anchorIsTooClose(const std::u32string& text, const float repeatDistance, Anchor&);
-    std::map<std::u32string, std::vector<Anchor>> compareText;
+    bool anchorIsTooClose(const std::u16string& text, const float repeatDistance, Anchor&);
+    std::map<std::u16string, std::vector<Anchor>> compareText;
 
     void addToDebugBuffers(CollisionTile&, SymbolBucket&);
 
@@ -91,6 +92,8 @@ class SymbolLayout {
     GlyphRangeSet ranges;
     std::vector<SymbolInstance> symbolInstances;
     std::vector<SymbolFeature> features;
+
+    BiDi bidi; // Consider moving this up to geometry tile worker to reduce reinstantiation costs; use of BiDi/ubiditransform object must be constrained to one thread
 };
 
 } // namespace mbgl

src/mbgl/text/bidi.cpp

@@ -0,0 +1,56 @@
+#include <memory>
+
+#include <mbgl/text/bidi.hpp>
+#include <unicode/ubidi.h>
+#include <unicode/ubiditransform.h>
+#include <unicode/ushape.h>
+
+namespace mbgl {
+
+BiDi::BiDi() {
+    UErrorCode errorCode = U_ZERO_ERROR;
+    transform = ubiditransform_open(&errorCode); // Only error is failure to allocate memory, in
+                                                 // that case ubidi_transform would fall back to
+                                                 // creating transform object on the fly
+}
+
+BiDi::~BiDi() {
+    if (transform)
+        ubiditransform_close(transform);
+}
+
+std::u16string BiDi::bidiTransform(const std::u16string& input) {
+    UErrorCode errorCode = U_ZERO_ERROR;
+
+    std::unique_ptr<UChar[]> outputText =
+        std::make_unique<UChar[]>(input.size() * 2); // Maximum output of ubidi_transform is twice
+                                                     // the size of input according to
+                                                     // ubidi_transform.h
+    uint32_t outputLength = ubiditransform_transform(
+        transform, input.c_str(), static_cast<int32_t>(input.size()), outputText.get(),
+        static_cast<int32_t>(input.size()) * 2,
+        UBIDI_DEFAULT_LTR,     // Assume input is LTR unless strong RTL characters are found
+        UBIDI_LOGICAL,         // Input is in logical order
+        UBIDI_LTR,             // Output is in "visual LTR" order
+        UBIDI_VISUAL,          //  ''
+        UBIDI_MIRRORING_ON,    // Use mirroring lookups for things like parentheses that need mirroring
+                               // in RTL text
+        U_SHAPE_LETTERS_SHAPE, // Add options here for handling numbers in bidirectional text
+        &errorCode);
+
+    // If the algorithm fails for any reason, fall back to non-transformed text
+    if (U_FAILURE(errorCode))
+        return input;
+
+    return std::u16string(outputText.get(), outputLength);
+}
+
+WritingDirection BiDi::baseWritingDirection(const std::u16string& input) {
+    // This just looks for the first character with a strong direction property, it does not perform
+    // the BiDi algorithm
+    return ubidi_getBaseDirection(input.c_str(), static_cast<int32_t>(input.size())) == UBIDI_RTL
+        ? WritingDirection::RightToLeft
+        : WritingDirection::LeftToRight;
+}
+
+} // end namespace mbgl

src/mbgl/text/bidi.hpp

@@ -0,0 +1,25 @@
+#pragma once
+
+#include <string>
+
+#include <mbgl/util/noncopyable.hpp>
+
+struct UBiDiTransform;
+
+namespace mbgl {
+
+enum class WritingDirection : bool { LeftToRight, RightToLeft };    
+
+class BiDi : private util::noncopyable {
+public:
+    BiDi();
+    ~BiDi();
+
+    std::u16string bidiTransform(const std::u16string&);
+    WritingDirection baseWritingDirection(const std::u16string&);
+
+private:
+    UBiDiTransform* transform;
+};
+
+} // end namespace mbgl

src/mbgl/text/glyph.cpp

@@ -3,7 +3,7 @@
 namespace mbgl {
 
 // Note: this only works for the BMP
-GlyphRange getGlyphRange(char32_t glyph) {
+GlyphRange getGlyphRange(char16_t glyph) {
     unsigned start = (glyph/256) * 256;
     unsigned end = (start + 255);
     if (start > 65280) start = 65280;

src/mbgl/text/glyph.hpp

@@ -11,7 +11,7 @@
 namespace mbgl {
 
 // Note: this only works for the BMP
-GlyphRange getGlyphRange(char32_t glyph);
+GlyphRange getGlyphRange(char16_t glyph);
 
 struct GlyphMetrics {
     explicit operator bool() const {
@@ -63,10 +63,10 @@ class PositionedGlyph {
 class Shaping {
     public:
     explicit Shaping() : top(0), bottom(0), left(0), right(0) {}
-    explicit Shaping(float x, float y, std::u32string text_)
+    explicit Shaping(float x, float y, std::u16string text_)
         : text(std::move(text_)), top(y), bottom(y), left(x), right(x) {}
     std::vector<PositionedGlyph> positionedGlyphs;
-    std::u32string text;
+    std::u16string text;
     int32_t top;
     int32_t bottom;
     int32_t left;

src/mbgl/text/glyph_atlas.cpp

@@ -81,7 +81,7 @@ void GlyphAtlas::setObserver(GlyphAtlasObserver* observer_) {
 }
 
 void GlyphAtlas::addGlyphs(uintptr_t tileUID,
-                           const std::u32string& text,
+                           const std::u16string& text,
                            const FontStack& fontStack,
                            const GlyphSet& glyphSet,
                            GlyphPositions& face)
@@ -90,7 +90,7 @@ void GlyphAtlas::addGlyphs(uintptr_t tileUID,
 
     const std::map<uint32_t, SDFGlyph>& sdfs = glyphSet.getSDFs();
 
-    for (uint32_t chr : text)
+    for (char16_t chr : text)
     {
         auto sdf_it = sdfs.find(chr);
         if (sdf_it == sdfs.end()) {

src/mbgl/text/glyph_atlas.hpp

@@ -55,7 +55,7 @@ class GlyphAtlas : public util::noncopyable {
     void setObserver(GlyphAtlasObserver* observer);
 
     void addGlyphs(uintptr_t tileUID,
-                   const std::u32string& text,
+                   const std::u16string& text,
                    const FontStack&,
                    const GlyphSet&,
                    GlyphPositions&);

src/mbgl/text/glyph_set.cpp

@@ -2,6 +2,7 @@
 #include <mbgl/platform/log.hpp>
 #include <mbgl/math/minmax.hpp>
 #include <mbgl/util/i18n.hpp>
+#include <mbgl/text/bidi.hpp>
 
 #include <cassert>
 
@@ -31,7 +32,7 @@ const std::map<uint32_t, SDFGlyph> &GlyphSet::getSDFs() const {
     return sdfs;
 }
 
-const Shaping GlyphSet::getShaping(const std::u32string &string, const float maxWidth,
+const Shaping GlyphSet::getShaping(const std::u16string &string, const WritingDirection writingDirection, const float maxWidth,
                                     const float lineHeight, const float horizontalAlign,
                                     const float verticalAlign, const float justify,
                                     const float spacing, const Point<float> &translate) const {
@@ -44,7 +45,7 @@ const Shaping GlyphSet::getShaping(const std::u32string &string, const float max
     const float y = yOffset;
 
     // Loop through all characters of this label and shape.
-    for (uint32_t chr : string) {
+    for (char16_t chr : string) {
         auto it = sdfs.find(chr);
         if (it != sdfs.end()) {
             shaping.positionedGlyphs.emplace_back(chr, x, y);
@@ -56,7 +57,7 @@ const Shaping GlyphSet::getShaping(const std::u32string &string, const float max
         return shaping;
 
     lineWrap(shaping, lineHeight, maxWidth, horizontalAlign, verticalAlign, justify, translate,
-             util::i18n::allowsIdeographicBreaking(string));
+             util::i18n::allowsIdeographicBreaking(string), writingDirection);
 
     return shaping;
 }
@@ -90,7 +91,9 @@ void justifyLine(std::vector<PositionedGlyph> &positionedGlyphs, const std::map<
 void GlyphSet::lineWrap(Shaping &shaping, const float lineHeight, float maxWidth,
                         const float horizontalAlign, const float verticalAlign,
                         const float justify, const Point<float> &translate,
-                        bool useBalancedIdeographicBreaking) const {
+                        bool useBalancedIdeographicBreaking, const WritingDirection writingDirection) const {
+    float lineFeedOffset = writingDirection == WritingDirection::RightToLeft ? -lineHeight : lineHeight;
+
     uint32_t lastSafeBreak = 0;
 
     uint32_t lengthBeforeCurrentLine = 0;
@@ -112,15 +115,15 @@ void GlyphSet::lineWrap(Shaping &shaping, const float lineHeight, float maxWidth
             PositionedGlyph &shape = positionedGlyphs[i];
 
             shape.x -= lengthBeforeCurrentLine;
-            shape.y += lineHeight * line;
+            shape.y += lineFeedOffset * line;
 
             if (shape.x > maxWidth && lastSafeBreak > 0) {
 
                 uint32_t lineLength = positionedGlyphs[lastSafeBreak + 1].x;
                 maxLineLength = util::max(lineLength, maxLineLength);
 
                 for (uint32_t k = lastSafeBreak + 1; k <= i; k++) {
-                    positionedGlyphs[k].y += lineHeight;
+                    positionedGlyphs[k].y += lineFeedOffset;
                     positionedGlyphs[k].x -= lineLength;
                 }
 

src/mbgl/text/glyph_set.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <mbgl/text/bidi.hpp>
 #include <mbgl/text/glyph.hpp>
 #include <mbgl/util/geometry.hpp>
 
@@ -9,12 +10,12 @@ class GlyphSet {
 public:
     void insert(uint32_t id, SDFGlyph&&);
     const std::map<uint32_t, SDFGlyph> &getSDFs() const;
-    const Shaping getShaping(const std::u32string &string, float maxWidth, float lineHeight,
+    const Shaping getShaping(const std::u16string &string, const WritingDirection writingDirection, float maxWidth, float lineHeight,
                              float horizontalAlign, float verticalAlign, float justify,
                              float spacing, const Point<float> &translate) const;
     void lineWrap(Shaping &shaping, float lineHeight, float maxWidth, float horizontalAlign,
                   float verticalAlign, float justify, const Point<float> &translate,
-                  bool useBalancedIdeographicBreaking) const;
+                  bool useBalancedIdeographicBreaking, const WritingDirection writingDirection) const;
 
 private:
     std::map<uint32_t, SDFGlyph> sdfs;