Skip to content

Commit

Permalink
[write-fonts] Additional validation for PairPos1
Browse files Browse the repository at this point in the history
Another check to ensure that all value records report the correct
format.
  • Loading branch information
cmyr committed Sep 7, 2023
1 parent 2ff7fa7 commit 84dbe3c
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 1 deletion.
1 change: 1 addition & 0 deletions resources/codegen_inputs/gpos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ table PairPosFormat1 {
/// of PairPos subtable, ordered by Coverage Index.
#[count($pair_set_count)]
#[read_offset_with($value_format1, $value_format2)]
#[validate(check_format_consistency)]
pair_set_offsets: [Offset16<PairSet>],
}

Expand Down
2 changes: 1 addition & 1 deletion write-fonts/generated/generated_gpos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -953,7 +953,7 @@ impl Validate for PairPosFormat1 {
if self.pair_sets.len() > (u16::MAX as usize) {
ctx.report("array exceeds max length");
}
self.pair_sets.validate_impl(ctx);
self.check_format_consistency(ctx);
});
})
}
Expand Down
66 changes: 66 additions & 0 deletions write-fonts/src/tables/gpos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,27 @@ impl PairPosFormat1 {
.map(|rec| rec.value_record2.format())
.unwrap_or(ValueFormat::empty())
}

fn check_format_consistency(&self, ctx: &mut ValidationCtx) {
let vf1 = self.compute_value_format1();
let vf2 = self.compute_value_format2();
ctx.in_array(|ctx| {
for pairset in &self.pair_sets {
ctx.array_item(|ctx| {
ctx.in_field("pair_value_records", |ctx| {
ctx.in_array(|ctx| {
if pairset.pair_value_records.iter().any(|pairset| {
pairset.value_record1.format() != vf1
|| pairset.value_record2.format() != vf2
}) {
ctx.report("all ValueRecords must have same format")
}
})
})
})
}
})
}
}

impl PairPosFormat2 {
Expand Down Expand Up @@ -197,6 +218,8 @@ mod tests {

use read_fonts::tables::{gpos as read_gpos, layout::LookupFlag};

use crate::tables::layout::VariationIndex;

use super::*;

// adapted from/motivated by https://github.com/fonttools/fonttools/issues/471
Expand Down Expand Up @@ -312,4 +335,47 @@ mod tests {
let ppf2 = PairPos::format_2(coverage, class1, class2, class1recs);
crate::dump_table(&ppf2).unwrap();
}

#[test]
fn validate_pairpos1() {
let coverage: CoverageTable = [1, 2].into_iter().map(GlyphId::new).collect();
let good_table = PairPosFormat1::new(
coverage.clone(),
vec![
PairSet::new(vec![PairValueRecord::new(
GlyphId::new(5),
ValueRecord::new().with_x_advance(5),
ValueRecord::new(),
)]),
PairSet::new(vec![PairValueRecord::new(
GlyphId::new(1),
ValueRecord::new().with_x_advance(42),
ValueRecord::new(),
)]),
],
);

let bad_table = PairPosFormat1::new(
coverage,
vec![
PairSet::new(vec![PairValueRecord::new(
GlyphId::new(5),
ValueRecord::new().with_x_advance(5),
ValueRecord::new(),
)]),
PairSet::new(vec![PairValueRecord::new(
GlyphId::new(1),
//this is a different format, which is not okay
ValueRecord::new().with_x_placement(42),
ValueRecord::new(),
)]),
],
);

assert!(crate::dump_table(&good_table).is_ok());
assert!(matches!(
crate::dump_table(&bad_table),
Err(crate::error::Error::ValidationFailed(_))
));
}
}

0 comments on commit 84dbe3c

Please sign in to comment.