Skip to content

QA Scripts & refined source, for Google Fonts onboarding#735

Merged
tonsky merged 0 commit intotonsky:masterfrom
thundernixon:qa
Apr 1, 2019
Merged

QA Scripts & refined source, for Google Fonts onboarding#735
tonsky merged 0 commit intotonsky:masterfrom
thundernixon:qa

Conversation

@thundernixon
Copy link
Copy Markdown
Contributor

Hi @tonsky! I'm not sure whether you actually want to merge this entire PR, but I wanted to ask a few questions around merging some work I've done, and this seems like a logical place to do so.

Context:

I've done work to make a PR to Google Fonts: google/fonts#1911

Part of this was making a variable font from Fira Code, and this required some edits to the GlyphsApp source. There were two main types of edits:

  1. Edits to metadata. I had to change some font info to ensure that exported metadata meets the OpenType spec & GF standards, as checked by FontBakery
  2. Edits to masters and outlines. FontMake (actually, varLib) can't extrapolate when it builds a variable font, so it requires masters that exactly match extreme instances. The Fira Code source had an extrapolated instance for Light, and an interpolated instance for Bold. So, I had to use GlyphsApp's "Instance as Master" feature to make exact masters from the Light and Bold instances, then export. As part of this, I was able to catch and fix some extrapolation and outline issues in the Light master (most were just a natural result of extrapolation): here's a summary of my edits

What's in this PR

My work here includes:

  • An edited GlyphsApp source. I think this should probably be merged, so the upgraded version can be the new source of truth, and so work isn't lost if you upgrade the font more in the future.
  • The googlefonts-qa folder, which includes some scripts and metadata files that are used to build fonts, then add new files to a local google/fonts repo to PR there. This is mostly helpful to the specific work of onboarding (or updating) the font to Google Fonts. I don't think it would hurt to merge this into master to make it obvious to future GF contractors, though we could instead have it in a branch for future use. If you do wish to keep it in a branch to keep master tidy, I can do a separate PR. However, we should probably try to find a way to make that obvious, because it's likely that the next time someone works on this for Google Fonts, it'll be someone other than myself.

Let me know how you'd like me to proceed. Thanks!

@tonsky
Copy link
Copy Markdown
Owner

tonsky commented Apr 1, 2019

This is fantastic! Much more attention than I would probably ever bring to those. I’m merging it fully. All changes seems well justified. And thanks for building VF variant, great addition!

One question though: is this Axis Location intended? It just caught my eye, and I have no idea if that’s something that should be that way or not.

Screenshot 2019-04-01 at 21 48 50

Thanks again!

@tonsky tonsky merged commit 58d6962 into tonsky:master Apr 1, 2019
@thundernixon
Copy link
Copy Markdown
Contributor Author

@tonsky thanks for taking a look and merging it in! Probably does make things simplest in the long run.

is this Axis Location intended?

Ouch, that one definitely looks like a mistake happened at some point. In the source, I had to give masters specific Axis Location custom params to make things build properly, including with leaving the Retina instance intact. However, they were like this:

  • Bold Master: Axis Location: Weight=700
  • Light Master: Axis Location: Weight=300

I'll check on the merged master, and see what's on my end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants