-
Notifications
You must be signed in to change notification settings - Fork 489
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
os2: Fill Panose when writing font #761
base: master
Are you sure you want to change the base?
Conversation
[why] When an existing font file is read from a buffer and then written back into a buffer all Panose values will be zero. The problem has been most likly been introduced with PR opentypejs#630 (but I did not check that). There are two disjunct data structures in the font.tables.os2 object that describe the panose values: * An array with the Panose values `.panose = [ 1, 2, ...]` * Dedicated properties for each Panose value e.g. `.bFamilyType` The aforementioned PR seems to address only fonts created from scratch and not parsed from a buffer. Writing out the font into a buffer will always use the dedicated Panose properties and ignore the .panose array. Parsing a font does set the array but not the dedicated properties. They are not even existing then. [how] When an existing font is parsed the `.panose` array is filled (as before). But now the dedicated properties are also created and filled with the individual values. [note] The written font is always using the dedicated values. If a user changes the panose array that will have no effect. There are no checks if the data is consistent. Having the same data in two disjunct structures is not so nice to handle. Signed-off-by: Fini Jastrow <[email protected]>
Well, now (with this PR) the user has both, the array and the individual properties (see image below). To avoid confusion I would drop one of the two. Anyhow, changing API with |
I believe how Panose is represented in other applications can be important, to see what users might expect.
|
Yeah, that's a good point... We have this issue in other places as well I think. Proxy objects would be a solution, or we should drop the raw values alltogether as you suggested. |
[why] There is no check if the os2 table can be written to a buffer and restored completely. [how] Do some spot checks for values. Signed-off-by: Fini Jastrow <[email protected]>
Added some test for os2 table parsing writing parsing (full round trip). I struggled a bit with where to put the tests, as Anyhow, this is at least some starting point, feel free to move/remove the tests ;-D |
Parsing a font from a buffer and writing it back to a buffer changes all Panose values to zero.
This PR fixes this.
Description
[why]
When an existing font file is read from a buffer and then written back into a buffer all Panose values will be zero.
The problem has been most likly been introduced with PR #630 (but I did not check that).
There are two disjunct data structures in the font.tables.os2 object that describe the panose values:
.panose = [ 1, 2, ...]
.bFamilyType
The aforementioned PR seems to address only fonts created from scratch and not parsed from a buffer.
Writing out the font into a buffer will always use the dedicated Panose properties and ignore the .panose array.
Parsing a font does set the array but not the dedicated properties. They are not even existing then.
[how]
When an existing font is parsed the
.panose
array is filled (as before). But now the dedicated properties are also created and filled with the individual values.[note]
The written font is always using the dedicated values. If a user changes the panose array that will have no effect. There are no checks if the data is consistent. Having the same data in two disjunct structures is not so nice to handle.
Motivation and Context
I want to read a font file and write it (modified) back to a file; as unchanged as possible.
How Has This Been Tested?
Manual tests with ttx ;)
Screenshots (if appropriate):
Types of changes
Checklist:
npm run test
and all tests passed green (including code styling checks).