F1: measure an added item with the face it is written as

`Family::width` chose its standard-14 face from `bold` alone and passed
`italic: false`, while `Family::base_font` writes the item out as the
italic or oblique face. Every italic item was therefore laid out with
upright metrics: Times-Italic `A` is 611 against Times-Roman's 722, so
boxes, wrapping and centring were all sized from the wrong face.

The measure now takes `italic` and the five call sites pass the item's
own flag, which they already had in scope. The test walks all twelve
style combinations and asserts the written face and the measured face
are the same one, so the two can't drift apart again.

Co-Authored-By: Claude <[email protected]>
This commit is contained in:
Tanish MankuandClaude committed 2026-10-12 13:57:43 -07:00
1 parent aaa19d87af
commit 117430fade
2 files changed
+38 -7

No files matched your search

+10 -7
View File
@@ -68,8 +68,11 @@ impl Family {
}
/// Advance width of `s` at `size` points, from the published standard-14 metrics.
pub fn width(self, s: &str, size: f64, bold: bool) -> f64 {
self.std14(bold, false).text_width(s, size)
///
/// `italic` matters: the item is written out as the oblique/italic face (see [`Family::base_font`]),
/// whose published widths differ from the upright face (Times-Italic `A` is 611, Times-Roman 722).
pub fn width(self, s: &str, size: f64, bold: bool, italic: bool) -> f64 {
self.std14(bold, italic).text_width(s, size)
}
/// The standard-14 face this family means at the given style.
@@ -252,7 +255,7 @@ fn wrapped(t: &AddedText) -> Vec<(String, bool)> {
// sum of its words' and shaping stays linear in the paragraph's length. The space is the
// paragraph's; a space between Latin words is drawn in the item's font, a few hundredths
// of an em apart.
let measure = |s: &str| if arabic || thai { shaped_width(t, s, rtl) } else { t.family.width(s, t.size, t.bold) };
let measure = |s: &str| if arabic || thai { shaped_width(t, s, rtl) } else { t.family.width(s, t.size, t.bold, t.italic) };
let space = measure(" ");
let (mut line, mut line_w) = (String::new(), 0.0);
for word in para.split(' ') {
@@ -426,7 +429,7 @@ fn shaped_layout(t: &AddedText, line: &str, rtl: bool) -> Result<(Vec<Piece>, f6
let width = pieces
.iter()
.map(|p| match p {
Piece::Latin(s) => t.family.width(s, t.size, t.bold),
Piece::Latin(s) => t.family.width(s, t.size, t.bold, t.italic),
Piece::Shaped(clusters) => clusters.iter().map(|c| c.advance).sum::<f64>() * t.size,
})
.sum();
@@ -436,7 +439,7 @@ fn shaped_layout(t: &AddedText, line: &str, rtl: bool) -> Result<(Vec<Piece>, f6
/// The width of `s` in a shaped-text item; the item's font's estimate if it can't be shaped (drawing
/// then reports why).
fn shaped_width(t: &AddedText, s: &str, rtl: bool) -> f64 {
shaped_layout(t, s, rtl).map(|(_, w)| w).unwrap_or_else(|_| t.family.width(s, t.size, t.bold))
shaped_layout(t, s, rtl).map(|(_, w)| w).unwrap_or_else(|_| t.family.width(s, t.size, t.bold, t.italic))
}
fn arabic_error(e: GlyphError, text: &str) -> EditError {
@@ -572,7 +575,7 @@ fn draw_laid_shaped(
out.extend(literal(&win_ansi(s)));
out.extend_from_slice(b" Tj\n");
current = Some(latin.as_str());
pen += t.family.width(s, t.size, t.bold) + tw * s.matches(' ').count() as f64;
pen += t.family.width(s, t.size, t.bold, t.italic) + tw * s.matches(' ').count() as f64;
}
Piece::Shaped(clusters) => {
for c in clusters {
@@ -725,7 +728,7 @@ fn draw(doc: &mut Document, c: &Content, view: [f64; 6], taken: &Dict, existing:
let [cr, cg, cb] = t.color.map(|v| v.clamp(0.0, 1.0));
out.extend(format!("BT /{name} {} Tf {} {} {} rg\n", n(t.size), n(cr), n(cg), n(cb)).bytes());
for (i, line) in lines(t).iter().enumerate() {
let w = t.family.width(line, t.size, t.bold);
let w = t.family.width(line, t.size, t.bold, t.italic);
let x = match t.align {
Align::Left | Align::Justify => r[0],
Align::Center => r[0] + ((r[2] - r[0]) - w) / 2.0,
+28
View File
@@ -3238,3 +3238,31 @@ fn subset_indic_font_without_legacy_name_keeps_logical_text() {
assert!(!lines[0].visual_order);
assert_eq!(lines[0].text, "\u{092C}\u{0922}\u{093C}\u{093F}\u{092F}\u{093E}");
}
/// An added item is written out as a specific standard-14 face (`/Times-Italic`, `/Helvetica-Bold`
/// …), so it has to be *measured* with that same face. Measuring the upright face while writing
/// the italic one mis-sizes every box: Times-Italic `A` is 611 against Times-Roman's 722.
#[test]
fn an_items_measured_face_is_the_face_it_is_written_as() {
for family in [Family::Helvetica, Family::Times, Family::Courier] {
for bold in [false, true] {
for italic in [false, true] {
let written = family.base_font(bold, italic);
let measured = family.std14(bold, italic);
assert_eq!(written, measured.base_font(), "{family:?} bold={bold} italic={italic}");
// ... and the public measure reaches that face, rather than an upright stand-in.
assert_eq!(
family.width("Anna", 12.0, bold, italic),
measured.text_width("Anna", 12.0),
"{family:?} bold={bold} italic={italic}"
);
}
}
}
// The faces really do differ, so the assertions above are not all comparing the same number.
let times = |italic| Family::Times.width("Anna", 12.0, false, italic);
assert_ne!(times(false), times(true), "Times-Italic is not Times-Roman");
// Courier is monospaced in every style: equal widths here are correct, not a collapsed face.
let courier = |italic| Family::Courier.width("Anna", 12.0, false, italic);
assert_eq!(courier(false), courier(true));
}