Skip to content

Commit

Permalink
Fix font sorting problem due to iOS 14 fonts being broader (flutter#2…
Browse files Browse the repository at this point in the history 
…0557)
  • Loading branch information
@xster
xster authored Aug 24, 2020
1 parent b792de7 commit f8d2d7b
Show file tree
Hide file tree
Showing 3 changed files with 160 additions and 9 deletions.
46 changes: 38 additions & 8 deletions third_party/txt/src/txt/font_collection.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
5 changes: 5 additions & 0 deletions third_party/txt/src/txt/font_collection.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
118 changes: 117 additions & 1 deletion third_party/txt/tests/font_collection_unittests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit f8d2d7b

Please sign in to comment.