googlefonts / glyphsLib

A bridge from Glyphs source files (.glyphs) to UFOs
Apache License 2.0
182 stars 51 forks source link

User/design space mapping computation: Improve default generation? #483

Open madig opened 5 years ago

madig commented 5 years ago

In the BraceTestFont.glyphs file, the generated axes mapping is

  <axes>
    <axis tag="wdth" name="Width" minimum="75" maximum="100" default="100">
      <map input="75" output="50"/>
      <map input="100" output="100"/>
    </axis>
    <axis tag="wght" name="Weight" minimum="100" maximum="700" default="100">
      <map input="100" output="100"/>
      <map input="700" output="1000"/>
    </axis>
  </axes>

If one removes the "Axis Location" custom parameters from the masters, one gets

    <axis tag="wdth" name="Width" minimum="50" maximum="100" default="100"/>
    <axis tag="wght" name="Weight" minimum="100" maximum="700" default="100">
      <map input="100" output="100"/>
      <map input="400" output="500"/>
      <map input="700" output="1000"/>
    </axis>

Looking at the instances, glyphsLib should be able to infer that we need to add a mapping that maps user 75 to design 50 and user 100 to design 100 ("Condensed" instances). Likewise, we have two weight masters and define weightClass 100 and 700 in the instances as min/max. Basically, glyphsLib should infer the first axes definition without the user setting the appropriate custom parameters.

anthrotype commented 5 years ago

Inferring master’s user location from the instances is the “old way”, for legacy compat, going forward we should encourage user to set explicitly the axis location master custom parameter

anthrotype commented 5 years ago

after https://github.com/googlefonts/glyphsLib/pull/451 was merged (glyphsLib >= 3.3.0), the old approach of building the DesignSpace axes mapping (from user-space to internal design locations) got an unexpected breaking change. The width axis is no longer assumed to be already in user-space coordinates (as in percentage of Normal width); but the instances' width classes are used as user-space locations, and the instances' width values as internal design locations. One problem is, unlike with weight where there's a way to override the "Weight" drop-down menu via instance custom parameter weightClass, with width one is stuck with the set of standard widths from the Width drop-down menu, which have a predefined releation defined by the OT spec with percentages of normal width. How can one get the same output as before with the new code from #451 ?

anthrotype commented 5 years ago

If one uses the new approach (i.e. Axes + Axis Location master custom parameters) to define the DesignSpace axes maps, one can only assign user-space locations to the masters, not to the intermediate locations corresponding to the interpolated instances in the previous method.

belluzj commented 5 years ago

How can one get the same output as before with the new code from #451 ?

