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

com.google.fonts/check/metadata/has_regular fails on variable font when default instance != 400 #2378

Open
thundernixon opened this issue Mar 6, 2019 · 4 comments

Comments

@thundernixon
Copy link
Contributor

thundernixon commented Mar 6, 2019

Observed behaviour

com.google.fonts/check/metadata/has_regular ("families should have a Regular style") fails for Encode Sans, because the default instance described in metadata is 100 weight. Encode Sans variable fonts do have regular instances, but these are not described in METADATA.pb, because (as far as I know) we are only describing the single VF file, not its instances.

Expected behaviour

This seems like a useful, basic thing to check for, but it should instead check the name and stat tables in the font file, rather than relying on what is described in the accompanying METADATA.pb, as I believe is currently happening.

Resources and exact process needed to replicate

You can reproducde this check fail by running fontbakery check-googlefonts on the VF at https://github.com/google/fonts/tree/73ce307df62af54796476ba69d222d88ab1fa525/ofl/encodesans

@felipesanches felipesanches changed the title Check 090 (font "should have a Regular style") fails on variable font when default instance != 400 com.google.fonts/check/metadata/has_regular fails on variable font when default instance != 400 Mar 23, 2019
@eliheuer
Copy link
Collaborator

I run into this issue a lot.

@davelab6 davelab6 added this to the 0.7.7 milestone Jun 9, 2019
@felipesanches felipesanches modified the milestones: 0.7.7, 0.7.8, 0.7.9 Jun 18, 2019
@felipesanches felipesanches modified the milestones: 0.7.9, 0.7.10 Jul 11, 2019
@felipesanches felipesanches self-assigned this Jul 12, 2019
@felipesanches
Copy link
Collaborator

@thundernixon, in the commit you gave as a resource for replicating the issue, I see that there's a single file called EncodeSans-Thin.ttf which is referenced in METADATA.pb as weight: 100 and there's a statics sub-directory containing individual TTFs.

Is EncodeSans-Thin.ttf a variable font? Nothing in that filename indicates that it is. That's probably why it is being treated as a static.

Since this issue was opened back in March, maybe you could update us here on what changed in Encode Sans regarding this check.

Should this check be SKIP'd for variable fonts? Should we add new requirements for how variable fonts are described in METADATA.pb files?

@felipesanches
Copy link
Collaborator

@thundernixon, could you please take a quick look at this one?

@felipesanches felipesanches modified the milestones: 0.7.11, 0.7.12 Aug 20, 2019
@felipesanches felipesanches modified the milestones: 0.7.12, 0.7.14 Sep 14, 2019
@thundernixon
Copy link
Contributor Author

Hi Felipe, really sorry for the delay; I'm just coming back to Encode Sans and have opened a new, simpler PR for it now that Google Fonts can accept multi-axis VFs.

To your question, yes, Encode Sans is a variable font. Here's the current fonts dict of its METADATA.pb:

fonts {
  name: "Encode Sans"
  style: "normal"
  weight: 100
  filename: "EncodeSans[wdth,wght].ttf"
  post_script_name: "EncodeSans-CondensedThin"
  full_name: "Encode Sans Condensed Thin"
  copyright: "Copyright 2020 The Encode Project Authors (https://github.com/thundernixon/Encode-Sans), with Reserved Font Name \'Encode Sans\'."
}

The default instance is wght 100, and this METADATA.pb is being generating from gftools/add-font, so as far as I know, it is correct. However, I'm still getting this fail:

🔥 FAIL: METADATA.pb: According Google Fonts standards, families should have a Regular style.
com.google.fonts/check/metadata/has_regular

🔥 FAIL This family lacks a Regular (style: normal and weight: 400) as required by Google Fonts standards. [code: lacks-regular]

To answer your question slightly more fully:

Is EncodeSans-Thin.ttf a variable font? Nothing in that filename indicates that it is. That's probably why it is being treated as a static.

We could query the is_variable_font condition in this check, and if it is, we probably don't need to run the check.

https://github.com/googlefonts/fontbakery/blob/547630259405878ebf924ceeb99f05ec8e67ca78/Lib/fontbakery/profiles/shared_conditions.py#L200-L202

Potentially, we could run a separate check to find if the default instance is correctly represented in the METADATA, but that could be a separate issue. If we wanted to do this, we could check the DefaultValue for the wght axis of the fvar table. For example, in Encode Sans, via TTX, this looks like:

    <!-- Weight -->
    <Axis>
      <AxisTag>wght</AxisTag>
      <Flags>0x0</Flags>
      <MinValue>100.0</MinValue>
      <DefaultValue>100.0</DefaultValue>
      <MaxValue>900.0</MaxValue>
      <AxisNameID>256</AxisNameID>
    </Axis>

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

No branches or pull requests

4 participants