I've just had a look at the Regular and it has the same issues.

When I diff the table's of the Reg using https://github.com/googlefonts/tools/blob/master/bin/gftools-compare-font.py. We get the following.

  Table Changes (delta bytes, from=>to, % change)
    BASE, +0, 0=>0, 0.0%
    CFF , +0, 0=>0, 0.0%
    DSIG, +0, 0=>0, 0.0%
    FFTM, +0, 0=>0, 0.0%
    GDEF, +656, 934=>1590, 0.2%
    GPOS, +65478, 5462=>70940, 21.3%
    GSUB, -74, 11536=>11462, -0.0%
    LTSH, +0, 0=>0, 0.0%
    OS/2, +0, 96=>96, 0.0%
    VORG, +0, 0=>0, 0.0%
    cmap, +20, 6408=>6428, 0.0%
    cvt , +0, 0=>0, 0.0%
    fpgm, +0, 0=>0, 0.0%
    gasp, +0, 0=>0, 0.0%
    glyf, -8517, 205393=>196876, -2.8%
    hdmx, +0, 0=>0, 0.0%
    head, +0, 54=>54, 0.0%
    hhea, +0, 36=>36, 0.0%
    hmtx, +34, 13546=>13580, 0.0%
    kern, +0, 0=>0, 0.0%
    loca, +32, 13552=>13584, 0.0%
    maxp, +0, 32=>32, 0.0%
    name, +0, 760=>760, 0.0%
    post, -12914, 49826=>36912, -4.2%
    prep, +0, 0=>0, 0.0%
    vhea, +0, 0=>0, 0.0%
    vmtx, +0, 0=>0, 0.0%
    TOTAL, +44715, 307635=>352350, 14.5%

As I mentioned in the first post, I believe the kerning has been rebuilt which is fine.

I think a lot of these reported changes above are false positives. I suspect this is being caused by fontmake converting glyph names to unixxxx' e.g Aogonek.NAV has become uni0104.NAV which is fine.

It seems your fonts are missing mark, mkmk features. This could be what is causing the issues I have already highlighted.

Thanks Marc!

@m4rc1e I spoke with @sberlow about this today and @asaumierdemers will take a look at it soon, and then it would be great if you could arrange a video call with them to go over how your review and checking workflow is going these days :)

I added @cjdunn and @djrrb to this.
They will be assisting in getting this done.

I sync'd with @sberlow just now and he says to expect a full report of the deltas next week likely from @justvanrossum

Order of events

1. @cjdunn will post fix to this specific issue
   (as not to make to many changes at once)

2. Mark reviews to make sure the accents are correct in all styles, mkmk features

3. @cjdunn will add the instances,

4. @cjdunn we will run Just's scripts to compare var instances to static....
   Report on diff

@davelab6 Here's my PR:
#5

Please let us know if this works for you

Looking into this now.

Class Kerns are missing. Only Format 1s exist.

@m4rc1e can you point me to Roboto sources which do have class kerning? The UFOs in the main Roboto repo here:
https://github.com/google/roboto/tree/master/src/v2
do not have class kerns or groups of any kind, only flat kerns. Thanks.

@davelab6 good question. I wasn't involved early on so I'm not sure what the original source was, but currently there is no class kerning in these UFOs and I'm trying to track it down.

Are these the shipping TTFs you're talking about:
https://github.com/google/roboto/releases/download/v2.138/roboto-unhinted.zip ? Do you know the source of the class kerning in here? If not, perhaps we can try extracting the class kerning from these TTFs. Please let me know if you have any further information about the kerning sources, thanks.

@davelab6 @dberlow @cjdunn
UFO were the sources.

Read the README.md for more details

see googlefonts/ufo2ft#203 (comment)

I'd recommend you extract these kerning classes that are currently defined in the Roboto UFOs features.fea files, and store them instead in groups.plist, and also use them in kerning.plist in place of the "key" glyphs that are currently being used.

This is how the ufo2ft kernFeatureWriter does it:
https://github.com/googlei18n/ufo2ft/blob/5c13d7fefa01090556e1ffbc3b673da06fec9485/Lib/ufo2ft/featureWriters/kernFeatureWriter.py#L159-L209

