Merge pull request #935 from googlefonts/nightly-avar

[fontbe/avar] Fix bug where PLM supplies a bad input to binary_search

fontbe/src/avar.rs

@@ -1,7 +1,7 @@
 //! Generates a [avar](https://learn.microsoft.com/en-us/typography/opentype/spec/avar) table.
 
 use fontdrasil::{
-    coords::{CoordConverter, DesignCoord, NormalizedCoord},
+    coords::NormalizedCoord,
     orchestration::{Access, Work},
     types::Axis,
 };
@@ -40,15 +40,7 @@ fn default_segment_map() -> SegmentMaps {
 }
 
 fn to_segment_map(axis: &Axis) -> SegmentMaps {
-    // default normalization
-    let default_converter = CoordConverter::new(
-        vec![
-            (axis.min, DesignCoord::new(-1.0)),
-            (axis.default, DesignCoord::new(0.0)),
-            (axis.max, DesignCoord::new(1.0)),
-        ],
-        1,
-    );
+    let default_converter = axis.default_converter();
 
     // We have to walk twice but we don't expect there to be a lot of values so don't stress
 
@@ -199,4 +191,29 @@ mod tests {
             dump(to_segment_map(&axis(mappings, 0)))
         );
     }
+
+    #[test]
+    fn zero_zero_map_should_always_be_present() {
+        // "wdth" axis mappings from NotoSerif.glyphspackage, followed by the
+        // expected avar mappings.
+        // A change in the implementation of core::slice::binary_search in Rust
+        // 1.83-nightly was causing the 0:0 map to be omitted from the avar table.
+        // https://github.com/googlefonts/fontc/issues/933
+        let mappings = vec![
+            (UserCoord::new(62.5), DesignCoord::new(70.0)),
+            (UserCoord::new(75.0), DesignCoord::new(79.0)),
+            (UserCoord::new(87.5), DesignCoord::new(89.0)),
+            (UserCoord::new(100.0), DesignCoord::new(100.0)),
+        ];
+        assert_eq!(
+            vec![
+                (-1.0, -1.0),
+                (-0.6667, -0.7),
+                (-0.3333, -0.3666),
+                (0.0, 0.0),
+                (1.0, 1.0)
+            ],
+            dump(to_segment_map(&axis(mappings, 3)))
+        );
+    }
 }

fontdrasil/src/coords.rs

@@ -191,6 +191,28 @@ impl CoordConverter {
         }
     }
 
+    /// Initialize default converter from user to predefined -1.0/0.0/1.0 normalized coords.
+    ///
+    /// This is normally the first step of `avar` normalization.
+    /// <https://learn.microsoft.com/en-us/typography/opentype/spec/avar#overview>
+    pub fn default_normalization(
+        min: UserCoord,
+        default: UserCoord,
+        max: UserCoord,
+    ) -> CoordConverter {
+        let mut default_mappings = Vec::new();
+        let mut default_idx = 0;
+        if min < default {
+            default_mappings.push((min, DesignCoord::new(-1.0)));
+            default_idx = 1;
+        }
+        default_mappings.push((default, DesignCoord::new(0.0)));
+        if max > default {
+            default_mappings.push((max, DesignCoord::new(1.0)));
+        }
+        CoordConverter::new(default_mappings, default_idx)
+    }
+
     /// Initialize a converter from just min/default/max user coords, e.g. a source with no mapping
     pub fn unmapped(min: UserCoord, default: UserCoord, max: UserCoord) -> CoordConverter {
         let mut mappings = vec![
@@ -456,7 +478,7 @@ impl Debug for NormalizedLocation {
 #[cfg(test)]
 mod tests {
 
-    use super::{CoordConverter, DesignCoord, UserCoord};
+    use super::{CoordConverter, DesignCoord, NormalizedCoord, UserCoord};
 
     // From <https://github.com/googlefonts/fontmake-rs/blob/main/resources/text/units.md>
     fn lexend_weight_mapping() -> (Vec<(UserCoord, DesignCoord)>, usize) {
@@ -582,4 +604,70 @@ mod tests {
             1
         );
     }
+
+    #[test]
+    fn default_normalization() {
+        let converter = CoordConverter::default_normalization(
+            UserCoord::new(50.0),
+            UserCoord::new(100.0),
+            UserCoord::new(150.0),
+        );
+
+        assert_eq!(converter.len(), 3);
+        assert_eq!(converter.default_idx, 1);
+        assert_eq!(
+            UserCoord::new(50.0).to_normalized(&converter),
+            NormalizedCoord::new(-1.0)
+        );
+        assert_eq!(
+            UserCoord::new(100.0).to_normalized(&converter),
+            NormalizedCoord::new(0.0)
+        );
+        assert_eq!(
+            UserCoord::new(150.0).to_normalized(&converter),
+            NormalizedCoord::new(1.0)
+        );
+    }
+
+    #[test]
+    fn default_normalization_contains_no_duplicates_when_min_equals_default() {
+        // https://github.com/googlefonts/fontc/issues/933
+        let converter = CoordConverter::default_normalization(
+            UserCoord::new(50.0),
+            UserCoord::new(50.0),
+            UserCoord::new(100.0),
+        );
+
+        assert_eq!(converter.len(), 2);
+        assert_eq!(converter.default_idx, 0);
+        assert_eq!(
+            UserCoord::new(50.0).to_normalized(&converter),
+            NormalizedCoord::new(0.0)
+        );
+        assert_eq!(
+            UserCoord::new(100.0).to_normalized(&converter),
+            NormalizedCoord::new(1.0)
+        );
+    }
+
+    #[test]
+    fn default_normalization_contains_no_duplicates_when_max_equals_default() {
+        // https://github.com/googlefonts/fontc/issues/933
+        let converter = CoordConverter::default_normalization(
+            UserCoord::new(50.0),
+            UserCoord::new(100.0),
+            UserCoord::new(100.0),
+        );
+
+        assert_eq!(converter.len(), 2);
+        assert_eq!(converter.default_idx, 1);
+        assert_eq!(
+            UserCoord::new(50.0).to_normalized(&converter),
+            NormalizedCoord::new(-1.0)
+        );
+        assert_eq!(
+            UserCoord::new(100.0).to_normalized(&converter),
+            NormalizedCoord::new(0.0)
+        );
+    }
 }

fontdrasil/src/types.rs

@@ -100,6 +100,11 @@ impl Axis {
     pub fn is_point(&self) -> bool {
         self.min == self.default && self.max == self.default
     }
+
+    /// Initialize a `CoordConverter` for the default normalization from this axis user space.
+    pub fn default_converter(&self) -> CoordConverter {
+        CoordConverter::default_normalization(self.min, self.default, self.max)
+    }
 }
 
 // OS/2 width class