Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
In introduced a regression in text layout: flutter/flutter#17502
  • Loading branch information
tvolkert authored May 11, 2018
1 parent b856303 commit 9ae10ef
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 53 deletions.
2 changes: 1 addition & 1 deletion third_party/txt/src/minikin/LineBreaker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ void LineBreaker::setIndents(const std::vector<float>& indents) {
// end of line. It is the Unicode set:
// [[:General_Category=Space_Separator:]-[:Line_Break=Glue:]], plus '\n'. Note:
// all such characters are in the BMP, so it's ok to use code units for this.
bool isLineEndSpace(uint16_t c) {
static bool isLineEndSpace(uint16_t c) {
return c == '\n' || c == ' ' || c == 0x1680 ||
(0x2000 <= c && c <= 0x200A && c != 0x2007) || c == 0x205F ||
c == 0x3000;
Expand Down
2 changes: 0 additions & 2 deletions third_party/txt/src/minikin/LineBreaker.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ enum HyphenationFrequency {
kHyphenationFrequency_Full = 2
};

bool isLineEndSpace(uint16_t c);

// TODO: want to generalize to be able to handle array of line widths
class LineWidths {
public:
Expand Down
48 changes: 21 additions & 27 deletions third_party/txt/src/txt/paragraph.cc
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,7 @@ bool Paragraph::ComputeLineBreaks() {
size_t block_size = block_end - block_start;

if (block_size == 0) {
line_ranges_.emplace_back(block_start, block_end, block_end,
block_end + 1, true);
line_ranges_.emplace_back(block_start, block_end, block_end + 1, true);
line_widths_.push_back(0);
continue;
}
Expand Down Expand Up @@ -312,14 +311,7 @@ bool Paragraph::ComputeLineBreaks() {
bool hard_break = i == breaks_count - 1;
size_t line_end_including_newline =
(hard_break && line_end < text_.size()) ? line_end + 1 : line_end;
size_t line_end_excluding_whitespace = line_end;
while (
line_end_excluding_whitespace > 0 &&
minikin::isLineEndSpace(text_[line_end_excluding_whitespace - 1])) {
line_end_excluding_whitespace--;
}
line_ranges_.emplace_back(line_start, line_end,
line_end_excluding_whitespace,
line_end_including_newline, hard_break);
line_widths_.push_back(breaker_.getWidths()[i]);
}
Expand Down Expand Up @@ -470,20 +462,13 @@ void Paragraph::Layout(double width, bool force) {
}
}

// Exclude trailing whitespace from right-justified lines so the last
// visible character in the line will be flush with the right margin.
size_t line_end_index =
(paragraph_style_.effective_align() == TextAlign::right)
? line_range.end_excluding_whitespace
: line_range.end;

// 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 &&
if (bidi_run.start() < line_range.end &&
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),
std::min(bidi_run.end(), line_range.end),
bidi_run.direction(), bidi_run.style());
}
}
Expand Down Expand Up @@ -702,7 +687,7 @@ void Paragraph::Layout(double width, bool force) {
}

// Adjust the glyph positions based on the alignment of the line.
double line_x_offset = GetLineXOffset(run_x_offset);
double line_x_offset = GetLineXOffset(line_number, run_x_offset);
if (line_x_offset) {
for (CodeUnitRun& code_unit_run : line_code_unit_runs) {
for (GlyphPosition& position : code_unit_run.positions) {
Expand Down Expand Up @@ -795,16 +780,25 @@ void Paragraph::Layout(double width, bool force) {
});
}

double Paragraph::GetLineXOffset(double line_total_advance) {
if (isinf(width_))
double Paragraph::GetLineXOffset(size_t line_number,
double line_total_advance) {
if (line_number >= line_widths_.size() || isinf(width_))
return 0;

TextAlign align = paragraph_style_.effective_align();

if (align == TextAlign::right) {
return width_ - line_total_advance;
} else if (align == TextAlign::center) {
return (width_ - line_total_advance) / 2;
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)) {
// If this line has a soft break, then use the line width calculated by the
// line breaker because that width excludes the soft break's whitespace.
double text_width = line_ranges_[line_number].hard_break
? line_total_advance
: line_widths_[line_number];
return width_ - text_width;
} else if (paragraph_style_.text_align == TextAlign::center) {
return (width_ - line_widths_[line_number]) / 2;
} else {
return 0;
}
Expand Down
11 changes: 3 additions & 8 deletions third_party/txt/src/txt/paragraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -187,14 +187,9 @@ class Paragraph {
mutable std::unique_ptr<icu::BreakIterator> word_breaker_;

struct LineRange {
LineRange(size_t s, size_t e, size_t eew, size_t ein, bool h)
: start(s),
end(e),
end_excluding_whitespace(eew),
end_including_newline(ein),
hard_break(h) {}
LineRange(size_t s, size_t e, size_t ewn, bool h)
: start(s), end(e), end_including_newline(ewn), hard_break(h) {}
size_t start, end;
size_t end_excluding_whitespace;
size_t end_including_newline;
bool hard_break;
};
Expand Down Expand Up @@ -305,7 +300,7 @@ class 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(size_t line_number, double line_total_advance);

// Creates and draws the decorations onto the canvas.
void PaintDecorations(SkCanvas* canvas, const PaintRecord& record);
Expand Down
12 changes: 0 additions & 12 deletions third_party/txt/src/txt/paragraph_style.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,4 @@ bool ParagraphStyle::ellipsized() const {
return !ellipsis.empty();
}

TextAlign ParagraphStyle::effective_align() const {
if (text_align == TextAlign::start) {
return (text_direction == TextDirection::ltr) ? TextAlign::left
: TextAlign::right;
} else if (text_align == TextAlign::end) {
return (text_direction == TextDirection::ltr) ? TextAlign::right
: TextAlign::left;
} else {
return text_align;
}
}

} // namespace txt
3 changes: 0 additions & 3 deletions third_party/txt/src/txt/paragraph_style.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,6 @@ class ParagraphStyle {

bool unlimited_lines() const;
bool ellipsized() const;

// Return a text alignment value that is not dependent on the text direction.
TextAlign effective_align() const;
};

} // namespace txt
Expand Down

0 comments on commit 9ae10ef

Please sign in to comment.