Fixes font-subset to not drop GSUB/GPOS/GDEF tables for variable fonts where they are needed Fixes #125704 (flutter/engine#41592)

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](https://github.com/flutter/flutter/issues/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](https://github.com/flutter/flutter/issues/125704)
for more details).

The issue [#125704](https://github.com/flutter/flutter/issues/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](https://timmaffett.github.io/material_symbols_icons_showing_tree_shake_fixed/)
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](https://timmaffett.github.io/material_symbols_icons_showing_tree_shake_bug/)
).

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..
lol.

Output of build using fixed font-subset.exe [live fix
example](https://timmaffett.github.io/material_symbols_icons_showing_tree_shake_fixed/)
:
```console
flutter build web --release --web-renderer canvaskit --base-href "/material_symbols_icons_showing_tree_shake_fixed/"

Font asset "CupertinoIcons.ttf" was tree-shaken, reducing it from 283452 to 1236 bytes (99.6% reduction). Tree-shaking can be disabled by providing the
--no-tree-shake-icons flag when building your app.
Font asset "MaterialIcons-Regular.otf" was tree-shaken, reducing it from 1645184 to 10808 bytes (99.3% reduction). Tree-shaking can be disabled by providing the
--no-tree-shake-icons flag when building your app.
Font asset "MaterialSymbolsSharp[FILL,GRAD,opsz,wght].ttf" was tree-shaken, reducing it from 5848492 to 5683212 bytes (2.8% reduction). Tree-shaking can be disabled by
providing the --no-tree-shake-icons flag when building your app.
Font asset "MaterialSymbolsOutlined[FILL,GRAD,opsz,wght].ttf" was tree-shaken, reducing it from 6944756 to 6779476 bytes (2.4% reduction). Tree-shaking can be disabled
by providing the --no-tree-shake-icons flag when building your app.
Font asset "MaterialSymbolsRounded[FILL,GRAD,opsz,wght].ttf" was tree-shaken, reducing it from 9361824 to 9196544 bytes (1.8% reduction). Tree-shaking can be disabled
by providing the --no-tree-shake-icons flag when building your app.
Compiling lib\main.dart for the Web...                             79.5s
```

BEFORE font-subset fix [live bug example
here](https://timmaffett.github.io/material_symbols_icons_showing_tree_shake_bug/):
```console
flutter build web --release --web-renderer canvaskit --base-href "/material_symbols_icons_showing_tree_shake_bug/"

Font asset "CupertinoIcons.ttf" was tree-shaken, reducing it from 283452 to 1236 bytes (99.6% reduction). Tree-shaking
can be disabled by providing the --no-tree-shake-icons flag when building your app.
Font asset "MaterialSymbolsSharp[FILL,GRAD,opsz,wght].ttf" was tree-shaken, reducing it from 5848492 to 4079548 bytes
(30.2% reduction). Tree-shaking can be disabled by providing the --no-tree-shake-icons flag when building your app.
Font asset "MaterialSymbolsOutlined[FILL,GRAD,opsz,wght].ttf" was tree-shaken, reducing it from 6944756 to 4781576 bytes(31.1% reduction). Tree-shaking can be disabled by providing the --no-tree-shake-icons flag when building your app.
Font asset "MaterialIcons-Regular.otf" was tree-shaken, reducing it from 1645184 to 10808 bytes (99.3% reduction).
Tree-shaking can be disabled by providing the --no-tree-shake-icons flag when building your app.
Font asset "MaterialSymbolsRounded[FILL,GRAD,opsz,wght].ttf" was tree-shaken, reducing it from 9361824 to 6397020 bytes
(31.7% reduction). Tree-shaking can be disabled by providing the --no-tree-shake-icons flag when building your app.
Compiling lib\main.dart for the Web...                             63.8s
```

engine/src/flutter/tools/font-subset/main.cc

@@ -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));
   }
 };

engine/src/flutter/tools/font-subset/test.py

@@ -28,6 +28,9 @@ PLATFORM_2_PATH = {
 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 @@ COMPARE_TESTS = (
         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 @@ FAIL_TESTS = [
     ]),  # 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
 ]