review for PR of TN master #26

pichotta · 2019-11-14T03:48:05Z

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

note issue on conversion with too many alignment values

Fea compatibilize

accepted pull request of [wght].ttf build. noting outstanding git issue #2 question.

m4rc1e · 2019-11-14T12:22:08Z

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

m4rc1e · 2019-11-14T14:25:30Z

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 · 2019-11-14T16:54:55Z

@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 · 2019-11-14T17:46:37Z

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

…ters

…match in all masters

…hacute.sc

… and check diffenator

Fea next master

pichotta · 2019-11-20T19:43:02Z

@m4rc1e
submitting another pull request after revisions

Taken from the upstream repo https://github.com/huertatipografica/Alegreya at commit huertatipografica/Alegreya#26

pichotta · 2020-09-18T01:40:39Z

@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 · 2020-09-18T07:28:00Z

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 · 2020-09-21T10:48:20Z

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

Update VF: build SC

strarsis · 2020-09-22T14:48:16Z

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

pichotta · 2020-09-22T14:52:35Z

@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 · 2020-09-23T07:01:41Z

Great! Thanks for fixing that.
Now merging

Jill Pichotta and others added 13 commits October 9, 2019 16:24

conversion Glyphs master src to RF 3.3 UFO

38936b2

note issue on conversion with too many alignment values

open UFOs

2cafcfb

fix /areas glyphs which caused error in fontmake

8bfbc5e

move previous fonts to "old" folder for now

7e5242f

add build.sh file

579d910

generate OTF, TTF, and VF and run post-processing scripts via build.sh

950bd91

remove old glyphs sources

1141516

remove old fonts

0dbc7d8

update build script

adfa156

rebuild all

562358b

Merge pull request #3 from TypeNetwork/fea-compatibilize

6ce793f

Fea compatibilize

added vf .ttfs

af5d55e

Merge pull request #4 from TypeNetwork/fea-compatibilize

0afb684

accepted pull request of [wght].ttf build. noting outstanding git issue #2 question.

add report script

04dc67e

CJ Dunn added 12 commits November 14, 2019 15:30

rename master to match instances

6d89339

rebuild all

917a6fd

decompose glyphs with components whose scale doesn't match in all mas…

53bb1b6

…ters

decompose glyphs in italic with components whose scale value doesn't …

4bd4cf9

…match in all masters

rebuild all

859c92b

update bracket layers to include /Oslashacute /oslashacute and /oslas…

9fcba8b

…hacute.sc

comment out additional swaps because these glyphs don't exist

298c1cb

update build script

db44565

rebuild all

507fd4b

added slashacute BRACKET glyphs, need to add designspace, rebuild all…

5a7501c

… and check diffenator

designspace

1dd1b8a

rebuild vf

6366cf5

CJ Dunn and others added 8 commits November 19, 2019 16:52

rebuild all

3af78ad

whitespace cleanup

52a358a

uncomment out build script

9826af9

out reports for roman

6ed4b13

out report italics

6d877c7

update out reports

16c9adb

cleaning up out reports; transferring posting via Issues

fa71e3d

Merge pull request #6 from TypeNetwork/fea-next-master

a613551

Fea next master

m4rc1e added a commit to google/fonts that referenced this pull request Dec 13, 2019

alegreya: v2.008 added.

083dc46

Taken from the upstream repo https://github.com/huertatipografica/Alegreya at commit huertatipografica/Alegreya#26

m4rc1e mentioned this pull request Dec 13, 2019

alegreya: v2.008 added. google/fonts#2290

Closed

m4rc1e mentioned this pull request Jul 28, 2020

Make Alegreya a variable font google/fonts#2560

Open

Remove anchor _bottomright from Shcha-cy, shcha-cy, shcha-cy.sc

0e18c66

moyogo and others added 8 commits September 21, 2020 23:03

build.sh: build SC family with freezer and subsetter

f28aba2

build.sh: process OTF

3525abd

Remove SC instances from designspace

03efd9d

Add requirements.txt

4da2ccb

Modify underline position and thickness, same value for all

c99aaa5

designspace: whitespace

8eecd00

Update fonts

d5e2cdc

Merge pull request #9 from TypeNetwork/update-vf

78b3afb

Update VF: build SC

juandelperal merged commit eb03318 into huertatipografica:master Sep 23, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

review for PR of TN master #26

review for PR of TN master #26

pichotta commented Nov 14, 2019

m4rc1e commented Nov 14, 2019

m4rc1e commented Nov 14, 2019

cjdunn commented Nov 14, 2019

cjdunn commented Nov 14, 2019

pichotta commented Nov 20, 2019

pichotta commented Sep 18, 2020

juandelperal commented Sep 18, 2020 •

edited

Loading

strarsis commented Sep 21, 2020

strarsis commented Sep 22, 2020

pichotta commented Sep 22, 2020

juandelperal commented Sep 23, 2020

review for PR of TN master #26

review for PR of TN master #26

Conversation

pichotta commented Nov 14, 2019

m4rc1e commented Nov 14, 2019

m4rc1e commented Nov 14, 2019

Fontbakery report

Summary

cjdunn commented Nov 14, 2019

cjdunn commented Nov 14, 2019

pichotta commented Nov 20, 2019

pichotta commented Sep 18, 2020

juandelperal commented Sep 18, 2020 • edited Loading

strarsis commented Sep 21, 2020

strarsis commented Sep 22, 2020

pichotta commented Sep 22, 2020

juandelperal commented Sep 23, 2020

juandelperal commented Sep 18, 2020 •

edited

Loading