Retina weight is loaded instead of regular weight for FiraCode Nerd Font

[fredizzimo]

When trying to load "FiraCode Nerd Font" using StyleProperty::FontStack(FontStack::Source("FiraCode Nerd Font")) the retina weight (450) is loaded instead of the regular 400 weight.

FiraCodeNerdFont-Regular.ttf has the normal 400 weight, so it should be loaded instead.

NotoSerif Nerd Font has a similar problem, but there it loads the Medium weight instead of Normal

This happens at least on Arch Linux with commit 5b60803

[fredizzimo]

On another Linux machine, it's actually loading the correct weight.

I'm not sure what the difference is, both are Arch Linux install and use the same version of the FiraCode NerdFont, and on both installs the Retina weight is available.

[xorgy]

@fredizzimo
Yeah I was getting a bit confused by that, as a fellow Arch Linux user. It's possible that you have overriding configs on one machine that you do not have on the other, or that if you forced cache reconstruction on both they would become the same.
Have you tried running fc-cache -sf? It might be an issue with post-install steps or something silly like that.

[fredizzimo]

I will try to debug exactly what happens during the weekend when I have access to that machine.

BTW, I have tests for all the bugs I have reported here:
https://github.com/fredizzimo/vide/blob/305d11fd76dd54cd07d7b757172fe65482ec033e/src/test.rs#L305-L756

Those goes through our code, but it would probably be worth introducing something similar here as well.

[xorgy]

Thanks, I'll take another look.

[fredizzimo]

I have now debugged it, and here's what's happening

The fontconfig information is correct

❯ fc-list :family style file weight family | grep FiraCodeNerdFont-
/usr/share/fonts/TTF/FiraCodeNerdFont-Bold.ttf: FiraCode Nerd Font:style=Bold:weight=200
/usr/share/fonts/TTF/FiraCodeNerdFont-Light.ttf: FiraCode Nerd Font,FiraCode Nerd Font Light:style=Light,Regular:weight=50
/usr/share/fonts/TTF/FiraCodeNerdFont-Medium.ttf: FiraCode Nerd Font,FiraCode Nerd Font Med:style=Medium,Regular:weight=100
/usr/share/fonts/TTF/FiraCodeNerdFont-Retina.ttf: FiraCode Nerd Font,FiraCode Nerd Font Ret:style=Retina,Regular:weight=90
/usr/share/fonts/TTF/FiraCodeNerdFont-SemiBold.ttf: FiraCode Nerd Font,FiraCode Nerd Font SemBd:style=SemiBold,Regular:weight=180
/usr/share/fonts/TTF/FiraCodeNerdFont-Regular.ttf: FiraCode Nerd Font:style=Regular:weight=80

But not every font has a weight defined here

parley/fontique/src/backend/fontconfig/cache.rs

