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

Update config.yaml #57

Closed
wants to merge 3 commits into from
Closed

Update config.yaml #57

wants to merge 3 commits into from

Conversation

RosaWagner
Copy link
Contributor

@RosaWagner RosaWagner commented Jul 19, 2023

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 ?

@RosaWagner RosaWagner marked this pull request as ready for review July 19, 2023 09:35
@@ -3,13 +3,17 @@
<axes>
<axis default="100" minimum="0" maximum="100" name="Roundness" tag="ROND"/>
<axis default="-100" minimum="-100" maximum="100" name="Vertical Element Alignment" tag="YELA">
<map input="-100" output="0"/>
<map input="100" output="1"/>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RosaWagner this maps -100..+100 user-space YELA values to 0..1 design-space alignment values, I don't think it can be removed, is there a specific reason for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I have confuse output and input. I saw weird fractional coordinates in the FVAR but that was maybe another axis.

<map input="500" output="75"/>
<map input="600" output="100"/>
<map input="700" output="128"/>
<map input="800" output="160"/>
Copy link
Owner

@dy dy Jul 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it won't increase increase complexity of font calculation? I'd need to sync it with linefont

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the same values you use for the instances, they need to be in sync with the axis mapping.

@dy
Copy link
Owner

dy commented Jul 19, 2023

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
Copy link
Contributor Author

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
Copy link
Owner

dy commented Jul 19, 2023

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

@RosaWagner
Copy link
Contributor Author

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
Copy link
Owner

dy commented Jul 19, 2023

@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
Copy link
Contributor Author

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
Copy link
Owner

dy commented Jul 19, 2023

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

@RosaWagner
Copy link
Contributor Author

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.

  • 🔥 FAIL Font names are incorrect:
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

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

@dy
Copy link
Owner

dy commented Jul 20, 2023

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

@dy
Copy link
Owner

dy commented Jul 20, 2023

@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
Copy link
Contributor Author

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
Copy link
Contributor Author

@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
Copy link
Owner

dy commented Jul 20, 2023

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
Copy link
Contributor Author

I opened an issue for the builder's bug: googlefonts/axisregistry#139

@RosaWagner
Copy link
Contributor Author

@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
Copy link
Owner

dy commented Jul 27, 2023

Great, done, thank you @RosaWagner

@dy
Copy link
Owner

dy commented Jul 27, 2023

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
Copy link
Contributor Author

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

@RosaWagner
Copy link
Contributor Author

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

@dy
Copy link
Owner

dy commented Jul 27, 2023

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
Copy link
Owner

dy commented Jul 27, 2023

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
Copy link
Contributor Author

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
Copy link
Owner

dy commented Jul 27, 2023

Oh, nice, updated to diffenator2, thank you

@RosaWagner RosaWagner closed this Jul 28, 2023
@dy
Copy link
Owner

dy commented Jul 28, 2023

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

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

Successfully merging this pull request may close these issues.

2 participants