Bug fixes in the FontMgr (#1838)

docs/docs/text/fonts.md

@@ -8,6 +8,19 @@ slug: /text/fonts
 In Skia, the `FontMgr` object manages a collection of font families.
 It allows you to access fonts from the system and manage custom fonts.
 
+The `matchFont()` function will always return a font.
+So a fast way to display text in React Native Skia would be the following:
+
+```tsx twoslash
+import {Text, matchFont} from "@shopify/react-native-skia";
+
+const Demo = () => {
+  return (
+    <Text text="Hello World" y={32} x={32} font={matchFont()} />
+  );
+};
+```
+
 ## Custom Fonts
 
 The `useFonts` hooks allows you to load custom fonts to be used for your Skia drawing.
@@ -121,11 +134,15 @@ The `fontStyle` object can have the following list of optional attributes:
 
 By default, `matchFont` uses the system font manager to match the font style. However, if you want to use your custom font manager, you can pass it as the second parameter to the `matchFont` function:
 
-```jsx
-const fontMgr = useFonts([
-  require("../../Tests/assets/Roboto-Medium.ttf"),
-  require("../../Tests/assets/Roboto-Bold.ttf"),
-]);
+```jsx twoslash
+import {matchFont, useFonts} from "@shopify/react-native-skia";
+
+const fontMgr = useFonts({
+  Roboto: [
+    require("../../Tests/assets/Roboto-Medium.ttf"),
+    require("../../Tests/assets/Roboto-Bold.ttf"),
+  ]
+});
 
 const font = matchFont(fontStyle, fontMgr);
 ```

example/src/Examples/API/FontMgr.tsx

@@ -70,8 +70,7 @@ export const FontMgr = () => {
         })}
         <Text font={titleFont} text="Custom Fonts" x={PADDING} y={title2Y} />
         {customfamilyNames.map((fontFamily, i) => {
-          const font = matchFont({ fontFamily }, customFontMgr);
-
+          const font = matchFont({ fontFamily, fontSize: 16 }, customFontMgr);
           const resolvedFont =
             font.getGlyphIDs(fontFamily)[0] === 0 ? subtitleFont : font;
           return (
@@ -84,6 +83,12 @@ export const FontMgr = () => {
             />
           );
         })}
+        <Text
+          font={matchFont()}
+          x={PADDING}
+          y={title2Y + 16 * (customfamilyNames.length + 1)}
+          text="Default font"
+        />
       </Canvas>
     </ScrollView>
   );

package/cpp/api/JsiSkFontMgr.h

@@ -51,4 +51,4 @@ class JsiSkFontMgr : public JsiSkWrappingSkPtrHostObject<SkFontMgr> {
                        JSI_EXPORT_FUNC(JsiSkFontMgr, matchFamilyStyle))
 };
 
-} // namespace RNSkia
+} // namespace RNSkia

package/cpp/api/JsiSkFontStyle.h

@@ -36,12 +36,18 @@ class JsiSkFontStyle : public JsiSkWrappingSharedPtrHostObject<SkFontStyle> {
     if (object.isHostObject(runtime)) {
       return object.asHostObject<JsiSkFontStyle>(runtime)->getObject();
     } else {
-      auto weight =
-          static_cast<int>(object.getProperty(runtime, "weight").asNumber());
-      auto width =
-          static_cast<int>(object.getProperty(runtime, "width").asNumber());
+      auto weightProp = object.getProperty(runtime, "weight");
+      auto weight = static_cast<int>(weightProp.isUndefined()
+                                         ? SkFontStyle::Weight::kNormal_Weight
+                                         : weightProp.asNumber());
+      auto widthProp = object.getProperty(runtime, "width");
+      auto width = static_cast<int>(widthProp.isUndefined()
+                                        ? SkFontStyle::Width::kNormal_Width
+                                        : widthProp.asNumber());
+      auto slantProp = object.getProperty(runtime, "slant");
       auto slant = static_cast<SkFontStyle::Slant>(
-          object.getProperty(runtime, "slant").asNumber());
+          slantProp.isUndefined() ? SkFontStyle::Slant::kUpright_Slant
+                                  : slantProp.asNumber());
       SkFontStyle style(weight, width, slant);
       return std::make_shared<SkFontStyle>(style);
     }

package/cpp/api/JsiSkTypeFaceFontProvider.h

@@ -27,7 +27,8 @@ namespace para = skia::textlayout;
 class JsiSkTypefaceFontProvider
     : public JsiSkWrappingSkPtrHostObject<para::TypefaceFontProvider> {
 public:
-  EXPORT_JSI_API_TYPENAME(JsiSkTypefaceFontProvider, FontMgr)
+  EXPORT_JSI_API_TYPENAME(JsiSkTypefaceFontProvider, TypefaceFontProvider)
+
   JSI_EXPORT_FUNCTIONS(
       JSI_EXPORT_FUNC(JsiSkTypefaceFontProvider, dispose),
       JSI_EXPORT_FUNC(JsiSkTypefaceFontProvider, registerFont),
@@ -46,7 +47,8 @@ class JsiSkTypefaceFontProvider
   JSI_HOST_FUNCTION(matchFamilyStyle) {
     auto name = arguments[0].asString(runtime).utf8(runtime);
     auto fontStyle = JsiSkFontStyle::fromValue(runtime, arguments[1]);
-    auto typeface = getObject()->matchFamilyStyle(name.c_str(), *fontStyle);
+    sk_sp<SkFontStyleSet> set(getObject()->onMatchFamily(name.c_str()));
+    sk_sp<SkTypeface> typeface(set->matchStyle(*fontStyle));
     return jsi::Object::createFromHostObject(
         runtime, std::make_shared<JsiSkTypeface>(getContext(), typeface));
   }

package/cpp/api/JsiSkTypeface.h

@@ -2,6 +2,7 @@
 
 #include <memory>
 #include <utility>
+#include <vector>
 
 #include <jsi/jsi.h>
 
@@ -23,12 +24,33 @@ namespace jsi = facebook::jsi;
 class JsiSkTypeface : public JsiSkWrappingSkPtrHostObject<SkTypeface> {
 public:
   EXPORT_JSI_API_TYPENAME(JsiSkTypeface, Typeface)
-  JSI_EXPORT_FUNCTIONS(JSI_EXPORT_FUNC(JsiSkTypeface, dispose))
+  JSI_EXPORT_FUNCTIONS(JSI_EXPORT_FUNC(JsiSkTypeface, getGlyphIDs),
+                       JSI_EXPORT_FUNC(JsiSkTypeface, dispose))
 
   JsiSkTypeface(std::shared_ptr<RNSkPlatformContext> context,
                 sk_sp<SkTypeface> typeface)
       : JsiSkWrappingSkPtrHostObject(std::move(context), std::move(typeface)) {}
 
+  JSI_HOST_FUNCTION(getGlyphIDs) {
+    auto str = arguments[0].asString(runtime).utf8(runtime);
+    int numGlyphIDs =
+        count > 1 && !arguments[1].isNull() && !arguments[1].isUndefined()
+            ? static_cast<int>(arguments[1].asNumber())
+            : getObject()->textToGlyphs(str.c_str(), str.length(),
+                                        SkTextEncoding::kUTF8, nullptr, 0);
+    std::vector<SkGlyphID> glyphIDs;
+    glyphIDs.resize(numGlyphIDs);
+    getObject()->textToGlyphs(str.c_str(), str.length(), SkTextEncoding::kUTF8,
+                              static_cast<SkGlyphID *>(glyphIDs.data()),
+                              numGlyphIDs);
+    auto jsiGlyphIDs = jsi::Array(runtime, numGlyphIDs);
+    for (int i = 0; i < numGlyphIDs; i++) {
+      jsiGlyphIDs.setValueAtIndex(runtime, i,
+                                  jsi::Value(static_cast<int>(glyphIDs[i])));
+    }
+    return jsiGlyphIDs;
+  }
+
   /**
    Returns the jsi object from a host object of this type
   */

package/src/renderer/__tests__/e2e/FontMgr.spec.tsx

@@ -2,31 +2,53 @@ import { itRunsE2eOnly } from "../../../__tests__/setup";
 import { surface, testingFonts } from "../setup";
 import { FontStyle } from "../../../skia/types";
 
+// Currently this tests only run on iOS and Android
+// We've contributed a patch to CanvasKit to make it work on Web too in the next release:
+// https://github.com/google/skia/blob/main/modules/canvaskit/CHANGELOG.md#unreleased
 describe("FontMgr", () => {
-  it("Custom font manager should work on every platform", async () => {
-    const names = await surface.eval(
-      (Skia, { fonts }) => {
-        const fontMgr = Skia.TypefaceFontProvider.Make();
-        (Object.keys(fonts) as (keyof typeof fonts)[]).flatMap((familyName) => {
-          const typefaces = fonts[familyName];
-          typefaces.forEach((typeface) => {
-            const data = Skia.Data.fromBytes(new Uint8Array(typeface));
-            fontMgr.registerFont(
-              Skia.Typeface.MakeFreeTypeFaceFromData(data)!,
-              familyName
-            );
-          });
-        });
-        return new Array(fontMgr.countFamilies())
-          .fill(0)
-          .map((_, i) => fontMgr.getFamilyName(i));
-      },
-      { fonts: testingFonts }
-    );
-    expect(names.length).toBeGreaterThan(0);
-    expect(names.indexOf("Helvetica")).toBe(-1);
-    expect(names.indexOf("Roboto")).not.toBe(-1);
-  });
+  itRunsE2eOnly(
+    "Custom font manager should work on every platform",
+    async () => {
+      const result = await surface.eval(
+        (Skia, { fonts }) => {
+          const fontMgr = Skia.TypefaceFontProvider.Make();
+          (Object.keys(fonts) as (keyof typeof fonts)[]).flatMap(
+            (familyName) => {
+              const typefaces = fonts[familyName];
+              typefaces.forEach((typeface) => {
+                const data = Skia.Data.fromBytes(new Uint8Array(typeface));
+                fontMgr.registerFont(
+                  Skia.Typeface.MakeFreeTypeFaceFromData(data)!,
+                  familyName
+                );
+              });
+            }
+          );
+          const familyNames = new Array(fontMgr.countFamilies())
+            .fill(0)
+            .map((_, i) => fontMgr.getFamilyName(i));
+
+          const identities = new Array(fontMgr.countFamilies())
+            .fill(0)
+            .map((_, i) => {
+              const t1 = fontMgr.matchFamilyStyle(fontMgr.getFamilyName(i), {
+                weight: 400,
+              });
+              const t2 = fontMgr.matchFamilyStyle(fontMgr.getFamilyName(i), {
+                weight: 900,
+              });
+              return t1 !== t2;
+            });
+          return { familyNames, identities };
+        },
+        { fonts: testingFonts }
+      );
+      expect(result.identities).toEqual([true]);
+      expect(result.familyNames.length).toBeGreaterThan(0);
+      expect(result.familyNames.indexOf("Helvetica")).toBe(-1);
+      expect(result.familyNames.indexOf("Roboto")).not.toBe(-1);
+    }
+  );
   itRunsE2eOnly("system font managers have at least one font", async () => {
     const names = await surface.eval((Skia) => {
       const fontMgr = Skia.FontMgr.System();
@@ -79,4 +101,7 @@ describe("FontMgr", () => {
       expect(width).not.toEqual([0, 0]);
     }
   });
+  // Add test
+  //     *  Passing |nullptr| as the parameter for |familyName| will return the
+  // *  default system font.
 });

package/src/skia/types/Font/FontMgr.ts

@@ -6,5 +6,5 @@ import type { FontStyle } from "./Font";
 export interface SkFontMgr extends SkJSIInstance<"FontMgr"> {
   countFamilies(): number;
   getFamilyName(index: number): string;
-  matchFamilyStyle(name: string, style: FontStyle): SkTypeface;
+  matchFamilyStyle(name?: string, style?: FontStyle): SkTypeface;
 }

package/src/skia/types/Typeface/Typeface.ts

@@ -1,3 +1,12 @@
 import type { SkJSIInstance } from "../JsiInstance";
 
-export type SkTypeface = SkJSIInstance<"Typeface">;
+export interface SkTypeface extends SkJSIInstance<"Typeface"> {
+  /**
+   * Retrieves the glyph ids for each code point in the provided string. This call is passed to
+   * the typeface of this font. Note that glyph IDs are typeface-dependent; different faces
+   * may have different ids for the same code point.
+   * @param str
+   * @param numCodePoints - the number of code points in the string. Defaults to str.length.
+   */
+  getGlyphIDs(str: string, numCodePoints?: number): number[];
+}

package/src/skia/web/JsiSkTypeface.ts

@@ -26,6 +26,10 @@ export class JsiSkTypeface
     return false;
   }
 
+  getGlyphIDs(str: string, numCodePoints?: number) {
+    return Array.from(this.ref.getGlyphIDs(str, numCodePoints));
+  }
+
   dispose = () => {
     this.ref.delete();
   };