From 6eff17a15ca0caaea27eb130fa959d529d58625b Mon Sep 17 00:00:00 2001 From: Garret Rieger Date: Fri, 10 Jan 2025 23:33:36 +0000 Subject: [PATCH] Fix glyph keyed patch application to fonts with long loca format. Adds test case too. --- font-test-data/src/ift.rs | 17 ++++- .../src/bin/ift_extend.rs | 4 +- incremental-font-transfer/src/font_patch.rs | 4 ++ incremental-font-transfer/src/glyph_keyed.rs | 71 ++++++++++++++++++- incremental-font-transfer/src/patch_group.rs | 2 + 5 files changed, 92 insertions(+), 6 deletions(-) diff --git a/font-test-data/src/ift.rs b/font-test-data/src/ift.rs index 6d676810d..d1963f30d 100644 --- a/font-test-data/src/ift.rs +++ b/font-test-data/src/ift.rs @@ -4,6 +4,7 @@ //! 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( + short_loca: bool, loca_mod: F, additional_tables: HashMap, ) -> Vec @@ -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 { test_font_for_patching_with_loca_mod( + true, |_| {}, HashMap::from([(Tag::new(b"IFT "), vec![0, 0, 0, 0].as_slice())]), ) diff --git a/incremental-font-transfer/src/bin/ift_extend.rs b/incremental-font-transfer/src/bin/ift_extend.rs index b0828d8da..38d1c4e05 100644 --- a/incremental-font-transfer/src/bin/ift_extend.rs +++ b/incremental-font-transfer/src/bin/ift_extend.rs @@ -110,7 +110,7 @@ fn main() { fn parse_unicodes(args: Vec, codepoints: &mut IntSet) -> 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) -> Result { let mut result = HashMap::>::default(); for arg in args { - if arg == "" { + if arg.is_empty() { continue; } diff --git a/incremental-font-transfer/src/font_patch.rs b/incremental-font-transfer/src/font_patch.rs index 4470232f8..76f869db9 100644 --- a/incremental-font-transfer/src/font_patch.rs +++ b/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())]), ); diff --git a/incremental-font-transfer/src/glyph_keyed.rs b/incremental-font-transfer/src/glyph_keyed.rs index 83c109e9f..95b40b4a8 100644 --- a/incremental-font-transfer/src/glyph_keyed.rs +++ b/incremental-font-transfer/src/glyph_keyed.rs @@ -367,7 +367,7 @@ fn synthesize_glyf_and_loca>( .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 = (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]; diff --git a/incremental-font-transfer/src/patch_group.rs b/incremental-font-transfer/src/patch_group.rs index d65c5e333..b09260e3b 100644 --- a/incremental-font-transfer/src/patch_group.rs +++ b/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()),