diff --git a/fontbe/src/post.rs b/fontbe/src/post.rs index 181744aaa..33256308c 100644 --- a/fontbe/src/post.rs +++ b/fontbe/src/post.rs @@ -7,6 +7,7 @@ use fontir::orchestration::WorkId as FeWorkId; use write_fonts::{ tables::post::Post, types::{FWord, Fixed}, + OtRound, }; use crate::{ @@ -29,6 +30,7 @@ impl Work for PostWork { fn read_access(&self) -> Access { Access::Set(HashSet::from([ FeWorkId::StaticMetadata.into(), + FeWorkId::GlobalMetrics.into(), FeWorkId::GlyphOrder.into(), ])) } @@ -39,6 +41,11 @@ impl Work for PostWork { // TODO optionally drop glyph names with format 3.0. // TODO a more serious post let static_metadata = context.ir.static_metadata.get(); + let metrics = context + .ir + .global_metrics + .get() + .at(static_metadata.default_location()); let postscript_names = &static_metadata.postscript_names; let glyph_order = context.ir.glyph_order.get(); let mut post = Post::new_v2( @@ -47,8 +54,8 @@ impl Work for PostWork { .map(|g| postscript_names.get(g).unwrap_or(g).as_str()), ); post.italic_angle = Fixed::from_f64(static_metadata.italic_angle.into_inner()); - post.underline_position = FWord::new(static_metadata.misc.underline_position.0 as i16); - post.underline_thickness = FWord::new(static_metadata.misc.underline_thickness.0 as i16); + post.underline_position = FWord::new(metrics.underline_position.ot_round()); + post.underline_thickness = FWord::new(metrics.underline_thickness.ot_round()); context.post.set_unconditionally(post.into()); Ok(()) } diff --git a/fontir/src/ir.rs b/fontir/src/ir.rs index b400fd38e..28aa9693b 100644 --- a/fontir/src/ir.rs +++ b/fontir/src/ir.rs @@ -94,9 +94,6 @@ pub struct MiscMetadata { /// See pub vendor_id: Tag, - pub underline_thickness: OrderedFloat, - pub underline_position: OrderedFloat, - /// UFO appears to allow negative major versions. /// /// See @@ -294,8 +291,6 @@ impl StaticMetadata { fs_type: None, // default is, sigh, inconsistent across source formats selection_flags: Default::default(), vendor_id: DEFAULT_VENDOR_ID_TAG, - underline_thickness: 0.0.into(), - underline_position: 0.0.into(), // https://github.com/googlefonts/ufo2ft/blob/0d2688cd847d003b41104534d16973f72ef26c40/Lib/ufo2ft/fontInfoData.py#L353-L354 version_major: 0, version_minor: 0, @@ -339,6 +334,8 @@ pub enum GlobalMetric { CaretSlopeRise, CaretSlopeRun, CaretOffset, + UnderlineThickness, + UnderlinePosition, XHeight, YSubscriptXSize, YSubscriptYSize, @@ -450,6 +447,14 @@ impl GlobalMetrics { set(GlobalMetric::YStrikeoutSize, 0.05 * units_per_em as f32); set(GlobalMetric::YStrikeoutPosition, x_height * 0.6); + // ufo2ft and Glyphs.app have different defaults for the post.underlinePosition: + // the former uses 0.075*UPEM whereas the latter 0.1*UPEM (both use the same + // underlineThickness 0.05*UPEM). We prefer to match Glyphs.app as more widely used. + // https://github.com/googlefonts/ufo2ft/blob/main/Lib/ufo2ft/fontInfoData.py#L313-L322 + // https://github.com/googlefonts/glyphsLib/blob/main/Lib/glyphsLib/builder/custom_params.py#L1116-L1125 + set(GlobalMetric::UnderlineThickness, 0.05 * units_per_em as f32); + set(GlobalMetric::UnderlinePosition, -0.1 * units_per_em as f32); + metrics } @@ -520,6 +525,8 @@ impl GlobalMetrics { hhea_ascender: self.get(GlobalMetric::HheaAscender, pos), hhea_descender: self.get(GlobalMetric::HheaDescender, pos), hhea_line_gap: self.get(GlobalMetric::HheaLineGap, pos), + underline_thickness: self.get(GlobalMetric::UnderlineThickness, pos), + underline_position: self.get(GlobalMetric::UnderlinePosition, pos), } } @@ -559,6 +566,8 @@ pub struct GlobalMetricsInstance { pub hhea_ascender: OrderedFloat, pub hhea_descender: OrderedFloat, pub hhea_line_gap: OrderedFloat, + pub underline_thickness: OrderedFloat, + pub underline_position: OrderedFloat, } /// Helps accumulate 'name' values. @@ -1580,8 +1589,6 @@ mod tests { fs_type: None, selection_flags: SelectionFlags::default(), vendor_id: Tag::from_be_bytes(*b"DUCK"), - underline_thickness: 0.15.into(), - underline_position: 16.0.into(), version_major: 42, version_minor: 24, head_flags: 42, diff --git a/glyphs-reader/src/font.rs b/glyphs-reader/src/font.rs index 7e0dbf862..448a4c4e7 100644 --- a/glyphs-reader/src/font.rs +++ b/glyphs-reader/src/font.rs @@ -293,6 +293,15 @@ impl CustomParameters { Some(*i) } + fn float(&self, name: &str) -> Option> { + let value = self.get(name)?; + match value { + CustomParameterValue::Int(i) => Some((*i as f64).into()), + CustomParameterValue::Float(f) => Some(*f), + _ => None, + } + } + fn bool(&self, name: &str) -> Option { self.int(name).map(|v| v == 1) } @@ -734,6 +743,8 @@ pub struct FontMaster { pub hhea_ascender: Option, pub hhea_descender: Option, pub hhea_line_gap: Option, + pub underline_thickness: Option>, + pub underline_position: Option>, } impl FontMaster { @@ -1907,6 +1918,8 @@ impl TryFrom for Font { hhea_ascender: m.custom_parameters.int("hheaAscender"), hhea_descender: m.custom_parameters.int("hheaDescender"), hhea_line_gap: m.custom_parameters.int("hheaLineGap"), + underline_thickness: m.custom_parameters.float("underlineThickness"), + underline_position: m.custom_parameters.float("underlinePosition"), }) .collect(); diff --git a/glyphs2fontir/src/source.rs b/glyphs2fontir/src/source.rs index 599b852cc..41e7c9dc8 100644 --- a/glyphs2fontir/src/source.rs +++ b/glyphs2fontir/src/source.rs @@ -385,10 +385,6 @@ impl Work for StaticMetadataWork { // Default per static_metadata.misc.fs_type = font.fs_type.or(Some(1 << 3)); - // - static_metadata.misc.underline_thickness = 50.0.into(); - static_metadata.misc.underline_position = (-100.0).into(); - static_metadata.misc.version_major = font.version_major; static_metadata.misc.version_minor = font.version_minor; @@ -497,6 +493,16 @@ impl Work for GlobalMetricWork { pos.clone(), master.hhea_line_gap.map(|v| v as f64), ); + metrics.set_if_some( + GlobalMetric::UnderlineThickness, + pos.clone(), + master.underline_thickness, + ); + metrics.set_if_some( + GlobalMetric::UnderlinePosition, + pos.clone(), + master.underline_position, + ) } context.global_metrics.set(metrics); @@ -1427,6 +1433,8 @@ mod tests { hhea_ascender: 1000.0.into(), hhea_descender: (-200.0).into(), hhea_line_gap: 0.0.into(), + underline_thickness: 50.0.into(), + underline_position: (-100.0).into(), ..Default::default() }, default_metrics @@ -1442,20 +1450,6 @@ mod tests { ); } - #[test] - fn default_underline_settings() { - let (_, context) = build_static_metadata(glyphs3_dir().join("Oswald-O.glyphs")); - let static_metadata = context.static_metadata.get(); - assert_eq!( - (1000, 50.0, -100.0), - ( - static_metadata.units_per_em, - static_metadata.misc.underline_thickness.0, - static_metadata.misc.underline_position.0 - ) - ); - } - #[test] fn build_glyph_contour_ir_containing_qcurves() { let glyph_name = "i"; diff --git a/ufo2fontir/src/source.rs b/ufo2fontir/src/source.rs index 93c8a7fc6..3c790f8a2 100644 --- a/ufo2fontir/src/source.rs +++ b/ufo2fontir/src/source.rs @@ -847,18 +847,6 @@ impl Work for StaticMetadataWork { // static_metadata.misc.fs_type = Some(1 << 2); - // - static_metadata.misc.underline_thickness = font_info_at_default - .postscript_underline_thickness - .map(|v| v as f32) - .unwrap_or(0.05 * units_per_em as f32) - .into(); - static_metadata.misc.underline_position = font_info_at_default - .postscript_underline_position - .map(|v| v as f32) - .unwrap_or(-0.075 * units_per_em as f32) - .into(); - static_metadata.misc.version_major = font_info_at_default .version_major .unwrap_or(static_metadata.misc.version_major); @@ -923,6 +911,14 @@ impl Work for GlobalMetricsWork { .map(|v| v as f32), static_metadata.italic_angle.into_inner(), ); + // ufo2ft default underline position differs from fontir/Glyphs.app's so + // we override it here. + // https://github.com/googlefonts/ufo2ft/blob/a421acc/Lib/ufo2ft/fontInfoData.py#L319-L322 + metrics.set( + GlobalMetric::UnderlinePosition, + static_metadata.default_location().clone(), + -0.075 * static_metadata.units_per_em as f32, + ); for source in self .designspace .sources @@ -993,6 +989,16 @@ impl Work for GlobalMetricsWork { pos.clone(), font_info.open_type_hhea_caret_offset.map(|v| v as f64), ); + metrics.set_if_some( + GlobalMetric::UnderlineThickness, + pos.clone(), + font_info.postscript_underline_thickness, + ); + metrics.set_if_some( + GlobalMetric::UnderlinePosition, + pos.clone(), + font_info.postscript_underline_position, + ); } trace!("{:#?}", metrics); @@ -1726,6 +1732,8 @@ mod tests { hhea_ascender: 1194.0.into(), hhea_descender: (-290.0).into(), hhea_line_gap: 43.0.into(), + underline_thickness: 50.0.into(), + underline_position: (-75.0).into(), ..Default::default() }, default_metrics @@ -1784,20 +1792,6 @@ mod tests { ); } - #[test] - fn default_underline_settings() { - let (_, context) = build_static_metadata("wght_var.designspace", default_test_flags()); - let static_metadata = &context.static_metadata.get(); - assert_eq!( - (1000, 50.0, -75.0), - ( - static_metadata.units_per_em, - static_metadata.misc.underline_thickness.0, - static_metadata.misc.underline_position.0 - ) - ); - } - #[test] fn groups_renamed_to_match_master() { let (_, context) = build_kerning("wght_var.designspace");