Add read / write support for CFF private DICT data #759

Finii · 2024-09-09T01:49:45Z

For CFF fonts the private DICT is not read and not written.

Some preparatory work for private DICT reading has been done with the preparation for CFF2, but not finished.
And in fact is useful for CFF1 also.

Writing is needed to have hints (BlueValues) in CFF fonts.

Description

[why]
The hinting of CFF (1 and 2) Open Type fonts is usually entirely in their private DICT data. To preserve the hinting - which is needed to make the font rendering look good and as expected - the private DICT data needs to be read and written.

[how]
Parts of the needed code seem to be added already in preparation for CFF2. I guess there was a misunderstanding or a mixture of versions, but the private DICT did not change between CFF1 and CFF2, and we need to always use the PRIVATE_DICT_META_CFF2, the link given in its definition is the same as given with link [1] for CFF1. See also [2]. So firstly we always refer to 'version 2', as the previous code did also but only in one case.

For writing the delta values the encoding function is missing and so that is added. encode.delta() just calls encode.number() repeatedly, figuratively speaking.

Last but not least the correct private DICT length has to be set.

[1] https://adobe-type-tools.github.io/font-tech-notes/pdfs/5176.CFF.pdf
[2] https://learn.microsoft.com/en-us/typography/opentype/otspec180/cff2#appendixD

Motivation and Context

It came unexpected that opentype.js silently drops the hinting of CFF fonts it reads and writes out.
This happened here:

Fonts that were downloaded via the website strips the PS Private dict eigilnikolajsen/commit-mono#69

The commit mono font is crafted with probably fontlab, but the release files are downloaded through opentype.js - which drops the hinting.

THAT has been reported to me here, because fontforge automagically adds the most basic hinting if there is none at all:

Unpatched CommitMono and patched CommitMono looks different ryanoasis/nerd-fonts#1696

How Has This Been Tested?

Took several CFF fonts with BlueValues, opened them with opentype.js and wrote the back.
The resulting font file has then been manually (:grimacing:) compared with the originals via ttx and fontforge.
Well, the PS private aka private DICT.

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I did npm run test and all tests passed green (including code styling checks).
I have added tests to cover my changes.
My change requires a change to the documentation. [3]
I have updated the README accordingly.
I have read the Contribute README section.

[3] I believe there is no documentation stub where I could add to. Please refer me if there is one.

Connum · 2024-09-09T08:49:39Z

Thanks for putting in the work!

Connum · 2024-09-09T08:54:10Z

There seems to be a problem now to load the CID font FDArrayTest257.otf, I will examine later.

The tests are running through fine - what problem are you encountering? And can you please add a test case for it? Thanks!

Finii · 2024-09-09T09:15:52Z

The tests are running through fine - what problem are you encountering?

I fixed it with the force push, there were some more bugs (in the original code) 😬 😉

aa2fe01 cff: Correct private DICT definition
a47a5f3 cff: Fix private DICT processing

I added another commit (84004b8) that translates the delta transport encoding to human readable numbers and back again on encode.
This of course would break code (if the private DICT would have worked before), but I believe users expect to be able to set the delta values as usual as absolute numbers and not mess around with transport encoding. At least I did.
But of course you can simply drop that commit.

I did not write any tests, but tested this manually with ttx and fontforge. I will update the description above now.

Finii · 2024-09-09T09:21:21Z

(more comments are in the commit messages, where I would search for them later on ;)

Finii · 2024-09-09T09:30:11Z

src/tables/cff.mjs

