From 41dcad24ed6fdeb02f1ec54c00ad98a5e3018a4d Mon Sep 17 00:00:00 2001 From: Laurenz Date: Thu, 28 May 2026 10:37:41 +0200 Subject: [PATCH 1/6] Clippy fixes --- src/lib.rs | 2 +- src/read.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 23fc7096..57b88015 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -551,7 +551,7 @@ impl Tag { } impl Readable<'_> for Tag { - const SIZE: usize = u8::SIZE * 4; + const SIZE: usize = 4; fn read(r: &mut Reader) -> Option { r.read::<[u8; 4]>().map(Self) diff --git a/src/read.rs b/src/read.rs index 0e6da956..d0d6b5a9 100644 --- a/src/read.rs +++ b/src/read.rs @@ -99,7 +99,7 @@ pub trait Readable<'a>: Sized { } impl Readable<'_> for [u8; N] { - const SIZE: usize = u8::SIZE * N; + const SIZE: usize = N; fn read(r: &mut Reader) -> Option { Some(r.read_bytes(N)?.try_into().unwrap_or([0; N])) From 03f50a8b529913d3b16351afb2fa944969dc15d8 Mon Sep 17 00:00:00 2001 From: Laurenz Date: Thu, 28 May 2026 17:09:13 +0200 Subject: [PATCH 2/6] Add `Interjector::is_skrifa` --- src/interjector.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/interjector.rs b/src/interjector.rs index b930905e..3d0d3cc5 100644 --- a/src/interjector.rs +++ b/src/interjector.rs @@ -6,6 +6,16 @@ pub(crate) enum Interjector<'a> { Skrifa(skrifa::SkrifaInterjector<'a>), } +impl Interjector<'_> { + pub fn is_skrifa(&self) -> bool { + match self { + Self::Dummy(_) => false, + #[cfg(feature = "variable-fonts")] + Self::Skrifa(_) => true, + } + } +} + #[cfg(feature = "variable-fonts")] pub(crate) mod skrifa { use crate::{MaxpData, Tag}; From fb8d0aaa213fe817f2e6eb2f96df089d7cc17f81 Mon Sep 17 00:00:00 2001 From: Laurenz Date: Thu, 28 May 2026 10:18:44 +0200 Subject: [PATCH 3/6] Assign maxp data when interjecting TTF with skrifa --- src/cff2.rs | 6 +++--- src/glyf.rs | 14 +++++++++++--- tests/ttx/NotoSans-Regular_var_1.ttx | 12 ++++++------ tests/ttx/NotoSans-Regular_var_2.ttx | 12 ++++++------ tests/ttx/NotoSans-Regular_var_3.ttx | 12 ++++++------ 5 files changed, 32 insertions(+), 24 deletions(-) diff --git a/src/cff2.rs b/src/cff2.rs index 79631363..3b92fd12 100644 --- a/src/cff2.rs +++ b/src/cff2.rs @@ -7,7 +7,7 @@ use std::borrow::Cow; pub fn subset(ctx: &mut Context) -> crate::Result<()> { let mut maxp_data = MaxpData::default(); - let result = glyf::subset_with(ctx, |old_gid, ctx| { + 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!(), @@ -18,8 +18,8 @@ pub fn subset(ctx: &mut Context) -> crate::Result<()> { }; Ok(data) - }); + })?; ctx.custom_maxp_data = Some(maxp_data); - result + Ok(()) } diff --git a/src/glyf.rs b/src/glyf.rs index 40cff8a4..e13259d4 100644 --- a/src/glyf.rs +++ b/src/glyf.rs @@ -47,7 +47,9 @@ pub fn closure(face: &Face, glyph_remapper: &mut GlyphRemapper) -> Result<()> { pub fn subset(ctx: &mut Context) -> Result<()> { let table = Table::new(&ctx.face).ok_or(MalformedFont)?; - let mut _maxp = MaxpData::default(); + + #[allow(unused_mut)] + let mut maxp_data = MaxpData::default(); subset_with(ctx, |old_gid, ctx| { let data = match &ctx.interjector { @@ -56,12 +58,18 @@ pub fn subset(ctx: &mut Context) -> Result<()> { } #[cfg(feature = "variable-fonts")] Interjector::Skrifa(s) => { - Cow::Owned(s.glyph_data(&mut _maxp, old_gid).ok_or(MalformedFont)?) + Cow::Owned(s.glyph_data(&mut maxp_data, old_gid).ok_or(MalformedFont)?) } }; Ok(data) - }) + })?; + + if ctx.interjector.is_skrifa() { + ctx.custom_maxp_data = Some(maxp_data); + } + + Ok(()) } pub(crate) fn subset_with<'a>( diff --git a/tests/ttx/NotoSans-Regular_var_1.ttx b/tests/ttx/NotoSans-Regular_var_1.ttx index f5a9e8f8..fc2d52ae 100644 --- a/tests/ttx/NotoSans-Regular_var_1.ttx +++ b/tests/ttx/NotoSans-Regular_var_1.ttx @@ -23,7 +23,7 @@ - + @@ -64,10 +64,10 @@ - - - - + + + + @@ -75,7 +75,7 @@ - + diff --git a/tests/ttx/NotoSans-Regular_var_2.ttx b/tests/ttx/NotoSans-Regular_var_2.ttx index 3ea7244e..f09ac0c9 100644 --- a/tests/ttx/NotoSans-Regular_var_2.ttx +++ b/tests/ttx/NotoSans-Regular_var_2.ttx @@ -23,7 +23,7 @@ - + @@ -64,10 +64,10 @@ - - - - + + + + @@ -75,7 +75,7 @@ - + diff --git a/tests/ttx/NotoSans-Regular_var_3.ttx b/tests/ttx/NotoSans-Regular_var_3.ttx index f1d62dc9..e5cd26f6 100644 --- a/tests/ttx/NotoSans-Regular_var_3.ttx +++ b/tests/ttx/NotoSans-Regular_var_3.ttx @@ -23,7 +23,7 @@ - + @@ -64,10 +64,10 @@ - - - - + + + + @@ -75,7 +75,7 @@ - + From fe7a0a2e960ec9d1b2459d98f03c7fd191644d6f Mon Sep 17 00:00:00 2001 From: Laurenz Date: Thu, 28 May 2026 17:09:06 +0200 Subject: [PATCH 4/6] Don't copy `cvt`, `fpgm`, and `prep` if we use interjection --- src/lib.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 57b88015..85720819 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -221,9 +221,15 @@ fn _subset(mut ctx: Context) -> Result> { if ctx.flavor == FontFlavor::TrueType { // LOCA will be handled by GLYF ctx.process(Tag::GLYF)?; - ctx.process(Tag::CVT)?; // won't be subsetted. - ctx.process(Tag::FPGM)?; // won't be subsetted. - ctx.process(Tag::PREP)?; // won't be subsetted. + + // Only copy hinting tables if we don't interject because skrifa will + // discard hinting information. + // TODO: Add a test for this. + if !ctx.interjector.is_skrifa() { + ctx.process(Tag::CVT)?; // won't be subsetted. + ctx.process(Tag::FPGM)?; // won't be subsetted. + ctx.process(Tag::PREP)?; // won't be subsetted. + } } else if ctx.flavor == FontFlavor::Cff { ctx.process(Tag::CFF)?; } else if ctx.flavor == FontFlavor::Cff2 { From d8af4f7bc04d9f9f9a27a2cca337a876dd7b0ab5 Mon Sep 17 00:00:00 2001 From: Laurenz Date: Thu, 28 May 2026 10:22:48 +0200 Subject: [PATCH 5/6] Fix potential panic for malformed `hhea` table --- src/hmtx.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hmtx.rs b/src/hmtx.rs index dd52dcad..1eb927d8 100644 --- a/src/hmtx.rs +++ b/src/hmtx.rs @@ -60,7 +60,7 @@ pub fn subset(ctx: &mut Context) -> Result<()> { let hhea = ctx.expect_table(Tag::HHEA).ok_or(MalformedFont)?; let mut sub_hhea = Writer::new(); - sub_hhea.extend(&hhea[0..hhea.len() - 2]); + sub_hhea.extend(hhea.get(..hhea.len() - 2).ok_or(MalformedFont)?); sub_hhea.write::(last_advance_width_index + 1); ctx.push(Tag::HHEA, sub_hhea.finish()); From 6174cf8e0a6c9d1836b0b7c50744982dd28c9b8a Mon Sep 17 00:00:00 2001 From: Laurenz Date: Thu, 28 May 2026 10:23:57 +0200 Subject: [PATCH 6/6] Add comment explaining why `head` modification is safe --- src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 85720819..dae99fdd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -309,7 +309,8 @@ fn construct(mut ctx: Context) -> Vec { let mut offset = 12 + ctx.tables.len() * 16; for (tag, data) in &mut ctx.tables { if *tag == Tag::HEAD { - // Zero out checksum field in head table. + // Zero out checksum field in head table. The head table has already + // been subset and is guaranteed to have the necessary minimum size. data.to_mut()[8..12].fill(0); checksum_adjustment_offset = Some(offset + 8); }