Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 57162a5

Browse files
author
Jonah Williams
authored
[Impeller] Cache Skia text bounds computation. (#45150)
Fixes flutter/flutter#133388 Just use the TextBounds that Skia gives us. These are slightly larger but already computed. If the larger bounds are a problem we can go back to Impeller computed bounds, but using a cached computation. ### Skia ![flutter_02](https://github.com/flutter/engine/assets/8975114/c1824eb3-bd82-47b0-a6af-282e51419a00) ### Impeller ![flutter_03](https://github.com/flutter/engine/assets/8975114/bb2eff8c-ae6e-4c59-90a5-228f8a115dca)
1 parent f2243d5 commit 57162a5

File tree

12 files changed

+82
-95
lines changed

12 files changed

+82
-95
lines changed

‎impeller/aiks/aiks_unittests.cc‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1278,7 +1278,11 @@ bool RenderTextInCanvasSkia(const std::shared_ptr<Context>& context,
12781278
}
12791279

12801280
// Create the Impeller text frame and draw it at the designated baseline.
1281-
auto frame = MakeTextFrameFromTextBlobSkia(blob);
1281+
auto maybe_frame = MakeTextFrameFromTextBlobSkia(blob);
1282+
if (!maybe_frame.has_value()) {
1283+
return false;
1284+
}
1285+
auto frame = maybe_frame.value();
12821286

12831287
Paint text_paint;
12841288
text_paint.color = Color::Yellow().WithAlpha(options.alpha);
@@ -1467,7 +1471,7 @@ TEST_P(AiksTest, CanRenderTextOutsideBoundaries) {
14671471
{
14681472
auto blob = SkTextBlob::MakeFromString(t.text, sk_font);
14691473
ASSERT_NE(blob, nullptr);
1470-
auto frame = MakeTextFrameFromTextBlobSkia(blob);
1474+
auto frame = MakeTextFrameFromTextBlobSkia(blob).value();
14711475
canvas.DrawTextFrame(frame, Point(), text_paint);
14721476
}
14731477
canvas.Restore();
@@ -3060,7 +3064,12 @@ TEST_P(AiksTest, TextForegroundShaderWithTransform) {
30603064

30613065
auto blob = SkTextBlob::MakeFromString("Hello", sk_font);
30623066
ASSERT_NE(blob, nullptr);
3063-
auto frame = MakeTextFrameFromTextBlobSkia(blob);
3067+
auto maybe_frame = MakeTextFrameFromTextBlobSkia(blob);
3068+
ASSERT_TRUE(maybe_frame.has_value());
3069+
if (!maybe_frame.has_value()) {
3070+
return;
3071+
}
3072+
auto frame = maybe_frame.value();
30643073
canvas.DrawTextFrame(frame, Point(), text_paint);
30653074

30663075
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));

‎impeller/display_list/dl_dispatcher.cc‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1111,7 +1111,11 @@ void DlDispatcher::drawDisplayList(
11111111
void DlDispatcher::drawTextBlob(const sk_sp<SkTextBlob>& blob,
11121112
SkScalar x,
11131113
SkScalar y) {
1114-
const auto text_frame = MakeTextFrameFromTextBlobSkia(blob);
1114+
const auto maybe_text_frame = MakeTextFrameFromTextBlobSkia(blob);
1115+
if (!maybe_text_frame.has_value()) {
1116+
return;
1117+
}
1118+
const auto text_frame = maybe_text_frame.value();
11151119
if (paint_.style == Paint::Style::kStroke ||
11161120
paint_.color_source.GetType() != ColorSource::Type::kColor) {
11171121
auto path = skia_conversions::PathDataFromTextBlob(blob);

‎impeller/entity/contents/text_contents.cc‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -59,16 +59,8 @@ void TextContents::SetOffset(Vector2 offset) {
5959
offset_ = offset;
6060
}
6161

62-
std::optional<Rect> TextContents::GetTextFrameBounds() const {
63-
return frame_.GetBounds();
64-
}
65-
6662
std::optional<Rect> TextContents::GetCoverage(const Entity& entity) const {
67-
auto bounds = frame_.GetBounds();
68-
if (!bounds.has_value()) {
69-
return std::nullopt;
70-
}
71-
return bounds->TransformBounds(entity.GetTransformation());
63+
return frame_.GetBounds().TransformBounds(entity.GetTransformation());
7264
}
7365

7466
void TextContents::PopulateGlyphAtlas(

‎impeller/entity/entity_unittests.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2174,7 +2174,7 @@ TEST_P(EntityTest, InheritOpacityTest) {
21742174
SkFont font;
21752175
font.setSize(30);
21762176
auto blob = SkTextBlob::MakeFromString("A", font);
2177-
auto frame = MakeTextFrameFromTextBlobSkia(blob);
2177+
auto frame = MakeTextFrameFromTextBlobSkia(blob).value();
21782178
auto lazy_glyph_atlas =
21792179
std::make_shared<LazyGlyphAtlas>(TypographerContextSkia::Make());
21802180
lazy_glyph_atlas->AddTextFrame(frame, 1.0f);

‎impeller/typographer/backends/skia/text_frame_skia.cc‎

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,17 @@ static Rect ToRect(const SkRect& rect) {
3939

4040
static constexpr Scalar kScaleSize = 100000.0f;
4141

42-
TextFrame MakeTextFrameFromTextBlobSkia(const sk_sp<SkTextBlob>& blob) {
42+
std::optional<TextFrame> MakeTextFrameFromTextBlobSkia(
43+
const sk_sp<SkTextBlob>& blob) {
44+
// Handling nullptr text blobs feels overly defensive here, as I don't
45+
// actually know if this happens.
4346
if (!blob) {
4447
return {};
4548
}
4649

47-
TextFrame frame;
48-
50+
std::vector<TextRun> runs;
51+
bool has_color = false;
4952
for (SkTextBlobRunIterator run(blob.get()); !run.done(); run.next()) {
50-
TextRun text_run(ToFont(run));
51-
5253
// TODO(jonahwilliams): ask Skia for a public API to look this up.
5354
// https://github.com/flutter/flutter/issues/112005
5455
SkStrikeSpec strikeSpec = SkStrikeSpec::MakeWithNoDevice(run.font());
@@ -57,12 +58,6 @@ TextFrame MakeTextFrameFromTextBlobSkia(const sk_sp<SkTextBlob>& blob) {
5758
const auto glyph_count = run.glyphCount();
5859
const auto* glyphs = run.glyphs();
5960
switch (run.positioning()) {
60-
case SkTextBlobRunIterator::kDefault_Positioning:
61-
FML_DLOG(ERROR) << "Unimplemented.";
62-
break;
63-
case SkTextBlobRunIterator::kHorizontal_Positioning:
64-
FML_DLOG(ERROR) << "Unimplemented.";
65-
break;
6661
case SkTextBlobRunIterator::kFull_Positioning: {
6762
std::vector<SkRect> glyph_bounds;
6863
glyph_bounds.resize(glyph_count);
@@ -75,32 +70,32 @@ TextFrame MakeTextFrameFromTextBlobSkia(const sk_sp<SkTextBlob>& blob) {
7570
font.setSize(kScaleSize);
7671
font.getBounds(glyphs, glyph_count, glyph_bounds.data(), nullptr);
7772

73+
std::vector<TextRun::GlyphPosition> positions;
74+
positions.reserve(glyph_count);
7875
for (auto i = 0u; i < glyph_count; i++) {
7976
// kFull_Positioning has two scalars per glyph.
8077
const SkPoint* glyph_points = run.points();
8178
const auto* point = glyph_points + i;
8279
Glyph::Type type = paths.glyph(glyphs[i])->isColor()
8380
? Glyph::Type::kBitmap
8481
: Glyph::Type::kPath;
82+
has_color |= type == Glyph::Type::kBitmap;
8583

86-
text_run.AddGlyph(
84+
positions.emplace_back(TextRun::GlyphPosition{
8785
Glyph{glyphs[i], type,
8886
ToRect(glyph_bounds[i]).Scale(font_size / kScaleSize)},
89-
Point{point->x(), point->y()});
87+
Point{point->x(), point->y()}});
9088
}
89+
TextRun text_run(ToFont(run), positions);
90+
runs.emplace_back(text_run);
9191
break;
9292
}
93-
case SkTextBlobRunIterator::kRSXform_Positioning:
94-
FML_DLOG(ERROR) << "Unimplemented.";
95-
break;
9693
default:
9794
FML_DLOG(ERROR) << "Unimplemented.";
9895
continue;
9996
}
100-
frame.AddTextRun(std::move(text_run));
10197
}
102-
103-
return frame;
98+
return TextFrame(runs, ToRect(blob->bounds()), has_color);
10499
}
105100

106101
} // namespace impeller

‎impeller/typographer/backends/skia/text_frame_skia.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
namespace impeller {
1212

13-
TextFrame MakeTextFrameFromTextBlobSkia(const sk_sp<SkTextBlob>& blob);
13+
std::optional<TextFrame> MakeTextFrameFromTextBlobSkia(
14+
const sk_sp<SkTextBlob>& blob);
1415

1516
} // namespace impeller

‎impeller/typographer/backends/stb/text_frame_stb.cc‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,16 @@ TextFrame MakeTextFrameSTB(const std::shared_ptr<TypefaceSTB>& typeface_stb,
5252
}
5353
}
5454

55-
TextFrame frame;
56-
frame.AddTextRun(std::move(run));
55+
std::optional<Rect> result;
56+
for (const auto& glyph_position : run.GetGlyphPositions()) {
57+
Rect glyph_rect =
58+
Rect(glyph_position.position + glyph_position.glyph.bounds.origin,
59+
glyph_position.glyph.bounds.size);
60+
result = result.has_value() ? result->Union(glyph_rect) : glyph_rect;
61+
}
5762

58-
return frame;
63+
std::vector<TextRun> runs = {run};
64+
return TextFrame(runs, result.value_or(Rect::MakeLTRB(0, 0, 0, 0)), false);
5965
}
6066

6167
} // namespace impeller

‎impeller/typographer/text_frame.cc‎

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,30 +8,13 @@ namespace impeller {
88

99
TextFrame::TextFrame() = default;
1010

11-
TextFrame::~TextFrame() = default;
12-
13-
std::optional<Rect> TextFrame::GetBounds() const {
14-
std::optional<Rect> result;
15-
16-
for (const auto& run : runs_) {
17-
for (const auto& glyph_position : run.GetGlyphPositions()) {
18-
Rect glyph_rect =
19-
Rect(glyph_position.position + glyph_position.glyph.bounds.origin,
20-
glyph_position.glyph.bounds.size);
21-
result = result.has_value() ? result->Union(glyph_rect) : glyph_rect;
22-
}
23-
}
11+
TextFrame::TextFrame(std::vector<TextRun>& runs, Rect bounds, bool has_color)
12+
: runs_(std::move(runs)), bounds_(bounds), has_color_(has_color) {}
2413

25-
return result;
26-
}
14+
TextFrame::~TextFrame() = default;
2715

28-
bool TextFrame::AddTextRun(TextRun&& run) {
29-
if (!run.IsValid()) {
30-
return false;
31-
}
32-
has_color_ |= run.HasColor();
33-
runs_.emplace_back(std::move(run));
34-
return true;
16+
Rect TextFrame::GetBounds() const {
17+
return bounds_;
3518
}
3619

3720
size_t TextFrame::GetRunCount() const {

‎impeller/typographer/text_frame.h‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ class TextFrame {
2020
public:
2121
TextFrame();
2222

23+
TextFrame(std::vector<TextRun>& runs, Rect bounds, bool has_color);
24+
2325
~TextFrame();
2426

2527
void CollectUniqueFontGlyphPairs(FontGlyphPair::Set& set, Scalar scale) const;
@@ -30,9 +32,9 @@ class TextFrame {
3032
/// @brief The conservative bounding box for this text frame.
3133
///
3234
/// @return The bounds rectangle. If there are no glyphs in this text
33-
/// frame, std::nullopt is returned.
35+
/// frame and empty Rectangle is returned instead.
3436
///
35-
std::optional<Rect> GetBounds() const;
37+
Rect GetBounds() const;
3638

3739
//----------------------------------------------------------------------------
3840
/// @brief The number of runs in this text frame.
@@ -41,15 +43,6 @@ class TextFrame {
4143
///
4244
size_t GetRunCount() const;
4345

44-
//----------------------------------------------------------------------------
45-
/// @brief Adds a new text run to the text frame.
46-
///
47-
/// @param[in] run The run
48-
///
49-
/// @return If the text run could be added to this frame.
50-
///
51-
bool AddTextRun(TextRun&& run);
52-
5346
//----------------------------------------------------------------------------
5447
/// @brief Returns a reference to all the text runs in this frame.
5548
///
@@ -77,6 +70,7 @@ class TextFrame {
7770

7871
private:
7972
std::vector<TextRun> runs_;
73+
Rect bounds_;
8074
bool has_color_ = false;
8175
};
8276

‎impeller/typographer/text_run.cc‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,18 @@ TextRun::TextRun(const Font& font) : font_(font) {
1313
is_valid_ = true;
1414
}
1515

16+
TextRun::TextRun(const Font& font, std::vector<GlyphPosition>& glyphs)
17+
: font_(font), glyphs_(std::move(glyphs)) {
18+
if (!font_.IsValid()) {
19+
return;
20+
}
21+
is_valid_ = true;
22+
}
23+
1624
TextRun::~TextRun() = default;
1725

1826
bool TextRun::AddGlyph(Glyph glyph, Point position) {
1927
glyphs_.emplace_back(GlyphPosition{glyph, position});
20-
has_color_ |= glyph.type == Glyph::Type::kBitmap;
2128
return true;
2229
}
2330

@@ -37,8 +44,4 @@ const Font& TextRun::GetFont() const {
3744
return font_;
3845
}
3946

40-
bool TextRun::HasColor() const {
41-
return has_color_;
42-
}
43-
4447
} // namespace impeller

0 commit comments

Comments
 (0)