Merge pull request #1102 from Hoolean/extra-panose-param

Support long parameter name for PANOSE, to match glyphsLib
googlefonts · Nov 7, 2024 · 9b7a563 · 9b7a563
2 parents 825bda4 + f5e25f3
commit 9b7a563
Show file tree

Hide file tree

Showing 4 changed files with 138 additions and 8 deletions.
diff --git a/glyphs-reader/src/font.rs b/glyphs-reader/src/font.rs
@@ -437,10 +437,14 @@ impl CustomParameters {
     }
 
     fn panose(&self) -> Option<&Vec<i64>> {
-        let Some(CustomParameterValue::Panose(values)) = self.get("panose") else {
-            return None;
-        };
-        Some(values)
+        // PANOSE custom parameter is accessible under a short name and a long name:
+        //     https://github.com/googlefonts/glyphsLib/blob/050ef62c/Lib/glyphsLib/builder/custom_params.py#L322-L323
+        // ...with the value under the short name taking precendence:
+        //     https://github.com/googlefonts/glyphsLib/blob/050ef62c/Lib/glyphsLib/builder/custom_params.py#L258-L269
+        match self.get("panose").or_else(|| self.get("openTypeOS2Panose")) {
+            Some(CustomParameterValue::Panose(values)) => Some(values),
+            _ => None,
+        }
     }
 }
 
@@ -569,7 +573,9 @@ impl FromPlist for CustomParameters {
                                 value =
                                     Some(CustomParameterValue::CodepageRange(tokenizer.parse()?));
                             }
-                            _ if name == Some(String::from("panose")) => {
+                            _ if name == Some(String::from("panose"))
+                                || name == Some(String::from("openTypeOS2Panose")) =>
+                            {
                                 let Token::OpenParen = peek else {
                                     return Err(Error::UnexpectedChar('('));
                                 };

diff --git a/glyphs2fontir/src/source.rs b/glyphs2fontir/src/source.rs
@@ -1135,7 +1135,7 @@ mod tests {
     };
     use glyphs_reader::{glyphdata::Category, Font};
     use indexmap::IndexSet;
-    use ir::{test_helpers::Round2, Panose};
+    use ir::test_helpers::Round2;
     use write_fonts::types::{NameId, Tag};
 
     use crate::source::names;
@@ -1966,8 +1966,31 @@ mod tests {
 
     #[test]
     fn captures_panose() {
+        // short parameter name
         let (_, context) = build_static_metadata(glyphs3_dir().join("WghtVarPanose.glyphs"));
-        let expected: Panose = [2, 0, 5, 3, 6, 0, 0, 2, 0, 3].into();
-        assert_eq!(Some(expected), context.static_metadata.get().misc.panose);
+        assert_eq!(
+            Some([2, 0, 5, 3, 6, 0, 0, 2, 0, 3].into()),
+            context.static_metadata.get().misc.panose
+        );
+    }
+
+    #[test]
+    fn captures_panose_long() {
+        // long parameter name
+        let (_, context) = build_static_metadata(glyphs3_dir().join("WghtVarPanoseLong.glyphs"));
+        assert_eq!(
+            Some([2, 0, 5, 3, 6, 0, 0, 2, 0, 3].into()),
+            context.static_metadata.get().misc.panose
+        );
+    }
+
+    #[test]
+    fn captures_panose_precedence() {
+        // both parameters; value under short name should be preferred
+        let (_, context) = build_static_metadata(glyphs3_dir().join("WghtVarPanoseBoth.glyphs"));
+        assert_eq!(
+            Some([2, 0, 5, 3, 6, 0, 0, 2, 0, 3].into()),
+            context.static_metadata.get().misc.panose
+        );
     }
 }
diff --git a/resources/testdata/glyphs3/WghtVarPanoseBoth.glyphs b/resources/testdata/glyphs3/WghtVarPanoseBoth.glyphs
@@ -0,0 +1,58 @@
+{
+.formatVersion = 3;
+customParameters = (
+{
+name = panose;
+value = (
+2,
+0,
+5,
+3,
+6,
+0,
+0,
+2,
+0,
+3
+);
+},
+{
+name = openTypeOS2Panose;
+value = (
+0,
+1,
+2,
+3,
+4,
+5,
+6,
+7,
+8,
+9
+);
+}
+);
+familyName = WghtVar;
+fontMaster = (
+{
+id = m01;
+name = Regular;
+}
+);
+glyphs = (
+{
+glyphname = space;
+layers = (
+{
+layerId = m01;
+width = 200;
+}
+);
+unicode = 32;
+}
+);
+
+unitsPerEm = 1234;
+versionMajor = 42;
+versionMinor = 42;
+}
diff --git a/resources/testdata/glyphs3/WghtVarPanoseLong.glyphs b/resources/testdata/glyphs3/WghtVarPanoseLong.glyphs
@@ -0,0 +1,43 @@
+{
+.formatVersion = 3;
+customParameters = (
+{
+name = openTypeOS2Panose;
+value = (
+2,
+0,
+5,
+3,
+6,
+0,
+0,
+2,
+0,
+3
+);
+}
+);
+familyName = WghtVar;
+fontMaster = (
+{
+id = m01;
+name = Regular;
+}
+);
+glyphs = (
+{
+glyphname = space;
+layers = (
+{
+layerId = m01;
+width = 200;
+}
+);
+unicode = 32;
+}
+);
+
+unitsPerEm = 1234;
+versionMajor = 42;
+versionMinor = 42;
+}