Skip to content

Fix left-side bearing for variable fonts #27

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
laurmaedje merged 1 commit into from
Jun 3, 2026
Merged

Fix left-side bearing for variable fonts #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/cff2.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,26 @@ use std::borrow::Cow;
/// CFF2 fonts will currently be converted into TTF fonts.
pub fn subset(ctx: &mut Context) -> crate::Result<()> {
let mut maxp_data = MaxpData::default();
let mut hmtx_data = Vec::new();

glyf::subset_with(ctx, |old_gid, ctx| {
let data = match &ctx.interjector {
// We reject CFF2 fonts earlier if `variable-fonts` feature is not enabled.
Interjector::Dummy(_) => unreachable!(),
#[cfg(feature = "variable-fonts")]
Interjector::Skrifa(s) => {
Cow::Owned(s.glyph_data(&mut maxp_data, old_gid).ok_or(MalformedFont)?)
let (advance, lsb, data) =
s.interject(&mut maxp_data, old_gid).ok_or(MalformedFont)?;
hmtx_data.push((advance, lsb));
Cow::Owned(data)
}
};

Ok(data)
})?;

ctx.custom_maxp_data = Some(maxp_data);
ctx.custom_hmtx_data = Some(hmtx_data);

Ok(())
}
8 changes: 7 additions & 1 deletion src/glyf.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ pub fn subset(ctx: &mut Context) -> Result<()> {

#[allow(unused_mut)]
let mut maxp_data = MaxpData::default();
#[allow(unused_mut)]
let mut hmtx_data = Vec::new();

subset_with(ctx, |old_gid, ctx| {
let data = match &ctx.interjector {
Expand All @@ -58,7 +60,10 @@ pub fn subset(ctx: &mut Context) -> Result<()> {
}
#[cfg(feature = "variable-fonts")]
Interjector::Skrifa(s) => {
Cow::Owned(s.glyph_data(&mut maxp_data, old_gid).ok_or(MalformedFont)?)
let (advance, lsb, data) =
s.interject(&mut maxp_data, old_gid).ok_or(MalformedFont)?;
hmtx_data.push((advance, lsb));
Cow::Owned(data)
}
};

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

if ctx.interjector.is_skrifa() {
ctx.custom_maxp_data = Some(maxp_data);
ctx.custom_hmtx_data = Some(hmtx_data);
}

Ok(())
Expand Down
6 changes: 2 additions & 4 deletions src/hmtx.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,8 @@ pub fn subset(ctx: &mut Context) -> Result<()> {
match &ctx.interjector {
Interjector::Dummy(_) => extract_metrics(hmtx, &mut new_metrics, ctx)?,
#[cfg(feature = "variable-fonts")]
Interjector::Skrifa(s) => {
for old_gid in ctx.mapper.remapped_gids() {
new_metrics.push(s.horizontal_metrics(old_gid).ok_or(MalformedFont)?);
}
Interjector::Skrifa(_) => {
new_metrics = ctx.custom_hmtx_data.take().ok_or(Error::SubsetError)?
}
}

Expand Down
45 changes: 23 additions & 22 deletions src/interjector.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,32 +48,19 @@ pub(crate) mod skrifa {
}

impl<'a> SkrifaInterjector<'a> {
/// Return the advance width and left side bearing of the glyph.
pub(crate) fn horizontal_metrics(&self, glyph: u16) -> Option<(u16, i16)> {
let metrics = self.font_ref.glyph_metrics(Size::unscaled(), &self.location);

let adv = metrics.advance_width(GlyphId::new(glyph as u32))?;
// Note that for variable fonts, our left side bearing points don't seem to
// match the ones from fonttools (they use some different technique for deriving
// it which isn't reflected in skrifa's API), but I _think_ that this shouldn't
// really be relevant in the context of PDF.
let lsb = metrics.left_side_bearing(GlyphId::new(glyph as u32))?;

Some((adv.round() as u16, lsb.round() as i16))
}

/// Return the glyph description in the `glyf` outline format.
pub(crate) fn glyph_data<'b>(
pub(crate) fn interject<'b>(
&'b self,
maxp_data: &'b mut MaxpData,
glyph: u16,
) -> Option<Vec<u8>> {
) -> Option<(u16, i16, Vec<u8>)> {
let outlines = self.font_ref.outline_glyphs();
let metrics = self.font_ref.glyph_metrics(Size::unscaled(), &self.location);

let mut outline_builder = OutlinePath::new();
let glyph = GlyphId::new(glyph as u32);
let glyph_id = GlyphId::new(glyph as u32);

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

let path = outline_builder.path;

let simple_glyph = SimpleGlyph::from_bezpath(&path).ok()?;
let advance = metrics.advance_width(glyph_id)?.round() as u16;

// We derive the LSB from the resulting bounding box rather than
// from the font's metrics, because the latter does not always agree
// with the `xMin` of the fresh outline we've generated.
//
// The OpenType spec heavily advises xMin and LSB to match (it
// actually requires it for variable fonts or when `head.flags` bit
// 1 is set).
//
// If `LSB != xMin`, glyphs get repositioned by PDF readers and the
// kerning gets very wonky.
let lsb = simple_glyph.bbox.x_min;

if path.is_empty() {
return Some(vec![]);
return Some((advance, lsb, vec![]));
}

let simple_glyph = SimpleGlyph::from_bezpath(&path).ok()?;

maxp_data.max_points = maxp_data
.max_points
.max(simple_glyph.contours.iter().map(|c| c.len() as u16).sum());
Expand All @@ -98,8 +98,9 @@ pub(crate) mod skrifa {

let mut writer = TableWriter::default();
simple_glyph.write_into(&mut writer);
let data = dump_table(&simple_glyph).ok()?;

dump_table(&simple_glyph).ok()
Some((advance, lsb, data))
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ fn prepare_context<'a>(
mapper: gid_remapper,
interjector,
custom_maxp_data: None,
custom_hmtx_data: None,
flavor,
tables: vec![],
long_loca: false,
Expand Down Expand Up @@ -383,6 +384,7 @@ struct Context<'a> {
/// Custom data that should be used for writing the `maxp` table. Only needed for CFF2,
/// where we need to synthesize a V1 table after converting.
pub(crate) custom_maxp_data: Option<MaxpData>,
pub(crate) custom_hmtx_data: Option<Vec<(u16, i16)>>,
/// Whether the long loca format was chosen.
long_loca: bool,
}
Expand Down
4 changes: 2 additions & 2 deletions tests/ttx/Cantarell-VF_1.ttx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
<!-- Most of this table will be recalculated by the compiler -->
<tableVersion value="1.0"/>
<fontRevision value="0.303"/>
<checkSumAdjustment value="0xae97bd7b"/>
<checkSumAdjustment value="0xae97bddf"/>
<magicNumber value="0x5f0f3cf5"/>
<flags value="00000000 00000011"/>
<unitsPerEm value="1000"/>
Expand Down Expand Up @@ -79,7 +79,7 @@
</maxp>

<hmtx>
<mtx name=".notdef" width="500" lsb="50"/>
<mtx name=".notdef" width="500" lsb="0"/>
<mtx name="A" width="626" lsb="7"/>
<mtx name="C" width="645" lsb="54"/>
<mtx name="Cacute" width="645" lsb="54"/>
Expand Down
28 changes: 14 additions & 14 deletions tests/ttx/Cantarell-VF_2.ttx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
<!-- Most of this table will be recalculated by the compiler -->
<tableVersion value="1.0"/>
<fontRevision value="0.303"/>
<checkSumAdjustment value="0xa3d604f1"/>
<checkSumAdjustment value="0xa3d206f7"/>
<magicNumber value="0x5f0f3cf5"/>
<flags value="00000000 00000011"/>
<unitsPerEm value="1000"/>
Expand Down Expand Up @@ -79,19 +79,19 @@
</maxp>

<hmtx>
<mtx name=".notdef" width="500" lsb="50"/>
<mtx name="A" width="691" lsb="7"/>
<mtx name="C" width="636" lsb="54"/>
<mtx name="Cacute" width="636" lsb="54"/>
<mtx name="Ccaron" width="636" lsb="54"/>
<mtx name="Ccedilla" width="636" lsb="54"/>
<mtx name="Ccircumflex" width="636" lsb="54"/>
<mtx name="Dcaron" width="750" lsb="92"/>
<mtx name="L" width="558" lsb="92"/>
<mtx name="Lacute" width="558" lsb="87"/>
<mtx name="uni01C7" width="1012" lsb="92"/>
<mtx name="uni1E08" width="636" lsb="54"/>
<mtx name="uni1EAA" width="691" lsb="7"/>
<mtx name=".notdef" width="500" lsb="0"/>
<mtx name="A" width="691" lsb="-7"/>
<mtx name="C" width="636" lsb="37"/>
<mtx name="Cacute" width="636" lsb="37"/>
<mtx name="Ccaron" width="636" lsb="37"/>
<mtx name="Ccedilla" width="636" lsb="37"/>
<mtx name="Ccircumflex" width="636" lsb="37"/>
<mtx name="Dcaron" width="750" lsb="71"/>
<mtx name="L" width="558" lsb="71"/>
<mtx name="Lacute" width="558" lsb="71"/>
<mtx name="uni01C7" width="1012" lsb="71"/>
<mtx name="uni1E08" width="636" lsb="37"/>
<mtx name="uni1EAA" width="691" lsb="-7"/>
</hmtx>

<loca>
Expand Down
24 changes: 12 additions & 12 deletions tests/ttx/NotoSans-Regular_var_2.ttx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
<!-- Most of this table will be recalculated by the compiler -->
<tableVersion value="1.0"/>
<fontRevision value="2.013"/>
<checkSumAdjustment value="0xe2047845"/>
<checkSumAdjustment value="0xe2047929"/>
<magicNumber value="0x5f0f3cf5"/>
<flags value="00000000 00000011"/>
<unitsPerEm value="1000"/>
Expand Down Expand Up @@ -80,20 +80,20 @@
</maxp>

<hmtx>
<mtx name=".notdef" width="582" lsb="94"/>
<mtx name="C" width="649" lsb="61"/>
<mtx name="Ccircumflex" width="649" lsb="61"/>
<mtx name="Cdotaccent" width="649" lsb="61"/>
<mtx name="E" width="549" lsb="97"/>
<mtx name="W" width="1039" lsb="12"/>
<mtx name=".notdef" width="582" lsb="85"/>
<mtx name="C" width="649" lsb="51"/>
<mtx name="Ccircumflex" width="649" lsb="51"/>
<mtx name="Cdotaccent" width="649" lsb="51"/>
<mtx name="E" width="549" lsb="77"/>
<mtx name="W" width="1039" lsb="15"/>
<mtx name="acute" width="418" lsb="40"/>
<mtx name="c" width="539" lsb="55"/>
<mtx name="cacute" width="539" lsb="55"/>
<mtx name="ccircumflex" width="539" lsb="55"/>
<mtx name="cdotaccent" width="539" lsb="55"/>
<mtx name="c" width="539" lsb="42"/>
<mtx name="cacute" width="539" lsb="42"/>
<mtx name="ccircumflex" width="539" lsb="42"/>
<mtx name="cdotaccent" width="539" lsb="42"/>
<mtx name="circumflex" width="523" lsb="40"/>
<mtx name="dotaccent" width="283" lsb="40"/>
<mtx name="quotesingle" width="292" lsb="65"/>
<mtx name="quotesingle" width="292" lsb="59"/>
</hmtx>

<loca>
Expand Down
24 changes: 12 additions & 12 deletions tests/ttx/NotoSans-Regular_var_3.ttx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
<!-- Most of this table will be recalculated by the compiler -->
<tableVersion value="1.0"/>
<fontRevision value="2.013"/>
<checkSumAdjustment value="0xec59e375"/>
<checkSumAdjustment value="0xec59e4fb"/>
<magicNumber value="0x5f0f3cf5"/>
<flags value="00000000 00000011"/>
<unitsPerEm value="1000"/>
Expand Down Expand Up @@ -80,20 +80,20 @@
</maxp>

<hmtx>
<mtx name=".notdef" width="586" lsb="94"/>
<mtx name="C" width="521" lsb="61"/>
<mtx name="Ccircumflex" width="521" lsb="61"/>
<mtx name="Cdotaccent" width="521" lsb="61"/>
<mtx name="E" width="456" lsb="97"/>
<mtx name="W" width="829" lsb="12"/>
<mtx name=".notdef" width="586" lsb="86"/>
<mtx name="C" width="521" lsb="44"/>
<mtx name="Ccircumflex" width="521" lsb="44"/>
<mtx name="Cdotaccent" width="521" lsb="44"/>
<mtx name="E" width="456" lsb="66"/>
<mtx name="W" width="829" lsb="2"/>
<mtx name="acute" width="336" lsb="40"/>
<mtx name="c" width="435" lsb="55"/>
<mtx name="cacute" width="435" lsb="55"/>
<mtx name="ccircumflex" width="435" lsb="55"/>
<mtx name="cdotaccent" width="435" lsb="55"/>
<mtx name="c" width="435" lsb="36"/>
<mtx name="cacute" width="435" lsb="36"/>
<mtx name="ccircumflex" width="435" lsb="36"/>
<mtx name="cdotaccent" width="435" lsb="36"/>
<mtx name="circumflex" width="422" lsb="40"/>
<mtx name="dotaccent" width="240" lsb="40"/>
<mtx name="quotesingle" width="249" lsb="65"/>
<mtx name="quotesingle" width="249" lsb="46"/>
</hmtx>

<loca>
Expand Down
Loading