[klippa] subset hdmx table

klippa/src/hdmx.rs

@@ -0,0 +1,70 @@
+//! impl subset() for hdmx
+
+use crate::serialize::{SerializeErrorFlags, Serializer};
+use crate::{Plan, Subset, SubsetError};
+use write_fonts::{
+    read::{
+        tables::hdmx::{DeviceRecord, Hdmx},
+        FontRef, TopLevelTable,
+    },
+    FontBuilder,
+};
+
+fn ceil_to_4(v: u32) -> u32 {
+    ((v - 1) | 3) + 1
+}
+
+// reference: subset() for hmtx/hhea in harfbuzz
+// <https://github.com/harfbuzz/harfbuzz/blob/e451e91ec3608a2ebfec34d0c4f0b3d880e00e33/src/hb-ot-hdmx-table.hh#L116>
+impl Subset for Hdmx<'_> {
+    fn subset(
+        &self,
+        plan: &Plan,
+        _font: &FontRef,
+        s: &mut Serializer,
+        _builder: &mut FontBuilder,
+    ) -> Result<(), SubsetError> {
+        s.embed(self.version())
+            .map_err(|_| SubsetError::SubsetTableError(Hdmx::TAG))?;
+        s.embed(self.num_records())
+            .map_err(|_| SubsetError::SubsetTableError(Hdmx::TAG))?;
+
+        let size_device_record = ceil_to_4(2 + plan.num_output_glyphs as u32);
+        s.embed(size_device_record)
+            .map_err(|_| SubsetError::SubsetTableError(Hdmx::TAG))?;
+
+        for record in self.records().iter() {
+            let Ok(r) = record else {
+                return Err(SubsetError::SubsetTableError(Hdmx::TAG));
+            };
+            serialize_device_record(&r, s, plan, size_device_record as usize)
+                .map_err(|_| SubsetError::SubsetTableError(Hdmx::TAG))?;
+        }
+        Ok(())
+    }
+}
+
+fn serialize_device_record(
+    record: &DeviceRecord,
+    s: &mut Serializer,
+    plan: &Plan,
+    size_device_record: usize,
+) -> Result<(), SerializeErrorFlags> {
+    s.embed(record.pixel_size())?;
+    let max_width_pos = s.embed(0_u8)?;
+    let widths_array_pos = s.allocate_size(size_device_record - 2, false)?;
+    let mut max_width = 0;
+    for (new_gid, old_gid) in plan.new_to_old_gid_list.iter() {
+        let old_idx = old_gid.to_u32() as usize;
+        let Some(wdth) = record.widths().get(old_idx) else {
+            return Err(SerializeErrorFlags::SERIALIZE_ERROR_OTHER);
+        };
+
+        let new_idx = new_gid.to_u32() as usize;
+        s.copy_assign(widths_array_pos + new_idx, *wdth);
+        max_width = max_width.max(*wdth);
+    }
+
+    s.copy_assign(max_width_pos, max_width);
+    Ok(())
+}

klippa/src/lib.rs

@@ -7,6 +7,7 @@ mod glyf_loca;
 mod gpos;
 mod gsub;
 mod gvar;
+mod hdmx;
 mod head;
 mod hmtx;
 mod layout;
