[Impeller] Use glyph bounds to compute the max glyph size instead of font metrics (#37998)
diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc
index 9b06d08..14bdcac 100644
--- a/impeller/aiks/aiks_unittests.cc
+++ b/impeller/aiks/aiks_unittests.cc
@@ -1118,6 +1118,43 @@
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
}
+TEST_P(AiksTest, CanRenderTextOutsideBoundaries) {
+ Canvas canvas;
+ canvas.Translate({200, 150});
+
+ // Construct the text blob.
+ auto mapping = OpenFixtureAsSkData("wtf.otf");
+ ASSERT_NE(mapping, nullptr);
+
+ Scalar font_size = 80;
+ SkFont sk_font(SkTypeface::MakeFromData(mapping), font_size);
+
+ Paint text_paint;
+ text_paint.color = Color::White().WithAlpha(0.8);
+
+ struct {
+ Point position;
+ const char* text;
+ } text[] = {{Point(0, 0), "0F0F0F0"},
+ {Point(1, 2), "789"},
+ {Point(1, 3), "456"},
+ {Point(1, 4), "123"},
+ {Point(0, 6), "0F0F0F0"}};
+ for (auto& t : text) {
+ canvas.Save();
+ canvas.Translate(t.position * Point(font_size * 2, font_size * 1.1));
+ {
+ auto blob = SkTextBlob::MakeFromString(t.text, sk_font);
+ ASSERT_NE(blob, nullptr);
+ auto frame = TextFrameFromTextBlob(blob);
+ canvas.DrawTextFrame(frame, Point(), text_paint);
+ }
+ canvas.Restore();
+ }
+
+ ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
+}
+
TEST_P(AiksTest, CanDrawPaint) {
Paint paint;
paint.color = Color::MediumTurquoise();
diff --git a/impeller/entity/contents/text_contents.cc b/impeller/entity/contents/text_contents.cc
index 1d6727d..1f30c98 100644
--- a/impeller/entity/contents/text_contents.cc
+++ b/impeller/entity/contents/text_contents.cc
@@ -131,8 +131,7 @@
auto glyph_size_ = font.GetMetrics().GetBoundingBox().size;
auto glyph_size = Point{static_cast<Scalar>(glyph_size_.width),
static_cast<Scalar>(glyph_size_.height)};
- auto metrics_offset =
- Point{font.GetMetrics().min_extent.x, font.GetMetrics().ascent};
+ auto metrics_offset = font.GetMetrics().min_extent;
for (const auto& glyph_position : run.GetGlyphPositions()) {
FontGlyphPair font_glyph_pair{font, glyph_position.glyph};
diff --git a/impeller/fixtures/BUILD.gn b/impeller/fixtures/BUILD.gn
index 30d609c..eb81d57 100644
--- a/impeller/fixtures/BUILD.gn
+++ b/impeller/fixtures/BUILD.gn
@@ -73,6 +73,7 @@
"table_mountain_pz.png",
"test_texture.frag",
"types.h",
+ "wtf.otf",
]
if (host_os == "mac") {
fixtures += [ "/System/Library/Fonts/Apple Color Emoji.ttc" ]
diff --git a/impeller/fixtures/wtf.otf b/impeller/fixtures/wtf.otf
new file mode 100644
index 0000000..ae0c62c
--- /dev/null
+++ b/impeller/fixtures/wtf.otf
Binary files differ
diff --git a/impeller/typographer/backends/skia/text_frame_skia.cc b/impeller/typographer/backends/skia/text_frame_skia.cc
index 8b649c6..7fc4f5f 100644
--- a/impeller/typographer/backends/skia/text_frame_skia.cc
+++ b/impeller/typographer/backends/skia/text_frame_skia.cc
@@ -4,8 +4,12 @@
#include "impeller/typographer/backends/skia/text_frame_skia.h"
+#include <vector>
+
#include "flutter/fml/logging.h"
#include "impeller/typographer/backends/skia/typeface_skia.h"
+#include "include/core/SkFontTypes.h"
+#include "include/core/SkRect.h"
#include "third_party/skia/include/core/SkFont.h"
#include "third_party/skia/include/core/SkFontMetrics.h"
#include "third_party/skia/src/core/SkStrikeSpec.h" // nogncheck
@@ -13,7 +17,8 @@
namespace impeller {
-static Font ToFont(const SkFont& font, Scalar scale) {
+static Font ToFont(const SkTextBlobRunIterator& run, Scalar scale) {
+ auto& font = run.font();
auto typeface = std::make_shared<TypefaceSkia>(font.refTypefaceOrDefault());
SkFontMetrics sk_metrics;
@@ -24,8 +29,20 @@
metrics.point_size = font.getSize();
metrics.ascent = sk_metrics.fAscent;
metrics.descent = sk_metrics.fDescent;
- metrics.min_extent = {sk_metrics.fXMin, sk_metrics.fAscent};
- metrics.max_extent = {sk_metrics.fXMax, sk_metrics.fDescent};
+ metrics.min_extent = {sk_metrics.fXMin, sk_metrics.fTop};
+ metrics.max_extent = {sk_metrics.fXMax, sk_metrics.fBottom};
+
+ std::vector<SkRect> glyph_bounds;
+ SkPaint paint;
+
+ glyph_bounds.resize(run.glyphCount());
+ run.font().getBounds(run.glyphs(), run.glyphCount(), glyph_bounds.data(),
+ nullptr);
+ for (auto& bounds : glyph_bounds) {
+ metrics.min_extent = metrics.min_extent.Min({bounds.fLeft, bounds.fTop});
+ metrics.max_extent =
+ metrics.max_extent.Max({bounds.fRight, bounds.fBottom});
+ }
return Font{std::move(typeface), metrics};
}
@@ -38,7 +55,7 @@
TextFrame frame;
for (SkTextBlobRunIterator run(blob.get()); !run.done(); run.next()) {
- TextRun text_run(ToFont(run.font(), scale));
+ TextRun text_run(ToFont(run, scale));
// TODO(jonahwilliams): ask Skia for a public API to look this up.
// https://github.com/flutter/flutter/issues/112005
diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc
index 1641171..5875411 100644
--- a/impeller/typographer/backends/skia/text_render_context_skia.cc
+++ b/impeller/typographer/backends/skia/text_render_context_skia.cc
@@ -77,6 +77,7 @@
for (size_t i = 0; i < pairs.size(); i++) {
const auto& pair = pairs[i];
+
const auto glyph_size =
ISize::Ceil(pair.font.GetMetrics().GetBoundingBox().size *
pair.font.GetMetrics().scale);
@@ -291,9 +292,9 @@
&glyph_id, // glyphs
&position, // positions
SkPoint::Make(-metrics.min_extent.x,
- -metrics.ascent), // origin
- sk_font, // font
- glyph_paint // paint
+ -metrics.min_extent.y), // origin
+ sk_font, // font
+ glyph_paint // paint
);
return true;
});
diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc
index 7bd8126..51d5dfe 100644
--- a/impeller/typographer/typographer_unittests.cc
+++ b/impeller/typographer/typographer_unittests.cc
@@ -110,7 +110,7 @@
// The 3 unique glyphs should not evenly fit into a square texture, so we
// should have a rectangular one.
- ASSERT_EQ(atlas->GetTexture()->GetSize().width * 2,
+ ASSERT_EQ(atlas->GetTexture()->GetSize().width,
atlas->GetTexture()->GetSize().height);
}
@@ -166,7 +166,7 @@
ASSERT_NE(atlas, nullptr);
ASSERT_NE(atlas->GetTexture(), nullptr);
- ASSERT_EQ(atlas->GetTexture()->GetSize().width * 2,
+ ASSERT_EQ(atlas->GetTexture()->GetSize().width,
atlas->GetTexture()->GetSize().height);
}
