From 224cebbd8ab104f1c94f7008a80ee14eddee0219 Mon Sep 17 00:00:00 2001 From: buivanvu94 <125793064+buivanvu94@users.noreply.github.com> Date: Mon, 12 Oct 2026 18:48:03 +0700 Subject: [PATCH] fix(automation): warn when text falls back from a missing font (#3391) (#3630) * fix(automation): warn when text falls back from a missing font (#3391) * cargo fmt Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Brandon Thomas Co-authored-by: Claude Opus 5.5 --- apps/photocraft-cli/src/lib.rs | 2 +- crates/automation/src/headless.rs | 7 ++- .../automation/tests/missing_font_warnings.rs | 57 +++++++++++++++++++ crates/engine/src/type_cmds.rs | 36 +++++++++++- crates/engine/src/type_extra_cmds.rs | 27 +++++++++ 5 files changed, 123 insertions(+), 6 deletions(-) create mode 100644 crates/automation/tests/missing_font_warnings.rs diff --git a/apps/photocraft-cli/src/lib.rs b/apps/photocraft-cli/src/lib.rs index 1743665d90..560eaf71fc 100644 --- a/apps/photocraft-cli/src/lib.rs +++ b/apps/photocraft-cli/src/lib.rs @@ -240,7 +240,7 @@ fn convert(a: &Args, _out: &mut dyn Write, err: &mut dyn Write) -> R { let opts = export_opts(a)?; let o = files::open(Path::new(input)).map_err(|e| e.to_string())?; warn_all(err, &o.warnings); - warn_all(err, &missing_font_warnings(photocraft_engine::type_extra_cmds::missing_fonts(&o.document))); + warn_all(err, &photocraft_engine::type_extra_cmds::missing_font_warnings(&o.document)); let ws = files::save(&o.document, Path::new(output), a.get("--format"), &opts, None).map_err(|e| e.to_string())?; warn_all(err, &ws); Ok(()) diff --git a/crates/automation/src/headless.rs b/crates/automation/src/headless.rs index dc18afe2f2..cd92f584a2 100644 --- a/crates/automation/src/headless.rs +++ b/crates/automation/src/headless.rs @@ -96,13 +96,15 @@ impl Headless { st.source_read_only = o.source_read_only; } let d = &self.session.documents()[index]; + let mut warnings = o.warnings; + warnings.extend(photocraft_engine::type_extra_cmds::missing_font_warnings(&d.doc)); Ok(json!({ "index": index, "name": d.doc.name, "width": d.doc.size.width, "height": d.doc.size.height, "layers": d.doc.layer_count(), - "warnings": o.warnings, + "warnings": warnings, })) } @@ -240,7 +242,7 @@ impl Headless { } }; let target_text = target.to_str().ok_or_else(|| AutomationError::BadRequest("automation paths must be valid UTF-8".into()))?; - let warnings = match &self.filesystem { + let mut warnings = match &self.filesystem { Filesystem::Denied => return Err(AutomationError::BadRequest("automation filesystem access is not granted: write authority is absent".into())), Filesystem::TrustedLocal => { let writer = self.writers.entry(doc.id.0).or_default(); @@ -252,6 +254,7 @@ impl Headless { warnings } }; + warnings.extend(photocraft_engine::type_extra_cmds::missing_font_warnings(&doc)); // A layered document save (PSD, PSB, OpenRaster or .pcraft) is a full write, not a flattened copy: // record the current revision as saved so the session stops reporting `dirty`, and make // the file the document's path. A flat export (PNG, JPEG, …) is a copy and leaves both diff --git a/crates/automation/tests/missing_font_warnings.rs b/crates/automation/tests/missing_font_warnings.rs new file mode 100644 index 0000000000..fe66e5c062 --- /dev/null +++ b/crates/automation/tests/missing_font_warnings.rs @@ -0,0 +1,57 @@ +//! #3391: `doc.save`, `doc.open`, and type commands emit warnings when text uses a missing font. + +use photocraft_automation::Headless; +use serde_json::json; + +fn tmp_dir() -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!( + "pc-font-warn-{}-{}", + std::process::id(), + std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos() + )); + std::fs::create_dir_all(&dir).unwrap(); + dir +} + +#[test] +fn missing_font_warnings_emitted_in_headless_and_type_commands() { + let dir = tmp_dir(); + let psd_path = dir.join("card.psd"); + let png_path = dir.join("card.png"); + + let mut h = Headless::trusted_local(); + h.handle("doc.new", json!({"width": 600, "height": 300, "background": "white"})).unwrap(); + + // 1. type.create with missing font + let created = h + .handle("engine.execute", json!({"command": "type.create", "params": {"x": 30, "y": 100, "text": "Headline", "font": "NoSuchFamily123", "size": 40}})) + .unwrap(); + assert_eq!(created["warnings"], json!(["font 'NoSuchFamily123' is not installed; text may render using a fallback face"])); + + // 2. doc.save card.psd + let saved_psd = h.handle("doc.save", json!({"path": psd_path.to_str().unwrap()})).unwrap(); + let warnings = saved_psd["warnings"].as_array().expect("warnings array in doc.save"); + assert!( + warnings.iter().any(|w| w == "font 'NoSuchFamily123' is not installed; text may render using a fallback face"), + "doc.save PSD warning: {saved_psd}" + ); + + // 3. doc.save card.png + let saved_png = h.handle("doc.save", json!({"path": png_path.to_str().unwrap()})).unwrap(); + let warnings = saved_png["warnings"].as_array().expect("warnings array in doc.save"); + assert!( + warnings.iter().any(|w| w == "font 'NoSuchFamily123' is not installed; text may render using a fallback face"), + "doc.save PNG warning: {saved_png}" + ); + + // 4. doc.close then doc.open + h.handle("doc.close", json!({})).unwrap(); + let opened = h.handle("doc.open", json!({"path": psd_path.to_str().unwrap()})).unwrap(); + let warnings = opened["warnings"].as_array().expect("warnings array in doc.open"); + assert!( + warnings.iter().any(|w| w.as_str().unwrap_or("").contains("is not installed; text may render using a fallback face")), + "doc.open warning: {opened}" + ); + + let _ = std::fs::remove_dir_all(dir); +} diff --git a/crates/engine/src/type_cmds.rs b/crates/engine/src/type_cmds.rs index e59ce9258e..4a4cb347bf 100644 --- a/crates/engine/src/type_cmds.rs +++ b/crates/engine/src/type_cmds.rs @@ -639,7 +639,8 @@ fn info(s: &Session, p: &Value) -> Result { .map(|ln| json!({ "start": char_at(text, ln.range.start), "end": char_at(text, ln.range.end), "baseline": ln.baseline, "x0": ln.x0, "x1": ln.x1, "ascent": ln.ascent, "descent": ln.descent })) .collect(); let bounds = t.cache.as_ref().map(|c| c.content_bounds()).map(|r| json!([r.x0, r.y0, r.x1, r.y1])); - Ok(json!({ + let warnings = crate::type_extra_cmds::missing_font_warnings_for_layer(t); + let mut out = json!({ "layer": id.0, "text": text, "shape": t.shape, @@ -652,7 +653,11 @@ fn info(s: &Session, p: &Value) -> Result { "lines": lines, "bounds": bounds, "pxPerPt": layout.px_per_pt, - })) + }); + if !warnings.is_empty() { + out["warnings"] = json!(warnings); + } + Ok(out) } /// A type layer (not yet rendered or added) from `type.create`'s params: text, point `x`/`y` or @@ -723,7 +728,14 @@ pub fn specs() -> Vec { Ok(id) })?; let bounds = s.active().and_then(|d| d.doc.layer(id)).and_then(|l| l.surface()).map(|c| c.content_bounds()); - Ok(json!({ "layer": id.0, "bounds": bounds.map(|r| [r.x0, r.y0, r.x1, r.y1]) })) + let mut out = json!({ "layer": id.0, "bounds": bounds.map(|r| [r.x0, r.y0, r.x1, r.y1]) }); + if let Some(LayerContent::Text(t)) = s.active().and_then(|d| d.doc.layer(id)).map(|l| &l.content) { + let warnings = crate::type_extra_cmds::missing_font_warnings_for_layer(t); + if !warnings.is_empty() { + out["warnings"] = json!(warnings); + } + } + Ok(out) }, }, CommandSpec { @@ -1582,4 +1594,22 @@ mod tests { assert!(s.active().unwrap().doc.layer(LayerId(id)).is_none()); } } + + #[test] + fn missing_font_emits_warning_in_create_set_style_and_edit() { + let mut s = session(); + let r = s.execute("type.create", json!({"x": 10, "y": 20, "text": "Hello", "font": "NoSuchFamily123"})).unwrap(); + assert_eq!(r["warnings"], json!(["font 'NoSuchFamily123' is not installed; text may render using a fallback face"])); + let id = r["layer"].as_u64().unwrap(); + let styled = s.execute("type.setStyle", json!({"layer": id, "font": "StillNoSuchFamily456"})).unwrap(); + assert_eq!(styled["warnings"], json!(["font 'StillNoSuchFamily456' is not installed; text may render using a fallback face"])); + let edited = s.execute("type.edit", json!({"layer": id, "runs": [{"start": 0, "end": 2, "font": "AnotherMissingFont789"}]})).unwrap(); + assert_eq!( + edited["warnings"], + json!([ + "font 'AnotherMissingFont789' is not installed; text may render using a fallback face", + "font 'StillNoSuchFamily456' is not installed; text may render using a fallback face" + ]) + ); + } } diff --git a/crates/engine/src/type_extra_cmds.rs b/crates/engine/src/type_extra_cmds.rs index 022146f113..e460a975b6 100644 --- a/crates/engine/src/type_extra_cmds.rs +++ b/crates/engine/src/type_extra_cmds.rs @@ -436,6 +436,33 @@ pub fn missing_fonts(doc: &Document) -> Vec { out } +/// Font families used by a type layer that the font database cannot supply. +pub fn missing_fonts_in_layer(t: &TextLayer) -> Vec { + let have = installed_families(); + let mut out: Vec = Vec::new(); + for r in t.char_runs() { + let f = r.style.font_family; + if !f.is_empty() && !have.contains(&f.to_lowercase()) && !out.contains(&f) { + out.push(f); + } + } + out +} + +/// Warnings for missing font families in `doc`. +pub fn missing_font_warnings(doc: &Document) -> Vec { + fonts_to_warnings(missing_fonts(doc)) +} + +/// Warnings for missing font families in `layer`. +pub fn missing_font_warnings_for_layer(t: &TextLayer) -> Vec { + fonts_to_warnings(missing_fonts_in_layer(t)) +} + +fn fonts_to_warnings(fonts: Vec) -> Vec { + fonts.into_iter().map(|font| format!("font '{font}' is not installed; text may render using a fallback face")).collect() +} + /// Replace missing families per `map` (missing → installed; unmapped ones → the default family). fn replace_fonts(s: &mut Session, map: &serde_json::Map, all: bool) -> Result { let d = s.active().ok_or(EngineError::NoDocument)?;