opentype.js icon indicating copy to clipboard operation
opentype.js copied to clipboard

Add read / write support for CFF private DICT data

Open Finii opened this issue 1 year ago • 14 comments
trafficstars

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:

  • https://github.com/eigilnikolajsen/commit-mono/issues/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:

  • https://github.com/ryanoasis/nerd-fonts/issues/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)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • [x] 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.

Finii avatar Sep 09 '24 01:09 Finii

Thanks for putting in the work!

Connum avatar Sep 09 '24 08:09 Connum

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!

Connum avatar Sep 09 '24 08:09 Connum

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) :grimacing: :wink:

  • aa2fe01959baa0a cff: Correct private DICT definition
  • a47a5f375a0b3edcd485d1ad4b18ac06f19331ee cff: Fix private DICT processing

I added another commit (84004b849b5d) 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 avatar Sep 09 '24 09:09 Finii

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

Finii avatar Sep 09 '24 09:09 Finii

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

Thank you!

Connum avatar Sep 09 '24 09:09 Connum

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!

Connum avatar Sep 09 '24 09:09 Connum

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~~

Finii avatar Sep 09 '24 09:09 Finii

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 :thinking: Hard to check writing just with the code that writes. Of course possible with some hard coded hexdump.

Finii avatar Sep 09 '24 09:09 Finii

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

image

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

Finii avatar Sep 09 '24 10:09 Finii

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 :unamused:~~

Finii avatar Sep 09 '24 12:09 Finii

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.

Connum avatar Sep 09 '24 13:09 Connum

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.

image

Taken from [1], added in edit

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

Finii avatar Sep 09 '24 15:09 Finii

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 avatar Sep 10 '24 16:09 Finii

Panose

  • #196 mentions missing Panose
  • #630 should fix this? Tests implemented now?

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

image

Edit: Panose fixed in

  • #761

Version

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

Edit: Version fix in

  • #762

Finii avatar Sep 10 '24 16:09 Finii