googlefonts / ufo2ft

A bridge from UFOs to FontTools objects (and therefore, OTFs and TTFs).
MIT License
152 stars 43 forks source link

[Designspace/VF] Regression in 2.28.0 #630

Closed justvanrossum closed 2 years ago

justvanrossum commented 2 years ago

We've found a regression with ufo2ft 2.28.0, related to the difference in behavior regarding postscript names for named instances.

After setting up a venv with fontmake 3.3.0, I built the two example fonts like this:

pip install --upgrade ufo2ft==2.27.0 && fontmake -m DS5BreakTest.designspace -o variable --output-path test-ufo2ft-2.27.0.ttf

And

pip install --upgrade ufo2ft==2.28.0 && fontmake -m DS5BreakTest.designspace -o variable --output-path test-ufo2ft-2.28.0.ttf

Sources and built fonts are included in the attached archive. ufo2ft_regression.zip

I pasted the fdiff for fvar and name tables below. It seems the instance PS names aren't computed correctly. Previously, they weren't added at all.

I bet it can be fixed by tweaking the source data (how, though?), but this is a rather nasty breakage for a setup that used to work fine.

--- test-ufo2ft-2.27.0.ttf  2022-07-07T10:43:13.917108+02:00
+++ test-ufo2ft-2.28.0.ttf  2022-07-07T10:43:23.797517+02:00
@@ -12,29 +12,24 @@
       <MaxValue>800.0</MaxValue>
       <AxisNameID>256</AxisNameID>
     </Axis>
-
-    <!-- ExtraLight -->
-    <NamedInstance flags="0x0" subfamilyNameID="257">
+    <!-- PostScript: DS5BreakTest- -->
+    <NamedInstance flags="0x0" postscriptNameID="258" subfamilyNameID="257">
       <coord axis="wght" value="200.0"/>
     </NamedInstance>
-
-    <!-- Regular -->
-    <NamedInstance flags="0x0" subfamilyNameID="258">
+    <!-- PostScript: DS5BreakTest- -->
+    <NamedInstance flags="0x0" postscriptNameID="259" subfamilyNameID="257">
       <coord axis="wght" value="400.0"/>
     </NamedInstance>
-
-    <!-- Medium -->
-    <NamedInstance flags="0x0" subfamilyNameID="259">
+    <!-- PostScript: DS5BreakTest- -->
+    <NamedInstance flags="0x0" postscriptNameID="260" subfamilyNameID="257">
       <coord axis="wght" value="480.0"/>
     </NamedInstance>
-
-    <!-- Bold -->
-    <NamedInstance flags="0x0" subfamilyNameID="260">
+    <!-- PostScript: DS5BreakTest- -->
+    <NamedInstance flags="0x0" postscriptNameID="261" subfamilyNameID="257">
       <coord axis="wght" value="666.66667"/>
     </NamedInstance>
-
-    <!-- ExtraBold -->
-    <NamedInstance flags="0x0" subfamilyNameID="261">
+    <!-- PostScript: DS5BreakTest- -->
+    <NamedInstance flags="0x0" postscriptNameID="262" subfamilyNameID="257">
       <coord axis="wght" value="800.0"/>
     </NamedInstance>
   </fvar>
@@ -44,19 +39,22 @@
       Weight
     </namerecord>
     <namerecord nameID="257" platformID="1" platEncID="0" langID="0x0" unicode="True">
-      ExtraLight
+      
     </namerecord>
     <namerecord nameID="258" platformID="1" platEncID="0" langID="0x0" unicode="True">
-      Regular
+      DS5BreakTest-
     </namerecord>
     <namerecord nameID="259" platformID="1" platEncID="0" langID="0x0" unicode="True">
-      Medium
+      DS5BreakTest-
     </namerecord>
     <namerecord nameID="260" platformID="1" platEncID="0" langID="0x0" unicode="True">
-      Bold
+      DS5BreakTest-
     </namerecord>
     <namerecord nameID="261" platformID="1" platEncID="0" langID="0x0" unicode="True">
-      ExtraBold
+      DS5BreakTest-
+    </namerecord>
+    <namerecord nameID="262" platformID="1" platEncID="0" langID="0x0" unicode="True">
+      DS5BreakTest-
     </namerecord>
     <namerecord nameID="0" platformID="3" platEncID="1" langID="0x409">
       None
@@ -92,19 +90,22 @@
       Weight
     </namerecord>
     <namerecord nameID="257" platformID="3" platEncID="1" langID="0x409">
-      ExtraLight
+      
     </namerecord>
     <namerecord nameID="258" platformID="3" platEncID="1" langID="0x409">
-      Regular
+      DS5BreakTest-
     </namerecord>
     <namerecord nameID="259" platformID="3" platEncID="1" langID="0x409">
-      Medium
+      DS5BreakTest-
     </namerecord>
     <namerecord nameID="260" platformID="3" platEncID="1" langID="0x409">
-      Bold
+      DS5BreakTest-
     </namerecord>
     <namerecord nameID="261" platformID="3" platEncID="1" langID="0x409">
-      ExtraBold
+      DS5BreakTest-
+    </namerecord>
+    <namerecord nameID="262" platformID="3" platEncID="1" langID="0x409">
+      DS5BreakTest-
     </namerecord>
   </name>

Cc. @jeremiehornus

anthrotype commented 2 years ago

I'll be away a couple of weeks so I hope @madig or @belluzj can take a look on my behalf, they should be able to cut releases of all the libraries involved, including fonttools

justvanrossum commented 2 years ago

(Sorry for the "DS5BreakTest" name: we were assuming it is related to the DS5 work, but that is not actually confirmed by us at this point.)

belluzj commented 2 years ago

No worries about the name, we'll have a look! It could be related to the new code to generate names from the STAT data.

belluzj commented 2 years ago

@justvanrossum Can I add your DS file as a test case in fontTools? It's indeed the code that makes names for instances on the file that is trying too hard, in this case there's no STAT data at all since it's DS4 and it's still trying to make some postscript names (and failing). We'll fix it in fontTools I believe.

Just FYI the problem can be observed by asking for the instance names using the new DS5 APIs like so:


from fontTools.designspaceLib import DesignSpaceDocument as DS
d = DS.fromfile("DS5BreakTest.designspace")
from fontTools.designspaceLib.statNames import getStatNames
names = getStatNames(d, d.instances[0].location)
StatNames(familyNames={'en': 'DS5BreakTest'}, styleNames={'en': ''}, postScriptFontName='DS5BreakTest-', styleMapFamilyNames={'en': 'DS5BreakTest'}, styleMapStyleName='regular')
justvanrossum commented 2 years ago

Sure. Feel free to strip it further from redundant stuff. This was adapted from a @blackfoundrycom retail font, but I think I removed everything that could be sensitive.

belluzj commented 2 years ago

This PR seems to have fixed the issue for us after we're reproduced it locally: https://github.com/fonttools/fonttools/pull/2684

Thanks for the test file!

belluzj commented 2 years ago

Fixed in https://github.com/fonttools/fonttools/releases/tag/4.34.3