Skip to content

Commit

Permalink
Revert "RTL handling for ghost runs, NotoNaskhArabic test font (flutt…
Browse files Browse the repository at this point in the history 
…er#8638)" (flutter#8681)

This reverts commit 6e79dcd.

Reverts flutter#8638

Reason: flutter#8638 breaks the post-submit Cirrus tests. See https://cirrus-ci.com/build/5143341531398144 and subsequent post-submit failures. Specifically, ParagraphTest.RightAlignParagraph is failing.

TBR: @GaryQian
  • Loading branch information
@liyuqian
liyuqian authored Apr 22, 2019
1 parent 8b5f776 commit b4ed303
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 228 deletions.
68 changes: 21 additions & 47 deletions third_party/txt/src/txt/paragraph.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,14 @@
#include "paragraph.h"

#include <hb.h>
#include <minikin/Layout.h>

#include <algorithm>
#include <limits>
#include <map>
#include <numeric>
#include <utility>
#include <vector>

#include <minikin/Layout.h>
#include "flutter/fml/logging.h"
#include "font_collection.h"
#include "font_skia.h"
Expand All @@ -35,6 +34,9 @@
#include "minikin/LayoutUtils.h"
#include "minikin/LineBreaker.h"
#include "minikin/MinikinFont.h"
#include "unicode/ubidi.h"
#include "unicode/utf16.h"

#include "third_party/skia/include/core/SkCanvas.h"
#include "third_party/skia/include/core/SkFont.h"
#include "third_party/skia/include/core/SkFontMetrics.h"
Expand All @@ -44,8 +46,6 @@
#include "third_party/skia/include/core/SkTypeface.h"
#include "third_party/skia/include/effects/SkDashPathEffect.h"
#include "third_party/skia/include/effects/SkDiscretePathEffect.h"
#include "unicode/ubidi.h"
#include "unicode/utf16.h"

