review for PR of TN master

[pichotta]

@m4rc1e
is this TN master branch ready for merge PR?

[m4rc1e]

Thanks @pichotta, I'll take a look now.

[m4rc1e]

Fontbakery report

Fontbakery version: 0.7.15

[4] Alegreya-Italic[wght].ttf

🔥 FAIL: Font has old ttfautohint applied?

• com.google.fonts/check/old_ttfautohint

--- Rationale ---

This check finds which version of ttfautohint was used, by inspecting name
table entries and then finds which version of ttfautohint is currently
installed in the system.

• 🔥 FAIL Failed to parse ttfautohint version values: installed = '1.8.3'; used_in_font = '1.8.1.43-b0c9' [code: parse-error]

🔥 FAIL: Are there caret positions declared for every ligature?

• com.google.fonts/check/ligature_carets

--- Rationale ---

All ligatures in a font must have corresponding caret (text cursor) positions
defined in the GDEF table, otherwhise, users may experience issues with caret
rendering.

• 🔥 FAIL Failed to lookup ligatures. This font file seems to be malformed. For more info, read: Fix ERROR on ligatures conditional fonttools/fontbakery#1596 [code: malformed]

🔥 FAIL: Is there kerning info for non-ligated sequences?

• com.google.fonts/check/kerning_for_non_ligated_sequences

--- Rationale ---

Fonts with ligatures should have kerning on the corresponding non-ligated
sequences for text where ligatures aren't used (eg
https://github.com/impallari/Raleway/issues/14).

• 🔥 FAIL Failed to lookup ligatures. This font file seems to be malformed. For more info, read: Fix ERROR on ligatures conditional fonttools/fontbakery#1596 [code: malformed]

⚠ WARN: Stricter unitsPerEm criteria for Google Fonts.

• com.google.fonts/check/unitsperem_strict

--- Rationale ---

Even though the OpenType spec allows unitsPerEm to be any value between 16 and
16384, the Google Fonts project aims at a narrower set of reasonable values.

The spec suggests usage of powers of two in order to get some performance
improvements on legacy renderers, so those values are acceptable.

But value of 500 or 1000 are also acceptable, with the added benefit that it
makes upm math easier for designers, while the performance hit of not using a
power of two is most likely negligible nowadays.

Another acceptable value is 2000. Since TT outlines are all integers (no
floats), then instances in a VF suffer rounding compromises, and therefore a
1000 UPM is to small because it forces too many such compromises.

Therefore 2000 is a good 'new VF standard', because 2000 is a simple 2x
conversion from existing fonts drawn on a 1000 UPM, and anyone who knows what
10 units can do for 1000 UPM will know what 20 units does too.

Additionally, values above 2048 would result in filesize increases with not
much added benefit.

• ⚠ WARN Even though unitsPerEm (1000) in this font is reasonable. It is strongly advised to consider changing it to 2000, since it will likely improve the quality of Variable Fonts by avoiding excessive rounding of coordinates on interpolations. [code: legacy-value]

[4] Alegreya[wght].ttf

🔥 FAIL: Font has old ttfautohint applied?

• com.google.fonts/check/old_ttfautohint

--- Rationale ---

This check finds which version of ttfautohint was used, by inspecting name
table entries and then finds which version of ttfautohint is currently
installed in the system.

• 🔥 FAIL Failed to parse ttfautohint version values: installed = '1.8.3'; used_in_font = '1.8.1.43-b0c9' [code: parse-error]

🔥 FAIL: Are there caret positions declared for every ligature?

• com.google.fonts/check/ligature_carets

--- Rationale ---

All ligatures in a font must have corresponding caret (text cursor) positions
defined in the GDEF table, otherwhise, users may experience issues with caret
rendering.

• 🔥 FAIL Failed to lookup ligatures. This font file seems to be malformed. For more info, read: Fix ERROR on ligatures conditional fonttools/fontbakery#1596 [code: malformed]

🔥 FAIL: Is there kerning info for non-ligated sequences?

• com.google.fonts/check/kerning_for_non_ligated_sequences

--- Rationale ---

Fonts with ligatures should have kerning on the corresponding non-ligated
sequences for text where ligatures aren't used (eg
https://github.com/impallari/Raleway/issues/14).

