Only a partial axis map is generated from GlyphsApp to a variable font

thundernixon commented 5 years ago

Currently, when I generate a variable font using FontMake, it is only generating a partial weight axis map. As a result, the "Regular" instance is at wght=391.6666717529297, rather than 400.

The Setup

I'm working in https://github.com/thundernixon/FiraCode/tree/qa/googlefonts-qa.

As a quick way to replicate my results, you can just be in that qa branch, and build with:

fontmake -g FiraCode.glyphs -o variable --output-dir distr/variable_ttf

My GlyphsApp source is set up with specific weightClass parameters for all instances, like the following:

It does have one instance at 450, which is a bit unique, but as shown below, shouldn't necessarily be a blocker.

There is also an Axis Location parameter for each master, like this:

However, the designspace generated from this gets the wght axis map of:

    <axis tag="wght" name="Weight" minimum="300" maximum="700" default="300">
      <map input="300" output="62"/>
      <map input="700" output="158"/>
    </axis>

This results in an incorrect value for intermediate instances.

However, if I manually paste the below code into the designspace, and then build from that, it works as expected.

    <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>

With default, generated designspace: FiraCode-VF-incorrect.ttf.zip

With corrected designspace (working correctly(: FiraCode-VF-correct-weights.ttf.zip

This is related to https://github.com/googlei18n/fontmake/issues/540 in that it is the same font, and has continued issues with weights and instances. I closed that thinking I was in the clear, only to later realize that there was a lingering problem. Still, this problem is a distinct one. I could potentially just correct the designspace as a step in the build process, but that seems like a hacky and brittle solution.

Is there anything I'm missing on my end, or is this something that might be an issue in FontMake/FontTools?

anthrotype commented 5 years ago

Are you using the Axis Location custom parameters for all the masters, as I suggested in the other thread? You should set those to user space location along the wght axis for each master.

thundernixon commented 5 years ago

Thanks, @anthrotype. Good question. Yes, together, my axis-specimen custom parameters look like:

Font
- Axes: Weight
Masters
- Light
- Axis Location: Weight=300
- Bold
- Axis Location: Weight=700
Instances
- Light
- weightClass: 300
- Regular
- weightClass: 400
- Retina
- weightClass: 450
- Medium
- weightClass: 500
- SemiBold (is active == false)
- weightClass: 600
- Bold
- weightClass: 700

anthrotype commented 5 years ago

Right. Ok so this looks like a feature request for glyphsLib to also take into account the instances weight classes when constructing the axes maps, and not just the masters’ Axis Locations. /cc @belluzj @madig

I thought it already did that, but apparently the two approaches are either using only the masters Axis Locations, or the instances Weight classes.

See the to_designspace_axes function here: https://github.com/googlei18n/glyphsLib/blob/6454fa59106cccdcc022f950f87063e77e440185/Lib/glyphsLib/builder/axes.py

@thundernixon try to remove the Axis Location custom params.. do you now get the expected mapping (this time derived only from the instances weight classes)?

thundernixon commented 5 years ago

Ahh, thank you for helping to pinpoint the issue, @anthrotype!

try to remove the Axis Location custom params.. do you now get the expected mapping

No, as before in in issue https://github.com/googlei18n/fontmake/issues/540, this mostly works, but generates a font with a default OS/2 weightClass of 400, rather than the expected 300.

Currently-working patch

I have found that if I use weightClass in both instances and masters, the font generates with all values as expected (at least for instance weights and OS/2 weight class).

Remaining issue

However, this is not ideal, because GlyphsApp grays out the weightClass custom parameter in masters, so this is a very non-intuitive solution.

It would be more understandable if it could also work with the setup mentioned two comments above (with Axis Location for masters and weightClass for instances).

madig commented 5 years ago

If you were to use Glyphs.app's export-to-VF facilities, what would you have to do to get correct weight values?

thundernixon commented 4 years ago

Coming back to this issue as I'm working to finish up https://github.com/thundernixon/encode-sans.

Unfortunately, it still seems to be the case that a full axis map cannot be generated from a Glyphs source with a default FontMake build, even if all masters and instances have accurate weight values + weightClass params.

If you were to use Glyphs.app's export-to-VF facilities, what would you have to do to get correct weight values?

If I try to generate from Glyphs from this version of the source I get the same problematic result: wght values that don't fall on expected 100-unit values. Here's an excerpt from the font's fvar table, which is the same via Glyphs export or FontMake's:

    <!-- Expanded SemiBold -->
    <NamedInstance flags="0x0" subfamilyNameID="299">
      <coord axis="wght" value="536.36363"/>
      <coord axis="wdth" value="125.0"/>
    </NamedInstance>

    <!-- Expanded Bold -->
    <NamedInstance flags="0x0" subfamilyNameID="300">
      <coord axis="wght" value="661.61615"/>
      <coord axis="wdth" value="125.0"/>
    </NamedInstance>

    <!-- Expanded ExtraBold -->
    <NamedInstance flags="0x0" subfamilyNameID="301">
      <coord axis="wght" value="790.90909"/>
      <coord axis="wdth" value="125.0"/>
    </NamedInstance>

    <!-- Expanded Black -->
    <NamedInstance flags="0x0" subfamilyNameID="302">
      <coord axis="wght" value="900.0"/>
      <coord axis="wdth" value="125.0"/>
    </NamedInstance>

If I generate from GlyphsApp with the immediate prior version of the source, the wght values don't have mapping at all, and are therefore much farther off of the required wght values:

    <!-- SemiBold -->
    <NamedInstance flags="0x0" subfamilyNameID="263">
      <coord axis="wght" value="142.0"/>
      <coord axis="wdth" value="125.0"/>
    </NamedInstance>

    <!-- Bold -->
    <NamedInstance flags="0x0" subfamilyNameID="264">
      <coord axis="wght" value="173.0"/>
      <coord axis="wdth" value="125.0"/>
    </NamedInstance>

    <!-- ExtraBold -->
    <NamedInstance flags="0x0" subfamilyNameID="265">
      <coord axis="wght" value="205.0"/>
      <coord axis="wdth" value="125.0"/>
    </NamedInstance>

    <!-- Black -->
    <NamedInstance flags="0x0" subfamilyNameID="266">
      <coord axis="wght" value="232.0"/>
      <coord axis="wdth" value="125.0"/>
    </NamedInstance>

If I generate via FontMake from that version (which lacks weightClass params on the instances but otherwise has things set correctly), the <map> is generated, resulting in correct values on the fvar table. However, this has the problem that the OS/2 table gets <usWeightClass value="400"/>, when due to the Thin default location, this should be 100. This continues to be the case even if I set Axis Location params in each Master, which fixes the usWeightClass problem, but causes this one.

However, maybe this is actually an issue (or has a related issue) that the usWeightClass isn't being set correctly in variable fonts? If that happened without Axis Locations being set in the masters, then glyphsLib would be correctly building the axis map from the normal values set in masters & instances.

madig commented 4 years ago

Can't have a look because I can't glyphs2ufo the source:

Encode Sans Thin Condensed: Glyph Germandbls, layer Nov 1 18, 16:25: Duplicate glyph layer name

I.e. maybe clear out all non-master layers unless you want to keep them.

In general, mapping generation in glyphsLib differs from what Glyphs is doing because until recently we didn't even know what Glyphs was doing.

anthrotype commented 4 years ago

I looked at your sources briefly this morning.

Axis Mappings is not supported yet, as I said above. It will be ignored by glyphsLib and fontmake.

If you have Axis Location custom parameters set for the masters, the instances user locations will not be even considered when generating the axes maps in designspace.

That's why earlier I suggested that you remove the Axis Location master parameter. When these are not present, the user-space location of a master will be inferred from the user-location of the corresponding instance (the instance at the same design-location as the master). That gives you more fine-tuned control.

You don't need to have weightClass in your instances if it is the same as the value that is assigned to them by selecting the Weight drop-down menu (see the number greyed-out next to the predefined Weight name). It's redundant. You only need weightClass if for any reasons it is different from the number that is assigned by selecting one of the predefined weights.

thundernixon commented 4 years ago

@m4rc1e helped figure out a way around this. The glyphs file generates a font with proper wght mapping plus a proper usWeightClass if:

The font has the Axes custom parameter with Weight, Width. No Axis Mappings required.
Masters do not have Axis Location custom parameters (or weightClass params).
Instances do not have weightClass custom parameters (or Axis Location params).

Here is the specific version of the Glyphs source that is working well: https://github.com/thundernixon/Encode-Sans/blob/fe44e5ca241ad7acc13fd225019855a87601da3e/sources/Encode-Sans.glyphs

This generates with fontmake -o variable -g sources/Encode-Sans.glyphs.

I'm not really sure if this issue should be closed – I'll leave that up to others. On the one hand, it's good that things work well with the right set of custom parameters excluded, and especially nice that it doesn't require many of them to work as desired. On the other hand, it seems incorrect that specifying Axis Location on masters should break mapping.

Thanks so much for the support and feedback here!

thundernixon commented 4 years ago

I had left the tab open from yesterday, so I made my update before seeing the newest comments. Thanks for your quick responses, @madig and @anthrotype!

The Germandbls had a duplicate layer name, and when removed, it built well.

I appreciate your explanations of the reasons for these issues, Cosimo! "If you have Axis Location custom parameters set for the masters, the instances user locations will not be even considered when generating the axes maps in designspace." Is there a usecase in which users would want to override inferenced location with master Axis Locations? If so, we should close this issue.

anthrotype commented 4 years ago

Axis Location and the new Axis Mapping parameters at best contain duplicate information, or at worst can potentially contradict one another, as I tried to explain in https://github.com/googlefonts/glyphsLib/issues/568#issuecomment-580321785

We haven't reached a mutual understanding since then..

googlefonts / fontmake