mirror of
https://github.com/storytold/vectorcraft.git
synced 2026-10-30 12:47:13 -06:00
Merge pull request #4641 from mockydevs/fix/prefs-survive-relaunch-4625
Fix #4625: UI settings survive a relaunch and a second window
This commit is contained in:
BFlat
@@ -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