According to the code, setting the Axis Location along the width axis should work (and override any other "inference" made by glyphsLib: https://github.com/googlefonts/glyphsLib/blob/master/Lib/glyphsLib/builder/axes.py#L354-L363

Is that an OK solution for you? Or do you mean that you want the old behaviour without switching your files to using Axis Location?

As for having mapping entries corresponding to instances, we had something like that in glyphsLib before the introduction of Axis Location, but I think it's a bad idea in general because it often happens that designers define lots of instances in Glyphs for testing purposes, and that doesn't mean that they want mapping entries for all of them. On the other hand, designers could come up with a font with only two masters, no instances, and still want a precise mapping curve with many entries. That indicates to me that a better mapping generation in glyphsLib should rely on a specific "mapping description" in the Glyphs file format, that is NOT attached to instances.

anthrotype commented 5 years ago

you mean, setting the Axis Location of, say, the NotoSans-MM.glyphs "Condensed" masters to be 70, i.e. the same as their internal design-space location? That could work for width in this case, in that I will get back the old fvar (with min=70) and no avar segment for width since all maps are equal.

But then using Axis Location would opt in the new approach also for weight, where currently NotoSans has as many map elements as there are instances. But using Axis Location on the masters would mean having mappings only for the masters (which are at 100, 400, 700 and 900), and none for the intermediate steps

anthrotype commented 5 years ago

it is extremely important that when for example one requests the wght=200 (and width=100) from NotoSans-VF.ttf, one gets exactly the same output as the ExtraLight static instance, and not something linearly interpolated between the Thin (100) and the Regular (400). That's why the DesignSpace axis maps (and coorespondingly the wght avar segementmap) needs to contain an entry for 200

anthrotype commented 5 years ago

a better mapping generation in glyphsLib should rely on a specific "mapping description" in the Glyphs file format, that is NOT attached to instances

nor attached to masters

anthrotype commented 5 years ago

to stay with the example above, for NotoSans-VF and ExtraLight instance. If one requests an instance with wght=200, this value gets normalized using default coordinate normalization to -0.6667; but then the avar tables "warps" the default normalization and gives -0.7969. This is the final value used internally to compute scalars and deltas. But if we remove those intermediate mappings, the wght=200 will no longer normalized to -0.7969.

    <axis tag="wght" name="Weight" minimum="100" maximum="900" default="400">
      <map input="100" output="26"/>
 <!-- <map input="200" output="39"/>
      <map input="300" output="58"/> -->
      <map input="400" output="90"/>
 <!-- <map input="500" output="108"/>
      <map input="600" output="128"/>
      <map input="700" output="151"/>
      <map input="800" output="169"/> -->
      <map input="900" output="190"/>
    </axis>

commenting out the intermediate mappings will produce the following avar table:

 <avar>
   <segment axis="wght">
     <mapping from="-1.0" to="-1.0"/>
<!-- <mapping from="-0.6667" to="-0.7969"/>
     <mapping from="-0.3333" to="-0.5"/> -->
     <mapping from="0.0" to="0.0"/>
<!-- <mapping from="0.2" to="0.18"/>
     <mapping from="0.4" to="0.38"/>
     <mapping from="0.6" to="0.61"/>
     <mapping from="0.8" to="0.79"/> -->
     <mapping from="1.0" to="1.0"/>
   </segment>
 </avar>
belluzj commented 5 years ago

I see your problem, I think there is no good solution currently. Either your mapping is not what you want for weight, or it's not what you want for width. The PR that you commented on improves slightly the situation for width because at least you don't have to stick to the design locations, but is still constrained by the limited list of options from the spec.

I think a good way to fix that would be to have an "Axis Mapping" custom parameter on the GSFont, which would give exactly the same liberties for defining mappings as the Designspace format.

I wonder how Glyphs users who produce variable fonts with Glyphs specify mappings?

anthrotype commented 5 years ago

I wonder how Glyphs users who produce variable fonts with Glyphs specify mappings?

Good question. Maybe Georg knows? @schriftgestalt

anthrotype commented 5 years ago

I think I will revert the change in https://github.com/googlefonts/glyphsLib/pull/451 for now, until we find a decent solution. It's not good that users that install fontmake in a new environment will get glyphsLib 3.3.0 which produces a different designspace (hence different varfont's fvar/avar) than if they do not upgrade glyphsLib and stay with 3.2.0.

davelab6 commented 5 years ago

Maybe release v4 and do the right thing. There will be a lot more vf in the future than the past.

anthrotype commented 5 years ago

Yeah, but it’s the definition of “the right thing” which is not only for me to decide. To use that proposed Axis Mapping custom parameter requires that Glyphs.app adds support for that first. Unknown (user-defined) custom parameters in Glyphs can only be parsed/written as string values, they have no predefined structure. For such an Axis Mapping we would at least need a dict of dicts: keys are the axis names or tags, values are dicts of user-space to internal locations.

schriftgestalt commented 5 years ago

I wonder how Glyphs users who produce variable fonts with Glyphs specify mappings?

Good question. Maybe Georg knows? @schriftgestalt

When I understand the problem correctly, the "Axis Location" CP is dealing with it.

Glyphs doesn't properly handle the instance mapping at all as there is no support for avar, yet.

anthrotype commented 5 years ago

the "Axis Location" CP is dealing with it.

the Axis Location applies to Masters only. We need a way to specify mappings (between user-space and internal design coordinate space) for arbitrary intermediate locations that may (or may) not correspond to the locations of the instances. Basically the equivalent of the designspace format's <axes><map> elements, which fontTools.varLib will use to build an avar table.

anthrotype commented 5 years ago

I convinced myself that we should use the instances' OS/2 widthClass to define the user-space locations for the wdth axis, the same way we do for wght (submitted #526 to re-revert the change from #451). I discussed this with @m4rc1e and I now undestand the problem better.

Take NotoSans, for example, where the most condensed master has width coordinate 70; the "Extra Condensed" instance is also at the same width coordinate, and has OS/2 widthClass 2. According to both OS/2 and CSS specs, "extra-condensed" should correspond to 62.5 % percent of the normal width (not 70 %): https://docs.microsoft.com/en-us/typography/opentype/spec/os2#uswidthclass https://developer.mozilla.org/en-US/docs/Web/CSS/font-stretch

If static instance is generated for such NotoSans-ExtraCondensed and it has OS/2.widthClass = 2, and it is selectable with font-stretch: extra-condensed or font-stretch: 62.5%, then if we were to swap the static font with an equilvalent variable font, we would like the same font-stretch to correspond visually to the "Extra Condensed" named instance. To do that, the fvar entry for the latter should have coordinate 62.5 (and not 70); but then the avar should have a mapping from one to the other, in order to interpolate the instance at the desired internal design location.

Even with #526, we would still miss support for defining custom avar mappings for intermediate locations that don't correspond to neither masters nor instances, but that's less of an issue. It's more important that we output a correct fvar and avar for the wdth axis.

anthrotype commented 5 years ago

another thing I am not quite at ease with is that, one the one hand we advertise using Axis Location custom parameter as the "new" approach to specifying the fvar's user-space locations, but on the other hand this only allows to specify user location of the masters and as soon as you have some instance in between them for which the OS/2 width class and the interpolation value do not exactly match, then the only solution is to not use the Axis Location parameter and revert back to the "old" approach where user-locations are inferred from the instances.

schriftgestalt commented 5 years ago

I’m open to define something like the Axis Location parameter for instances, too. Would it be sufficient to have the same UI and data?

belluzj commented 5 years ago

I’m open to define something like the Axis Location parameter for instances, too. Would it be sufficient to have the same UI and data?

No it wouldn't; please do not tie the Axis Location to the instances, that would not solve the problem that is described here.

The designspace mapping can have "Axis Locations" that are independent from masters and instances. ie. you can have 2 masters, no instances, and 5 mapping entries, or 3 masters, 2 instances, and 4 mapping entries, or any other combination.

schriftgestalt commented 5 years ago

Right. Something in Font then. Can you suggest a data structure? I’ll think about a UI then?

anthrotype commented 5 years ago

Thanks. I wrote above

For such an Axis Mapping we would at least need a dict of dicts: keys are the axis names or tags, values are dicts of user-space to internal locations.

{
  “wght”: {
    100: 20,
    400: 50,
    700: 80,
  },
  “wdth”: {
    50: 0,
    100: 200,
    150: 400,
  }
}
belluzj commented 5 years ago

I agree with Cosimo's suggestion, and just because usually people are confused about which side of the mapping is user location or design location, I would make it explicit on each tuple like so:

{
  "wght": [
    { user: 100, design: 20 },
    { user: 400, design: 50 },
    { user: 700, design: 80 },
  ],
  "wdth": [
    { user: 50, design: 0 },
    { user: 100, design: 200 },
    { user: 150, design: 400 },
  ]
}

(but it's not necessarily a good idea because then people can write stuff like [{user: 100, design: 50}, {user: 100, design: 75}] which doesn't make sense)

anthrotype commented 5 years ago

As for the UI I suggest something similar to the graph in the avar spec: https://docs.microsoft.com/en-us/typography/opentype/spec/avar

With user-space coordinates on the horizontal axis, and the internal design locations on the vertical axis (the avar graph is using normalized coordinates but it’s the same).

anthrotype commented 5 years ago

Also there must be mappings for at least the minimum, default and maximum location of each axis; the coordinates must be unique (no duplicates) and in increasing order. See avar spec for details why this is necessary.

anthrotype commented 5 years ago

The Axis Location custom parameters provide mappings for the masters, so if those are present the Axis Mapping should be pre-populated with those values, and the user can add more intermediate mappings if needed but not modify the master’s axis location (otherwise the two pieces of data would get out of sync)

schriftgestalt commented 5 years ago

Could the Axis Mapping supersede the Axis Location in the masters? Or is it a good idea to keep them separate?

anthrotype commented 5 years ago

Supersede sgtm

madig commented 5 years ago

Should Glyphs.app auto-migrate master-level Axis Locations to the hypothetical new Axis Mapping? glyphsLib could then do the same and we could phase out support for Axis Location over time.

anthrotype commented 5 years ago

@schriftgestalt I was wondering if this new proposed "Axis Mapping" custom parameter has been implemented yet, perhaps in beta version?

schriftgestalt commented 5 years ago

Didn’t have time for this, yet. It’s on my list.

madig commented 5 years ago

Random note to myself: look at if it's possible to get rid of the com.schriftgestaltung.width and com.schriftgestaltung.weight Designspace lib key dumps by reading the mapping value from wght and wdth axes.