From c1939da9f8f58d7aa772f9f7fdfa810a4cd555d5 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Wed, 30 Nov 2022 21:08:10 -0800 Subject: [PATCH 1/3] [Impeller] Use glyph bounds instead of font metrics to compute per-glyph coverage --- impeller/entity/contents/text_contents.cc | 3 +-- .../backends/skia/text_frame_skia.cc | 25 ++++++++++++++++--- .../backends/skia/text_render_context_skia.cc | 7 +++--- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/impeller/entity/contents/text_contents.cc b/impeller/entity/contents/text_contents.cc index 1d6727d30565d..1f30c9815582c 100644 --- a/impeller/entity/contents/text_contents.cc +++ b/impeller/entity/contents/text_contents.cc @@ -131,8 +131,7 @@ static bool CommonRender( auto glyph_size_ = font.GetMetrics().GetBoundingBox().size; auto glyph_size = Point{static_cast(glyph_size_.width), static_cast(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/typographer/backends/skia/text_frame_skia.cc b/impeller/typographer/backends/skia/text_frame_skia.cc index 8b649c6c5efca..7fc4f5fd977bd 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 + #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(font.refTypefaceOrDefault()); SkFontMetrics sk_metrics; @@ -24,8 +29,20 @@ static Font ToFont(const SkFont& font, Scalar scale) { 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 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 TextFrameFromTextBlob(const sk_sp& blob, Scalar scale) { 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 1641171f56a75..58754115fa7c4 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 @@ static size_t PairsFitInAtlasOfSize(const FontGlyphPair::Vector& pairs, 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 @@ static std::shared_ptr CreateAtlasBitmap(const GlyphAtlas& atlas, &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; }); From a84eb9a1fca16a79b4e65190357e615691857155 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Wed, 30 Nov 2022 22:18:31 -0800 Subject: [PATCH 2/3] Add test --- impeller/aiks/aiks_unittests.cc | 37 ++++++++++++++++++++++++++++++++ impeller/fixtures/BUILD.gn | 1 + impeller/fixtures/wtf.otf | Bin 0 -> 2552 bytes 3 files changed, 38 insertions(+) create mode 100644 impeller/fixtures/wtf.otf diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 9b06d08802be8..14bdcacaba0ba 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -1118,6 +1118,43 @@ TEST_P(AiksTest, CanRenderTextInSaveLayer) { 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/fixtures/BUILD.gn b/impeller/fixtures/BUILD.gn index 30d609cd4f138..eb81d57237ccb 100644 --- a/impeller/fixtures/BUILD.gn +++ b/impeller/fixtures/BUILD.gn @@ -73,6 +73,7 @@ test_fixtures("file_fixtures") { "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 0000000000000000000000000000000000000000..ae0c62c61a728bbf4ae8952dc8134a727dc634ac GIT binary patch literal 2552 zcmdT`eQZ-z6hF6pukA+HF~*x!ALB9k7&>OxjIRNa57!vLjllRCh#Om3w`|>LyAQ@- z4rL5)#UUm#L<9pmL75p9L^k|G$V~JPG@59(3?v#yqA}pdd+mE$J+FP6<0~QYFK=`2 z`JHp`Irn!?@4b1sd3m`I3on2kge4A#P&;qyegL)|fB}ai@7cH7tKR{L=>q^hch!^j zb!|^K0Fbe$7Zg<#R&iB-PQtaBXj|!W78a*$zVs{3eSm$I3k^mm#s>Ha`(#&zuP&37 zlQ15}hs=t?x+;iUfsmMRFt`gVoShd2-$T6$KzFIC((4PKhYaM?g7X3pbtyo-HSX~9 z@$>(JG5P_tz|{}itWi1ehGLbM8m|~FsGC5iTCt&zUI@>Dd^Sa~Dpuo_NTldobc_Q) z9ykKS=twKDxc|$#F4X-|Nv=T#!FW1gV@4cRJ|k_#mh588)plLmE6QznC*5x#;7|3i z!0BNtEPw_L6SRM?VI3Gj)UY1X$a)PkFqX7yn1y)qgN6-Y(HZd{B#g?f9(xoM;NepZ z>oEQ=8rH))2y2)Dfi!EFg=BJG!v^5V4Ojw|PzBq-1Eo*`F7ROvsZa#dQQEpU>$L7FGLPl^$=pu&BITh%EF99;esosc{yg`&wjFh&-aq zZs!GSU4=f3rG)}miJO)n)pB(4pz18CE-ysjK;mu;qDl|0P*b!cIUC9>?8BSfzX0m0 z448r4j^l92z)|V;IVwFRPQjjT6S4%A&~4de2^ljo>@(D~|IOuo+eGpi?GCUeQcN%Y zZE6MrGPWTHmAU$ZHBOJWw9+luY#He`8ybc)d7yHs=PTNI?F24YPYv35d^Ru|-o5DU zn+uNX5Dufo7DeH&!UzvJlQn?zuy6b}+lqzO7HSua_cRI|Rz%nUW~@>1S-g*RGQ9zKuf0mkys zm%zhRUf($m7}eDT*{~Xls@sJ3J=30Fpcbi zIF(5X%I3R$K1hpdKfrIwSjg8?Pm zXpxuI5NTI1z%6`AnXP0|dU3t9O4&%MNN!3Kq{)3Cg~IJ?I`uVE zxlYkqYzSsBG7}IPqvZabTTsD=>J%5HJ#{>LMA?2%PCPrlmPmdSocy?)Hh5#OpVFJt zLUBWn;(E7O5Ihw;MX9`4TsZh;@EbJ^HRaHjv-;5fR8F)M$QENDc!n#MV>=DXG0{De z!AK?G6wi93GK@0r93MI?ZBnD)$*3htD~d=-meqvH0whR-J=|t7C4f~a=0v7LW(-nt zsEjLpLMh-wE%zwzb7BfkspZjPVr{gb!M0!%r}QgVJLrKiMzO3GPeSN*y z`b=Uwm7C?=Vzb=L%FSXkXZickfY>-7O%n}7dQ6PP3dHC`KlgL8PW+C2=}7&-T56JK z8|urN%3pGu#5uHAZXDb}`@}|Z0cD%s-FdR^v`Nl1)Y5G`ch{L@M($;scf7Lw^~M8@ zZ7&|8reK)*DSQ0r(GwHtYkLp0>^CXiV2G0wrc99&CejUWZaT5Wzt`WgZ;!Z4_O6#x zyXjYSxwu%IDK7Q?bjJzX1ea`W?ZCt5-c z<9X&BSNnFbiCzrtGEj32HOB^GJ6g@=&M}>@n&a>yCaQOOGM?L+%KUp27SHWmX%?1M q{nD|kUljTQ#Tu3RfhZn|yW0*KYzpkd8>ha>(7*hTTdExWo%9cet*_hw literal 0 HcmV?d00001 From d7dce3566f13eeeb2034494177fa77df505a9a84 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Thu, 1 Dec 2022 00:26:40 -0800 Subject: [PATCH 3/3] Fix test --- impeller/typographer/typographer_unittests.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc index 7bd81264c4df7..51d5dfe9b1587 100644 --- a/impeller/typographer/typographer_unittests.cc +++ b/impeller/typographer/typographer_unittests.cc @@ -110,7 +110,7 @@ TEST_P(TypographerTest, GlyphAtlasWithOddUniqueGlyphSize) { // 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 @@ TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) { 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); }