Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Impeller] use public API for subpixel/color/emoji #55438

Closed
wants to merge 1 commit into from
Closed

[Impeller] use public API for subpixel/color/emoji #55438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 13 additions & 33 deletions impeller/typographer/backends/skia/text_frame_skia.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,30 +14,10 @@
#include "third_party/skia/include/core/SkFontMetrics.h"
#include "third_party/skia/include/core/SkPaint.h"
#include "third_party/skia/include/core/SkRect.h"
#include "third_party/skia/src/core/SkStrikeSpec.h" // nogncheck
#include "third_party/skia/src/core/SkTextBlobPriv.h" // nogncheck

namespace impeller {

/// @brief Convert a Skia axis alignment into an Impeller alignment.
///
/// This does not include a case for AxisAlignment::kNone, that should
/// be used if SkFont::isSubpixel is false.
static AxisAlignment ToAxisAligment(SkAxisAlignment aligment) {
switch (aligment) {
case SkAxisAlignment::kNone:
// Skia calls this case none, meaning alignment in both X and Y.
// Impeller will call it "all" since that is less confusing. "none"
// is reserved for no subpixel alignment.
return AxisAlignment::kAll;
case SkAxisAlignment::kX:
return AxisAlignment::kX;
case SkAxisAlignment::kY:
return AxisAlignment::kY;
}
FML_UNREACHABLE();
}

static Font ToFont(const SkTextBlobRunIterator& run, AxisAlignment alignment) {
auto& font = run.font();
auto typeface = std::make_shared<TypefaceSkia>(font.refTypeface());
Expand All @@ -59,18 +39,21 @@ static Rect ToRect(const SkRect& rect) {
}

std::shared_ptr<TextFrame> MakeTextFrameFromTextBlobSkia(
const sk_sp<SkTextBlob>& blob) {
bool has_color = false;
const sk_sp<SkTextBlob>& blob,
bool contains_color_glyphs) {
Glyph::Type type =
contains_color_glyphs ? Glyph::Type::kBitmap : Glyph::Type::kPath;
std::vector<TextRun> runs;

for (SkTextBlobRunIterator run(blob.get()); !run.done(); run.next()) {
// TODO(112005): Ask Skia for a public API to look this up. This is using a
// private API today.
SkStrikeSpec strikeSpec = SkStrikeSpec::MakeWithNoDevice(run.font());
SkBulkGlyphMetricsAndPaths paths{strikeSpec};
AxisAlignment alignment = AxisAlignment::kNone;
if (run.font().isSubpixel()) {
alignment = ToAxisAligment(
strikeSpec.createScalerContext()->computeAxisAlignmentForHText());
// If baseline snap or color glyphs, then no axis alignment
if (run.font().isBaselineSnap() || contains_color_glyphs) {
alignment = AxisAlignment::kNone;
} else {
alignment = AxisAlignment::kX;
}
}

const auto glyph_count = run.glyphCount();
Expand All @@ -83,10 +66,6 @@ std::shared_ptr<TextFrame> MakeTextFrameFromTextBlobSkia(
// kFull_Positioning has two scalars per glyph.
const SkPoint* glyph_points = run.points();
const SkPoint* point = glyph_points + i;
Glyph::Type type = paths.glyph(glyphs[i])->isColor()
? Glyph::Type::kBitmap
: Glyph::Type::kPath;
has_color |= type == Glyph::Type::kBitmap;
positions.emplace_back(
TextRun::GlyphPosition{Glyph{glyphs[i], type}, Point{
point->x(),
Expand All @@ -102,7 +81,8 @@ std::shared_ptr<TextFrame> MakeTextFrameFromTextBlobSkia(
continue;
}
}
return std::make_shared<TextFrame>(runs, ToRect(blob->bounds()), has_color);
return std::make_shared<TextFrame>(runs, ToRect(blob->bounds()),
contains_color_glyphs);
}

} // namespace impeller
3 changes: 2 additions & 1 deletion impeller/typographer/backends/skia/text_frame_skia.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
namespace impeller {

std::shared_ptr<impeller::TextFrame> MakeTextFrameFromTextBlobSkia(
const sk_sp<SkTextBlob>& blob);
const sk_sp<SkTextBlob>& blob,
bool contains_color_glyphs = false);

} // namespace impeller

Expand Down
9 changes: 6 additions & 3 deletions impeller/typographer/typographer_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ TEST_P(TypographerTest, LazyAtlasTracksColor) {
lazy_atlas.AddTextFrame(*frame, 1.0f, {0, 0}, {});

frame = MakeTextFrameFromTextBlobSkia(
SkTextBlob::MakeFromString("😀 ", emoji_font));
SkTextBlob::MakeFromString("😀 ", emoji_font),
/*contains_color_glyphs=*/true);

ASSERT_TRUE(frame->GetAtlasType() == GlyphAtlas::Type::kColorBitmap);

Expand Down Expand Up @@ -305,9 +306,11 @@ TEST_P(TypographerTest, GlyphColorIsPartOfCacheKey) {
// Create two frames with the same character and a different color, expect
// that it adds a character.
auto frame = MakeTextFrameFromTextBlobSkia(
SkTextBlob::MakeFromString("😂", emoji_font));
SkTextBlob::MakeFromString("😂", emoji_font),
/*contains_color_glyphs=*/true);
auto frame_2 = MakeTextFrameFromTextBlobSkia(
SkTextBlob::MakeFromString("😂", emoji_font));
SkTextBlob::MakeFromString("😂", emoji_font),
/*contains_color_glyphs=*/true);
auto properties = {
GlyphProperties{.color = Color::Red()},
GlyphProperties{.color = Color::Blue()},
Expand Down
32 changes: 23 additions & 9 deletions third_party/txt/src/skia/paragraph_skia.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,13 @@ class DisplayListParagraphPainter : public skt::ParagraphPainter {
/// See https://github.com/flutter/flutter/issues/126673. It
/// probably makes sense to eventually make this a compile-time
/// decision (i.e. with `#ifdef`) instead of a runtime option.
DisplayListParagraphPainter(DisplayListBuilder* builder,
const std::vector<DlPaint>& dl_paints,
bool impeller_enabled)
DisplayListParagraphPainter(
DisplayListBuilder* builder,
const std::unique_ptr<skia::textlayout::Paragraph>& paragraph,
const std::vector<DlPaint>& dl_paints,
bool impeller_enabled)
: builder_(builder),
paragraph_(paragraph),
dl_paints_(dl_paints),
impeller_enabled_(impeller_enabled) {}

Expand All @@ -92,7 +95,10 @@ class DisplayListParagraphPainter : public skt::ParagraphPainter {
// If there is no path, this is an emoji and should be drawn as is,
// ignoring the color source.
if (path.isEmpty()) {
builder_->DrawTextFrame(impeller::MakeTextFrameFromTextBlobSkia(blob),
auto contains_color_glyphs =
paragraph_->containsColorFontOrBitmap(blob.get());
builder_->DrawTextFrame(impeller::MakeTextFrameFromTextBlobSkia(
blob, contains_color_glyphs),
x, y, dl_paints_[paint_id]);

return;
Expand All @@ -103,8 +109,11 @@ class DisplayListParagraphPainter : public skt::ParagraphPainter {
builder_->DrawPath(transformed, dl_paints_[paint_id]);
return;
}
builder_->DrawTextFrame(impeller::MakeTextFrameFromTextBlobSkia(blob), x,
y, dl_paints_[paint_id]);
auto contains_color_glyphs =
paragraph_->containsColorFontOrBitmap(blob.get());
builder_->DrawTextFrame(
impeller::MakeTextFrameFromTextBlobSkia(blob, contains_color_glyphs),
x, y, dl_paints_[paint_id]);
return;
}
#endif // IMPELLER_SUPPORTS_RENDERING
Expand All @@ -126,8 +135,11 @@ class DisplayListParagraphPainter : public skt::ParagraphPainter {
paint.setMaskFilter(&filter);
}
if (impeller_enabled_) {
builder_->DrawTextFrame(impeller::MakeTextFrameFromTextBlobSkia(blob), x,
y, paint);
auto contains_color_glyphs =
paragraph_->containsColorFontOrBitmap(blob.get());
builder_->DrawTextFrame(
impeller::MakeTextFrameFromTextBlobSkia(blob, contains_color_glyphs),
x, y, paint);
return;
}
builder_->DrawTextBlob(blob, x, y, paint);
Expand Down Expand Up @@ -207,6 +219,7 @@ class DisplayListParagraphPainter : public skt::ParagraphPainter {
}

DisplayListBuilder* builder_;
const std::unique_ptr<skia::textlayout::Paragraph>& paragraph_;
const std::vector<DlPaint>& dl_paints_;
const bool impeller_enabled_;
};
Expand Down Expand Up @@ -303,7 +316,8 @@ void ParagraphSkia::Layout(double width) {
}

bool ParagraphSkia::Paint(DisplayListBuilder* builder, double x, double y) {
DisplayListParagraphPainter painter(builder, dl_paints_, impeller_enabled_);
DisplayListParagraphPainter painter(builder, paragraph_, dl_paints_,
impeller_enabled_);
paragraph_->paint(&painter, x, y);
return true;
}
Expand Down