dy / wavefont

Typeface for rendering waveform/data
https://dy.github.io/wavefont/scripts/
SIL Open Font License 1.1
416 stars 7 forks source link

Update config.yaml #57

Closed RosaWagner closed 1 year ago

RosaWagner commented 1 year ago

Reviewed axis mapping and STAT. Let's see the result once built. I don't remember, was there something against having YELA's default at 0 ?

dy commented 1 year ago

Hi @RosaWagner! The reason was that YELA default=0 requires additional 3 master points, (since there's 3 axes) - that increases font size by 30% - not essential for woff, but essential for otf/ttf.

RosaWagner commented 1 year ago

Okay thanks I remember now. Basically this PR tries to fix these values and the Medium weight value:

Screenshot 2023-07-19 at 15 04 15
dy commented 1 year ago

I incorporated your changes into main branch please check if that's ok. Thank you for the sync up.

RosaWagner commented 1 year ago

Mapping looks correct. If default of YELA is 0 in STAT, it should be 0 also for the FVAR instances (they are currently at -100), or we should have -100 as default in the STAT.

dy commented 1 year ago

@RosaWagner the thing is that <instances> have it at xvalue=0 because that's design-space value from 0..1 range, which corresponds to -100 in userspace. Whereas default YELA in <axes> is -100, in user-space.

RosaWagner commented 1 year ago

then the value of the instances could be 0.5 (to be mapped to 0), or the STAT table should have -100, one or the other, I am only showing the inconsistency ;)

Screenshot 2023-07-19 at 19 00 10
dy commented 1 year ago

Ok, updated stat YELA to -100. Thanks for the point @RosaWagner

RosaWagner commented 1 year ago

Almost looking good !

I forgot about the name tables. Basically we have that requirement: https://googlefonts.github.io/gf-guide/variable.html#font-zero-origin, and so we would need the style name of the family being Thin so it matches the origin of the font. I don't know why I missed it before.

nameID current expected
Family Name Wavefont Wavefont Thin
Subfamily Name Regular Regular
Full Name Wavefont Regular Wavefont Thin
Poscript Name Wavefont-Regular Wavefont-Thin
Typographic Family Name N/A Wavefont
Typographic Subfamily Name N/A Thin [code: bad-names]

fyi we opened an issue in fonttools for what is blocking us in sandbox: https://github.com/fonttools/fonttools/issues/3211 and I use Samsa to have a human readable STAT/FVAR/AVAR table https://lorp.github.io/samsa/src/samsa-gui.html

dy commented 1 year ago

Good point. Updated to Wavefont Think, thank you. Let me test

dy commented 1 year ago

@RosaWagner do you know how to add Typographic Family Name ? Where did you get this table from? I don't find Typographic Family Name in fontinfo.plist

RosaWagner commented 1 year ago

Cool cool We have a bug on our side I think about how the name tables are built:

🔥 FAIL 'Thin' instance has the same coordinates as the default instance; its postscript name should be 'Wavefont-Thin', instead of 'WavefontDefaultDefault-Thin'. [code: invalid-default-instance-postscript-name]

RosaWagner commented 1 year ago

@dy typographic family name is name ID 16, but gftools builder is making these tables based on the family name and style name of the family

dy commented 1 year ago

I see. Well both of familyName and styleName are in fontinfo.plist, so. I transfered these changes to linefont as well. lmk if anything else is needed

RosaWagner commented 1 year ago

I opened an issue for the builder's bug: https://github.com/googlefonts/axisregistry/issues/139

RosaWagner commented 1 year ago

@dy We found the problem, and actually gftools is fixed for a while now, but the requirements in the repo indicates this: gftools<0.9.15 when we would need gftools>=0.9.33

dy commented 1 year ago

Great, done, thank you @RosaWagner

dy commented 1 year ago

Somehow it cannot build linefont with latest gftools though:

...
fontTools.varLib.errors.FoundANone: 

Couldn't merge the fonts, because one of the values in a list was empty when
it shouldn't have been. This happened while performing the following
operation: GPOS.table.LookupList.Lookup[0][0][1].EntryAnchor

The problem is likely to be in Linefont Thin:
.EntryAnchor==[None, None, None, None, None, None, None, None]

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/div/projects/linefont/venv/bin/gftools", line 8, in <module>
    sys.exit(main())
...
RosaWagner commented 1 year ago

Ah there is a problem with fonttools. I'll open another issue there…

RosaWagner commented 1 year ago

But so is it working with wavefont? I haven't look at Linefont so much, so maybe there is something that we missed.

dy commented 1 year ago

Also there's something changed in gftools, the proof command doesn't exist anymore:

gftools gen-html proof
...
gftools: error: unrecognized arguments: proof fonts/ttf/Wavefont-ExtraBold.ttf fonts/ttf/Wavefont-Medium.ttf fonts/ttf/Wavefont-Black.ttf fonts/ttf/Wavefont-Light.ttf fonts/ttf/Wavefont-Bold.ttf fonts/ttf/Wavefont-Thin.ttf fonts/ttf/Wavefont-Regular.ttf fonts/ttf/Wavefont-ExtraLight.ttf fonts/ttf/Wavefont-SemiBold.ttf -o out/proof
make: *** [proof] Error 2

Maybe @simoncozens knows if there's a way to upgrade the command?

dy commented 1 year ago

But so is it working with wavefont?

Yes, wavefont builds fine except for the proof command. I remember I decided limit gftools version because of that API change, I wasn't able to figure out how to properly work that around.

I reflected all changes that you mentioned for Wavefont in Linefont

RosaWagner commented 1 year ago

Ah yes we use diffenator2 now, actually you can find the new command in the template repo where it is updated https://github.com/googlefonts/googlefonts-project-template/blob/main/Makefile#L36

dy commented 1 year ago

Oh, nice, updated to diffenator2, thank you

dy commented 1 year ago

Should I post the issue with linefont somewhere in gftools or you'll be able to do that @RosaWagner ?