• 🔥 FAIL Failed to lookup ligatures. This font file seems to be malformed. For more info, read: Fix ERROR on ligatures conditional fonttools/fontbakery#1596 [code: malformed]

⚠ WARN: Stricter unitsPerEm criteria for Google Fonts.

• com.google.fonts/check/unitsperem_strict

--- Rationale ---

Even though the OpenType spec allows unitsPerEm to be any value between 16 and
16384, the Google Fonts project aims at a narrower set of reasonable values.

The spec suggests usage of powers of two in order to get some performance
improvements on legacy renderers, so those values are acceptable.

But value of 500 or 1000 are also acceptable, with the added benefit that it
makes upm math easier for designers, while the performance hit of not using a
power of two is most likely negligible nowadays.

Another acceptable value is 2000. Since TT outlines are all integers (no
floats), then instances in a VF suffer rounding compromises, and therefore a
1000 UPM is to small because it forces too many such compromises.

Therefore 2000 is a good 'new VF standard', because 2000 is a simple 2x
conversion from existing fonts drawn on a 1000 UPM, and anyone who knows what
10 units can do for 1000 UPM will know what 20 units does too.

Additionally, values above 2048 would result in filesize increases with not
much added benefit.

• ⚠ WARN Even though unitsPerEm (1000) in this font is reasonable. It is strongly advised to consider changing it to 2000, since it will likely improve the quality of Variable Fonts by avoiding excessive rounding of coordinates on interpolations. [code: legacy-value]

Summary

💔 ERROR	🔥 FAIL	⚠ WARN	💤 SKIP	ℹ INFO	🍞 PASS	🔎 DEBUG
0	6	2	127	12	146	0
0%	2%	1%	43%	4%	50%	0%

Note: The following loglevels were omitted in this report:

• SKIP
• INFO
• PASS
• DEBUG

I'll look into the caret and lig fails later on today.

---

Regressions

Bracket layers missing on oslash/Oslash glyphs

Double accents have collisions

[cjdunn]

@m4rc1e Hmm, I'm not sure what the problem is with the /Oslash

Currently the var font swaps /Oslash between Bold (has bar) and Extra Bold (has no bar). See screenshots attached.

This seems to match the live Alegreya fonts on Google Fonts:
https://fonts.google.com/specimen/Alegreya?selection.family=Alegreya
files here:
Alegreya-ExtraBold.ttf.zip
Alegreya-Bold.ttf.zip

Am I missing something?

##############################

Regarding the double accents:
I see what you mean about the collisions. Unfortunately it seems that fontmake doesn't do a good job of handling nested components, ie the /Otildeacute glyph in the Black named instance doesn't match the same glyph in the Black UFO master when it uses components. But when it is decomposed it matches. To me this is a fontmake bug. (see screenshot: background is with components, foreground is decomposed)

I'll file a bug report for fontmake.

In the mean time, it looks like we'll have to decompose all nested components

[cjdunn]

Here's my fontmake bug report:
googlefonts/fontmake#595

[pichotta]

@m4rc1e
submitting another pull request after revisions

[pichotta]

@juandelperal
The weight axis variable font for Alegreya is complete. See Issue #23 Will you accept this pull request to merge the [wght].ttf and the .ufo sources into the master?

[juandelperal]

Thanks @pichotta
Thanks for your work!

At a first glance I can see that:

• There are some problems with glyphs collisions:

• Now the SC styles aren't small caps variations. (In the glyphs sources we had glyphs replacements and variations for the SC). Take a look at the custom parameters of the SC instances. They had several replacements: (Update Features, Remove Classes, Replace Class…)
• Connected with the previews point: the instances names are duplicated: there are 2 regulars, 2 bolds… (Before there was one for the normal family and other for SC subfamily)

Thanks

[strarsis]

This is awesome! When this PR is merged, will the Google Fonts API automatically provide the Variable font format for Alegraya?

[strarsis]

@pichotta: The PR is merged! Will this be automatically now used by the Google Fonts API?

[pichotta]

@juandelperal
@strarsis
Yes, it will be rolled into the Google Fonts API. Do the most recent commits address the concerns posted above on Sept 18?

[juandelperal]

Great! Thanks for fixing that.
Now merging