From 4482584842d6a9c66ae2993996c7075bb068f82f Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Wed, 25 Sep 2024 13:22:45 -0700
Subject: [PATCH 1/2] [Impeller] avoid reading font while parsing sktextblob.

---
 .../backends/skia/text_frame_skia.cc          | 55 ++++++-------------
 1 file changed, 18 insertions(+), 37 deletions(-)

diff --git a/impeller/typographer/backends/skia/text_frame_skia.cc b/impeller/typographer/backends/skia/text_frame_skia.cc
index 55f8ad9084c1e..e032bca038ca5 100644
--- a/impeller/typographer/backends/skia/text_frame_skia.cc
+++ b/impeller/typographer/backends/skia/text_frame_skia.cc
@@ -19,25 +19,6 @@
 
 namespace impeller {
 
-/// @brief Convert a Skia axis alignment into an Impeller alignment.
-///
-///        This does not include a case for AxisAlignment::kNone, that should
-///        be used if SkFont::isSubpixel is false.
-static AxisAlignment ToAxisAligment(SkAxisAlignment aligment) {
-  switch (aligment) {
-    case SkAxisAlignment::kNone:
-      // Skia calls this case none, meaning alignment in both X and Y.
-      // Impeller will call it "all" since that is less confusing. "none"
-      // is reserved for no subpixel alignment.
-      return AxisAlignment::kAll;
-    case SkAxisAlignment::kX:
-      return AxisAlignment::kX;
-    case SkAxisAlignment::kY:
-      return AxisAlignment::kY;
-  }
-  FML_UNREACHABLE();
-}
-
 static Font ToFont(const SkTextBlobRunIterator& run, AxisAlignment alignment) {
   auto& font = run.font();
   auto typeface = std::make_shared<TypefaceSkia>(font.refTypeface());
@@ -63,35 +44,35 @@ std::shared_ptr<TextFrame> MakeTextFrameFromTextBlobSkia(
   bool has_color = false;
   std::vector<TextRun> runs;
   for (SkTextBlobRunIterator run(blob.get()); !run.done(); run.next()) {
-    // TODO(112005): Ask Skia for a public API to look this up. This is using a
-    // private API today.
     SkStrikeSpec strikeSpec = SkStrikeSpec::MakeWithNoDevice(run.font());
     SkBulkGlyphMetricsAndPaths paths{strikeSpec};
+    SkSpan<const SkGlyph*> glyphs =
+        paths.glyphs(SkSpan(run.glyphs(), run.glyphCount()));
+
+    for (const auto& glyph : glyphs) {
+      has_color |= glyph->isColor();
+    }
+
     AxisAlignment alignment = AxisAlignment::kNone;
-    if (run.font().isSubpixel()) {
-      alignment = ToAxisAligment(
-          strikeSpec.createScalerContext()->computeAxisAlignmentForHText());
+    if (run.font().isSubpixel() && !run.font().isBaselineSnap() && !has_color) {
+      alignment = AxisAlignment::kX;
     }
 
-    const auto glyph_count = run.glyphCount();
-    const auto* glyphs = run.glyphs();
     switch (run.positioning()) {
       case SkTextBlobRunIterator::kFull_Positioning: {
         std::vector<TextRun::GlyphPosition> positions;
-        positions.reserve(glyph_count);
-        for (auto i = 0u; i < glyph_count; i++) {
+        positions.reserve(run.glyphCount());
+        for (auto i = 0u; i < run.glyphCount(); i++) {
           // kFull_Positioning has two scalars per glyph.
           const SkPoint* glyph_points = run.points();
           const SkPoint* point = glyph_points + i;
-          Glyph::Type type = paths.glyph(glyphs[i])->isColor()
-                                 ? Glyph::Type::kBitmap
-                                 : Glyph::Type::kPath;
-          has_color |= type == Glyph::Type::kBitmap;
-          positions.emplace_back(
-              TextRun::GlyphPosition{Glyph{glyphs[i], type}, Point{
-                                                                 point->x(),
-                                                                 point->y(),
-                                                             }});
+          Glyph::Type type =
+              glyphs[i]->isColor() ? Glyph::Type::kBitmap : Glyph::Type::kPath;
+          positions.emplace_back(TextRun::GlyphPosition{
+              Glyph{glyphs[i]->getGlyphID(), type}, Point{
+                                                        point->x(),
+                                                        point->y(),
+                                                    }});
         }
         TextRun text_run(ToFont(run, alignment), positions);
         runs.emplace_back(text_run);

From b1deee8d5b2237aa1a8c5dd934df76d2f2880354 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Wed, 25 Sep 2024 15:27:40 -0700
Subject: [PATCH 2/2] ++

---
 impeller/typographer/backends/skia/text_frame_skia.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/impeller/typographer/backends/skia/text_frame_skia.cc b/impeller/typographer/backends/skia/text_frame_skia.cc
index e032bca038ca5..38e81acd04442 100644
--- a/impeller/typographer/backends/skia/text_frame_skia.cc
+++ b/impeller/typographer/backends/skia/text_frame_skia.cc
@@ -54,7 +54,7 @@ std::shared_ptr<TextFrame> MakeTextFrameFromTextBlobSkia(
     }
 
     AxisAlignment alignment = AxisAlignment::kNone;
-    if (run.font().isSubpixel() && !run.font().isBaselineSnap() && !has_color) {
+    if (run.font().isSubpixel() && run.font().isBaselineSnap() && !has_color) {
       alignment = AxisAlignment::kX;
     }