Note that in the next version of ufo2ft we are planning to drop support for this -- as far as I'm aware, this trick is only used by Roboto, and it is not the way UFO is supposed to store class kerning.

We will only keep supporting the following:

• UFO format 2: kerning classes in groups.plist/kerning.plist prefixed with @MMK_L_ and @MMK_R_;
• UFO format >= 3: kerning classes with public.kern1. and public.kern2. prefixes

@anthrotype thank you for this. I was in the middle of writing something similar.

UFO were the sources

But those UFOs were derived from the release TTFs, right?

I made a PR to clarify the README :)

@davelab6 during the process of running the original Roboto build chain, it will generate ufos for all weights and then make the ttfs. I'm pretty sure these ufos have been used https://github.com/TypeNetwork/Roboto/tree/master/master_ufo

@davelab6 @m4rc1e exactly!

Okay :) In that case I will update the README PR.

Still, the brief is to make the fonts a 1:1 match for the TTFs.

@m4rc1e @davelab6 I tried copying the groups and class kerns from the post-processed TTFs. Here's an updated var font:

Roboto-VF.ttf.zip

Please let me know if this works for you.
cc @sberlow

Yes - please prioritize this project above everything except Markazi

@m4rc1e If you look at the data in the font files you'll see that the kern values for these pairs in the Robot var font and the Robot non-var font are the same. They are as follows:
/A /Ocircumflex -5
/A /Gcommaaccent -5
/A /Gdotaccent -5
/A /ydieresis -24
/A /O -5

The very small differences you're seeing may be due to rounding errors or grid fitting issues related to rasterization for var fonts vs non-var fonts or something else, but it is not the kerning.

@cjdunn

I tried copying the groups and class kerns from the post-processed TTFs. Here's an updated var font

Your results are much better than mine. I mistakenly didn't diff the VF you attached above. Ignore my previous two comments.

When you say you copied the groups and classes, did you mean you copied/manipulated them from the GPOS table in the ttf? are they now stored in the sources? will running fontmake produce this result?

If the answer is yes to the two previous questions, I'll be buying you a brewery, a beer isn't enough.

I mistakenly didn't diff the VF you attached above

But you have now done so and the diff check is showing 0 differences?

When you say you copied the groups and classes, did you mean you copied/manipulated them from the GPOS table in the ttf?

That was what I understood - since the rest of the project took a starting point from quadratic UFOs, not TTFs, but the GPOS table data apparently wasn't in those UFOs.

are they now stored in the sources? will running fontmake produce this result?

@anthrotype does fontmake allow TTX sources to be included and stitched in, as a standard feature?

@m4rc1e @davelab6

When you say you copied the groups and classes, did you mean you copied/manipulated them from the GPOS table in the ttf?
Yes, after researching this problem a bit I figured out I could use this script:
https://github.com/adobe-type-tools/kern-dump/blob/master/convertKernedOTFtoKernedUFO.py
to get compiled kerning and groups from TTFs to non-var UFOs. I then imported the groups and kerning into the var UFO masters. And then I ran a script to remove redundant exceptions, which I think were in the originals but perhaps overlooked because of this roundabout way of getting class kerning.

are they now stored in the sources?
Yes.

will running fontmake produce this result?
Yes, this var font was made via fontmake with no post-processing.

Here's the PR with my updated UFOs: #8

does fontmake allow TTX sources to be included and stitched in, as a standard feature?

Yes, just place the ttx files inside a UFO’s data subfolder, and they will be merged in at compile time.
But I would not recommend to do so for the GPOS kerning. Better to try reconstruct a UFO3-compliant class kerning source from the data available in the UFO2 sources, as I explained above. Not from the binary TTFs generated by fontmake.

@cjdunn Fixing the missing marks is relatively straight forward. If you inspect the missing marks from the report in #8, you'll see the mark anchor is named differently to the base anchor.

If you remove "mark" from all mark anchors, the amount missing goes from 5,000+ to 900. The remaining 900 all involve uni035D, which I'm looking into now.

How the original chain handled these:
https://github.com/google/roboto/blob/master/scripts/lib/fontbuild/markFeature.py#L24-L39

Source setupAnchorPairs func
https://github.com/googlei18n/ufo2ft/blob/6076d4e8f09bb516b9b59f1c1aea824f4bc4c8c6/Lib/ufo2ft/featureWriters/markFeatureWriter.py#L198-L235

