Skip to content

Commit

Permalink
libtxt: track the font metrics of each run in order to calculate heig…
Browse files Browse the repository at this point in the history 
…hts in GetRectsForRange (flutter#4442)

Previously GetRectsForRange was assigning each rectangle a height matching the
height of the entire line.  If the line includes multiple text styles, callers
will expect each span's rectangle to reflect the height of that span.
(see text_painter_rtl_test.dart)
  • Loading branch information
@jason-simmons
jason-simmons authored Dec 11, 2017
1 parent edaecdc commit 9f8cf7e
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 26 deletions.
44 changes: 24 additions & 20 deletions third_party/txt/src/txt/paragraph.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,13 @@ Paragraph::CodeUnitRun::CodeUnitRun(std::vector<GlyphPosition>&& p,
Range<size_t> cu,
Range<double> x,
size_t line,
const SkPaint::FontMetrics& metrics,
TextDirection dir)
: positions(std::move(p)),
code_units(cu),
x_pos(x),
line_number(line),
font_metrics(metrics),
direction(dir) {}

Paragraph::Paragraph() {
Expand Down Expand Up @@ -484,7 +486,6 @@ void Paragraph::Layout(double width, bool force) {
blob_start += blob_len;
}

std::vector<GlyphPosition> glyph_positions;
size_t code_unit_index;
if (run.is_rtl()) {
code_unit_index = text_count;
Expand All @@ -496,6 +497,8 @@ void Paragraph::Layout(double width, bool force) {

// Build a Skia text blob from each group of glyphs.
for (const Range<size_t>& glyph_blob : glyph_blobs) {
std::vector<GlyphPosition> glyph_positions;

paint.setTypeface(GetTypefaceForGlyph(layout, glyph_blob.start));
const SkTextBlobBuilder::RunBuffer& blob_buffer =
builder.allocRunPos(paint, glyph_blob.end - glyph_blob.start);
Expand Down Expand Up @@ -578,23 +581,23 @@ void Paragraph::Layout(double width, bool force) {
paint_records.emplace_back(run.style, SkPoint::Make(run_x_offset, 0),
builder.make(), metrics, line_number,
layout.getAdvance());
}

line_glyph_positions.insert(line_glyph_positions.end(),
glyph_positions.begin(),
glyph_positions.end());

// Add a record of glyph positions sorted by code unit index.
std::vector<GlyphPosition> code_unit_positions(glyph_positions);
std::sort(code_unit_positions.begin(), code_unit_positions.end(),
[](const GlyphPosition& a, const GlyphPosition& b) {
return a.code_units.start < b.code_units.start;
});
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, run.direction);
line_glyph_positions.insert(line_glyph_positions.end(),
glyph_positions.begin(),
glyph_positions.end());

// Add a record of glyph positions sorted by code unit index.
std::vector<GlyphPosition> code_unit_positions(glyph_positions);
std::sort(code_unit_positions.begin(), code_unit_positions.end(),
[](const GlyphPosition& a, const GlyphPosition& b) {
return a.code_units.start < b.code_units.start;
});
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);
}

run_x_offset += layout.getAdvance();
}
Expand Down Expand Up @@ -625,6 +628,7 @@ void Paragraph::Layout(double width, bool force) {

line_heights_.push_back((line_heights_.empty() ? 0 : line_heights_.back()) +
round(max_line_spacing + max_descent));
line_baselines_.push_back(line_heights_.back() - max_descent);
y_offset += round(max_line_spacing + prev_max_descent);
prev_max_descent = max_descent;

Expand Down Expand Up @@ -897,9 +901,9 @@ std::vector<Paragraph::TextBox> Paragraph::GetRectsForRange(size_t start,
if (run.code_units.end <= start)
continue;

SkScalar top =
(run.line_number > 0) ? line_heights_[run.line_number - 1] : 0;
SkScalar bottom = line_heights_[run.line_number];
double baseline = line_baselines_[run.line_number];
SkScalar top = baseline + run.font_metrics.fAscent;
SkScalar bottom = baseline + run.font_metrics.fDescent;

SkScalar left, right;
if (run.code_units.start >= start && run.code_units.end <= end) {
Expand Down
3 changes: 3 additions & 0 deletions third_party/txt/src/txt/paragraph.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ class Paragraph {
std::vector<PaintRecord> records_;

std::vector<double> line_heights_;
std::vector<double> line_baselines_;
bool did_exceed_max_lines_;

struct GlyphPosition {
Expand All @@ -232,12 +233,14 @@ class Paragraph {
Range<size_t> code_units;
Range<double> x_pos;
size_t line_number;
SkPaint::FontMetrics font_metrics;
TextDirection direction;

CodeUnitRun(std::vector<GlyphPosition>&& p,
Range<size_t> cu,
Range<double> x,
size_t line,
const SkPaint::FontMetrics& metrics,
TextDirection dir);
};

Expand Down
12 changes: 6 additions & 6 deletions third_party/txt/tests/paragraph_unittests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -968,7 +968,7 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(GetRectsForRangeParagraph)) {
}
EXPECT_EQ(boxes.size(), 1ull);
EXPECT_FLOAT_EQ(boxes[0].rect.left(), 0);
EXPECT_FLOAT_EQ(boxes[0].rect.top(), 0);
EXPECT_FLOAT_EQ(boxes[0].rect.top(), 0.40625);
EXPECT_FLOAT_EQ(boxes[0].rect.right(), 28.417969);
EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 59);

Expand All @@ -979,7 +979,7 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(GetRectsForRangeParagraph)) {
}
EXPECT_EQ(boxes.size(), 1ull);
EXPECT_FLOAT_EQ(boxes[0].rect.left(), 56.835938);
EXPECT_FLOAT_EQ(boxes[0].rect.top(), 0);
EXPECT_FLOAT_EQ(boxes[0].rect.top(), 0.40625);
EXPECT_FLOAT_EQ(boxes[0].rect.right(), 177.44922);
EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 59);

Expand All @@ -990,7 +990,7 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(GetRectsForRangeParagraph)) {
}
EXPECT_EQ(boxes.size(), 1ull);
EXPECT_FLOAT_EQ(boxes[0].rect.left(), 177);
EXPECT_FLOAT_EQ(boxes[0].rect.top(), 0);
EXPECT_FLOAT_EQ(boxes[0].rect.top(), 0.40625);
EXPECT_FLOAT_EQ(boxes[0].rect.right(), 506.08984);
EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 59);

