From 311b343219325f885b296e8aab7cc88c0f449694 Mon Sep 17 00:00:00 2001 From: Caio Pontes Date: Mon, 26 Oct 2026 00:49:53 -0300 Subject: [PATCH] Fix #4128: text.create and text.setStyle store a font style the family has Given a family and no font style, text.create and text.createInPath kept the default Regular, and text.setStyle kept the text's previous style, even when the family has no such style. A family whose styles are numbered (Hiragino W3, W6) has no Regular, so the file named a font that isn't installed and opened with Missing Fonts. They now keep the style when the family has it, else take the family's closest one, as text.setRangeStyle already did. Its rule moves to `textedit::fit_font_style`, which the three commands share; Graph labels styled through text.setStyle follow it too. A style the caller names is kept as given. The test builds a family with W3 and W6 and no Regular in code (`test_fonts::numbered_font`), so no font file is added. --- crates/engine/src/cmd/create.rs | 5 ++++- crates/engine/src/cmd/textedit.rs | 22 +++++++++++++++------- crates/engine/src/cmd/typecmd.rs | 9 ++++++++- crates/engine/src/tests_textedit.rs | 29 +++++++++++++++++++++++++++++ crates/text/src/test_fonts.rs | 23 +++++++++++++++++++++++ 5 files changed, 79 insertions(+), 9 deletions(-) diff --git a/crates/engine/src/cmd/create.rs b/crates/engine/src/cmd/create.rs index dd841c661..8c4e71f1f 100644 --- a/crates/engine/src/cmd/create.rs +++ b/crates/engine/src/cmd/create.rs @@ -82,7 +82,7 @@ pub fn specs() -> Vec { "Create Text", [], None, - "{x, y, text, rotation?: deg (about x, y), vertical?: bool = false, size?: pt, font?: family, style?, color?, keepModel?: bool (as paint.setFill), area?: {width, height, fit?: none|autoHeight|shrinkText, fitMinPercent?}, placeholder?: bool (fill it with placeholder text instead, as the Type tools do with prefs placeholderText), justify?: \"auto\" (default: the start of each paragraph's direction)|\"left\"|\"center\"|\"right\"|\"justifyAll\" (point type: aligned on x, so \"center\" centres each line on the point and \"right\" ends it there; text.setStyle changes it later), leadingModel?: \"romanBaseline\"|\"emBoxTop\", charAlign?: \"romanBaseline\"|\"emBoxTop\"|\"emBoxCenter\"|\"emBoxBottom\"|\"icfTop\"|\"icfBottom\" (default: emBoxTop and emBoxCenter while the interface is in Japanese, else romanBaseline)} → {id, bounds: {x, y, width, height} (the new type's box as document.inspect reports it, after justify), overflow?: [id] (area type whose text doesn't fit its frame: the lines past it aren't drawn; left out when it fits)}; new area type gets Auto Size (fit autoHeight) when the autoSizeAreaType preference is on", + "{x, y, text, rotation?: deg (about x, y), vertical?: bool = false, size?: pt, font?: family, style? (default Regular, or the family's closest style when it has no Regular), color?, keepModel?: bool (as paint.setFill), area?: {width, height, fit?: none|autoHeight|shrinkText, fitMinPercent?}, placeholder?: bool (fill it with placeholder text instead, as the Type tools do with prefs placeholderText), justify?: \"auto\" (default: the start of each paragraph's direction)|\"left\"|\"center\"|\"right\"|\"justifyAll\" (point type: aligned on x, so \"center\" centres each line on the point and \"right\" ends it there; text.setStyle changes it later), leadingModel?: \"romanBaseline\"|\"emBoxTop\", charAlign?: \"romanBaseline\"|\"emBoxTop\"|\"emBoxCenter\"|\"emBoxBottom\"|\"icfTop\"|\"icfBottom\" (default: emBoxTop and emBoxCenter while the interface is in Japanese, else romanBaseline)} → {id, bounds: {x, y, width, height} (the new type's box as document.inspect reports it, after justify), overflow?: [id] (area type whose text doesn't fit its frame: the lines past it aren't drawn; left out when it fits)}; new area type gets Auto Size (fit autoHeight) when the autoSizeAreaType preference is on", has_doc, text_create ), @@ -794,6 +794,9 @@ pub(crate) fn new_type_style(s: &Session, p: &Value) -> CharStyle { } if let Some(f) = str_param(p, "style") { style.font_style = f.to_string(); + } else if str_param(p, "font").is_some() { + // A family without a Regular (#4128): its closest style, as text.setRangeStyle takes. + super::textedit::fit_font_style(&mut style); } style.fill = match p.get("color").and_then(color_value) { Some(c) => vectorcraft_color::Paint::solid(super::paint::new_color(s, p, c)), diff --git a/crates/engine/src/cmd/textedit.rs b/crates/engine/src/cmd/textedit.rs index ecc4b1d42..4b8b368f4 100644 --- a/crates/engine/src/cmd/textedit.rs +++ b/crates/engine/src/cmd/textedit.rs @@ -274,6 +274,20 @@ fn get_range(s: &mut Session, p: &Value) -> Result { Ok(json!({"text": text, "runs": runs, "length": len})) } +/// Keep `st`'s font style when its family has it, else take the family's closest one: a family +/// whose styles are numbered (W3, W6) has no Regular (#4128). A family that isn't installed keeps +/// the style as it is. +pub(crate) fn fit_font_style(st: &mut CharStyle) { + let db = vectorcraft_text::FontDb::global(); + let styles = db.styles(&st.font_family); + if !styles.is_empty() + && !styles.iter().any(|s| s.eq_ignore_ascii_case(&st.font_style)) + && let Some(face) = db.face(&st.font_family, &st.font_style) + { + st.font_style = face.style.clone(); + } +} + /// Character attribute changes parsed from params (shared by range and whole-object styling). #[derive(Default)] pub(crate) struct CharChange { @@ -540,13 +554,7 @@ impl CharChange { st.font_family = f.clone(); // Keep the style when the new family has it, else its closest match. if self.style.is_none() { - let styles = vectorcraft_text::FontDb::global().styles(f); - if !styles.is_empty() - && !styles.iter().any(|s| s.eq_ignore_ascii_case(&st.font_style)) - && let Some(face) = vectorcraft_text::FontDb::global().face(f, &st.font_style) - { - st.font_style = face.style.clone(); - } + fit_font_style(st); } } if let Some(v) = &self.style { diff --git a/crates/engine/src/cmd/typecmd.rs b/crates/engine/src/cmd/typecmd.rs index 92d20deee..265341495 100644 --- a/crates/engine/src/cmd/typecmd.rs +++ b/crates/engine/src/cmd/typecmd.rs @@ -120,7 +120,7 @@ pub fn specs() -> Vec { "Character", [], None, - "{ids?|id?, font?, style?, size?: pt, leading?: pt|\"auto\", tracking?: 1/1000 em, justify?: \"auto\" (the start of each paragraph's direction)|\"left\"|\"center\"|\"right\"|\"justifyAll\", fill?: colour, keepModel?: bool (as paint.setFill), features?: [\"dlig\", \"-liga\", …] OpenType, start?: byte, end?: byte} (size/leading are document points after object scaling; with a range: the character attributes style that range and justify the paragraphs it touches; without: all the text) → {ids, overflow?: [id…] (area type and type on a path whose text no longer fits; left out when all fits)}", + "{ids?|id?, font?, style? (font alone: the style is kept when the new family has it, else its closest), size?: pt, leading?: pt|\"auto\", tracking?: 1/1000 em, justify?: \"auto\" (the start of each paragraph's direction)|\"left\"|\"center\"|\"right\"|\"justifyAll\", fill?: colour, keepModel?: bool (as paint.setFill), features?: [\"dlig\", \"-liga\", …] OpenType, start?: byte, end?: byte} (size/leading are document points after object scaling; with a range: the character attributes style that range and justify the paragraphs it touches; without: all the text) → {ids, overflow?: [id…] (area type and type on a path whose text no longer fits; left out when all fits)}", has_doc, set_style ), @@ -811,6 +811,10 @@ fn set_style(s: &mut Session, p: &Value) -> Result { style_chars(t, range, |st| { if let Some(f) = &font { st.font_family = f.clone(); + // Keep the style when the new family has it, else its closest match (#4128). + if style.is_none() { + super::textedit::fit_font_style(st); + } } if let Some(f) = &style { st.font_style = f.clone(); @@ -853,6 +857,9 @@ fn set_style(s: &mut Session, p: &Value) -> Result { st.font_family = f.clone(); st.font_version = None; st.font_axes.clear(); + if style.is_none() { + super::textedit::fit_font_style(st); + } } if let Some(f) = &style { st.font_style = f.clone(); diff --git a/crates/engine/src/tests_textedit.rs b/crates/engine/src/tests_textedit.rs index 3fc4e968d..0b8843a31 100644 --- a/crates/engine/src/tests_textedit.rs +++ b/crates/engine/src/tests_textedit.rs @@ -1427,3 +1427,32 @@ fn clicking_the_in_port_then_the_canvas_threads_a_new_frame_before_it() { let strokes = s.overlays(v).iter().filter(|o| matches!(o, vectorcraft_tools::Overlay::Line { .. })).count(); assert_eq!(strokes, 6, "both frames selected: the first one's out port and the second one's in port show arrows"); } + +/// Given a family without a style, text.create, text.createInPath and text.setStyle keep the style +/// when the family has it, else take the family's closest one, as text.setRangeStyle does: a family +/// whose styles are numbered has no Regular to store (#4128). A style given is kept as given. +#[test] +fn a_family_without_its_style_takes_its_closest_style() { + use vectorcraft_text::test_fonts::{NUMBERED_FAMILY as FAMILY, numbered_font}; + let db = vectorcraft_text::FontDb::global(); + db.add_font(numbered_font("W3").unwrap()); + db.add_font(numbered_font("W6").unwrap()); + let numbered = ["W3", "W6"]; + let style = |s: &Session, id: NodeId| obj(s, id).runs[0].style.font_style.clone(); + let create = |s: &mut Session, p: Value| NodeId(s.execute("text.create", &p).unwrap()["id"].as_u64().unwrap()); + let mut s = session(); + let created = create(&mut s, json!({"x": 10, "y": 50, "text": "a", "font": FAMILY})); + assert!(numbered.contains(&style(&s, created).as_str()), "text.create: {}", style(&s, created)); + let pid = s.execute("path.create", &json!({"d": "M100 400 L400 100"})).unwrap()["id"].as_u64().unwrap(); + let on_path = s.execute("text.createInPath", &json!({"path": pid, "mode": "onPath", "text": "a", "font": FAMILY})).unwrap(); + let on_path = NodeId(on_path["id"].as_u64().unwrap()); + assert!(numbered.contains(&style(&s, on_path).as_str()), "text.createInPath: {}", style(&s, on_path)); + let given = create(&mut s, json!({"x": 10, "y": 90, "text": "a", "font": FAMILY, "style": "Medium"})); + assert_eq!(style(&s, given), "Medium", "a style given is kept"); + let set = text(&mut s, "a"); + s.execute("text.setStyle", &json!({"ids": [set.0], "font": FAMILY})).unwrap(); + assert!(numbered.contains(&style(&s, set).as_str()), "text.setStyle: {}", style(&s, set)); + s.execute("text.setStyle", &json!({"ids": [set.0], "style": "W6"})).unwrap(); + s.execute("text.setStyle", &json!({"ids": [set.0], "font": FAMILY})).unwrap(); + assert_eq!(style(&s, set), "W6", "a style the family has is kept"); +} diff --git a/crates/text/src/test_fonts.rs b/crates/text/src/test_fonts.rs index e57fb92d9..76f6603ff 100644 --- a/crates/text/src/test_fonts.rs +++ b/crates/text/src/test_fonts.rs @@ -408,6 +408,29 @@ pub fn twin_font(newer: bool) -> Option> { Some(builder.build()) } +/// The family of [`numbered_font`]: styles numbered W3 and W6, and no Regular. +pub const NUMBERED_FAMILY: &str = "Numtest Mincho"; + +/// A face of family [`NUMBERED_FAMILY`] in `style` (`W3` or `W6`; the bundled Source Sans 3 +/// Regular renamed), as families whose styles are numbered have, with no Regular among them. +/// `None` when it can't be built. +pub fn numbered_font(style: &str) -> Option> { + const FACE: &[u8] = include_bytes!("../../../assets/fonts/SourceSans3-Regular.ttf"); + let font = skrifa::FontRef::new(FACE).ok()?; + let full = format!("{NUMBERED_FAMILY} {style}"); + let postscript = format!("NumtestMincho-{style}"); + let name = name_table(&[(1, NUMBERED_FAMILY), (2, style), (4, &full), (5, "Version 1.000"), (6, &postscript)])?; + let mut builder = FontBuilder::new(); + builder.add_raw(Tag::new(b"name"), name); + for r in font.table_directory.table_records() { + let tag = Tag::new(&r.tag().to_be_bytes()); + if !builder.contains(tag) { + builder.add_raw(tag, font.table_data(r.tag())?.as_bytes()); + } + } + Some(builder.build()) +} + /// The character whose glyph [`colour_bitmap_font`] draws as a colour bitmap. pub const COLOUR_CHAR: char = 'E';