From 0f67acda395dabf66a0fa230bcddb81d15deaa89 Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Wed, 11 Oct 2017 16:15:57 -0700 Subject: [PATCH] libtxt: support right-to-left text (flutter/engine#4198) --- .../flutter/lib/ui/text/paragraph_builder.cc | 26 +++++++--------- .../flutter/lib/ui/text/paragraph_impl_txt.cc | 6 ++-- engine/src/flutter/third_party/txt/BUILD.gn | 1 - .../txt/src/txt/font_collection.cc | 2 ++ .../third_party/txt/src/txt/paragraph.cc | 26 ++++++++++------ .../third_party/txt/src/txt/paragraph_style.h | 21 ++++++++++--- .../third_party/txt/src/txt/text_align.h | 31 ------------------- .../txt/tests/paragraph_unittests.cc | 19 ++++++------ 8 files changed, 58 insertions(+), 74 deletions(-) delete mode 100644 engine/src/flutter/third_party/txt/src/txt/text_align.h diff --git a/engine/src/flutter/lib/ui/text/paragraph_builder.cc b/engine/src/flutter/lib/ui/text/paragraph_builder.cc index a86fcc93159..9b5b3b37fa2 100644 --- a/engine/src/flutter/lib/ui/text/paragraph_builder.cc +++ b/engine/src/flutter/lib/ui/text/paragraph_builder.cc @@ -16,7 +16,6 @@ #include "flutter/third_party/txt/src/txt/font_style.h" #include "flutter/third_party/txt/src/txt/font_weight.h" #include "flutter/third_party/txt/src/txt/paragraph_style.h" -#include "flutter/third_party/txt/src/txt/text_align.h" #include "flutter/third_party/txt/src/txt/text_decoration.h" #include "flutter/third_party/txt/src/txt/text_style.h" #include "lib/fxl/tasks/task_runner.h" @@ -217,22 +216,21 @@ ParagraphBuilder::ParagraphBuilder(tonic::Int32List& encoded, if (mask & psTextAlignMask) style.text_align = txt::TextAlign(encoded[psTextAlignIndex]); - if (mask & (psFontWeightMask | psFontStyleMask | psFontFamilyMask | - psFontSizeMask)) { - if (mask & psFontWeightMask) - style.font_weight = - static_cast(encoded[psFontWeightIndex]); + if (mask & psTextDirectionMask) + style.text_direction = txt::TextDirection(encoded[psTextDirectionIndex]); - if (mask & psFontStyleMask) - style.font_style = - static_cast(encoded[psFontStyleIndex]); + if (mask & psFontWeightMask) + style.font_weight = + static_cast(encoded[psFontWeightIndex]); - if (mask & psFontFamilyMask) - style.font_family = fontFamily; + if (mask & psFontStyleMask) + style.font_style = static_cast(encoded[psFontStyleIndex]); - if (mask & psFontSizeMask) - style.font_size = fontSize; - } + if (mask & psFontFamilyMask) + style.font_family = fontFamily; + + if (mask & psFontSizeMask) + style.font_size = fontSize; if (mask & psLineHeightMask) style.line_height = lineHeight; diff --git a/engine/src/flutter/lib/ui/text/paragraph_impl_txt.cc b/engine/src/flutter/lib/ui/text/paragraph_impl_txt.cc index f51921c85fd..df61016be57 100644 --- a/engine/src/flutter/lib/ui/text/paragraph_impl_txt.cc +++ b/engine/src/flutter/lib/ui/text/paragraph_impl_txt.cc @@ -66,9 +66,9 @@ std::vector ParagraphImplTxt::getRectsForRange(unsigned start, std::vector result; std::vector rects = m_paragraph->GetRectsForRange(start, end); for (size_t i = 0; i < rects.size(); ++i) { - result.push_back(TextBox(rects[i], m_paragraph->GetParagraphStyle().rtl - ? TextDirection::RTL - : TextDirection::LTR)); + result.push_back(TextBox( + rects[i], static_cast( + m_paragraph->GetParagraphStyle().text_direction))); } return result; } diff --git a/engine/src/flutter/third_party/txt/BUILD.gn b/engine/src/flutter/third_party/txt/BUILD.gn index 8221a8328d8..e2dcd08a190 100644 --- a/engine/src/flutter/third_party/txt/BUILD.gn +++ b/engine/src/flutter/third_party/txt/BUILD.gn @@ -89,7 +89,6 @@ source_set("txt") { "src/txt/platform.h", "src/txt/styled_runs.cc", "src/txt/styled_runs.h", - "src/txt/text_align.h", "src/txt/text_baseline.h", "src/txt/text_decoration.cc", "src/txt/text_decoration.h", diff --git a/engine/src/flutter/third_party/txt/src/txt/font_collection.cc b/engine/src/flutter/third_party/txt/src/txt/font_collection.cc index 461468cd801..b8ba00baa49 100644 --- a/engine/src/flutter/third_party/txt/src/txt/font_collection.cc +++ b/engine/src/flutter/third_party/txt/src/txt/font_collection.cc @@ -37,6 +37,8 @@ namespace { const std::vector fallback_characters{ 0x1f600, // emoji 0x4e00, // CJK + 0x5d0, // Hebrew + 0x627, // Arabic }; } // anonymous namespace diff --git a/engine/src/flutter/third_party/txt/src/txt/paragraph.cc b/engine/src/flutter/third_party/txt/src/txt/paragraph.cc index 1b356b51e55..3601bd7e550 100644 --- a/engine/src/flutter/third_party/txt/src/txt/paragraph.cc +++ b/engine/src/flutter/third_party/txt/src/txt/paragraph.cc @@ -307,7 +307,9 @@ void Paragraph::Layout(double width, bool force) { size_t line_run_end = std::min(run.end, line_end); uint16_t* text_ptr = text_.data() + line_run_start; size_t text_count = line_run_end - line_run_start; - bool bidiFlags = paragraph_style_.rtl; + int bidiFlags = (paragraph_style_.text_direction == TextDirection::rtl) + ? minikin::kBidi_RTL + : minikin::kBidi_LTR; if (text_count == 0) continue; @@ -366,7 +368,7 @@ void Paragraph::Layout(double width, bool force) { blob_start += blob_len; } - double current_x_position = 0; + double justify_x_offset = 0; size_t code_unit_index = 0; for (const Range& glyph_blob : glyph_blobs) { @@ -380,7 +382,8 @@ void Paragraph::Layout(double width, bool force) { blob_buffer.glyphs[blob_index] = layout.getGlyphId(glyph_index); size_t pos_index = blob_index * 2; - blob_buffer.pos[pos_index] = current_x_position; + double glyph_x_offset = layout.getX(glyph_index) + justify_x_offset; + blob_buffer.pos[pos_index] = glyph_x_offset; blob_buffer.pos[pos_index + 1] = layout.getY(glyph_index); float glyph_advance = layout.getCharAdvance(code_unit_index); @@ -403,7 +406,7 @@ void Paragraph::Layout(double width, bool force) { float subglyph_advance = glyph_advance / subglyph_code_unit_counts.size(); glyph_single_line_position_x.emplace_back( - run_x_offset + current_x_position, subglyph_advance, + run_x_offset + glyph_x_offset, subglyph_advance, subglyph_code_unit_counts[0]); // Compute positions for the additional characters in the ligature. @@ -413,11 +416,9 @@ void Paragraph::Layout(double width, bool force) { subglyph_advance, subglyph_code_unit_counts[i]); } - current_x_position += glyph_advance; - if (justify_line && word_index < words.size() && code_unit_index == words[word_index].end) { - current_x_position += word_gap_width; + justify_x_offset += word_gap_width; word_index++; } } @@ -427,8 +428,8 @@ void Paragraph::Layout(double width, bool force) { paint.getFontMetrics(&metrics); paint_records.emplace_back(run.style, SkPoint::Make(run_x_offset, 0), builder.make(), metrics, line_number, - current_x_position); - run_x_offset += current_x_position; + layout.getAdvance()); + run_x_offset += layout.getAdvance(); } double max_line_spacing = 0; @@ -477,7 +478,12 @@ double Paragraph::GetLineXOffset(size_t line) { if (line >= breaks_count_) return 0; - if (paragraph_style_.text_align == TextAlign::right) { + TextAlign align = paragraph_style_.text_align; + TextDirection direction = paragraph_style_.text_direction; + + if (align == TextAlign::right || + (align == TextAlign::start && direction == TextDirection::rtl) || + (align == TextAlign::end && direction == TextDirection::ltr)) { return width_ - breaker_.getWidths()[line]; } else if (paragraph_style_.text_align == TextAlign::center) { return (width_ - breaker_.getWidths()[line]) / 2; diff --git a/engine/src/flutter/third_party/txt/src/txt/paragraph_style.h b/engine/src/flutter/third_party/txt/src/txt/paragraph_style.h index 1eebb9e73b0..e63ba761163 100644 --- a/engine/src/flutter/third_party/txt/src/txt/paragraph_style.h +++ b/engine/src/flutter/third_party/txt/src/txt/paragraph_style.h @@ -23,10 +23,23 @@ #include "font_style.h" #include "font_weight.h" #include "minikin/LineBreaker.h" -#include "text_align.h" namespace txt { +enum class TextAlign { + left, + right, + center, + justify, + start, + end, +}; + +enum class TextDirection { + rtl, + ltr, +}; + class ParagraphStyle { public: FontWeight font_weight = FontWeight::w400; @@ -34,7 +47,8 @@ class ParagraphStyle { std::string font_family = ""; double font_size = 14; - TextAlign text_align = TextAlign::left; + TextAlign text_align = TextAlign::start; + TextDirection text_direction = TextDirection::ltr; size_t max_lines = std::numeric_limits::max(); double line_height = 1.0; std::u16string ellipsis; @@ -44,9 +58,6 @@ class ParagraphStyle { // kBreakStrategy_Balanced will balance between the two. minikin::BreakStrategy break_strategy = minikin::BreakStrategy::kBreakStrategy_Greedy; - // TODO(garyq): Implement right to left. - // Right to left (Arabic, Hebrew, etc). - bool rtl = false; }; } // namespace txt diff --git a/engine/src/flutter/third_party/txt/src/txt/text_align.h b/engine/src/flutter/third_party/txt/src/txt/text_align.h deleted file mode 100644 index 52922a2f72f..00000000000 --- a/engine/src/flutter/third_party/txt/src/txt/text_align.h +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright 2017 Google Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef LIB_TXT_SRC_TEXT_ALIGN_H_ -#define LIB_TXT_SRC_TEXT_ALIGN_H_ - -namespace txt { - -enum class TextAlign { - left, - right, - center, - justify, -}; - -} // namespace txt - -#endif // LIB_TXT_SRC_TEXT_ALIGN_H_ diff --git a/engine/src/flutter/third_party/txt/tests/paragraph_unittests.cc b/engine/src/flutter/third_party/txt/tests/paragraph_unittests.cc index 4e5e1c725d0..136646d5ec4 100644 --- a/engine/src/flutter/third_party/txt/tests/paragraph_unittests.cc +++ b/engine/src/flutter/third_party/txt/tests/paragraph_unittests.cc @@ -22,7 +22,6 @@ #include "txt/font_weight.h" #include "txt/paragraph.h" #include "txt/paragraph_builder.h" -#include "txt/text_align.h" #include "utils.h" namespace txt { @@ -809,7 +808,7 @@ TEST_F(ParagraphTest, DISABLED_ArabicParagraph) { txt::ParagraphStyle paragraph_style; paragraph_style.max_lines = 14; paragraph_style.text_align = TextAlign::right; - paragraph_style.rtl = true; + paragraph_style.text_direction = TextDirection::rtl; txt::ParagraphBuilder builder(paragraph_style, GetTestFontCollection()); txt::TextStyle text_style; @@ -838,7 +837,7 @@ TEST_F(ParagraphTest, DISABLED_ArabicParagraph) { ASSERT_TRUE(paragraph->runs_.styles_[1].equals(text_style)); ASSERT_EQ(paragraph->records_[0].style().color, text_style.color); ASSERT_EQ(paragraph->records_.size(), 2ull); - ASSERT_EQ(paragraph->paragraph_style_.rtl, true); + ASSERT_EQ(paragraph->paragraph_style_.text_direction, TextDirection::rtl); for (size_t i = 0; i < u16_text.length(); i++) { ASSERT_EQ(paragraph->text_[i], u16_text[u16_text.length() - i]); @@ -978,7 +977,7 @@ TEST_F(ParagraphTest, GetRectsForRangeParagraph) { EXPECT_EQ(rects.size(), 1ull); EXPECT_FLOAT_EQ(rects[0].left(), 56.835938); EXPECT_FLOAT_EQ(rects[0].top(), 0); - EXPECT_FLOAT_EQ(rects[0].right(), 177.97266); + EXPECT_FLOAT_EQ(rects[0].right(), 177.44922); EXPECT_FLOAT_EQ(rects[0].bottom(), 59); paint.setColor(SK_ColorGREEN); @@ -987,9 +986,9 @@ TEST_F(ParagraphTest, GetRectsForRangeParagraph) { GetCanvas()->drawRect(rects[i], paint); } EXPECT_EQ(rects.size(), 1ull); - EXPECT_FLOAT_EQ(rects[0].left(), 177.97266); + EXPECT_FLOAT_EQ(rects[0].left(), 177); EXPECT_FLOAT_EQ(rects[0].top(), 0); - EXPECT_FLOAT_EQ(rects[0].right(), 507.02344); + EXPECT_FLOAT_EQ(rects[0].right(), 506.08984); EXPECT_FLOAT_EQ(rects[0].bottom(), 59); paint.setColor(SK_ColorRED); @@ -998,9 +997,9 @@ TEST_F(ParagraphTest, GetRectsForRangeParagraph) { GetCanvas()->drawRect(rects[i], paint); } EXPECT_EQ(rects.size(), 4ull); - EXPECT_FLOAT_EQ(rects[0].left(), 211.375); + EXPECT_FLOAT_EQ(rects[0].left(), 210.83594); EXPECT_FLOAT_EQ(rects[0].top(), 59); - EXPECT_FLOAT_EQ(rects[0].right(), 463.61719); + EXPECT_FLOAT_EQ(rects[0].right(), 463.44922); EXPECT_FLOAT_EQ(rects[0].bottom(), 118); // TODO(garyq): The following set of vals are definetly wrong and @@ -1016,9 +1015,9 @@ TEST_F(ParagraphTest, GetRectsForRangeParagraph) { GetCanvas()->drawRect(rects[i], paint); } EXPECT_EQ(rects.size(), 1ull); - EXPECT_FLOAT_EQ(rects[0].left(), 450.1875); + EXPECT_FLOAT_EQ(rects[0].left(), 449.25391); EXPECT_FLOAT_EQ(rects[0].top(), 0); - EXPECT_FLOAT_EQ(rects[0].right(), 519.47266); + EXPECT_FLOAT_EQ(rects[0].right(), 519.44922); EXPECT_FLOAT_EQ(rects[0].bottom(), 59); paint.setColor(SK_ColorRED);