mirror of
https://github.com/storytold/vectorcraft.git
synced 2026-10-30 04:37:29 -06:00
Fix #4625: UI settings survive a relaunch and a second window
Two ways the next launch came back with settings from before: - The workspace, panels, toolbars and other settings kept only in the UI state were written only on quitting (Preferences OK already saved at once, #1747). A session ended any other way lost them. The app now saves within 2 seconds of a change, and only when something changed. - Each launch is its own process, so two windows are common (opening a second file from the file manager). A window wrote back the whole state it started from when it quit, so the one quitting last put back the settings it read at its start, even with nothing changed in it. A window now saves only what it changed since it last read or saved, merged into the file as it is: each UI field, and each engine preference on its own. A setting both windows changed takes the last save. The status line, open dialog and other fields a start resets neither trigger a save nor replace another window's.
This commit is contained in:
Don Omondi
@@ -1,6 +1,6 @@
|
||||
#[cfg(all(target_os = "macos", feature = "accessibility", feature = "wgpu"))]
|
||||
use super::gpu;
|
||||
use super::prefs::save_prefs;
|
||||
use super::prefs::{save_prefs, save_prefs_soon};
|
||||
#[cfg(all(windows, not(target_vendor = "win7")))]
|
||||
use super::windows_stylus;
|
||||
use super::{App, VectorcraftApp, file_access, window};
|
||||
@@ -71,6 +71,9 @@ impl eframe::App for App {
|
||||
// (the system shutting down, the process ended or crashing) used to lose them (#1747).
|
||||
if self.app.take_prefs_changed() {
|
||||
file_access::unconfined(|| save_prefs(&self.app));
|
||||
} else {
|
||||
// The workspace, panels and other UI settings change without the dialog (#4625).
|
||||
file_access::unconfined(|| save_prefs_soon(&self.app));
|
||||
}
|
||||
// Not while a file dialog's sheet has the keyboard: its name field's composition would go.
|
||||
#[cfg(target_os = "macos")]
|
||||
|
||||
@@ -84,24 +84,200 @@ pub(crate) fn load_prefs(app: &mut VectorcraftApp, saved: Option<vectorcraft_ui_
|
||||
app.ui = ui.sanitized();
|
||||
}
|
||||
vectorcraft_ui_egui::prefs_dialog::restore(app);
|
||||
// What this window started from: only what changes after it is this window's to save.
|
||||
set_saved_base(prefs_value(app).ok());
|
||||
}
|
||||
|
||||
/// Save what this window changed since it last read or saved the preferences, merged into the
|
||||
/// file as it is now ([`merge_changes`]): another VectorCraft window open at the same time, which
|
||||
/// read the file before this one saved, then keeps this window's changes when it saves or quits
|
||||
/// (#4625). Nothing is written when nothing changed.
|
||||
pub(crate) fn save_prefs(app: &VectorcraftApp) {
|
||||
if !prefs_enabled() || KEEP_UI_PREFS.load(std::sync::atomic::Ordering::Relaxed) {
|
||||
return;
|
||||
}
|
||||
if let Some(p) = prefs_path() {
|
||||
let _ = std::fs::create_dir_all(p.parent().unwrap_or(std::path::Path::new(".")));
|
||||
if let Ok(bytes) = prefs_bytes(app) {
|
||||
// Preferences are best effort: a failed write keeps the previous file.
|
||||
let _ = fileio::write_atomic(&p, &bytes);
|
||||
}
|
||||
let Some(p) = prefs_path() else { return };
|
||||
let Ok(mine) = prefs_value(app) else { return };
|
||||
let base = saved_base();
|
||||
if base.as_ref().is_some_and(|b| same_lasting(b, &mine)) {
|
||||
return;
|
||||
}
|
||||
// The file as another window may have saved it since (none, or unreadable: this one's own).
|
||||
let on_disk = std::fs::read(&p).ok().and_then(|b| serde_json::from_slice::<serde_json::Value>(&b).ok());
|
||||
let merged = match (base.as_ref(), on_disk) {
|
||||
(Some(base), Some(on_disk)) => merge_changes(base, &mine, on_disk),
|
||||
_ => mine.clone(),
|
||||
};
|
||||
let _ = std::fs::create_dir_all(p.parent().unwrap_or(std::path::Path::new(".")));
|
||||
let Ok(bytes) = serde_json::to_vec_pretty(&merged) else { return };
|
||||
// Preferences are best effort: a failed write keeps the previous file, and the changes are
|
||||
// tried again on the next save.
|
||||
if fileio::write_atomic(&p, &bytes).is_ok() {
|
||||
set_saved_base(Some(mine));
|
||||
}
|
||||
}
|
||||
|
||||
/// How often [`save_prefs_soon`] looks for changes to save.
|
||||
const SAVE_EVERY: std::time::Duration = std::time::Duration::from_secs(2);
|
||||
|
||||
/// Save the preferences at most every [`SAVE_EVERY`] while the app runs, when something changed:
|
||||
/// the workspace, panels, toolbars and the other UI settings kept only in the UI state were saved
|
||||
/// only on quitting, so a session ended any other way (the window closed by the system, the
|
||||
/// process ended or crashing) started the next one from the settings before it (#4625).
|
||||
pub(crate) fn save_prefs_soon(app: &VectorcraftApp) {
|
||||
static LAST: std::sync::Mutex<Option<std::time::Instant>> = std::sync::Mutex::new(None);
|
||||
let Ok(mut last) = LAST.lock() else { return };
|
||||
if last.is_some_and(|t| t.elapsed() < SAVE_EVERY) {
|
||||
return;
|
||||
}
|
||||
*last = Some(std::time::Instant::now());
|
||||
drop(last);
|
||||
save_prefs(app);
|
||||
}
|
||||
|
||||
/// The preferences as this window last read or saved them (`None`: not read yet).
|
||||
static SAVED_BASE: std::sync::Mutex<Option<serde_json::Value>> = std::sync::Mutex::new(None);
|
||||
|
||||
fn saved_base() -> Option<serde_json::Value> {
|
||||
SAVED_BASE.lock().ok().and_then(|b| b.clone())
|
||||
}
|
||||
|
||||
fn set_saved_base(value: Option<serde_json::Value>) {
|
||||
if let Ok(mut b) = SAVED_BASE.lock() {
|
||||
*b = value;
|
||||
}
|
||||
}
|
||||
|
||||
/// Fields of the UI state that a start resets ([`vectorcraft_ui_egui::UiState::sanitized`]): a
|
||||
/// change to them alone isn't saved, and they are never merged over another window's.
|
||||
const PASSING: &[&str] = &["status", "dialog", "flyout", "palette_open", "palette_query", "about", "open_panel"];
|
||||
|
||||
/// Do `a` and `b` hold the same preferences, leaving out what a start resets ([`PASSING`])?
|
||||
fn same_lasting(a: &serde_json::Value, b: &serde_json::Value) -> bool {
|
||||
match (a.as_object(), b.as_object()) {
|
||||
(Some(a), Some(b)) => {
|
||||
let keys: std::collections::BTreeSet<&String> = a.keys().chain(b.keys()).filter(|k| !PASSING.contains(&k.as_str())).collect();
|
||||
keys.into_iter().all(|k| a.get(k) == b.get(k))
|
||||
}
|
||||
_ => a == b,
|
||||
}
|
||||
}
|
||||
|
||||
/// `on_disk` with what changed from `base` to `mine` put over it: each top-level field of the UI
|
||||
/// state, and each engine preference (`engine_prefs`) on its own, so two windows that change
|
||||
/// different settings keep both. A field both changed takes this window's value (it saves last).
|
||||
fn merge_changes(base: &serde_json::Value, mine: &serde_json::Value, on_disk: serde_json::Value) -> serde_json::Value {
|
||||
use serde_json::Value;
|
||||
let (Some(base), Some(mine), Value::Object(mut out)) = (base.as_object(), mine.as_object(), on_disk) else {
|
||||
return mine.clone();
|
||||
};
|
||||
let keys: std::collections::BTreeSet<&String> = base.keys().chain(mine.keys()).collect();
|
||||
for key in keys {
|
||||
if PASSING.contains(&key.as_str()) {
|
||||
if let Some(v) = mine.get(key) {
|
||||
out.entry(key.clone()).or_insert_with(|| v.clone());
|
||||
}
|
||||
continue;
|
||||
}
|
||||
let (was, now) = (base.get(key), mine.get(key));
|
||||
if was == now {
|
||||
continue;
|
||||
}
|
||||
if key == "engine_prefs"
|
||||
&& let (Some(Value::Object(was)), Some(Value::Object(now)), Some(Value::Object(theirs))) = (was, now, out.get_mut(key))
|
||||
{
|
||||
let names: std::collections::BTreeSet<&String> = was.keys().chain(now.keys()).collect();
|
||||
for name in names {
|
||||
match (was.get(name), now.get(name)) {
|
||||
(a, b) if a == b => {}
|
||||
(_, Some(v)) => {
|
||||
theirs.insert(name.clone(), v.clone());
|
||||
}
|
||||
(_, None) => {
|
||||
theirs.remove(name);
|
||||
}
|
||||
}
|
||||
}
|
||||
continue;
|
||||
}
|
||||
match now {
|
||||
Some(v) => {
|
||||
out.insert(key.clone(), v.clone());
|
||||
}
|
||||
None => {
|
||||
out.remove(key);
|
||||
}
|
||||
}
|
||||
}
|
||||
Value::Object(out)
|
||||
}
|
||||
|
||||
/// The preferences file's contents: the UI state with the engine preferences.
|
||||
#[cfg(test)]
|
||||
pub(crate) fn prefs_bytes(app: &VectorcraftApp) -> serde_json::Result<Vec<u8>> {
|
||||
serde_json::to_vec_pretty(&prefs_value(app)?)
|
||||
}
|
||||
|
||||
/// [`prefs_bytes`] as JSON.
|
||||
fn prefs_value(app: &VectorcraftApp) -> serde_json::Result<serde_json::Value> {
|
||||
let mut ui = app.ui.clone();
|
||||
ui.engine_prefs = app.session.prefs.to_json();
|
||||
serde_json::to_vec_pretty(&ui)
|
||||
serde_json::to_value(&ui)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use serde_json::json;
|
||||
|
||||
use super::{merge_changes, same_lasting};
|
||||
|
||||
/// Two windows open at once (#4625): the one that saves last keeps the other's changes and
|
||||
/// adds its own, instead of putting back the preferences it started from.
|
||||
#[test]
|
||||
fn a_window_saves_only_what_it_changed_over_another_windows_save() {
|
||||
let base = json!({"workspace": "Essentials", "toolbar": true, "engine_prefs": {"uiAccent": "blue", "uiScaling": 1.0}});
|
||||
// The other window switched the workspace and the accent, and saved.
|
||||
let on_disk = json!({"workspace": "Painting", "toolbar": true, "engine_prefs": {"uiAccent": "violet", "uiScaling": 1.0}});
|
||||
// This one changed only the UI scaling.
|
||||
let mine = json!({"workspace": "Essentials", "toolbar": true, "engine_prefs": {"uiAccent": "blue", "uiScaling": 1.25}});
|
||||
let merged = merge_changes(&base, &mine, on_disk);
|
||||
assert_eq!(merged, json!({"workspace": "Painting", "toolbar": true, "engine_prefs": {"uiAccent": "violet", "uiScaling": 1.25}}));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_setting_both_windows_changed_takes_the_last_saved() {
|
||||
let base = json!({"workspace": "Essentials", "engine_prefs": {"uiAccent": "blue"}});
|
||||
let on_disk = json!({"workspace": "Painting", "engine_prefs": {"uiAccent": "violet"}});
|
||||
let mine = json!({"workspace": "Typography", "engine_prefs": {"uiAccent": "green"}});
|
||||
assert_eq!(merge_changes(&base, &mine, on_disk), json!({"workspace": "Typography", "engine_prefs": {"uiAccent": "green"}}));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fields_added_or_removed_by_this_window_are_saved() {
|
||||
let base = json!({"docking": {"root": 1}, "engine_prefs": {"old": 1}});
|
||||
let on_disk = json!({"docking": {"root": 1}, "recent_files": ["a.svg"], "engine_prefs": {"old": 1}});
|
||||
let mine = json!({"dock_width": 300.0, "engine_prefs": {"new": 2}});
|
||||
assert_eq!(merge_changes(&base, &mine, on_disk), json!({"dock_width": 300.0, "recent_files": ["a.svg"], "engine_prefs": {"new": 2}}));
|
||||
}
|
||||
|
||||
/// The status line and open dialogs reset at start: they don't make a save, and they don't
|
||||
/// replace another window's.
|
||||
#[test]
|
||||
fn what_a_start_resets_is_left_out() {
|
||||
let base = json!({"status": "", "dialog": null, "workspace": "Essentials"});
|
||||
let mine = json!({"status": "Saved", "dialog": {"kind": "preferences"}, "workspace": "Essentials"});
|
||||
assert!(same_lasting(&base, &mine));
|
||||
let on_disk = json!({"status": "Opened", "dialog": null, "workspace": "Painting"});
|
||||
let mine = json!({"status": "Saved", "dialog": {"kind": "preferences"}, "workspace": "Essentials", "toolbar": false});
|
||||
let merged = merge_changes(&base, &mine, on_disk);
|
||||
assert_eq!(merged, json!({"status": "Opened", "dialog": null, "workspace": "Painting", "toolbar": false}));
|
||||
assert!(!same_lasting(&base, &mine));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_file_that_is_not_an_object_is_replaced() {
|
||||
let base = json!({"workspace": "Essentials"});
|
||||
let mine = json!({"workspace": "Painting"});
|
||||
assert_eq!(merge_changes(&base, &mine, json!([1, 2])), mine);
|
||||
}
|
||||
}
|
||||
|
||||
+2
-1
@@ -1,6 +1,6 @@
|
||||
# UI and interaction parity
|
||||
|
||||
> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-30 · **Change:** minor (Character panel menu › Show Snap to Glyph Options shows the Snap to Glyph section (Baseline, x-height, Glyph Bounds, Proximity Guides, Angular Guides, Anchor Point; `view.snapToGlyph.options`) (#4232); scores and estimates unchanged) · **Target:** Adobe Illustrator 2026 (30.x)
|
||||
> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-30 · **Change:** minor (Preferences: the workspace, panels, toolbars and other UI settings are saved within 2 seconds of a change instead of only on quitting, and a window saves only what it changed, merged into the file as another VectorCraft window left it, so a second window closing last no longer puts back the settings it started with (#4625); scores unchanged) · **Target:** Adobe Illustrator 2026 (30.x)
|
||||
|
||||
## Feature submission evidence
|
||||
|
||||
@@ -361,6 +361,7 @@ separate. Scores and hours unchanged. [Behavior and command details](development
|
||||
|
||||
| Date | Change | Summary |
|
||||
|---|---|---|
|
||||
| 2026-10-30 | minor | Preferences: the workspace, panels, toolbars and other UI settings are saved within 2 seconds of a change instead of only on quitting, and a window saves only what it changed, merged into the file as another VectorCraft window left it, so a second window closing last no longer puts back the settings it started with (#4625). Tests `prefs::tests` in `apps/vectorcraft`; scores and estimates unchanged |
|
||||
| 2026-10-30 | minor | Character panel menu › Show Snap to Glyph Options shows the Snap to Glyph section (Baseline, x-height, Glyph Bounds, Proximity Guides, Angular Guides, Anchor Point; `view.snapToGlyph.options`) (#4232); scores and estimates unchanged |
|
||||
| 2026-10-30 | minor | Contextual Task Bar: with the Direct Selection tool, a path selected whole offers Corner, Smooth (every anchor), Connect (open paths) and Simplify instead of Edit Path and Offset Path, as the public overview lists for that tool (<https://helpx.adobe.com/illustrator/desktop/get-started/learn-the-basics/contextual-task-bar-overview.html>); a compound path under that tool no longer offers Edit Path (#4469). Tests in `crates/ui-egui/src/tests/taskbar.rs`; scores and estimates unchanged |
|
||||
| 2026-10-30 | minor | Align panel menu › Align to Glyph Bounds › Point Text and Area Text align and distribute type by the ink of its glyphs (#4231); the Character and Properties panels' controls for it are not done; scores and estimates unchanged |
|
||||
|
||||
Reference in New Issue
Block a user