Skip to content

Commit 7582134

Browse files
committed
Fix left-side bearing for variable fonts
1 parent cd35e66 commit 7582134

9 files changed

Lines changed: 81 additions & 68 deletions

File tree

src/cff2.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,26 @@ use std::borrow::Cow;
66
/// CFF2 fonts will currently be converted into TTF fonts.
77
pub fn subset(ctx: &mut Context) -> crate::Result<()> {
88
let mut maxp_data = MaxpData::default();
9+
let mut hmtx_data = Vec::new();
910

1011
glyf::subset_with(ctx, |old_gid, ctx| {
1112
let data = match &ctx.interjector {
1213
// We reject CFF2 fonts earlier if `variable-fonts` feature is not enabled.
1314
Interjector::Dummy(_) => unreachable!(),
1415
#[cfg(feature = "variable-fonts")]
1516
Interjector::Skrifa(s) => {
16-
Cow::Owned(s.glyph_data(&mut maxp_data, old_gid).ok_or(MalformedFont)?)
17+
let (advance, lsb, data) =
18+
s.interject(&mut maxp_data, old_gid).ok_or(MalformedFont)?;
19+
hmtx_data.push((advance, lsb));
20+
Cow::Owned(data)
1721
}
1822
};
1923

2024
Ok(data)
2125
})?;
2226

2327
ctx.custom_maxp_data = Some(maxp_data);
28+
ctx.custom_hmtx_data = Some(hmtx_data);
29+
2430
Ok(())
2531
}

