Sitelet https://github.com/flutter/flutter/pull/166199/files
Skip to content

[Impeller] small cpu perf for text contents. #166199

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

Merged
auto-submit merged 5 commits into from
Apr 1, 2025
Merged

[Impeller] small cpu perf for text contents. #166199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
62cda0b
[Impeller] small cpu perf for text contents.
Mar 29, 2025
4c85295
use std::array.
Mar 31, 2025
e1e5967
Revert "use std::array."
Mar 31, 2025
b50ba7c
++
Mar 31, 2025
bdff6c0
fixed for real.
Mar 31, 2025
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
73 changes: 45 additions & 28 deletions engine/src/flutter/impeller/entity/contents/text_contents.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,7 @@ void TextContents::ComputeVertexData(
// interpolated vertex information is also used in the fragment shader to
// sample from the glyph atlas.

constexpr std::array<Point, 6> unit_points = {Point{0, 0}, Point{1, 0},
Point{0, 1}, Point{1, 0},
constexpr std::array<Point, 4> unit_points = {Point{0, 0}, Point{1, 0},
Point{0, 1}, Point{1, 1}};

ISize atlas_size = atlas->GetTexture()->GetSize();
Expand All @@ -119,9 +118,19 @@ void TextContents::ComputeVertexData(
VS::PerVertexData vtx;
size_t i = 0u;
size_t bounds_offset = 0u;
Rational rounded_scale = frame->GetScale();
Scalar inverted_rounded_scale = static_cast<Scalar>(rounded_scale.Invert());
Matrix unscaled_basis =
basis_transform *
Matrix::MakeScale({inverted_rounded_scale, inverted_rounded_scale, 1});

// In typical scales < 48x these values should be -1 or 1. We round to
// those to avoid inaccuracies.
unscaled_basis.m[0] = AttractToOne(unscaled_basis.m[0]);
unscaled_basis.m[5] = AttractToOne(unscaled_basis.m[5]);

for (const TextRun& run : frame->GetRuns()) {
const Font& font = run.GetFont();
Rational rounded_scale = frame->GetScale();
const Matrix transform = frame->GetOffsetTransform();
FontGlyphAtlas* font_atlas = nullptr;

Expand Down Expand Up @@ -181,26 +190,15 @@ void TextContents::ComputeVertexData(
atlas_glyph_bounds = maybe_atlas_glyph_bounds.value().atlas_bounds;
}

Scalar inverted_rounded_scale =
static_cast<Scalar>(rounded_scale.Invert());
Rect scaled_bounds = glyph_bounds.Scale(inverted_rounded_scale);
// For each glyph, we compute two rectangles. One for the vertex
// positions and one for the texture coordinates (UVs). The atlas
// glyph bounds are used to compute UVs in cases where the
// destination and source sizes may differ due to clamping the sizes
// of large glyphs.
Point uv_origin = (atlas_glyph_bounds.GetLeftTop()) / atlas_size;
Point uv_origin = atlas_glyph_bounds.GetLeftTop() / atlas_size;
Point uv_size = SizeToPoint(atlas_glyph_bounds.GetSize()) / atlas_size;

Matrix unscaled_basis =
basis_transform * Matrix::MakeScale({inverted_rounded_scale,
inverted_rounded_scale, 1});

// In typical scales < 48x these values should be -1 or 1. We round to
// those to avoid inaccuracies.
unscaled_basis.m[0] = AttractToOne(unscaled_basis.m[0]);
unscaled_basis.m[5] = AttractToOne(unscaled_basis.m[5]);

Point unrounded_glyph_position =
// This is for RTL text.
unscaled_basis * glyph_bounds.GetLeftTop() +
Expand Down Expand Up @@ -231,12 +229,12 @@ void TextContents::ComputeVertexData(
bool TextContents::Render(const ContentContext& renderer,
const Entity& entity,
RenderPass& pass) const {
auto color = GetColor();
Color color = GetColor();
if (color.IsTransparent()) {
return true;
}

auto type = frame_->GetAtlasType();
GlyphAtlas::Type type = frame_->GetAtlasType();
const std::shared_ptr<GlyphAtlas>& atlas =
renderer.GetLazyGlyphAtlas()->CreateOrGetGlyphAtlas(
*renderer.GetContext(), renderer.GetTransientsBuffer(), type);
Expand Down Expand Up @@ -298,26 +296,45 @@ bool TextContents::Render(const ContentContext& renderer,
sampler_desc) // sampler
);

auto& host_buffer = renderer.GetTransientsBuffer();
size_t vertex_count = 0;
HostBuffer& host_buffer = renderer.GetTransientsBuffer();
size_t glyph_count = 0;
for (const auto& run : frame_->GetRuns()) {
vertex_count += run.GetGlyphPositions().size();
glyph_count += run.GetGlyphPositions().size();
}
vertex_count *= 6;
size_t vertex_count = glyph_count * 4;
size_t index_count = glyph_count * 6;

BufferView buffer_view = host_buffer.Emplace(
vertex_count * sizeof(VS::PerVertexData), alignof(VS::PerVertexData),
[&](uint8_t* contents) {
[&](uint8_t* data) {
VS::PerVertexData* vtx_contents =
reinterpret_cast<VS::PerVertexData*>(contents);
ComputeVertexData(vtx_contents, frame_, scale_,
/*entity_transform=*/entity_transform, offset_,
GetGlyphProperties(), atlas);
reinterpret_cast<VS::PerVertexData*>(data);
ComputeVertexData(/*vtx_contents=*/vtx_contents,
/*frame=*/frame_,
/*scale=*/scale_,
/*entity_transform=*/entity_transform,
/*offset=*/offset_,
/*glyph_properties=*/GetGlyphProperties(),
/*atlas=*/atlas);
});
BufferView index_buffer_view = host_buffer.Emplace(
index_count * sizeof(uint16_t), alignof(uint16_t), [&](uint8_t* data) {
uint16_t* indices = reinterpret_cast<uint16_t*>(data);
size_t j = 0;
for (auto i = 0u; i < glyph_count; i++) {

@gaaclarke gaaclarke Mar 31, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has to loop through everything twice. Why not calculate the index buffer in the ComputeVertexData's loop?

@jonahwilliams jonahwilliams Mar 31, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of the way the emplace callback works, we can't easy emplace into two buffers at once unless I copy everything. I think this is still OK.

size_t base = i * 4;
indices[j++] = base + 0;
indices[j++] = base + 1;
indices[j++] = base + 2;
indices[j++] = base + 1;
indices[j++] = base + 2;
indices[j++] = base + 3;
Comment on lines +326 to +331

@gaaclarke gaaclarke Mar 31, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Switching to triangle strip would be more efficient.

@jonahwilliams jonahwilliams Mar 31, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The glyph rects aren't connected, so switching to strip would be move overall geometry:

Before: 6 pts
This PR: 4 pts
w/ Strip: 4pts + 4 to break primitive

}
});

pass.SetVertexBuffer(std::move(buffer_view));
pass.SetIndexBuffer({}, IndexType::kNone);
pass.SetElementCount(vertex_count);
pass.SetIndexBuffer(index_buffer_view, IndexType::k16bit);
pass.SetElementCount(index_count);

return pass.Draw().ok();
}
Expand Down
71 changes: 36 additions & 35 deletions engine/src/flutter/impeller/entity/contents/text_contents_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,13 @@ std::shared_ptr<GlyphAtlas> CreateGlyphAtlas(
}

Rect PerVertexDataPositionToRect(
GlyphAtlasPipeline::VertexShader::PerVertexData data[6]) {
std::vector<GlyphAtlasPipeline::VertexShader::PerVertexData>::iterator
data) {
Scalar right = FLT_MIN;
Scalar left = FLT_MAX;
Scalar top = FLT_MAX;
Scalar bottom = FLT_MIN;
for (int i = 0; i < 6; ++i) {
for (int i = 0; i < 4; ++i) {
right = std::max(right, data[i].position.x);
left = std::min(left, data[i].position.x);
top = std::min(top, data[i].position.y);
Expand All @@ -85,13 +86,13 @@ Rect PerVertexDataPositionToRect(
}

Rect PerVertexDataUVToRect(
GlyphAtlasPipeline::VertexShader::PerVertexData data[6],
std::vector<GlyphAtlasPipeline::VertexShader::PerVertexData>::iterator data,
ISize texture_size) {
Scalar right = FLT_MIN;
Scalar left = FLT_MAX;
Scalar top = FLT_MAX;
Scalar bottom = FLT_MIN;
for (int i = 0; i < 6; ++i) {
for (int i = 0; i < 4; ++i) {
right = std::max(right, data[i].uv.x * texture_size.width);
left = std::min(left, data[i].uv.x * texture_size.width);
top = std::min(top, data[i].uv.y * texture_size.height);
Expand All @@ -111,7 +112,7 @@ TEST_P(TextContentsTest, SimpleComputeVertexData) {
GTEST_SKIP() << "Results aren't stable across linux and macos.";
#endif

GlyphAtlasPipeline::VertexShader::PerVertexData data[6];
std::vector<GlyphAtlasPipeline::VertexShader::PerVertexData> data(4);

std::shared_ptr<TextFrame> text_frame =
MakeTextFrame("1", "ahem.ttf", TextOptions{.font_size = 50});
Expand All @@ -128,13 +129,13 @@ TEST_P(TextContentsTest, SimpleComputeVertexData) {
atlas_context, text_frame, /*offset=*/{0, 0});

ISize texture_size = atlas->GetTexture()->GetSize();
TextContents::ComputeVertexData(data, text_frame, /*scale=*/1.0,
TextContents::ComputeVertexData(data.data(), text_frame, /*scale=*/1.0,
/*entity_transform=*/Matrix(),
/*offset=*/Vector2(0, 0),
/*glyph_properties=*/std::nullopt, atlas);

Rect position_rect = PerVertexDataPositionToRect(data);
Rect uv_rect = PerVertexDataUVToRect(data, texture_size);
Rect position_rect = PerVertexDataPositionToRect(data.begin());
Rect uv_rect = PerVertexDataUVToRect(data.begin(), texture_size);
// The -1 offset comes from Skia in `ComputeGlyphSize`. So since the font size
// is 50, the math appears to be to get back a 50x50 rect and apply 1 pixel
// of padding.
Expand All @@ -147,8 +148,7 @@ TEST_P(TextContentsTest, SimpleComputeVertexData2x) {
GTEST_SKIP() << "Results aren't stable across linux and macos.";
#endif

GlyphAtlasPipeline::VertexShader::PerVertexData data[6];

std::vector<GlyphAtlasPipeline::VertexShader::PerVertexData> data(4);
std::shared_ptr<TextFrame> text_frame =
MakeTextFrame("1", "ahem.ttf", TextOptions{.font_size = 50});

Expand All @@ -166,15 +166,15 @@ TEST_P(TextContentsTest, SimpleComputeVertexData2x) {

ISize texture_size = atlas->GetTexture()->GetSize();
TextContents::ComputeVertexData(
data, text_frame, static_cast<Scalar>(font_scale),
data.data(), text_frame, static_cast<Scalar>(font_scale),
/*entity_transform=*/
Matrix::MakeScale({static_cast<Scalar>(font_scale),
static_cast<Scalar>(font_scale), 1}),
/*offset=*/Vector2(0, 0),
/*glyph_properties=*/std::nullopt, atlas);

Rect position_rect = PerVertexDataPositionToRect(data);
Rect uv_rect = PerVertexDataUVToRect(data, texture_size);
Rect position_rect = PerVertexDataPositionToRect(data.begin());
Rect uv_rect = PerVertexDataUVToRect(data.begin(), texture_size);
EXPECT_RECT_NEAR(position_rect, Rect::MakeXYWH(-1, -81, 102, 102));
EXPECT_RECT_NEAR(uv_rect, Rect::MakeXYWH(1.0, 1.0, 102, 102));
}
Expand All @@ -196,24 +196,25 @@ TEST_P(TextContentsTest, MaintainsShape) {
Rect uv_rect[2];

{
GlyphAtlasPipeline::VertexShader::PerVertexData data[12];
std::vector<GlyphAtlasPipeline::VertexShader::PerVertexData> data(12);

std::shared_ptr<GlyphAtlas> atlas =
CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
GlyphAtlas::Type::kAlphaBitmap, font_scale,
atlas_context, text_frame, /*offset=*/{0, 0});
ISize texture_size = atlas->GetTexture()->GetSize();

TextContents::ComputeVertexData(
data, text_frame, static_cast<Scalar>(font_scale),
data.data(), text_frame, static_cast<Scalar>(font_scale),
/*entity_transform=*/
Matrix::MakeScale({static_cast<Scalar>(font_scale),
static_cast<Scalar>(font_scale), 1}),
/*offset=*/Vector2(0, 0),
/*glyph_properties=*/std::nullopt, atlas);
position_rect[0] = PerVertexDataPositionToRect(data);
uv_rect[0] = PerVertexDataUVToRect(data, texture_size);
position_rect[1] = PerVertexDataPositionToRect(data + 6);
uv_rect[1] = PerVertexDataUVToRect(data + 6, texture_size);
position_rect[0] = PerVertexDataPositionToRect(data.begin());
uv_rect[0] = PerVertexDataUVToRect(data.begin(), texture_size);
position_rect[1] = PerVertexDataPositionToRect(data.begin() + 4);
uv_rect[1] = PerVertexDataUVToRect(data.begin() + 4, texture_size);
}
EXPECT_NEAR(GetAspectRatio(position_rect[1]), GetAspectRatio(uv_rect[1]),
0.001)
Expand All @@ -226,7 +227,7 @@ TEST_P(TextContentsTest, SimpleSubpixel) {
GTEST_SKIP() << "Results aren't stable across linux and macos.";
#endif

GlyphAtlasPipeline::VertexShader::PerVertexData data[6];
std::vector<GlyphAtlasPipeline::VertexShader::PerVertexData> data(4);

std::shared_ptr<TextFrame> text_frame = MakeTextFrame(
"1", "ahem.ttf", TextOptions{.font_size = 50, .is_subpixel = true});
Expand All @@ -245,12 +246,12 @@ TEST_P(TextContentsTest, SimpleSubpixel) {

ISize texture_size = atlas->GetTexture()->GetSize();
TextContents::ComputeVertexData(
data, text_frame, /*scale=*/1.0,
data.data(), text_frame, /*scale=*/1.0,
/*entity_transform=*/Matrix::MakeTranslation(offset), offset,
/*glyph_properties=*/std::nullopt, atlas);

Rect position_rect = PerVertexDataPositionToRect(data);
Rect uv_rect = PerVertexDataUVToRect(data, texture_size);
Rect position_rect = PerVertexDataPositionToRect(data.begin());
Rect uv_rect = PerVertexDataUVToRect(data.begin(), texture_size);
// The values at Point(0, 0).
// EXPECT_RECT_NEAR(position_rect, Rect::MakeXYWH(-1, -41, 52, 52));
// EXPECT_RECT_NEAR(uv_rect, Rect::MakeXYWH(1.0, 1.0, 52, 52));
Expand All @@ -263,7 +264,7 @@ TEST_P(TextContentsTest, SimpleSubpixel3x) {
GTEST_SKIP() << "Results aren't stable across linux and macos.";
#endif

GlyphAtlasPipeline::VertexShader::PerVertexData data[6];
std::vector<GlyphAtlasPipeline::VertexShader::PerVertexData> data(4);

std::shared_ptr<TextFrame> text_frame = MakeTextFrame(
"1", "ahem.ttf", TextOptions{.font_size = 50, .is_subpixel = true});
Expand All @@ -283,16 +284,16 @@ TEST_P(TextContentsTest, SimpleSubpixel3x) {

ISize texture_size = atlas->GetTexture()->GetSize();
TextContents::ComputeVertexData(
data, text_frame, static_cast<Scalar>(font_scale),
data.data(), text_frame, static_cast<Scalar>(font_scale),
/*entity_transform=*/
Matrix::MakeTranslation(offset) *
Matrix::MakeScale({static_cast<Scalar>(font_scale),
static_cast<Scalar>(font_scale), 1}),
offset,
/*glyph_properties=*/std::nullopt, atlas);

Rect position_rect = PerVertexDataPositionToRect(data);
Rect uv_rect = PerVertexDataUVToRect(data, texture_size);
Rect position_rect = PerVertexDataPositionToRect(data.begin());
Rect uv_rect = PerVertexDataUVToRect(data.begin(), texture_size);
// Values at Point(0, 0)
// EXPECT_RECT_NEAR(position_rect, Rect::MakeXYWH(-1, -121, 152, 152));
// EXPECT_RECT_NEAR(uv_rect, Rect::MakeXYWH(1.0, 1.0, 152, 152));
Expand All @@ -307,7 +308,7 @@ TEST_P(TextContentsTest, SimpleSubpixel26) {
GTEST_SKIP() << "Results aren't stable across linux and macos.";
#endif

GlyphAtlasPipeline::VertexShader::PerVertexData data[6];
std::vector<GlyphAtlasPipeline::VertexShader::PerVertexData> data(4);

std::shared_ptr<TextFrame> text_frame = MakeTextFrame(
"1", "ahem.ttf", TextOptions{.font_size = 50, .is_subpixel = true});
Expand All @@ -326,12 +327,12 @@ TEST_P(TextContentsTest, SimpleSubpixel26) {

ISize texture_size = atlas->GetTexture()->GetSize();
TextContents::ComputeVertexData(
data, text_frame, /*scale=*/1.0,
data.data(), text_frame, /*scale=*/1.0,
/*entity_transform=*/Matrix::MakeTranslation(offset), offset,
/*glyph_properties=*/std::nullopt, atlas);

Rect position_rect = PerVertexDataPositionToRect(data);
Rect uv_rect = PerVertexDataUVToRect(data, texture_size);
Rect position_rect = PerVertexDataPositionToRect(data.begin());
Rect uv_rect = PerVertexDataUVToRect(data.begin(), texture_size);
// The values without subpixel.
// EXPECT_RECT_NEAR(position_rect, Rect::MakeXYWH(-1, -41, 52, 52));
// EXPECT_RECT_NEAR(uv_rect, Rect::MakeXYWH(1.0, 1.0, 52, 52));
Expand All @@ -344,7 +345,7 @@ TEST_P(TextContentsTest, SimpleSubpixel80) {
GTEST_SKIP() << "Results aren't stable across linux and macos.";
#endif

GlyphAtlasPipeline::VertexShader::PerVertexData data[6];
std::vector<GlyphAtlasPipeline::VertexShader::PerVertexData> data(4);

std::shared_ptr<TextFrame> text_frame = MakeTextFrame(
"1", "ahem.ttf", TextOptions{.font_size = 50, .is_subpixel = true});
Expand All @@ -363,12 +364,12 @@ TEST_P(TextContentsTest, SimpleSubpixel80) {

ISize texture_size = atlas->GetTexture()->GetSize();
TextContents::ComputeVertexData(
data, text_frame, /*scale=*/1.0,
data.data(), text_frame, /*scale=*/1.0,
/*entity_transform=*/Matrix::MakeTranslation(offset), offset,
/*glyph_properties=*/std::nullopt, atlas);

Rect position_rect = PerVertexDataPositionToRect(data);
Rect uv_rect = PerVertexDataUVToRect(data, texture_size);
Rect position_rect = PerVertexDataPositionToRect(data.begin());
Rect uv_rect = PerVertexDataUVToRect(data.begin(), texture_size);
// The values without subpixel.
// EXPECT_RECT_NEAR(position_rect, Rect::MakeXYWH(-1, -41, 52, 52));
// EXPECT_RECT_NEAR(uv_rect, Rect::MakeXYWH(1.0, 1.0, 52, 52));
Expand Down