From d6bf496f075d4ff08367b0e6311a84839f00f8e9 Mon Sep 17 00:00:00 2001
From: jonahwilliams <jonahwilliams@google.com>
Date: Wed, 22 Feb 2023 10:46:16 -0800
Subject: [PATCH] [Impeller] ensure glyph type contributes to FontGlyphPair
 hash/eq

---
 impeller/typographer/font_glyph_pair.h        |  5 +++--
 impeller/typographer/glyph.h                  | 11 ++++++++++-
 impeller/typographer/typographer_unittests.cc | 18 ++++++++++++++++++
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/impeller/typographer/font_glyph_pair.h b/impeller/typographer/font_glyph_pair.h
index b8cb7b0af2811..ed455095356dd 100644
--- a/impeller/typographer/font_glyph_pair.h
+++ b/impeller/typographer/font_glyph_pair.h
@@ -31,12 +31,13 @@ struct FontGlyphPair {
 
   struct Hash {
     std::size_t operator()(const FontGlyphPair& p) const {
-      return fml::HashCombine(p.font.GetHash(), p.glyph);
+      return fml::HashCombine(p.font.GetHash(), p.glyph.index, p.glyph.type);
     }
   };
   struct Equal {
     bool operator()(const FontGlyphPair& lhs, const FontGlyphPair& rhs) const {
-      return lhs.font.IsEqual(rhs.font) && lhs.glyph.index == rhs.glyph.index;
+      return lhs.font.IsEqual(rhs.font) && lhs.glyph.index == rhs.glyph.index &&
+             lhs.glyph.type == rhs.glyph.type;
     }
   };
 };
diff --git a/impeller/typographer/glyph.h b/impeller/typographer/glyph.h
index 82ba5e4b299d8..b036e415cc540 100644
--- a/impeller/typographer/glyph.h
+++ b/impeller/typographer/glyph.h
@@ -7,6 +7,7 @@
 #include <cstdint>
 #include <functional>
 
+#include "flutter/fml/hash_combine.h"
 #include "flutter/fml/macros.h"
 #include "impeller/geometry/rect.h"
 
@@ -43,7 +44,15 @@ struct Glyph {
 template <>
 struct std::hash<impeller::Glyph> {
   constexpr std::size_t operator()(const impeller::Glyph& g) const {
-    return g.index;
+    return fml::HashCombine(g.index, g.type);
+  }
+};
+
+template <>
+struct std::equal_to<impeller::Glyph> {
+  constexpr bool operator()(const impeller::Glyph& lhs,
+                            const impeller::Glyph& rhs) const {
+    return lhs.index == rhs.index && lhs.type == rhs.type;
   }
 };
 
diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc
index 57eefbb36d4cf..a4d174100a7ba 100644
--- a/impeller/typographer/typographer_unittests.cc
+++ b/impeller/typographer/typographer_unittests.cc
@@ -201,5 +201,23 @@ TEST_P(TypographerTest, GlyphAtlasTextureIsRecycledIfUnchanged) {
   ASSERT_EQ(old_packer, new_packer);
 }
 
+TEST_P(TypographerTest, FontGlyphPairTypeChangesHashAndEquals) {
+  Font font = Font(nullptr, {});
+  FontGlyphPair pair_1 = {
+      .font = font,
+      .glyph = Glyph(0, Glyph::Type::kBitmap, Rect::MakeXYWH(0, 0, 1, 1))};
+  // Same glyph same type.
+  FontGlyphPair pair_2 = {
+      .font = font,
+      .glyph = Glyph(0, Glyph::Type::kBitmap, Rect::MakeXYWH(0, 0, 1, 1))};
+  // Same glyph different type.
+  FontGlyphPair pair_3 = {
+      .font = font,
+      .glyph = Glyph(0, Glyph::Type::kPath, Rect::MakeXYWH(0, 0, 1, 1))};
+
+  ASSERT_TRUE(FontGlyphPair::Equal{}(pair_1, pair_2));
+  ASSERT_FALSE(FontGlyphPair::Equal{}(pair_1, pair_3));
+}
+
 }  // namespace testing
 }  // namespace impeller