src/glyf.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ pub fn subset(ctx: &mut Context) -> Result<()> {
5050

5151
#[allow(unused_mut)]
5252
let mut maxp_data = MaxpData::default();
53+
#[allow(unused_mut)]
54+
let mut hmtx_data = Vec::new();
5355

5456
subset_with(ctx, |old_gid, ctx| {
5557
let data = match &ctx.interjector {
@@ -58,7 +60,10 @@ pub fn subset(ctx: &mut Context) -> Result<()> {
5860
}
5961
#[cfg(feature = "variable-fonts")]
6062
Interjector::Skrifa(s) => {
61-
Cow::Owned(s.glyph_data(&mut maxp_data, old_gid).ok_or(MalformedFont)?)
63+
let (advance, lsb, data) =
64+
s.interject(&mut maxp_data, old_gid).ok_or(MalformedFont)?;
65+
hmtx_data.push((advance, lsb));
66+
Cow::Owned(data)
6267
}
6368
};
6469

@@ -67,6 +72,7 @@ pub fn subset(ctx: &mut Context) -> Result<()> {
6772

6873
if ctx.interjector.is_skrifa() {
6974
ctx.custom_maxp_data = Some(maxp_data);
75+
ctx.custom_hmtx_data = Some(hmtx_data);
7076
}
7177

7278
Ok(())

src/hmtx.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,8 @@ pub fn subset(ctx: &mut Context) -> Result<()> {
2222
match &ctx.interjector {
2323
Interjector::Dummy(_) => extract_metrics(hmtx, &mut new_metrics, ctx)?,
2424
#[cfg(feature = "variable-fonts")]
25-
Interjector::Skrifa(s) => {
26-
for old_gid in ctx.mapper.remapped_gids() {
27-
new_metrics.push(s.horizontal_metrics(old_gid).ok_or(MalformedFont)?);
28-
}
25+
Interjector::Skrifa(_) => {
26+
new_metrics = ctx.custom_hmtx_data.take().ok_or(Error::SubsetError)?
2927
}
3028
}
3129

src/interjector.rs

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -48,32 +48,19 @@ pub(crate) mod skrifa {
4848
}
4949

5050
impl<'a> SkrifaInterjector<'a> {
51-
/// Return the advance width and left side bearing of the glyph.
52-
pub(crate) fn horizontal_metrics(&self, glyph: u16) -> Option<(u16, i16)> {
53-
let metrics = self.font_ref.glyph_metrics(Size::unscaled(), &self.location);
54-
55-
let adv = metrics.advance_width(GlyphId::new(glyph as u32))?;
56-
// Note that for variable fonts, our left side bearing points don't seem to
57-
// match the ones from fonttools (they use some different technique for deriving
58-
// it which isn't reflected in skrifa's API), but I _think_ that this shouldn't
59-
// really be relevant in the context of PDF.
60-
let lsb = metrics.left_side_bearing(GlyphId::new(glyph as u32))?;
61-
62-
Some((adv.round() as u16, lsb.round() as i16))
63-
}
64-
6551
/// Return the glyph description in the `glyf` outline format.
66-
pub(crate) fn glyph_data<'b>(
52+
pub(crate) fn interject<'b>(
6753
&'b self,
6854
maxp_data: &'b mut MaxpData,
6955
glyph: u16,
70-
) -> Option<Vec<u8>> {
56+
) -> Option<(u16, i16, Vec<u8>)> {
7157
let outlines = self.font_ref.outline_glyphs();
58+
let metrics = self.font_ref.glyph_metrics(Size::unscaled(), &self.location);
7259

7360
let mut outline_builder = OutlinePath::new();
74-
let glyph = GlyphId::new(glyph as u32);
61+
let glyph_id = GlyphId::new(glyph as u32);
7562

76-
if let Some(outline_glyph) = outlines.get(glyph) {
63+
if let Some(outline_glyph) = outlines.get(glyph_id) {
7764
outline_glyph
7865
.draw(
7966
DrawSettings::unhinted(Size::unscaled(), &self.location),
@@ -84,12 +71,25 @@ pub(crate) mod skrifa {
8471

8572
let path = outline_builder.path;
8673

74+
let simple_glyph = SimpleGlyph::from_bezpath(&path).ok()?;
75+
let advance = metrics.advance_width(glyph_id)?.round() as u16;
76+
77+
// We derive the LSB from the resulting bounding box rather than
78+
// from the font's metrics, because the latter does not always agree
79+
// with the `xMin` of the fresh outline we've generated.
80+
//
81+
// The OpenType spec heavily advises xMin and LSB to match (it
82+
// actually requires it for variable fonts or when `head.flags` bit
83+
// 1 is set).
84+
//
85+
// If `LSB != xMin`, glyphs get repositioned by PDF readers and the
86+
// kerning gets very wonky.
87+
let lsb = simple_glyph.bbox.x_min;
88+
8789
if path.is_empty() {
88-
return Some(vec![]);
90+
return Some((advance, lsb, vec![]));
8991
}
9092

91-
let simple_glyph = SimpleGlyph::from_bezpath(&path).ok()?;
92-
9393
maxp_data.max_points = maxp_data
9494
.max_points
9595
.max(simple_glyph.contours.iter().map(|c| c.len() as u16).sum());
@@ -98,8 +98,9 @@ pub(crate) mod skrifa {
9898

9999
let mut writer = TableWriter::default();
100100
simple_glyph.write_into(&mut writer);
101+
let data = dump_table(&simple_glyph).ok()?;
101102

102-
dump_table(&simple_glyph).ok()
103+
Some((advance, lsb, data))
103104
}
104105
}
105106

src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ fn prepare_context<'a>(
200200
mapper: gid_remapper,
201201
interjector,
202202
custom_maxp_data: None,
203+
custom_hmtx_data: None,
203204
flavor,
204205
tables: vec![],
205206
long_loca: false,
@@ -383,6 +384,7 @@ struct Context<'a> {
383384
/// Custom data that should be used for writing the `maxp` table. Only needed for CFF2,
384385
/// where we need to synthesize a V1 table after converting.
385386
pub(crate) custom_maxp_data: Option<MaxpData>,
387+
pub(crate) custom_hmtx_data: Option<Vec<(u16, i16)>>,
386388
/// Whether the long loca format was chosen.
387389
long_loca: bool,
388390
}

tests/ttx/Cantarell-VF_1.ttx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
<!-- Most of this table will be recalculated by the compiler -->
2323
<tableVersion value="1.0"/>
2424
<fontRevision value="0.303"/>
25-
<checkSumAdjustment value="0xae97bd7b"/>
25+
<checkSumAdjustment value="0xae97bddf"/>
2626
<magicNumber value="0x5f0f3cf5"/>
2727
<flags value="00000000 00000011"/>
2828
<unitsPerEm value="1000"/>
@@ -79,7 +79,7 @@
7979
</maxp>
8080

8181
<hmtx>
82-
<mtx name=".notdef" width="500" lsb="50"/>
82+
<mtx name=".notdef" width="500" lsb="0"/>
8383
<mtx name="A" width="626" lsb="7"/>
8484
<mtx name="C" width="645" lsb="54"/>
8585
<mtx name="Cacute" width="645" lsb="54"/>

tests/ttx/Cantarell-VF_2.ttx

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
<!-- Most of this table will be recalculated by the compiler -->
2323
<tableVersion value="1.0"/>
2424
<fontRevision value="0.303"/>
25-
<checkSumAdjustment value="0xa3d604f1"/>
25+
<checkSumAdjustment value="0xa3d206f7"/>
2626
<magicNumber value="0x5f0f3cf5"/>
2727
<flags value="00000000 00000011"/>
2828
<unitsPerEm value="1000"/>
@@ -79,19 +79,19 @@
7979
</maxp>
8080

8181
<hmtx>
82-
<mtx name=".notdef" width="500" lsb="50"/>
83-
<mtx name="A" width="691" lsb="7"/>
84-
<mtx name="C" width="636" lsb="54"/>
85-
<mtx name="Cacute" width="636" lsb="54"/>
86-
<mtx name="Ccaron" width="636" lsb="54"/>
87-
<mtx name="Ccedilla" width="636" lsb="54"/>
88-
<mtx name="Ccircumflex" width="636" lsb="54"/>
89-
<mtx name="Dcaron" width="750" lsb="92"/>
90-
<mtx name="L" width="558" lsb="92"/>
91-
<mtx name="Lacute" width="558" lsb="87"/>
92-
<mtx name="uni01C7" width="1012" lsb="92"/>
93-
<mtx name="uni1E08" width="636" lsb="54"/>
94-
<mtx name="uni1EAA" width="691" lsb="7"/>
82+
<mtx name=".notdef" width="500" lsb="0"/>
83+
<mtx name="A" width="691" lsb="-7"/>
84+
<mtx name="C" width="636" lsb="37"/>
85+
<mtx name="Cacute" width="636" lsb="37"/>
86+
<mtx name="Ccaron" width="636" lsb="37"/>
87+
<mtx name="Ccedilla" width="636" lsb="37"/>
88+
<mtx name="Ccircumflex" width="636" lsb="37"/>
89+
<mtx name="Dcaron" width="750" lsb="71"/>
90+
<mtx name="L" width="558" lsb="71"/>
91+
<mtx name="Lacute" width="558" lsb="71"/>
92+
<mtx name="uni01C7" width="1012" lsb="71"/>
93+
<mtx name="uni1E08" width="636" lsb="37"/>
94+
<mtx name="uni1EAA" width="691" lsb="-7"/>
9595
</hmtx>
9696

9797
<loca>

tests/ttx/NotoSans-Regular_var_2.ttx

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
<!-- Most of this table will be recalculated by the compiler -->
2424
<tableVersion value="1.0"/>
2525
<fontRevision value="2.013"/>
26-
<checkSumAdjustment value="0xe2047845"/>
26+
<checkSumAdjustment value="0xe2047929"/>
2727
<magicNumber value="0x5f0f3cf5"/>
2828
<flags value="00000000 00000011"/>
2929
<unitsPerEm value="1000"/>
@@ -80,20 +80,20 @@
8080
</maxp>
8181

8282
<hmtx>
83-
<mtx name=".notdef" width="582" lsb="94"/>
84-
<mtx name="C" width="649" lsb="61"/>
85-
<mtx name="Ccircumflex" width="649" lsb="61"/>
86-
<mtx name="Cdotaccent" width="649" lsb="61"/>
87-
<mtx name="E" width="549" lsb="97"/>
88-
<mtx name="W" width="1039" lsb="12"/>
83+
<mtx name=".notdef" width="582" lsb="85"/>
84+
<mtx name="C" width="649" lsb="51"/>
85+
<mtx name="Ccircumflex" width="649" lsb="51"/>
86+
<mtx name="Cdotaccent" width="649" lsb="51"/>
87+
<mtx name="E" width="549" lsb="77"/>
88+
<mtx name="W" width="1039" lsb="15"/>
8989
<mtx name="acute" width="418" lsb="40"/>
90-
<mtx name="c" width="539" lsb="55"/>
91-
<mtx name="cacute" width="539" lsb="55"/>
92-
<mtx name="ccircumflex" width="539" lsb="55"/>
93-
<mtx name="cdotaccent" width="539" lsb="55"/>
90+
<mtx name="c" width="539" lsb="42"/>
91+
<mtx name="cacute" width="539" lsb="42"/>
92+
<mtx name="ccircumflex" width="539" lsb="42"/>
93+
<mtx name="cdotaccent" width="539" lsb="42"/>
9494
<mtx name="circumflex" width="523" lsb="40"/>
9595
<mtx name="dotaccent" width="283" lsb="40"/>
96-
<mtx name="quotesingle" width="292" lsb="65"/>
96+
<mtx name="quotesingle" width="292" lsb="59"/>
9797
</hmtx>
9898

9999
<loca>

tests/ttx/NotoSans-Regular_var_3.ttx

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
<!-- Most of this table will be recalculated by the compiler -->
2424
<tableVersion value="1.0"/>
2525
<fontRevision value="2.013"/>
26-
<checkSumAdjustment value="0xec59e375"/>
26+
<checkSumAdjustment value="0xec59e4fb"/>
2727
<magicNumber value="0x5f0f3cf5"/>
2828
<flags value="00000000 00000011"/>
2929
<unitsPerEm value="1000"/>
@@ -80,20 +80,20 @@
8080
</maxp>
8181

8282
<hmtx>
83-
<mtx name=".notdef" width="586" lsb="94"/>
84-
<mtx name="C" width="521" lsb="61"/>
85-
<mtx name="Ccircumflex" width="521" lsb="61"/>
86-
<mtx name="Cdotaccent" width="521" lsb="61"/>
87-
<mtx name="E" width="456" lsb="97"/>
88-
<mtx name="W" width="829" lsb="12"/>
83+
<mtx name=".notdef" width="586" lsb="86"/>
84+
<mtx name="C" width="521" lsb="44"/>
85+
<mtx name="Ccircumflex" width="521" lsb="44"/>
86+
<mtx name="Cdotaccent" width="521" lsb="44"/>
87+
<mtx name="E" width="456" lsb="66"/>
88+
<mtx name="W" width="829" lsb="2"/>
8989
<mtx name="acute" width="336" lsb="40"/>
90-
<mtx name="c" width="435" lsb="55"/>
91-
<mtx name="cacute" width="435" lsb="55"/>
92-
<mtx name="ccircumflex" width="435" lsb="55"/>
93-
<mtx name="cdotaccent" width="435" lsb="55"/>
90+
<mtx name="c" width="435" lsb="36"/>
91+
<mtx name="cacute" width="435" lsb="36"/>
92+
<mtx name="ccircumflex" width="435" lsb="36"/>
93+
<mtx name="cdotaccent" width="435" lsb="36"/>
9494
<mtx name="circumflex" width="422" lsb="40"/>
9595
<mtx name="dotaccent" width="240" lsb="40"/>
96-
<mtx name="quotesingle" width="249" lsb="65"/>
96+
<mtx name="quotesingle" width="249" lsb="46"/>
9797
</hmtx>
9898

9999
<loca>

0 commit comments

Comments
 (0)