namespace txt {
namespace {
Expand Down Expand Up @@ -552,39 +552,28 @@ void Paragraph::Layout(double width, bool force) {
// Find the runs comprising this line.
std::vector<BidiRun> line_runs;
for (const BidiRun& bidi_run : bidi_runs) {
if (bidi_run.start() < line_end_index &&
bidi_run.end() > line_range.start) {
line_runs.emplace_back(std::max(bidi_run.start(), line_range.start),
std::min(bidi_run.end(), line_end_index),
bidi_run.direction(), bidi_run.style());
}
// A "ghost" run is a run that does not impact the layout, breaking,
// alignment, width, etc but is still "visible" through getRectsForRange.
// alignment, width, etc but is still "visible" though getRectsForRange.
// For example, trailing whitespace on centered text can be scrolled
// through with the caret but will not wrap the line.
//
// Here, we add an additional run for the whitespace, but dont
// let it impact metrics. After layout of the whitespace run, we do not
// add its width into the x-offset adjustment, effectively nullifying its
// impact on the layout.
std::unique_ptr<BidiRun> ghost_run = nullptr;
if (paragraph_style_.ellipsis.empty() &&
line_range.end_excluding_whitespace < line_range.end &&
bidi_run.start() <= line_range.end &&
bidi_run.end() > line_end_index) {
ghost_run = std::make_unique<BidiRun>(
std::max(bidi_run.start(), line_end_index),
std::min(bidi_run.end(), line_range.end), bidi_run.direction(),
bidi_run.style(), true);
}
// Include the ghost run before normal run if RTL
if (bidi_run.direction() == TextDirection::rtl && ghost_run != nullptr) {
line_runs.push_back(*ghost_run);
}
// Emplace a normal line run.
if (bidi_run.start() < line_end_index &&
bidi_run.end() > line_range.start) {
line_runs.emplace_back(std::max(bidi_run.start(), line_range.start),
std::min(bidi_run.end(), line_end_index),
bidi_run.direction(), bidi_run.style());
}
// Include the ghost run after normal run if LTR
if (bidi_run.direction() == TextDirection::ltr && ghost_run != nullptr) {
line_runs.push_back(*ghost_run);
line_runs.emplace_back(std::max(bidi_run.start(), line_end_index),
std::min(bidi_run.end(), line_range.end),
bidi_run.direction(), bidi_run.style(), true);
}
}
bool line_runs_all_rtl =
Expand Down Expand Up @@ -672,17 +661,6 @@ void Paragraph::Layout(double width, bool force) {
if (layout.nGlyphs() == 0)
continue;

// When laying out RTL ghost runs, shift the run_x_offset here by the
// advance so that the ghost run is positioned to the left of the first
// real run of text in the line. However, since we do not want it to
// impact the layout of real text, this advance is subsequently added
// back into the run_x_offset after the ghost run positions have been
// calcuated and before the next real run of text is laid out, ensuring
// later runs are laid out in the same position as if there were no ghost
// run.
if (run.is_ghost() && run.is_rtl())
run_x_offset -= layout.getAdvance();

std::vector<float> layout_advances(text_count);
layout.getAdvances(layout_advances.data());

Expand Down Expand Up @@ -830,25 +808,21 @@ void Paragraph::Layout(double width, bool force) {
[](const GlyphPosition& a, const GlyphPosition& b) {
return a.code_units.start < b.code_units.start;
});

line_code_unit_runs.emplace_back(
std::move(code_unit_positions),
Range<size_t>(run.start(), run.end()),
Range<double>(glyph_positions.front().x_pos.start,
glyph_positions.back().x_pos.end),
line_number, metrics, run.direction());

if (!run.is_ghost()) {
min_left_ = std::min(min_left_, glyph_positions.front().x_pos.start);
max_right_ = std::max(max_right_, glyph_positions.back().x_pos.end);
}
min_left_ = std::min(min_left_, glyph_positions.front().x_pos.start);
max_right_ = std::max(max_right_, glyph_positions.back().x_pos.end);
} // for each in glyph_blobs
// Do not increase x offset for LTR trailing ghost runs as it should not
// impact the layout of visible glyphs. RTL tailing ghost runs have the
// 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()) {

// Do not increase x offset for trailing ghost runs as it should not
// impact the layout of visible glyphs. We do keep the record though so
// GetRectsForRange() can find metrics for trailing spaces.
if (!run.is_ghost()) {
run_x_offset += layout.getAdvance();
}
} // for each in line_runs
Expand Down
92 changes: 3 additions & 89 deletions third_party/txt/tests/paragraph_unittests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,6 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(JustifyRTL)) {
txt::ParagraphStyle paragraph_style;
paragraph_style.max_lines = 14;
paragraph_style.text_align = TextAlign::justify;
paragraph_style.text_direction = TextDirection::rtl;
txt::ParagraphBuilder builder(paragraph_style, GetTestFontCollection());

txt::TextStyle text_style;
Expand All @@ -726,33 +725,16 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(JustifyRTL)) {

paragraph->Paint(GetCanvas(), 0, 0);

ASSERT_TRUE(Snapshot());

auto glyph_line_width = [&paragraph](int index) {
size_t second_to_last_position_index =
paragraph->glyph_lines_[index].positions.size() - 1;
paragraph->glyph_lines_[index].positions.size() - 2;
return paragraph->glyph_lines_[index]
.positions[second_to_last_position_index]
.x_pos.end;
};

SkPaint paint;
paint.setStyle(SkPaint::kStroke_Style);
paint.setAntiAlias(true);
paint.setStrokeWidth(1);

// Tests for GetRectsForRange()
Paragraph::RectHeightStyle rect_height_style =
Paragraph::RectHeightStyle::kMax;
Paragraph::RectWidthStyle rect_width_style =
Paragraph::RectWidthStyle::kTight;
paint.setColor(SK_ColorRED);
std::vector<txt::Paragraph::TextBox> boxes =
paragraph->GetRectsForRange(0, 100, 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(), 5ull);
ASSERT_TRUE(Snapshot());

// All lines except the last should be justified to the width of the
// paragraph.
for (size_t i = 0; i < paragraph->glyph_lines_.size() - 1; ++i) {
Expand Down Expand Up @@ -989,74 +971,6 @@ TEST_F(ParagraphTest, DISABLED_ArabicParagraph) {
ASSERT_TRUE(Snapshot());
}

// Checks if the rects are in the correct positions after typing spaces in
// Arabic.
TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(ArabicRectsParagraph)) {
const char* text = "بمباركة التقليدية قام عن. تصفح يد ";
auto icu_text = icu::UnicodeString::fromUTF8(text);
std::u16string u16_text(icu_text.getBuffer(),
icu_text.getBuffer() + icu_text.length());

txt::ParagraphStyle paragraph_style;
paragraph_style.max_lines = 14;
paragraph_style.text_align = TextAlign::right;
paragraph_style.text_direction = TextDirection::rtl;
txt::ParagraphBuilder builder(paragraph_style, GetTestFontCollection());

txt::TextStyle text_style;
text_style.font_families = std::vector<std::string>(1, "Noto Naskh Arabic");
text_style.font_size = 26;
text_style.letter_spacing = 1;
text_style.word_spacing = 5;
text_style.color = SK_ColorBLACK;
text_style.height = 1;
text_style.decoration = TextDecoration::kUnderline;
text_style.decoration_color = SK_ColorBLACK;
builder.PushStyle(text_style);

builder.AddText(u16_text);

builder.Pop();

auto paragraph = builder.Build();
paragraph->Layout(GetTestCanvasWidth() - 100);

paragraph->Paint(GetCanvas(), 0, 0);

SkPaint paint;
paint.setStyle(SkPaint::kStroke_Style);
paint.setAntiAlias(true);
paint.setStrokeWidth(1);

// Tests for GetRectsForRange()
Paragraph::RectHeightStyle rect_height_style =
Paragraph::RectHeightStyle::kMax;
Paragraph::RectWidthStyle rect_width_style =
Paragraph::RectWidthStyle::kTight;
paint.setColor(SK_ColorRED);
std::vector<txt::Paragraph::TextBox> boxes =
paragraph->GetRectsForRange(0, 100, rect_height_style, rect_width_style);
for (size_t i = 0; i < boxes.size(); ++i) {
GetCanvas()->drawRect(boxes[i].rect, paint);
}
EXPECT_EQ(boxes.size(), 2ull);

EXPECT_FLOAT_EQ(boxes[0].rect.left(), 556.54688);
EXPECT_FLOAT_EQ(boxes[0].rect.top(), -0.26855469);
EXPECT_FLOAT_EQ(boxes[0].rect.right(), 900);
EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 44);

EXPECT_FLOAT_EQ(boxes[1].rect.left(), 510.09375);
EXPECT_FLOAT_EQ(boxes[1].rect.top(), -0.26855469);
EXPECT_FLOAT_EQ(boxes[1].rect.right(), 557.04688);
EXPECT_FLOAT_EQ(boxes[1].rect.bottom(), 44);

ASSERT_EQ(paragraph_style.text_align,
paragraph->GetParagraphStyle().text_align);

ASSERT_TRUE(Snapshot());
}

TEST_F(ParagraphTest, GetGlyphPositionAtCoordinateParagraph) {
const char* text =
"12345 67890 12345 67890 12345 67890 12345 67890 12345 67890 12345 "
Expand Down
92 changes: 0 additions & 92 deletions third_party/txt/third_party/fonts/NotoNaskhArabic-LICENSE.txt
View file

This file was deleted.

Binary file removed third_party/txt/third_party/fonts/NotoNaskhArabic-Regular.ttf
View file
Binary file not shown.

0 comments on commit b4ed303

Please sign in to comment.