From d9a3309e8dc02fa9a2148e44ae7d866c558fbd3e Mon Sep 17 00:00:00 2001 From: Gary Qian Date: Wed, 17 Jul 2019 13:55:43 -0700 Subject: [PATCH] Fix justify for RTL paragraphs. (flutter/engine#9859) --- .../third_party/txt/src/txt/paragraph_txt.cc | 13 +++++++++---- .../third_party/txt/src/txt/paragraph_txt.h | 4 +++- .../txt/tests/paragraph_unittests.cc | 18 ++++++++++++++---- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/engine/src/flutter/third_party/txt/src/txt/paragraph_txt.cc b/engine/src/flutter/third_party/txt/src/txt/paragraph_txt.cc index 50fe8bb2109..5e62ea8dab9 100644 --- a/engine/src/flutter/third_party/txt/src/txt/paragraph_txt.cc +++ b/engine/src/flutter/third_party/txt/src/txt/paragraph_txt.cc @@ -1047,14 +1047,14 @@ void ParagraphTxt::Layout(double width) { // advance subtracted, so we do add the advance here to reset the // run_x_offset. We do keep the record though so GetRectsForRange() can // find metrics for trailing spaces. - // if (!run.is_ghost() || run.is_rtl()) { if ((!run.is_ghost() || run.is_rtl()) && !run.is_placeholder_run()) { run_x_offset += layout.getAdvance(); } } // for each in line_runs // Adjust the glyph positions based on the alignment of the line. - double line_x_offset = GetLineXOffset(run_x_offset); + double line_x_offset = + GetLineXOffset(run_x_offset, line_number, line_limit); if (line_x_offset) { for (CodeUnitRun& code_unit_run : line_code_unit_runs) { code_unit_run.Shift(line_x_offset); @@ -1174,13 +1174,18 @@ void ParagraphTxt::Layout(double width) { longest_line_ = max_right_ - min_left_; } -double ParagraphTxt::GetLineXOffset(double line_total_advance) { +double ParagraphTxt::GetLineXOffset(double line_total_advance, + size_t line_number, + size_t line_limit) { if (isinf(width_)) return 0; TextAlign align = paragraph_style_.effective_align(); - if (align == TextAlign::right) { + if (align == TextAlign::right || + (align == TextAlign::justify && + paragraph_style_.text_direction == TextDirection::rtl && + line_number == line_limit - 1)) { return width_ - line_total_advance; } else if (align == TextAlign::center) { return (width_ - line_total_advance) / 2; diff --git a/engine/src/flutter/third_party/txt/src/txt/paragraph_txt.h b/engine/src/flutter/third_party/txt/src/txt/paragraph_txt.h index b049902b55b..d1b68ab6f95 100644 --- a/engine/src/flutter/third_party/txt/src/txt/paragraph_txt.h +++ b/engine/src/flutter/third_party/txt/src/txt/paragraph_txt.h @@ -362,7 +362,9 @@ class ParagraphTxt : public Paragraph { // Calculate the starting X offset of a line based on the line's width and // alignment. - double GetLineXOffset(double line_total_advance); + double GetLineXOffset(double line_total_advance, + size_t line_number, + size_t line_limit); // Creates and draws the decorations onto the canvas. void PaintDecorations(SkCanvas* canvas, 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 69ad4e5f10d..06f636e620d 100644 --- a/engine/src/flutter/third_party/txt/tests/paragraph_unittests.cc +++ b/engine/src/flutter/third_party/txt/tests/paragraph_unittests.cc @@ -1856,15 +1856,25 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(JustifyRTL)) { GetCanvas()->drawRect(boxes[i].rect, paint); } ASSERT_EQ(boxes.size(), 5ull); + + paint.setColor(SK_ColorBLUE); + boxes = paragraph->GetRectsForRange(240, 250, rect_height_style, + rect_width_style); + for (size_t i = 0; i < boxes.size(); ++i) { + GetCanvas()->drawRect(boxes[i].rect, paint); + } + ASSERT_EQ(boxes.size(), 1ull); + EXPECT_FLOAT_EQ(boxes[0].rect.left(), 588); + EXPECT_FLOAT_EQ(boxes[0].rect.top(), 130); + EXPECT_FLOAT_EQ(boxes[0].rect.right(), 640); + EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 156); ASSERT_TRUE(Snapshot()); - // All lines except the last should be justified to the width of the + // All lines should be justified to the width of the // paragraph. - for (size_t i = 0; i < paragraph->glyph_lines_.size() - 1; ++i) { + for (size_t i = 0; i < paragraph->glyph_lines_.size(); ++i) { ASSERT_EQ(glyph_line_width(i), paragraph_width); } - ASSERT_NE(glyph_line_width(paragraph->glyph_lines_.size() - 1), - paragraph_width); } TEST_F(ParagraphTest, DecorationsParagraph) {