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

Named instances revisited #83

Closed
RoelN opened this issue Sep 9, 2020 · 12 comments
Closed

Named instances revisited #83

RoelN opened this issue Sep 9, 2020 · 12 comments
Labels
bug-elsewhere Possible bug in whatever generate one (or more) font(s).

Comments

@RoelN
Copy link
Collaborator

RoelN commented Sep 9, 2020

Thanks @Pomax for the fix in #78 and the extensive explanation!

I see that the postscriptname ID's are off by one, though:

subfamily: Thin Condensed , postscriptname: Thin
subfamily: Thin , postscriptname: Thin Expanded
subfamily: Thin Expanded , postscriptname: ExtraLight Condensed
subfamily: ExtraLight Condensed , postscriptname: ExtraLight
subfamily: ExtraLight , postscriptname: ExtraLight Expanded
[... snip ...]
subfamily: Black Condensed , postscriptname: Black
subfamily: Black , postscriptname: Black Expanded
subfamily: Black Expanded , postscriptname: GRADUATE

You see they are one ahead of the subfamily name. For the last instance, it maps to the first entry in the name table ("GRADUATE").

Based on your explanation and reading the spec, I looks to me that the InstanceRecord is defined properly. So I'm curious as where this is happening.

(This is not very urgent to me, as I don't expose postscript names in WF!)

@Pomax
Copy link
Owner

Pomax commented Sep 9, 2020

Wow, yeah that's super weird.

@Pomax
Copy link
Owner

Pomax commented Sep 9, 2020

Honestly, this looks like an error in GRADUATE.ttf, and other fonts - some fonts are correct, others are not, so this might actually be a specific tool having an off-by-one bug when writing the postscript name id:

GRADUATE.ttf (incorrect)

subfamily: Thin Condensed , postscriptname: Thin
subfamily: Thin , postscriptname: Thin Expanded
subfamily: Thin Expanded , postscriptname: ExtraLight Condensed
[... snip ...]
subfamily: Black Condensed , postscriptname: Black
subfamily: Black , postscriptname: Black Expanded
subfamily: Black Expanded , postscriptname: GRADUATE

SourceCodeVariable-Roman.ttf (correct)

subfamily: ExtraLight , postscriptname: SourceCodeRoman-ExtraLight
subfamily: Light , postscriptname: SourceCodeRoman-Light
subfamily: Regular , postscriptname: SourceCodeRoman-Regular
[... snip ...]
subfamily: Semibold , postscriptname: SourceCodeRoman-Semibold
subfamily: Bold , postscriptname: SourceCodeRoman-Bold
subfamily: Black , postscriptname: SourceCodeRoman-Black

Inconsolata[wdth,wght].ttf (incorrect)

subfamily: UltraCondensed ExtraLight , postscriptname: UltraCondensed Light
subfamily: UltraCondensed Light , postscriptname: UltraCondensed Regular
subfamily: UltraCondensed Regular , postscriptname: UltraCondensed Medium
[... snip ...]
subfamily: UltraExpanded Black , postscriptname: Regular
subfamily: Regular , postscriptname: Bold
subfamily: Bold , postscriptname:  I n c o n s o l a t a

Recursive (correct)

subfamily: Mono Linear Light , postscriptname: RecursiveMonoLnr-Light
subfamily: Mono Linear Light Italic , postscriptname: RecursiveMonoLnr-LightItalic
subfamily: Mono Casual Light , postscriptname: RecursiveMonoCsl-Light
[... snip ...]
subfamily: Sans Linear ExtraBlack Italic , postscriptname: RecursiveSansLnr-XBlkItalic
subfamily: Sans Casual ExtraBlack , postscriptname: RecursiveSansCsl-XBlk
subfamily: Sans Casual ExtraBlack Italic , postscriptname: RecursiveSansCsl-XBlkItalic

Ancho (incorrect)

subfamily:  T h i n , postscriptname:  L i g h t
subfamily:  L i g h t , postscriptname:  R e g u l a r
subfamily:  R e g u l a r , postscriptname:  M e d i u m
subfamily:  M e d i u m , postscriptname:  B o l d
subfamily:  B o l d , postscriptname:  U l t r a B o l d
subfamily:  U l t r a B o l d , postscriptname: Ancho

(spacing in those strings are getting tackled in #74)

@Pomax
Copy link
Owner

Pomax commented Sep 9, 2020

@raphlinus do you know which tool in the Inconsolata build chain is responsible for the fvar table?

@Pomax Pomax added the bug-elsewhere Possible bug in whatever generate one (or more) font(s). label Sep 9, 2020
@raphlinus
Copy link

@Pomax fascinating! The source for that is https://github.com/googlefonts/Inconsolata/blob/master/sources/build.sh , which generally I didn't touch, it's adapted from the Google Fonts standard build scripts (@m4rc1e I think is the main author and will be very interested in tracking this down). The main tool is the fontmake invocation on line 24, but there are also a lot of "gftools" passes that do various things. Obviously the thing to do is examine the intermediate stages of that build chain and see where the error pops up.

@Pomax
Copy link
Owner

Pomax commented Sep 9, 2020

Oh nice, thanks for the explanation! I guess it might also be time to summon a wild @davelab6, to see if he knows how to effectively test the g-side of things =P

@Pomax
Copy link
Owner

Pomax commented Sep 9, 2020

The author of Ancho reports they used Glyphs so I'll drop them a line to see if they have a bug in their InstanceRecord code (or, build on top of open source that might?)

@m4rc1e
Copy link

m4rc1e commented Sep 10, 2020

If I ttx the nametable for Inconsolata on google/fonts, I don't get this issue.

@Pomax, can you point me to the Inconsolata ttf you used?

@m4rc1e
Copy link

m4rc1e commented Sep 10, 2020

@Pomax
Copy link
Owner

Pomax commented Sep 10, 2020

I used the one from https://github.com/googlefonts/Inconsolata/tree/master/fonts/variable, but if you're seeing it do the right thing, then I guess it's dissection time for the code that parses, and links, strings. The parsing code for instance records is:

class InstanceRecord {
  constructor(p, axisCount) {
    this.subfamilyNameID = p.uint16;
    p.uint16;
    this.coordinates = [...new Array(axisCount)].map((_) => p.fixed);
    this.postScriptNameID = p.uint16;
  }
}

checking this against https://docs.microsoft.com/en-us/typography/opentype/spec/fvar#instancerecord, the subfamilyNameID and reserved uint16 are fine, so it's possible that the coordinates calculation is off by a single fixed for some fonts, but not others. Looking at the TTX dump for Recursive vs Incosolata, it looks like TTX sees Recursive indicating a postscript name:

    <!-- Mono Linear Light -->
    <!-- PostScript: RecursiveMonoLnr-Light -->
    <NamedInstance flags="0x0" postscriptNameID="275" subfamilyNameID="274">
      <coord axis="MONO" value="1.0"/>
      <coord axis="CASL" value="0.0"/>
      <coord axis="wght" value="300.0"/>
      <coord axis="slnt" value="0.0"/>
      <coord axis="CRSV" value="0.5"/>
    </NamedInstance>

But it does not see Inconsolata as having postscript names:

    <!-- UltraCondensed ExtraLight -->
    <NamedInstance flags="0x0" subfamilyNameID="258">
      <coord axis="wght" value="200.0"/>
      <coord axis="wdth" value="50.0"/>
    </NamedInstance>

Looking back at the spec, the field'd description lists it as optional, with the text under it also saying it should be included, so that's probably why things went wrong here:

The postScriptNameID field is optional, but should be included in all variable fonts, and may be required in some platforms.

The code is not checking the record size to determine whether to include the postscriptname, so this is definitely a bug to be fixed in this code, rather than a bug in any other software. Thanks for the reality check, @m4rc1e!

@Pomax
Copy link
Owner

Pomax commented Sep 10, 2020

(filed MicrosoftDocs/typography-issues#613 to hopefully get the spec text updated to something clearer here)

@Pomax
Copy link
Owner

Pomax commented Sep 10, 2020

Fix landed in 552bbf7 with the following code:

import { Font } from "./Font.js";

const f = new Font("gsub testing");

f.onerror = (e) => console.error(e);

f.onload = function test(e) {
  const font = e.detail.font;
  const { name, fvar } = font.opentype.tables;
  const instances = fvar.instances;
  console.log(instances.map(i => {
    let sname = i.subfamilyNameID;
    let psname = i.postScriptNameID;
    return `${name.get(sname)}, ${psname ? name.get(psname) : `-`}`;
  }).join(`\n`));
}

f.src = "./fonts/Inconsolata[wdth,wght].ttf";

Yielding the following output for Inconsolata:

UltraCondensed ExtraLight, -
UltraCondensed Light, -
UltraCondensed Regular, -
[... snip ...]
UltraExpanded Black, -
Regular, -
Bold, -

@Pomax Pomax closed this as completed Sep 10, 2020
@RoelN
Copy link
Collaborator Author

RoelN commented Sep 10, 2020

I enjoyed this font sleuthing, thanks for the explanation, @Pomax!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-elsewhere Possible bug in whatever generate one (or more) font(s).
Projects
None yet
Development

No branches or pull requests

4 participants