papandreou / subset-font

Create a subset of a TrueType/OpenType/WOFF/WOFF2 font using the wasm build of harfbuzz/hb-subset
BSD 3-Clause "New" or "Revised" License
85 stars 6 forks source link

Update harfbuzzjs to ^0.2.0 (harfbuzz 3.0.0) #13

Closed papandreou closed 2 years ago

papandreou commented 2 years ago

Discussion here: https://github.com/papandreou/subset-font/issues/12

coveralls commented 2 years ago

Pull Request Test Coverage Report for Build 1443083249


Changes Missing Coverage Covered Lines Changed/Added Lines %
index.js 13 14 92.86%
<!-- Total: 13 14 92.86% -->
Files with Coverage Reduction New Missed Lines %
index.js 5 87.88%
<!-- Total: 5 -->
Totals Coverage Status
Change from base Build 997199723: -12.1%
Covered Lines: 48
Relevant Lines: 54

💛 - Coveralls
papandreou commented 2 years ago

@ebraminio, thanks for the help! I'm still a bit worried about that failing reference image test in the subfont test suite, so I tried creating the subset font before/after upgrading to harfbuzzjs 0.2.0. Here's the diff of the ttx representations as well as the files themselves: https://gist.github.com/papandreou/cca1b1a17adb7f60b1f7794bea608f6f

Any idea what's going on?

Rendering of the original web page (or when subset with harfbuzzjs 0.1.5)

before

Rendering of the web page when subset with harfbuzzjs 0.2.0

after

subfont --debug says that these glyphs are being included in the subset, in case that helps:

[
  {
    assetFileName: 'testdata/referenceImages/fontVariant/index.html',
    fontUsages: [
      {
        smallestOriginalSize: 77420,
        smallestOriginalFormat: 'woff',
        texts: [
          '   a á ă â ä ạ à ả ā ą å ǻ ã ắ ặ ằ ẳ ẵ ấ ậ ầ ẩ ẫ ğ ĝ ģ ġ ß α ά а ӓ ӑ ¯ ˙ ¨ ˝ ´ ` ˆ ˇ ˚ ΄ ̉     g g 0 0 ˜ ˜ ˜ ˘ ˘ ˘ ',
          '   0 1 2 3 4 5 6 7 8 9 ',
          '   0 1 2 3 4 5 6 7 8 9 /   ⁄ ₀ ₁ ₂ ₃ ₄ ₅ ₆ ₇ ₈ ₉   ² ³ ¹ ⁰ ⁴ ⁵ ⁶ ⁷ ⁸ ⁹   ⁄² ⁄³ ⁄¹ ⁄⁰ ⁄⁴ ⁄⁵ ⁄⁶ ⁄⁷ ⁄⁸ ⁄⁹ ₀² ₀³ ₀¹ ₀⁰ ₀⁴ ₀⁵ ₀⁶ ₀⁷ ₀⁸ ₀⁹ ₁² ₁³ ₁¹ ₁⁰ ₁⁴ ₁⁵ ₁⁶ ₁⁷ ₁⁸ ₁⁹ ₂² ₂³ ₂¹ ₂⁰ ₂⁴ ₂⁵ ₂⁶ ₂⁷ ₂⁸ ₂⁹ ₃² ₃³ ₃¹ ₃⁰ ₃⁴ ₃⁵ ₃⁶ ₃⁷ ₃⁸ ₃⁹ ₅² ₅³ ₅¹ ₅⁰ ₅⁴ ₅⁵ ₅⁶ ₅⁷ ₅⁸ ₅⁹ ₆² ₆³ ₆¹ ₆⁰ ₆⁴ ₆⁵ ₆⁶ ₆⁷ ₆⁸ ₆⁹ ₇² ₇³ ₇¹ ₇⁰ ₇⁴ ₇⁵ ₇⁶ ₇⁷ ₇⁸ ₇⁹ ₈² ₈³ ₈¹ ₈⁰ ₈⁴ ₈⁵ ₈⁶ ₈⁷ ₈⁸ ₈⁹ ₉² ₉³ ₉¹ ₉⁰ ₉⁴ ₉⁵ ₉⁶ ₉⁷ ₉⁸ ₉⁹ ',
          '   a o ',
          '   a g 0 á ă â ä ạ à ả ā ą å ǻ ã ắ ặ ằ ẳ ẵ ấ ậ ầ ẩ ẫ ğ ĝ ģ ġ ß α ά а ӓ ӑ ',
          '   a á ă â ä ạ à ả ā ą å ǻ ã ắ ặ ằ ẳ ẵ ấ ậ ầ ẩ ẫ α ά а ӓ ӑ ',
          '   g ğ ĝ ģ ġ ',
          '   0 ',
          '   ß '
        ],
        pageText: ' /0123456789`ago¨¯²³´¹ßàáâãäåāăąĝğġģǻˆˇ˘˙˚˜˝̉΄άαаӑӓạảấầẩẫậắằẳẵặ⁄⁰⁴⁵⁶⁷⁸⁹₀₁₂₃₄₅₆₇₈₉',
        text: ' /0123456789`ago¨¯²³´¹ßàáâãäåāăąĝğġģǻˆˇ˘˙˚˜˝̉΄άαаӑӓạảấầẩẫậắằẳẵặ⁄⁰⁴⁵⁶⁷⁸⁹₀₁₂₃₄₅₆₇₈₉',
        props: {
          'font-stretch': 'normal',
          'font-weight': 'normal',
          'font-style': 'normal',
          'font-family': 'IBMPlexSans',
          src: 'url(IBMPlexSans-Regular.woff) format("woff")',
          'font-display': 'swap'
        },
        fontUrl: 'file:///home/andreas/work/subfont/testdata/referenceImages/fontVariant/IBMPlexSans-Regular.woff',
        fontFamilies: Set { 'IBMPlexSans' },
        preload: true,
        codepoints: {
          original: [
              0,  13,  32,  33,  34,  35,  36,  37,  38,  39,  40,  41,
             42,  43,  44,  45,  46,  47,  48,  49,  50,  51,  52,  53,
             54,  55,  56,  57,  58,  59,  60,  61,  62,  63,  64,  65,
             66,  67,  68,  69,  70,  71,  72,  73,  74,  75,  76,  77,
             78,  79,  80,  81,  82,  83,  84,  85,  86,  87,  88,  89,
             90,  91,  92,  93,  94,  95,  96,  97,  98,  99, 100, 101,
            102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113,
            114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125,
            126, 160, 161, 162,
            ... 727 more items
          ],
          used: [
               32,    47,   48,   49,   50,   51,   52,   53,   54,
               55,    56,   57,   96,   97,  103,  111,  168,  175,
              178,   179,  180,  185,  223,  224,  225,  226,  227,
              228,   229,  257,  259,  261,  285,  287,  289,  291,
              507,   710,  711,  728,  729,  730,  732,  733,  777,
              900,   940,  945, 1072, 1233, 1235, 7841, 7843, 7845,
             7847,  7849, 7851, 7853, 7855, 7857, 7859, 7861, 7863,
             8260,  8304, 8308, 8309, 8310, 8311, 8312, 8313, 8320,
             8321,  8322, 8323, 8324, 8325, 8326, 8327, 8328, 8329,
            63191, 63192
          ],
          unused: [
              0,  13,  33,  34,  35,  36,  37,  38,  39,  40,  41,  42,
             43,  44,  45,  46,  58,  59,  60,  61,  62,  63,  64,  65,
             66,  67,  68,  69,  70,  71,  72,  73,  74,  75,  76,  77,
             78,  79,  80,  81,  82,  83,  84,  85,  86,  87,  88,  89,
             90,  91,  92,  93,  94,  95,  98,  99, 100, 101, 102, 104,
            105, 106, 107, 108, 109, 110, 112, 113, 114, 115, 116, 117,
            118, 119, 120, 121, 122, 123, 124, 125, 126, 160, 161, 162,
            163, 164, 165, 166, 167, 169, 170, 171, 172, 173, 174, 176,
            177, 181, 182, 183,
            ... 644 more items
          ],
          page: [
               32,    47,   48,   49,   50,   51,   52,   53,   54,
               55,    56,   57,   96,   97,  103,  111,  168,  175,
              178,   179,  180,  185,  223,  224,  225,  226,  227,
              228,   229,  257,  259,  261,  285,  287,  289,  291,
              507,   710,  711,  728,  729,  730,  732,  733,  777,
              900,   940,  945, 1072, 1233, 1235, 7841, 7843, 7845,
             7847,  7849, 7851, 7853, 7855, 7857, 7859, 7861, 7863,
             8260,  8304, 8308, 8309, 8310, 8311, 8312, 8313, 8320,
             8321,  8322, 8323, 8324, 8325, 8326, 8327, 8328, 8329,
            63191, 63192
          ]
        },
        smallestSubsetSize: 6752,
        smallestSubsetFormat: 'woff2'
      }
    ]
  }
]
ebraminio commented 2 years ago

