diff --git a/write-fonts/src/graph/splitting.rs b/write-fonts/src/graph/splitting.rs index b1ea6970c..b623264f0 100644 --- a/write-fonts/src/graph/splitting.rs +++ b/write-fonts/src/graph/splitting.rs @@ -204,6 +204,7 @@ fn split_off_ppf1(graph: &mut Graph, subtable: ObjectId, start: u16, end: u16) - // based off of // fn split_pair_pos_format_2(graph: &mut Graph, subtable: ObjectId) -> Option> { + // the minimum size of a format 2 subtable const BASE_SIZE: usize = 8 * u16::RAW_BYTE_LEN; let data = &graph.objects[&subtable]; @@ -279,12 +280,10 @@ fn split_pair_pos_format_2(graph: &mut Graph, subtable: ObjectId) -> Option MAX_TABLE_SIZE { split_points.push(idx); @@ -630,12 +629,10 @@ fn size_of_value_record_children( .filter_map(|offset| (!offset.is_null()).then_some(*offset.offset())) .map(|_| { let obj = offsets[*next_offset_idx].object; - if seen.insert(obj) { - *next_offset_idx += 1; - graph.objects[&obj].bytes.len() - } else { - 0 - } + *next_offset_idx += 1; + seen.insert(obj) + .then(|| graph.objects[&obj].bytes.len()) + .unwrap_or(0) }) .sum() } @@ -668,7 +665,7 @@ mod tests { Class1Record, Class2Record, PairPos, PairSet, PairValueRecord, PositionLookup, ValueRecord, }, - layout::{CoverageTableBuilder, DeviceOrVariationIndex, VariationIndex}, + layout::{CoverageTableBuilder, Device, DeviceOrVariationIndex, VariationIndex}, }, FontWrite, TableWriter, }; @@ -915,6 +912,20 @@ mod tests { assert_eq!(count_num_ranges(&make_input(&[1, 2, 3, 5, 6, 7, 10])), 3); } + fn dummy_class_def( + n_classes: u16, + n_glyphs_per_class: u16, + first_gid: u16, + ) -> wlayout::ClassDef { + let n_glyphs = n_classes * n_glyphs_per_class; + (first_gid..first_gid + n_glyphs) + .map(|gid| { + let class = (gid - 1) / n_glyphs_per_class; + (GlyphId::new(gid), class) + }) + .collect() + } + fn make_pairpos2() -> PositionLookup { fn next_class2_rec(i: usize) -> Class2Record { // idk how better to cast bits directly @@ -945,24 +956,11 @@ mod tests { } } - fn make_class_def( - n_classes: u16, - n_glyphs_per_class: u16, - first_gid: u16, - ) -> wlayout::ClassDef { - let n_glyphs = n_classes * n_glyphs_per_class; - (first_gid..first_gid + n_glyphs) - .map(|gid| { - let class = (gid - 1) / n_glyphs_per_class; - (GlyphId::new(gid), class) - }) - .collect() - } const CLASS1_COUNT: u16 = 100; const CLASS2_COUNT: u16 = 100; - let class_def1 = make_class_def(CLASS1_COUNT, 4, 1); - let class_def2 = make_class_def(CLASS2_COUNT, 3, 1); + let class_def1 = dummy_class_def(CLASS1_COUNT, 4, 1); + let class_def2 = dummy_class_def(CLASS2_COUNT, 3, 1); assert_eq!(class_def1.class_count(), CLASS1_COUNT); assert_eq!(class_def2.class_count(), CLASS2_COUNT); @@ -1066,4 +1064,79 @@ mod tests { .sum(); assert_eq!(total_c2recs, expected_n_c2_recs); } + + #[test] + fn size_of_value_record_children_sanity() { + // let's have single class1class, and three class2 classes + // we want a duplicate varidx, a null varidx, and a device table? + + fn val_record_with_xadv(x_advance: i16) -> ValueRecord { + let format = ValueFormat::X_ADVANCE | ValueFormat::X_ADVANCE_DEVICE; + ValueRecord::new() + .with_explicit_value_format(format) + .with_x_advance(x_advance) + } + + // number of classes, number of glyphs per class, GID of first glyph + let class_def1 = dummy_class_def(1, 4, 1); + let class_def2 = dummy_class_def(3, 3, 1); + let coverage = class_def1.iter().map(|(gid, _)| gid).collect(); + let actual_device_table = Device::new(12, 15, &[118, 119, 127, 99]); + // sanity check the size of the device table, these are weird: + assert_eq!(crate::dump_table(&actual_device_table).unwrap().len(), 10); + let class1_records = vec![Class1Record::new(vec![ + Class2Record::new( + val_record_with_xadv(5), + val_record_with_xadv(6) + .with_x_advance_device(DeviceOrVariationIndex::variation_index(4, 20)), + ), + Class2Record::new( + val_record_with_xadv(7) + .with_x_advance_device(DeviceOrVariationIndex::Device(actual_device_table)), + // a duplicate table + val_record_with_xadv(8) + .with_x_advance_device(DeviceOrVariationIndex::variation_index(4, 20)), + ), + Class2Record::new( + val_record_with_xadv(9) + .with_x_advance_device(DeviceOrVariationIndex::variation_index(6, 9)), + val_record_with_xadv(10), + ), + ])]; + let ppf2 = PairPos::format_2(coverage, class_def1, class_def2, class1_records); + + // now we need to pretend we're in the split_pair_pos_format_2 fn + let mut graph = TableWriter::make_graph(&ppf2); + assert!(graph.pack_objects()); + let root_id = graph.root; + let ppf2_data = &graph.objects[&root_id]; + let ppf2 = ppf2_data.reparse::().unwrap(); + assert_eq!(ppf2.class1_records().len(), 1); + let c1rec = ppf2.class1_records().get(0).unwrap(); + let mut visited = HashSet::new(); + let mut next_device_offset = 3; + assert_eq!(c1rec.class2_records.len(), 3); + + // a little helper so we don't have to have this huge fn call in each assert + let mut children_size = |record: &rgpos::ValueRecord| -> usize { + size_of_value_record_children( + record, + &graph, + &ppf2_data.offsets, + &mut next_device_offset, + &mut visited, + ) + }; + + let c2rec1 = c1rec.class2_records().get(0).unwrap(); + assert_eq!(children_size(c2rec1.value_record1()), 0, "no subtables"); + assert_eq!(children_size(c2rec1.value_record2()), 6, "one new varidx"); + let c2rec2 = c1rec.class2_records().get(1).unwrap(); + assert_eq!(children_size(c2rec2.value_record1()), 10, "a device table"); + assert_eq!(children_size(c2rec2.value_record2()), 0, "duplicate table"); + let c2rec3 = c1rec.class2_records().get(2).unwrap(); + assert_eq!(children_size(c2rec3.value_record1()), 6, "new varidx table"); + assert_eq!(children_size(c2rec3.value_record2()), 0, "a null offset"); + assert_eq!(next_device_offset, 7, "we visited all offsets"); + } }