font-subset removes necessary GSUB table for variable fonts resulting in font rendering errors #125704
Closed
2 tasks done
Comments
dnfield
pushed a commit
to flutter/engine
that referenced
this issue
May 1, 2023
…s where they are needed Fixes #125704 (#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](flutter/flutter#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](flutter/flutter#125704) for more details). The issue [#125704](flutter/flutter#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-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this issue
May 1, 2023
…iable fonts where they are needed Fixes flutter#125704 (flutter/engine#41592)
auto-submit bot
pushed a commit
that referenced
this issue
May 1, 2023
…125839) flutter/engine@58cc541...3fa6084 2023-05-01 timmaffett@gmail.com Fixes font-subset to not drop GSUB/GPOS/GDEF tables for variable fonts where they are needed Fixes #125704 (flutter/engine#41592) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC jsimmons@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
|
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Is there an existing issue for this?
Steps to reproduce
My package material_symbols_icons provides support for the Material Symbols Icons using the variable fonts from https://github.com/google/material-design-icons. (The variable fonts are found here ).
This package addresses material symbols icon support as defined in #102560.
When building a web release of the example from this package the icon font tree-shaking is performed. (this is now the default). This breaks these variable fonts when rendering some axes variations.
Here is the correct working example built using using
--no-tree-shake-iconsflagYou could also checkout the material_symbols_icons and build the example using
or for working version building without icon tree shaking:
Expected results
Actual results
Code sample
Code sample
[Paste your code here]Screenshots or Video
Screenshots / Video demonstration
[Upload media here]
Logs
Logs
This is the results of the release build of the web example. Note icon tree-shake is resulting in 30% savings even though the example uses every icon. This is caused by
font-subsetdropping the GSUB font table, which is needed for these variable fonts to properly render all variation axes.Flutter Doctor output
Doctor output
The text was updated successfully, but these errors were encountered: