Skip to content
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

Fonts that were downloaded via the website strips the PS Private dict #69

Open
AnggaSP opened this issue Nov 17, 2023 · 9 comments
Open
Labels
Issue Something to look into

Comments

@AnggaSP
Copy link

AnggaSP commented Nov 17, 2023

Description:
It seems the font customization strips the PS Private? See this Image:
image

This affects postscript hinting on mac and more importantly, this prevents afdko/otfautohint from running.

@eigilnikolajsen eigilnikolajsen added the Issue Something to look into label Nov 17, 2023
@eigilnikolajsen
Copy link
Owner

Must be an OpenType.js issue.

@Finii
Copy link

Finii commented Aug 28, 2024

The problem also exists if one downloads the release zip archive from

https://github.com/eigilnikolajsen/commit-mono/releases/tag/v1.143

Is there a recommended way to get an intact font file?

image

@Finii
Copy link

Finii commented Aug 28, 2024

These file contains the blues, maybe that was meant by AnggaSP

image

while the zip archive in-repo also has fonts without blues...; this does not contain blues:

image

Strangeness.

@Finii
Copy link

Finii commented Aug 29, 2024

There are even more astonishing differences.

Here I compare the zip (release) version with the 'from fontlab directory' version:

$ ls -l
total 5352
-rw-r--r-- 1 fini fini  274752 Aug 29 13:15 CommitMono-400-Regular.otf
-rw-rw-r-- 1 fini fini  161124 Aug 29 13:14 CommitMonoV143-400Regular.otf
$ ttx C*
...
$ ls -l
total 5352
-rw-r--r-- 1 fini fini  274752 Aug 29 13:15 CommitMono-400-Regular.otf
-rw-rw-r-- 1 fini fini 2660722 Aug 29 13:24 CommitMono-400-Regular.ttx
-rw-rw-r-- 1 fini fini  161124 Aug 29 13:14 CommitMonoV143-400Regular.otf
-rw-rw-r-- 1 fini fini 2373405 Aug 29 13:24 CommitMonoV143-400Regular.ttx
$ diff -u CommitMono-400-Regular.ttx CommitMonoV143-400Regular.ttx

The diff colored, zip is red and fontlab is green:

   <head>
     <!-- Most of this table will be recalculated by the compiler -->
     <tableVersion value="1.0"/>
-    <fontRevision value="1.0"/>
-    <checkSumAdjustment value="0xebc3c57b"/>
+    <fontRevision value="1.14299"/>
+    <checkSumAdjustment value="0xa7a818a8"/>
     <magicNumber value="0x5f0f3cf5"/>
     <flags value="00000000 00000011"/>
     <unitsPerEm value="1000"/>
-    <created value="Thu Dec 28 09:00:29 2023"/>
-    <modified value="Thu Dec 28 09:00:29 2023"/>
+    <created value="Sun Jun 11 09:40:24 2023"/>
+    <modified value="Thu Dec 28 08:58:15 2023"/>
     <xMin value="-120"/>
     <yMin value="-500"/>
     <xMax value="1644"/>
@@ -1964,9 +1964,9 @@
     <descent value="-200"/>
     <lineGap value="0"/>
     <advanceWidthMax value="600"/>
-    <minLeftSideBearing value="30"/>
-    <minRightSideBearing value="-918"/>
-    <xMaxExtent value="1794"/>
+    <minLeftSideBearing value="-120"/>
+    <minRightSideBearing value="-1044"/>
+    <xMaxExtent value="1644"/>
     <caretSlopeRise value="1"/>
     <caretSlopeRun value="0"/>
     <caretOffset value="0"/>
@@ -1975,7 +1975,7 @@
     <reserved2 value="0"/>
     <reserved3 value="0"/>
     <metricDataFormat value="0"/>
-    <numberOfHMetrics value="1932"/>
+    <numberOfHMetrics value="1"/>
   </hhea>
 
   <maxp>
@@ -2003,23 +2003,23 @@
     <yStrikeoutPosition value="324"/>
     <sFamilyClass value="0"/>
     <panose>
-      <bFamilyType value="0"/>
-      <bSerifStyle value="0"/>
-      <bWeight value="0"/>
-      <bProportion value="0"/>
-      <bContrast value="0"/>
-      <bStrokeVariation value="0"/>
-      <bArmStyle value="0"/>
-      <bLetterForm value="0"/>
-      <bMidline value="0"/>
-      <bXHeight value="0"/>
+      <bFamilyType value="2"/>
+      <bSerifStyle value="11"/>
+      <bWeight value="8"/>
+      <bProportion value="9"/>
+      <bContrast value="2"/>
+      <bStrokeVariation value="2"/>
+      <bArmStyle value="2"/>
+      <bLetterForm value="2"/>
+      <bMidline value="2"/>
+      <bXHeight value="4"/>
     </panose>
     <ulUnicodeRange1 value="10000000 00000000 00000000 10100111"/>
     <ulUnicodeRange2 value="00010000 00000000 00111001 11110011"/>
     <ulUnicodeRange3 value="00000000 00000100 00000000 00100000"/>
     <ulUnicodeRange4 value="00000000 00000000 00000000 00000000"/>
     <achVendID value="    "/>
-    <fsSelection value="00000000 00000000"/>
+    <fsSelection value="00000000 11000000"/>
     <usFirstCharIndex value="0"/>
     <usLastCharIndex value="65533"/>
     <sTypoAscender value="900"/>
@@ -2033,48 +2033,45 @@
     <sCapHeight value="700"/>
     <usDefaultChar value="0"/>
     <usBreakChar value="32"/>
-    <usMaxContext value="0"/>
+    <usMaxContext value="7"/>
   </OS_2>
 
   <name>
...

Analysis of diff:

  • zip has version 1.0 ?!
  • Font file creation (modification date) is almost the same, but actual creation date lost?
  • Min/max bearings completely different, contains different glyphs?
  • The fontlab version does not really contain HMetrics (1 instead of 1900), thus it is much smaller (see above)
  • Panose in zip broken
  • fsSelection in zip broken

Lets have a look at the small letter a

-    <mtx name="a" width="600" lsb="30"/>
+    <mtx name="a" width="600" lsb="81"/>

How could the lsb be different? At least when read by Glyphs3 both font have the same lsb for letter a, see image below - I do not know where this comes from, a ttx bug? Later: Ah maybe that is due to the nice "smart kerning" feature

And this...

image

Maybe @eigilnikolajsen can shed some light on this. What are the different fonts, which is supposed to be 'the' release font?
I guess something with the production workflow is amiss. The dropped Panose also happen(s/ed) with CascadiaCode and happens because of the tooling for example.
But why can the files in commit-mono/src/fonts/CommitMono-1.143.zip be different from the file commit-mono/src/fonts/fontlab/CommitMonoV143-400Regular.otf?

A guess

What I guess is that the font has been created with Fontlab. The fonts in the fontlab directory are the originals, with the default Stylistic Sets and Character Variants. The website (i.e. https://commitmono.com/) allows to 'burn in' some SS and CV. Even the default font is run through that workflow and the zip file from the website with default settings is re-imported into the repo as "the zip", which ends up in the directory mentioned above and on the release page.
That website workflow "breaks" stuff in the font file, like we already know from CascadiaCode and Monaspace iirc, lets see... Hmm, microsoft/cascadia-code#674 does not have DHowett's explanation; I remember some thread over there where we discussed this and other problems and DHowett did identify the problematic build step. Cant find it. The workflow over there was, iirc, design in Glyphs, export UFO, use scripts to further process etc.

@eigilnikolajsen
Copy link
Owner

Thank you for this.
I think most of the differences are due to OpenType.js being incomplete in the data it exports (or imports for that matter). I don't really know how to fix that.

@Finii
Copy link

Finii commented Aug 29, 2024

Ah now I see how you do it ...

image

Respect! I could not put everything into the website like this.

At least this answers what are the "real" files, as the website also loads the font that are in the fontlab directory:
image

I can try to help out here. Is there a repo for the website?

@Finii
Copy link

Finii commented Sep 4, 2024

It is indeed a problem of opentype.js.

First I tried to update it to current master (which has many unreleased fixes, notably adding CFF2 which in principle should support the missing blue values in PS private.) And at least it reads them now correctly (with some help from my side):

image

But there is still a problem to write PS private back, continuing...

@Finii
Copy link

Finii commented Sep 4, 2024

To not forget my changes in opentype.js

Wrong version used in parsing

There are multiple versions in a font, meaning the version of the header standard.
In other similar places upstream already has 2 hardcoded.

-- /home/ujastrow/git/opentype.js/dist/opentype.js	2024-09-04 11:44:38.794842197 +0200
+++ opentype.git.js	2024-09-04 15:30:41.782125178 +0200
@@ -6575,7 +6575,7 @@
       const privateSize = version < 2 ? topDict.private[0] : 0;
       const privateOffset = version < 2 ? topDict.private[1] : 0;
       if (privateSize !== 0 && privateOffset !== 0) {
-        const privateDict = parseCFFPrivateDict(data, privateOffset + start, privateSize, strings, version);
+        const privateDict = parseCFFPrivateDict(data, privateOffset + start, privateSize, strings, 2);
         topDict._defaultWidthX = privateDict.defaultWidthX;
         topDict._nominalWidthX = privateDict.nominalWidthX;
         if (privateDict.subrs !== 0) {
@@ -7272,7 +7272,7 @@
     }
     if (header.formatMajor < 2) {
       const privateDictOffset = start + topDict.private[1];
-      const privateDict = parseCFFPrivateDict(data, privateDictOffset, topDict.private[0], stringIndex.objects, header.formatMajor);
+      const privateDict = parseCFFPrivateDict(data, privateDictOffset, topDict.private[0], stringIndex.objects, 2);
       font.defaultWidthX = privateDict.defaultWidthX;
       font.nominalWidthX = privateDict.nominalWidthX;
       if (privateDict.subrs !== 0) {

Font generation

The tables collected by the parsing is not used, they actually fail to fill this.
And here they forgot to add the version number where the called function must work with 'undefined'.

--- /home/ujastrow/git/opentype.js/dist/opentype.js	2024-09-04 11:44:38.794842197 +0200
+++ opentype.git.js	2024-09-04 15:30:41.782125178 +0200
@@ -7534,7 +7534,7 @@
       attrs.paintType = topDictOptions.paintType;
       attrs.strokeWidth = topDictOptions.strokeWidth || 0;
     }
-    const privateAttrs = {};
+    const privateAttrs = topDictOptions._privateDict || {};
     const glyphNames = [];
     let glyph;
     for (let i = 1; i < glyphs.length; i += 1) {
@@ -7549,7 +7549,13 @@
     t.globalSubrIndex = makeGlobalSubrIndex();
     t.charsets = makeCharsets(glyphNames, strings);
     t.charStringsIndex = makeCharStringsIndex(glyphs, cffVersion);
-    t.privateDict = makePrivateDict(privateAttrs, strings);
+    t.privateDict = makePrivateDict(privateAttrs, strings, 2);
     t.stringIndex = makeStringIndex(strings);
     const startOffset = t.header.sizeOf() + t.nameIndex.sizeOf() + t.topDictIndex.sizeOf() + t.stringIndex.sizeOf() + t.globalSubrIndex.sizeOf();
     attrs.charset = startOffset;

Remaining issues

This can of course be wrong, these are just my ideas at the moment.

There is no implementation it seems to translate type: "delta" table entries in the 'make' direction (parsing the font does work). On the other hand a encode.delta function does not help either 🤔

And the whole font generation fails if there are Private entries, even when they are no deltas.

@Finii
Copy link

Finii commented Sep 12, 2024

Dear @eigilnikolajsen

Thank you for this. I think most of the differences are due to OpenType.js being incomplete in the data it exports (or imports for that matter). I don't really know how to fix that.

The last days I looked into opentype.js and created 3 PRs in their repo to fix the most obvious problems.

I am not sure if or when they will pull them, but for your font distribution it would be beneficial. I would suggest that you use instead of (your anyhow own-hosted) opentype.min.js a custom opentype.js which is based on their master branch (which is quite ahead of their release version) and additionally the small fixes I suggested over there - to get a custom Commit Mono with all capabilities preserved.

The change is simple for you (just exchange that one file) and the only thing needed for you is cloning my PR and running npm run build. I can help you with that, but I guess you are better at Javascript than I am. But ask if anything is unclear :-)

Of course the default / Gitlab release font will then also change (i.e. be fixed). It would be nice if that happens before the next NerdFont release (end September) so we can already incorporate it.

Cheers,

Fini

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue Something to look into
Projects
None yet
Development

No branches or pull requests

3 participants