From 8a4a961554cd1dc4cfc6c5b6e23d50ad577a33e8 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Wed, 14 Sep 2022 20:21:48 -0700
Subject: [PATCH 1/6] [Impeller] add support for COLR v0 and v1 fonts

---
 impeller/entity/contents/text_contents.cc     | 12 ++++++---
 impeller/entity/shaders/glyph_atlas.frag      | 27 +++++++++++++------
 impeller/entity/shaders/glyph_atlas.vert      |  6 -----
 .../backends/skia/text_render_context_skia.cc | 26 +++++++++---------
 .../backends/skia/typeface_skia.cc            | 18 +++++++++++++
 .../typographer/backends/skia/typeface_skia.h |  3 +++
 impeller/typographer/font.cc                  |  1 +
 impeller/typographer/font.h                   |  3 +++
 impeller/typographer/glyph_atlas.cc           |  5 ++++
 impeller/typographer/glyph_atlas.h            |  3 +++
 impeller/typographer/typeface.h               |  8 ++++++
 11 files changed, 82 insertions(+), 30 deletions(-)

diff --git a/impeller/entity/contents/text_contents.cc b/impeller/entity/contents/text_contents.cc
index 3768dc05efe15..ad14ccf6b8208 100644
--- a/impeller/entity/contents/text_contents.cc
+++ b/impeller/entity/contents/text_contents.cc
@@ -89,16 +89,20 @@ bool TextContents::Render(const ContentContext& renderer,
   VS::FrameInfo frame_info;
   frame_info.mvp = Matrix::MakeOrthographic(pass.GetRenderTargetSize()) *
                    entity.GetTransformation();
-  frame_info.atlas_size =
-      Point{static_cast<Scalar>(atlas->GetTexture()->GetSize().width),
-            static_cast<Scalar>(atlas->GetTexture()->GetSize().height)};
-  frame_info.text_color = ToVector(color_.Premultiply());
   VS::BindFrameInfo(cmd, pass.GetTransientsBuffer().EmplaceUniform(frame_info));
 
   SamplerDescriptor sampler_desc;
   sampler_desc.min_filter = MinMagFilter::kLinear;
   sampler_desc.mag_filter = MinMagFilter::kLinear;
 
+  FS::FragInfo frag_info;
+  frag_info.font_has_color = atlas->HasColor() ? 1.0 : 0.0;
+  frag_info.text_color = ToVector(color_.Premultiply());
+  frag_info.atlas_size =
+      Point{static_cast<Scalar>(atlas->GetTexture()->GetSize().width),
+            static_cast<Scalar>(atlas->GetTexture()->GetSize().height)};
+  FS::BindFragInfo(cmd, pass.GetTransientsBuffer().EmplaceUniform(frag_info));
+
   // Common fragment uniforms for all glyphs.
   FS::BindGlyphAtlasSampler(
       cmd,                  // command
diff --git a/impeller/entity/shaders/glyph_atlas.frag b/impeller/entity/shaders/glyph_atlas.frag
index f41d9a181d899..ebfc4f0ff61ee 100644
--- a/impeller/entity/shaders/glyph_atlas.frag
+++ b/impeller/entity/shaders/glyph_atlas.frag
@@ -4,20 +4,31 @@
 
 uniform sampler2D glyph_atlas_sampler;
 
+uniform FragInfo {
+  vec2 atlas_size;
+  vec4 text_color;
+  float font_has_color;
+} frag_info;
+
 in vec2 v_unit_vertex;
 in vec2 v_atlas_position;
 in vec2 v_atlas_glyph_size;
-in vec2 v_atlas_size;
-in vec4 v_text_color;
 
 out vec4 frag_color;
 
 void main() {
-  vec2 scale_perspective = v_atlas_glyph_size / v_atlas_size;
-  vec2 offset = v_atlas_position / v_atlas_size;
+  vec2 scale_perspective = v_atlas_glyph_size / frag_info.atlas_size;
+  vec2 offset = v_atlas_position / frag_info.atlas_size;
 
-  frag_color = texture(
-    glyph_atlas_sampler,
-    v_unit_vertex * scale_perspective + offset
-  ).aaaa * v_text_color;
+  if (frag_info.font_has_color == 1.0) {
+    frag_color = texture(
+      glyph_atlas_sampler,
+      v_unit_vertex * scale_perspective + offset
+    ) * frag_info.text_color;
+  } else {
+    frag_color = texture(
+      glyph_atlas_sampler,
+      v_unit_vertex * scale_perspective + offset
+    ).aaaa * frag_info.text_color;
+  }
 }
diff --git a/impeller/entity/shaders/glyph_atlas.vert b/impeller/entity/shaders/glyph_atlas.vert
index 1e1b04b637fc4..05de00251c312 100644
--- a/impeller/entity/shaders/glyph_atlas.vert
+++ b/impeller/entity/shaders/glyph_atlas.vert
@@ -4,8 +4,6 @@
 
 uniform FrameInfo {
   mat4 mvp;
-  vec2 atlas_size;
-  vec4 text_color;
 } frame_info;
 
 in vec2 unit_vertex;
@@ -17,8 +15,6 @@ in vec2 atlas_glyph_size;
 out vec2 v_unit_vertex;
 out vec2 v_atlas_position;
 out vec2 v_atlas_glyph_size;
-out vec2 v_atlas_size;
-out vec4 v_text_color;
 
 void main() {
   vec4 translate = frame_info.mvp[0] * glyph_position.x
@@ -40,6 +36,4 @@ void main() {
   v_unit_vertex = unit_vertex;
   v_atlas_position = atlas_position;
   v_atlas_glyph_size = atlas_glyph_size;
-  v_atlas_size = frame_info.atlas_size;
-  v_text_color = frame_info.text_color;
 }
diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc
index aab49ed2fece9..6738bb58a0a5e 100644
--- a/impeller/typographer/backends/skia/text_render_context_skia.cc
+++ b/impeller/typographer/backends/skia/text_render_context_skia.cc
@@ -107,7 +107,9 @@ static std::shared_ptr<SkBitmap> CreateAtlasBitmap(const GlyphAtlas& atlas,
                                                    size_t atlas_size) {
   TRACE_EVENT0("impeller", __FUNCTION__);
   auto bitmap = std::make_shared<SkBitmap>();
-  auto image_info = SkImageInfo::MakeA8(atlas_size, atlas_size);
+  auto image_info = atlas.HasColor()
+                        ? SkImageInfo::MakeN32Premul(atlas_size, atlas_size)
+                        : SkImageInfo::MakeA8(atlas_size, atlas_size);
   if (!bitmap->tryAllocPixels(image_info)) {
     return nullptr;
   }
@@ -122,24 +124,21 @@ static std::shared_ptr<SkBitmap> CreateAtlasBitmap(const GlyphAtlas& atlas,
 
   atlas.IterateGlyphs([canvas](const FontGlyphPair& font_glyph,
                                const Rect& location) -> bool {
-    const auto position =
-        SkPoint::Make(location.origin.x / font_glyph.font.GetMetrics().scale,
-                      location.origin.y / font_glyph.font.GetMetrics().scale);
+    const auto& metrics = font_glyph.font.GetMetrics();
+    const auto position = SkPoint::Make(location.origin.x / metrics.scale,
+                                        location.origin.y / metrics.scale);
     SkGlyphID glyph_id = font_glyph.glyph.index;
 
     SkFont sk_font(
         TypefaceSkia::Cast(*font_glyph.font.GetTypeface()).GetSkiaTypeface(),
-        font_glyph.font.GetMetrics().point_size);
-
-    const auto& metrics = font_glyph.font.GetMetrics();
+        metrics.point_size);
 
     auto glyph_color = SK_ColorWHITE;
 
     SkPaint glyph_paint;
     glyph_paint.setColor(glyph_color);
     canvas->resetMatrix();
-    canvas->scale(font_glyph.font.GetMetrics().scale,
-                  font_glyph.font.GetMetrics().scale);
+    canvas->scale(metrics.scale, metrics.scale);
     canvas->drawGlyphs(1u,         // count
                        &glyph_id,  // glyphs
                        &position,  // positions
@@ -157,7 +156,8 @@ static std::shared_ptr<SkBitmap> CreateAtlasBitmap(const GlyphAtlas& atlas,
 static std::shared_ptr<Texture> UploadGlyphTextureAtlas(
     std::shared_ptr<Allocator> allocator,
     std::shared_ptr<SkBitmap> bitmap,
-    size_t atlas_size) {
+    size_t atlas_size,
+    PixelFormat format) {
   TRACE_EVENT0("impeller", __FUNCTION__);
   if (!allocator) {
     return nullptr;
@@ -168,7 +168,7 @@ static std::shared_ptr<Texture> UploadGlyphTextureAtlas(
 
   TextureDescriptor texture_descriptor;
   texture_descriptor.storage_mode = StorageMode::kHostVisible;
-  texture_descriptor.format = PixelFormat::kA8UNormInt;
+  texture_descriptor.format = format;
   texture_descriptor.size = ISize::MakeWH(atlas_size, atlas_size);
 
   if (pixmap.rowBytes() * pixmap.height() !=
@@ -251,8 +251,10 @@ std::shared_ptr<GlyphAtlas> TextRenderContextSkia::CreateGlyphAtlas(
   // ---------------------------------------------------------------------------
   // Step 6: Upload the atlas as a texture.
   // ---------------------------------------------------------------------------
+  auto format = glyph_atlas->HasColor() ? PixelFormat::kR8G8B8A8UNormInt
+                                        : PixelFormat::kA8UNormInt;
   auto texture = UploadGlyphTextureAtlas(GetContext()->GetResourceAllocator(),
-                                         bitmap, atlas_size);
+                                         bitmap, atlas_size, format);
   if (!texture) {
     return nullptr;
   }
diff --git a/impeller/typographer/backends/skia/typeface_skia.cc b/impeller/typographer/backends/skia/typeface_skia.cc
index 552e159f1f824..19e72a69d5429 100644
--- a/impeller/typographer/backends/skia/typeface_skia.cc
+++ b/impeller/typographer/backends/skia/typeface_skia.cc
@@ -39,6 +39,24 @@ Rect TypefaceSkia::GetBoundingBox() const {
                         bounds.bottom());
 }
 
+static const SkFontTableTag kCOLRTableTag =
+    SkSetFourByteTag('C', 'O', 'L', 'R');
+
+bool TypefaceSkia::HasColor() const {
+  auto count = typeface_->countTables();
+  if (count == 0) {
+    return false;
+  }
+  std::vector<SkFontTableTag> tags(count);
+  typeface_->getTableTags(tags.data());
+  for (auto tag : tags) {
+    if (tag == kCOLRTableTag) {
+      return true;
+    }
+  }
+  return false;
+}
+
 const sk_sp<SkTypeface>& TypefaceSkia::GetSkiaTypeface() const {
   return typeface_;
 }
diff --git a/impeller/typographer/backends/skia/typeface_skia.h b/impeller/typographer/backends/skia/typeface_skia.h
index 6ac52c47761fe..058d649b99e5a 100644
--- a/impeller/typographer/backends/skia/typeface_skia.h
+++ b/impeller/typographer/backends/skia/typeface_skia.h
@@ -31,6 +31,9 @@ class TypefaceSkia final : public Typeface,
   // |Typeface|
   Rect GetBoundingBox() const override;
 
+  // |Typeface|
+  bool HasColor() const override;
+
   const sk_sp<SkTypeface>& GetSkiaTypeface() const;
 
  private:
diff --git a/impeller/typographer/font.cc b/impeller/typographer/font.cc
index f8287341a2f10..a0a546d027bef 100644
--- a/impeller/typographer/font.cc
+++ b/impeller/typographer/font.cc
@@ -12,6 +12,7 @@ Font::Font(std::shared_ptr<Typeface> typeface, Metrics metrics)
     return;
   }
   is_valid_ = true;
+  has_color_ = typeface_->HasColor();
 }
 
 Font::~Font() = default;
diff --git a/impeller/typographer/font.h b/impeller/typographer/font.h
index 2830900b7de9f..71fc577a6b1c1 100644
--- a/impeller/typographer/font.h
+++ b/impeller/typographer/font.h
@@ -99,12 +99,15 @@ class Font : public Comparable<Font> {
   // |Comparable<Font>|
   std::size_t GetHash() const override;
 
+  bool HasColor() const { return has_color_; }
+
   // |Comparable<Font>|
   bool IsEqual(const Font& other) const override;
 
  private:
   std::shared_ptr<Typeface> typeface_;
   Metrics metrics_ = {};
+  bool has_color_ = false;
   bool is_valid_ = false;
 };
 
diff --git a/impeller/typographer/glyph_atlas.cc b/impeller/typographer/glyph_atlas.cc
index 7ed4578bbd923..8dca18d1d798a 100644
--- a/impeller/typographer/glyph_atlas.cc
+++ b/impeller/typographer/glyph_atlas.cc
@@ -23,9 +23,14 @@ void GlyphAtlas::SetTexture(std::shared_ptr<Texture> texture) {
 }
 
 void GlyphAtlas::AddTypefaceGlyphPosition(FontGlyphPair pair, Rect rect) {
+  has_color_ = pair.font.HasColor();
   positions_[pair] = rect;
 }
 
+bool GlyphAtlas::HasColor() const {
+  return has_color_;
+}
+
 std::optional<Rect> GlyphAtlas::FindFontGlyphPosition(
     const FontGlyphPair& pair) const {
   auto found = positions_.find(pair);
diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h
index 896ca7c8f2d6e..35622cb9bc122 100644
--- a/impeller/typographer/glyph_atlas.h
+++ b/impeller/typographer/glyph_atlas.h
@@ -32,6 +32,8 @@ class GlyphAtlas {
 
   bool IsValid() const;
 
+  bool HasColor() const;
+
   //----------------------------------------------------------------------------
   /// @brief      Set the texture for the glyph atlas.
   ///
@@ -86,6 +88,7 @@ class GlyphAtlas {
 
  private:
   std::shared_ptr<Texture> texture_;
+  bool has_color_ = false;
 
   std::unordered_map<FontGlyphPair,
                      Rect,
diff --git a/impeller/typographer/typeface.h b/impeller/typographer/typeface.h
index f3a4fe265af7f..65c0ed4aebe29 100644
--- a/impeller/typographer/typeface.h
+++ b/impeller/typographer/typeface.h
@@ -33,6 +33,14 @@ class Typeface : public Comparable<Typeface> {
   ///
   virtual Rect GetBoundingBox() const = 0;
 
+  //----------------------------------------------------------------------------
+  /// @brief      Indicates the presence of a COLR table in the typeface. This
+  ///             implies that an R8G8B8A8 atlas must be used.
+  ///
+  /// @return     Whether the typeface contains a COLR table.
+  ///
+  virtual bool HasColor() const = 0;
+
  private:
   FML_DISALLOW_COPY_AND_ASSIGN(Typeface);
 };

From e0944c5a7650e8e1129b90787e55821962b82d27 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Thu, 15 Sep 2022 17:15:32 -0700
Subject: [PATCH 2/6] add emoji support

---
 impeller/entity/contents/text_contents.cc     |  1 -
 impeller/entity/shaders/glyph_atlas.frag      | 15 +++------
 .../backends/skia/text_render_context_skia.cc | 31 ++++++++++++++-----
 .../backends/skia/typeface_skia.cc            | 18 -----------
 .../typographer/backends/skia/typeface_skia.h |  3 --
 impeller/typographer/font.cc                  |  5 ++-
 impeller/typographer/font.h                   |  3 +-
 impeller/typographer/glyph_atlas.cc           |  5 ---
 impeller/typographer/glyph_atlas.h            |  3 --
 impeller/typographer/typeface.h               |  9 ------
 10 files changed, 33 insertions(+), 60 deletions(-)

diff --git a/impeller/entity/contents/text_contents.cc b/impeller/entity/contents/text_contents.cc
index ad14ccf6b8208..c876f206fa982 100644
--- a/impeller/entity/contents/text_contents.cc
+++ b/impeller/entity/contents/text_contents.cc
@@ -96,7 +96,6 @@ bool TextContents::Render(const ContentContext& renderer,
   sampler_desc.mag_filter = MinMagFilter::kLinear;
 
   FS::FragInfo frag_info;
-  frag_info.font_has_color = atlas->HasColor() ? 1.0 : 0.0;
   frag_info.text_color = ToVector(color_.Premultiply());
   frag_info.atlas_size =
       Point{static_cast<Scalar>(atlas->GetTexture()->GetSize().width),
diff --git a/impeller/entity/shaders/glyph_atlas.frag b/impeller/entity/shaders/glyph_atlas.frag
index ebfc4f0ff61ee..ec9ebc47716b6 100644
--- a/impeller/entity/shaders/glyph_atlas.frag
+++ b/impeller/entity/shaders/glyph_atlas.frag
@@ -20,15 +20,8 @@ void main() {
   vec2 scale_perspective = v_atlas_glyph_size / frag_info.atlas_size;
   vec2 offset = v_atlas_position / frag_info.atlas_size;
 
-  if (frag_info.font_has_color == 1.0) {
-    frag_color = texture(
-      glyph_atlas_sampler,
-      v_unit_vertex * scale_perspective + offset
-    ) * frag_info.text_color;
-  } else {
-    frag_color = texture(
-      glyph_atlas_sampler,
-      v_unit_vertex * scale_perspective + offset
-    ).aaaa * frag_info.text_color;
-  }
+  frag_color = texture(
+    glyph_atlas_sampler,
+    v_unit_vertex * scale_perspective + offset
+  ) * frag_info.text_color;
 }
diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc
index 6738bb58a0a5e..bbcae78e1f47f 100644
--- a/impeller/typographer/backends/skia/text_render_context_skia.cc
+++ b/impeller/typographer/backends/skia/text_render_context_skia.cc
@@ -15,8 +15,9 @@
 #include "third_party/skia/include/core/SkFontMetrics.h"
 #include "third_party/skia/include/core/SkRSXform.h"
 #include "third_party/skia/include/core/SkSurface.h"
-#include "third_party/skia/src/core/SkIPoint16.h"   //nogncheck
-#include "third_party/skia/src/gpu/GrRectanizer.h"  //nogncheck
+#include "third_party/skia/src/core/SkIPoint16.h"    //nogncheck
+#include "third_party/skia/src/core/SkStrikeSpec.h"  // nogncheck
+#include "third_party/skia/src/gpu/GrRectanizer.h"   //nogncheck
 
 namespace impeller {
 
@@ -104,10 +105,25 @@ static size_t OptimumAtlasSizeForFontGlyphPairs(
 }
 
 static std::shared_ptr<SkBitmap> CreateAtlasBitmap(const GlyphAtlas& atlas,
-                                                   size_t atlas_size) {
+                                                   size_t atlas_size,
+                                                   bool* has_color) {
   TRACE_EVENT0("impeller", __FUNCTION__);
   auto bitmap = std::make_shared<SkBitmap>();
-  auto image_info = atlas.HasColor()
+
+  atlas.IterateGlyphs([&has_color](const FontGlyphPair& font_glyph,
+                                   const Rect& location) -> bool {
+    SkFont sk_font(
+        TypefaceSkia::Cast(*font_glyph.font.GetTypeface()).GetSkiaTypeface(),
+        font_glyph.font.GetMetrics().point_size);
+
+    SkStrikeSpec strikeSpec = SkStrikeSpec::MakeWithNoDevice(sk_font);
+    SkBulkGlyphMetricsAndPaths paths{strikeSpec};
+    auto sk_glyph = paths.glyph(font_glyph.glyph.index);
+    *has_color |= sk_glyph->isColor();
+
+    return true;
+  });
+  auto image_info = *has_color
                         ? SkImageInfo::MakeN32Premul(atlas_size, atlas_size)
                         : SkImageInfo::MakeA8(atlas_size, atlas_size);
   if (!bitmap->tryAllocPixels(image_info)) {
@@ -243,7 +259,8 @@ std::shared_ptr<GlyphAtlas> TextRenderContextSkia::CreateGlyphAtlas(
   // ---------------------------------------------------------------------------
   // Step 5: Draw font-glyph pairs in the correct spot in the atlas.
   // ---------------------------------------------------------------------------
-  auto bitmap = CreateAtlasBitmap(*glyph_atlas, atlas_size);
+  bool has_color = false;
+  auto bitmap = CreateAtlasBitmap(*glyph_atlas, atlas_size, &has_color);
   if (!bitmap) {
     return nullptr;
   }
@@ -251,8 +268,8 @@ std::shared_ptr<GlyphAtlas> TextRenderContextSkia::CreateGlyphAtlas(
   // ---------------------------------------------------------------------------
   // Step 6: Upload the atlas as a texture.
   // ---------------------------------------------------------------------------
-  auto format = glyph_atlas->HasColor() ? PixelFormat::kR8G8B8A8UNormInt
-                                        : PixelFormat::kA8UNormInt;
+  auto format =
+      has_color ? PixelFormat::kR8G8B8A8UNormInt : PixelFormat::kA8UNormInt;
   auto texture = UploadGlyphTextureAtlas(GetContext()->GetResourceAllocator(),
                                          bitmap, atlas_size, format);
   if (!texture) {
diff --git a/impeller/typographer/backends/skia/typeface_skia.cc b/impeller/typographer/backends/skia/typeface_skia.cc
index 19e72a69d5429..552e159f1f824 100644
--- a/impeller/typographer/backends/skia/typeface_skia.cc
+++ b/impeller/typographer/backends/skia/typeface_skia.cc
@@ -39,24 +39,6 @@ Rect TypefaceSkia::GetBoundingBox() const {
                         bounds.bottom());
 }
 
-static const SkFontTableTag kCOLRTableTag =
-    SkSetFourByteTag('C', 'O', 'L', 'R');
-
-bool TypefaceSkia::HasColor() const {
-  auto count = typeface_->countTables();
-  if (count == 0) {
-    return false;
-  }
-  std::vector<SkFontTableTag> tags(count);
-  typeface_->getTableTags(tags.data());
-  for (auto tag : tags) {
-    if (tag == kCOLRTableTag) {
-      return true;
-    }
-  }
-  return false;
-}
-
 const sk_sp<SkTypeface>& TypefaceSkia::GetSkiaTypeface() const {
   return typeface_;
 }
diff --git a/impeller/typographer/backends/skia/typeface_skia.h b/impeller/typographer/backends/skia/typeface_skia.h
index 058d649b99e5a..6ac52c47761fe 100644
--- a/impeller/typographer/backends/skia/typeface_skia.h
+++ b/impeller/typographer/backends/skia/typeface_skia.h
@@ -31,9 +31,6 @@ class TypefaceSkia final : public Typeface,
   // |Typeface|
   Rect GetBoundingBox() const override;
 
-  // |Typeface|
-  bool HasColor() const override;
-
   const sk_sp<SkTypeface>& GetSkiaTypeface() const;
 
  private:
diff --git a/impeller/typographer/font.cc b/impeller/typographer/font.cc
index a0a546d027bef..3553816b3d131 100644
--- a/impeller/typographer/font.cc
+++ b/impeller/typographer/font.cc
@@ -12,7 +12,10 @@ Font::Font(std::shared_ptr<Typeface> typeface, Metrics metrics)
     return;
   }
   is_valid_ = true;
-  has_color_ = typeface_->HasColor();
+}
+
+bool Font::HasColor(const Glyph& glyph) const {
+  return false;
 }
 
 Font::~Font() = default;
diff --git a/impeller/typographer/font.h b/impeller/typographer/font.h
index 71fc577a6b1c1..27a2081e0925f 100644
--- a/impeller/typographer/font.h
+++ b/impeller/typographer/font.h
@@ -99,7 +99,7 @@ class Font : public Comparable<Font> {
   // |Comparable<Font>|
   std::size_t GetHash() const override;
 
-  bool HasColor() const { return has_color_; }
+  bool HasColor(const Glyph& glyph) const;
 
   // |Comparable<Font>|
   bool IsEqual(const Font& other) const override;
@@ -107,7 +107,6 @@ class Font : public Comparable<Font> {
  private:
   std::shared_ptr<Typeface> typeface_;
   Metrics metrics_ = {};
-  bool has_color_ = false;
   bool is_valid_ = false;
 };
 
diff --git a/impeller/typographer/glyph_atlas.cc b/impeller/typographer/glyph_atlas.cc
index 8dca18d1d798a..7ed4578bbd923 100644
--- a/impeller/typographer/glyph_atlas.cc
+++ b/impeller/typographer/glyph_atlas.cc
@@ -23,14 +23,9 @@ void GlyphAtlas::SetTexture(std::shared_ptr<Texture> texture) {
 }
 
 void GlyphAtlas::AddTypefaceGlyphPosition(FontGlyphPair pair, Rect rect) {
-  has_color_ = pair.font.HasColor();
   positions_[pair] = rect;
 }
 
-bool GlyphAtlas::HasColor() const {
-  return has_color_;
-}
-
 std::optional<Rect> GlyphAtlas::FindFontGlyphPosition(
     const FontGlyphPair& pair) const {
   auto found = positions_.find(pair);
diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h
index 35622cb9bc122..896ca7c8f2d6e 100644
--- a/impeller/typographer/glyph_atlas.h
+++ b/impeller/typographer/glyph_atlas.h
@@ -32,8 +32,6 @@ class GlyphAtlas {
 
   bool IsValid() const;
 
-  bool HasColor() const;
-
   //----------------------------------------------------------------------------
   /// @brief      Set the texture for the glyph atlas.
   ///
@@ -88,7 +86,6 @@ class GlyphAtlas {
 
  private:
   std::shared_ptr<Texture> texture_;
-  bool has_color_ = false;
 
   std::unordered_map<FontGlyphPair,
                      Rect,
diff --git a/impeller/typographer/typeface.h b/impeller/typographer/typeface.h
index 65c0ed4aebe29..428b553c4d4ff 100644
--- a/impeller/typographer/typeface.h
+++ b/impeller/typographer/typeface.h
@@ -33,15 +33,6 @@ class Typeface : public Comparable<Typeface> {
   ///
   virtual Rect GetBoundingBox() const = 0;
 
-  //----------------------------------------------------------------------------
-  /// @brief      Indicates the presence of a COLR table in the typeface. This
-  ///             implies that an R8G8B8A8 atlas must be used.
-  ///
-  /// @return     Whether the typeface contains a COLR table.
-  ///
-  virtual bool HasColor() const = 0;
-
- private:
   FML_DISALLOW_COPY_AND_ASSIGN(Typeface);
 };
 

From d047bb79456e7d37dfacee7d775c1626820ac407 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Thu, 15 Sep 2022 17:17:21 -0700
Subject: [PATCH 3/6] ++

---
 impeller/typographer/font.cc    | 4 ----
 impeller/typographer/font.h     | 2 --
 impeller/typographer/typeface.h | 1 +
 3 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/impeller/typographer/font.cc b/impeller/typographer/font.cc
index 3553816b3d131..f8287341a2f10 100644
--- a/impeller/typographer/font.cc
+++ b/impeller/typographer/font.cc
@@ -14,10 +14,6 @@ Font::Font(std::shared_ptr<Typeface> typeface, Metrics metrics)
   is_valid_ = true;
 }
 
-bool Font::HasColor(const Glyph& glyph) const {
-  return false;
-}
-
 Font::~Font() = default;
 
 bool Font::IsValid() const {
diff --git a/impeller/typographer/font.h b/impeller/typographer/font.h
index 27a2081e0925f..2830900b7de9f 100644
--- a/impeller/typographer/font.h
+++ b/impeller/typographer/font.h
@@ -99,8 +99,6 @@ class Font : public Comparable<Font> {
   // |Comparable<Font>|
   std::size_t GetHash() const override;
 
-  bool HasColor(const Glyph& glyph) const;
-
   // |Comparable<Font>|
   bool IsEqual(const Font& other) const override;
 
diff --git a/impeller/typographer/typeface.h b/impeller/typographer/typeface.h
index 428b553c4d4ff..f3a4fe265af7f 100644
--- a/impeller/typographer/typeface.h
+++ b/impeller/typographer/typeface.h
@@ -33,6 +33,7 @@ class Typeface : public Comparable<Typeface> {
   ///
   virtual Rect GetBoundingBox() const = 0;
 
+ private:
   FML_DISALLOW_COPY_AND_ASSIGN(Typeface);
 };
 

From 3d2654bf86b93b98d64aac9bb693e6e77745691b Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Thu, 15 Sep 2022 18:54:40 -0700
Subject: [PATCH 4/6] ++

---
 impeller/entity/contents/text_contents.cc     |  6 +++-
 impeller/entity/shaders/glyph_atlas.frag      | 17 ++++++---
 impeller/entity/shaders/glyph_atlas.vert      |  3 ++
 .../backends/skia/text_render_context_skia.cc | 36 ++++++++-----------
 impeller/typographer/glyph_atlas.cc           | 24 +++++++++++++
 impeller/typographer/glyph_atlas.h            | 24 +++++++++++++
 6 files changed, 83 insertions(+), 27 deletions(-)

diff --git a/impeller/entity/contents/text_contents.cc b/impeller/entity/contents/text_contents.cc
index c876f206fa982..6cc0c292219e7 100644
--- a/impeller/entity/contents/text_contents.cc
+++ b/impeller/entity/contents/text_contents.cc
@@ -4,6 +4,7 @@
 
 #include "impeller/entity/contents/text_contents.h"
 
+#include <iostream>
 #include <optional>
 
 #include "impeller/entity/contents/content_context.h"
@@ -126,11 +127,13 @@ bool TextContents::Render(const ContentContext& renderer,
     auto font = run.GetFont();
     auto glyph_size = ISize::Ceil(font.GetMetrics().GetBoundingBox().size);
     for (const auto& glyph_position : run.GetGlyphPositions()) {
+      FontGlyphPair font_glyph_pair{font, glyph_position.glyph};
+      auto color_glyph =
+          atlas->IsColorFontGlyphPair(font_glyph_pair) ? 1.0 : 0.0;
       for (const auto& point : unit_vertex_points) {
         VS::PerVertexData vtx;
         vtx.unit_vertex = point;
 
-        FontGlyphPair font_glyph_pair{font, glyph_position.glyph};
         auto atlas_glyph_pos = atlas->FindFontGlyphPosition(font_glyph_pair);
         if (!atlas_glyph_pos.has_value()) {
           VALIDATION_LOG << "Could not find glyph position in the atlas.";
@@ -146,6 +149,7 @@ bool TextContents::Render(const ContentContext& renderer,
                                             1 / atlas_glyph_pos->size.height};
         vtx.atlas_glyph_size =
             Point{atlas_glyph_pos->size.width, atlas_glyph_pos->size.height};
+        vtx.color_glyph = color_glyph;
         vertex_builder.AppendVertex(std::move(vtx));
       }
     }
diff --git a/impeller/entity/shaders/glyph_atlas.frag b/impeller/entity/shaders/glyph_atlas.frag
index ec9ebc47716b6..ea8162c3e865e 100644
--- a/impeller/entity/shaders/glyph_atlas.frag
+++ b/impeller/entity/shaders/glyph_atlas.frag
@@ -13,15 +13,22 @@ uniform FragInfo {
 in vec2 v_unit_vertex;
 in vec2 v_atlas_position;
 in vec2 v_atlas_glyph_size;
+in float v_color_glyph;
 
 out vec4 frag_color;
 
 void main() {
   vec2 scale_perspective = v_atlas_glyph_size / frag_info.atlas_size;
   vec2 offset = v_atlas_position / frag_info.atlas_size;
-
-  frag_color = texture(
-    glyph_atlas_sampler,
-    v_unit_vertex * scale_perspective + offset
-  ) * frag_info.text_color;
+  if (v_color_glyph == 1.0) {
+    frag_color = texture(
+      glyph_atlas_sampler,
+      v_unit_vertex * scale_perspective + offset
+    );
+  } else {
+    frag_color = texture(
+      glyph_atlas_sampler,
+      v_unit_vertex * scale_perspective + offset
+    ).aaaa * frag_info.text_color;
+  }
 }
diff --git a/impeller/entity/shaders/glyph_atlas.vert b/impeller/entity/shaders/glyph_atlas.vert
index 05de00251c312..1ac172c082909 100644
--- a/impeller/entity/shaders/glyph_atlas.vert
+++ b/impeller/entity/shaders/glyph_atlas.vert
@@ -11,10 +11,12 @@ in vec2 glyph_position;
 in vec2 glyph_size;
 in vec2 atlas_position;
 in vec2 atlas_glyph_size;
+in float color_glyph;
 
 out vec2 v_unit_vertex;
 out vec2 v_atlas_position;
 out vec2 v_atlas_glyph_size;
+out float v_color_glyph;
 
 void main() {
   vec4 translate = frame_info.mvp[0] * glyph_position.x
@@ -36,4 +38,5 @@ void main() {
   v_unit_vertex = unit_vertex;
   v_atlas_position = atlas_position;
   v_atlas_glyph_size = atlas_glyph_size;
+  v_color_glyph = color_glyph;
 }
diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc
index bbcae78e1f47f..24d5505f1442e 100644
--- a/impeller/typographer/backends/skia/text_render_context_skia.cc
+++ b/impeller/typographer/backends/skia/text_render_context_skia.cc
@@ -105,25 +105,10 @@ static size_t OptimumAtlasSizeForFontGlyphPairs(
 }
 
 static std::shared_ptr<SkBitmap> CreateAtlasBitmap(const GlyphAtlas& atlas,
-                                                   size_t atlas_size,
-                                                   bool* has_color) {
+                                                   size_t atlas_size) {
   TRACE_EVENT0("impeller", __FUNCTION__);
   auto bitmap = std::make_shared<SkBitmap>();
-
-  atlas.IterateGlyphs([&has_color](const FontGlyphPair& font_glyph,
-                                   const Rect& location) -> bool {
-    SkFont sk_font(
-        TypefaceSkia::Cast(*font_glyph.font.GetTypeface()).GetSkiaTypeface(),
-        font_glyph.font.GetMetrics().point_size);
-
-    SkStrikeSpec strikeSpec = SkStrikeSpec::MakeWithNoDevice(sk_font);
-    SkBulkGlyphMetricsAndPaths paths{strikeSpec};
-    auto sk_glyph = paths.glyph(font_glyph.glyph.index);
-    *has_color |= sk_glyph->isColor();
-
-    return true;
-  });
-  auto image_info = *has_color
+  auto image_info = atlas.ContainsColorGlyph()
                         ? SkImageInfo::MakeN32Premul(atlas_size, atlas_size)
                         : SkImageInfo::MakeA8(atlas_size, atlas_size);
   if (!bitmap->tryAllocPixels(image_info)) {
@@ -218,6 +203,15 @@ std::shared_ptr<GlyphAtlas> TextRenderContextSkia::CreateGlyphAtlas(
   }
 
   auto glyph_atlas = std::make_shared<GlyphAtlas>();
+  glyph_atlas->SetFontColorCallback([](const FontGlyphPair& font_glyph) {
+    SkFont sk_font(
+        TypefaceSkia::Cast(*font_glyph.font.GetTypeface()).GetSkiaTypeface(),
+        font_glyph.font.GetMetrics().point_size);
+
+    SkStrikeSpec strikeSpec = SkStrikeSpec::MakeWithNoDevice(sk_font);
+    SkBulkGlyphMetricsAndPaths paths{strikeSpec};
+    return paths.glyph(font_glyph.glyph.index)->isColor();
+  });
 
   // ---------------------------------------------------------------------------
   // Step 1: Collect unique font-glyph pairs in the frame.
@@ -259,8 +253,7 @@ std::shared_ptr<GlyphAtlas> TextRenderContextSkia::CreateGlyphAtlas(
   // ---------------------------------------------------------------------------
   // Step 5: Draw font-glyph pairs in the correct spot in the atlas.
   // ---------------------------------------------------------------------------
-  bool has_color = false;
-  auto bitmap = CreateAtlasBitmap(*glyph_atlas, atlas_size, &has_color);
+  auto bitmap = CreateAtlasBitmap(*glyph_atlas, atlas_size);
   if (!bitmap) {
     return nullptr;
   }
@@ -268,8 +261,9 @@ std::shared_ptr<GlyphAtlas> TextRenderContextSkia::CreateGlyphAtlas(
   // ---------------------------------------------------------------------------
   // Step 6: Upload the atlas as a texture.
   // ---------------------------------------------------------------------------
-  auto format =
-      has_color ? PixelFormat::kR8G8B8A8UNormInt : PixelFormat::kA8UNormInt;
+  auto format = glyph_atlas->ContainsColorGlyph()
+                    ? PixelFormat::kR8G8B8A8UNormInt
+                    : PixelFormat::kA8UNormInt;
   auto texture = UploadGlyphTextureAtlas(GetContext()->GetResourceAllocator(),
                                          bitmap, atlas_size, format);
   if (!texture) {
diff --git a/impeller/typographer/glyph_atlas.cc b/impeller/typographer/glyph_atlas.cc
index 7ed4578bbd923..0248aa8e5bc12 100644
--- a/impeller/typographer/glyph_atlas.cc
+++ b/impeller/typographer/glyph_atlas.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "impeller/typographer/glyph_atlas.h"
+#include <iostream>
 
 namespace impeller {
 
@@ -14,6 +15,10 @@ bool GlyphAtlas::IsValid() const {
   return !!texture_;
 }
 
+bool GlyphAtlas::ContainsColorGlyph() const {
+  return has_color_glyph;
+}
+
 const std::shared_ptr<Texture>& GlyphAtlas::GetTexture() const {
   return texture_;
 }
@@ -23,9 +28,28 @@ void GlyphAtlas::SetTexture(std::shared_ptr<Texture> texture) {
 }
 
 void GlyphAtlas::AddTypefaceGlyphPosition(FontGlyphPair pair, Rect rect) {
+  if (callback_.has_value()) {
+    auto has_color = callback_.value()(pair);
+    has_color_glyph |= has_color;
+    colors_[pair] = has_color;
+  }
+
   positions_[pair] = rect;
 }
 
+void GlyphAtlas::SetFontColorCallback(
+    std::function<bool(const FontGlyphPair& pair)> callback) {
+  callback_ = std::move(callback);
+}
+
+bool GlyphAtlas::IsColorFontGlyphPair(const FontGlyphPair& pair) const {
+  auto found = colors_.find(pair);
+  if (found == colors_.end()) {
+    return false;
+  }
+  return found->second;
+}
+
 std::optional<Rect> GlyphAtlas::FindFontGlyphPosition(
     const FontGlyphPair& pair) const {
   auto found = positions_.find(pair);
diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h
index 896ca7c8f2d6e..2d44428b2080b 100644
--- a/impeller/typographer/glyph_atlas.h
+++ b/impeller/typographer/glyph_atlas.h
@@ -32,6 +32,11 @@ class GlyphAtlas {
 
   bool IsValid() const;
 
+  //----------------------------------------------------------------------------
+  /// @brief   Whether at least one font-glyph pair has colors.
+  ///
+  bool ContainsColorGlyph() const;
+
   //----------------------------------------------------------------------------
   /// @brief      Set the texture for the glyph atlas.
   ///
@@ -39,6 +44,17 @@ class GlyphAtlas {
   ///
   void SetTexture(std::shared_ptr<Texture> texture);
 
+  //----------------------------------------------------------------------------
+  /// @brief      Set a callback that determines if a glyph-font pair
+  ///             has color.
+  ///
+  /// @param[in]  callback  The callback
+  ///
+  void SetFontColorCallback(
+      std::function<bool(const FontGlyphPair& pair)> callback);
+
+  bool IsColorFontGlyphPair(const FontGlyphPair& pair) const;
+
   //----------------------------------------------------------------------------
   /// @brief      Get the texture for the glyph atlas.
   ///
@@ -86,6 +102,8 @@ class GlyphAtlas {
 
  private:
   std::shared_ptr<Texture> texture_;
+  std::optional<std::function<bool(const FontGlyphPair& pair)>> callback_;
+  bool has_color_glyph = false;
 
   std::unordered_map<FontGlyphPair,
                      Rect,
@@ -93,6 +111,12 @@ class GlyphAtlas {
                      FontGlyphPair::Equal>
       positions_;
 
+  std::unordered_map<FontGlyphPair,
+                     bool,
+                     FontGlyphPair::Hash,
+                     FontGlyphPair::Equal>
+      colors_;
+
   FML_DISALLOW_COPY_AND_ASSIGN(GlyphAtlas);
 };
 

From 657f2f7e76f2017bd32307c0ae7b5e0de36b4ac6 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Thu, 15 Sep 2022 21:00:53 -0700
Subject: [PATCH 5/6] ++

---
 impeller/typographer/backends/skia/text_render_context_skia.cc | 1 +
 impeller/typographer/glyph_atlas.h                             | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/impeller/typographer/backends/skia/text_render_context_skia.cc b/impeller/typographer/backends/skia/text_render_context_skia.cc
index 24d5505f1442e..d6e718c3a64f5 100644
--- a/impeller/typographer/backends/skia/text_render_context_skia.cc
+++ b/impeller/typographer/backends/skia/text_render_context_skia.cc
@@ -204,6 +204,7 @@ std::shared_ptr<GlyphAtlas> TextRenderContextSkia::CreateGlyphAtlas(
 
   auto glyph_atlas = std::make_shared<GlyphAtlas>();
   glyph_atlas->SetFontColorCallback([](const FontGlyphPair& font_glyph) {
+    // TODO(jonahwilliams): ask Skia for a public API to look this up.
     SkFont sk_font(
         TypefaceSkia::Cast(*font_glyph.font.GetTypeface()).GetSkiaTypeface(),
         font_glyph.font.GetMetrics().point_size);
diff --git a/impeller/typographer/glyph_atlas.h b/impeller/typographer/glyph_atlas.h
index 2d44428b2080b..cec7222ae9e5b 100644
--- a/impeller/typographer/glyph_atlas.h
+++ b/impeller/typographer/glyph_atlas.h
@@ -53,6 +53,9 @@ class GlyphAtlas {
   void SetFontColorCallback(
       std::function<bool(const FontGlyphPair& pair)> callback);
 
+  //----------------------------------------------------------------------------
+  /// @brief      Whether the provided glyph-font pair contains color.
+  ///
   bool IsColorFontGlyphPair(const FontGlyphPair& pair) const;
 
   //----------------------------------------------------------------------------

From 8f613f3485b1a4563aaa962b6a8857296b49bd1e Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Thu, 15 Sep 2022 21:09:00 -0700
Subject: [PATCH 6/6] add 'test'

---
 impeller/typographer/typographer_unittests.cc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc
index 80b17678ab201..d0a0e318b76a1 100644
--- a/impeller/typographer/typographer_unittests.cc
+++ b/impeller/typographer/typographer_unittests.cc
@@ -40,6 +40,7 @@ TEST_P(TypographerTest, CanCreateGlyphAtlas) {
   ASSERT_TRUE(blob);
   auto atlas = context->CreateGlyphAtlas(TextFrameFromTextBlob(blob));
   ASSERT_NE(atlas, nullptr);
+  ASSERT_FALSE(atlas->ContainsColorGlyph());
   OpenPlaygroundHere([](RenderTarget&) { return true; });
 }