Match HarfBuzz's CFF2 coordinate and metric arithmetic

skera/src/cff/bounds.rs

@@ -210,3 +210,147 @@ pub(super) fn bounds(font: &FontRef, axes: &AxisPlan) -> Result<Vec<Option<[f64;
         })
         .collect()
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::cff::charstring::flatten;
+    use crate::cff::charstring::Command;
+    use read_fonts::ps::{cff::index::Index, charstring as cs};
+
+    #[test]
+    fn zero_extents_preserve_the_original_bearing() {
+        use crate::{Plan, SubsetFlags};
+        use read_fonts::{collections::IntSet, types::NameId};
+        use write_fonts::FontBuilder;
+
+        let font = FontRef::new(include_bytes!(
+            "../../test-data/fonts/cff2-nested-blends.otf"
+        ))
+        .unwrap();
+        let source = Source::new(font.cff2().unwrap().offset_data().as_bytes()).unwrap();
+        let plan = Plan::new(
+            &IntSet::all(),
+            &IntSet::empty(),
+            &font,
+            SubsetFlags::SUBSET_FLAGS_NOTDEF_OUTLINE,
+            &IntSet::empty(),
+            &IntSet::all(),
+            &IntSet::all(),
+            &IntSet::<NameId>::all(),
+            &IntSet::all(),
+        );
+        let mut chars: Vec<_> = (0..source.font.num_glyphs() as usize)
+            .map(|gid| source.font.charstrings().get(gid).unwrap().to_vec())
+            .collect();
+        // A drawing operator with zero extents, rather than an empty program.
+        chars[0] = vec![139, 139, 21, 139, 139, 5];
+        let cff = crate::cff::subset::assemble::<Cff2>(
+            &source,
+            &plan,
+            crate::cff::subset::Programs {
+                chars,
+                globals: vec![],
+                locals: vec![vec![]; source.fds.len()],
+            },
+        )
+        .unwrap();
+        let mut metrics = font
+            .data_for_tag(Tag::new(b"hmtx"))
+            .unwrap()
+            .as_bytes()
+            .to_vec();
+        metrics[2..4].copy_from_slice(&50i16.to_be_bytes());
+        let mut builder = FontBuilder::new();
+        builder
+            .add_raw(Tag::new(b"CFF2"), cff)
+            .add_raw(Tag::new(b"hmtx"), metrics);
+        let input = builder.copy_missing_tables(font).build();
+        let output = crate::instance_font(
+            &FontRef::new(&input).unwrap(),
+            &crate::parse_axis_limits("wght=0.5,wdth=0.5").unwrap(),
+        )
+        .unwrap();
+        assert_eq!(
+            FontRef::new(&output)
+                .unwrap()
+                .hmtx()
+                .unwrap()
+                .side_bearing(GlyphId::new(0)),
+            Some(50)
+        );
+    }
+
+    #[test]
+    fn control_bounds_cover_every_type2_path_operator() {
+        let scalars = Vec::<Vec<f64>>::new();
+        let empty = Index::Empty;
+        let context = (&[][..], &empty, &empty, &empty);
+        for (op, counts) in [
+            (5, vec![2, 6]),
+            (6, vec![1, 4]),
+            (7, vec![1, 4]),
+            (8, vec![6, 12]),
+            (24, vec![8, 14]),
+            (25, vec![8, 12]),
+            (26, vec![4, 5, 8, 9]),
+            (27, vec![4, 5, 8, 9]),
+            (30, vec![4, 5, 8, 9, 12, 13]),
+            (31, vec![4, 5, 8, 9, 12, 13]),
+            (0x122, vec![7]),
+            (0x123, vec![13]),
+            (0x124, vec![9]),
+            (0x125, vec![11]),
+        ] {
+            for n in counts {
+                for swap in [false, true] {
+                    let args: Vec<_> = (0..n)
+                        .map(|i| {
+                            if (i % 2 == 0) == swap {
+                                (i + 1) as f64
+                            } else {
+                                -((i + 1) as f64)
+                            }
+                        })
+                        .collect();
+                    let mut bounds = Bounds {
+                        scalars: &scalars,
+                        args: Vec::new(),
+                        point: [0.; 2],
+                        extent: None,
+                        open: false,
+                        remaining: 200_000,
+                        result: Ok(()),
+                    };
+                    bounds.path(op, &args).unwrap();
+                    let commands = [
+                        Command {
+                            op: 21,
+                            args: vec![Value::plain(0.); 2],
+                            mask: vec![],
+                        },
+                        Command {
+                            op,
+                            args: args.iter().copied().map(Value::plain).collect(),
+                            mask: vec![],
+                        },
+                    ];
+                    let bytes = flatten(&commands, None, true, 0).unwrap();
+                    let mut reference = cs::ControlBoundsSink::new();
+                    cs::evaluate(&context, None, &bytes, &mut reference).unwrap();
+                    let b = reference.bounding_box().unwrap();
+                    assert_eq!(
+                        bounds.extent,
+                        Some([
+                            b.x_min.to_f64(),
+                            b.y_min.to_f64(),
+                            b.x_max.to_f64(),
+                            b.y_max.to_f64()
+                        ]),
+                        "op={op} n={n} swap={swap}"
+                    );
+                }
+            }
+        }
+    }
+}

skera/src/cff/charstring.rs

