Skip to content

Commit

Permalink
libtxt: If no glyphs were rendered, then compute baseline metrics usi…
Browse files Browse the repository at this point in the history
…ng the paragraph style's font (flutter#4462)

Fixes the "empty text baseline" case in text_painter_rtl_test
  • Loading branch information
jason-simmons authored Dec 15, 2017
1 parent d945976 commit 2b2e21a
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 23 deletions.
57 changes: 37 additions & 20 deletions third_party/txt/src/txt/paragraph.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,14 @@ bool GetItalic(const TextStyle& style) {
}
}

minikin::FontStyle GetMinikinFontStyle(const TextStyle& style) {
return minikin::FontStyle(GetWeight(style), GetItalic(style));
}

void GetFontAndMinikinPaint(const TextStyle& style,
minikin::FontStyle* font,
minikin::MinikinPaint* paint) {
*font = minikin::FontStyle(GetWeight(style), GetItalic(style));
*font = GetMinikinFontStyle(style);
paint->size = style.font_size;
// Divide by font size so letter spacing is pixels, not proportional to font
// size.
Expand All @@ -115,8 +119,14 @@ void GetFontAndMinikinPaint(const TextStyle& style,
paint->paintFlags |= minikin::LinearTextFlag;
}

void GetPaint(const TextStyle& style, SkPaint* paint) {
paint->setTextSize(style.font_size);
sk_sp<SkTypeface> GetDefaultSkiaTypeface(
const std::shared_ptr<txt::FontCollection>& font_collection,
const TextStyle& style) {
std::shared_ptr<minikin::FontCollection> collection =
font_collection->GetMinikinFontCollectionForFamily(style.font_family);
minikin::FakedFont faked_font =
collection->baseFontFaked(GetMinikinFontStyle(style));
return static_cast<FontSkia*>(faked_font.font)->GetSkTypeface();
}

void FindWords(const std::vector<uint16_t>& text,
Expand Down Expand Up @@ -258,6 +268,9 @@ bool Paragraph::ComputeLineBreaks() {
}

bool Paragraph::ComputeBidiRuns(std::vector<BidiRun>* result) {
if (text_.empty())
return true;

auto ubidi_closer = [](UBiDi* b) { ubidi_close(b); };
std::unique_ptr<UBiDi, decltype(ubidi_closer)> bidi(ubidi_open(),
ubidi_closer);
Expand Down Expand Up @@ -406,7 +419,7 @@ void Paragraph::Layout(double width, bool force) {
minikin::FontStyle font;
minikin::MinikinPaint minikin_paint;
GetFontAndMinikinPaint(run.style, &font, &minikin_paint);
GetPaint(run.style, &paint);
paint.setTextSize(run.style.font_size);

std::shared_ptr<minikin::FontCollection> minikin_font_collection =
font_collection_->GetMinikinFontCollectionForFamily(
Expand Down Expand Up @@ -467,16 +480,8 @@ void Paragraph::Layout(double width, bool force) {
layout.doLayout(text_ptr, text_start, text_count, text_.size(), bidiFlags,
font, minikin_paint, minikin_font_collection);

if (layout.nGlyphs() == 0) {
// This run is empty, so insert a placeholder paint record that captures
// the current font metrics.
SkPaint::FontMetrics metrics;
paint.getFontMetrics(&metrics);
paint_records.emplace_back(run.style, SkPoint::Make(run_x_offset, 0),
builder.make(), metrics, line_number,
layout.getAdvance());
if (layout.nGlyphs() == 0)
continue;
}

// Break the layout into blobs that share the same SkPaint parameters.
std::vector<Range<size_t>> glyph_blobs;
Expand Down Expand Up @@ -604,26 +609,38 @@ void Paragraph::Layout(double width, bool force) {

double max_line_spacing = 0;
double max_descent = 0;
for (const PaintRecord& paint_record : paint_records) {
const SkPaint::FontMetrics& metrics = paint_record.metrics();
double style_height = paint_record.style().height;
auto update_line_metrics = [&](const SkPaint::FontMetrics& metrics,
const TextStyle& style) {
double line_spacing =
(line_number == 0)
? -metrics.fAscent * style_height
: (-metrics.fAscent + metrics.fLeading) * style_height;
? -metrics.fAscent * style.height
: (-metrics.fAscent + metrics.fLeading) * style.height;
if (line_spacing > max_line_spacing) {
max_line_spacing = line_spacing;
if (line_number == 0) {
alphabetic_baseline_ = line_spacing;
// TODO(garyq): Properly implement ideographic_baseline_.
ideographic_baseline_ =
(metrics.fUnderlinePosition - metrics.fAscent) * style_height;
(metrics.fUnderlinePosition - metrics.fAscent) * style.height;
}
}
max_line_spacing = std::max(line_spacing, max_line_spacing);

double descent = metrics.fDescent * style_height;
double descent = metrics.fDescent * style.height;
max_descent = std::max(descent, max_descent);
};
for (const PaintRecord& paint_record : paint_records) {
update_line_metrics(paint_record.metrics(), paint_record.style());
}
// If no fonts were actually rendered, then compute a baseline based on the
// font of the paragraph style.
if (paint_records.empty()) {
SkPaint::FontMetrics metrics;
TextStyle style(paragraph_style_.GetTextStyle());
paint.setTypeface(GetDefaultSkiaTypeface(font_collection_, style));
paint.setTextSize(style.font_size);
paint.getFontMetrics(&metrics);
update_line_metrics(metrics, style);
}

line_heights_.push_back((line_heights_.empty() ? 0 : line_heights_.back()) +
Expand Down
9 changes: 8 additions & 1 deletion third_party/txt/src/txt/paragraph_style.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@

namespace txt {

//
TextStyle ParagraphStyle::GetTextStyle() const {
TextStyle result;
result.font_weight = font_weight;
result.font_style = font_style;
result.font_family = font_family;
result.font_size = font_size;
return result;
}

} // namespace txt
3 changes: 3 additions & 0 deletions third_party/txt/src/txt/paragraph_style.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "font_style.h"
#include "font_weight.h"
#include "minikin/LineBreaker.h"
#include "text_style.h"

namespace txt {

Expand Down Expand Up @@ -58,6 +59,8 @@ class ParagraphStyle {
// kBreakStrategy_Balanced will balance between the two.
minikin::BreakStrategy break_strategy =
minikin::BreakStrategy::kBreakStrategy_Greedy;

TextStyle GetTextStyle() const;
};

} // namespace txt
Expand Down
3 changes: 1 addition & 2 deletions third_party/txt/tests/paragraph_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1345,7 +1345,7 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(NewlineParagraph)) {

ASSERT_TRUE(Snapshot());

ASSERT_EQ(paragraph->records_.size(), 7ull);
ASSERT_EQ(paragraph->records_.size(), 6ull);
EXPECT_DOUBLE_EQ(paragraph->records_[0].offset().x(), 0);
EXPECT_DOUBLE_EQ(paragraph->records_[1].offset().x(), 0);
EXPECT_DOUBLE_EQ(paragraph->records_[1].offset().y(), 126);
Expand All @@ -1354,7 +1354,6 @@ TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(NewlineParagraph)) {
EXPECT_DOUBLE_EQ(paragraph->records_[4].offset().x(), 0);
EXPECT_DOUBLE_EQ(paragraph->records_[3].offset().y(), 266);
EXPECT_DOUBLE_EQ(paragraph->records_[5].offset().x(), 0);
EXPECT_DOUBLE_EQ(paragraph->records_[6].offset().x(), 0);
}

TEST_F(ParagraphTest, DISABLE_ON_WINDOWS(EmojiParagraph)) {
Expand Down

0 comments on commit 2b2e21a

Please sign in to comment.