diff --git a/skera/src/gpos/pair_pos.rs b/skera/src/gpos/pair_pos.rs index 2932065e0..5d14e4e34 100644 --- a/skera/src/gpos/pair_pos.rs +++ b/skera/src/gpos/pair_pos.rs @@ -1,11 +1,8 @@ //! impl subset() for PairPos subtable use crate::{ - gpos::value_record::compute_effective_format, - layout::{ - intersected_coverage_indices, intersected_glyphs_and_indices, map_gsub_glyph, - ClassDefSubsetStruct, - }, + gpos::value_record::{compute_effective_format, compute_record_len}, + layout::{map_gsub_glyph, ClassDefSubsetStruct}, offset::{SerializeSerialize, SerializeSubset}, offset_array::SubsetOffsetArray, serialize::{SerializeErrorFlags, SerializeResultEmpty, Serializer}, @@ -17,13 +14,11 @@ use write_fonts::{ read::{ collections::IntSet, tables::{ - gpos::{ - PairPos, PairPosFormat1, PairPosFormat2, PairSet, PairValueRecord, ValueFormat, - }, + gpos::{PairPos, PairPosFormat1, PairPosFormat2, PairSet, ValueFormat, ValueRecord}, layout::CoverageTable, }, types::GlyphId, - FontRef, ReadError, TableProvider, + ArrayOfOffsets, FontData, FontRef, ReadError, TableProvider, }, types::Offset16, }; @@ -45,48 +40,117 @@ impl<'a> SubsetTable<'a> for PairPos<'_> { } } +pub(crate) struct PairSetInfo<'a> { + coverage: &'a CoverageTable<'a>, + pair_sets: &'a ArrayOfOffsets<'a, PairSet<'a>>, + pair_set_count: u16, + value_format1: ValueFormat, + value_format2: ValueFormat, + record1_size: usize, + pair_record_size: usize, + new_format1: ValueFormat, + new_format2: ValueFormat, +} + +fn compute_pair_set_effective_formats( + pair_set: &PairSet, + glyph_set: &IntSet, + pair_set_info: &mut PairSetInfo, + strip_hints: bool, + strip_empty: bool, +) -> Result<(), ReadError> { + let (value_format1, value_format2, record1_size, pair_record_size, new_format1, new_format2) = ( + pair_set_info.value_format1, + pair_set_info.value_format2, + pair_set_info.record1_size, + pair_set_info.pair_record_size, + &mut pair_set_info.new_format1, + &mut pair_set_info.new_format2, + ); + for i in 0..pair_set.pair_value_count() as usize { + let offset = 2 + i * pair_record_size; + let font_data = pair_set.offset_data(); + let second_glyph = font_data.read_at::(offset)?; + if !glyph_set.contains(GlyphId::from(second_glyph)) { + continue; + } + + let value_record1 = ValueRecord::new(font_data, offset + 2, value_format1); + *new_format1 |= compute_effective_format(&value_record1, strip_hints, strip_empty)?; + + let value_record2 = ValueRecord::new(font_data, offset + 2 + record1_size, value_format2); + *new_format2 |= compute_effective_format(&value_record2, strip_hints, strip_empty)?; + } + Ok(()) +} + fn compute_effective_pair_formats_1( - pair_pos: &PairPosFormat1, glyph_set: &IntSet, + pair_set_info: &mut PairSetInfo, strip_hints: bool, strip_empty: bool, -) -> Result<(ValueFormat, ValueFormat), ReadError> { - let mut new_format1 = ValueFormat::empty(); - let mut new_format2 = ValueFormat::empty(); - - let orig_format1 = pair_pos.value_format1(); - let orig_format2 = pair_pos.value_format2(); - - let coverage = pair_pos.coverage()?; - let pair_sets = pair_pos.pair_sets(); - let partset_idxes = intersected_coverage_indices(&coverage, glyph_set); - for i in partset_idxes.iter() { - let pair_set = match pair_sets.get(i as usize) { - Err(ReadError::NullOffset) => continue, - other => other, - }?; - for pair_value_rec in pair_set.pair_value_records().iter() { - let pair_value_rec = pair_value_rec?; - let second_glyph = pair_value_rec.second_glyph(); - if !glyph_set.contains(GlyphId::from(second_glyph)) { - continue; +) -> Result<(), ReadError> { + let (coverage, pair_sets, pair_set_count, value_format1, value_format2) = ( + pair_set_info.coverage, + pair_set_info.pair_sets, + pair_set_info.pair_set_count, + pair_set_info.value_format1, + pair_set_info.value_format2, + ); + let bit_storage = 16 - pair_set_count.leading_zeros() as u64; + + if pair_set_count as u64 > glyph_set.len() * bit_storage { + for g in glyph_set.iter() { + if let Some(idx) = coverage.get(g) { + let pair_set = match pair_sets.get(idx as usize) { + Err(ReadError::NullOffset) => continue, + other => other, + }?; + + compute_pair_set_effective_formats( + &pair_set, + glyph_set, + pair_set_info, + strip_hints, + strip_empty, + )?; + if pair_set_info.new_format1 == value_format1 + && pair_set_info.new_format2 == value_format2 + { + break; + } } - - new_format1 |= - compute_effective_format(pair_value_rec.value_record1(), strip_hints, strip_empty); - new_format2 |= - compute_effective_format(pair_value_rec.value_record2(), strip_hints, strip_empty); } - if new_format1 == orig_format1 && new_format2 == orig_format2 { - break; + } else { + for idx in coverage + .iter() + .enumerate() + .filter_map(|(i, g)| glyph_set.contains(GlyphId::from(g)).then_some(i)) + { + let pair_set = match pair_sets.get(idx) { + Err(ReadError::NullOffset) => continue, + other => other, + }?; + compute_pair_set_effective_formats( + &pair_set, + glyph_set, + pair_set_info, + strip_hints, + strip_empty, + )?; + if pair_set_info.new_format1 == value_format1 + && pair_set_info.new_format2 == value_format2 + { + break; + } } } - Ok((new_format1, new_format2)) + Ok(()) } -impl SubsetTable<'_> for PairSet<'_> { - type ArgsForSubset = (ValueFormat, ValueFormat); +impl<'a> SubsetTable<'a> for PairSet<'_> { + type ArgsForSubset = &'a PairSetInfo<'a>; type Output = (); fn subset( &self, @@ -99,17 +163,88 @@ impl SubsetTable<'_> for PairSet<'_> { let mut count = 0_u16; let glyph_map = &plan.glyph_map_gsub; - let (new_format1, new_format2) = args; + let ( + value_format1, + value_format2, + new_format1, + new_format2, + record1_size, + pair_record_size, + ) = ( + args.value_format1, + args.value_format2, + args.new_format1, + args.new_format2, + args.record1_size, + args.pair_record_size, + ); + + let pair_value_count = self.pair_value_count(); + let bit_storage = 16 - pair_value_count.leading_zeros() as u16; + let font_data = self.offset_data(); + if pair_value_count as u64 > plan.glyphset_gsub.len() * bit_storage as u64 { + for g in plan.glyphset_gsub.iter() { + let mut hi = pair_value_count as usize; + let mut lo = 0; + while lo < hi { + // This recommends using usize::midpoint which expands to u128. + // We definitely do not want to do that here since the input values + // are 16-bit. + #[allow(clippy::manual_midpoint)] + let mid = (lo + hi) / 2; + let pair_record_offset = 2 + mid * pair_record_size; + let glyph_id = GlyphId::from( + font_data + .read_at::(pair_record_offset) + .map_err(|_| SerializeErrorFlags::SERIALIZE_ERROR_READ_ERROR)?, + ); + if glyph_id < g { + lo = mid + 1; + } else if glyph_id > g { + hi = mid; + } else { + let new_gid = map_gsub_glyph(glyph_map, glyph_id) + .ok_or_else(|| s.set_err(SerializeErrorFlags::SERIALIZE_ERROR_OTHER))?; + s.embed(new_gid.to_u32() as u16)?; + + let offset = pair_record_offset + 2; + let value_record1 = ValueRecord::new(font_data, offset, value_format1); + value_record1.subset(plan, s, new_format1)?; + + let value_record2 = + ValueRecord::new(font_data, offset + record1_size, value_format2); + value_record2.subset(plan, s, new_format2)?; + + count += 1; + break; + } + } + } + } else { + for i in 0..pair_value_count as usize { + let pair_record_offset = 2 + i * pair_record_size; + let glyph_id = GlyphId::from( + font_data + .read_at::(pair_record_offset) + .map_err(|_| SerializeErrorFlags::SERIALIZE_ERROR_READ_ERROR)?, + ); + + let Some(new_gid) = map_gsub_glyph(glyph_map, glyph_id) else { + continue; + }; - for pairvalue_rec in self.pair_value_records().iter() { - let pairvalue_rec = pairvalue_rec - .map_err(|_| s.set_err(SerializeErrorFlags::SERIALIZE_ERROR_READ_ERROR))?; - let Some(gid) = map_gsub_glyph(glyph_map, GlyphId::from(pairvalue_rec.second_glyph())) - else { - continue; - }; - pairvalue_rec.subset(plan, s, (&gid, new_format1, new_format2))?; - count += 1; + s.embed(new_gid.to_u32() as u16)?; + + let offset = pair_record_offset + 2; + let value_record1 = ValueRecord::new(font_data, offset, value_format1); + value_record1.subset(plan, s, new_format1)?; + + let value_record2 = + ValueRecord::new(font_data, offset + record1_size, value_format2); + value_record2.subset(plan, s, new_format2)?; + + count += 1; + } } if count == 0 { @@ -120,25 +255,6 @@ impl SubsetTable<'_> for PairSet<'_> { } } -impl<'a> SubsetTable<'a> for PairValueRecord<'_> { - type ArgsForSubset = (&'a GlyphId, ValueFormat, ValueFormat); - type Output = (); - fn subset( - &self, - plan: &Plan, - s: &mut Serializer, - args: Self::ArgsForSubset, - ) -> Result<(), SerializeErrorFlags> { - let (new_gid, new_format1, new_format2) = args; - // second glyph - s.embed(new_gid.to_u32() as u16)?; - - //value records - self.value_record1().subset(plan, s, new_format1)?; - self.value_record2().subset(plan, s, new_format2) - } -} - impl<'a> SubsetTable<'a> for PairPosFormat1<'_> { type ArgsForSubset = (&'a SubsetState, &'a FontRef<'a>); type Output = (); @@ -157,11 +273,33 @@ impl<'a> SubsetTable<'a> for PairPosFormat1<'_> { // format s.embed(self.pos_format())?; + let coverage = self + .coverage() + .map_err(|_| s.set_err(SerializeErrorFlags::SERIALIZE_ERROR_READ_ERROR))?; + let pair_sets = self.pair_sets(); + let pair_set_count = self.pair_set_count(); + // coverage offset let cov_offset_pos = s.embed(0_u16)?; // value_formats - let (new_format1, new_format2) = if plan + let value_format1 = self.value_format1(); + let value_format2 = self.value_format2(); + let record1_size = 2 * compute_record_len(value_format1); + let pair_record_size = 2 + record1_size + 2 * compute_record_len(value_format2); + let mut pair_set_info = PairSetInfo { + coverage: &coverage, + pair_sets: &pair_sets, + pair_set_count, + value_format1, + value_format2, + record1_size, + pair_record_size, + new_format1: ValueFormat::empty(), + new_format2: ValueFormat::empty(), + }; + + if plan .subset_flags .contains(SubsetFlags::SUBSET_FLAGS_NO_HINTING) { @@ -172,71 +310,120 @@ impl<'a> SubsetTable<'a> for PairPosFormat1<'_> { true }; - compute_effective_pair_formats_1(self, glyph_set, strip_hints, true) - .map_err(|_| s.set_err(SerializeErrorFlags::SERIALIZE_ERROR_READ_ERROR))? + compute_effective_pair_formats_1(glyph_set, &mut pair_set_info, strip_hints, true) + .map_err(|_| s.set_err(SerializeErrorFlags::SERIALIZE_ERROR_READ_ERROR))?; } else { - (self.value_format1(), self.value_format2()) + pair_set_info.new_format1 = value_format1; + pair_set_info.new_format2 = value_format2; }; - s.embed(new_format1)?; - s.embed(new_format2)?; + + s.embed(pair_set_info.new_format1)?; + s.embed(pair_set_info.new_format2)?; // pairset count let pairset_count_pos = s.embed(0_u16)?; let mut pairset_count = 0_u16; - let coverage = self - .coverage() - .map_err(|_| s.set_err(SerializeErrorFlags::SERIALIZE_ERROR_READ_ERROR))?; - let pair_sets = self.pair_sets(); + let mut retained_glyphs = + Vec::with_capacity((pair_set_count as usize).min(glyph_set.len() as usize)); - let (glyphs, pairset_idxes) = - intersected_glyphs_and_indices(&coverage, glyph_set, glyph_map); - if glyphs.is_empty() { - return Err(SerializeErrorFlags::SERIALIZE_ERROR_EMPTY); - } + let bit_storage = 16 - pair_set_count.leading_zeros() as u64; + if pair_set_count as u64 > glyph_set.len() * bit_storage { + for g in glyph_set.iter() { + let Some(pair_set_idx) = coverage.get(g) else { + continue; + }; - let mut retained_glyphs = Vec::with_capacity(glyphs.len()); - for (i, g) in pairset_idxes.iter().zip(glyphs) { - if !pair_sets - .subset_offset(i as usize, s, plan, (new_format1, new_format2)) - .is_empty()? - { - pairset_count += 1; - retained_glyphs.push(g); + if !pair_sets + .subset_offset(pair_set_idx as usize, s, plan, &pair_set_info) + .is_empty()? + { + pairset_count += 1; + let new_g = map_gsub_glyph(glyph_map, g) + .ok_or_else(|| s.set_err(SerializeErrorFlags::SERIALIZE_ERROR_OTHER))?; + retained_glyphs.push(new_g); + } + } + } else { + for (i, g) in coverage.iter().enumerate().filter_map(|(i, g)| { + map_gsub_glyph(glyph_map, GlyphId::from(g)).map(|new_g| (i, new_g)) + }) { + if !pair_sets + .subset_offset(i, s, plan, &pair_set_info) + .is_empty()? + { + pairset_count += 1; + retained_glyphs.push(g); + } } } + if retained_glyphs.is_empty() { + return Err(SerializeErrorFlags::SERIALIZE_ERROR_EMPTY); + } s.copy_assign(pairset_count_pos, pairset_count); Offset16::serialize_serialize::(s, &retained_glyphs, cov_offset_pos) } } +struct PairPosFormat2Info<'a> { + font_data: FontData<'a>, + value_format1: ValueFormat, + value_format2: ValueFormat, + class1_count: u16, + class2_count: usize, + records_offset: usize, + record1_size: usize, + record_size: usize, + new_format1: ValueFormat, + new_format2: ValueFormat, +} + fn compute_effective_pair_formats_2( - pair_pos: &PairPosFormat2, - class1_idxes: &[u16], + pairpos2_info: &mut PairPosFormat2Info, + class1_map: &FnvHashMap, class2_idxes: &[u16], strip_hints: bool, strip_empty: bool, -) -> Result<(ValueFormat, ValueFormat), ReadError> { - let mut new_format1 = ValueFormat::empty(); - let mut new_format2 = ValueFormat::empty(); - - let orig_format1 = pair_pos.value_format1(); - let orig_format2 = pair_pos.value_format2(); - - let class_records = pair_pos.class_value_records(); - for i in class1_idxes { +) -> Result<(), ReadError> { + let ( + font_data, + value_format1, + value_format2, + class1_count, + class2_count, + records_offset, + record1_size, + record_size, + new_format1, + new_format2, + ) = ( + pairpos2_info.font_data, + pairpos2_info.value_format1, + pairpos2_info.value_format2, + pairpos2_info.class1_count, + pairpos2_info.class2_count, + pairpos2_info.records_offset, + pairpos2_info.record1_size, + pairpos2_info.record_size, + &mut pairpos2_info.new_format1, + &mut pairpos2_info.new_format2, + ); + + for i in (0..class1_count).filter(|i| class1_map.contains_key(i)) { for j in class2_idxes { - let [rec1, rec2] = class_records.get(*i, *j).ok_or(ReadError::OutOfBounds)?; + let offset = records_offset + (i as usize * class2_count + *j as usize) * record_size; + let record1 = ValueRecord::new(font_data, offset, value_format1); + let record2 = ValueRecord::new(font_data, offset + record1_size, value_format2); - new_format1 |= compute_effective_format(&rec1, strip_hints, strip_empty); - new_format2 |= compute_effective_format(&rec2, strip_hints, strip_empty); + *new_format1 |= compute_effective_format(&record1, strip_hints, strip_empty)?; + *new_format2 |= compute_effective_format(&record2, strip_hints, strip_empty)?; } - if new_format1 == orig_format1 && new_format2 == orig_format2 { + if *new_format1 == value_format1 && *new_format2 == value_format2 { break; } } - Ok((new_format1, new_format2)) + Ok(()) } impl<'a> SubsetTable<'a> for PairPosFormat2<'_> { @@ -258,15 +445,6 @@ impl<'a> SubsetTable<'a> for PairPosFormat2<'_> { return Err(s.set_err(SerializeErrorFlags::SERIALIZE_ERROR_READ_ERROR)); }; - let glyphs: Vec = coverage - .intersect_set(&plan.glyphset_gsub) - .iter() - .filter_map(|g| map_gsub_glyph(&plan.glyph_map_gsub, g)) - .collect(); - if glyphs.is_empty() { - return Err(SerializeErrorFlags::SERIALIZE_ERROR_EMPTY); - } - // format s.embed(self.pos_format())?; @@ -338,14 +516,32 @@ impl<'a> SubsetTable<'a> for PairPosFormat2<'_> { // value formats let (subset_state, font) = args; - let class1_idxes: Vec = (0..self.class1_count()) - .filter(|i| class1_map.contains_key(i)) - .collect(); let class2_idxes: Vec = (0..self.class2_count()) .filter(|i| class2_map.contains_key(i)) .collect(); - let (new_format1, new_format2) = if plan + let value_format1 = self.value_format1(); + let value_format2 = self.value_format2(); + let class2_count = self.class2_count() as usize; + let records_offset = self.class2_count_byte_range().end; + let record1_size = 2 * compute_record_len(value_format1); + let record_size = record1_size + 2 * compute_record_len(value_format2); + let font_data = self.offset_data(); + let class1_count = self.class1_count(); + let mut pairpos2_info = PairPosFormat2Info { + font_data, + value_format1, + value_format2, + class1_count, + class2_count, + records_offset, + record1_size, + record_size, + new_format1: ValueFormat::empty(), + new_format2: ValueFormat::empty(), + }; + + if plan .subset_flags .contains(SubsetFlags::SUBSET_FLAGS_NO_HINTING) { @@ -356,50 +552,106 @@ impl<'a> SubsetTable<'a> for PairPosFormat2<'_> { true }; - compute_effective_pair_formats_2(self, &class1_idxes, &class2_idxes, strip_hints, true) - .map_err(|_| s.set_err(SerializeErrorFlags::SERIALIZE_ERROR_READ_ERROR))? + compute_effective_pair_formats_2( + &mut pairpos2_info, + &class1_map, + &class2_idxes, + strip_hints, + true, + ) + .map_err(|_| s.set_err(SerializeErrorFlags::SERIALIZE_ERROR_READ_ERROR))?; } else { - (self.value_format1(), self.value_format2()) + pairpos2_info.new_format1 = value_format1; + pairpos2_info.new_format2 = value_format2; }; - s.copy_assign(value_format1_pos, new_format1); - s.copy_assign(value_format2_pos, new_format2); + s.copy_assign(value_format1_pos, pairpos2_info.new_format1); + s.copy_assign(value_format2_pos, pairpos2_info.new_format2); // serialize value records - let class_records = self.class_value_records(); - for i in class1_idxes { + for i in (0..self.class1_count()).filter(|i| class1_map.contains_key(i)) { for j in &class2_idxes { - let Some([rec1, rec2]) = class_records.get(i, *j) else { - return Err(s.set_err(SerializeErrorFlags::SERIALIZE_ERROR_READ_ERROR)); - }; - - rec1.subset(plan, s, new_format1)?; - rec2.subset(plan, s, new_format2)?; + let offset = + records_offset + (i as usize * class2_count + *j as usize) * record_size; + let record1 = ValueRecord::new(self.offset_data(), offset, value_format1); + let record2 = + ValueRecord::new(self.offset_data(), offset + record1_size, value_format2); + + record1.subset(plan, s, pairpos2_info.new_format1)?; + record2.subset(plan, s, pairpos2_info.new_format2)?; } } // this can be moved, put it at last so we have the same binary data with Harfbuzz subsetter - Offset16::serialize_serialize::(s, &glyphs, cov_offset_pos) + Offset16::serialize_subset(&coverage, s, plan, (), cov_offset_pos) } } -impl CollectVariationIndices for PairSet<'_> { - fn collect_variation_indices(&self, plan: &Plan, varidx_set: &mut IntSet) { +fn collect_pairset_variation_indices( + pair_set: &PairSet, + pair_set_info: &PairSetInfo, + plan: &Plan, + varidx_set: &mut IntSet, +) { + let (value_format1, value_format2, record1_size, pair_record_size) = ( + pair_set_info.value_format1, + pair_set_info.value_format2, + pair_set_info.record1_size, + pair_set_info.pair_record_size, + ); + + let pair_value_count = pair_set.pair_value_count(); + let bit_storage = 16 - pair_value_count.leading_zeros() as u16; + let font_data = pair_set.offset_data(); + if pair_value_count as u64 > plan.glyphset_gsub.len() * bit_storage as u64 { + for g in plan.glyphset_gsub.iter() { + let mut hi = pair_value_count as usize; + let mut lo = 0; + while lo < hi { + // This recommends using usize::midpoint which expands to u128. + // We definitely do not want to do that here since the input values + // are 16-bit. + #[allow(clippy::manual_midpoint)] + let mid = (lo + hi) / 2; + let pair_record_offset = 2 + mid * pair_record_size; + let Ok(glyph_id) = font_data.read_at::(pair_record_offset) else { + return; + }; + let glyph_id = GlyphId::from(glyph_id); + if glyph_id < g { + lo = mid + 1; + } else if glyph_id > g { + hi = mid; + } else { + let offset = pair_record_offset + 2; + let value_record1 = ValueRecord::new(font_data, offset, value_format1); + value_record1.collect_variation_indices(plan, varidx_set); + + let value_record2 = + ValueRecord::new(font_data, offset + record1_size, value_format2); + value_record2.collect_variation_indices(plan, varidx_set); + break; + } + } + } + } else { let glyph_set = &plan.glyphset_gsub; - for pairvalue_record in self.pair_value_records().iter() { - let Ok(pairvalue_record) = pairvalue_record else { + for i in 0..pair_value_count as usize { + let pair_record_offset = 2 + i * pair_record_size; + let Ok(glyph_id) = font_data.read_at::(pair_record_offset) else { return; }; - if !glyph_set.contains(GlyphId::from(pairvalue_record.second_glyph())) { + + if !glyph_set.contains(GlyphId::from(glyph_id)) { continue; } - pairvalue_record - .value_record1() - .collect_variation_indices(plan, varidx_set); - pairvalue_record - .value_record2() - .collect_variation_indices(plan, varidx_set); + let offset = pair_record_offset + 2; + let value_record1 = ValueRecord::new(font_data, offset, value_format1); + value_record1.collect_variation_indices(plan, varidx_set); + + let value_record2 = ValueRecord::new(font_data, offset + record1_size, value_format2); + value_record2.collect_variation_indices(plan, varidx_set); } } } @@ -430,10 +682,46 @@ impl CollectVariationIndices for PairPosFormat1<'_> { let glyph_set = &plan.glyphset_gsub; let pair_sets = self.pair_sets(); - let pairset_idxes = intersected_coverage_indices(&coverage, glyph_set); - for i in pairset_idxes.iter() { - if let Ok(pair_set) = pair_sets.get(i as usize) { - pair_set.collect_variation_indices(plan, varidx_set); + let pair_set_count = self.pair_set_count(); + + let record1_size = 2 * compute_record_len(value_format1); + let pair_record_size = 2 + record1_size + 2 * compute_record_len(value_format2); + let pair_set_info = PairSetInfo { + coverage: &coverage, + pair_sets: &pair_sets, + pair_set_count, + value_format1, + value_format2, + record1_size, + pair_record_size, + new_format1: ValueFormat::empty(), + new_format2: ValueFormat::empty(), + }; + + let bit_storage = 16 - pair_set_count.leading_zeros() as u64; + if pair_set_count as u64 > glyph_set.len() * bit_storage { + for g in glyph_set.iter() { + if let Some(idx) = coverage.get(g) { + let pair_set = match pair_sets.get(idx as usize) { + Ok(pair_set) => pair_set, + Err(ReadError::NullOffset) => continue, + Err(_) => return, + }; + collect_pairset_variation_indices(&pair_set, &pair_set_info, plan, varidx_set); + } + } + } else { + for idx in coverage + .iter() + .enumerate() + .filter_map(|(i, g)| glyph_set.contains(GlyphId::from(g)).then_some(i)) + { + let pair_set = match pair_sets.get(idx) { + Ok(pair_set) => pair_set, + Err(ReadError::NullOffset) => continue, + Err(_) => return, + }; + collect_pairset_variation_indices(&pair_set, &pair_set_info, plan, varidx_set); } } } @@ -477,14 +765,26 @@ impl CollectVariationIndices for PairPosFormat2<'_> { } class2_set.insert(0); - let class_records = self.class_value_records(); - for class1 in class1_set.iter() { - for class2 in class2_set.iter() { - let Some([rec1, rec2]) = class_records.get(class1, class2) else { - return; - }; - rec1.collect_variation_indices(plan, varidx_set); - rec2.collect_variation_indices(plan, varidx_set); + let class2_count = self.class2_count() as usize; + let records_offset = self.class2_count_byte_range().end; + let record1_size = 2 * compute_record_len(value_format1); + let record_size = record1_size + 2 * compute_record_len(value_format2); + let font_data = self.offset_data(); + + for i in class1_set.iter() { + for j in class2_set.iter() { + let offset = + records_offset + (i as usize * class2_count + j as usize) * record_size; + + if value_format1.intersects(ValueFormat::ANY_DEVICE_OR_VARIDX) { + let record1 = ValueRecord::new(font_data, offset, value_format1); + record1.collect_variation_indices(plan, varidx_set); + } + + if value_format2.intersects(ValueFormat::ANY_DEVICE_OR_VARIDX) { + let record2 = ValueRecord::new(font_data, offset + record1_size, value_format2); + record2.collect_variation_indices(plan, varidx_set); + } } } } @@ -555,6 +855,7 @@ mod test { let subset_state = SubsetState::default(); let mut plan = Plan { glyph_map_gsub: vec![crate::INVALID_GID; 6737], + font_num_glyphs: 6782, ..Default::default() }; diff --git a/skera/src/gpos/single_pos.rs b/skera/src/gpos/single_pos.rs index 28405e076..26e8e813d 100644 --- a/skera/src/gpos/single_pos.rs +++ b/skera/src/gpos/single_pos.rs @@ -2,8 +2,8 @@ use crate::fnv::FnvHashMap; use crate::{ - gpos::value_record::compute_effective_format, - layout::{intersected_coverage_indices, intersected_glyphs_and_indices, map_gsub_glyph}, + gpos::value_record::{compute_effective_format, compute_record_len}, + layout::{intersected_glyphs_and_indices, map_gsub_glyph}, offset::SerializeSerialize, serialize::{SerializeErrorFlags, Serializer}, CollectVariationIndices, Plan, Serialize, SubsetFlags, SubsetState, SubsetTable, @@ -16,7 +16,7 @@ use write_fonts::{ layout::CoverageTable, }, types::GlyphId, - FontRef, TableProvider, + FontData, FontRef, ReadError, TableProvider, }, types::Offset16, }; @@ -75,6 +75,7 @@ impl<'a> SubsetTable<'a> for SinglePosFormat1<'_> { true }; compute_effective_format(&value_record, strip_hints, true) + .map_err(|_| s.set_err(SerializeErrorFlags::SERIALIZE_ERROR_READ_ERROR))? } else { self.value_format() }; @@ -102,14 +103,29 @@ impl<'a> Serialize<'a> for SinglePosFormat1<'_> { } } -fn compute_new_value_format<'a>( +pub(crate) struct SinglePosInfo<'a> { + value_format: ValueFormat, + records_offset: usize, + record_size: usize, + font_data: FontData<'a>, + new_format: ValueFormat, +} + +fn compute_new_value_format( + singlepos_info: &mut SinglePosInfo, plan: &Plan, has_gdef_varstore: bool, font: &FontRef, - value_records: impl IntoIterator>, -) -> ValueFormat { + retained_rec_idxes: &IntSet, +) -> Result<(), ReadError> { // TODO: support instancing - let mut new_format = ValueFormat::empty(); + let (value_format, records_offset, record_size, font_data, new_format) = ( + singlepos_info.value_format, + singlepos_info.records_offset, + singlepos_info.record_size, + singlepos_info.font_data, + &mut singlepos_info.new_format, + ); if plan .subset_flags .contains(SubsetFlags::SUBSET_FLAGS_NO_HINTING) @@ -121,14 +137,16 @@ fn compute_new_value_format<'a>( true }; - for record in value_records { - new_format |= compute_effective_format(&record, strip_hints, true); + for i in retained_rec_idxes.iter() { + let offset = records_offset + i as usize * record_size; + let value_record = ValueRecord::new(font_data, offset, value_format); + *new_format |= compute_effective_format(&value_record, strip_hints, true)?; } - } else if let Some(rec) = value_records.into_iter().next() { - new_format = rec.format(); + } else { + *new_format = value_format; } - new_format + Ok(()) } impl<'a> SubsetTable<'a> for SinglePosFormat2<'_> { @@ -155,43 +173,68 @@ impl<'a> SubsetTable<'a> for SinglePosFormat2<'_> { } let (state, font) = args; - let value_records = self.value_records(); - let it = retained_rec_idxes - .iter() - .filter_map(|i| value_records.get(i as usize).ok()); - let new_format = compute_new_value_format(plan, state.has_gdef_varstore, font, it); - - let Ok(first_retained_rec) = - value_records.get(retained_rec_idxes.first().unwrap() as usize) - else { - return Err(s.set_err(SerializeErrorFlags::SERIALIZE_ERROR_READ_ERROR)); + let value_format = self.value_format(); + let records_offset = self.value_count_byte_range().end; + let record_size = 2 * compute_record_len(value_format); + let font_data = self.offset_data(); + let mut singlepos_info = SinglePosInfo { + value_format, + records_offset, + record_size, + font_data, + new_format: ValueFormat::empty(), }; - let mut table_format = 1; - for i in retained_rec_idxes.iter().skip(1) { - let Ok(rec) = value_records.get(i as usize) else { - return Err(s.set_err(SerializeErrorFlags::SERIALIZE_ERROR_READ_ERROR)); - }; + compute_new_value_format( + &mut singlepos_info, + plan, + state.has_gdef_varstore, + font, + &retained_rec_idxes, + ) + .map_err(|_| s.set_err(SerializeErrorFlags::SERIALIZE_ERROR_READ_ERROR))?; + + let Some(first_rec_idx) = retained_rec_idxes.first() else { + return Err(s.set_err(SerializeErrorFlags::SERIALIZE_ERROR_OTHER)); + }; + let first_retained_rec = ValueRecord::new( + font_data, + records_offset + first_rec_idx as usize * record_size, + value_format, + ); - if rec != first_retained_rec { - table_format = 2; - break; - } - } + let table_format = if retained_rec_idxes + .iter() + .skip(1) + .map(|i| { + ValueRecord::new( + font_data, + records_offset + i as usize * record_size, + value_format, + ) + }) + .all(|rec| rec == first_retained_rec) + { + 1 + } else { + 2 + }; if table_format == 1 { - SinglePosFormat1::serialize(s, (&retained_glyphs, first_retained_rec, new_format, plan)) - } else { - SinglePosFormat2::serialize( + SinglePosFormat1::serialize( s, ( &retained_glyphs, - new_format, - self, - &retained_rec_idxes, + first_retained_rec, + singlepos_info.new_format, plan, ), ) + } else { + SinglePosFormat2::serialize( + s, + (&retained_glyphs, &singlepos_info, &retained_rec_idxes, plan), + ) } } } @@ -199,8 +242,7 @@ impl<'a> SubsetTable<'a> for SinglePosFormat2<'_> { impl<'a> Serialize<'a> for SinglePosFormat2<'_> { type Args = ( &'a [GlyphId], - ValueFormat, - &'a SinglePosFormat2<'a>, + &'a SinglePosInfo<'a>, &'a IntSet, &'a Plan, ); @@ -211,7 +253,14 @@ impl<'a> Serialize<'a> for SinglePosFormat2<'_> { // coverage offset let cov_offset_pos = s.embed(0_u16)?; - let (glyphs, value_format, table, retained_rec_idxes, plan) = args; + let (glyphs, singlepos_info, retained_rec_idxes, plan) = args; + let (value_format, records_offset, record_size, font_data, new_format) = ( + singlepos_info.value_format, + singlepos_info.records_offset, + singlepos_info.record_size, + singlepos_info.font_data, + singlepos_info.new_format, + ); //value format s.embed(value_format)?; @@ -220,11 +269,9 @@ impl<'a> Serialize<'a> for SinglePosFormat2<'_> { s.embed(value_count as u16)?; for i in retained_rec_idxes.iter() { - let value_record = table - .value_records() - .get(i as usize) - .map_err(|_| s.set_err(SerializeErrorFlags::SERIALIZE_ERROR_READ_ERROR))?; - value_record.subset(plan, s, value_format)?; + let offset = records_offset + i as usize * record_size; + let value_record = ValueRecord::new(font_data, offset, value_format); + value_record.subset(plan, s, new_format)?; } Offset16::serialize_serialize::(s, glyphs, cov_offset_pos) @@ -255,10 +302,8 @@ impl CollectVariationIndices for SinglePosFormat1<'_> { impl CollectVariationIndices for SinglePosFormat2<'_> { fn collect_variation_indices(&self, plan: &Plan, varidx_set: &mut IntSet) { - if !self - .value_format() - .intersects(ValueFormat::ANY_DEVICE_OR_VARIDX) - { + let value_format = self.value_format(); + if value_format.intersects(ValueFormat::ANY_DEVICE_OR_VARIDX) { return; } @@ -266,13 +311,28 @@ impl CollectVariationIndices for SinglePosFormat2<'_> { return; }; let glyph_set = &plan.glyphset_gsub; - let value_record_idxes = intersected_coverage_indices(&coverage, glyph_set); - let value_records = self.value_records(); - for i in value_record_idxes.iter() { - let Ok(value_record) = value_records.get(i as usize) else { - return; - }; - value_record.collect_variation_indices(plan, varidx_set); + let value_count = self.value_count(); + let record_size = compute_record_len(value_format); + let records_offset = self.value_count_byte_range().end; + let font_data = self.offset_data(); + + let bit_storage = 16 - value_count.leading_zeros() as u64; + if value_count as u64 > glyph_set.len() * bit_storage { + for idx in glyph_set.iter().filter_map(|g| coverage.get(g)) { + let offset = records_offset + idx as usize * record_size; + let value_record = ValueRecord::new(font_data, offset, value_format); + value_record.collect_variation_indices(plan, varidx_set); + } + } else { + for i in coverage + .iter() + .enumerate() + .filter_map(|(idx, g)| glyph_set.contains(GlyphId::from(g)).then_some(idx)) + { + let offset = records_offset + i * record_size; + let value_record = ValueRecord::new(font_data, offset, value_format); + value_record.collect_variation_indices(plan, varidx_set); + } } } } diff --git a/skera/src/gpos/value_record.rs b/skera/src/gpos/value_record.rs index fccdf16c7..4b37f5cb4 100644 --- a/skera/src/gpos/value_record.rs +++ b/skera/src/gpos/value_record.rs @@ -8,11 +8,23 @@ use crate::{ use write_fonts::{ read::{ collections::IntSet, - tables::gpos::{ValueFormat, ValueRecord}, + tables::{ + gpos::{ValueFormat, ValueRecord}, + layout::DeviceOrVariationIndex, + }, + ReadError, ResolveOffset, }, types::Offset16, }; +/// The device fields of a value record, in on-disk order. +const NON_DEVICE_FIELDS: [ValueFormat; 4] = [ + ValueFormat::X_PLACEMENT, + ValueFormat::Y_PLACEMENT, + ValueFormat::X_ADVANCE, + ValueFormat::Y_ADVANCE, +]; + /// The device fields of a value record, in on-disk order. const DEVICE_FIELDS: [ValueFormat; 4] = [ ValueFormat::X_PLACEMENT_DEVICE, @@ -21,11 +33,25 @@ const DEVICE_FIELDS: [ValueFormat; 4] = [ ValueFormat::Y_ADVANCE_DEVICE, ]; +// Returns the number of 1 bits in a u8 +#[inline] +fn popcount8(v: u8) -> u8 { + const POPCOUNT4: [u8; 16] = [0, 1, 1, 2, 1, 2, 2, 3, 1, 2, 2, 3, 2, 3, 3, 4]; + POPCOUNT4[(v & 0xF) as usize] + POPCOUNT4[(v >> 4) as usize] +} + +// use faster popcount8 that only processes the lowest 8 bits +// Harfbuzz ref: +pub(super) fn compute_record_len(value_format: ValueFormat) -> usize { + let v = value_format.bits() as u8; + popcount8(v) as usize +} + pub(crate) fn compute_effective_format( value_record: &ValueRecord, strip_hints: bool, strip_empty: bool, -) -> ValueFormat { +) -> Result { let mut effective = value_record.format(); if strip_hints { @@ -33,35 +59,25 @@ pub(crate) fn compute_effective_format( } if !strip_empty { - return effective; + return Ok(effective); } - // A field contributes nothing when its sixteen bits are zero, whether it - // holds a value or an offset to a device table. The two cases are treated - // alike: `strip_empty` governs both. - for (field, value) in [ - (ValueFormat::X_PLACEMENT, value_record.x_placement()), - (ValueFormat::Y_PLACEMENT, value_record.y_placement()), - (ValueFormat::X_ADVANCE, value_record.x_advance()), - (ValueFormat::Y_ADVANCE, value_record.y_advance()), - ] { - if value == Some(0) { - effective -= field; + let mut offset = value_record.offset(); + let value_format = value_record.format(); + let font_data = value_record.offset_data(); + for &field in NON_DEVICE_FIELDS.iter().chain(DEVICE_FIELDS.iter()) { + if !value_format.contains(field) { + continue; } - } - if effective.intersects(ValueFormat::ANY_DEVICE_OR_VARIDX) { - for field in DEVICE_FIELDS { - if value_record - .device_offset(field) - .is_some_and(|offset| offset.is_null()) - { - effective -= field; - } + let value = font_data.read_at::(offset)?; + if value == 0 { + effective -= field; } + offset += 2; } - effective + Ok(effective) } impl<'a> SubsetTable<'a> for ValueRecord<'_> { @@ -78,40 +94,45 @@ impl<'a> SubsetTable<'a> for ValueRecord<'_> { return Ok(()); } - if new_format.contains(ValueFormat::X_PLACEMENT) { - s.embed(self.x_placement().unwrap_or(0))?; - } - - if new_format.contains(ValueFormat::Y_PLACEMENT) { - s.embed(self.y_placement().unwrap_or(0))?; - } - - if new_format.contains(ValueFormat::X_ADVANCE) { - s.embed(self.x_advance().unwrap_or(0))?; - } + let mut offset = self.offset(); + let font_data = self.offset_data(); + let value_format = self.format(); - if new_format.contains(ValueFormat::Y_ADVANCE) { - s.embed(self.y_advance().unwrap_or(0))?; - } + for field in NON_DEVICE_FIELDS { + if !value_format.contains(field) { + continue; + } + if !new_format.contains(field) { + offset += 2; + continue; + } - if !new_format.intersects(ValueFormat::ANY_DEVICE_OR_VARIDX) { - return Ok(()); + let value_bytes = font_data + .slice(offset..offset + 2) + .ok_or_else(|| s.set_err(SerializeErrorFlags::SERIALIZE_ERROR_READ_ERROR))?; + s.embed_bytes(value_bytes.as_bytes())?; + offset += 2; } - for (field, device) in [ - (ValueFormat::X_PLACEMENT_DEVICE, self.x_placement_device()), - (ValueFormat::Y_PLACEMENT_DEVICE, self.y_placement_device()), - (ValueFormat::X_ADVANCE_DEVICE, self.x_advance_device()), - (ValueFormat::Y_ADVANCE_DEVICE, self.y_advance_device()), - ] { + for field in DEVICE_FIELDS { + if !value_format.contains(field) { + continue; + } if !new_format.contains(field) { + offset += 2; continue; } + let offset_pos = s.embed(0_u16)?; - if let Some(device) = device - .transpose() - .map_err(|_| s.set_err(SerializeErrorFlags::SERIALIZE_ERROR_READ_ERROR))? - { + let device_offset = font_data + .read_at::(offset) + .map_err(|_| s.set_err(SerializeErrorFlags::SERIALIZE_ERROR_READ_ERROR))?; + + if !device_offset.is_null() { + let device = device_offset + .resolve::(font_data) + .map_err(|_| s.set_err(SerializeErrorFlags::SERIALIZE_ERROR_READ_ERROR))?; + Offset16::serialize_subset( &device, s, @@ -121,6 +142,7 @@ impl<'a> SubsetTable<'a> for ValueRecord<'_> { ) .is_empty()?; } + offset += 2; } Ok(()) } @@ -132,17 +154,34 @@ impl CollectVariationIndices for ValueRecord<'_> { return; } - for device in [ - self.x_placement_device(), - self.y_placement_device(), - self.x_advance_device(), - self.y_advance_device(), - ] - .into_iter() - .flatten() - .flatten() - { + let mut offset = self.offset(); + let value_format = self.format(); + for field in NON_DEVICE_FIELDS { + if value_format.contains(field) { + offset += 2; + } + } + + let font_data = self.offset_data(); + for field in DEVICE_FIELDS { + if !value_format.contains(field) { + continue; + } + + let Ok(device_offset) = font_data.read_at::(offset) else { + return; + }; + + if device_offset.is_null() { + offset += 2; + continue; + } + + let Ok(device) = device_offset.resolve::(font_data) else { + return; + }; device.collect_variation_indices(plan, varidx_set); + offset += 2; } } } @@ -169,10 +208,13 @@ mod test { fn keeps_everything_when_stripping_nothing() { // with neither flag set the format is reported as-is, even for fields // that are zero: the caller has not asked for anything to be dropped - assert_eq!(compute_effective_format(&record(&EMPTY), false, false), ALL); + assert_eq!( + compute_effective_format(&record(&EMPTY), false, false), + Ok(ALL) + ); assert_eq!( compute_effective_format(&record(&PARTLY_SET), false, false), - ALL + Ok(ALL) ); } @@ -181,12 +223,12 @@ mod test { // an all-zero record keeps nothing assert_eq!( compute_effective_format(&record(&EMPTY), false, true), - ValueFormat::empty() + Ok(ValueFormat::empty()) ); // and only the two fields that are actually set survive assert_eq!( compute_effective_format(&record(&PARTLY_SET), false, true), - ValueFormat::X_PLACEMENT | ValueFormat::X_PLACEMENT_DEVICE + Ok(ValueFormat::X_PLACEMENT | ValueFormat::X_PLACEMENT_DEVICE) ); } @@ -196,11 +238,11 @@ mod test { // or not empty fields are being stripped assert_eq!( compute_effective_format(&record(&PARTLY_SET), true, false), - ALL - ValueFormat::ANY_DEVICE_OR_VARIDX + Ok(ALL - ValueFormat::ANY_DEVICE_OR_VARIDX) ); assert_eq!( compute_effective_format(&record(&PARTLY_SET), true, true), - ValueFormat::X_PLACEMENT + Ok(ValueFormat::X_PLACEMENT) ); } @@ -211,10 +253,10 @@ mod test { let format = ValueFormat::Y_ADVANCE | ValueFormat::Y_ADVANCE_DEVICE; let bytes = [0u8, 7, 0, 12]; let rec = ValueRecord::new(FontData::new(&bytes), 0, format); - assert_eq!(compute_effective_format(&rec, false, true), format); + assert_eq!(compute_effective_format(&rec, false, true), Ok(format)); assert_eq!( compute_effective_format(&rec, true, true), - ValueFormat::Y_ADVANCE + Ok(ValueFormat::Y_ADVANCE) ); } }