Skip to content
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

Font with default weight of 300 is built with OS/2 weight of 400 #540

Closed
thundernixon opened this issue Mar 26, 2019 · 13 comments
Closed

Font with default weight of 300 is built with OS/2 weight of 400 #540

thundernixon opened this issue Mar 26, 2019 · 13 comments

Comments

@thundernixon
Copy link

thundernixon commented Mar 26, 2019

I'm working to onboard Fira Code to Google Fonts. I've gotten past most issues, but this one has me confused ...

Fira Code has a two-master setup, Light and Bold. The default axis is at weight: 300, but in the OS/2 table, it classifies its weight as 400:

<usWeightClass value="400"/>

This results in the following FontBakery fail:

🔥 FAIL: Checking OS/2 usWeightClass.

In other variable fonts I've built with FontMake (Encode Sans, Signika, Libre Caslon Text), this value has corresponded to the default instance.

Is there something I'm missing in the Fira Code source that is causing this value to be set in an unexpected way? Or, could this somehow be due to an issue in FontMake or FontTools?

To Reproduce

If you wish to reproduce, please follow build instructions at https://github.com/thundernixon/FiraCode/tree/qa/googlefonts-qa.

Context

FontMake 1.9.1
Python 3.7.2

@madig
Copy link
Collaborator

madig commented Mar 27, 2019

Something is weird with the axis setup. If you run glyphs2ufo FiraCode.glyphs, the Designspace file will contain:

  <axes>
    <axis tag="wght" name="Weight" minimum="300" maximum="700" default="432.352941">
      <map input="300" output="62"/>
      <map input="400" output="132"/>
      <map input="450" output="96"/>
      <map input="500" output="112"/>
      <map input="700" output="158"/>
    </axis>
  </axes>

Note the non-progression in the output column, denoting design space coordinates. Check the coordinates of all your masters and instances and possibly Axis Location parameters.

@anthrotype
Copy link
Member

I haven't checked, but this is probably related fonttools/fontbakery#2420 (comment)

@thundernixon
Copy link
Author

thundernixon commented Mar 27, 2019

@madig Huh, that's interesting. I can confirm that glyphs2ufo gives me the same result.

However, the designspace output by FontMake into master_ufo doesn't seem to have the same issue:

<axes>
    <axis tag="wght" name="Weight" minimum="300" maximum="700" default="300">
      <map input="300" output="62"/>
      <map input="400" output="84"/>
      <map input="450" output="96"/>
      <map input="500" output="112"/>
      <map input="700" output="158"/>
    </axis>
</axes>

As referred to in the issue pointed out by @anthrotype (and is shown in the code above), Fira Code does have an instance at weight=450. However, even if I mark this instance as not active in Glyphs (preventing it from exporting), the OS/2 weight is still 400 rather than 300, as I would expect.

In fact, even if I delete the "Retina" instance from the GlyphsApp source entirely, the problem persists. I have also deleted the non-active "SemiBold" instance as an experiment, but the OS/2 value is still off.

@anthrotype
Copy link
Member

try adding a weightClass=300 custom parameter to the default Light master, see if glyphsLib exports that to the generated master UFO. Ufo2ft should then apply it to the master TTF for the default VF instance.

@anthrotype
Copy link
Member

actually, that may be a bit of a hack (weightClass custom parameter is for instances, not masters).
Are you using the "Axis Location" custom parameter for the masters?
That's supposed to be the location (in user-space coordinates) that each master is assigned along each axis. GlyphsLib should use those to create the <axes> maps, and -- I would expect -- also set the master's OS/2.WeightClass.

@thundernixon
Copy link
Author

try adding a weightClass=300 custom parameter to the default Light master

Actually, that did it!

image

The OS/2 table now includes:

<usWeightClass value="300"/>

I didn't think that a grayed-out custom parameter in a master would have any effect, but it seems to have solved my problem. However...

Are you using the "Axis Location" custom parameter for the masters?

I wasn't, but this also solves the issue, and does indeed seem like much less of a hack.

image

Interestingly, my other GlyphsApp sources haven't (as far as I can tell) needed this custom parameter in masters. Do you find that leaving it out tends to be a problem?

Thanks for helping me debug my issue, @anthrotype!


This still seems like a sub-optimal result for FontMake (I don't see why a custom parameter is needed in the master, especially when all instances are already marked with weightClass), but if you think this is working as it should, feel free to close the issue. I'll suggest that FontBakery adds the suggestion of adding an Axis Location custom parameter to masters in GlyphsApp, to help the next person who runs into this.

@anthrotype
Copy link
Member

yes, i recommend you use both the Axes (font custom parameter) and Axis Location (master custom parameter) so that glyphsLib doesn't have to guess those values from the instances.

@madig
Copy link
Collaborator

madig commented Mar 27, 2019

I'm confused. The light master from https://github.com/thundernixon/FiraCode/blob/master/sources/FiraCode.glyphs looks quite regular and the source location is set to 94 or something. Why is your light at 62?

Keep in mind that glyphsLib interprets parameters itself and not always like Glyphs does, so a grayed out option in Glyphs means little.

@anthrotype
Copy link
Member

and don't use the greyed-out weightClass for the masters, that's reserved for instances. The fact that it works in glyphsLib is sort of unintended.

@thundernixon
Copy link
Author

@madig: Why is your light at 62?

The link you posted is not the source I am working from. I am working from the GlyphsApp source in my qa branch: https://github.com/thundernixon/FiraCode/blob/qa/FiraCode.glyphs

It isn't possible (as far as I know) to build a variable font from the source in the master branch, because it has extrapolated instances. So, I've had to use Instance as Master in GlyphsApp to make sure that the actual Light is the Light master, and the actual Bold is the Bold master.

I will need to do a round of outline QA to make sure this doesn't leave any extrapolation artifacts.

@madig
Copy link
Collaborator

madig commented Mar 27, 2019

Ah ok, then ignore my XML dump above.

Extrapolation is something that someone may one day work on in varLib. One day.

@thundernixon
Copy link
Author

In cases like this, it's not such a bad thing that extrapolation doesn't work – it gives a better chance to check for problems in the process, whereas if it built with no issue, it would be easier to miss outline problems.

Of course, I'm sure there are cases in which extrapolation in varLib would be useful, but I'm sure it's nothing trivial to add in.

@thundernixon
Copy link
Author

Thanks for taking a look at this and helping to identify the issue, @madig!

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

No branches or pull requests

3 participants