Fix font sorting problem due to iOS 14 fonts being broader

third_party/txt/src/txt/font_collection.cc

@@ -202,6 +202,43 @@ std::shared_ptr<minikin::FontFamily> FontCollection::FindFontFamilyInManagers(
   return nullptr;
 }
 
+void FontCollection::SortSkTypefaces(
+    std::vector<sk_sp<SkTypeface>>& sk_typefaces) {
+  std::sort(
+      sk_typefaces.begin(), sk_typefaces.end(),
+      [](const sk_sp<SkTypeface>& a, const sk_sp<SkTypeface>& b) {
+        SkFontStyle a_style = a->fontStyle();
+        SkFontStyle b_style = b->fontStyle();
+
+        int a_delta = std::abs(a_style.width() - SkFontStyle::kNormal_Width);
+        int b_delta = std::abs(b_style.width() - SkFontStyle::kNormal_Width);
+
+        if (a_delta != b_delta) {
+          // If a family name query is so generic it ends up bringing in fonts
+          // of multiple widths (e.g. condensed, expanded), opt to be
+          // conservative and select the most standard width.
+          //
+          // If a specific width is desired, it should be be narrowed down via
+          // the family name.
+          //
+          // The font weights are also sorted lightest to heaviest but Flutter
+          // APIs have the weight specified to narrow it down later. The width
+          // ordering here is more consequential since TextStyle doesn't have
+          // letter width APIs.
+          return a_delta < b_delta;
+        } else if (a_style.width() != b_style.width()) {
+          // However, if the 2 fonts are equidistant from the "normal" width,
+          // just arbitrarily but consistently return the more condensed font.
+          return a_style.width() < b_style.width();
+        } else if (a_style.weight() != b_style.weight()) {
+          return a_style.weight() < b_style.weight();
+        } else {
+          return a_style.slant() < b_style.slant();
+        }
+        // Use a cascade of conditions so results are consistent each time.
+      });
+}
+
 std::shared_ptr<minikin::FontFamily> FontCollection::CreateMinikinFontFamily(
     const sk_sp<SkFontMgr>& manager,
     const std::string& family_name) {
@@ -223,14 +260,7 @@ std::shared_ptr<minikin::FontFamily> FontCollection::CreateMinikinFontFamily(
     }
   }
 