Any idea what's going on?

Unfortunately I'm no longer involved with the upstream :/ comparing fonttools's ttx result of the two should give a good clue.

papandreou commented 2 years ago

@ebraminio, oh, sorry to hear that 😿

The above gist does include a ttx diff, but it's rather big, so it isn't clear to me what the actual issue might be. I guess I'll spend some time looking at it and maybe report it upstream if I can reproduce it with the hb_subset binary.

papandreou commented 2 years ago

Hmm, I can reproduce it with just the hb-subset binary wired into subfont instead. That setup also reproduces it with harfbuzz 3.1.1 and 3.0.0, but 2.8.1 (which harfbuzzjs was using before) is fine.

git bisect says that the behavior changed in https://github.com/harfbuzz/harfbuzz/commit/cb5a6b5a27cfe616113bafe7f23ad33f1b0d0a1e:

cb5a6b5a27cfe616113bafe7f23ad33f1b0d0a1e is the first bad commit
commit cb5a6b5a27cfe616113bafe7f23ad33f1b0d0a1e
Author: Qunxin Liu <qxliu@google.com>
Date:   Wed May 19 17:33:46 2021 -0700

    [subset] support option --layout-features

Passing --layout-features=aalt,dnom,frac,kern fixes it for the particular test case, but I guess we should just pass --layout-features=* as we did to pyftsubset -- that also works and should be safer.

We can ape what hb-subset --layout-features=* does:

diff --git a/index.js b/index.js
index ba08ff2..e079161 100644
--- a/index.js
+++ b/index.js
@@ -49,6 +49,14 @@ async function subsetFont(
   const face = exports.hb_face_create(blob, 0);
   exports.hb_blob_destroy(blob);

+  // Do the equivalent of --font-features=*
+  const layoutFeatures = exports.hb_subset_input_set(
+    input,
+    6 // HB_SUBSET_SETS_LAYOUT_FEATURE_TAG
+  );
+  exports.hb_set_clear(layoutFeatures);
+  exports.hb_set_invert(layoutFeatures);
+
   if (preserveNameIds) {
     const inputNameIds = exports.hb_subset_input_set(
       input,

... but that requires harfbuzzjs to expose hb_set_clear and hb_set_invert.

ebraminio commented 2 years ago

but that requires harfbuzzjs to expose hb_set_clear and hb_set_invert.

just published 0.2.1 with them :)

papandreou commented 2 years ago

Released in 1.4.0. Thanks for all the help, @ebraminio :hugs: