From fa4c9bac31d94e69971942e3c47fcfdb7c5d4adc Mon Sep 17 00:00:00 2001 From: Harry Dalton Date: Wed, 20 Nov 2024 22:41:28 +0000 Subject: [PATCH 1/7] Add and register work and IR to compile a `meta` table into a built font --- fontbe/src/font.rs | 6 +++- fontbe/src/lib.rs | 1 + fontbe/src/meta.rs | 56 +++++++++++++++++++++++++++++++++++++ fontbe/src/orchestration.rs | 6 ++++ fontbe/src/paths.rs | 1 + fontc/src/lib.rs | 8 ++++++ fontc/src/timing.rs | 1 + fontir/src/ir.rs | 7 ++++- 8 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 fontbe/src/meta.rs diff --git a/fontbe/src/font.rs b/fontbe/src/font.rs index b4848e5fc..541abd424 100644 --- a/fontbe/src/font.rs +++ b/fontbe/src/font.rs @@ -8,7 +8,7 @@ use write_fonts::{ tables::{ avar::Avar, cmap::Cmap, fvar::Fvar, gdef::Gdef, glyf::Glyf, gpos::Gpos, gsub::Gsub, gvar::Gvar, head::Head, hhea::Hhea, hmtx::Hmtx, hvar::Hvar, loca::Loca, maxp::Maxp, - mvar::Mvar, name::Name, os2::Os2, post::Post, stat::Stat, + meta::Meta, mvar::Mvar, name::Name, os2::Os2, post::Post, stat::Stat, }, types::Tag, FontBuilder, @@ -45,6 +45,7 @@ const TABLES_TO_MERGE: &[(WorkId, Tag, TableType)] = &[ (WorkId::Gvar, Gvar::TAG, TableType::Variable), (WorkId::Loca, Loca::TAG, TableType::Static), (WorkId::Maxp, Maxp::TAG, TableType::Static), + (WorkId::Meta, Meta::TAG, TableType::Static), (WorkId::Name, Name::TAG, TableType::Static), (WorkId::Os2, Os2::TAG, TableType::Static), (WorkId::Post, Post::TAG, TableType::Static), @@ -68,6 +69,7 @@ fn has(context: &Context, id: WorkId) -> bool { WorkId::Gvar => context.gvar.try_get().is_some(), WorkId::Loca => context.loca.try_get().is_some(), WorkId::Maxp => context.maxp.try_get().is_some(), + WorkId::Meta => context.meta.try_get().is_some(), WorkId::Name => context.name.try_get().is_some(), WorkId::Os2 => context.os2.try_get().is_some(), WorkId::Post => context.post.try_get().is_some(), @@ -94,6 +96,7 @@ fn bytes_for(context: &Context, id: WorkId) -> Result>, Error> { WorkId::Gvar => Some(context.gvar.get().as_ref().get().to_vec()), WorkId::Loca => Some(context.loca.get().as_ref().get().to_vec()), WorkId::Maxp => to_bytes(context.maxp.get().as_ref()), + WorkId::Meta => to_bytes(context.meta.get().as_ref()), WorkId::Name => to_bytes(context.name.get().as_ref()), WorkId::Os2 => to_bytes(context.os2.get().as_ref()), WorkId::Post => to_bytes(context.post.get().as_ref()), @@ -125,6 +128,7 @@ impl Work for FontWork { .variant(WorkId::Gvar) .variant(WorkId::Loca) .variant(WorkId::Maxp) + .variant(WorkId::Meta) .variant(WorkId::Name) .variant(WorkId::Os2) .variant(WorkId::Post) diff --git a/fontbe/src/lib.rs b/fontbe/src/lib.rs index 6781e9e27..2c0cc68fd 100644 --- a/fontbe/src/lib.rs +++ b/fontbe/src/lib.rs @@ -9,6 +9,7 @@ pub mod glyphs; pub mod gvar; pub mod head; pub mod hvar; +pub mod meta; pub mod metrics_and_limits; pub mod mvar; pub mod name; diff --git a/fontbe/src/meta.rs b/fontbe/src/meta.rs new file mode 100644 index 000000000..ebd1552b4 --- /dev/null +++ b/fontbe/src/meta.rs @@ -0,0 +1,56 @@ +//! Generates a [meta](https://learn.microsoft.com/en-us/typography/opentype/spec/meta) table. + +use fontdrasil::orchestration::{Access, AccessBuilder, Work}; +use fontir::orchestration::WorkId as FeWorkId; +use write_fonts::tables::meta::{DataMapRecord, Meta}; + +use crate::{ + error::Error, + orchestration::{AnyWorkId, BeWork, Context, WorkId}, +}; + +#[derive(Debug)] +struct MetaWork {} + +pub fn create_meta_work() -> Box { + Box::new(MetaWork {}) +} + +impl Work for MetaWork { + fn id(&self) -> AnyWorkId { + WorkId::Meta.into() + } + + fn read_access(&self) -> Access { + AccessBuilder::new() + .variant(FeWorkId::StaticMetadata) // For meta records + .build() + } + + /// Generate [meta](https://learn.microsoft.com/en-us/typography/opentype/spec/meta) + fn exec(&self, context: &Context) -> Result<(), Error> { + // Build from static metadata, if at least one record is provided. + let meta = { + let static_metadata = context.ir.static_metadata.get(); + let records = &static_metadata.misc.meta; + + if records.is_empty() { + None + } else { + // Instantiate DataMapRecords from tuples. + Some(Meta::new( + records + .iter() + .map(|(tag, metadata)| DataMapRecord::new(*tag, metadata.clone())) + .collect(), + )) + } + }; + + if let Some(meta) = meta { + context.meta.set(meta); + }; + + Ok(()) + } +} diff --git a/fontbe/src/orchestration.rs b/fontbe/src/orchestration.rs index e93462ac2..1cbd409e7 100644 --- a/fontbe/src/orchestration.rs +++ b/fontbe/src/orchestration.rs @@ -52,6 +52,7 @@ use write_fonts::{ hvar::Hvar, loca::LocaFormat, maxp::Maxp, + meta::Meta, mvar::Mvar, name::Name, os2::Os2, @@ -99,6 +100,7 @@ pub enum WorkId { LocaFormat, Marks, Maxp, + Meta, Mvar, Name, Os2, @@ -142,6 +144,7 @@ impl Identifier for WorkId { WorkId::LocaFormat => "BeLocaFormat", WorkId::Marks => "BeMarks", WorkId::Maxp => "BeMaxp", + WorkId::Meta => "BeMeta", WorkId::Mvar => "BeMvar", WorkId::Name => "BeName", WorkId::Os2 => "BeOs2", @@ -810,6 +813,7 @@ pub struct Context { pub loca: BeContextItem, pub loca_format: BeContextItem, pub maxp: BeContextItem, + pub meta: BeContextItem, pub name: BeContextItem, pub os2: BeContextItem, pub head: BeContextItem, @@ -848,6 +852,7 @@ impl Context { loca: self.loca.clone_with_acl(acl.clone()), loca_format: self.loca_format.clone_with_acl(acl.clone()), maxp: self.maxp.clone_with_acl(acl.clone()), + meta: self.meta.clone_with_acl(acl.clone()), name: self.name.clone_with_acl(acl.clone()), os2: self.os2.clone_with_acl(acl.clone()), head: self.head.clone_with_acl(acl.clone()), @@ -894,6 +899,7 @@ impl Context { persistent_storage.clone(), ), maxp: ContextItem::new(WorkId::Maxp.into(), acl.clone(), persistent_storage.clone()), + meta: ContextItem::new(WorkId::Meta.into(), acl.clone(), persistent_storage.clone()), name: ContextItem::new(WorkId::Name.into(), acl.clone(), persistent_storage.clone()), os2: ContextItem::new(WorkId::Os2.into(), acl.clone(), persistent_storage.clone()), head: ContextItem::new(WorkId::Head.into(), acl.clone(), persistent_storage.clone()), diff --git a/fontbe/src/paths.rs b/fontbe/src/paths.rs index 71a49c173..2346b2b68 100644 --- a/fontbe/src/paths.rs +++ b/fontbe/src/paths.rs @@ -89,6 +89,7 @@ impl Paths { WorkId::GatherBeKerning => self.build_dir.join("kern_gather.bin"), WorkId::Marks => self.build_dir.join("marks.bin"), WorkId::Maxp => self.build_dir.join("maxp.table"), + WorkId::Meta => self.build_dir.join("meta.table"), WorkId::Mvar => self.build_dir.join("mvar.table"), WorkId::Name => self.build_dir.join("name.table"), WorkId::Os2 => self.build_dir.join("os2.table"), diff --git a/fontc/src/lib.rs b/fontc/src/lib.rs index c8546a686..0d642fc3a 100644 --- a/fontc/src/lib.rs +++ b/fontc/src/lib.rs @@ -29,6 +29,7 @@ use fontbe::{ gvar::create_gvar_work, head::create_head_work, hvar::create_hvar_work, + meta::create_meta_work, metrics_and_limits::create_metric_and_limit_work, mvar::create_mvar_work, name::create_name_work, @@ -455,6 +456,12 @@ fn add_hvar_be_job(workload: &mut Workload) -> Result<(), Error> { Ok(()) } +fn add_meta_be_job(workload: &mut Workload) -> Result<(), Error> { + let work = create_meta_work().into(); + workload.add(work, workload.change_detector.static_metadata_ir_change()); + Ok(()) +} + fn add_mvar_be_job(workload: &mut Workload) -> Result<(), Error> { let work = create_mvar_work().into(); workload.add( @@ -516,6 +523,7 @@ pub fn create_workload( add_marks_be_job(&mut workload)?; add_metric_and_limits_job(&mut workload)?; add_hvar_be_job(&mut workload)?; + add_meta_be_job(&mut workload)?; add_mvar_be_job(&mut workload)?; add_name_be_job(&mut workload)?; add_os2_be_job(&mut workload)?; diff --git a/fontc/src/timing.rs b/fontc/src/timing.rs index 57d75a9df..9088fdf62 100644 --- a/fontc/src/timing.rs +++ b/fontc/src/timing.rs @@ -156,6 +156,7 @@ fn short_name(id: &AnyWorkId) -> &'static str { AnyWorkId::Be(BeWorkIdentifier::LocaFormat) => "loca-fmt", AnyWorkId::Be(BeWorkIdentifier::Marks) => "Marks", AnyWorkId::Be(BeWorkIdentifier::Maxp) => "maxp", + AnyWorkId::Be(BeWorkIdentifier::Meta) => "meta", AnyWorkId::Be(BeWorkIdentifier::Mvar) => "MVAR", AnyWorkId::Be(BeWorkIdentifier::Name) => "name", AnyWorkId::Be(BeWorkIdentifier::Os2) => "OS/2", diff --git a/fontir/src/ir.rs b/fontir/src/ir.rs index 63295149b..653dc6b0f 100644 --- a/fontir/src/ir.rs +++ b/fontir/src/ir.rs @@ -14,7 +14,7 @@ use ordered_float::OrderedFloat; use serde::{de::Error as _, Deserialize, Serialize}; use smol_str::SmolStr; use write_fonts::{ - tables::{gdef::GlyphClassDef, os2::SelectionFlags}, + tables::{gdef::GlyphClassDef, meta::Metadata, os2::SelectionFlags}, types::{GlyphId16, NameId, Tag}, OtRound, }; @@ -130,6 +130,9 @@ pub struct MiscMetadata { // Allows source to explicitly control bits. pub codepage_range_bits: Option>, + + // See + pub meta: Vec<(Tag, Metadata)>, } /// PANOSE bytes @@ -472,6 +475,7 @@ impl StaticMetadata { panose: None, unicode_range_bits: None, codepage_range_bits: None, + meta: Default::default(), }, }) } @@ -2134,6 +2138,7 @@ mod tests { panose: None, unicode_range_bits: None, codepage_range_bits: None, + meta: vec![(Tag::new(b"abcd"), Metadata::Other("efghijkl".into()))], }, number_values: Default::default(), } From 93cdcedeea08310ff7d5b0370e7bcdb355f7f3f0 Mon Sep 17 00:00:00 2001 From: Harry Dalton Date: Wed, 20 Nov 2024 23:01:31 +0000 Subject: [PATCH 2/7] Add initial compilation of Glyphs' "meta Table" custom parameter This is sufficient to resolve a few ttx table diffs on fontc_crater, but needs refinement. --- glyphs-reader/src/font.rs | 33 ++++++++++++++++++++++++++++++ glyphs2fontir/src/source.rs | 40 ++++++++++++++++++++++++++++++++++++- 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/glyphs-reader/src/font.rs b/glyphs-reader/src/font.rs index 80c5d4f4e..043faad36 100644 --- a/glyphs-reader/src/font.rs +++ b/glyphs-reader/src/font.rs @@ -89,6 +89,9 @@ pub struct Font { pub unicode_range_bits: Option>, pub codepage_range_bits: Option>, pub panose: Option>, + + /// Read from "meta Table" custom parameter. + pub meta: Vec<(String, String)>, } /// master id => { (name or class, name or class) => adjustment } @@ -415,6 +418,13 @@ impl CustomParameters { }) } + fn meta(&self) -> Option<&Vec> { + let Some(CustomParameterValue::MetaRecords(records)) = self.get("meta Table") else { + return None; + }; + Some(records) + } + fn fs_type(&self) -> Option<&Vec> { let Some(CustomParameterValue::FsType(bits)) = self.get("fsType") else { return None; @@ -462,6 +472,7 @@ enum CustomParameterValue { UnicodeRange(Vec), CodepageRange(Vec), Panose(Vec), + MetaRecords(Vec), } /// Hand-parse these because they take multiple shapes @@ -581,6 +592,12 @@ impl FromPlist for CustomParameters { }; value = Some(CustomParameterValue::Panose(tokenizer.parse()?)); } + _ if name == Some(String::from("meta Table")) => { + let Token::OpenParen = peek else { + return Err(Error::UnexpectedChar('(')); + }; + value = Some(CustomParameterValue::MetaRecords(tokenizer.parse()?)); + } _ => tokenizer.skip_rec()?, } // once we've seen the value we're always done @@ -629,6 +646,12 @@ pub struct AxisMapping { user_to_design: Vec<(OrderedFloat, OrderedFloat)>, } +#[derive(Default, Debug, Clone, PartialEq, Eq, Hash, FromPlist)] +pub struct MetaRecord { + tag: String, + data: String, +} + impl FromPlist for AxisMapping { fn parse(tokenizer: &mut Tokenizer<'_>) -> Result { let tag = tokenizer.parse()?; @@ -2423,6 +2446,15 @@ impl TryFrom for Font { }) .collect(); + // If any meta records were provided, convert to tuples for IR. + let meta = from + .custom_parameters + .meta() + .into_iter() + .flatten() + .map(|record| (record.tag.clone(), record.data.clone())) + .collect(); + Ok(Font { units_per_em, fs_type, @@ -2465,6 +2497,7 @@ impl TryFrom for Font { unicode_range_bits, codepage_range_bits, panose, + meta, }) } } diff --git a/glyphs2fontir/src/source.rs b/glyphs2fontir/src/source.rs index 3f2aa7ae1..d0f20117c 100644 --- a/glyphs2fontir/src/source.rs +++ b/glyphs2fontir/src/source.rs @@ -32,7 +32,11 @@ use glyphs_reader::{ use ordered_float::OrderedFloat; use smol_str::SmolStr; use write_fonts::{ - tables::{gdef::GlyphClassDef, os2::SelectionFlags}, + tables::{ + gdef::GlyphClassDef, + meta::{Metadata, ScriptLangTag, DLNG, SLNG}, + os2::SelectionFlags, + }, types::{NameId, Tag}, }; @@ -130,6 +134,8 @@ impl GlyphsIrSource { unicode_range_bits: font.unicode_range_bits.clone(), codepage_range_bits: font.codepage_range_bits.clone(), panose: font.panose.clone(), + // TODO: Is this correct? + meta: font.meta.clone(), }; state.track_memory("/font_master".to_string(), &font)?; Ok(state) @@ -182,6 +188,8 @@ impl GlyphsIrSource { unicode_range_bits: None, codepage_range_bits: None, panose: None, + // TODO: Is this correct? + meta: Default::default(), }; state.track_memory("/font_master".to_string(), &font)?; Ok(state) @@ -491,6 +499,36 @@ impl Work for StaticMetadataWork { }) .or(static_metadata.misc.created); + // TODO: Use idiomatic error handling. + // TODO: This diverges from glyphsLib, which must convert a list of + // records into UFO's intermediate dict representation; is this + // actually an improvement, or a breaking change? What does Glyphs + // do? + // https://github.com/googlefonts/glyphsLib/blob/74c63244fdbef1da540d646b0784ae6d2c3ca834/Lib/glyphsLib/builder/custom_params.py#L568-L584 + static_metadata.misc.meta = font + .meta + .iter() + .map(|(tag, data)| { + let tag = Tag::new_checked(tag.as_bytes()) + .expect("Malformed tag in meta custom parameter"); + + // Glyphs expects ScriptLangTag formatting for DLNG and SLNG. + // https://handbook.glyphsapp.com/custom-parameter-descriptions/ + let metadata = match tag { + DLNG | SLNG => Metadata::ScriptLangTags( + data.split(",") + .map(Into::into) + .map(ScriptLangTag::new) + .collect::>() + .expect("Invalid ScriptLangTag in meta custom parameter"), + ), + _ => Metadata::Other(data.clone().into_bytes()), + }; + + (tag, metadata) + }) + .collect(); + context.static_metadata.set(static_metadata); let glyph_order = font From 726501aba2f091c20f23269aff1e34254160fbeb Mon Sep 17 00:00:00 2001 From: Harry Dalton Date: Wed, 20 Nov 2024 23:16:37 +0000 Subject: [PATCH 3/7] Add initial compilation of UFO's "public.openTypeMeta" property This is heavily WIP; it needs a tidy and is untested, and should be considered an initial draft only. --- ufo2fontir/src/source.rs | 127 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 126 insertions(+), 1 deletion(-) diff --git a/ufo2fontir/src/source.rs b/ufo2fontir/src/source.rs index 1ed893b08..af8984dfb 100644 --- a/ufo2fontir/src/source.rs +++ b/ufo2fontir/src/source.rs @@ -29,7 +29,11 @@ use norad::{ fontinfo::StyleMapStyle, }; use write_fonts::{ - tables::{gdef::GlyphClassDef, os2::SelectionFlags}, + tables::{ + gdef::GlyphClassDef, + meta::{Metadata, ScriptLangTag, DLNG, SLNG}, + os2::SelectionFlags, + }, types::{InvalidTag, NameId, Tag}, OtRound, }; @@ -664,6 +668,125 @@ fn postscript_names(lib_plist: &plist::Dictionary) -> Result Result, BadSource> { + const OPENTYPE_CATEGORIES: &str = "public.openTypeCategories"; + + // Registered tags that UFOv3 specifies but that are not in write-fonts + const APPL: Tag = Tag::new(b"appl"); + const BILD: Tag = Tag::new(b"bild"); + + let property = lib_plist.get(OPENTYPE_CATEGORIES); + + // TODO: Use idiomatic error handling. + // TODO: Simplify. + + match property.map(plist::Value::as_dictionary) { + // Dictionary is present, interpret: + Some(Some(dictionary)) => dictionary + .iter() + .map(|(key, value)| { + // Parse each tag, and metadata as appropriate from tag. + let tag = Tag::new_checked(key.as_bytes()).map_err(|err| { + BadSource::custom( + "lib.plist", + format!("meta record has an invalid tag '{}': {}", key, err), + ) + })?; + + let metadata = match tag { + // "If present, [...] the registered tags dlng and slng have + // their values each stored as an array of ScriptLangTag + // strings" + DLNG | SLNG => Metadata::ScriptLangTags({ + // Interpret and validate as a plist array: + let array = value.as_array().ok_or_else(|| { + BadSource::custom( + "lib.plist", + format!( + "meta {} record must contain an array of ScriptLangTag strings", + tag + ), + ) + })?; + + // ...and as a plist array of strings + let strings = array + .iter() + .map(plist::Value::as_string) + .map(|value| { + value.ok_or_else(|| { + BadSource::custom( + "lib.plist", + format!( + "meta {} record array must only contain values", + tag + ), + ) + }) + }) + .collect::, _>>()?; + + // ...and as a plist array of ScriptLangTags + strings + .into_iter() + .map(|string| { + ScriptLangTag::new(string.into()).map_err(|err| { + BadSource::custom( + "lib.plist", + format!( + "meta {} record contains an invalid \ + ScriptLangTag '{}': {}", + tag, string, err + ), + ) + }) + }) + .collect::>()? + }), + // "If present, the registered tags appl and bild have their + // values stored as data" + APPL | BILD => Metadata::Other( + value + .as_data() + .ok_or_else(|| { + BadSource::custom( + "lib.plist", + format!("meta table {} record must contain ", tag), + ) + })? + .to_vec(), + ), + // "Any private tag must have its value stored as data or + // string" + _ => { + let other: Vec = match value { + plist::Value::Data(vec) => Ok(vec.to_owned()), + plist::Value::String(string) => Ok(string.as_bytes().to_vec()), + _ => Err(BadSource::custom( + "lib.plist", + "Private meta table records must be or ", + )), + }?; + + Metadata::Other(other) + } + }; + + Ok((tag, metadata)) + }) + .collect(), + + // Key is present but was not a dictionary, error: + Some(None) => Err(BadSource::custom( + "lib.plist", + "public.postscriptNames must be a dictionary", + )), + // Key was not present, use default: + None => Ok(Default::default()), + } +} + fn units_per_em<'a>(font_infos: impl Iterator) -> Result { const MIN_UPEM: f64 = 16.0; const MAX_UPEM: f64 = 16384.0; @@ -1008,6 +1131,8 @@ impl Work for StaticMetadataWork { try_parse_date(font_info_at_default.open_type_head_created.as_ref()) .or(static_metadata.misc.created); + static_metadata.misc.meta = meta_records(&lib_plist)?; + context.preliminary_glyph_order.set(glyph_order); context.static_metadata.set(static_metadata); Ok(()) From 4588d8f3728f08eca6bcf14f5497e84cd6ed9a26 Mon Sep 17 00:00:00 2001 From: Harry Dalton Date: Wed, 20 Nov 2024 23:22:04 +0000 Subject: [PATCH 4/7] Replace excessive nesting with early exit --- ufo2fontir/src/source.rs | 203 ++++++++++++++++++++------------------- 1 file changed, 102 insertions(+), 101 deletions(-) diff --git a/ufo2fontir/src/source.rs b/ufo2fontir/src/source.rs index af8984dfb..96cfee1b3 100644 --- a/ufo2fontir/src/source.rs +++ b/ufo2fontir/src/source.rs @@ -670,121 +670,122 @@ fn postscript_names(lib_plist: &plist::Dictionary) -> Result Result, BadSource> { + // TODO: Use idiomatic error handling. + // TODO: Simplify. + const OPENTYPE_CATEGORIES: &str = "public.openTypeCategories"; // Registered tags that UFOv3 specifies but that are not in write-fonts const APPL: Tag = Tag::new(b"appl"); const BILD: Tag = Tag::new(b"bild"); - let property = lib_plist.get(OPENTYPE_CATEGORIES); + let Some(property) = lib_plist.get(OPENTYPE_CATEGORIES) else { + // Key was not present, use default: + return Ok(Default::default()); + }; - // TODO: Use idiomatic error handling. - // TODO: Simplify. + let Some(dictionary) = property.as_dictionary() else { + // Key is present but was not a dictionary, error. + return Err(BadSource::custom( + "lib.plist", + "public.postscriptNames must be a dictionary", + )); + }; - match property.map(plist::Value::as_dictionary) { - // Dictionary is present, interpret: - Some(Some(dictionary)) => dictionary - .iter() - .map(|(key, value)| { - // Parse each tag, and metadata as appropriate from tag. - let tag = Tag::new_checked(key.as_bytes()).map_err(|err| { - BadSource::custom( - "lib.plist", - format!("meta record has an invalid tag '{}': {}", key, err), - ) - })?; - - let metadata = match tag { - // "If present, [...] the registered tags dlng and slng have - // their values each stored as an array of ScriptLangTag - // strings" - DLNG | SLNG => Metadata::ScriptLangTags({ - // Interpret and validate as a plist array: - let array = value.as_array().ok_or_else(|| { - BadSource::custom( - "lib.plist", - format!( - "meta {} record must contain an array of ScriptLangTag strings", - tag - ), - ) - })?; - - // ...and as a plist array of strings - let strings = array - .iter() - .map(plist::Value::as_string) - .map(|value| { - value.ok_or_else(|| { - BadSource::custom( - "lib.plist", - format!( - "meta {} record array must only contain values", - tag - ), - ) - }) - }) - .collect::, _>>()?; - - // ...and as a plist array of ScriptLangTags - strings - .into_iter() - .map(|string| { - ScriptLangTag::new(string.into()).map_err(|err| { - BadSource::custom( - "lib.plist", - format!( - "meta {} record contains an invalid \ - ScriptLangTag '{}': {}", - tag, string, err - ), - ) - }) + // Dictionary is present, interpret. + dictionary + .iter() + .map(|(key, value)| { + // Parse each tag, and metadata as appropriate from tag. + let tag = Tag::new_checked(key.as_bytes()).map_err(|err| { + BadSource::custom( + "lib.plist", + format!("meta record has an invalid tag '{}': {}", key, err), + ) + })?; + + let metadata = match tag { + // "If present, [...] the registered tags dlng and slng have + // their values each stored as an array of ScriptLangTag + // strings" + DLNG | SLNG => Metadata::ScriptLangTags({ + // Interpret and validate as a plist array: + let array = value.as_array().ok_or_else(|| { + BadSource::custom( + "lib.plist", + format!( + "meta {} record must contain an array of ScriptLangTag strings", + tag + ), + ) + })?; + + // ...and as a plist array of strings + let strings = array + .iter() + .map(plist::Value::as_string) + .map(|value| { + value.ok_or_else(|| { + BadSource::custom( + "lib.plist", + format!( + "meta {} record array must only contain values", + tag + ), + ) }) - .collect::>()? - }), - // "If present, the registered tags appl and bild have their - // values stored as data" - APPL | BILD => Metadata::Other( - value - .as_data() - .ok_or_else(|| { + }) + .collect::, _>>()?; + + // ...and as a plist array of ScriptLangTags + strings + .into_iter() + .map(|string| { + ScriptLangTag::new(string.into()).map_err(|err| { BadSource::custom( "lib.plist", - format!("meta table {} record must contain ", tag), + format!( + "meta {} record contains an invalid \ + ScriptLangTag '{}': {}", + tag, string, err + ), ) - })? - .to_vec(), - ), - // "Any private tag must have its value stored as data or - // string" - _ => { - let other: Vec = match value { - plist::Value::Data(vec) => Ok(vec.to_owned()), - plist::Value::String(string) => Ok(string.as_bytes().to_vec()), - _ => Err(BadSource::custom( + }) + }) + .collect::>()? + }), + // "If present, the registered tags appl and bild have their + // values stored as data" + APPL | BILD => Metadata::Other( + value + .as_data() + .ok_or_else(|| { + BadSource::custom( "lib.plist", - "Private meta table records must be or ", - )), - }?; - - Metadata::Other(other) - } - }; - - Ok((tag, metadata)) - }) - .collect(), + format!("meta table {} record must contain ", tag), + ) + })? + .to_vec(), + ), + // "Any private tag must have its value stored as data or + // string" + _ => { + let other: Vec = match value { + plist::Value::Data(vec) => Ok(vec.to_owned()), + plist::Value::String(string) => Ok(string.as_bytes().to_vec()), + _ => Err(BadSource::custom( + "lib.plist", + "Private meta table records must be or ", + )), + }?; + + Metadata::Other(other) + } + }; - // Key is present but was not a dictionary, error: - Some(None) => Err(BadSource::custom( - "lib.plist", - "public.postscriptNames must be a dictionary", - )), - // Key was not present, use default: - None => Ok(Default::default()), - } + Ok((tag, metadata)) + }) + .collect() } fn units_per_em<'a>(font_infos: impl Iterator) -> Result { From b8b45435824d752025e6207d5b02ddb80d87fffd Mon Sep 17 00:00:00 2001 From: Harry Dalton Date: Thu, 21 Nov 2024 00:10:26 +0000 Subject: [PATCH 5/7] Fix copy-paste typo for `meta` key name --- ufo2fontir/src/source.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ufo2fontir/src/source.rs b/ufo2fontir/src/source.rs index 96cfee1b3..b244c9a20 100644 --- a/ufo2fontir/src/source.rs +++ b/ufo2fontir/src/source.rs @@ -673,13 +673,13 @@ fn meta_records(lib_plist: &plist::Dictionary) -> Result, B // TODO: Use idiomatic error handling. // TODO: Simplify. - const OPENTYPE_CATEGORIES: &str = "public.openTypeCategories"; + const OPENTYPE_META: &str = "public.openTypeMeta"; // Registered tags that UFOv3 specifies but that are not in write-fonts const APPL: Tag = Tag::new(b"appl"); const BILD: Tag = Tag::new(b"bild"); - let Some(property) = lib_plist.get(OPENTYPE_CATEGORIES) else { + let Some(property) = lib_plist.get(OPENTYPE_META) else { // Key was not present, use default: return Ok(Default::default()); }; @@ -688,7 +688,7 @@ fn meta_records(lib_plist: &plist::Dictionary) -> Result, B // Key is present but was not a dictionary, error. return Err(BadSource::custom( "lib.plist", - "public.postscriptNames must be a dictionary", + "public.openTypeMeta must be a dictionary", )); }; From c25d71058dfa14864e1059791ef911726d996940 Mon Sep 17 00:00:00 2001 From: Harry Dalton Date: Thu, 21 Nov 2024 00:16:03 +0000 Subject: [PATCH 6/7] Use an automatic link to fix bare URL Thanks cargo doc! --- ufo2fontir/src/source.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ufo2fontir/src/source.rs b/ufo2fontir/src/source.rs index b244c9a20..08d2fd5bf 100644 --- a/ufo2fontir/src/source.rs +++ b/ufo2fontir/src/source.rs @@ -668,7 +668,7 @@ fn postscript_names(lib_plist: &plist::Dictionary) -> Result fn meta_records(lib_plist: &plist::Dictionary) -> Result, BadSource> { // TODO: Use idiomatic error handling. // TODO: Simplify. From 4b779687f82d5850624b5a41af47d150b3357e5c Mon Sep 17 00:00:00 2001 From: Harry Dalton Date: Thu, 21 Nov 2024 00:24:43 +0000 Subject: [PATCH 7/7] Expect `Meta` to be part of compilation work to fix failing test --- fontc/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/fontc/src/lib.rs b/fontc/src/lib.rs index 0d642fc3a..006b293f8 100644 --- a/fontc/src/lib.rs +++ b/fontc/src/lib.rs @@ -830,6 +830,7 @@ mod tests { BeWorkIdentifier::LocaFormat.into(), BeWorkIdentifier::Marks.into(), BeWorkIdentifier::Maxp.into(), + BeWorkIdentifier::Meta.into(), BeWorkIdentifier::Mvar.into(), BeWorkIdentifier::Name.into(), BeWorkIdentifier::Os2.into(),