Replace all calls to SkTypeface::Make with SkFontMgr ones (#48319)

kjlubick · web-flow · commit c18d3df967dc · 2023-11-28T07:08:35.000-05:00
Similar to #48179, Flutter needs to stop depending on the default font manager, which the `SkTypeface::Make*` calls do implicitly ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat
diff --git a/display_list/BUILD.gn b/display_list/BUILD.gn
@@ -158,6 +158,7 @@ if (enable_unittests) {
       "//flutter/display_list/testing:display_list_testing",
       "//flutter/impeller/typographer/backends/skia:typographer_skia_backend",
       "//flutter/testing",
+      "//flutter/third_party/txt",
     ]
 
     if (!defined(defines)) {
diff --git a/display_list/testing/BUILD.gn b/display_list/testing/BUILD.gn
@@ -17,6 +17,7 @@ source_set("display_list_testing") {
   deps = [
     "//flutter/skia",
     "//flutter/testing:testing_lib",
+    "//flutter/third_party/txt",
   ]
 
   public_deps = [ "//flutter/display_list:display_list" ]
diff --git a/display_list/testing/dl_rendering_unittests.cc b/display_list/testing/dl_rendering_unittests.cc
@@ -23,15 +23,18 @@
 
 #include "third_party/skia/include/core/SkBBHFactory.h"
 #include "third_party/skia/include/core/SkColorFilter.h"
+#include "third_party/skia/include/core/SkFontMgr.h"
 #include "third_party/skia/include/core/SkPictureRecorder.h"
 #include "third_party/skia/include/core/SkStream.h"
 #include "third_party/skia/include/core/SkSurface.h"
+#include "third_party/skia/include/core/SkTypeface.h"
 #include "third_party/skia/include/effects/SkGradientShader.h"
 #include "third_party/skia/include/effects/SkImageFilters.h"
 #include "third_party/skia/include/encode/SkPngEncoder.h"
 #include "third_party/skia/include/gpu/GrDirectContext.h"
 #include "third_party/skia/include/gpu/GrRecordingContext.h"
 #include "third_party/skia/include/gpu/GrTypes.h"
+#include "txt/platform.h"
 
 namespace flutter {
 namespace testing {
@@ -2747,7 +2750,8 @@ class CanvasCompareTester {
 
   static sk_sp<SkTextBlob> MakeTextBlob(const std::string& string,
                                         SkScalar font_height) {
-    SkFont font(SkTypeface::MakeFromName("ahem", SkFontStyle::Normal()),
+    SkFont font(txt::GetDefaultFontManager()->matchFamilyStyle(
+                    "ahem", SkFontStyle::Normal()),
                 font_height);
     return SkTextBlob::MakeFromText(string.c_str(), string.size(), font,
                                     SkTextEncoding::kUTF8);
diff --git a/display_list/testing/dl_test_snippets.cc b/display_list/testing/dl_test_snippets.cc
@@ -5,6 +5,9 @@
 #include "flutter/display_list/testing/dl_test_snippets.h"
 #include "flutter/display_list/dl_builder.h"
 #include "flutter/display_list/dl_op_receiver.h"
+#include "third_party/skia/include/core/SkFontMgr.h"
+#include "third_party/skia/include/core/SkTypeface.h"
+#include "txt/platform.h"
 
 namespace flutter {
 namespace testing {
@@ -978,7 +981,7 @@ SkFont CreateTestFontOfSize(SkScalar scalar) {
   static constexpr const char* kTestFontFixture = "Roboto-Regular.ttf";
   auto mapping = flutter::testing::OpenFixtureAsSkData(kTestFontFixture);
   FML_CHECK(mapping);
-  return SkFont{SkTypeface::MakeFromData(mapping), scalar};
+  return SkFont{txt::GetDefaultFontManager()->makeFromData(mapping), scalar};
 }
 
 sk_sp<SkTextBlob> GetTestTextBlob(int index) {
diff --git a/flow/BUILD.gn b/flow/BUILD.gn
@@ -94,6 +94,7 @@ source_set("flow") {
     "//flutter/common/graphics",
     "//flutter/display_list",
     "//flutter/fml",
+    "//flutter/third_party/txt",
   ]
 
   deps = [ "//flutter/skia" ]
diff --git a/flow/layers/performance_overlay_layer.cc b/flow/layers/performance_overlay_layer.cc
@@ -13,7 +13,10 @@
 #include "flow/stopwatch_dl.h"
 #include "flow/stopwatch_sk.h"
 #include "third_party/skia/include/core/SkFont.h"
+#include "third_party/skia/include/core/SkFontMgr.h"
 #include "third_party/skia/include/core/SkTextBlob.h"
+#include "third_party/skia/include/core/SkTypeface.h"
+#include "txt/platform.h"
 #ifdef IMPELLER_SUPPORTS_RENDERING
 #include "impeller/typographer/backends/skia/text_frame_skia.h"  // nogncheck
 #endif  // IMPELLER_SUPPORTS_RENDERING
@@ -72,7 +75,8 @@ sk_sp<SkTextBlob> PerformanceOverlayLayer::MakeStatisticsText(
     const std::string& font_path) {
   SkFont font;
   if (font_path != "") {
-    font = SkFont(SkTypeface::MakeFromFile(font_path.c_str()));
+    sk_sp<SkFontMgr> font_mgr = txt::GetDefaultFontManager();
+    font = SkFont(font_mgr->makeFromFile(font_path.c_str()));
   }
   font.setSize(15);
 
diff --git a/impeller/aiks/BUILD.gn b/impeller/aiks/BUILD.gn
@@ -88,6 +88,7 @@ impeller_component("aiks_unittests") {
     "//flutter/impeller/scene",
     "//flutter/impeller/typographer/backends/stb:typographer_stb_backend",
     "//flutter/testing:testing_lib",
+    "//flutter/third_party/txt",
   ]
 
   if (!impeller_trace_canvas) {
@@ -119,6 +120,7 @@ impeller_component("aiks_unittests_golden") {
     "//flutter/impeller/scene",
     "//flutter/impeller/typographer/backends/stb:typographer_stb_backend",
     "//flutter/testing:testing_lib",
+    "//flutter/third_party/txt",
   ]
 }
 
diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc
@@ -46,6 +46,9 @@
 #include "impeller/typographer/backends/stb/typographer_context_stb.h"
 #include "third_party/imgui/imgui.h"
 #include "third_party/skia/include/core/SkData.h"
+#include "third_party/skia/include/core/SkFontMgr.h"
+#include "third_party/skia/include/core/SkTypeface.h"
+#include "txt/platform.h"
 
 namespace impeller {
 namespace testing {
@@ -1398,7 +1401,8 @@ bool RenderTextInCanvasSkia(const std::shared_ptr<Context>& context,
   if (!mapping) {
     return false;
   }
-  SkFont sk_font(SkTypeface::MakeFromData(mapping), options.font_size);
+  sk_sp<SkFontMgr> font_mgr = txt::GetDefaultFontManager();
+  SkFont sk_font(font_mgr->makeFromData(mapping), options.font_size);
   auto blob = SkTextBlob::MakeFromString(text.c_str(), sk_font);
   if (!blob) {
     return false;
@@ -1576,7 +1580,8 @@ TEST_P(AiksTest, CanRenderTextOutsideBoundaries) {
   ASSERT_NE(mapping, nullptr);
 
   Scalar font_size = 80;
-  SkFont sk_font(SkTypeface::MakeFromData(mapping), font_size);
+  sk_sp<SkFontMgr> font_mgr = txt::GetDefaultFontManager();
+  SkFont sk_font(font_mgr->makeFromData(mapping), font_size);
 
   Paint text_paint;
   text_paint.color = Color::Blue().WithAlpha(0.8);
@@ -3449,7 +3454,8 @@ TEST_P(AiksTest, TextForegroundShaderWithTransform) {
   ASSERT_NE(mapping, nullptr);
 
   Scalar font_size = 100;
-  SkFont sk_font(SkTypeface::MakeFromData(mapping), font_size);
+  sk_sp<SkFontMgr> font_mgr = txt::GetDefaultFontManager();
+  SkFont sk_font(font_mgr->makeFromData(mapping), font_size);
 
   Paint text_paint;
   text_paint.color = Color::Blue();
diff --git a/impeller/display_list/BUILD.gn b/impeller/display_list/BUILD.gn
@@ -63,6 +63,7 @@ impeller_component("display_list_unittests") {
     "../playground:playground_test",
     "//flutter/impeller/scene",
     "//flutter/impeller/typographer/backends/stb:typographer_stb_backend",
+    "//flutter/third_party/txt",
   ]
 
   if (!defined(defines)) {
diff --git a/impeller/display_list/dl_playground.cc b/impeller/display_list/dl_playground.cc
@@ -10,6 +10,9 @@
 #include "impeller/typographer/backends/skia/typographer_context_skia.h"
 #include "third_party/imgui/imgui.h"
 #include "third_party/skia/include/core/SkData.h"
+#include "third_party/skia/include/core/SkFontMgr.h"
+#include "third_party/skia/include/core/SkTypeface.h"
+#include "txt/platform.h"
 
 namespace impeller {
 
@@ -56,7 +59,8 @@ SkFont DlPlayground::CreateTestFontOfSize(SkScalar scalar) {
   static constexpr const char* kTestFontFixture = "Roboto-Regular.ttf";
   auto mapping = flutter::testing::OpenFixtureAsSkData(kTestFontFixture);
   FML_CHECK(mapping);
-  return SkFont{SkTypeface::MakeFromData(mapping), scalar};
+  sk_sp<SkFontMgr> font_mgr = txt::GetDefaultFontManager();
+  return SkFont{font_mgr->makeFromData(mapping), scalar};
 }
 
 SkFont DlPlayground::CreateTestFont() {
diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc
@@ -12,6 +12,7 @@
 #include "third_party/skia/include/core/SkFontMgr.h"
 #include "third_party/skia/include/core/SkRect.h"
 #include "third_party/skia/include/core/SkTextBlob.h"
+#include "third_party/skia/include/core/SkTypeface.h"
 #include "txt/platform.h"
 
 // TODO(zanderso): https://github.com/flutter/flutter/issues/127701
@@ -94,7 +95,8 @@ TEST_P(TypographerTest, LazyAtlasTracksColor) {
   auto mapping = flutter::testing::OpenFixtureAsSkData("NotoColorEmoji.ttf");
 #endif
   ASSERT_TRUE(mapping);
-  SkFont emoji_font(SkTypeface::MakeFromData(mapping), 50.0);
+  sk_sp<SkFontMgr> font_mgr = txt::GetDefaultFontManager();
+  SkFont emoji_font(font_mgr->makeFromData(mapping), 50.0);
   SkFont sk_font;
 
   auto blob = SkTextBlob::MakeFromString("hello", sk_font);
diff --git a/lib/ui/text/asset_manager_font_provider.cc b/lib/ui/text/asset_manager_font_provider.cc
@@ -8,9 +8,11 @@
 
 #include "flutter/fml/logging.h"
 #include "third_party/skia/include/core/SkData.h"
+#include "third_party/skia/include/core/SkFontMgr.h"
 #include "third_party/skia/include/core/SkStream.h"
 #include "third_party/skia/include/core/SkString.h"
 #include "third_party/skia/include/core/SkTypeface.h"
+#include "txt/platform.h"
 
 namespace flutter {
 
@@ -116,8 +118,9 @@ auto AssetManagerFontStyleSet::createTypeface(int i) -> CreateTypefaceRet {
         MappingReleaseProc, asset_mapping_ptr);
     std::unique_ptr<SkMemoryStream> stream = SkMemoryStream::Make(asset_data);
 
+    sk_sp<SkFontMgr> font_mgr = txt::GetDefaultFontManager();
     // Ownership of the stream is transferred.
-    asset.typeface = SkTypeface::MakeFromStream(std::move(stream));
+    asset.typeface = font_mgr->makeFromStream(std::move(stream));
     if (!asset.typeface) {
       FML_DLOG(ERROR) << "Unable to load font asset for family: "
                       << family_name_;
diff --git a/lib/ui/text/font_collection.cc b/lib/ui/text/font_collection.cc
@@ -21,7 +21,9 @@
 #include "third_party/tonic/logging/dart_invoke.h"
 #include "third_party/tonic/typed_data/typed_list.h"
 #include "txt/asset_font_manager.h"
+#include "txt/platform.h"
 #include "txt/test_font_manager.h"
+
 #if FML_OS_MACOSX || FML_OS_IOS
 #include "txt/platform_mac.h"
 #endif
@@ -158,8 +160,8 @@ void FontCollection::LoadFontFromList(Dart_Handle font_data_handle,
 
   std::unique_ptr<SkStreamAsset> font_stream = std::make_unique<SkMemoryStream>(
       font_data.data(), font_data.num_elements(), true);
-  sk_sp<SkTypeface> typeface =
-      SkTypeface::MakeFromStream(std::move(font_stream));
+  sk_sp<SkFontMgr> font_mgr = txt::GetDefaultFontManager();
+  sk_sp<SkTypeface> typeface = font_mgr->makeFromStream(std::move(font_stream));
   txt::TypefaceFontAssetProvider& font_provider =
       font_collection.dynamic_font_manager_->font_provider();
   if (family_name.empty()) {
diff --git a/runtime/BUILD.gn b/runtime/BUILD.gn
@@ -10,7 +10,10 @@ source_set("test_font") {
     "test_font_data.cc",
     "test_font_data.h",
   ]
-  deps = [ "//flutter/skia" ]
+  deps = [
+    "//flutter/skia",
+    "//flutter/third_party/txt",
+  ]
   public_configs = [ "//flutter:config" ]
   defines = []
   if (flutter_runtime_mode == "debug" || current_toolchain == host_toolchain) {
diff --git a/runtime/test_font_data.cc b/runtime/test_font_data.cc
@@ -46,6 +46,10 @@
 
 #if EMBED_TEST_FONT_DATA
 
+#include "third_party/skia/include/core/SkFontMgr.h"
+#include "third_party/skia/include/core/SkTypeface.h"
+#include "txt/platform.h"
+
 static const unsigned char kAhemFont[] = {
     0x00, 0x01, 0x00, 0x00, 0x00, 0x0b, 0x00, 0x80, 0x00, 0x03, 0x00, 0x30,
     0x4f, 0x53, 0x2f, 0x32, 0x77, 0x60, 0xf9, 0x6f, 0x00, 0x00, 0x01, 0x38,
@@ -1621,11 +1625,12 @@ namespace flutter {
 std::vector<sk_sp<SkTypeface>> GetTestFontData() {
   std::vector<sk_sp<SkTypeface>> typefaces;
 #if EMBED_TEST_FONT_DATA
-  typefaces.push_back(SkTypeface::MakeFromStream(
+  sk_sp<SkFontMgr> font_mgr = txt::GetDefaultFontManager();
+  typefaces.push_back(font_mgr->makeFromStream(
       SkMemoryStream::MakeDirect(kFlutterTestFont, kFlutterTestFontLength)));
-  typefaces.push_back(SkTypeface::MakeFromStream(
+  typefaces.push_back(font_mgr->makeFromStream(
       SkMemoryStream::MakeDirect(kAhemFont, kAhemFontLength)));
-  typefaces.push_back(SkTypeface::MakeFromStream(
+  typefaces.push_back(font_mgr->makeFromStream(
       SkMemoryStream::MakeDirect(kCoughFont, kCoughFontLength)));
 #endif  // EMBED_TEST_FONT_DATA
   return typefaces;

Original file line number	Diff line number	Diff line change
`@@ -158,6 +158,7 @@ if (enable_unittests) {`
`158`	`158`	`"//flutter/display_list/testing:display_list_testing",`
`159`	`159`	`"//flutter/impeller/typographer/backends/skia:typographer_skia_backend",`
`160`	`160`	`"//flutter/testing",`
	`161`	`+ "//flutter/third_party/txt",`
`161`	`162`	`]`
`162`	`163`
`163`	`164`	`if (!defined(defines)) {`
Original file line number	Diff line number	Diff line change
`@@ -17,6 +17,7 @@ source_set("display_list_testing") {`
`17`	`17`	`deps = [`
`18`	`18`	`"//flutter/skia",`
`19`	`19`	`"//flutter/testing:testing_lib",`
	`20`	`+ "//flutter/third_party/txt",`
`20`	`21`	`]`
`21`	`22`
`22`	`23`	`public_deps = [ "//flutter/display_list:display_list" ]`
Original file line number	Diff line number	Diff line change
`@@ -94,6 +94,7 @@ source_set("flow") {`
`94`	`94`	`"//flutter/common/graphics",`
`95`	`95`	`"//flutter/display_list",`
`96`	`96`	`"//flutter/fml",`
	`97`	`+ "//flutter/third_party/txt",`
`97`	`98`	`]`
`98`	`99`
`99`	`100`	`deps = [ "//flutter/skia" ]`
Original file line number	Diff line number	Diff line change
`@@ -88,6 +88,7 @@ impeller_component("aiks_unittests") {`
`88`	`88`	`"//flutter/impeller/scene",`
`89`	`89`	`"//flutter/impeller/typographer/backends/stb:typographer_stb_backend",`
`90`	`90`	`"//flutter/testing:testing_lib",`
	`91`	`+ "//flutter/third_party/txt",`
`91`	`92`	`]`
`92`	`93`
`93`	`94`	`if (!impeller_trace_canvas) {`
`@@ -119,6 +120,7 @@ impeller_component("aiks_unittests_golden") {`
`119`	`120`	`"//flutter/impeller/scene",`
`120`	`121`	`"//flutter/impeller/typographer/backends/stb:typographer_stb_backend",`
`121`	`122`	`"//flutter/testing:testing_lib",`
	`123`	`+ "//flutter/third_party/txt",`
`122`	`124`	`]`
`123`	`125`	`}`
`124`	`126`
Original file line number	Diff line number	Diff line change
`@@ -63,6 +63,7 @@ impeller_component("display_list_unittests") {`
`63`	`63`	`"../playground:playground_test",`
`64`	`64`	`"//flutter/impeller/scene",`
`65`	`65`	`"//flutter/impeller/typographer/backends/stb:typographer_stb_backend",`
	`66`	`+ "//flutter/third_party/txt",`
`66`	`67`	`]`
`67`	`68`
`68`	`69`	`if (!defined(defines)) {`