Skip to content

Commit

Permalink
[write] Minor improvment to cmap 4 segmentation
Browse files Browse the repository at this point in the history
There was one case where fonttools was saving several bytes on us, which
felt bad. We are now equal or better, for every font I've examined.
  • Loading branch information
cmyr committed Nov 13, 2024
1 parent 6a4e7d7 commit dfaa41e
Showing 1 changed file with 22 additions and 18 deletions.
40 changes: 22 additions & 18 deletions write-fonts/src/tables/cmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,16 +434,18 @@ impl<'a> Format4SegmentComputer<'a> {
}

let combined = prev.combine(current);
let should_merge = match next.as_ref() {
// if we can merge on both sides, consider total cost
Some(next) if current.can_merge(next) => {
combined.combine(*next).cost() < prev.cost() + current.cost() + next.cost()
}
// else just consider the left
_ => combined.cost() < prev.cost() + current.cost(),
};

if should_merge {
// if we should merge, based just on the previous segment cost
let should_merge = combined.cost() < prev.cost() + current.cost();
// but if we can also merge with the next segment we want to check
// that too
let should_merge_considering_next = next
.as_ref()
// don't bother computing if we have already decided to merge
.filter(|next| !should_merge && current.can_merge(next))
.map(|next| combined.combine(*next).cost() < prev.cost() + current.cost())
.unwrap_or(false);

if should_merge || should_merge_considering_next {
*prev = combined;
continue;
}
Expand Down Expand Up @@ -907,18 +909,20 @@ mod tests {
}

#[test]
// a small ordered segment between two larger unordered segments;
// merging this correctly requires us to consider the next segment as well
fn f4_sandwich_segment() {
// if we have a small segment that is mergeable on both sides,
// merging it all produces more savings than if it's only mergeable
// to the left.

let mapping = MappingBuilder::default()
.extend(['a', 'e'])
.extend('b'..='d')
.extend(['\r'])
.extend(('\x20'..='\x27').rev()) // cost = 8*2 + 8 = 24
.extend('\x28'..='\x29') // cost = 8
.extend(('\x2a'..='\x2f').rev()) // cost = 6*2 + 8 = 20
// combined =
// 16 * 2 + 8 = 40
.extend('\x30'..='\x39')
.build();

let format4 = expect_f4(&mapping);
// should end up with one generated segment (+ 0xfff)
assert_eq!(format4.end_code.len(), 2);
assert_eq!(format4.end_code.len(), 4);
}
}

0 comments on commit dfaa41e

Please sign in to comment.