From 55ba002acf591c2351857e9a928a7967dbcb76f0 Mon Sep 17 00:00:00 2001 From: FeiLiuEM Date: Fri, 9 Oct 2026 20:21:05 +0800 Subject: [PATCH 1/2] Text: the variable-font test picks an axis that moves the advance `style_variations_change_advances_and_outlines` picked `wdth`, else `wght`, else the first axis with a range, and then asserted that the advance moves. That assumes the chosen axis changes advances, which a monospaced family does not: Google Sans Code - the first variable face on a machine where it is installed, and the one this check met on Linux - moves its outlines along `wght` but keeps its advances fixed by design, so the test failed with "wght 300..800: advances 780 vs 780" while `measure` and the font were both right. The test now tries the axes in order and uses the first one whose advance actually moves (Google Sans Code answers on its own MONO axis, Bahnschrift on `wdth`), and skips with a message on a machine whose variable faces never move an advance - the same shape as the "no variable font installed" skip next to it. The outline assertion keeps its teeth: the axis is picked for moving the advance, but whether the outline follows it is still checked. On Linux with Google Sans Code and Bahnschrift installed: `cargo test -p effectcraft-text` reported 43 passed / 1 failed before and 44 passed after. `cargo fmt --all --check` and `cargo clippy -p effectcraft-text --all-targets -- -D warnings` are clean. --- crates/text/src/variable.rs | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/crates/text/src/variable.rs b/crates/text/src/variable.rs index 6594e2dc..2a0bf932 100644 --- a/crates/text/src/variable.rs +++ b/crates/text/src/variable.rs @@ -197,26 +197,40 @@ mod tests { eprintln!("no variable font installed: skipped"); return; }; - // An axis that changes widths (wdth or wght, else the first that moves). - let a = axes.iter().find(|a| a.tag == "wdth" && a.max > a.min).or_else(|| axes.iter().find(|a| a.tag == "wght" && a.max > a.min)).unwrap_or(&axes[0]); let info = crate::fonts::face(face).info.clone(); - let st = |v: f32| crate::TextStyle { + // An axis that moves the advance: `wdth` and `wght` usually do, but a monospaced family + // keeps its advances fixed along `wght` (Google Sans Code does) and moves them along its + // own axis instead, so try the axes in order, take the first one that moves, and skip a + // machine whose variable faces never do. + let st = |a: &FontAxis, v: f32| crate::TextStyle { family: info.family.clone(), style: info.style.clone(), size: 100.0, variations: vec![(a.tag.clone(), v)], ..Default::default() }; - let (lo, hi) = (crate::layout::measure("Hamburgefonts", &st(a.min)), crate::layout::measure("Hamburgefonts", &st(a.max))); + let mut moving = None; + for a in axes.iter().filter(|a| a.max > a.min) { + let (lo, hi) = (crate::layout::measure("Hamburgefonts", &st(a, a.min)), crate::layout::measure("Hamburgefonts", &st(a, a.max))); + if (lo - hi).abs() > 1.0 { + moving = Some((a.clone(), lo, hi)); + break; + } + } + let Some((a, lo, hi)) = moving else { + eprintln!("no variable axis installed here moves the advance: skipped"); + return; + }; if crate::resolve(&info.family, &info.style).face != face { eprintln!("variable face not reachable by family/style: skipped"); return; } + // The axis was picked because it moves the advance; the outline has to follow it too. assert!((lo - hi).abs() > 1.0, "{} {}..{}: advances {lo} vs {hi}", a.tag, a.min, a.max); let ink = |v: f32| { - let l = crate::layout::layout("H", &st(v), &crate::ParagraphStyle::default()); + let l = crate::layout::layout("H", &st(&a, v), &crate::ParagraphStyle::default()); l.glyphs.iter().map(|g| crate::glyph_outline(g).area().abs()).sum::() }; - assert!((ink(a.min) - ink(a.max)).abs() > 1.0); + assert!((ink(a.min) - ink(a.max)).abs() > 1.0, "{} {}..{}: outlines do not move", a.tag, a.min, a.max); } } From 66df1696ac0ba4bb52b593c36890473ab644a47e Mon Sep 17 00:00:00 2001 From: FeiLiuEM Date: Fri, 9 Oct 2026 23:01:54 +0800 Subject: [PATCH 2/2] Text: the variable-font test fails when nothing moves instead of skipping The test picked the first axis that moves an advance and skipped when none did, so a variation path that broke completely - every axis measuring the same - took the skip and hid exactly the regression the check is there for. Only a face with no axis carrying a range skips now. When no ranged axis moves the advance (a monospaced family holds its advances on purpose), the outline must still move along `wght`, or, on a face without one, along the first axis that carries a range; a path that moves neither fails with a message that says so. Verified on Linux with Google Sans Code and Bahnschrift installed: - the normal path passes (the MONO axis is the one that moves the advance), - forcing the branch where no axis moves the advance passes, so a legitimate monospaced face does not fail, - forcing that branch and freezing the outline area fails with "wght 300..800: neither the advances nor the outlines move, so variation support looks broken". `cargo fmt --all --check` and `cargo clippy -p effectcraft-text --all-targets -- -D warnings` clean. --- crates/text/src/variable.rs | 39 ++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/crates/text/src/variable.rs b/crates/text/src/variable.rs index 2a0bf932..a673578e 100644 --- a/crates/text/src/variable.rs +++ b/crates/text/src/variable.rs @@ -200,8 +200,9 @@ mod tests { let info = crate::fonts::face(face).info.clone(); // An axis that moves the advance: `wdth` and `wght` usually do, but a monospaced family // keeps its advances fixed along `wght` (Google Sans Code does) and moves them along its - // own axis instead, so try the axes in order, take the first one that moves, and skip a - // machine whose variable faces never do. + // own axis instead, so try the axes in order and take the first one that moves. A face + // with no axis carrying a range is the only case that skips: a variation path that broke + // completely moves neither the advances nor the outlines, and that has to fail below. let st = |a: &FontAxis, v: f32| crate::TextStyle { family: info.family.clone(), style: info.style.clone(), @@ -209,28 +210,40 @@ mod tests { variations: vec![(a.tag.clone(), v)], ..Default::default() }; + let ranged: Vec<&FontAxis> = axes.iter().filter(|a| a.max > a.min).collect(); + if ranged.is_empty() { + eprintln!("no variable axis installed here carries a range: skipped"); + return; + } let mut moving = None; - for a in axes.iter().filter(|a| a.max > a.min) { + for a in &ranged { let (lo, hi) = (crate::layout::measure("Hamburgefonts", &st(a, a.min)), crate::layout::measure("Hamburgefonts", &st(a, a.max))); if (lo - hi).abs() > 1.0 { - moving = Some((a.clone(), lo, hi)); + moving = Some(((*a).clone(), lo, hi)); break; } } - let Some((a, lo, hi)) = moving else { - eprintln!("no variable axis installed here moves the advance: skipped"); - return; - }; if crate::resolve(&info.family, &info.style).face != face { eprintln!("variable face not reachable by family/style: skipped"); return; } - // The axis was picked because it moves the advance; the outline has to follow it too. - assert!((lo - hi).abs() > 1.0, "{} {}..{}: advances {lo} vs {hi}", a.tag, a.min, a.max); - let ink = |v: f32| { - let l = crate::layout::layout("H", &st(&a, v), &crate::ParagraphStyle::default()); + let ink = |a: &FontAxis, v: f32| { + let l = crate::layout::layout("H", &st(a, v), &crate::ParagraphStyle::default()); l.glyphs.iter().map(|g| crate::glyph_outline(g).area().abs()).sum::() }; - assert!((ink(a.min) - ink(a.max)).abs() > 1.0, "{} {}..{}: outlines do not move", a.tag, a.min, a.max); + if let Some((a, lo, hi)) = moving { + // The axis was picked because it moves the advance; the outline has to follow it too. + assert!((lo - hi).abs() > 1.0, "{} {}..{}: advances {lo} vs {hi}", a.tag, a.min, a.max); + let (i_lo, i_hi) = (ink(&a, a.min), ink(&a, a.max)); + assert!((i_lo - i_hi).abs() > 1.0, "{} {}..{}: outlines do not move", a.tag, a.min, a.max); + } else { + // Nothing here moves an advance. Holding the advances is what a monospaced family does + // on purpose, but it is also what a broken variation path does, so this case still has + // to fail rather than skip: the outline must move along `wght`, or, on a face without + // one, along the first axis that carries a range. + let a = axes.iter().find(|a| a.tag == "wght" && a.max > a.min).unwrap_or(ranged[0]); + let (i_lo, i_hi) = (ink(a, a.min), ink(a, a.max)); + assert!((i_lo - i_hi).abs() > 1.0, "{} {}..{}: neither the advances nor the outlines move, so variation support looks broken", a.tag, a.min, a.max); + } } }