Lines 118 to 209 in 6ecf702

	fn parse_font(
	pattern: &Pattern,
	name_free_list: &mut Vec<String>,
	font: &mut CachedFont,
	) -> Option<()> {
	name_free_list.append(&mut font.family);
	font.clear();
	for elt in pattern.elts().ok()? {
	let Ok(obj) = elt.object() else {
	continue;
	};
	match obj {
	Object::Family => {
	for val in elt.values().ok()? {
	let val = val.ok()?;
	if let Value::String(s) = val {
	let mut name = name_free_list.pop().unwrap_or_default();
	name.clear();
	name.push_str(core::str::from_utf8(s.str().ok()?).ok()?);
	font.family.push(name);
	}
	}
	}
	Object::File => {
	for val in elt.values().ok()? {
	let val = val.ok()?;
	if let Value::String(s) = val {
	font.path.clear();
	font.path.push(core::str::from_utf8(s.str().ok()?).ok()?);
	if font.path.extension() == Some(std::ffi::OsStr::new("t1")) {
	return None;
	}
	}
	}
	}
	Object::Slant => {
	for val in elt.values().ok()? {
	if let Value::Int(i) = val.ok()? {
	font.style = Style::from_fc(i as _);
	}
	}
	}
	Object::Weight => {
	for val in elt.values().ok()? {
	if let Value::Int(i) = val.ok()? {
	font.weight = Weight::from_fc(i as _);
	}
	}
	}
	Object::Width => {
	for val in elt.values().ok()? {
	if let Value::Int(i) = val.ok()? {
	font.stretch = Stretch::from_fc(i as _);
	}
	}
	}
	Object::Index => {
	for val in elt.values().ok()? {
	let val = val.ok()?;
	if let Value::Int(i) = val {
	font.index = i as u32;
	// Ignore named instances
	if font.index >> 16 != 0 {
	return None;
	}
	}
	}
	}
	Object::CharSet => {
	for val in elt.values().ok()? {
	let val = val.ok()?;
	if let Value::CharSet(set) = val {
	font.coverage.clear();
	font.coverage
	.numbers
	.extend_from_slice(set.numbers().ok()?.as_slice().ok()?);
	for leaf in set.leaves().ok()? {
	let leaf = leaf.ok()?;
	font.coverage.leaves.push(leaf);
	}
	}
	}
	}
	_ => {}
	}
	}
	if !font.family.is_empty() && !font.path.as_os_str().is_empty() {
	Some(())
	} else {
	None
	}
	}

Because the weight and some other properties are not cleared here

parley/fontique/src/backend/fontconfig/cache.rs

Lines 77 to 82 in 6ecf702

	fn clear(&mut self) {
	self.family.clear();
	self.path.clear();
	self.index = 0;
	self.coverage.clear();
	}

Then the weight remains set to what it was before parse_font was called. In my case for the normal style that happens to be 800 - ExtraBold.

Then later on the maybe_override_attributes forces the loaded normal font to have a weight of 800.

parley/fontique/src/font.rs

Lines 208 to 224 in 6ecf702

	pub(crate) fn maybe_override_attributes(
	&mut self,
	stretch: Stretch,
	style: Style,
	weight: Weight,
	) {
	if self.stretch == Stretch::default() {
	self.stretch = stretch;
	}
	if self.style == Style::default() {
	self.style = style;
	}
	if self.weight == Weight::default() {
	self.weight = weight;
	}
	}
	}

Now, it's no longer matching in this condition

parley/fontique/src/matching.rs

Line 380 in 6ecf702

if (400.0..=500.0).contains(&weight) {

, so the Retina variant becomes the lowest weight closest to normal to use.

Changing the clear function to clear everything to the default values might fix the problem, but I would rather see that something more stable like https://docs.rs/fontconfig/latest/fontconfig/ was used instead of trying to parse the caches manually.

[fredizzimo]

Yes adding:

diff --git a/fontique/src/backend/fontconfig/cache.rs b/fontique/src/backend/fontconfig/cache.rs
index e13c93e..8b48501 100644
--- a/fontique/src/backend/fontconfig/cache.rs
+++ b/fontique/src/backend/fontconfig/cache.rs
@@ -79,6 +79,9 @@ impl CachedFont {
         self.path.clear();
         self.index = 0;
         self.coverage.clear();
+        self.weight = Weight::default();
+        self.style = Style::default();
+        self.stretch = Stretch::default();
     }
 }

Seems to fix this issue, and also these

• Embolden is not set properly #93 (the issue not the additional suggestion of a numerical embolden)
• Italic CascadiaCove Nerd Font is loaded completely wrong #95
• No fallback for styles with missing glyphs #96 - This works even when selecting different weights now, I'm not sure what's the difference is between that and when the weight was automatically selected and wrong

[xorgy]

Thank you for drilling down on this. I think that your last fix here is good. If you want you can put it up as a PR or I will do it.

As for linking against actual fontconfig, we would like to avoid that for a variety of reasons.

[fredizzimo]

Closing this, as it should have been closed by #104