Fix glyph keyed patch application to fonts with long loca format.

Adds test case too.

font-test-data/src/ift.rs

@@ -4,6 +4,7 @@
 //! <https://w3c.github.io/IFT/Overview.html>
 
 use std::collections::HashMap;
+use std::iter;
 
 use read_fonts::types::Tag;
 use read_fonts::{be_buffer, be_buffer_add, test_helpers::BeBuffer, types::Int24, types::Uint24};
@@ -591,6 +592,7 @@ pub fn noop_table_keyed_patch() -> BeBuffer {
 }
 
 pub fn test_font_for_patching_with_loca_mod<F>(
+    short_loca: bool,
     loca_mod: F,
     additional_tables: HashMap<Tag, &[u8]>,
 ) -> Vec<u8>
@@ -610,14 +612,21 @@ where
     font_builder.add_table(&maxp).unwrap();
 
     let head = Head {
-        index_to_loc_format: 0,
+        index_to_loc_format: if short_loca { 0 } else { 1 },
         ..Default::default()
     };
     font_builder.add_table(&head).unwrap();
 
     // ## glyf ##
     // glyphs are padded to even number of bytes since loca format will be short.
-    let glyf = BeBuffer::new()
+    let glyf = if !short_loca {
+        // Since we want a long loca artificially inflate glyf table to ensure long offsets are needed.
+        BeBuffer::new().extend(iter::repeat(0).take(70000))
+    } else {
+        BeBuffer::new()
+    };
+
+    let glyf = glyf
         .push_with_tag(1u8, "gid_0")
         .extend([2, 3, 4, 5u8, 0u8])
         .push_with_tag(6u8, "gid_1")
@@ -626,12 +635,13 @@ where
         .extend([10, 11, 12u8]);
 
     // ## loca ##