-  std::sort(skia_typefaces.begin(), skia_typefaces.end(),
-            [](const sk_sp<SkTypeface>& a, const sk_sp<SkTypeface>& b) {
-              SkFontStyle a_style = a->fontStyle();
-              SkFontStyle b_style = b->fontStyle();
-              return (a_style.weight() != b_style.weight())
-                         ? a_style.weight() < b_style.weight()
-                         : a_style.slant() < b_style.slant();
-            });
+  SortSkTypefaces(skia_typefaces);
 
   std::vector<minikin::Font> minikin_fonts;
   for (const sk_sp<SkTypeface>& skia_typeface : skia_typefaces) {

third_party/txt/src/txt/font_collection.h

@@ -127,6 +127,11 @@ class FontCollection : public std::enable_shared_from_this<FontCollection> {
       const sk_sp<SkFontMgr>& manager,
       const std::string& family_name);
 
+  // Sorts in-place a group of SkTypeface from an SkTypefaceSet into a
+  // reasonable order for future queries.
+  FRIEND_TEST(FontCollectionTest, CheckSkTypefacesSorting);
+  static void SortSkTypefaces(std::vector<sk_sp<SkTypeface>>& sk_typefaces);
+
   const std::shared_ptr<minikin::FontFamily>& GetFallbackFontFamily(
       const sk_sp<SkFontMgr>& manager,
       const std::string& family_name);

third_party/txt/tests/font_collection_unittests.cc

@@ -14,14 +14,130 @@
  * limitations under the License.
  */
 
-#include "flutter/fml/command_line.h"
 #include "flutter/fml/logging.h"
 #include "gtest/gtest.h"
+#include "third_party/skia/include/utils/SkCustomTypeface.h"
 #include "txt/font_collection.h"
 #include "txt_test_utils.h"
 
 namespace txt {
 
+// We don't really need a fixture but a class in a namespace is needed for
+// the FRIEND_TEST macro.
+class FontCollectionTest : public ::testing::Test {};
+
+namespace {
+// This function does some boilerplate to fill a builder with enough real
+// font-like data. Otherwise, detach won't actually build an SkTypeface.
+void PopulateUserTypefaceBoilerplate(SkCustomTypefaceBuilder* builder) {
+  constexpr float upem = 200;
+
+  {
+    SkFontMetrics metrics;
+    metrics.fFlags = 0;
+    metrics.fTop = -200;
+    metrics.fAscent = -150;
+    metrics.fDescent = 50;
+    metrics.fBottom = -75;
+    metrics.fLeading = 10;
+    metrics.fAvgCharWidth = 150;
+    metrics.fMaxCharWidth = 300;
+    metrics.fXMin = -20;
+    metrics.fXMax = 290;
+    metrics.fXHeight = -100;
+    metrics.fCapHeight = 0;
+    metrics.fUnderlineThickness = 5;
+    metrics.fUnderlinePosition = 2;
+    metrics.fStrikeoutThickness = 5;
+    metrics.fStrikeoutPosition = -50;
+    builder->setMetrics(metrics, 1.0f / upem);
+  }
+
+  const SkMatrix scale = SkMatrix::Scale(1.0f / upem, 1.0f / upem);
+  for (SkGlyphID index = 0; index <= 67; ++index) {
+    SkScalar width;
+    width = 100;
+    SkPath path;
+    path.addCircle(50, -50, 75);
+
+    builder->setGlyph(index, width / upem, path.makeTransform(scale));
+  }
+}
+}  // namespace
+
+TEST(FontCollectionTest, CheckSkTypefacesSorting) {
+  // We have to make a real SkTypeface here. Not all the structs from the
+  // SkTypeface headers are fully declared to be able to gmock.
+  // SkCustomTypefaceBuilder is the simplest way to get a simple SkTypeface.
+  SkCustomTypefaceBuilder typefaceBuilder1;
+  typefaceBuilder1.setFontStyle(SkFontStyle(SkFontStyle::kThin_Weight,
+                                            SkFontStyle::kExpanded_Width,
+                                            SkFontStyle::kItalic_Slant));
+  // For the purpose of this test, we need to fill this to make the SkTypeface
+  // build but it doesn't matter. We only care about the SkFontStyle.
+  PopulateUserTypefaceBoilerplate(&typefaceBuilder1);
+  sk_sp<SkTypeface> typeface1{typefaceBuilder1.detach()};
+
+  SkCustomTypefaceBuilder typefaceBuilder2;
+  typefaceBuilder2.setFontStyle(SkFontStyle(SkFontStyle::kLight_Weight,
+                                            SkFontStyle::kNormal_Width,
+                                            SkFontStyle::kUpright_Slant));
+  PopulateUserTypefaceBoilerplate(&typefaceBuilder2);
+  sk_sp<SkTypeface> typeface2{typefaceBuilder2.detach()};
+
+  SkCustomTypefaceBuilder typefaceBuilder3;
+  typefaceBuilder3.setFontStyle(SkFontStyle(SkFontStyle::kNormal_Weight,
+                                            SkFontStyle::kNormal_Width,
+                                            SkFontStyle::kUpright_Slant));
+  PopulateUserTypefaceBoilerplate(&typefaceBuilder3);
+  sk_sp<SkTypeface> typeface3{typefaceBuilder3.detach()};
+
+  SkCustomTypefaceBuilder typefaceBuilder4;
+  typefaceBuilder4.setFontStyle(SkFontStyle(SkFontStyle::kThin_Weight,
+                                            SkFontStyle::kCondensed_Width,
+                                            SkFontStyle::kUpright_Slant));
+  PopulateUserTypefaceBoilerplate(&typefaceBuilder4);
+  sk_sp<SkTypeface> typeface4{typefaceBuilder4.detach()};
+
+  std::vector<sk_sp<SkTypeface>> candidateTypefaces = {typeface1, typeface2,
+                                                       typeface3, typeface4};
+
+  // This sorts the vector in-place.
+  txt::FontCollection::SortSkTypefaces(candidateTypefaces);
+
+  // The second one is first because it's both the most normal width font
+  // with the lightest weight.
+  ASSERT_EQ(candidateTypefaces[0].get(), typeface2.get());
+  // Then the most normal width font with normal weight.
+  ASSERT_EQ(candidateTypefaces[1].get(), typeface3.get());
+  // Then a less normal (condensed) width font.
+  ASSERT_EQ(candidateTypefaces[2].get(), typeface4.get());
+  // All things equal, 4 came before 1 because we arbitrarily chose to make the
+  // narrower font come first.
+  ASSERT_EQ(candidateTypefaces[3].get(), typeface1.get());
+
+  // Double check.
+  ASSERT_EQ(candidateTypefaces[0]->fontStyle().weight(),
+            SkFontStyle::kLight_Weight);
+  ASSERT_EQ(candidateTypefaces[0]->fontStyle().width(),
+            SkFontStyle::kNormal_Width);
+
+  ASSERT_EQ(candidateTypefaces[1]->fontStyle().weight(),
+            SkFontStyle::kNormal_Weight);
+  ASSERT_EQ(candidateTypefaces[1]->fontStyle().width(),
+            SkFontStyle::kNormal_Width);
+
+  ASSERT_EQ(candidateTypefaces[2]->fontStyle().weight(),
+            SkFontStyle::kThin_Weight);
+  ASSERT_EQ(candidateTypefaces[2]->fontStyle().width(),
+            SkFontStyle::kCondensed_Width);
+
+  ASSERT_EQ(candidateTypefaces[3]->fontStyle().weight(),
+            SkFontStyle::kThin_Weight);
+  ASSERT_EQ(candidateTypefaces[3]->fontStyle().width(),
+            SkFontStyle::kExpanded_Width);
+}
+
 #if 0
 
 TEST(FontCollection, HasDefaultRegistrations) {