Skip to content

Commit 3c5ae1e

Browse files
authored
Fixed the text aspect ratio (#162415)
fixes flutter/flutter#162348 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
1 parent 1902fb2 commit 3c5ae1e

File tree

2 files changed

+49
-8
lines changed

2 files changed

+49
-8
lines changed

engine/src/flutter/impeller/entity/contents/text_contents.cc

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@
1818
#include "impeller/typographer/glyph_atlas.h"
1919

2020
namespace impeller {
21+
Point SizeToPoint(Size size) {
22+
return Point(size.width, size.height);
23+
}
2124

2225
using VS = GlyphAtlasPipeline::VertexShader;
2326
using FS = GlyphAtlasPipeline::FragmentShader;
@@ -167,9 +170,8 @@ void TextContents::ComputeVertexData(
167170
// glyph bounds are used to compute UVs in cases where the
168171
// destination and source sizes may differ due to clamping the sizes
169172
// of large glyphs.
170-
Point uv_origin =
171-
(atlas_glyph_bounds.GetLeftTop() - Point(0.5, 0.5)) / atlas_size;
172-
Point uv_size = (atlas_glyph_bounds.GetSize() + Point(1, 1)) / atlas_size;
173+
Point uv_origin = (atlas_glyph_bounds.GetLeftTop()) / atlas_size;
174+
Point uv_size = SizeToPoint(atlas_glyph_bounds.GetSize()) / atlas_size;
173175

174176
Point unrounded_glyph_position =
175177
// This is for RTL text.

engine/src/flutter/impeller/entity/contents/text_contents_unittests.cc

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,9 @@ Rect PerVertexDataUVToRect(
8888
return Rect::MakeLTRB(left, top, right, bottom);
8989
}
9090

91+
double GetAspectRatio(Rect rect) {
92+
return static_cast<double>(rect.GetWidth()) / rect.GetHeight();
93+
}
9194
} // namespace
9295

9396
TEST_P(TextContentsTest, SimpleComputeVertexData) {
@@ -123,10 +126,7 @@ TEST_P(TextContentsTest, SimpleComputeVertexData) {
123126
// is 50, the math appears to be to get back a 50x50 rect and apply 1 pixel
124127
// of padding.
125128
EXPECT_RECT_NEAR(position_rect, Rect::MakeXYWH(-1, -41, 52, 52));
126-
// (0.5, 0.5) gets us sampling from the exact middle of the first pixel, the
127-
// extra width takes us 0.5 past the end of the glyph too to sample fully the
128-
// last pixel.
129-
EXPECT_RECT_NEAR(uv_rect, Rect::MakeXYWH(0.5, 0.5, 53, 53));
129+
EXPECT_RECT_NEAR(uv_rect, Rect::MakeXYWH(1.0, 1.0, 52, 52));
130130
}
131131

132132
TEST_P(TextContentsTest, SimpleComputeVertexData2x) {
@@ -160,7 +160,46 @@ TEST_P(TextContentsTest, SimpleComputeVertexData2x) {
160160
Rect position_rect = PerVertexDataPositionToRect(data);
161161
Rect uv_rect = PerVertexDataUVToRect(data, texture_size);
162162
EXPECT_RECT_NEAR(position_rect, Rect::MakeXYWH(-1, -81, 102, 102));
163-
EXPECT_RECT_NEAR(uv_rect, Rect::MakeXYWH(0.5, 0.5, 103, 103));
163+
EXPECT_RECT_NEAR(uv_rect, Rect::MakeXYWH(1.0, 1.0, 102, 102));
164+
}
165+
166+
TEST_P(TextContentsTest, MaintainsShape) {
167+
std::shared_ptr<TextFrame> text_frame =
168+
MakeTextFrame("th", "ahem.ttf", /*font_size=*/50);
169+
170+
std::shared_ptr<TypographerContext> context = TypographerContextSkia::Make();
171+
std::shared_ptr<GlyphAtlasContext> atlas_context =
172+
context->CreateGlyphAtlasContext(GlyphAtlas::Type::kAlphaBitmap);
173+
std::shared_ptr<HostBuffer> host_buffer = HostBuffer::Create(
174+
GetContext()->GetResourceAllocator(), GetContext()->GetIdleWaiter());
175+
ASSERT_TRUE(context && context->IsValid());
176+
for (int i = 0; i <= 1000; ++i) {
177+
Scalar font_scale = 0.440 + (i / 1000.0);
178+
Rect position_rect[2];
179+
Rect uv_rect[2];
180+
181+
{
182+
GlyphAtlasPipeline::VertexShader::PerVertexData data[12];
183+
std::shared_ptr<GlyphAtlas> atlas =
184+
CreateGlyphAtlas(*GetContext(), context.get(), *host_buffer,
185+
GlyphAtlas::Type::kAlphaBitmap, font_scale,
186+
atlas_context, text_frame);
187+
ISize texture_size = atlas->GetTexture()->GetSize();
188+
189+
TextContents::ComputeVertexData(
190+
data, text_frame, font_scale,
191+
/*entity_transform=*/Matrix::MakeScale({font_scale, font_scale, 1}),
192+
/*offset=*/Vector2(0, 0),
193+
/*glyph_properties=*/std::nullopt, atlas);
194+
position_rect[0] = PerVertexDataPositionToRect(data);
195+
uv_rect[0] = PerVertexDataUVToRect(data, texture_size);
196+
position_rect[1] = PerVertexDataPositionToRect(data + 6);
197+
uv_rect[1] = PerVertexDataUVToRect(data + 6, texture_size);
198+
}
199+
EXPECT_NEAR(GetAspectRatio(position_rect[1]), GetAspectRatio(uv_rect[1]),
200+
0.001)
201+
<< i;
202+
}
164203
}
165204

166205
} // namespace testing

0 commit comments

Comments
 (0)