+    let gid_0 = glyf.offset_for("gid_0") as u32;
     let gid_1 = glyf.offset_for("gid_1") as u32;
     let gid_8 = glyf.offset_for("gid_8") as u32;
     let end = gid_8 + 4;
 
     let mut loca = vec![
-        0,     // gid 0
+        gid_0, // gid 0
         gid_1, // gid 1
         gid_8, // gid 2
         gid_8, // gid 3
@@ -662,6 +672,7 @@ where
 
 pub fn test_font_for_patching() -> Vec<u8> {
     test_font_for_patching_with_loca_mod(
+        true,
         |_| {},
         HashMap::from([(Tag::new(b"IFT "), vec![0, 0, 0, 0].as_slice())]),
     )

incremental-font-transfer/src/bin/ift_extend.rs

@@ -110,7 +110,7 @@ fn main() {
 
 fn parse_unicodes(args: Vec<String>, codepoints: &mut IntSet<u32>) -> Result<(), ParsingError> {
     for unicode_string in args {
-        if unicode_string == "" {
+        if unicode_string.is_empty() {
             continue;
         }
 
@@ -141,7 +141,7 @@ fn parse_design_space(args: Vec<String>) -> Result<DesignSpace, ParsingError> {
 
     let mut result = HashMap::<Tag, RangeSet<Fixed>>::default();
     for arg in args {
-        if arg == "" {
+        if arg.is_empty() {
             continue;
         }
 

incremental-font-transfer/src/font_patch.rs

@@ -233,6 +233,7 @@ mod tests {
         iftx_table.write_at("compat_id[0]", 2u32);
 
         let font = test_font_for_patching_with_loca_mod(
+            true,
             |_| {},
             HashMap::from([
                 (IFT_TAG, ift_table.as_slice()),
@@ -265,6 +266,7 @@ mod tests {
 
         let ift_table = codepoints_only_format2();
         let font = test_font_for_patching_with_loca_mod(
+            true,
             |_| {},
             HashMap::from([(IFT_TAG, ift_table.as_slice())]),
         );
@@ -291,6 +293,7 @@ mod tests {
 
         let ift_table = codepoints_only_format2();
         let font = test_font_for_patching_with_loca_mod(
+            true,
             |_| {},
             HashMap::from([(IFT_TAG, ift_table.as_slice())]),
         );
@@ -325,6 +328,7 @@ mod tests {
         ift_table.write_at("compat_id[3]", 9u32);
 
         let font = test_font_for_patching_with_loca_mod(
+            true,
             |_| {},
             HashMap::from([(IFT_TAG, ift_table.as_slice())]),
         );

incremental-font-transfer/src/glyph_keyed.rs

@@ -367,7 +367,7 @@ fn synthesize_glyf_and_loca<OffsetType: LocaOffset + TryFrom<usize>>(
         .map_err(|_| PatchingError::InternalError)?;
     loca_off.write_to(
         new_loca
-            .get_mut(new_loca.len() - 2..)
+            .get_mut(new_loca.len() - off_size..)
             .ok_or(PatchingError::InternalError)?,
     );
 
@@ -540,6 +540,7 @@ pub(crate) mod tests {
 
         // Application bit will be set in the patched font.
         let expected_font = test_font_for_patching_with_loca_mod(
+            true,
             |_| {},
             HashMap::from([(IFT_TAG, vec![0b0001_0000, 0, 0, 0].as_slice())]),
         );
@@ -609,6 +610,72 @@ pub(crate) mod tests {
         );
     }
 
+    #[test]
+    fn basic_glyph_keyed_with_long_loca() {
+        let patch =
+            assemble_glyph_keyed_patch(glyph_keyed_patch_header(), glyf_u16_glyph_patches());
+        let patch: &[u8] = &patch;
+        let patch = GlyphKeyedPatch::read(FontData::new(patch)).unwrap();
+
+        let font = test_font_for_patching_with_loca_mod(
+            false, // force long loca
+            |_| {},
+            HashMap::from([(Tag::new(b"IFT "), vec![0, 0, 0, 0].as_slice())]),
+        );
+        let font = FontRef::new(&font).unwrap();
+
+        let patch_info = patch_info(IFT_TAG, 28);
+        let patched = apply_glyph_keyed_patches(&[(&patch_info, patch)], &font).unwrap();
+        let patched = FontRef::new(&patched).unwrap();
+
+        let new_ift: &[u8] = patched.table_data(IFT_TAG).unwrap().as_bytes();
+        assert_eq!(&[0, 0, 0, 0b0001_0000], new_ift);
+
+        let new_glyf: &[u8] = patched.table_data(Tag::new(b"glyf")).unwrap().as_bytes();
+        assert_eq!(
+            &[
+                1, 2, 3, 4, 5, 0, // gid 0
+                6, 7, 8, 0, // gid 1
+                b'a', b'b', b'c', // gid2
+                b'd', b'e', b'f', b'g', // gid 7
+                b'h', b'i', b'j', b'k', b'l', // gid 8 + 9
+                b'm', b'n', // gid 13
+            ],
+            new_glyf
+        );
+
+        let new_loca = patched.loca(None).unwrap();
+        let indices: Vec<u32> = (0..=15).map(|gid| new_loca.get_raw(gid).unwrap()).collect();
+
+        assert_eq!(
+            vec![
+                0,  // gid 0
+                6,  // gid 1
+                10, // gid 2
+                13, // gid 3
+                13, // gid 4
+                13, // gid 5
+                13, // gid 6
+                13, // gid 7
+                17, // gid 8
+                17, // gid 9
+                22, // gid 10
+                22, // gid 11
+                22, // gid 12
+                22, // gid 13
+                24, // gid 14
+                24, // end
+            ],
+            indices
+        );
+
+        check_tables_equal(
+            &font,
+            &patched,
+            [IFT_TAG, Tag::new(b"glyf"), Tag::new(b"loca")].into(),
+        );
+    }
+
     #[test]
     fn multiple_glyph_keyed() {
         let patch =
@@ -624,6 +691,7 @@ pub(crate) mod tests {
         let patch_info_2 = patch_info(IFTX_TAG, 28);
 
         let font = test_font_for_patching_with_loca_mod(
+            true,
             |_| {},
             HashMap::from([(IFTX_TAG, vec![0, 0, 0, 0].as_slice())]),
         );
@@ -912,6 +980,7 @@ pub(crate) mod tests {
 
         // unorder offsets related to a glyph not being replaced
         let font = test_font_for_patching_with_loca_mod(
+            true,
             |loca| {
                 let loca_gid_1 = loca[1];
                 let loca_gid_2 = loca[2];

incremental-font-transfer/src/patch_group.rs

@@ -1306,6 +1306,7 @@ mod tests {
         iftx_builder.write_at("id_delta", Int24::new(1));
 
         let font = test_font_for_patching_with_loca_mod(
+            true,
             |_| {},
             HashMap::from([
                 (IFT_TAG, ift_builder.as_slice()),
@@ -1366,6 +1367,7 @@ mod tests {
         iftx_builder.write_at("id_delta", Int24::new(1));
 
         let font = test_font_for_patching_with_loca_mod(
+            true,
             |_| {},
             HashMap::from([
                 (IFT_TAG, ift_builder.as_slice()),