Skip to content
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

Name 'standard' axes to match fonttools #1032

Merged
merged 2 commits into from
Oct 14, 2024
Merged

Name 'standard' axes to match fonttools #1032

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b34ebc6
add test and file containing 'standard' axis names
anthrotype Oct 14, 2024
7813a68
use Axis::ui_label_name() for axes names to match fonttools standard …
anthrotype Oct 14, 2024
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
6 changes: 3 additions & 3 deletions fontbe/src/fvar.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ fn generate_fvar(static_metadata: &StaticMetadata) -> Option<Fvar> {
// To match fontmake we should use the font-specific name range and not reuse
// a well-known name, even if the name matches.
.filter(|(key, _)| key.name_id.to_u16() > 255)
.map(|(key, name)| (name, key.name_id))
.map(|(key, name)| (name.as_str(), key.name_id))
.collect();

let axes_and_instances = AxisInstanceArrays::new(
Expand All @@ -50,7 +50,7 @@ fn generate_fvar(static_metadata: &StaticMetadata) -> Option<Fvar> {
min_value: ir_axis.min.into(),
default_value: ir_axis.default.into(),
max_value: ir_axis.max.into(),
axis_name_id: *reverse_names.get(&ir_axis.name).unwrap(),
axis_name_id: *reverse_names.get(ir_axis.ui_label_name()).unwrap(),
..Default::default()
};
if ir_axis.hidden {
Expand All @@ -63,7 +63,7 @@ fn generate_fvar(static_metadata: &StaticMetadata) -> Option<Fvar> {
.named_instances
.iter()
.map(|ni| InstanceRecord {
subfamily_name_id: *reverse_names.get(&ni.name).unwrap(),
subfamily_name_id: *reverse_names.get(ni.name.as_str()).unwrap(),
coordinates: static_metadata
.axes
.iter()
Expand Down
4 changes: 2 additions & 2 deletions fontbe/src/stat.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl Work<Context, AnyWorkId, Error> for StatWork {
// To match fontmake we should use the font-specific name range and not reuse
// a well-known name, even if the name matches.
.filter(|(key, _)| key.name_id.to_u16() > 255)
.map(|(key, name)| (name, key.name_id))
.map(|(key, name)| (name.as_str(), key.name_id))
.collect();

context.stat.set_unconditionally(
Expand All @@ -62,7 +62,7 @@ impl Work<Context, AnyWorkId, Error> for StatWork {
.enumerate()
.map(|(idx, a)| AxisRecord {
axis_tag: a.tag,
axis_name_id: *reverse_names.get(&a.name).unwrap(),
axis_name_id: *reverse_names.get(a.ui_label_name()).unwrap(),
axis_ordering: idx as u16,
})
.collect::<Vec<_>>()
Expand Down
45 changes: 45 additions & 0 deletions fontc/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1729,6 +1729,51 @@ mod tests {
})
}

#[test]
fn standard_axis_names() {
// test that we match fonttools' naming of standard axes
// https://github.com/googlefonts/fontc/issues/1020
let result = TestCompile::compile_source("glyphs3/StandardAxisNames.glyphs");
let static_metadata = result.fe_context.static_metadata.get();

assert_eq!(
vec![
"weight".to_string(),
"width".to_string(),
"italic".to_string(),
"slant".to_string(),
"optical".to_string(),
"foobarbaz".to_string(),
],
static_metadata
.axes
.iter()
.map(|axis| axis.name.clone())
.collect::<Vec<_>>()
);

let font = result.font();
let name = font.name().unwrap();
let fvar = font.fvar().unwrap();

assert_eq!(
vec![
"Weight".to_string(),
"Width".to_string(),
"Italic".to_string(),
"Slant".to_string(),
"Optical Size".to_string(),
// This axis is not 'standard' so its UI label was not renamed
"foobarbaz".to_string(),
],
fvar.axes()
.unwrap()
.iter()
.map(|axis| resolve_name(&name, axis.axis_name_id()).unwrap())
.collect::<Vec<_>>()
)
}

fn assert_named_instances(source: &str, expected: Vec<(String, Vec<(&str, f32)>)>) {
let result = TestCompile::compile_source(source);
let font = result.font();
Expand Down
30 changes: 30 additions & 0 deletions fontdrasil/src/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,36 @@ impl Axis {
pub fn default_converter(&self) -> CoordConverter {
CoordConverter::default_normalization(self.min, self.default, self.max)
}

/// Display name for the axis.
///
/// Some frontends (e.g. designspace) support the notion of an axis' UI label name distinct
/// from the axis name; the former is used for displaying the axis in UI and can be
/// localised, whereas the latter is only used for internal cross-references.
/// In Designspace documents, these are stored in the axes' `<labelname>` elements.
/// FontTools uses these to build the name records associated with axis names referenced
/// by fvar and STAT tables, or else falls back to the axis.name. fontc doesn't know about
/// them until norad is able to parse them (<https://github.com/linebender/norad/issues/323>).
/// But even when the labelnames are ommited, there's a special group of registered
/// axis names that were common in old MutatorMath source files before the `<labelname>`
/// element itself got standardised, which continue to receive a special treatment in
/// fonttools: i.e., the lowercase, shortened name gets replaced with a title-case,
/// expanded one (e.g. 'weight' => 'Weight', 'optical' => 'Optical Size' etc.).
/// For the sake of matching fontmake (which uses fonttools), we do the same here.
///
/// For additional info see: <https://github.com/googlefonts/fontc/issues/1020>
pub fn ui_label_name(&self) -> &str {
// TODO: support localised labelnames when norad does
let axis_name = self.name.as_str();
match axis_name {
"weight" => "Weight",
"width" => "Width",
"slant" => "Slant",
"optical" => "Optical Size",
"italic" => "Italic",
_ => axis_name,
}
}
}

// OS/2 width class
Expand Down
6 changes: 3 additions & 3 deletions fontir/src/ir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,14 +422,14 @@ impl StaticMetadata {

variable_axes
.iter()
.map(|axis| &axis.name)
.chain(named_instances.iter().map(|ni| &ni.name))
.map(|axis| axis.ui_label_name())
.chain(named_instances.iter().map(|ni| ni.name.as_ref()))
.for_each(|name| {
if !visited.insert(name) {
return;
}
name_id_gen += 1;
key_to_name.insert(NameKey::new(name_id_gen.into(), name), name.clone());
key_to_name.insert(NameKey::new(name_id_gen.into(), name), name.to_string());
});

let variation_model = VariationModel::new(global_locations, variable_axes.clone())?;
Expand Down
193 changes: 193 additions & 0 deletions resources/testdata/glyphs3/StandardAxisNames.glyphs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
{
.appVersion = "3324";
.formatVersion = 3;
axes = (
{
name = weight;
tag = wght;
},
{
name = width;
tag = wdth;
},
{
name = italic;
tag = ital;
},
{
name = slant;
tag = slnt;
},
{
name = optical;
tag = opsz;
},
{
name = foobarbaz;
tag = FOOB;
}
);
date = "2024-10-10 14:11:11 +0000";
familyName = "New Font";
fontMaster = (
{
axesValues = (
0,
0,
0,
0,
0,
0
);
id = m01;
metricValues = (
{
}
);
name = Regular;
},
{
axesValues = (
0,
0,
0,
0,
10,
0
);
id = "1FD21B2D-09AF-4981-B811-2D872E70C4D7";
metricValues = (
{
}
);
name = Display;
},
{
axesValues = (
10,
0,
0,
0,
0,
0
);
id = "FEF7F46C-F4CB-4286-8DAB-0F5B4E3112C4";
metricValues = (
{
}
);
name = Bold;
},
{
axesValues = (
0,
10,
0,
0,
0,
0
);
id = "35AAE790-B0B9-4EC3-B9E7-BFB27ABD6F7D";
metricValues = (
{
}
);
name = Expanded;
},
{
axesValues = (
0,
0,
1,
0,
0,
0
);
id = "19C9E9CF-9763-4DDF-A379-0445DCA72D4F";
metricValues = (
{
pos = 10;
}
);
name = Italic;
},
{
axesValues = (
0,
0,
0,
10,
0,
0
);
id = "AD527805-E4EB-47AA-92AC-2C0C755E4536";
metricValues = (
{
pos = 10;
}
);
name = Slanted;
},
{
axesValues = (
0,
0,
0,
0,
0,
10
);
id = "A4969238-40F2-4056-9F02-70F1E4A1FE86";
metricValues = (
{
}
);
name = "Foo Bar Baz";
}
);
glyphs = (
{
glyphname = space;
lastChange = "2024-10-14 10:45:18 +0000";
layers = (
{
layerId = m01;
width = 200;
},
{
layerId = "1FD21B2D-09AF-4981-B811-2D872E70C4D7";
width = 600;
},
{
layerId = "FEF7F46C-F4CB-4286-8DAB-0F5B4E3112C4";
width = 600;
},
{
layerId = "35AAE790-B0B9-4EC3-B9E7-BFB27ABD6F7D";
width = 600;
},
{
layerId = "19C9E9CF-9763-4DDF-A379-0445DCA72D4F";
width = 600;
},
{
layerId = "AD527805-E4EB-47AA-92AC-2C0C755E4536";
width = 600;
},
{
layerId = "A4969238-40F2-4056-9F02-70F1E4A1FE86";
width = 600;
}
);
unicode = 32;
}
);
metrics = (
{
type = "italic angle";
}
);
unitsPerEm = 1000;
versionMajor = 1;
versionMinor = 0;
}
Loading