@@ -23,7 +24,6 @@ pub use parsing_util::{
 
 use fnv::FnvHashMap;
 use serialize::Serializer;
-use skrifa::raw::tables::cmap::CmapSubtable;
 use skrifa::MetadataProvider;
 use thiserror::Error;
 use write_fonts::types::GlyphId;
@@ -34,11 +34,14 @@ use write_fonts::{
         tables::{
             cff::Cff,
             cff2::Cff2,
-            cmap::Cmap,
+            cmap::{Cmap, CmapSubtable},
+            cvar::Cvar,
+            gasp,
             glyf::{Glyf, Glyph},
             gpos::Gpos,
             gsub::Gsub,
             gvar::Gvar,
+            hdmx::Hdmx,
             head::Head,
             loca::Loca,
             name::Name,
@@ -172,6 +175,7 @@ pub struct Plan {
     codepoint_to_glyph: FnvHashMap<u32, GlyphId>,
 
     subset_flags: SubsetFlags,
+    no_subset_tables: IntSet<Tag>,
     drop_tables: IntSet<Tag>,
     name_ids: IntSet<NameId>,
     name_languages: IntSet<u16>,
@@ -214,6 +218,11 @@ impl Plan {
             ..Default::default()
         };
 
+        // ref: <https://github.com/harfbuzz/harfbuzz/blob/b5a65e0f20c30a7f13b2f6619479a6d666e603e0/src/hb-subset-input.cc#L71>
+        let default_no_subset_tables = [gasp::Gasp::TAG, FPGM, PREP, VDMX, DSIG];
+        this.no_subset_tables
+            .extend(default_no_subset_tables.iter().copied());
+
         this.populate_unicodes_to_retain(input_gids, input_unicodes, font);
         this.populate_gids_to_retain(font);
         this.create_old_gid_to_new_gid_map();
@@ -562,6 +571,23 @@ pub trait NameIdClosure {
     fn collect_name_ids(&self, plan: &mut Plan);
 }
 
+pub const CVT: Tag = Tag::new(b"cvt ");
+pub const DSIG: Tag = Tag::new(b"DSIG");
+pub const EBSC: Tag = Tag::new(b"EBSC");
+pub const FPGM: Tag = Tag::new(b"fpgm");
+pub const GLAT: Tag = Tag::new(b"Glat");
+pub const GLOC: Tag = Tag::new(b"Gloc");
+pub const JSTF: Tag = Tag::new(b"JSTF");
+pub const LTSH: Tag = Tag::new(b"LTSH");
+pub const MORX: Tag = Tag::new(b"morx");
+pub const MORT: Tag = Tag::new(b"mort");
+pub const KERX: Tag = Tag::new(b"kerx");
+pub const KERN: Tag = Tag::new(b"kern");
+pub const PCLT: Tag = Tag::new(b"PCLT");
+pub const PREP: Tag = Tag::new(b"prep");
+pub const SILF: Tag = Tag::new(b"Silf");
+pub const SILL: Tag = Tag::new(b"Sill");
+pub const VDMX: Tag = Tag::new(b"VDMX");
 // This trait is implemented for all font tables
 pub trait Subset {
     /// Subset this table, if successful a subset version of this table will be added to builder
@@ -579,27 +605,31 @@ pub fn subset_font(font: &FontRef, plan: &Plan) -> Result<Vec<u8>, SubsetError>
 
     for record in font.table_directory.table_records() {
         let tag = record.tag();
-        if plan.drop_tables.contains(tag) {
+        if should_drop_table(tag, plan) {
             continue;
         }
-
-        let table_len = record.length();
-        match tag {
-            Head::TAG => {
-                if font.glyf().is_err() {
-                    subset(tag, font, plan, &mut builder, table_len)?;
-                }
-            }
-            //Skip, handled by glyf
-            Loca::TAG => continue,
-            //Skip, handled by Hmtx
-            Hhea::TAG => continue,
-            _ => subset(tag, font, plan, &mut builder, table_len)?,
-        }
+        subset(tag, font, plan, &mut builder, record.length())?;
     }
     Ok(builder.build())
 }
 
+fn should_drop_table(tag: Tag, plan: &Plan) -> bool {
+    if plan.drop_tables.contains(tag) {
+        return true;
+    }
+
+    let no_hinting = plan
+        .subset_flags
+        .contains(SubsetFlags::SUBSET_FLAGS_NO_HINTING);
+
+    match tag {
+        // hint tables
+        Cvar::TAG | CVT | FPGM | PREP | Hdmx::TAG | VDMX => no_hinting,
+        //TODO: drop var tables during instancing when all axes are pinned
+        _ => false,
+    }
+}
+
 fn subset<'a>(
     table_tag: Tag,
     font: &FontRef<'a>,
@@ -620,7 +650,10 @@ fn subset<'a>(
     }
 
     //TODO: repack when there's an offset overflow
-    builder.add_raw(table_tag, s.copy_bytes());
+    let subsetted_data = s.copy_bytes();
+    if !subsetted_data.is_empty() {
+        builder.add_raw(table_tag, subsetted_data);
+    }
     Ok(())
 }
 
@@ -657,6 +690,10 @@ fn subset_table<'a>(
     builder: &mut FontBuilder<'a>,
     s: &mut Serializer,
 ) -> Result<(), SubsetError> {
+    if plan.no_subset_tables.contains(tag) {
+        return passthrough_table(tag, font, s);
+    }
+
     match tag {
         Cmap::TAG => font
             .cmap()
@@ -672,18 +709,30 @@ fn subset_table<'a>(
             .gvar()
             .map_err(|_| SubsetError::SubsetTableError(Gvar::TAG))?
             .subset(plan, font, s, builder),
+
+        Hdmx::TAG => font
+            .hdmx()
+            .map_err(|_| SubsetError::SubsetTableError(Hdmx::TAG))?
+            .subset(plan, font, s, builder),
+
         //handled by glyf table if exists
         Head::TAG => font.glyf().map(|_| ()).or_else(|_| {
             font.head()
                 .map_err(|_| SubsetError::SubsetTableError(Head::TAG))?
                 .subset(plan, font, s, builder)
         }),
 
+        //Skip, handled by Hmtx
+        Hhea::TAG => Ok(()),
+
         Hmtx::TAG => font
             .hmtx()
             .map_err(|_| SubsetError::SubsetTableError(Hmtx::TAG))?
             .subset(plan, font, s, builder),
 
+        //Skip, handled by glyf
+        Loca::TAG => Ok(()),
+
         Maxp::TAG => font
             .maxp()
             .map_err(|_| SubsetError::SubsetTableError(Maxp::TAG))?
@@ -703,18 +752,19 @@ fn subset_table<'a>(
             .post()
             .map_err(|_| SubsetError::SubsetTableError(Post::TAG))?
             .subset(plan, font, s, builder),
-        _ => {
-            if let Some(data) = font.data_for_tag(tag) {
-                s.embed_bytes(data.as_bytes())
-                    .map_err(|_| SubsetError::SubsetTableError(tag))?;
-                Ok(())
-            } else {
-                Err(SubsetError::SubsetTableError(tag))
-            }
-        }
+
+        _ => passthrough_table(tag, font, s),
     }
 }
 
+fn passthrough_table(tag: Tag, font: &FontRef<'_>, s: &mut Serializer) -> Result<(), SubsetError> {
+    if let Some(data) = font.data_for_tag(tag) {
+        s.embed_bytes(data.as_bytes())
+            .map_err(|_| SubsetError::SubsetTableError(tag))?;
+    }
+    Ok(())
+}
+
 pub fn estimate_subset_table_size(font: &FontRef, table_tag: Tag, plan: &Plan) -> usize {
     let Some(table_data) = font.data_for_tag(table_tag) else {
         return 0;

klippa/src/main.rs

@@ -7,9 +7,15 @@
 use clap::Parser;
 use klippa::{
     parse_drop_tables, parse_name_ids, parse_name_languages, parse_unicodes, populate_gids,
-    subset_font, Plan, SubsetFlags,
+    subset_font, Plan, SubsetFlags, DSIG, EBSC, GLAT, GLOC, JSTF, KERN, KERX, LTSH, MORT, MORX,
+    PCLT, SILF, SILL,
+};
+use write_fonts::read::{
+    collections::IntSet,
+    tables::{ebdt, eblc, feat, svg},
+    types::{NameId, Tag},
+    FontRef, TopLevelTable,
 };
-use write_fonts::read::{collections::IntSet, types::NameId, FontRef};
 
 #[derive(Parser, Debug)]
 //Allow name_IDs, so we keep the option name consistent with HB and fonttools
@@ -115,11 +121,40 @@ fn main() {
 
     let font_bytes = std::fs::read(&args.path).expect("Invalid input font file found");
     let font = FontRef::new(&font_bytes).expect("Error reading font bytes");
-    let drop_tables = match parse_drop_tables(&args.drop_tables.unwrap_or_default()) {
-        Ok(drop_tables) => drop_tables,
-        Err(e) => {
-            eprintln!("{e}");
-            std::process::exit(1);
+    let drop_tables = match &args.drop_tables {
+        Some(drop_tables_input) => match parse_drop_tables(drop_tables_input) {
+            Ok(drop_tables) => drop_tables,
+            Err(e) => {
+                eprintln!("{e}");
+                std::process::exit(1);
+            }
+        },
+        //default value: <https://github.com/harfbuzz/harfbuzz/blob/b5a65e0f20c30a7f13b2f6619479a6d666e603e0/src/hb-subset-input.cc#L46>
+        None => {
+            let default_drop_tables = [
+                // Layout disabled by default
+                MORX,
+                MORT,
+                KERX,
+                KERN,
+                // Copied from fontTools
+                JSTF,
+                DSIG,
+                ebdt::Ebdt::TAG,
+                eblc::Eblc::TAG,
+                EBSC,
+                svg::Svg::TAG,
+                PCLT,
+                LTSH,
+                // Graphite tables
+                feat::Feat::TAG,
+                GLAT,
+                GLOC,
+                SILF,
+                SILL,
+            ];
+            let drop_tables: IntSet<Tag> = default_drop_tables.iter().copied().collect();
+            drop_tables
         }
     };
 
@@ -131,7 +166,7 @@ fn main() {
                 std::process::exit(1);
             }
         },
-        // default value: https://github.com/harfbuzz/harfbuzz/blob/main/src/hb-subset-input.cc#L43
+        // default value: <https://github.com/harfbuzz/harfbuzz/blob/b5a65e0f20c30a7f13b2f6619479a6d666e603e0/src/hb-subset-input.cc#L43>
         None => {
             let mut default_name_ids = IntSet::<NameId>::empty();
             default_name_ids.insert_range(NameId::from(0)..=NameId::from(6));

klippa/tests/integration_test.rs

@@ -337,7 +337,7 @@ impl SubsetTestCase {
         Command::new("fonttools")
             .arg("subset")
             .arg(&org_font_file)
-            .arg("--drop-tables+=DSIG,BASE,GSUB,GPOS,GDEF,hdmx,fpgm,prep,cvt,gasp,cvar,HVAR,STAT")
+            .arg("--drop-tables+=DSIG,BASE,GSUB,GPOS,GDEF,fpgm,prep,cvt,gasp,cvar,HVAR,STAT")
             .arg("--drop-tables-=sbix")
             .arg("--no-harfbuzz-repacker")
             .arg("--no-prune-codepage-ranges")
@@ -389,7 +389,7 @@ fn gen_subset_font_file(
     let font = FontRef::new(&org_font_bytes).unwrap();
 
     let unicodes = parse_unicodes(subset).unwrap();
-    let drop_tables_str = "DSIG,BASE,GSUB,GPOS,GDEF,hdmx,fpgm,prep,cvt,gasp,cvar,HVAR,STAT";
+    let drop_tables_str = "morx,mort,kerx,kern,JSTF,DSIG,EBDT,EBLC,EBSC,SVG,PCLT,LTSH,feat,Glat,Gloc,Silf,Sill,BASE,GSUB,GPOS,GDEF,fpgm,prep,cvt,gasp,cvar,HVAR,STAT";
     let mut drop_tables = IntSet::empty();
     for str in drop_tables_str.split(',') {
         let tag = Tag::new_checked(str.as_bytes()).unwrap();