@@ -40,6 +40,9 @@ pub(super) struct Blend {
 
 pub(super) trait BlendScalars {
     fn scalars(&self, ivs: usize) -> Result<&[f64]>;
+    fn finish_blend(&self, value: f64) -> f64 {
+        value.round()
+    }
 }
 impl BlendScalars for [Vec<f64>] {
     fn scalars(&self, ivs: usize) -> Result<&[f64]> {
@@ -140,7 +143,11 @@ impl Value {
             .base
             .resolve_with_rounding(scalars, remaining, round)?
             + delta_sum;
-        Ok(if round { result.round() } else { result })
+        Ok(if round {
+            scalars.finish_blend(result)
+        } else {
+            result
+        })
     }
     pub fn remap_ivs(&mut self, map: &BTreeMap<usize, usize>) -> Result<()> {
         if let Some(blend) = &mut self.blend {

skera/src/cff/mod.rs

@@ -80,13 +80,13 @@ pub(crate) fn instance(
     instance::instance(font, axes).map_err(|_| SubsetError::SubsetTableError(Tag::new(b"CFF2")))
 }
 
-pub(crate) fn downgrade(font: &FontRef) -> std::result::Result<Vec<u8>, SubsetError> {
-    downgrade::convert(font).map_err(|_| SubsetError::SubsetTableError(Tag::new(b"CFF2")))
-}
-
 pub(crate) fn instance_bounds(
     font: &FontRef,
     axes: &crate::instance::AxisPlan,
 ) -> std::result::Result<Vec<Option<[f64; 4]>>, SubsetError> {
     bounds::bounds(font, axes).map_err(|_| SubsetError::SubsetTableError(Tag::new(b"CFF2")))
 }
+
+pub(crate) fn downgrade(font: &FontRef) -> std::result::Result<Vec<u8>, SubsetError> {
+    downgrade::convert(font).map_err(|_| SubsetError::SubsetTableError(Tag::new(b"CFF2")))
+}

skera/src/instance/axes.rs

@@ -162,8 +162,7 @@ impl AxisPlan {
                 "font has no variation axes".into(),
             ));
         }
-        let metric_coords = axes.location(settings.iter().copied()).coords().to_vec();
-        let mut coords = metric_coords.clone();
+        let mut coords = vec![F2Dot14::ZERO; axes.len()];
         let avar = font.avar().ok();
         let coupled = !pinned.iter().all(|p| *p)
             && avar
@@ -210,6 +209,8 @@ impl AxisPlan {
                     .transpose()
                     .map_err(|_| SubsetError::SubsetTableError(Tag::new(b"avar")))?;
                 let input = coords.clone();
+                let scalars = super::scalars::VariationScalars::new(&store, &input)
+                    .ok_or(SubsetError::SubsetTableError(Tag::new(b"avar")))?;
                 for (i, coord) in coords.iter_mut().enumerate() {
                     let index = if let Some(map) = &index_map {
                         map.get(i as u32)
@@ -220,9 +221,9 @@ impl AxisPlan {
                             inner: i as u16,
                         }
                     };
-                    let delta = store
-                        .compute_delta(index, &input)
-                        .map_or(0, |d| d.to_f64().round() as i32);
+                    let delta = scalars
+                        .delta(&store, index)
+                        .map_or(0, |d| (d.clamp(-32768., 32768.) + 0.5).floor() as i32);
                     *coord = F2Dot14::from_bits(
                         (coord.to_bits() as i32 + delta.clamp(-32768, 32768)).clamp(-16384, 16384)
                             as i16,
@@ -231,6 +232,59 @@ impl AxisPlan {
             }
         }
         let user_pinned = pinned.clone();
+        // HarfBuzz uses its font's 16.16 normalization path for CFF bounds and
+        // advances, while outline/store instancing uses the 2.14 plan above.
+        let mut metric_coords: Vec<i32> = axes
+            .iter()
+            .map(|axis| {
+                let n = axis_to_normalized(
+                    user[axis.index()].1,
+                    axis.min_value() as f64,
+                    axis.default_value() as f64,
+                    axis.max_value() as f64,
+                ) as f32;
+                (n * 65536. + 0.5).floor() as i32
+            })
+            .collect();
+        if let Some(maps) = &maps {
+            for (coord, map) in metric_coords.iter_mut().zip(maps) {
+                let mapped = map_float(map, *coord as f64 / 65536., false) as f32;
+                *coord = (mapped * 65536. + 0.5).floor() as i32;
+            }
+        }
+        if let Some(avar) = &avar {
+            if let Some(store) = avar.var_store() {
+                let store = store.map_err(|_| SubsetError::SubsetTableError(Tag::new(b"avar")))?;
+                let intermediate: Vec<_> = metric_coords
+                    .iter()
+                    .map(|c| F2Dot14::from_bits(((c + 2) >> 2) as i16))
+                    .collect();
+                let scalars = super::scalars::VariationScalars::new(&store, &intermediate)
+                    .ok_or(SubsetError::SubsetTableError(Tag::new(b"avar")))?;
+                let map = avar
+                    .axis_index_map()
+                    .transpose()
+                    .map_err(|_| SubsetError::SubsetTableError(Tag::new(b"avar")))?;
+                for (i, coord) in metric_coords.iter_mut().enumerate() {
+                    let index = if let Some(map) = &map {
+                        map.get(i as u32)
+                            .map_err(|_| SubsetError::SubsetTableError(Tag::new(b"avar")))?
+                    } else {
+                        read_fonts::tables::variations::DeltaSetIndex {
+                            outer: 0,
+                            inner: i as u16,
+                        }
+                    };
+                    let delta =
+                        (scalars.delta(&store, index).unwrap_or(0.) * 4.).clamp(-131072., 131072.);
+                    *coord = (*coord + (delta + 0.5).floor() as i32).clamp(-65536, 65536);
+                }
+            }
+        }
+        let metric_coords = metric_coords
+            .into_iter()
+            .map(|c| F2Dot14::from_bits(((c + 2) >> 2) as i16))
+            .collect();
         if coupled {
             pinned.fill(false);
         }