Skip to content

Commit

Permalink
[read-fonts] Properly compute length of delta sets
Browse files Browse the repository at this point in the history
We were previously just taking all remaining data here, but we have
enough information to compute the expected length exactly. This will let
us better handle the case where data is missing.
  • Loading branch information
cmyr committed Mar 25, 2024
1 parent db35434 commit 262ec9d
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 11 deletions.
10 changes: 10 additions & 0 deletions font-codegen/src/parsing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,8 @@ pub(crate) enum CountTransform {
DeltaSetIndexData,
/// three args: the axis count, the tuple index, and a constant on that index
TupleLen,
/// only ItemVariationStore: requires item_count, word_delta_count, region_index_count
ItemVariationDataLen,
}

/// Attributes for specifying how to compile a field
Expand Down Expand Up @@ -1396,6 +1398,10 @@ static TRANSFORM_IDENTS: &[(CountTransform, &str)] = &[
(CountTransform::DeltaValueCount, "delta_value_count"),
(CountTransform::DeltaSetIndexData, "delta_set_index_data"),
(CountTransform::TupleLen, "tuple_len"),
(
CountTransform::ItemVariationDataLen,
"item_variation_data_len",
),
];

impl FromStr for CountTransform {
Expand Down Expand Up @@ -1428,6 +1434,7 @@ impl CountTransform {
CountTransform::DeltaValueCount => 3,
CountTransform::DeltaSetIndexData => 2,
CountTransform::TupleLen => 3,
CountTransform::ItemVariationDataLen => 3,
}
}
}
Expand Down Expand Up @@ -1561,6 +1568,9 @@ impl Count {
(CountTransform::TupleLen, [a, b, c]) => {
quote!(TupleIndex::tuple_len(#a, #b, #c))
}
(CountTransform::ItemVariationDataLen, [a, b, c]) => {
quote!(ItemVariationData::delta_sets_len(#a, #b, #c))
}
_ => unreachable!("validated before now"),
},
}
Expand Down
8 changes: 5 additions & 3 deletions read-fonts/generated/generated_variations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1137,12 +1137,14 @@ impl ItemVariationDataMarker {
impl<'a> FontRead<'a> for ItemVariationData<'a> {
fn read(data: FontData<'a>) -> Result<Self, ReadError> {
let mut cursor = data.cursor();
cursor.advance::<u16>();
cursor.advance::<u16>();
let item_count: u16 = cursor.read()?;
let word_delta_count: u16 = cursor.read()?;
let region_index_count: u16 = cursor.read()?;
let region_indexes_byte_len = region_index_count as usize * u16::RAW_BYTE_LEN;
cursor.advance_by(region_indexes_byte_len);
let delta_sets_byte_len = cursor.remaining_bytes() / u8::RAW_BYTE_LEN * u8::RAW_BYTE_LEN;
let delta_sets_byte_len =
ItemVariationData::delta_sets_len(item_count, word_delta_count, region_index_count)
* u8::RAW_BYTE_LEN;
cursor.advance_by(delta_sets_byte_len);
cursor.finish(ItemVariationDataMarker {
region_indexes_byte_len,
Expand Down
59 changes: 52 additions & 7 deletions read-fonts/src/tables/variations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -981,24 +981,39 @@ impl<'a> ItemVariationData<'a> {
/// inner index.
pub fn delta_set(&self, inner_index: u16) -> impl Iterator<Item = i32> + 'a + Clone {
let word_delta_count = self.word_delta_count();
let region_count = self.region_index_count();
let bytes_per_row = Self::delta_row_len(word_delta_count, region_count);
let long_words = word_delta_count & 0x8000 != 0;
let (word_size, small_size) = if long_words { (4, 2) } else { (2, 1) };
let word_delta_count = word_delta_count & 0x7FFF;
let region_count = self.region_index_count() as usize;
let row_size = word_delta_count as usize * word_size
+ region_count.saturating_sub(word_delta_count as usize) * small_size;
let offset = row_size * inner_index as usize;

let offset = bytes_per_row * inner_index as usize;
ItemDeltas {
cursor: FontData::new(self.delta_sets())
.slice(offset..)
.unwrap_or_default()
.cursor(),
word_delta_count,
long_words,
len: region_count as u16,
len: region_count,
pos: 0,
}
}

/// the length of one delta set
fn delta_row_len(word_delta_count: u16, region_index_count: u16) -> usize {
let region_count = region_index_count as usize;
let long_words = word_delta_count & 0x8000 != 0;
let (word_size, small_size) = if long_words { (4, 2) } else { (2, 1) };
let long_delta_count = (word_delta_count & 0x7FFF) as usize;
let short_delta_count = region_count.saturating_sub(long_delta_count);
long_delta_count * word_size + short_delta_count * small_size
}

// called from generated code: compute the length in bytes of the delta_sets data
fn delta_sets_len(item_count: u16, word_delta_count: u16, region_index_count: u16) -> usize {
let bytes_per_row = Self::delta_row_len(word_delta_count, region_index_count);
bytes_per_row * item_count as usize
}
}

#[derive(Clone)]
Expand Down Expand Up @@ -1062,7 +1077,7 @@ pub(crate) fn item_delta(
#[cfg(test)]
mod tests {
use super::*;
use crate::{FontRef, TableProvider};
use crate::{test_helpers::BeBuffer, FontRef, TableProvider};

#[test]
fn ivs_regions() {
Expand Down Expand Up @@ -1258,4 +1273,34 @@ mod tests {
}
}
}

#[test]
fn ivs_data_len_short() {
let data = BeBuffer::new()
.push(2u16) // item_count
.push(3u16) // word_delta_count
.push(5u16) // region_index_count
.extend([0u16, 1, 2, 3, 4]) // region_indices
.extend([1u8; 128]); // this is much more data than we need!

let ivs = ItemVariationData::read(data.font_data()).unwrap();
let row_len = (3 * u16::RAW_BYTE_LEN) + (2 * u8::RAW_BYTE_LEN); // 3 word deltas, 2 byte deltas
let expected_len = 2 * row_len;
assert_eq!(ivs.delta_sets().len(), expected_len);
}

#[test]
fn ivs_data_len_long() {
let data = BeBuffer::new()
.push(2u16) // item_count
.push(2u16 | 0x8000) // word_delta_count, long deltas
.push(4u16) // region_index_count
.extend([0u16, 1, 2]) // region_indices
.extend([1u8; 128]); // this is much more data than we need!

let ivs = ItemVariationData::read(data.font_data()).unwrap();
let row_len = (2 * u32::RAW_BYTE_LEN) + (2 * u16::RAW_BYTE_LEN); // 1 word (4-byte) delta, 2 short (2-byte)
let expected_len = 2 * row_len;
assert_eq!(ivs.delta_sets().len(), expected_len);
}
}
2 changes: 1 addition & 1 deletion resources/codegen_inputs/variations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ table ItemVariationData {
#[count($region_index_count)]
region_indexes: [u16],
/// Delta-set rows.
#[count(..)]
#[count(item_variation_data_len($item_count, $word_delta_count, $region_index_count))]
delta_sets: [u8],
}

0 comments on commit 262ec9d

Please sign in to comment.