Closed m4rc1e closed 6 years ago
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
@cjdunn will post fix to this specific issue (as not to make to many changes at once)
Mark reviews to make sure the accents are correct in all styles, mkmk features
@cjdunn will add the instances,
@cjdunn we will run Just's scripts to compare var instances to static.... Report on diff
@davelab6 Here's my PR: https://github.com/TypeNetwork/Roboto/pull/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.
Isn't this VF derived from the shipping ttf though, not the UFO sources?
On Thu, Apr 12, 2018, 4:27 PM CJ Dunn notifications@github.com wrote:
@m4rc1e https://github.com/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.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/TypeNetwork/Roboto/issues/4#issuecomment-380824987, or mute the thread https://github.com/notifications/unsubscribe-auth/AAP9yyxxJTL9ZNSJ4NeZkcM71h3eEH6pks5tn2RrgaJpZM4QpBTL .
@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.
The TTFs were the sources I believe, and had, I believe, flat kerning.
Sent from my iPad
On Apr 13, 2018, at 4:36 PM, CJ Dunn notifications@github.com wrote:
@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.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.
@davelab6 @dberlow @cjdunn UFO were the sources.
Read the README.md for more details
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:
@MMK_L_
and @MMK_R_
;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:
Please let me know if this works for you. cc @sberlow
@cjdunn Still got some issues.
Imo, @anthrotype's approach is the only way to do this. @davelab6 can I devote some time to attempt this myself tomorrow?
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: https://github.com/TypeNetwork/Roboto/pull/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.
attribs 1 modified
table | attrib | value_a | value_b |
---|---|---|---|
head | modified | 2016/12/15 22:52:08 | 2018/05/15 09:38:12 |
mkmks 7656 new
mark1_glyph | mark2_glyph | mark1_x | mark1_y | mark2_x | mark2_y |
---|---|---|---|---|---|
acutecomb | macroncomb_uni1ABB | -566 | 1290 | -578 | 1540 |
acutecomb | dieresisnosp_uni1ABB | -566 | 1290 | -671 | 1540 |
acutecomb | tildecomb_uni1ABB | -566 | 1290 | -537 | 1562 |
acutecomb | uni034A | -566 | 1290 | -583 | 1644 |
acutecomb | acuterightnosp | -566 | 1290 | -103 | 1614 |
names 2 modified
id | string_a | string_b |
---|---|---|
(3, 3, 1, 1033) | Google:Roboto Regular:2016 | Google:Roboto Regular:2017 |
(4, 3, 1, 1033) | Roboto | Roboto Regular |
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 https://github.com/TypeNetwork/Roboto/pull/8 - the kerning issue is now solved.
The only issue remaining is the mark positioning. Cosimo says this should be fixed at source, https://github.com/TypeNetwork/Roboto/issues/4#issuecomment-389116168
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:
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:
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.
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
First things first, the quality of the chain and font work is fantastic. Building Roboto using the official buildchain takes a long time. I was super happy to see your chain produce the unhinted fonts in a tenth of the time.
For this review, I'm testing ttfs which have been built by running fontmake on your design space files with the following command:
Once the fonts were built, I've compared them against the official Roboto v2.138 unhinted fonts
So far I've only focused on the Black weight.
Has the kerning been rebuilt? I don't mind if it has. As long as line lengths remain consistent, I'm happy. I'll figure out a way to test this.
I've used a new tool we're currently developing in order to catch visual regressions. I'm attaching a zip of the sample images it has generated as well. If I produce a plot of all the characters and view them across 4 browsers, I can only find 2 issues.
Acutes have shifted before![screen shot 2017-11-23 at 16 50 59](https://user-images.githubusercontent.com/7525512/33183125-812aa0ea-d06e-11e7-83f5-0e0f8dd70bf2.png)
after![screen shot 2017-11-23 at 16 49 32](https://user-images.githubusercontent.com/7525512/33183107-73cf7d12-d06e-11e7-8bad-a11157eb8c2f.png)
Misplaced mark positions before![screen shot 2017-11-23 at 16 54 15](https://user-images.githubusercontent.com/7525512/33183256-08760aa8-d06f-11e7-9a73-c7579fe15949.png)
after![screen shot 2017-11-23 at 16 54 06](https://user-images.githubusercontent.com/7525512/33183274-12941be2-d06f-11e7-943e-e5601a5368fb.png)
Keep this thread open, I need to do further checks.
TN_Roboto_img.zip