diff --git a/crates/engine/src/cmd/distortcmds.rs b/crates/engine/src/cmd/distortcmds.rs index 689f1dbb5..a1c29df13 100644 --- a/crates/engine/src/cmd/distortcmds.rs +++ b/crates/engine/src/cmd/distortcmds.rs @@ -746,13 +746,9 @@ fn puppet_warp(s: &mut Session, p: &Value) -> Result { } /// Forget the Puppet Warp pins: not an undo step, and a saved document stays saved (pins are -/// never saved). +/// never saved, [`crate::DocState::change_unsaved`]). pub(crate) fn drop_puppet_pins(st: &mut crate::DocState) { - let clean = !st.is_dirty(); - Arc::make_mut(&mut st.doc).puppet = None; - if clean { - st.mark_saved(); - } + st.change_unsaved(|d| Arc::make_mut(d).puppet = None); } fn puppet_pins(s: &mut Session, p: &Value) -> Result { @@ -1335,7 +1331,8 @@ fn edit_text(s: &mut Session, p: &Value) -> Result { } /// Edit Text ends (isolation mode on `id` exits): the type is projected again, in the document and -/// in every undo state (the flat view is never an undo step of its own). +/// in every undo state (the flat view is never an undo step of its own, and never saved: a saved +/// state stays saved, [`crate::DocState::change_unsaved`]). pub(crate) fn finish_edit_text(st: &mut crate::DocState, id: NodeId) { let clear = |doc: &mut Arc| { // Undo steps keep no lookup cache: look without building one, and drop the one an edit @@ -1346,14 +1343,14 @@ pub(crate) fn finish_edit_text(st: &mut crate::DocState, id: NodeId) { p.editing = false; } }; - let saved = Arc::ptr_eq(&st.doc, &st.saved_doc); - clear(&mut st.doc); - if saved { - st.saved_doc = st.doc.clone(); - } + st.change_unsaved(clear); for e in st.history.undo.iter_mut().chain(st.history.redo.iter_mut()) { + let saved = Arc::ptr_eq(&e.doc, &st.saved_doc); clear(&mut e.doc); e.doc.drop_lookup_cache(); + if saved { + st.saved_doc = e.doc.clone(); + } } if let Some(it) = &mut st.interaction { clear(&mut it.doc); diff --git a/crates/engine/src/cmd/fileio/save.rs b/crates/engine/src/cmd/fileio/save.rs index 76d1ad229..c0debea3e 100644 --- a/crates/engine/src/cmd/fileio/save.rs +++ b/crates/engine/src/cmd/fileio/save.rs @@ -648,7 +648,8 @@ impl SaveJob { /// Once written: Save and Save As (except to a template) make the file `st`'s own: path, /// title, format, options, and the snapshot as the saved state (edits made since the job was - /// made keep the document modified). + /// made keep the document modified; changes that are not edits to save don't, + /// [`DocState::same_content`]). pub fn finish(self, st: &mut DocState) { let retargets = self.plan.retargets(); if let (true, Some(path)) = (retargets, self.plan.path) { @@ -660,6 +661,12 @@ impl SaveJob { if let Some(at) = self.stamped { if Arc::ptr_eq(&st.doc, &self.source) { st.doc = self.snapshot.clone(); + } else if st.same_content(&self.source) { + // Changed while the save ran only by changes that are not edits to save + // (Puppet Warp pins dropped, Edit Text ended, type laid out again after a font + // scan): with the dates, the document counts as saved. + stamp_save_dates(st, at); + return st.mark_saved(); } else { // Edited while the save ran: the dates still go on, the edits stay unsaved. The // undo history holds the document as it was before the dates, so that is the diff --git a/crates/engine/src/cmd/fonts.rs b/crates/engine/src/cmd/fonts.rs index bf604bfff..bab4ab82c 100644 --- a/crates/engine/src/cmd/fonts.rs +++ b/crates/engine/src/cmd/fonts.rs @@ -475,16 +475,14 @@ pub(crate) fn rescan(s: &mut Session, _: &Value) -> Result { } }); if !texts.is_empty() { - let saved = !d.is_dirty(); - let doc = std::sync::Arc::make_mut(&mut d.doc); - for id in texts { - if let Some(NodeKind::Text(t)) = doc.node_mut(id).map(|n| &mut n.kind) { - refresh_bounds(t); + d.change_unsaved(|doc| { + let doc = std::sync::Arc::make_mut(doc); + for id in texts { + if let Some(NodeKind::Text(t)) = doc.node_mut(id).map(|n| &mut n.kind) { + refresh_bounds(t); + } } - } - if saved { - d.mark_saved(); - } + }); } d.revision += 1; } diff --git a/crates/engine/src/cmd/recovery.rs b/crates/engine/src/cmd/recovery.rs index ce4ce7a53..9f046b39b 100644 --- a/crates/engine/src/cmd/recovery.rs +++ b/crates/engine/src/cmd/recovery.rs @@ -697,7 +697,7 @@ pub fn skip_reason(s: &Session, st: &DocState) -> Option<&'static str> { if !st.is_dirty() { return Some("no unsaved changes"); } - if st.recovery.as_ref().is_some_and(|c| Arc::ptr_eq(&c.doc, &st.doc)) { + if st.recovery.as_ref().is_some_and(|c| st.same_content(&c.doc)) { return Some("unchanged since its last recovery copy"); } if s.prefs.recovery_off_for_complex && object_count(&st.doc) > COMPLEX_OBJECTS { diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 27fc05e5b..cca2188fa 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -287,6 +287,8 @@ pub struct DocState { /// The document as last saved (or opened). Structural sharing makes "unchanged" a pointer /// comparison, and undoing back to the saved state counts as clean. saved_doc: Arc, + /// The documents that changes that are not edits to save replaced ([`DocState::same_content`]). + unsaved_changes: SameContent, /// The layer new art goes into (the "current layer" in the Layers panel). pub active_layer: Option, /// The rows highlighted in the Layers panel (layers, sublayers, groups or objects, in the order @@ -452,6 +454,36 @@ impl RecalledSelection { } } +/// The documents that changes that are not edits to save (Puppet Warp pins dropped, Edit Text ended, +/// type laid out again after a font scan) replaced, each counted as having the content of the +/// document it was replaced by ([`DocState::change_unsaved`]). A document that nothing holds any +/// more is dropped the next time one is added. +#[derive(Clone, Debug, Default)] +struct SameContent { + /// The document the last such change made. + now: std::sync::Weak, + /// The documents with its content that such changes replaced, oldest first. + before: Vec>, +} + +impl SameContent { + /// Note that `now` replaced `before` without a change to its content. + fn replaced(&mut self, before: std::sync::Weak, now: &Arc) { + if !self.now.ptr_eq(&before) { + self.before.clear(); + } + self.before.retain(|d| d.strong_count() > 0); + if before.strong_count() > 0 { + self.before.push(before); + } + self.now = Arc::downgrade(now); + } + /// Whether `doc` is one of the documents with the content of `now`. + fn holds(&self, now: &Arc, doc: &Arc) -> bool { + std::ptr::eq(self.now.as_ptr(), Arc::as_ptr(now)) && self.before.iter().any(|d| std::ptr::eq(d.as_ptr(), Arc::as_ptr(doc))) + } +} + impl DocState { pub fn new(mut doc: Document, path: Option) -> Self { let active_layer = doc.default_layer(); @@ -463,6 +495,7 @@ impl DocState { let doc = Arc::new(doc); Self { saved_doc: doc.clone(), + unsaved_changes: SameContent::default(), doc, selection: Selection::default(), history: History { limit: 500, ..Default::default() }, @@ -493,9 +526,10 @@ impl DocState { last_perspective: None, } } - /// Unsaved changes: the document differs from the saved one (selection changes don't count). + /// Unsaved changes: the document differs from the saved one (selection changes and changes that + /// are not edits to save don't count, [`DocState::same_content`]). pub fn is_dirty(&self) -> bool { - !Arc::ptr_eq(&self.doc, &self.saved_doc) + !self.same_content(&self.saved_doc) } /// Record the current document as saved. pub fn mark_saved(&mut self) { @@ -506,6 +540,26 @@ impl DocState { pub fn mark_saved_as(&mut self, snapshot: &Arc) { self.saved_doc = snapshot.clone(); } + /// Whether `doc` counts as having the content of the document now: it is the document, or one + /// that changes that are not edits to save replaced since ([`DocState::change_unsaved`]). + pub fn same_content(&self, doc: &Arc) -> bool { + Arc::ptr_eq(&self.doc, doc) || self.unsaved_changes.holds(&self.doc, doc) + } + /// Make a change that is not an edit to save with `f`, without an undo step: editing state that + /// is never saved, or type laid out again after a font scan (which can also refit the frame of + /// Auto Size area type). A saved document stays saved, and a save or recovery copy taken of the + /// document before counts as having its content ([`DocState::same_content`]). + pub(crate) fn change_unsaved(&mut self, f: impl FnOnce(&mut Arc)) { + let saved = !self.is_dirty(); + let before = Arc::downgrade(&self.doc); + f(&mut self.doc); + if !std::ptr::eq(before.as_ptr(), Arc::as_ptr(&self.doc)) { + self.unsaved_changes.replaced(before, &self.doc); + } + if saved { + self.mark_saved(); + } + } /// Keep what interaction `it` (taken from this document) changed, as one undo step. pub(crate) fn keep_interaction(&mut self, it: Interaction) { if !Arc::ptr_eq(&it.doc, &self.doc) { diff --git a/crates/engine/src/tests_fontlist.rs b/crates/engine/src/tests_fontlist.rs index 7991e73aa..e66bb1297 100644 --- a/crates/engine/src/tests_fontlist.rs +++ b/crates/engine/src/tests_fontlist.rs @@ -44,3 +44,22 @@ fn refreshing_the_font_list_redraws_type_without_editing_the_document() { assert!(d.revision > revision, "the canvas redraws"); assert!(!d.is_dirty(), "nothing to save"); } + +/// Refreshing the font list while a background save writes the file leaves the document saved +/// once the file is written. +#[test] +fn refreshing_the_font_list_during_a_save_leaves_the_document_saved() { + use crate::cmd::fileio::{SaveMode, save_job, save_plan}; + let mut s = Session::new(); + s.execute("file.new", &json!({"width": 200, "height": 200})).unwrap(); + s.execute("text.create", &json!({"x": 10, "y": 40, "text": "Type", "font": "No Such Font"})).unwrap(); + let uid = s.doc().unwrap().uid; + let plan = save_plan(&s, SaveMode::SaveAs, &json!({"path": "/x/type.vectorcraft", "modified": 777})).unwrap(); + let job = save_job(&mut s, plan).unwrap(); + s.execute("text.rescanFonts", &json!({})).unwrap(); + job.write(|_, _| Ok(())).unwrap(); + job.complete(&mut s, uid); + let d = s.doc().unwrap(); + assert_eq!(d.doc.metadata.modified, Some(777)); + assert!(!d.is_dirty()); +} diff --git a/crates/engine/src/tests_persp_text.rs b/crates/engine/src/tests_persp_text.rs index 24ccc4b70..88c9ac1fe 100644 --- a/crates/engine/src/tests_persp_text.rs +++ b/crates/engine/src/tests_persp_text.rs @@ -141,6 +141,37 @@ fn edit_text_shows_it_flat_in_isolation_then_projects_it_again() { assert!(s2.execute("perspective.editText", &json!({"id": rect.0})).is_err()); } +/// Edit Text ending while a background save writes the file leaves the document saved once the +/// file is written: the flat view is never saved. Saved while editing and edited after: undoing +/// the edit once Edit Text has ended is back at what the file holds. +#[test] +fn ending_edit_text_during_a_save_leaves_the_document_saved() { + use crate::cmd::fileio::{SaveMode, save_job, save_plan, save_with}; + let (mut s, _) = persp_session(); + let id = persp_text(&mut s); + s.execute("select.set", &json!({"ids": [id.0]})).unwrap(); + s.execute("perspective.editText", &json!({})).unwrap(); + let uid = s.doc().unwrap().uid; + let plan = save_plan(&s, SaveMode::SaveAs, &json!({"path": "/x/type.vectorcraft", "modified": 777})).unwrap(); + let job = save_job(&mut s, plan).unwrap(); + s.execute("object.exitIsolation", &json!({})).unwrap(); + job.write(|_, _| Ok(())).unwrap(); + job.complete(&mut s, uid); + assert!(node(&s, id).projection().is_some(), "projected again"); + let st = s.doc().unwrap(); + assert_eq!(st.doc.metadata.modified, Some(777)); + assert!(!st.is_dirty()); + s.execute("perspective.editText", &json!({})).unwrap(); + let plan = save_plan(&s, SaveMode::Save, &json!({"modified": 778})).unwrap(); + save_with(&mut s, plan, |_, _| Ok(())).unwrap(); + assert!(!s.doc().unwrap().is_dirty()); + s.execute("text.setText", &json!({"id": id.0, "text": "HELLO WORLD"})).unwrap(); + s.execute("object.exitIsolation", &json!({})).unwrap(); + assert!(s.doc().unwrap().is_dirty()); + s.execute("edit.undo", &json!({})).unwrap(); + assert!(!s.doc().unwrap().is_dirty()); +} + #[test] fn symbols_in_perspective_stay_instances() { let (mut s, rect) = persp_session(); diff --git a/crates/engine/src/tests_puppetwarp.rs b/crates/engine/src/tests_puppetwarp.rs index fab70c7ae..6730b5464 100644 --- a/crates/engine/src/tests_puppetwarp.rs +++ b/crates/engine/src/tests_puppetwarp.rs @@ -213,6 +213,39 @@ fn a_new_selection_clears_the_pins() { assert_eq!(p2["pins"], p2["moved"]); } +/// Selecting other art while a background save writes the file drops the pins and leaves the +/// document saved once the file is written; an edit made meanwhile leaves it modified until it is +/// undone. +#[test] +fn a_new_selection_during_a_save_leaves_the_document_saved() { + use crate::cmd::fileio::{SaveMode, save_job, save_plan}; + let mut s = session(); + let a = rect(&mut s, 100.0, 100.0, 300.0, 60.0); + let b = rect(&mut s, 100.0, 300.0, 50.0, 50.0); + s.select_tool("puppetWarp", ViewInfo::default()).unwrap(); + let uid = s.doc().unwrap().uid; + for edit in [false, true] { + s.execute("select.set", &json!({"ids": [a.0]})).unwrap(); + click(&mut s, 250.0, 150.0); + assert!(s.doc().unwrap().doc.puppet.is_some()); + let plan = save_plan(&s, SaveMode::SaveAs, &json!({"path": "/x/pins.vectorcraft", "modified": 777})).unwrap(); + let job = save_job(&mut s, plan).unwrap(); + s.execute("select.set", &json!({"ids": [b.0]})).unwrap(); + if edit { + s.execute("object.move", &json!({"dx": 10, "dy": 0})).unwrap(); + } + job.write(|_, _| Ok(())).unwrap(); + job.complete(&mut s, uid); + let st = s.doc().unwrap(); + assert_eq!(st.doc.metadata.modified, Some(777)); + assert!(st.doc.puppet.is_none(), "the pins stay dropped"); + assert_eq!(st.is_dirty(), edit, "modified only by the edit"); + } + // Undoing the edit is back at what the file holds. + s.execute("edit.undo", &json!({})).unwrap(); + assert!(!s.doc().unwrap().is_dirty()); +} + #[test] fn puppet_warp_works_on_groups_compound_paths_and_open_strokes() { for kind in ["group", "compound", "open"] { diff --git a/crates/engine/src/tests_recovery.rs b/crates/engine/src/tests_recovery.rs index 37f5ae81c..3d732ab5c 100644 --- a/crates/engine/src/tests_recovery.rs +++ b/crates/engine/src/tests_recovery.rs @@ -88,6 +88,22 @@ fn recovery_save_writes_modified_documents_once() { assert!(offered(&mut s).is_empty()); } +/// Selecting other art drops the Puppet Warp pins, which a copy never holds: no new copy is written. +#[test] +fn dropping_puppet_warp_pins_writes_no_new_copy() { + let (mut s, _) = session(); + let id = s.doc().unwrap().selection.objects[0]; + s.execute("object.puppetWarp", &json!({"id": id.0, "pins": [[35, 20]], "moved": [[35, 20]], "rest": true})).unwrap(); + assert!(s.doc().unwrap().doc.puppet.is_some()); + let r = s.execute("file.recovery.save", &json!({})).unwrap(); + assert_eq!(r["saved"].as_array().unwrap().len(), 1, "{r}"); + s.execute("select.none", &json!({})).unwrap(); + assert!(s.doc().unwrap().doc.puppet.is_none()); + let again = s.execute("file.recovery.save", &json!({})).unwrap(); + assert!(again["saved"].as_array().unwrap().is_empty(), "{again}"); + assert_eq!(again["skipped"][0]["reason"], "unchanged since its last recovery copy"); +} + #[test] fn a_crash_leaves_copies_that_restore_modified_and_titled() { let (mut s, store) = session(); diff --git a/crates/ui-egui/src/graphics.rs b/crates/ui-egui/src/graphics.rs index 7268fb273..e246f987e 100644 --- a/crates/ui-egui/src/graphics.rs +++ b/crates/ui-egui/src/graphics.rs @@ -97,7 +97,7 @@ impl VectorcraftApp { if let Err(e) = self.session.execute("file.recovery.save", &serde_json::json!({})) { log::warn!("vectorcraft: no recovery copies after losing the graphics ({why}): {e}"); } - self.session.documents().iter().filter(|st| st.is_dirty()).all(|st| st.recovery.as_ref().is_some_and(|c| Arc::ptr_eq(&c.doc, &st.doc))) + self.session.documents().iter().filter(|st| st.is_dirty()).all(|st| st.recovery.as_ref().is_some_and(|c| st.same_content(&c.doc))) } /// Textures live in one egui context. When the host hands the app a new one (it started again