-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fira code: v1.207 added. #1911
fira code: v1.207 added. #1911
Conversation
This comment has been minimized.
This comment has been minimized.
@tonsky You're welcome to take a look at this, if you like! I know you care about this project a lot, so I want to make sure you have some say before we merge it. Is the variable font working as you expect? Is the html description looking good to you? Are you finding any issues I may have missed? If you're busy with other work; no worries – I think things are solid here, but I just wanted to welcome input from you, if you have any. |
This comment has been minimized.
This comment has been minimized.
@thundernixon thanks for pinging me! Looks great! BTW not sure if this is intentional or not, but shouldn’t this file be called FiraCode.ttf (no -Light suffix)? |
@tonsky cheers! Thanks for taking a look.
Good question! Variable font filenames will soon be converted to a format described in GF issue 1817. For Fira Code, I think that will look like In general, VFs contain some default naming metadata based on their default instance, so that they'll make sense as a fallback in legacy software that doesn't understand the new OpenType tables used for variable fonts. As a random example, if you load the Fira Code VF in a 2011 MS Word, it will (as far as I know) show up as |
Shouldn’t Regular be the default then? |
Another good question! To make this work as a variable font, it was necessary to set up the font file so that it wouldn't rely on extrapolation for the extremes. In your version of the source, there are Regular and Bold masters which output Light through Bold instances. Those can be built as static fonts, but VarLib doesn't currently support such a setup. So, the two choices were to
|
I just dived into your PR and noticed there’s no Regular master anymore. That explains it, thanks! |
d641892
to
a9b7dff
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Fontbakery reportFontbakery version: 0.7.1 [3] Family checks🔥 FAIL: METADATA.pb: According Google Fonts standards, families should have a Regular style.
⚠ WARN: METADATA.pb: Fontfamily is listed on Google Fonts API?
⚠ WARN: Is the command `ftxvalidator` (Apple Font Tool Suite) available?
[10] FiraCode-Light.ttf🔥 FAIL: Checking file is named canonically.
🔥 FAIL: Copyright notices match canonical pattern in METADATA.pb
🔥 FAIL: Copyright notices match canonical pattern in fonts
🔥 FAIL: Variable font weight coordinates must be multiples of 100.
🔥 FAIL: Glyph names are all valid?
⚠ WARN: Checking OS/2 achVendID.
⚠ WARN: Combined length of family and style must not exceed 20 characters.
⚠ WARN: Checking correctness of monospaced metadata.
⚠ WARN: Monospace font has hhea.advanceWidthMax equal to each glyph's advanceWidth?
⚠ WARN: Does GPOS table have kerning information?
Summary
Note: The following loglevels were omitted in this report:
Diff images: qa.zip |
Fontbakery reportFontbakery version: 0.7.1 [3] Family checks🔥 FAIL: METADATA.pb: According Google Fonts standards, families should have a Regular style.
⚠ WARN: METADATA.pb: Fontfamily is listed on Google Fonts API?
⚠ WARN: Is the command `ftxvalidator` (Apple Font Tool Suite) available?
[10] FiraCode-Light.ttf🔥 FAIL: Checking file is named canonically.
🔥 FAIL: Copyright notices match canonical pattern in METADATA.pb
🔥 FAIL: Copyright notices match canonical pattern in fonts
🔥 FAIL: Variable font weight coordinates must be multiples of 100.
🔥 FAIL: Glyph names are all valid?
⚠ WARN: Checking OS/2 achVendID.
⚠ WARN: Combined length of family and style must not exceed 20 characters.
⚠ WARN: Checking correctness of monospaced metadata.
⚠ WARN: Monospace font has hhea.advanceWidthMax equal to each glyph's advanceWidth?
⚠ WARN: Does GPOS table have kerning information?
Summary
Note: The following loglevels were omitted in this report:
Diff images: qa.zip |
The latest update adds fresh statics to the build – previously, these were accidentally left out. |
Fontbakery reportFontbakery version: 0.7.1 [3] Family checks🔥 FAIL: METADATA.pb: According Google Fonts standards, families should have a Regular style.
⚠ WARN: METADATA.pb: Fontfamily is listed on Google Fonts API?
⚠ WARN: Is the command `ftxvalidator` (Apple Font Tool Suite) available?
[10] FiraCode-Light.ttf🔥 FAIL: Checking file is named canonically.
🔥 FAIL: Copyright notices match canonical pattern in METADATA.pb
🔥 FAIL: Copyright notices match canonical pattern in fonts
🔥 FAIL: Variable font weight coordinates must be multiples of 100.
🔥 FAIL: Glyph names are all valid?
⚠ WARN: Checking OS/2 achVendID.
⚠ WARN: Combined length of family and style must not exceed 20 characters.
⚠ WARN: Checking correctness of monospaced metadata.
⚠ WARN: Monospace font has hhea.advanceWidthMax equal to each glyph's advanceWidth?
⚠ WARN: Does GPOS table have kerning information?
Summary
Note: The following loglevels were omitted in this report:
Diff images: qa.zip |
@thundernixon mozilla/Fira#219 is merged! |
The VF contains 5 instances. One of them is the following style, which is in between Regular and Medium:
@garretrieger Will having such an instance cause issues? if not, we can merge this. I've attached the full fvar dump |
We don't rely on named instances, shouldn't matter |
Is this live because I search for it and don't see it. |
@twhite96 There are internal engineering processes that happen to further optimize fonts (especially variable fonts) to make things work with the API / generate CSS, statics, etc. But, this PR merging into the public GF repo means we're a lot closer than before! It also means that the font has gotten some useful improvements, and should be solid if you'd like to use it for self-hosting before it makes it onto the public directory & API. |
@thundernixon okay! No worries. Sent with GitHawk |
Fira Code is a highly popular monospace font, famous for including "code ligatures."
A variable font has been built from it, and a few pieces of metadata have been cleaned up to pass FontBakery checks.
This PR isn't immediately ready to merge – remaining to-do items are:
Upstream Repo: https://github.com/tonsky/FiraCode
Working QA branch: https://github.com/thundernixon/FiraCode/tree/qa
PR Closes: #1460