From 9a312e8adae1210b07a080e003ec94951cebda0d Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 8 Nov 2026 14:23:43 -0800 Subject: [PATCH 1/2] Type: type in a composite font is measured in its own document's definition - A file read for opening is laid out in the composite fonts registered then. Opening a document and reverting the front one now measure again the type set in a font named like a composite font that an open document defines, the font database holds or the session registered earlier. Type in other fonts, missing ones included, keeps the bounds it was read with. - Switching to a document or closing the front one measures again the type set in a composite font whose definition changed, and any type without bounds. - A document reverted while in the back measures its type in composite fonts when it comes to the front, not in the front document's. - Text holding an inline graphic whose symbol's type was measured again takes the graphic's new size. - A failed or panicked command puts the font database's composite fonts back with the documents. - Setting a composite font to the definition an export lends keeps it after the export ends. --- crates/engine/src/cmd/compositefonts.rs | 27 +++- crates/engine/src/cmd/compositefonts/tests.rs | 129 ++++++++++++++++ crates/engine/src/lib.rs | 3 + crates/engine/src/session/documents.rs | 142 +++++++++++++----- crates/engine/src/session/execute.rs | 3 + crates/text/src/fontdb/composite.rs | 18 ++- crates/text/src/tests/composite.rs | 7 + 7 files changed, 286 insertions(+), 43 deletions(-) diff --git a/crates/engine/src/cmd/compositefonts.rs b/crates/engine/src/cmd/compositefonts.rs index 2b356ab85..887e90515 100644 --- a/crates/engine/src/cmd/compositefonts.rs +++ b/crates/engine/src/cmd/compositefonts.rs @@ -71,24 +71,39 @@ pub fn specs() -> Vec { /// other open documents (or an earlier active document) defined. Run after each command and when /// another document takes the front, so an undo, a revert, an opened or closed file or a document /// switch draws type in the active document's own composite fonts, and type naming one it does -/// not define shows as missing. Whether a composite font was added, changed or removed. -pub(crate) fn sync(s: &mut Session) -> bool { +/// not define shows as missing. The lowercased names of the composite fonts added, changed or +/// removed. +pub(crate) fn sync(s: &mut Session) -> Vec { let db = FontDb::global(); let mut names = Vec::new(); - let mut changed = false; + let mut changed = Vec::new(); for f in s.active().map(|d| d.doc.composite_fonts.to_vec()).unwrap_or_default() { - changed |= db.set_composite_font(&f); - names.push(f.name.to_lowercase()); + let name = f.name.to_lowercase(); + if db.set_composite_font(&f) { + changed.push(name.clone()); + } + names.push(name); } let others = s.docs.iter().flat_map(|d| d.doc.composite_fonts.iter().map(|f| f.name.to_lowercase())); let stale: Vec = s.composite_names.iter().cloned().chain(others).filter(|n| !names.contains(n)).collect(); for gone in stale { - changed |= db.remove_composite_font(&gone); + if db.remove_composite_font(&gone) { + changed.push(gone); + } } + s.composite_names_registered.extend(names.iter().chain(&changed).cloned()); s.composite_names = names; changed } +/// The lowercased names of the composite fonts the open documents define, the font database holds +/// and the session has registered before: the names type may have been laid out in. +pub(crate) fn names_in_use(s: &Session) -> std::collections::HashSet { + let defined = s.docs.iter().flat_map(|d| d.doc.composite_fonts.iter().map(|f| f.name.to_lowercase())); + let held = FontDb::global().composite_font_names().into_iter().map(|n| n.to_lowercase()); + defined.chain(held).chain(s.composite_names_registered.iter().cloned()).collect() +} + /// Leave out a file's unusable composite fonts before registering and measuring its type. pub(crate) fn filter_loaded(doc: &mut vectorcraft_doc::Document) { let db = FontDb::global(); diff --git a/crates/engine/src/cmd/compositefonts/tests.rs b/crates/engine/src/cmd/compositefonts/tests.rs index 670a85ee2..ae8bc5f8b 100644 --- a/crates/engine/src/cmd/compositefonts/tests.rs +++ b/crates/engine/src/cmd/compositefonts/tests.rs @@ -395,6 +395,135 @@ fn opened_document_measures_missing_composite_font_as_missing() { assert_eq!(actual.cached_bounds, Some(expected)); } +/// The native file of a document whose composite font `name` sets Roman at full size, with type +/// "Ab12" set in it → (the file, the type's id, its width). +fn full_roman_file(name: &str) -> (Vec, u64, f64) { + let mut s = session(); + let mut full = font(name); + full["parts"]["roman"]["size"] = json!(100); + s.execute("type.compositeFonts.set", &json!({"fonts": [full]})).unwrap(); + let id = s.execute("text.create", &json!({"x": 10, "y": 50, "text": "Ab12", "size": 20, "font": name})).unwrap()["id"].as_u64().unwrap(); + let width = text(&s, id).cached_bounds.unwrap().width(); + (vectorcraft_format::save_file(&s.doc().unwrap().doc), id, width) +} + +/// Type in a missing font that no composite font is named after keeps the bounds it was measured +/// with when its file was read: opening the document does not lay it out again. +#[test] +fn opening_leaves_type_in_an_unrelated_missing_font_as_measured() { + let mut s = session(); + s.execute("type.compositeFonts.set", &json!({"fonts": [font("CF Unrelated")]})).unwrap(); + let id = + s.execute("text.create", &json!({"x": 10, "y": 50, "text": "Ab", "size": 20, "font": "No Such Family CF"})).unwrap()["id"].as_u64().unwrap(); + let mut doc = (*s.doc().unwrap().doc).clone(); + let kept = vectorcraft_geom::Rect::new(1.0, 2.0, 3.0, 4.0); + if let NodeKind::Text(t) = &mut doc.node_mut(NodeId(id)).unwrap().kind { + t.cached_bounds = Some(kept); + } + s.add_document(doc, None); + assert_eq!(text(&s, id).cached_bounds, Some(kept)); +} + +/// Text holding an inline graphic whose symbol's art is type in a composite font is measured with +/// the graphic at the size the opened document's own definition gives it. +#[test] +fn opening_measures_inline_graphics_of_composite_type_in_its_own_definition() { + let mut s = session(); + let mut full = font("CF Inline"); + full["parts"]["roman"]["size"] = json!(100); + s.execute("type.compositeFonts.set", &json!({"fonts": [full]})).unwrap(); + let art = s.execute("text.create", &json!({"x": 10, "y": 50, "text": "Ab12", "size": 20, "font": "CF Inline"})).unwrap()["id"].as_u64().unwrap(); + s.execute("symbol.new", &json!({"name": "CF Inline Mark", "ids": [art]})).unwrap(); + let id = s.execute("text.create", &json!({"x": 10, "y": 150, "text": "x", "size": 20})).unwrap()["id"].as_u64().unwrap(); + s.execute("text.insertInline", &json!({"id": id, "at": 0, "symbol": "CF Inline Mark"})).unwrap(); + let width = text(&s, id).cached_bounds.unwrap().width(); + let bytes = vectorcraft_format::save_file(&s.doc().unwrap().doc); + let mut s = session(); + s.execute("type.compositeFonts.set", &json!({"fonts": [font("CF Inline")]})).unwrap(); + let file = crate::cmd::fileio::read_for_open("inline.vectorcraft", &bytes, None, &json!({}), false, false).unwrap(); + crate::cmd::fileio::open_read(&mut s, file).unwrap(); + assert!((text(&s, id).cached_bounds.unwrap().width() - width).abs() < 1e-6); +} + +/// Type read for opening while a composite font of its name was registered is measured again when +/// it opens, even after the document that defined the font has closed in between. +#[test] +fn opening_measures_type_read_in_a_composite_font_closed_since() { + let mut s = session(); + let id = s.execute("text.create", &json!({"x": 10, "y": 50, "text": "Ab12", "size": 20, "font": "CF Gone"})).unwrap()["id"].as_u64().unwrap(); + let bytes = vectorcraft_format::save_file(&s.doc().unwrap().doc); + s.execute("file.new", &json!({"width": 100, "height": 100})).unwrap(); + s.execute("type.compositeFonts.set", &json!({"fonts": [font("CF Gone")]})).unwrap(); + let file = crate::cmd::fileio::read_for_open("gone.vectorcraft", &bytes, None, &json!({}), false, false).unwrap(); + assert!(s.close_document(1)); + crate::cmd::fileio::open_read(&mut s, file).unwrap(); + let actual = text(&s, id); + let expected = vectorcraft_text::layout(vectorcraft_text::FontDb::global(), &actual).bounds; + assert_eq!(actual.cached_bounds, Some(expected)); +} + +/// A file read for opening lays out its type in the composite fonts registered then (the front +/// document's, which defines the name with Roman at half size); once the file opens, its type is +/// measured again in its own. +#[test] +fn an_opened_file_measures_its_type_in_its_own_composite_fonts() { + let (bytes, id, width) = full_roman_file("CF Opened"); + let mut s = session(); + s.execute("type.compositeFonts.set", &json!({"fonts": [font("CF Opened")]})).unwrap(); + let file = crate::cmd::fileio::read_for_open("opened.vectorcraft", &bytes, None, &json!({}), false, false).unwrap(); + crate::cmd::fileio::open_read(&mut s, file).unwrap(); + let actual = text(&s, id); + assert!((actual.cached_bounds.unwrap().width() - width).abs() < 1e-6); + let expected = vectorcraft_text::layout(vectorcraft_text::FontDb::global(), &actual).bounds; + assert_eq!(actual.cached_bounds, Some(expected)); +} + +/// A document replaced while in the back (File › Revert) is measured in its own composite fonts +/// when it comes to the front, not in the front document's. +#[test] +fn a_document_replaced_in_the_back_is_measured_in_its_own_composite_fonts() { + let (bytes, id, width) = full_roman_file("CF Replaced"); + let mut s = session(); + s.execute("type.compositeFonts.set", &json!({"fonts": [font("CF Replaced")]})).unwrap(); + s.execute("file.new", &json!({"width": 100, "height": 100})).unwrap(); + s.execute("type.compositeFonts.set", &json!({"fonts": [font("CF Replaced")]})).unwrap(); + assert!(s.replace_document(0, vectorcraft_format::load(&bytes).unwrap())); + assert!(s.set_active(0)); + let actual = text(&s, id); + assert!((actual.cached_bounds.unwrap().width() - width).abs() < 1e-6); + let expected = vectorcraft_text::layout(vectorcraft_text::FontDb::global(), &actual).bounds; + assert_eq!(actual.cached_bounds, Some(expected)); +} + +/// So is one that comes to the front because the front document closes. +#[test] +fn a_document_replaced_in_the_back_is_measured_when_the_front_one_closes() { + let (bytes, id, width) = full_roman_file("CF Closed"); + let mut s = session(); + s.execute("file.new", &json!({"width": 100, "height": 100})).unwrap(); + s.execute("type.compositeFonts.set", &json!({"fonts": [font("CF Closed")]})).unwrap(); + assert!(s.replace_document(0, vectorcraft_format::load(&bytes).unwrap())); + assert!(s.close_document(1)); + let actual = text(&s, id); + assert!((actual.cached_bounds.unwrap().width() - width).abs() < 1e-6); + let expected = vectorcraft_text::layout(vectorcraft_text::FontDb::global(), &actual).bounds; + assert_eq!(actual.cached_bounds, Some(expected)); +} + +/// A failed batch takes back the composite fonts it set, in the font database too. +#[test] +fn a_failed_batch_takes_back_its_composite_fonts() { + let db = vectorcraft_text::FontDb::global(); + let mut s = session(); + let steps = json!([ + {"command": "type.compositeFonts.set", "params": {"fonts": [font("CF Failed Batch")]}}, + {"command": "no.such.command"}, + ]); + assert!(s.execute("command.batch", &json!({"commands": steps})).is_err()); + assert!(names(&s).is_empty()); + assert!(db.composite_font("CF Failed Batch").is_none()); +} + /// A loaded composite font cannot replace an installed family used by existing type. #[test] fn loaded_composite_font_cannot_take_an_installed_family_name() { diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 27972ef20..03082e7f5 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -175,6 +175,9 @@ pub struct Session { /// The composite fonts (lowercase names) that the open documents defined when the font /// database was last brought in step with them ([`cmd::compositefonts::sync`]). pub(crate) composite_names: Vec, + /// Every composite font (lowercase name) this session has registered in the font database: + /// type read for opening may have been laid out in one that is gone by the time it opens. + pub(crate) composite_names_registered: std::collections::HashSet, /// While a command that changes nothing in the document runs (a query, or one of /// [`command_sets::JOURNALED_READS`]): it reads the instance of an object selected in a symbol /// instance's art. diff --git a/crates/engine/src/session/documents.rs b/crates/engine/src/session/documents.rs index b0619c992..20fd05355 100644 --- a/crates/engine/src/session/documents.rs +++ b/crates/engine/src/session/documents.rs @@ -1,3 +1,7 @@ +use std::collections::HashSet; + +use vectorcraft_doc::{Node, TextObject}; + use super::{ Arc, Clipboard, Color, CommandInfo, DocState, Document, DrawMode, EngineError, GradientPaint, Journal, NodeKind, PaintDefaults, Prefs, Result, Session, ViewInfo, cmd, command_specs, file_access, @@ -84,6 +88,7 @@ impl Session { batch_steps: None, batch_paint_docs: Vec::new(), composite_names: Vec::new(), + composite_names_registered: HashSet::new(), reading: false, place_hand_off: None, queue_hand_off: None, @@ -182,10 +187,8 @@ impl Session { self.reset_tool_for_doc_switch(); } self.active = Some(index); - // Type is drawn in the composite fonts of the document now in front, and measured again - // when they differ from those of the document before. - if switched && cmd::compositefonts::sync(self) { - self.refresh_document_text_bounds(index); + if switched { + self.sync_front_composite_fonts(); } true } else { @@ -193,6 +196,16 @@ impl Session { } } + /// Register the composite fonts of the document now in front ([`cmd::compositefonts::sync`]), + /// after a switch or a close brought it there. Its type set in a composite font that differs + /// from the one of that name before (added, changed or removed) is measured again, and type not + /// measured yet (in a document replaced while in the back) is measured now. + fn sync_front_composite_fonts(&mut self) { + let changed: HashSet = cmd::compositefonts::sync(self).into_iter().collect(); + let Some(index) = self.active else { return }; + self.refresh_document_text_bounds(index, |t| t.cached_bounds.is_none() || sets_in_any(t, &changed)); + } + /// The name [`Session::next_untitled`] gives next (New Document's Name field). pub fn peek_untitled(&self) -> String { format!("Untitled-{}", self.untitled_counter + 1) @@ -211,19 +224,44 @@ impl Session { pub(crate) fn refresh_text_bounds(doc: &mut Document) { // Inline graphics in text take their size from their symbols' art (not saved either). doc.resolve_inline_art(); - let trees = doc.layers.iter_mut().chain(doc.symbols.iter_mut().map(|s| &mut s.art)); - let trees = trees.chain(doc.graph_designs.iter_mut().map(|g| &mut g.art)); - for tree in trees.chain(doc.patterns.iter_mut().flat_map(|p| p.art.iter_mut())) { - refresh_tree_text_bounds(tree); + let mut uncached = |t: &TextObject| t.cached_bounds.is_none(); + for tree in text_trees_mut(doc) { + each_text_where(tree, &mut uncached, &mut cmd::typecmd::refresh_bounds_for_first_frame); } } - /// [`Session::refresh_text_bounds`] for open document `index`, as a change that is not an - /// edit: a saved document stays saved, and its undo history keeps its documents as they are. - fn refresh_document_text_bounds(&mut self, index: usize) { - if let Some(st) = self.docs.get_mut(index) { - st.change_unsaved(|doc| Self::refresh_text_bounds(Arc::make_mut(doc))); + /// Measure the type in open document `index` that `stale` picks (in its trees as + /// [`Session::refresh_text_bounds`] walks them), as a change that is not an edit: a saved + /// document stays saved, and its undo history keeps its documents as they are. The document is + /// left as it is when `stale` picks nothing. + fn refresh_document_text_bounds(&mut self, index: usize, mut stale: impl FnMut(&TextObject) -> bool) { + let Some(st) = self.docs.get_mut(index) else { return }; + if !text_trees(&st.doc).any(|n| has_text_where(n, &mut stale)) { + return; } + st.change_unsaved(|doc| { + let doc = Arc::make_mut(doc); + for tree in text_trees_mut(doc) { + each_text_where(tree, &mut stale, &mut cmd::typecmd::refresh_bounds_for_first_frame); + } + // Inline graphics take their size from their symbols' art, whose type may just have + // been measured again: the text holding one whose size changed is measured again too. + if !doc.resolve_inline_art().is_empty() { + for tree in text_trees_mut(doc) { + each_text_where(tree, &mut |t| t.cached_bounds.is_none(), &mut cmd::typecmd::refresh_bounds_for_first_frame); + } + } + }); + } + + /// Register the composite fonts of open document `index`, which is in front, and measure its + /// type not measured yet and its type set in a font of a name that a composite font of an open + /// document or of the font database has, or had before: that type may have been laid out in + /// another document's composite fonts (those registered while its file was read for opening). + fn sync_opened_composite_fonts(&mut self, index: usize) { + let mut names = cmd::compositefonts::names_in_use(self); + names.extend(cmd::compositefonts::sync(self)); + self.refresh_document_text_bounds(index, |t| t.cached_bounds.is_none() || sets_in_any(t, &names)); } /// Add a document and make it active. Between `edit.undoGroup.begin` and `edit.undoGroup.end`, @@ -234,6 +272,8 @@ impl Session { // (importers and the native reader have seen to theirs already, with a warning). doc.limit_depth(); cmd::compositefonts::filter_loaded(&mut doc); + // Inline graphics in text take their size from their symbols' art (not saved). + doc.resolve_inline_art(); // Guides from files saved before guides had layers go on a layer. doc.adopt_guides(); cmd::dimension::conform(&mut doc); @@ -244,8 +284,7 @@ impl Session { self.docs.push(st); let i = self.docs.len() - 1; self.active = Some(i); - cmd::compositefonts::sync(self); - self.refresh_document_text_bounds(i); + self.sync_opened_composite_fonts(i); if self.depth == 0 && let Some(label) = self.explicit_undo_group.clone() { @@ -262,11 +301,26 @@ impl Session { } doc.limit_depth(); cmd::compositefonts::filter_loaded(&mut doc); + doc.resolve_inline_art(); doc.adopt_guides(); cmd::dimension::conform(&mut doc); + // In the back, type set in a font named like a composite font is measured when the document + // comes to the front and its own composite fonts are registered, not in the front + // document's. + let mut back_names = None; if self.active == Some(index) { // Pending tool work (typing, a drag) belongs to the content being thrown away. self.reset_tool_for_doc_switch(); + } else { + let mut names = cmd::compositefonts::names_in_use(self); + names.extend(doc.composite_fonts.iter().map(|f| f.name.to_lowercase())); + for tree in text_trees_mut(&mut doc) { + each_text_where(tree, &mut |t| t.cached_bounds.is_some() && sets_in_any(t, &names), &mut |t| { + t.cached_bounds = None; + t.cached_baselines.clear(); + }); + } + back_names = Some(names); } let old = &self.docs[index]; let mut st = DocState::new(doc, old.path.clone()); @@ -297,12 +351,10 @@ impl Session { if let Some(stash) = &mut self.batch_stash { stash.push(old); } - // Type in the composite fonts of a document in the back is measured again when it comes to - // the front. - if self.active == Some(index) { - cmd::compositefonts::sync(self); + match back_names { + Some(names) => self.refresh_document_text_bounds(index, |t| t.cached_bounds.is_none() && !sets_in_any(t, &names)), + None => self.sync_opened_composite_fonts(index), } - self.refresh_document_text_bounds(index); true } @@ -319,7 +371,7 @@ impl Session { stash.push(old); } self.active = if self.docs.is_empty() { None } else { Some(index.min(self.docs.len() - 1)) }; - cmd::compositefonts::sync(self); + self.sync_front_composite_fonts(); true } @@ -350,27 +402,49 @@ impl Session { } } -/// Lay out the type in tree `n` (opacity mask art included) whose bounds aren't cached. Only the -/// nodes on the way to such type are copied. -fn refresh_tree_text_bounds(n: &mut Arc) { - fn stale(n: &vectorcraft_doc::Node) -> bool { - matches!(&n.kind, NodeKind::Text(t) if t.cached_bounds.is_none()) - || n.extras().mask.as_deref().is_some_and(|m| stale(&m.art)) - || n.children().is_some_and(|ch| ch.iter().any(|c| stale(c))) - } - if !stale(n) { +/// Whether some of type `t` is set in a font named (in any case) one of `names`, lowercased. +fn sets_in_any(t: &TextObject, names: &HashSet) -> bool { + !names.is_empty() && t.runs.iter().any(|r| names.contains(&r.style.font_family.to_lowercase())) +} + +/// The trees of `doc` that hold type: its layers, symbol definitions, graph designs and pattern +/// tiles. +fn text_trees(doc: &Document) -> impl Iterator> { + let trees = doc.layers.iter().chain(doc.symbols.iter().map(|s| &s.art)); + let trees = trees.chain(doc.graph_designs.iter().map(|g| &g.art)); + trees.chain(doc.patterns.iter().flat_map(|p| p.art.iter())) +} + +/// [`text_trees`], to change. +fn text_trees_mut(doc: &mut Document) -> impl Iterator> { + let trees = doc.layers.iter_mut().chain(doc.symbols.iter_mut().map(|s| &mut s.art)); + let trees = trees.chain(doc.graph_designs.iter_mut().map(|g| &mut g.art)); + trees.chain(doc.patterns.iter_mut().flat_map(|p| p.art.iter_mut())) +} + +/// Whether tree `n` (opacity mask art included) holds type that `pick` picks. +fn has_text_where(n: &Node, pick: &mut dyn FnMut(&TextObject) -> bool) -> bool { + matches!(&n.kind, NodeKind::Text(t) if pick(t)) + || n.extras().mask.as_deref().is_some_and(|m| has_text_where(&m.art, pick)) + || n.children().is_some_and(|ch| ch.iter().any(|c| has_text_where(c, pick))) +} + +/// Run `act` on the type in tree `n` (opacity mask art included) that `pick` picks. Only the nodes +/// on the way to such type are copied. +fn each_text_where(n: &mut Arc, pick: &mut dyn FnMut(&TextObject) -> bool, act: &mut dyn FnMut(&mut TextObject)) { + if !has_text_where(n, pick) { return; } let n = Arc::make_mut(n); if let NodeKind::Text(t) = &mut n.kind - && t.cached_bounds.is_none() + && pick(t) { - cmd::typecmd::refresh_bounds_for_first_frame(t); + act(t); } if let Some(m) = n.extras_if_any().and_then(|e| e.mask.as_deref_mut()) { - refresh_tree_text_bounds(&mut m.art); + each_text_where(&mut m.art, pick, act); } for c in n.children_mut().into_iter().flatten() { - refresh_tree_text_bounds(c); + each_text_where(c, pick, act); } } diff --git a/crates/engine/src/session/execute.rs b/crates/engine/src/session/execute.rs index 5ad84885f..e05b9962b 100644 --- a/crates/engine/src/session/execute.rs +++ b/crates/engine/src/session/execute.rs @@ -131,6 +131,9 @@ impl Session { let r = if self.depth == 0 { self.run_guarded(id, run) } else { self.run_nested(run) }; if r.is_err() && self.depth == 0 { self.batch_paint_docs.clear(); + // A failed or panicked command's documents are put back: type is drawn in their + // composite fonts again, not in those the command set. + cmd::compositefonts::sync(self); } let r = r?; if self.depth == 0 { diff --git a/crates/text/src/fontdb/composite.rs b/crates/text/src/fontdb/composite.rs index faee695ca..0e1d9afa8 100644 --- a/crates/text/src/fontdb/composite.rs +++ b/crates/text/src/fontdb/composite.rs @@ -31,6 +31,13 @@ impl FontDb { { let mut map = self.composites.write().unwrap_or_else(|e| e.into_inner()); if map.get(&key).is_some_and(|f| **f == *font) { + // The same definition a guard lends: it stays when the guard drops. + let mut loans = self.composite_loans.lock().unwrap_or_else(|e| e.into_inner()); + if let Some(loan) = loans.get_mut(&key) + && map.get(&key).is_some_and(|have| Arc::ptr_eq(have, &loan.font)) + { + loan.adopted = true; + } return false; } map.insert(key, Arc::new(font.clone())); @@ -194,7 +201,7 @@ impl FontDb { } let font = Arc::new((*f).clone()); let previous = map.insert(key.clone(), font.clone()); - loans.insert(key.clone(), CompositeLoan { font: font.clone(), previous, guards: 1 }); + loans.insert(key.clone(), CompositeLoan { font: font.clone(), previous, guards: 1, adopted: false }); lent.push((key, font)); changed = true; } @@ -300,6 +307,9 @@ pub(super) struct CompositeLoan { previous: Option>, /// How many guards lend it. guards: usize, + /// Whether [`FontDb::set_composite_font`] set the same definition while it was lent: the last + /// guard to drop then leaves it in place. + adopted: bool, } /// The composite fonts [`FontDb::lend_composite_fonts`] made available, given back when it drops. @@ -327,9 +337,11 @@ impl Drop for LentCompositeFonts<'_> { continue; } let previous = loan.previous.take(); + let adopted = loan.adopted; loans.remove(key); - // Given back unless a font of that name has been set or removed since. - if map.get(key).is_some_and(|have| Arc::ptr_eq(have, font)) { + // Given back unless a font of that name has been set (the same definition included) + // or removed since. + if !adopted && map.get(key).is_some_and(|have| Arc::ptr_eq(have, font)) { match previous { Some(p) => map.insert(key.clone(), p), None => map.remove(key), diff --git a/crates/text/src/tests/composite.rs b/crates/text/src/tests/composite.rs index 370fa4557..729afa0d5 100644 --- a/crates/text/src/tests/composite.rs +++ b/crates/text/src/tests/composite.rs @@ -129,6 +129,13 @@ fn lent_composite_fonts_last_as_long_as_a_guard() { db.set_composite_font(&replaced); drop(lent); assert_eq!(db.composite_font("Lent").unwrap().parts.roman.size, 60.0); + // So does the same definition set while lent, after every guard lending it drops. + assert!(db.remove_composite_font("Lent")); + let (one, two) = (db.lend_composite_fonts(&fonts[..1]), db.lend_composite_fonts(&fonts[..1])); + assert!(!db.set_composite_font(&fonts[0])); + drop(one); + drop(two); + assert!(db.composite_font("Lent").is_some()); // A guard whose font was removed doesn't give back a later guard's loan of that name. assert!(db.remove_composite_font("Lent")); let old = db.lend_composite_fonts(&fonts[..1]); From 4fadb4456672eaaaad85c6bf679a99ac8ea3499a Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 8 Nov 2026 15:33:33 -0800 Subject: [PATCH 2/2] Type: a refused composite font edit leaves no new font in the font database The session now lists the new fonts' names before the edit, so sync removes a name the document does not define when the edit is refused. --- crates/engine/src/cmd/compositefonts.rs | 3 +++ crates/engine/src/cmd/compositefonts/tests.rs | 15 +++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/crates/engine/src/cmd/compositefonts.rs b/crates/engine/src/cmd/compositefonts.rs index 887e90515..3925612a1 100644 --- a/crates/engine/src/cmd/compositefonts.rs +++ b/crates/engine/src/cmd/compositefonts.rs @@ -187,6 +187,9 @@ fn replace(s: &mut Session, label: &str, after: Vec) -> Result<() } let changed: Vec = before.iter().filter(|b| !after.contains(b)).chain(after.iter().filter(|a| !before.contains(a))).map(|f| f.name.to_lowercase()).collect(); + // A refused or panicked edit leaves the document without the new fonts: `sync` forgets a name + // listed here that the document doesn't define. + s.composite_names.extend(after.iter().map(|f| f.name.to_lowercase())); let db = FontDb::global(); for f in &before { if !after.iter().any(|a| CompositeFont::same_name(&a.name, &f.name)) { diff --git a/crates/engine/src/cmd/compositefonts/tests.rs b/crates/engine/src/cmd/compositefonts/tests.rs index ae8bc5f8b..f51f066df 100644 --- a/crates/engine/src/cmd/compositefonts/tests.rs +++ b/crates/engine/src/cmd/compositefonts/tests.rs @@ -524,6 +524,21 @@ fn a_failed_batch_takes_back_its_composite_fonts() { assert!(db.composite_font("CF Failed Batch").is_none()); } +/// A composite font whose edit is refused, because it makes selected type too wide for the canvas, +/// is not left in the font database. +#[test] +fn a_refused_edit_takes_back_its_new_composite_font() { + let db = vectorcraft_text::FontDb::global(); + let mut s = session(); + let x = crate::MAX_COORD - 1000.0; + s.execute("text.create", &json!({"x": x, "y": 50, "text": "MMMMMMMM", "size": 100, "font": "CF Refused Edit"})).unwrap(); + let wide = json!({"name": "CF Refused Edit", "parts": {"kanji": {"font": "Source Sans 3", "size": 400}}}); + let e = s.execute("type.compositeFonts.set", &json!({"fonts": [wide]})).unwrap_err().to_string(); + assert!(e.contains("exceed the canvas"), "{e}"); + assert!(names(&s).is_empty()); + assert!(db.composite_font("CF Refused Edit").is_none()); +} + /// A loaded composite font cannot replace an installed family used by existing type. #[test] fn loaded_composite_font_cannot_take_an_installed_family_name() {