-
Notifications
You must be signed in to change notification settings - Fork 219
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
Add new Opentype STAT table #96
base: master
Are you sure you want to change the base?
Conversation
New file `src/tables/STAT.js` will decode/encode Opentype STAT table data. The undocumented struct version 0x00010000 is handled as example font files included this version number. This new table type is added to those mentioned in `src/tables/index.js`. The test file executes using sample STAT tables from four different open-licensed font files. Both input binary data and expected results are included for each sample. Each sample is slightly different and exercises variations on the STAT table format. Only the AxisValue format 3 is not tested by these samples. One additional open-licensed font file is included, Selawik-variable.ttf, as mentioned in the test file. This font file demonstrates many of the latest Opentype variation table additions. The test includes extracting a STAT table from this font file, exercising decoding by mainline code. The test file lastly tests encoding back to original binary data.
Any problems with source code formatting? The new font file has many of the newer font variations OTF tables, including the STAT table, and so I wanted to include it in the pool of test fonts. It is OFL licensed in the original production font, and this variation includes that same OFL mention in its name table strings. This font file includes tables:
and so this font could be useful for other testing/development. Please let me know any other concerns, and I'd expect you'll have me do at least some changes... :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for implementing this! Just a few formatting things.
@@ -0,0 +1,54 @@ | |||
import r from 'restructure'; | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra whitespace line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in all places
axisTag: new r.String(4), // e.g. 'ital', 'wght' | ||
axisNameID: r.uint16, | ||
axisOrdering: r.uint16, | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolons
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh frabjous day. Attempting to check for missing semicolons also in the test file, to copy styles seen in other test files, eslint just kinda mentioned that I wasn't actually using a couple of the binary data sample tables 02, 03, 04. Missing semicolons and missed tests verified. Other cleanups done.
}, | ||
// Obsoleted version 1.0 ? | ||
0x00010000: { | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put this on the same line as the opening brace
valueNameID: r.uint16, | ||
}, | ||
1: { | ||
value: r.fixed32be, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything is assumed to be big endian by default. r.fixed32
should work and is an alias for r.fixed32be
0x00, 0x00, // 0004 uint16 flags | ||
0x01, 0x05, // 0006 uint16 valueNameID | ||
0x02, 0xbc, 0x00, 0x00 // 0008 Fixed(16.16) value 700.0 | ||
]; // 0078 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of including the binary data in the source code, can we just include the font file in the repo and read the table directly in the tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned in change message and test file, checking even the few example font files I could find with STAT table, found differing data formats. Interestingly the deprecated/undocumented STAT format 1.0 was found twice. My intent with the sets of binary/expanded data was to exercise the format variations found so far, including missing or present (non-null) data. Otherwise the older format 0 file would have been a surprise requiring a fix.
I was thinking including the self-contained binary data in the test was better than committing to including multiple font files. And certainly thinking that testing variations is better than not.
If Selawik-variable.ttf
is included as proposed here, then one of the binary stanzas can go away in favor of reading from the file directly. (I picked this font file to include because of all the other newer tables it also contains)
The TestGVAREight.ttf
is from the same series of test file you have already sampled, and is quite small. We could switch to include it here and skip that binary table.
Questions:
- multiple tests where variations seen? (formats, data presence/absence, etc.)
- self-contained data for multiple testing where sample fonts are otherwise uninteresting?
- what guidance on when to include new font files to drive testing?
Notes:
481,504 Selawik-variable.ttf
106,184 AmstelvarAlpha-VF.ttf
264,380 Decovar-VF.ttf
4,692 TestGVAREight.ttf (from text-rendering-tests)
# Hinting | ||
This version of Selawik is primarily hinted with the light Latin hinting style Microsoft recommends for variable Latin fonts. The VTT Light Latin autohinter was used to create the first round of hints, which were then reviewed and touched up. | ||
|
||
This hinting style only uses CVTs for vertical metrics anchors (ascender, descender, cap height, x-height, and baseline). While this makes for an easy job hinting a Latin font, it makes for a poor test case because Selawik doesn't vary vertical metrics with weight, thus doesn't vary CVTs, thus doesn't need a cvar. So, to make it more interesting, we added CVT-based stem hints to the lowercase only. This provided the need to vary CVTs and thus require a cvar. Note that this was only done for testing purposes. For variable fonts, Microsoft recommends the light hinting style using the `ResYDist()` command instead of a CVT-based stem hint. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a license available for this font that we can include?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both the original selawk.ttf and this modified Selawik-variable.ttf have the same notices in the name
table:
0: © 2015 Microsoft Corporation (www.microsoft.com), with Reserved Font Name Selawik. Selawik is a trademark of Microsoft Corporation in the United States and/or other countries.
13: This Font Software is licensed under the SIL Open Font License, Version 1.1.
14: http://opensource.org/licenses/OFL-1.1
I had ginned up an OFL.txt that began with these lines:
Copyright (c) 2015 Microsoft Corporation (www.microsoft.com), with Reserved Font Name Selawik. Selawik is a trademark of Microsoft Corporation in the United States and/or other countries.
This Font Software is licensed under the SIL Open Font License, Version 1.1.
This license is copied below, and is also available with a FAQ at:
http://scripts.sil.org/OFL
followed by the OFL 1.1 text.
While I can't find a copy of any license file in the (original 2015) Selawik_Release.zip
, I kinda felt presumptuous generating the missing license file. I'll do so to match your other font file entries if you wish.
|
||
let readEncoded = estream.read() | ||
// Is there a better way to extract bytes from stream buffer? | ||
let readArray = readEncoded.toString('binary').split('').map((c) => c.charCodeAt()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can do Array.from(readEncoded)
// let result = tableSTAT.decode(stream); | ||
let result = tables.STAT.decode(stream); | ||
|
||
//console.log(' decoded STAT table: %j', result); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the commented logs
New file
src/tables/STAT.js
will decode/encode OpentypeSTAT
table data.The undocumented struct version 0x00010000 is handled as example font files
included this version number. This new table type is added to those
mentioned in
src/tables/index.js
.The test file executes using sample
STAT
tables from four differentopen-licensed font files. Both input binary data and expected results
are included for each sample. Each sample is slightly different and
exercises variations on the STAT table format. Only the
AxisValue
format 3 is not tested by these samples.
One additional open-licensed font file is included, Selawik-variable.ttf,
as mentioned in the test file. This font file demonstrates many of the
latest Opentype variation table additions. The test includes extracting
a
STAT
table from this font file, exercising decoding by mainline code.The test file lastly tests encoding back to original binary data.