Bidi segmentation BEFORE anything else

Rusino · Skia Commit-Bot · commit c88a3bc3f561 · 2020-01-23T20:38:26.000Z
Change-Id: I94637e663bc1ffc7d9d6e1c0fb0b28509af45f60 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/266200 Commit-Queue: Julia Lavrova <jlavrova@google.com> Reviewed-by: Ben Wagner <bungeman@google.com>
diff --git a/modules/skparagraph/src/OneLineShaper.cpp b/modules/skparagraph/src/OneLineShaper.cpp
@@ -335,7 +335,8 @@ void OneLineShaper::sortOutGlyphs(std::function<void(GlyphRange)>&& sortOutUnres
     }
 }
 
-void OneLineShaper::iterateThroughFontStyles(SkSpan<Block> styleSpan,
+void OneLineShaper::iterateThroughFontStyles(TextRange textRange,
+                                             SkSpan<Block> styleSpan,
                                              const ShapeSingleFontVisitor& visitor) {
     Block combinedBlock;
     SkTArray<SkShaper::Feature> features;
@@ -357,20 +358,23 @@ void OneLineShaper::iterateThroughFontStyles(SkSpan<Block> styleSpan,
     };
 
     for (auto& block : styleSpan) {
-        SkASSERT(combinedBlock.fRange.width() == 0 ||
-                 combinedBlock.fRange.end == block.fRange.start);
+        BlockRange blockRange(SkTMax(block.fRange.start, textRange.start), SkTMin(block.fRange.end, textRange.end));
+        if (blockRange.empty()) {
+            continue;
+        }
+        SkASSERT(combinedBlock.fRange.width() == 0 || combinedBlock.fRange.end == block.fRange.start);
 
         if (!combinedBlock.fRange.empty()) {
             if (block.fStyle.matchOneAttribute(StyleType::kFont, combinedBlock.fStyle)) {
-                combinedBlock.add(block.fRange);
+                combinedBlock.add(blockRange);
                 addFeatures(block);
                 continue;
             }
             // Resolve all characters in the block for this style
             visitor(combinedBlock, features);
         }
 
-        combinedBlock.fRange = block.fRange;
+        combinedBlock.fRange = blockRange;
         combinedBlock.fStyle = block.fStyle;
         features.reset();
         addFeatures(block);
@@ -412,17 +416,38 @@ void OneLineShaper::matchResolvedFonts(const TextStyle& textStyle,
 
 bool OneLineShaper::iterateThroughShapingRegions(const ShapeVisitor& shape) {
 
+    SkTArray<BidiRegion> bidiRegions;
+    if (!fParagraph->calculateBidiRegions(&bidiRegions)) {
+        return false;
+    }
+
+    size_t bidiIndex = 0;
+
     SkScalar advanceX = 0;
     for (auto& placeholder : fParagraph->fPlaceholders) {
-        // Shape the text
+
         if (placeholder.fTextBefore.width() > 0) {
-            // Set up the iterators
-            SkSpan<const char> textSpan = fParagraph->text(placeholder.fTextBefore);
-            SkSpan<Block> styleSpan(fParagraph->fTextStyles.begin() + placeholder.fBlocksBefore.start,
-                                    placeholder.fBlocksBefore.width());
+            // Shape the text by bidi regions
+            while (bidiIndex < bidiRegions.size()) {
+                BidiRegion& bidiRegion = bidiRegions[bidiIndex];
+                auto start = SkTMax(bidiRegion.text.start, placeholder.fTextBefore.start);
+                auto end = SkTMin(bidiRegion.text.end, placeholder.fTextBefore.end);
+
+                // Set up the iterators (the style iterator points to a bigger region that it could
+                TextRange textRange(start, end);
+                auto blockRange = fParagraph->findAllBlocks(textRange);
+                SkSpan<Block> styleSpan(fParagraph->blocks(blockRange));
+
+                // Shape the text between placeholders
+                if (!shape(textRange, styleSpan, advanceX, start, bidiRegion.direction)) {
+                    return false;
+                }
 
-            if (!shape(textSpan, styleSpan, advanceX, placeholder.fTextBefore.start)) {
-                return false;
+                if (end == bidiRegion.text.end) {
+                    ++bidiIndex;
+                } else /*if (end == placeholder.fTextBefore.end)*/ {
+                    break;
+                }
             }
         }
 
@@ -465,12 +490,11 @@ bool OneLineShaper::shape() {
 
     // The text can be broken into many shaping sequences
     // (by place holders, possibly, by hard line breaks or tabs, too)
-    uint8_t textDirection = fParagraph->fParagraphStyle.getTextDirection() == TextDirection::kLtr  ? 2 : 1;
     auto limitlessWidth = std::numeric_limits<SkScalar>::max();
 
     auto result = iterateThroughShapingRegions(
-            [this, textDirection, limitlessWidth]
-            (SkSpan<const char> textSpan, SkSpan<Block> styleSpan, SkScalar& advanceX, TextIndex textStart) {
+            [this, limitlessWidth]
+            (TextRange textRange, SkSpan<Block> styleSpan, SkScalar& advanceX, TextIndex textStart, uint8_t textDirection) {
 
         // Set up the shaper and shape the next
         auto shaper = SkShaper::MakeShapeDontWrapOrReorder();
@@ -479,7 +503,7 @@ bool OneLineShaper::shape() {
             return false;
         }
 
-        iterateThroughFontStyles(styleSpan,
+        iterateThroughFontStyles(textRange, styleSpan,
                 [this, &shaper, textDirection, limitlessWidth, &advanceX]
                 (Block block, SkTArray<SkShaper::Feature> features) {
             auto blockSpan = SkSpan<Block>(&block, 1);
diff --git a/modules/skparagraph/src/OneLineShaper.h b/modules/skparagraph/src/OneLineShaper.h
@@ -54,11 +54,11 @@ class OneLineShaper : public SkShaper::RunHandler {
     };
 
     using ShapeVisitor =
-            std::function<SkScalar(SkSpan<const char>, SkSpan<Block>, SkScalar&, TextIndex)>;
+            std::function<SkScalar(TextRange textRange, SkSpan<Block>, SkScalar&, TextIndex, uint8_t)>;
     bool iterateThroughShapingRegions(const ShapeVisitor& shape);
 
     using ShapeSingleFontVisitor = std::function<void(Block, SkTArray<SkShaper::Feature>)>;
-    void iterateThroughFontStyles(SkSpan<Block> styleSpan, const ShapeSingleFontVisitor& visitor);
+    void iterateThroughFontStyles(TextRange textRange, SkSpan<Block> styleSpan, const ShapeSingleFontVisitor& visitor);
 
     using TypefaceVisitor = std::function<bool(sk_sp<SkTypeface> typeface)>;
     void matchResolvedFonts(const TextStyle& textStyle, const TypefaceVisitor& visitor);
diff --git a/modules/skparagraph/src/ParagraphImpl.cpp b/modules/skparagraph/src/ParagraphImpl.cpp
@@ -19,12 +19,19 @@ namespace textlayout {
 namespace {
 
 using ICUUText = std::unique_ptr<UText, SkFunctionWrapper<decltype(utext_close), utext_close>>;
+using ICUBiDi  = std::unique_ptr<UBiDi, SkFunctionWrapper<decltype(ubidi_close), ubidi_close>>;
 
 SkScalar littleRound(SkScalar a) {
     // This rounding is done to match Flutter tests. Must be removed..
   return SkScalarRoundToScalar(a * 100.0)/100.0;
 }
 
+/** Replaces invalid utf-8 sequences with REPLACEMENT CHARACTER U+FFFD. */
+static inline SkUnichar utf8_next(const char** ptr, const char* end) {
+    SkUnichar val = SkUTF::NextUTF8(ptr, end);
+    return val < 0 ? 0xFFFD : val;
+}
+
 }
 
 TextRange operator*(const TextRange& a, const TextRange& b) {
@@ -674,11 +681,14 @@ std::vector<TextBox> ParagraphImpl::getRectsForRange(unsigned start,
                     }
                 }
 
+                auto runInLineWidth = line.measureTextInsideOneRun(textRange, run, runOffset, 0, true, false).clip.width();
                 runOffset += *width;
 
                 // Found a run that intersects with the text
                 auto context = line.measureTextInsideOneRun(intersect, run, runOffset, 0, true, true);
-                *width += context.clip.width();
+
+                //*width += context.clip.width();
+                *width = runInLineWidth;
 
                 SkRect clip = context.clip;
                 SkRect trailingSpaces = SkRect::MakeEmpty();
@@ -1132,5 +1142,78 @@ void ParagraphImpl::updateBackgroundPaint(size_t from, size_t to, SkPaint paint)
     }
 }
 
+bool ParagraphImpl::calculateBidiRegions(SkTArray<BidiRegion>* regions) {
+
+    regions->reset();
+
+    // ubidi only accepts utf16 (though internally it basically works on utf32 chars).
+    // We want an ubidi_setPara(UBiDi*, UText*, UBiDiLevel, UBiDiLevel*, UErrorCode*);
+    size_t utf8Bytes = fText.size();
+    const char* utf8 = fText.c_str();
+    uint8_t bidiLevel = fParagraphStyle.getTextDirection() == TextDirection::kLtr
+                            ? UBIDI_LTR
+                            : UBIDI_RTL;
+    if (!SkTFitsIn<int32_t>(utf8Bytes)) {
+        SkDEBUGF("Bidi error: text too long");
+        return false;
+    }
+
+    // Getting the length like this seems to always set U_BUFFER_OVERFLOW_ERROR
+    UErrorCode status = U_ZERO_ERROR;
+    int32_t utf16Units;
+    u_strFromUTF8(nullptr, 0, &utf16Units, utf8, utf8Bytes, &status);
+    status = U_ZERO_ERROR;
+    std::unique_ptr<UChar[]> utf16(new UChar[utf16Units]);
+    u_strFromUTF8(utf16.get(), utf16Units, nullptr, utf8, utf8Bytes, &status);
+    if (U_FAILURE(status)) {
+        SkDEBUGF("Invalid utf8 input: %s", u_errorName(status));
+        return false;
+    }
+
+    ICUBiDi bidi(ubidi_openSized(utf16Units, 0, &status));
+    if (U_FAILURE(status)) {
+        SkDEBUGF("Bidi error: %s", u_errorName(status));
+        return false;
+    }
+    SkASSERT(bidi);
+
+    // The required lifetime of utf16 isn't well documented.
+    // It appears it isn't used after ubidi_setPara except through ubidi_getText.
+    ubidi_setPara(bidi.get(), utf16.get(), utf16Units, bidiLevel, nullptr, &status);
+    if (U_FAILURE(status)) {
+        SkDEBUGF("Bidi error: %s", u_errorName(status));
+        return false;
+    }
+
+    SkTArray<BidiRegion> bidiRegions;
+    const char* start8 = utf8;
+    const char* end8 = utf8 + utf8Bytes;
+    TextRange textRange(0, 0);
+    UBiDiLevel currentLevel = 0;
+
+    int32_t pos16 = 0;
+    int32_t end16 = ubidi_getLength(bidi.get());
+    while (pos16 < end16) {
+        auto level = ubidi_getLevelAt(bidi.get(), pos16);
+        if (pos16 == 0) {
+            currentLevel = level;
+        } else if (level != currentLevel) {
+            textRange.end = start8 - utf8;
+            regions->emplace_back(textRange.start, textRange.end, currentLevel);
+            currentLevel = level;
+            textRange = TextRange(textRange.end, textRange.end);
+        }
+        SkUnichar u = utf8_next(&start8, end8);
+        pos16 += SkUTF::ToUTF16(u);
+    }
+
+    textRange.end = start8 - utf8;
+    if (!textRange.empty()) {
+        regions->emplace_back(textRange.start, textRange.end, currentLevel);
+    }
+
+    return true;
+}
+
 }  // namespace textlayout
 }  // namespace skia
diff --git a/modules/skparagraph/src/ParagraphImpl.h b/modules/skparagraph/src/ParagraphImpl.h
@@ -49,6 +49,13 @@ struct ResolvedFontDescriptor {
     TextIndex fTextStart;
 };
 
+struct BidiRegion {
+    BidiRegion(size_t start, size_t end, uint8_t dir)
+        : text(start, end), direction(dir) { }
+    TextRange text;
+    uint8_t direction;
+};
+
 class TextBreaker {
 public:
     TextBreaker() : fInitialized(false), fPos(-1) {}
@@ -218,6 +225,8 @@ class ParagraphImpl final : public Paragraph {
 
     void computeEmptyMetrics();
 
+    bool calculateBidiRegions(SkTArray<BidiRegion>* regions);
+
     // Input
     SkTArray<StyleBlock<SkScalar>> fLetterSpaceStyles;
     SkTArray<StyleBlock<SkScalar>> fWordSpaceStyles;
diff --git a/samplecode/SampleParagraph.cpp b/samplecode/SampleParagraph.cpp
@@ -1927,6 +1927,42 @@ class ParagraphView26 : public ParagraphView_Base {
     typedef Sample INHERITED;
 };
 
+class ParagraphView27 : public ParagraphView_Base {
+protected:
+    SkString name() override { return SkString("Paragraph27"); }
+
+    void onDrawContent(SkCanvas* canvas) override {
+        canvas->drawColor(SK_ColorWHITE);
+
+        ParagraphStyle paragraph_style;
+        paragraph_style.setTextDirection(TextDirection::kRtl);
+        TextStyle text_style;
+        text_style.setColor(SK_ColorBLACK);
+        text_style.setFontFamilies({SkString("Google Sans")});
+        text_style.setFontSize(20);
+        {
+            ParagraphBuilderImpl builder(paragraph_style, getFontCollection());
+            builder.pushStyle(text_style);
+            builder.addText("31 December 2021");
+            auto paragraph = builder.Build();
+            paragraph->layout(600);
+            paragraph->paint(canvas, 0, 0);
+        }
+        canvas->translate(0, 200);
+        {
+            ParagraphBuilderImpl builder(paragraph_style, getFontCollection());
+            builder.pushStyle(text_style);
+            builder.addText("December 31 2021");
+            auto paragraph = builder.Build();
+            paragraph->layout(600);
+            paragraph->paint(canvas, 0, 0);
+        }
+    }
+
+private:
+    typedef Sample INHERITED;
+};
+
 //////////////////////////////////////////////////////////////////////////////
 
 DEF_SAMPLE(return new ParagraphView1();)
@@ -1954,3 +1990,4 @@ DEF_SAMPLE(return new ParagraphView23();)
 DEF_SAMPLE(return new ParagraphView24();)
 DEF_SAMPLE(return new ParagraphView25();)
 DEF_SAMPLE(return new ParagraphView26();)
+DEF_SAMPLE(return new ParagraphView27();)
diff --git a/tests/SkParagraphTest.cpp b/tests/SkParagraphTest.cpp