@@ -308,6 +308,17 @@ function interpretDict(dict, meta, strings) {
            if (m.type === 'SID') {
                value = getCFFString(strings, value);
            }
+            if (m.type === 'delta' && value !== null) {
+                if (!Array.isArray(value) || value.length % 2 != 0) {


I fact I can not see in the spec that an odd number of values is forbidden. But of course it would not make any sense, a BlueZone that starts somewhere and ends nowhere? I believe I remember to have read sometime that there must always be an even number of numbers grr, an even amount of numbers; but I can not find a reference for that.

Maybe someone knows more and/or you like to remove the even/odd test. It's in princible only a safeguard against unusable delta arrays. 🤷‍♀️

Connum · 2024-09-09T09:38:21Z

The change regarding the delta values might be something I'll have to keep in mind when further working on #701

Thank you!

Connum · 2024-09-09T09:39:20Z

Can you please fix the linting issues so the tests run through? We should also really have tests for this, so that nothing breaks in the future! Thanks!

[why] FamilyBlues has two Ops, probably a copy and paste leftover. And two Ops are missing. This is now in line with CFF and CFF2 fonts. Signed-off-by: Fini Jastrow <[email protected]>

[why] Tests (would) fail with FDArrayTest257.otf [how] Some sanity checks have been ommited, add them in and process the structures only if there is anything. Signed-off-by: Fini Jastrow <[email protected]>

[why] The hinting of CFF (1 and 2) Open Type fonts is usually entirely in their private DICT data. To preserve the hinting - which is needed to make the font rendering look good and as expected - the private DICT data needs to be read and written. [how] Parts of the needed code seem to be added already in preparation for CFF2. I guess there was a misunderstanding or a mixture of versions, but the private DICT did not change between CFF1 and CFF2, and we need to always use the PRIVATE_DICT_META_CFF2, the link given in its definition is the same as given with link [1] for CFF1. See also [2]. So firstly we always refer to 'version 2', as the previous code did also but only in one case. The OtherBlues and FamilyOtherBlues operators must occur after the BlueValues and FamilyBlues operators, respectively. This is implicitely guaranteed by the way the private DICT is set up and finally Object.keys() works. For writing the delta values the encoding function is missing and so that is added. encode.delta() just calls encode.number() repeatedly, figuratively speaking. Last but not least the correct private DICT length has to be set. [1] https://adobe-type-tools.github.io/font-tech-notes/pdfs/5176.CFF.pdf [2] https://learn.microsoft.com/en-us/typography/opentype/otspec180/cff2#appendixD Signed-off-by: Fini Jastrow <[email protected]>

Finii · 2024-09-09T09:52:13Z

linting

shure ... oopsi you forced a 2nd time ;)

$ git push --force-with-lease
To https://github.com/Finii/opentype.js.git
 ! [rejected]        feature/output-CFF-private-DICT -> feature/output-CFF-private-DICT (stale info)
error: failed to push some refs to 'https://github.com/Finii/opentype.js.git'

~~You did it now already I guess~~

[why] At the moment the delta values are not de- or encoded. The delta format is a packed format to have the smallest possible values in the array of numbers (i.e. relative to the previous number). But usually users do not face this but are shown absolute values; in all other font specific applications. For example BlueValues [ 500, 550 ] // User sees the BlueZone is from 500 to 550 Encoded as [ 500, 50 ] opentype.js at the moment does not translate these deltas in any way and users must know this and use the inconvenient 'packed' encoding format. [how] Convert the read relative delta values to absolute coordinates in the blueValues (and other) properties, that users can interact with. On font encoding time the absolute coordinates are converted back to relative ones and encoded in the font file. [note] https://adobe-type-tools.github.io/font-tech-notes/pdfs/5176.CFF.pdf The second and subsequent numbers in a delta are encoded as the difference between successive values. For example, an array a0, a1, ..., an would be encoded as: a0 (a1–a0) (a2–a1) ..., (an–a(n–1)) This is done because small numbers can be encoded with fewer bytes and the total aim is to reduce the size. Signed-off-by: Fini Jastrow <[email protected]>

Finii · 2024-09-09T09:58:09Z

We should also really have tests for this, so that nothing breaks in the future!

If you define 'this' I could probably throw something together ;-D
For checking the written files I would add some testing dependency like fonttools or just ttx 🤔
Hard to check writing just with the code that writes.
Of course possible with some hard coded hexdump.

[why] The StemSnapH and StemSnapV values are deltas, according to [1]. Also noticed that HERE we have deltas with an odd number of values, so the check for even number of elements is wrong. [how] Change data type and remove check on read/write. Adapt test. [1] https://adobe-type-tools.github.io/font-tech-notes/pdfs/5176.CFF.pdf (CFF version 1.0, Table 23 'Private DICT Operators') Signed-off-by: Fini Jastrow <[email protected]>

Finii · 2024-09-09T10:31:18Z

See commit message for explanation. Obviously wrong type in the table - changed that.

Right: Example font (from test/fonts/ directory) that has 3 values in delta of StemSnapH

Finii · 2024-09-09T12:20:26Z

Added test code that checks

font file with private DICT
parse it
create new font from it
parse the new font
check the parse result's private DICT

and it turns out there is a problem with reproducing subrs, or at least with checing it.
Working on it.

Code:

     it('does round trip CFF private DICT', function() {
         const font = loadSync('./test/fonts/AbrilFatface-Regular.otf');
         // from ttx:
         //     <Private>
         //        <BlueValues value="-10 0 476 486 700 711"/>
         //        <OtherBlues value="-250 -238"/>
         //        <BlueScale value="0.039625"/>
         //        <BlueShift value="7"/>
         //        <BlueFuzz value="1"/>
         //        <StdHW value="18"/>
         //        <StdVW value="186"/>
         //        <StemSnapH value="16 18 21"/>
         //        <StemSnapV value="120 186 205"/>
         //        <ForceBold value="0"/>
         const checkerFunktion = function(inputFont) {
             const privateDict = inputFont.tables.cff.topDict._privateDict;
             assert.deepEqual(privateDict.blueValues, [-10, 0, 476, 486, 700, 711]);
             assert.deepEqual(privateDict.otherBlues, [-250, -238]);
             assert.equal(privateDict.stdHW, 18);
             assert.deepEqual(privateDict.stemSnapH, [16, 18, 21]);
             assert.equal(privateDict.nominalWidthX, 590);
         };
         checkerFunktion(font);
         let buffer = font.toArrayBuffer()
         let font2 = parse(buffer);
         checkerFunktion(font2);
     });

Already parse() fails 😒

Connum · 2024-09-09T13:55:38Z

If you define 'this' I could probably throw something together ;-D
this = everything you're fixing/implementing with this PR ;)

Hard to check writing just with the code that writes.

The way we check other writing functionality is loading the produced font data and checking that the read-out data is as expected.

[why] The code does not handle subrs, but it includes a previously existing subrs operator in the private DICT which throws all the offsets off. [how] Either include the subrs subtable and keep the subrs operator, or drop both. This commits just drops the operator (for simplicity). Proper subrs handling can be added later on. Signed-off-by: Fini Jastrow <[email protected]>

[why] Writing of the private DICT has been added together with the translation of delta values from transport encoding to readable and back. But there is no automated test to see if something breaks. [how] Take one of the fonts in test/fonts/ and run in through ttx to see its private DICT data analyzed by someone else. Add a test that also reads that fond and checks some of the private DICT values. Write the font into a buffer and read it back in; repeat the check on the read back font. This also checks if writing is correct. Signed-off-by: Fini Jastrow <[email protected]>

Finii · 2024-09-09T15:30:23Z

Fix private DICT in the presence of subrs (i.e. still does not include it as before, but correctly say so by removing a possible subrs operator from the private DICT.

Then introduce a test for what has been added; checking both reading and writing (by re-reading).
The bitstream itself is not checked and also not checked with some other tool. I think more testtools should rather be added by main Maintainers; suggestion ttx.

Taken from [1], added in edit

[1] https://adobe-type-tools.github.io/font-tech-notes/pdfs/5176.CFF.pdf

[why] While CFF has no `vsindex` operator in the private DICT it is a defaulted operator in CFF2. This can not be really disentangled with the current code and the opertor ends up erroreously written to CFF fonts (where it is usually ignored). [how] It took a bit time to understand the CFF version code (that I believe is not finished), but reinstate that code with different tables to be used. Sorry I messed with the code in the first place; I had overlooked the `vsindex` operator and how a defaulted operator in the tables turn out in the written files. I just reverted the version 'detection' code as it was, not checking anything. Note that there still is one hard-coded `2` somewhere, but that was already there when I started. Signed-off-by: Fini Jastrow <[email protected]>

Finii · 2024-09-10T16:18:02Z

Did manual check on different machine.

Start from scratch opentype repo and convert one example font
Use opentype from the PR and convert same font
Show diff of the two generated fonts:

$ git clone https://github.com/opentypejs/opentype.js.git
$ cd opentype.js
$ git remote add fork https://github.com/Finii/opentype.js.git
$ git fetch fork
$ npm install
$ npm run build
$ node
> const { default: opentype } = await import("./dist/opentype.js");
> const { default: fs } = await import("fs");
> let font = opentype.parse(fs.readFileSync('./test/fonts/AbrilFatface-Regular.otf'));
> fs.writeFileSync('abril.otf', Buffer.from(font.toArrayBuffer()));
> .exit
$ git checkout feature/output-CFF-private-DICT
$ npm run build
$ node
> const { default: opentype } = await import("./dist/opentype.js");
> const { default: fs } = await import("fs");
> let font = opentype.parse(fs.readFileSync('./test/fonts/AbrilFatface-Regular.otf'));
> fs.writeFileSync('abril2.otf', Buffer.from(font.toArrayBuffer()));
> .exit
$ ttx abril.otf abril2.otf
$ diff -u abril.ttx abril2.ttx

--- abril.ttx	2024-09-10 13:08:45.540452926 +0200
+++ abril2.ttx	2024-09-10 13:08:45.466452833 +0200
@@ -433,12 +433,12 @@
     <!-- Most of this table will be recalculated by the compiler -->
     <tableVersion value="1.0"/>
     <fontRevision value="1.0"/>
-    <checkSumAdjustment value="0xffbb1068"/>
+    <checkSumAdjustment value="0x88bcf41f"/>
     <magicNumber value="0x5f0f3cf5"/>
     <flags value="00000000 00000011"/>
     <unitsPerEm value="1000"/>
-    <created value="Tue Sep 10 11:05:57 2024"/>
-    <modified value="Tue Sep 10 11:05:57 2024"/>
+    <created value="Tue Sep 10 11:07:57 2024"/>
+    <modified value="Tue Sep 10 11:07:57 2024"/>
     <xMin value="-181"/>
     <yMin value="-291"/>
     <xMax value="1131"/>
@@ -1071,15 +1071,21 @@
       <!-- charset is dumped separately as the 'GlyphOrder' element -->
       <Encoding name="StandardEncoding"/>
       <Private>
+        <BlueValues value="-10 0 476 486 700 711"/>
+        <OtherBlues value="-250 -238"/>
         <BlueScale value="0.039625"/>
         <BlueShift value="7"/>
         <BlueFuzz value="1"/>
+        <StdHW value="18"/>
+        <StdVW value="186"/>
+        <StemSnapH value="16 18 21"/>
+        <StemSnapV value="120 186 205"/>
         <ForceBold value="0"/>
         <LanguageGroup value="0"/>
         <ExpansionFactor value="0.06"/>
         <initialRandomSeed value="0"/>
-        <defaultWidthX value="0"/>
-        <nominalWidthX value="0"/>
+        <defaultWidthX value="500"/>
+        <nominalWidthX value="590"/>
       </Private>
       <CharStrings>
         <CharString name=".notdef">

Then I did a comparsion original font versus opentype.js export from master branch, and there are these findings

--- AbrilFatface-Regular.ttx	2024-09-10 13:11:55.420692296 +0200
+++ ../../abril.ttx	2024-09-10 13:08:45.540452926 +0200
-    <fontRevision value="1.001"/>
+    <fontRevision value="1.0"/>

-    <xMaxExtent value="1131"/>
+    <xMaxExtent value="1489"/>

     <panose>
-      <bFamilyType value="2"/>
+      <bFamilyType value="0"/>
       <bSerifStyle value="0"/>
-      <bWeight value="5"/>
-      <bProportion value="3"/>
+      <bWeight value="0"/>
+      <bProportion value="0"/>
       <bContrast value="0"/>
       <bStrokeVariation value="0"/>
       <bArmStyle value="0"/>
-      <bLetterForm value="2"/>
+      <bLetterForm value="0"/>
       <bMidline value="0"/>
-      <bXHeight value="3"/>
+      <bXHeight value="0"/>
     </panose>

     <CFFFont name="AbrilFatface-Regular">
-      <version value="001.001"/>
+      <version value="Version 1.001"/>

-      <FontBBox value="-181 -291 1131 1058"/>
+      <FontBBox value="0 -291 1058 1141"/>

       <Private>
-        <BlueValues value="-10 0 476 486 700 711"/>
-        <OtherBlues value="-250 -238"/>
         <BlueScale value="0.039625"/>
         <BlueShift value="7"/>
         <BlueFuzz value="1"/>
-        <StdHW value="18"/>
-        <StdVW value="186"/>
-        <StemSnapH value="16 18 21"/>
-        <StemSnapV value="120 186 205"/>
         <ForceBold value="0"/>
         <LanguageGroup value="0"/>
         <ExpansionFactor value="0.06"/>
         <initialRandomSeed value="0"/>
-        <defaultWidthX value="500"/>
-        <nominalWidthX value="590"/>
-        <Subrs>
-          <CharString index="0">
           ...
       </Private>

       <CharStrings>
           I guess the subrs have been included here
-  <GPOS>
           Completely removed

Problems

Version number
Something changes with the glyph boundingboxes / bearings
Panose is dropped?
Private is dropped (thus this PR)
GPOS dropped

At least Panose and the version numbers seem to be easily fixed. Is there an interest in a PR; then I can give it some attention.

Finii · 2024-09-10T16:46:50Z

Panose

Fix OS/2 usFirstCharIndex #196 mentions missing Panose
Fixed metadata that enables fonts to be recognised as a family (tested!) #630 should fix this? Tests implemented now?

Seems to be a write problem (i.e. reading works)?

Edit: Panose fixed in

os2: Fill Panose when writing font #761

Version

Hard to search for but I can not find any issue/mention

Edit: Version fix in

cff: Fix version number #762

Finii force-pushed the feature/output-CFF-private-DICT branch from aeb3319 to aea28e5 Compare September 9, 2024 08:01

Finii marked this pull request as ready for review September 9, 2024 09:20

Finii commented Sep 9, 2024

View reviewed changes

Connum force-pushed the feature/output-CFF-private-DICT branch from 84004b8 to f33a89c Compare September 9, 2024 09:37

Finii added 3 commits September 9, 2024 11:47

cff: Correct private DICT definition

08c751e

[why] FamilyBlues has two Ops, probably a copy and paste leftover. And two Ops are missing. This is now in line with CFF and CFF2 fonts. Signed-off-by: Fini Jastrow <[email protected]>

cff: Fix private DICT processing

aabf3b8

[why] Tests (would) fail with FDArrayTest257.otf [how] Some sanity checks have been ommited, add them in and process the structures only if there is anything. Signed-off-by: Fini Jastrow <[email protected]>

Connum force-pushed the feature/output-CFF-private-DICT branch from f33a89c to ccab03f Compare September 9, 2024 09:47

Finii force-pushed the feature/output-CFF-private-DICT branch from ccab03f to ffcfd38 Compare September 9, 2024 09:54

Finii added 2 commits September 9, 2024 17:18

Finii mentioned this pull request Sep 11, 2024

cff: Fix version number #762

Open

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add read / write support for CFF private DICT data #759

Add read / write support for CFF private DICT data #759

Finii commented Sep 9, 2024 •

edited

Loading

Connum commented Sep 9, 2024

Connum commented Sep 9, 2024

Finii commented Sep 9, 2024

Finii commented Sep 9, 2024

Finii Sep 9, 2024

Connum commented Sep 9, 2024

Connum commented Sep 9, 2024

Finii commented Sep 9, 2024 •

edited

Loading

Finii commented Sep 9, 2024

Finii commented Sep 9, 2024

Finii commented Sep 9, 2024 •

edited

Loading

Connum commented Sep 9, 2024

Finii commented Sep 9, 2024 •

edited

Loading

Finii commented Sep 10, 2024

Finii commented Sep 10, 2024 •

edited

Loading

Add read / write support for CFF private DICT data #759

Are you sure you want to change the base?

Add read / write support for CFF private DICT data #759

Conversation

Finii commented Sep 9, 2024 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Connum commented Sep 9, 2024

Connum commented Sep 9, 2024

Finii commented Sep 9, 2024

Finii commented Sep 9, 2024

Finii Sep 9, 2024

Choose a reason for hiding this comment

Connum commented Sep 9, 2024

Connum commented Sep 9, 2024

Finii commented Sep 9, 2024 • edited Loading

Finii commented Sep 9, 2024

Finii commented Sep 9, 2024

Finii commented Sep 9, 2024 • edited Loading

Connum commented Sep 9, 2024

Finii commented Sep 9, 2024 • edited Loading

Finii commented Sep 10, 2024

Finii commented Sep 10, 2024 • edited Loading

Finii commented Sep 9, 2024 •

edited

Loading

Finii commented Sep 9, 2024 •

edited

Loading

Finii commented Sep 9, 2024 •

edited

Loading

Finii commented Sep 9, 2024 •

edited

Loading

Finii commented Sep 10, 2024 •

edited

Loading