This PR fixes font-subset to check to see if a font is a variable font with variable font axes before additionally dropping the GSUB/GPOS/GDEF tables. These tables were being forced dropped in all cases (even though harfbuzz had been modified to always keep them). I made the change only drop the tables for variable fonts to preserve the previous behavior in the most possible cases.

This PR fixes #125704.

To see the bug (or verify it is fixed) in the live web examples below you must select the font variations Fill->1 and Weight->100.
(See issue #125704 for more details).

The issue #125704 includes examples of the font-subset being used (and breaking) the variable fonts, as well as example of the --no-tree-shake-icons being used where the fonts do not break.

Additionally, I have created an additional live example where this PR has been applied to font-subset and icon tree shaking is still taking place.

(Example w/ icon tree-shaking using the broken font-subset for icon tree shaking is found here ).

In the example build output below note that the non-variable fonts "CupertinoIcons.ttf" and "MaterialIcons-Regular.otf" have the same size savings as before the change, but the variable fonts "MaterialSymbolsSharp[FILL,GRAD,opsz,wght].ttf", "MaterialSymbolsOutlined[FILL,GRAD,opsz,wght].ttf", and "MaterialSymbolsRounded[FILL,GRAD,opsz,wght].ttf" now have a much more reasonable saving of ~2% because every icon in the font is included in the example. The previous extra ~30% savings was from having the GSUB table removed. The 30% size savings for a tree-shaking for a case where every icon is used in the example probably should have been suspect.

Author: dnfield

tools/font-subset/fixtures/1variable.ttf

@@ -59,14 +59,20 @@ struct HarfBuzzSubset {
     // The prior version of harfbuzz automatically dropped layout tables,
     // but in the new version they are kept by default. So re-add them to the
     // drop list to retain the same behaviour.
-
-    hb_set_add(hb_subset_input_set(input, HB_SUBSET_SETS_DROP_TABLE_TAG),
-               HB_TAG('G', 'S', 'U', 'B'));
-    hb_set_add(hb_subset_input_set(input, HB_SUBSET_SETS_DROP_TABLE_TAG),
-               HB_TAG('G', 'P', 'O', 'S'));
-    hb_set_add(hb_subset_input_set(input, HB_SUBSET_SETS_DROP_TABLE_TAG),
-               HB_TAG('G', 'D', 'E', 'F'));
-
+    if (!hb_ot_var_has_data(face) || hb_ot_var_get_axis_count(face) == 0) {
+      // we can only drop GSUB/GPOS/GDEF for non variable fonts, they may be
+      // needed for variable fonts (guessing we need to keep all of these, but
+      // in Material Symbols Icon variable fonts if we drop the GSUB table (they
+      // do not have GPOS/DEF) then the Fill=1,Weight=100 variation is rendered
+      // incorrect. (and other variations are probably less noticibly
+      // incorrect))
+      hb_set_add(hb_subset_input_set(input, HB_SUBSET_SETS_DROP_TABLE_TAG),
+                 HB_TAG('G', 'S', 'U', 'B'));
+      hb_set_add(hb_subset_input_set(input, HB_SUBSET_SETS_DROP_TABLE_TAG),
+                 HB_TAG('G', 'P', 'O', 'S'));
+      hb_set_add(hb_subset_input_set(input, HB_SUBSET_SETS_DROP_TABLE_TAG),
+                 HB_TAG('G', 'D', 'E', 'F'));
+    }
     return HarfbuzzWrappers::HbFacePtr(hb_subset_or_fail(face, input));
   }
 };

tools/font-subset/fixtures/2variable.ttf

@@ -28,6 +28,9 @@
 SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
 SRC_DIR = os.path.normpath(os.path.join(SCRIPT_DIR, '..', '..', '..'))
 MATERIAL_TTF = os.path.join(SCRIPT_DIR, 'fixtures', 'MaterialIcons-Regular.ttf')
+VARIABLE_MATERIAL_TTF = os.path.join(
+    SCRIPT_DIR, 'fixtures', 'MaterialSymbols-Variable.ttf'
+)
 IS_WINDOWS = sys.platform.startswith(('cygwin', 'win'))
 EXE = '.exe' if IS_WINDOWS else ''
 BAT = '.bat' if IS_WINDOWS else ''
@@ -67,6 +70,27 @@
         r'0xE004',
         r'0xE021',
     ]),
+    # repeat tests with variable input font and verified variable output goldens
+    (True, '1variable.ttf', VARIABLE_MATERIAL_TTF, [r'57347']),
+    (True, '1variable.ttf', VARIABLE_MATERIAL_TTF, [r'0xE003']),
+    (True, '1variable.ttf', VARIABLE_MATERIAL_TTF, [r'\uE003']),
+    (False, '1variable.ttf', VARIABLE_MATERIAL_TTF,
+     [r'57348']),  # False because different codepoint
+    (True, '2variable.ttf', VARIABLE_MATERIAL_TTF, [r'0xE003', r'0xE004']),
+    (
+        True, '2variable.ttf', VARIABLE_MATERIAL_TTF, [
+            r'0xE003',
+            r'0xE004',
+            r'57347',
+        ]
+    ),  # Duplicated codepoint
+    (
+        True, '3variable.ttf', VARIABLE_MATERIAL_TTF, [
+            r'0xE003',
+            r'0xE004',
+            r'0xE021',
+        ]
+    ),
 )
 
 FAIL_TESTS = [
@@ -95,6 +119,29 @@
     ]),  # empty input
     ([FONT_SUBSET, 'output.ttf', MATERIAL_TTF], []),  # empty input
     ([FONT_SUBSET, 'output.ttf', MATERIAL_TTF], ['']),  # empty input
+    # repeat tests with variable input font
+    ([FONT_SUBSET, 'output.ttf', VARIABLE_MATERIAL_TTF], [
+        '0xFFFFFFFF',
+    ]),  # Value too big.
+    ([FONT_SUBSET, 'output.ttf', VARIABLE_MATERIAL_TTF], [
+        '-1',
+    ]),  # invalid value
+    ([FONT_SUBSET, 'output.ttf', VARIABLE_MATERIAL_TTF], [
+        'foo',
+    ]),  # no valid values
+    ([FONT_SUBSET, 'output.ttf', VARIABLE_MATERIAL_TTF], [
+        '0xE003',
+        '0x12',
+        '0xE004',
+    ]),  # codepoint not in font
+    ([FONT_SUBSET, 'non-existent-dir/output.ttf', VARIABLE_MATERIAL_TTF], [
+        '0xE003',
+    ]),  # dir doesn't exist
+    ([FONT_SUBSET, 'output.ttf', VARIABLE_MATERIAL_TTF], [
+        ' ',
+    ]),  # empty input
+    ([FONT_SUBSET, 'output.ttf', VARIABLE_MATERIAL_TTF], []),  # empty input
+    ([FONT_SUBSET, 'output.ttf', VARIABLE_MATERIAL_TTF], ['']),  # empty input
 ]