@m4rc1e OK got it, I have removed "mark" from all mark anchors. I'm still not sure why these names don't match since we started with your sources. But if you can let me know what other anchors need to be renamed to resolve those remaining 900 missing, I can renamed those as well.

Here's the updated var font:
Roboto-VF.ttf.zip

Please let me know if this works better now, thanks.

@cjdunn I've just discovered that fontmake allows us to add our own custom --kern-writer-module and --mark-writer-module. I've just written one quickly based on the MarkFeatureWriter in the original Roboto. The results are pretty much spot on.

Roboto-Regular.ttf vs Roboto-Regular.ttf

attribs 1 modified

mkmks 7656 new

names 2 modified

The new mkmks may be false. I need to double check these and update the diffenator if so.

@anthrotype I need a second opinion. I think it may be better to provide a custom MarkFeatureWriter and KernFeatureWriter, based off the original chain. Atm, we're changing the sources so the fonts will have the same output when genned with default fontmake settings. I think reviewers will appreciate seeing sources which are as close as possible to the previous.

I was planning to drop those two fontmake command-line options actually... The plan is to have these custom writers specified in the UFO lib (as custom key/value pairs), and ufo2ft will load and use these instead of the default. There's a feature-writer-wip branch in ufo2ft repo where I'm currently working on that. The API of the feature writer class will change also: instead of manipulating strings, they will work directly on the feaLib AST that results from parsing the existing feature file, and extend that in place before compilation. I would still recommend you change your sources so that they won't require any custom feature writer. There's nothing special about Roboto's kern or mark feature, it should be possible to upgrade them to UFO3 standard without need of a custom writer.

@m4rc1e According to what @anthrotype wrote, it sounds like we should still change the sources.

I already renamed all of the pairs mentioned in the link you posted above, which are:

        self.anchorPairs = [
            ["top", "_marktop"],
            ["bottom", "_markbottom"],
            ["top_dd", "_marktop_dd"],
            ["bottom_dd", "_markbottom_dd"],
            ["rhotichook", "_markrhotichook"],
            ["top0315", "_marktop0315"],
            ["parent_top", "_markparent_top"],
            ["parenthesses.w1", "_markparenthesses.w1"],
            ["parenthesses.w2", "_markparenthesses.w2"],
            ["parenthesses.w3", "_markparenthesses.w3"]]

        self.mkmkAnchorPairs = [
            ["mkmktop", "_marktop"],
["mkmkbottom_acc", "_markbottom"],
            ["", "_bottom"],
]

If you can let me know what other anchors need to be renamed to resolve those remaining 900 missing, I can renamed those as well, thanks.

Hey @cjdunn. Sorry but I've had to work on other projects for the time being.

Could we have a hangout sometime next week? I'd like to run you through how I'm QAing so you can do it yourself. A large part of this project is understanding Roboto's buildchain (pre fontmake days)

@m4rc1e Sure, some time towards the end of next week would be good for me, thanks.

@cjdunn @m4rc1e Thursday noon?

@sberlow @m4rc1e that time works for me

Here are the notes Marc prepared for the review on 2018-06-07:

We need to merge #8 - the kerning issue is now solved.

The only issue remaining is the mark positioning. Cosimo says this should be fixed at source, #4 (comment)

Test Setup

To test the fonts, we’re comparing the unhinted v2.136 fonts against your chain, by running fontmake to generate the fonts from your build sources like this:

fontmake --no-production-names -m Roboto.designspace -o ttf

This generates static ttfs. We want to compare the v2.136 ttfs against these instead of a variable font to reduce any noise which may be caused by fontmake producing variable fonts. This noise is not an issue you should be dealing with.

To make the comparisons, we’re using my fontdiffenator like this:

diffenator Roboto-Regular.ttf (v2.136) Roboto-Regular.ttf (from our chain) -ol 5

We should get the following report once the pr is merged:

Roboto-Regular.ttf vs Roboto-Regular.ttf

attribs 1 modified

table               attrib              value_a             value_b
head                modified            2016/12/15 22:52:08 2018/06/07 14:47:00

mkmks 615 new

