Merge pull request #1272 from googlefonts/even_more_panose

Support panose from instances
googlefonts · Feb 13, 2025 · 4935617 · 4935617
2 parents e21efe3 + 1663795
commit 4935617
Show file tree

Hide file tree

Showing 3 changed files with 203 additions and 9 deletions.
diff --git a/glyphs-reader/src/font.rs b/glyphs-reader/src/font.rs
@@ -1065,6 +1065,7 @@ pub struct Instance {
     pub type_: InstanceType,
     pub axis_mappings: BTreeMap<String, AxisUserToDesignMap>,
     pub axes_values: Vec<OrderedFloat<f64>>,
+    pub custom_parameters: CustomParameters,
 }
 
 /// <https://github.com/googlefonts/glyphsLib/blob/6f243c1f732ea1092717918d0328f3b5303ffe56/Lib/glyphsLib/classes.py#L150>
@@ -1102,6 +1103,7 @@ struct RawInstance {
 
     weight_class: Option<String>,
     width_class: Option<String>,
+    custom_parameters: RawCustomParameters,
 }
 
 impl RawInstance {
@@ -2328,7 +2330,7 @@ impl Instance {
     ///
     /// Mappings based on
     /// <https://github.com/googlefonts/glyphsLib/blob/6f243c1f732ea1092717918d0328f3b5303ffe56/Lib/glyphsLib/classes.py#L3451>
-    fn new(axes: &[Axis], value: &RawInstance) -> Self {
+    fn new(axes: &[Axis], value: &RawInstance) -> Result<Self, Error> {
         let active = value.is_active();
         let mut axis_mappings = BTreeMap::new();
 
@@ -2363,7 +2365,7 @@ impl Instance {
                 .unwrap_or(100.0),
         );
 
-        Instance {
+        Ok(Instance {
             name: value.name.clone(),
             active,
             type_: value
@@ -2373,7 +2375,8 @@ impl Instance {
                 .unwrap_or(InstanceType::Single),
             axis_mappings,
             axes_values: value.axes_values.clone(),
-        }
+            custom_parameters: value.custom_parameters.to_custom_params()?,
+        })
     }
 }
 
@@ -2443,7 +2446,7 @@ impl TryFrom<RawFont> for Font {
             .instances
             .iter()
             .map(|ri| Instance::new(&axes, ri))
-            .collect();
+            .collect::<Result<Vec<_>, Error>>()?;
 
         let default_master_idx = default_master_idx(&from);
         let axis_mappings = UserToDesignMapping::new(&from, &instances);
@@ -2590,6 +2593,12 @@ impl Font {
         &self.masters[self.default_master_idx]
     }
 
+    pub fn instance(&self, master: &FontMaster) -> Option<&Instance> {
+        self.instances
+            .iter()
+            .find(|i| i.active && i.type_ == InstanceType::Variable && i.name == master.name)
+    }
+
     pub fn vendor_id(&self) -> Option<&String> {
         self.names.get("vendorID")
     }

diff --git a/glyphs2fontir/src/source.rs b/glyphs2fontir/src/source.rs
@@ -325,11 +325,11 @@ impl Work<Context, WorkId, Error> for StaticMetadataWork {
             .as_ref()
             .map(|bits| bits.iter().copied().collect());
 
-        if let Some(raw_panose) = font
-            .default_master()
-            .custom_parameters
-            .panose
-            .as_ref()
+        let default_master = font.default_master();
+        let default_instance = font.instance(default_master);
+        if let Some(raw_panose) = default_instance
+            .and_then(|di| di.custom_parameters.panose.as_ref())
+            .or(default_master.custom_parameters.panose.as_ref())
             .or(font.custom_parameters.panose.as_ref())
         {
             let mut bytes = [0u8; 10];
@@ -1839,4 +1839,16 @@ mod tests {
         let static_metadata = context.static_metadata.get();
         assert_eq!(Tag::new(b"NONE"), static_metadata.misc.vendor_id);
     }
+
+    #[test]
+    fn which_panose_shall_i_get() {
+        // Learned from GSC that panose can live in instances
+        let (_, context) =
+            build_static_metadata(glyphs3_dir().join("WghtVarInstancePanose.glyphs"));
+        let static_metadata = context.static_metadata.get();
+        assert_eq!(
+            Some(Panose::from([4, 5, 6, 7, 8, 9, 10, 11, 12, 13])),
+            static_metadata.misc.panose
+        );
+    }
 }
diff --git a/resources/testdata/glyphs3/WghtVarInstancePanose.glyphs b/resources/testdata/glyphs3/WghtVarInstancePanose.glyphs
@@ -0,0 +1,173 @@
+{
+.formatVersion = 3;
+familyName = WghtVarInstancePanose;
+
+axes = (
+{
+name = Weight;
+tag = wght;
+}
+);
+
+customParameters = (
+{
+name = panose;
+value = (
+1,
+2,
+3,
+4,
+5,
+6,
+7,
+8,
+9,
+10
+);
+}
+);
+
+fontMaster = (
+{
+id = m01;
+name = Regular;
+axesValues = (
+400
+);
+customParameters = (
+{
+name = panose;
+value = (
+2,
+3,
+4,
+5,
+6,
+7,
+8,
+9,
+10,
+11
+);
+}
+);
+},
+{
+id = m02;
+name = Bold;
+axesValues = (
+700
+);
+customParameters = (
+{
+name = panose;
+value = (
+3,
+4,
+5,
+6,
+7,
+8,
+9,
+10,
+11,
+12
+);
+}
+);
+}
+);
+glyphs = (
+{
+glyphname = space;
+layers = (
+{
+layerId = m01;
+width = 200;
+},
+{
+layerId = m02;
+width = 250;
+}
+);
+unicode = 32;
+}
+);
+
+instances = (
+{
+customParameters = (
+{
+name = panose;
+value = (
+4,
+5,
+6,
+7,
+8,
+9,
+10,
+11,
+12,
+13
+);
+}
+);
+name = Regular;
+type = variable;
+},
+{
+axesValues = (
+400
+);
+customParameters = (
+{
+name = panose;
+value = (
+5,
+6,
+7,
+8,
+9,
+10,
+11,
+12,
+13,
+14
+);
+}
+);
+name = Regular;
+},
+{
+axesValues = (
+700
+);
+customParameters = (
+{
+name = panose;
+value = (
+6,
+7,
+8,
+9,
+10,
+11,
+12,
+13,
+14,
+15
+);
+}
+);
+isBold = 1;
+linkStyle = Regular;
+name = Bold;
+weightClass = 700;
+}
+);
+
+unitsPerEm = 1024;
+versionMajor = 42;
+versionMinor = 42;
+}