Expand All @@ -1001,14 +1001,14 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(GetRectsForRangeParagraph)) {
}
EXPECT_EQ(boxes.size(), 4ull);
EXPECT_FLOAT_EQ(boxes[0].rect.left(), 210.83594);
EXPECT_FLOAT_EQ(boxes[0].rect.top(), 59);
EXPECT_FLOAT_EQ(boxes[0].rect.top(), 59.40625);
EXPECT_FLOAT_EQ(boxes[0].rect.right(), 463.44922);
EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 118);

// TODO(garyq): The following set of vals are definetly wrong and
// end of paragraph handling needs to be fixed in a later patch.
EXPECT_FLOAT_EQ(boxes[3].rect.left(), 0);
EXPECT_FLOAT_EQ(boxes[3].rect.top(), 236);
EXPECT_FLOAT_EQ(boxes[3].rect.top(), 236.40625);
EXPECT_FLOAT_EQ(boxes[3].rect.right(), 142.08984);
EXPECT_FLOAT_EQ(boxes[3].rect.bottom(), 295);

Expand All @@ -1019,7 +1019,7 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(GetRectsForRangeParagraph)) {
}
EXPECT_EQ(boxes.size(), 1ull);
EXPECT_FLOAT_EQ(boxes[0].rect.left(), 449.25391);
EXPECT_FLOAT_EQ(boxes[0].rect.top(), 0);
EXPECT_FLOAT_EQ(boxes[0].rect.top(), 0.40625);
EXPECT_FLOAT_EQ(boxes[0].rect.right(), 519.44922);
EXPECT_FLOAT_EQ(boxes[0].rect.bottom(), 59);

Expand Down

0 comments on commit 9f8cf7e

Please sign in to comment.