mark1_glyph         mark2_glyph         mark1_x             mark1_y             mark2_x             mark2_y
acute               uni0237             225                 1290                266                 1249
acutecomb           uni0237             -566                1290                266                 1249
breve               uni0237             429                 1290                266                 1249
breveinvnosp        uni0237             -585                1196                266                 1249
candrabindunosp     uni0237             -585                1196                266                 1249

mkmks 3000 missing

mark1_glyph         mark2_glyph         mark1_x             mark1_y             mark2_x             mark2_y
acutesubnosp        dotdblsubnosp_uni1ABD-607                0                   -671                -352
acutesubnosp        breveinvsubnosp_uni1ABD-607                0                   -582                -372
acutesubnosp        tildesubnosp_uni1ABD-607                0                   -585                -426
acutesubnosp        bridgesubnosp       -607                0                   -585                -464
acutesubnosp        commaaccent         -607                0                   256                 -162

names 1 modified

id                  string_a            string_b
(3, 3, 1, 1033)     Google:Roboto Regular:2016Google:Roboto Regular:2017

marks 59390 new

mark1_glyph         mark2_glyph         mark1_x             mark1_y             mark2_x             mark2_y
A                   uni1AB9             686                 0                   -235                0
A                   uni1ABA             686                 0                   -236                0
A                   uni02BE             672                 1601                70                  1160
A                   uni02F3             686                 0                   302                 -1
A                   comma               1306                0                   128                 68

marks 1777 modified

mark1_glyph         mark2_glyph         diff_x              diff_y
l                   uniFE26             314.0               -157.0
l                   uniFE24             314.0               -157.0
l                   uniFE25             314.0               -157.0
l                   uniFE23             314.0               -157.0
l                   uniFE20             314.0               -157.0

marks 305569 missing

mark1_glyph         mark2_glyph         mark1_x             mark1_y             mark2_x             mark2_y
A                   uni2DE8             672                 1601                -588                1170
A                   ringsubnosp_uni1ABD 686                 0                   -588                0
A                   uniFE21             672                 1601                360                 1170
A                   uniFE24             672                 1601                -312                1170
A                   uniFE20             672                 1601                -360                1170

glyphs 23 modified

glyph               diff
uni1AB5             5577.0
uni047A             1869.08333333
uniFFFC             1280.0
threeeighths        1112.58333333
uni20BA             1056.79166667

We don’t care about these 23 glyphs that are modified, because the diff amounts are negligible - and being introduced by fontmake.

The only issues we care about are the missing and modified marks and mkmks.

When I added my own Markwriters to fontmake, I got the desired result.

Once the marks are fixed, we should end up with this report:

Roboto-Regular.ttf vs Roboto-Regular.ttf

attribs 1 modified

table               attrib              value_a             value_b
head                modified            2016/12/15 22:52:08 2018/06/07 14:47:00

glyphs 23 modified

glyph               diff
uni1AB5             5577.0
uni047A             1869.08333333
uniFFFC             1280.0
threeeighths        1112.58333333
uni20BA             1056.79166667

New marks and mkmks are fine.

Notes from meeting

Here’s the test results of CJ’s anchor renaming:

marks 59390 new

mark1_glyph         mark2_glyph         mark1_x             mark1_y             mark2_x             mark2_y
A                   uni1AB9             686                 0                   -235                0
A                   uni02BE             672                 1601                70                  1160
A                   uni02F3             686                 0                   302                 -1
A                   cedilla             686                 0                   213                 0
A                   uni1FBF             672                 1601                181                 1182

marks 2437 modified

mark1_glyph         mark2_glyph         diff_x              diff_y
uni1E79             uni1ABC             -1.0                664.0
uni1E4D             uni1ABC             -1.0                664.0
uni1E4D             uni1ABB             -1.0                664.0
uni1E79             uni1ABB             -1.0                664.0
uni1E53             uni1ABB             0.0                 646.0

marks 3871 missing

mark1_glyph         mark2_glyph         mark1_x             mark1_y             mark2_x             mark2_y
A                   uni035D             1336                1600                0                   1162
A                   uni0361             1336                1600                0                   1193
A                   uni035E             1336                1600                0                   1243
A                   uni0360             1336                1600                0                   1274
A.smcp              uni035E             1158                1600                0                   1243

I believe this review is now complete; @m4rc1e please close if so