diff --git a/Cargo.lock b/Cargo.lock --- a/Cargo.lock +++ b/Cargo.lock @@ -283,6 +283,7 @@ dependencies = [ "artcraft-launcher-ui-egui", "eframe", "egui", + "egui-winit", "env_logger", "image", "log", diff --git a/apps/artcraft-launcher/Cargo.toml b/apps/artcraft-launcher/Cargo.toml --- a/apps/artcraft-launcher/Cargo.toml +++ b/apps/artcraft-launcher/Cargo.toml @@ -19,6 +19,8 @@ local-mirror = ["artcraft-launcher-native/local-mirror"] [dependencies] artcraft-launcher-core = { workspace = true } artcraft-launcher-ui-egui = { workspace = true } +# Persist only valid native window geometry; eframe already uses this crate. +egui-winit = { version = "0.36", default-features = false, features = ["serde"] } artcraft-launcher-native = { workspace = true } eframe = { workspace = true, features = ["default_fonts", "wgpu", "accesskit", "wayland", "x11", "persistence", "links"] } egui = { workspace = true } diff --git a/apps/artcraft-launcher/src/main.rs b/apps/artcraft-launcher/src/main.rs --- a/apps/artcraft-launcher/src/main.rs +++ b/apps/artcraft-launcher/src/main.rs @@ -13,6 +13,7 @@ mod spaces; #[cfg(target_os = "linux")] mod system_theme; mod tray; +mod window_state; use std::path::PathBuf; use std::process::ExitCode; @@ -30,6 +31,7 @@ use eframe::wgpu::rwh::{HasDisplayHandle, RawDisplayHandle}; const APP_ID: &str = "ai.storyteller.artcraft-launcher"; const SETTINGS_KEY: &str = "settings"; +const MIN_WINDOW_SIZE: [f32; 2] = [720.0, 500.0]; type Wake = Arc>>; @@ -83,6 +85,13 @@ fn main() -> ExitCode { let backend_waker = waker.clone(); let settings_path = settings_file::path(APP_ID); + if let Some(path) = settings_path.as_deref() { + match settings_file::discard_invalid_window(path) { + Ok(true) => log::warn!("discarded invalid persisted window geometry; starting with the default window size"), + Ok(false) => {} + Err(e) => log::warn!("couldn't repair persisted window geometry: {e}"), + } + } let mut launcher = Launcher::new(Box::new(NativeBackend::new(move || backend_waker.wake())), starts, settings_path.clone()); #[cfg(target_os = "linux")] { @@ -136,7 +145,7 @@ fn native_options(hidden: bool) -> eframe::NativeOptions { .with_title("ArtCraft Launcher") .with_app_id(APP_ID) .with_inner_size([1120.0, 760.0]) - .with_min_inner_size([720.0, 500.0]) + .with_min_inner_size(MIN_WINDOW_SIZE) .with_visible(!hidden); // The header is the title bar (see `DesktopHost::title_bar`). viewport = if cfg!(target_os = "macos") { @@ -622,6 +631,7 @@ struct Shell<'a> { window_hides: bool, visible: bool, quitting: bool, + window_state: window_state::WindowState, /// The "show on the current desktop" setting last applied to the window (macOS). #[cfg_attr(not(target_os = "macos"), allow(dead_code))] applied_follow: Option, @@ -632,7 +642,14 @@ impl<'a> Shell<'a> { let window_hides = window_can_hide(cc.display_handle().ok().map(|d| d.as_raw())); // Started hidden at login but there's no tray to come back from: show the window. let visible = !hidden || launcher.tray.is_none(); - let mut shell = Self { launcher, window_hides, visible: true, quitting: false, applied_follow: None }; + let mut shell = Self { + launcher, + window_hides, + visible: true, + quitting: false, + window_state: window_state::WindowState::new(cc.storage), + applied_follow: None, + }; if !visible { shell.put_away(&cc.egui_ctx); } else if hidden { @@ -680,7 +697,10 @@ impl<'a> Shell<'a> { } impl eframe::App for Shell<'_> { - fn logic(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) { + fn logic(&mut self, ctx: &egui::Context, frame: &mut eframe::Frame) { + // eframe saves the native window (even if minimized) before calling App::save. + // Remember only sane, visible geometry so save() can replace that snapshot. + self.window_state.record(frame, ctx.zoom_factor(), self.visible); let requests = self.launcher.tick(ctx); #[cfg(target_os = "macos")] if self.applied_follow != Some(self.launcher.ui.settings.show_on_current_desktop) { @@ -722,6 +742,7 @@ impl eframe::App for Shell<'_> { fn save(&mut self, storage: &mut dyn eframe::Storage) { eframe::set_value(storage, SETTINGS_KEY, &self.launcher.ui.settings); + self.window_state.save(storage); } fn clear_color(&self, visuals: &egui::Visuals) -> [f32; 4] { @@ -965,7 +986,14 @@ mod tests { fn quitting_overrides_a_close_to_the_tray() { let (mut launcher, _show, _) = launcher(); let ctx = launcher.windowless.clone(); - let mut shell = Shell { launcher: &mut launcher, window_hides: false, visible: true, quitting: false, applied_follow: None }; + let mut shell = Shell { + launcher: &mut launcher, + window_hides: false, + visible: true, + quitting: false, + window_state: window_state::WindowState::default(), + applied_follow: None, + }; shell.put_away(&ctx); assert!(shell.launcher.closed_to_tray); shell.quit(&ctx); @@ -976,7 +1004,14 @@ mod tests { fn a_window_that_hides_is_not_closed_to_the_tray() { let (mut launcher, _show, _) = launcher(); let ctx = launcher.windowless.clone(); - let mut shell = Shell { launcher: &mut launcher, window_hides: true, visible: true, quitting: false, applied_follow: None }; + let mut shell = Shell { + launcher: &mut launcher, + window_hides: true, + visible: true, + quitting: false, + window_state: window_state::WindowState::default(), + applied_follow: None, + }; shell.put_away(&ctx); assert!(!shell.visible); assert!(!shell.launcher.closed_to_tray); diff --git a/apps/artcraft-launcher/src/settings_file.rs b/apps/artcraft-launcher/src/settings_file.rs --- a/apps/artcraft-launcher/src/settings_file.rs +++ b/apps/artcraft-launcher/src/settings_file.rs @@ -8,6 +8,8 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; +use eframe::Storage; + /// Where eframe keeps its state for `app_id`, as it picks it for a window with that app id. pub fn path(app_id: &str) -> Option { eframe::storage_dir(app_id).map(|dir| dir.join("app.ron")) @@ -18,6 +20,23 @@ pub fn load(path: &Path, key: &str) -> Option eframe::get_value(&RonFile::read(path), key) } +/// Remove window geometry that cannot safely be restored by eframe, without touching user +/// settings or egui memory. Called before eframe opens the window on each launch. +pub fn discard_invalid_window(path: &Path) -> Result { + const WINDOW_KEY: &str = "window"; + let mut file = RonFile::read(path); + if file.get_string(WINDOW_KEY).is_none() { + return Ok(false); + } + let valid = eframe::get_value::(&file, WINDOW_KEY).is_some_and(|settings| crate::window_state::is_valid(&settings)); + if valid { + return Ok(false); + } + file.remove_string(WINDOW_KEY); + file.write(path)?; + Ok(true) +} + /// Stores `value` under `key`, keeping what else the file holds. pub fn save(path: &Path, key: &str, value: &T) -> Result<(), String> { let mut file = RonFile::read(path); @@ -127,6 +146,40 @@ mod tests { assert_eq!(std::fs::read(&tmp).unwrap(), b"another writer's data"); } + #[test] + fn discards_minimized_window_but_keeps_other_settings() { + let dir = Scratch::new("invalid-window"); + let window = "(inner_size_points:Some((x:0.0,y:0.0)),outer_position_pixels:Some((x:-32000.0,y:-32000.0)))"; + let mut file = RonFile::default(); + file.set_string("window", window.to_owned()); + file.set_string("settings", "(notify:true)".to_owned()); + file.set_string("egui", "remember my layout".to_owned()); + assert_eq!(file.write(&dir.file()), Ok(())); + assert_eq!(discard_invalid_window(&dir.file()), Ok(true)); + let restored = RonFile::read(&dir.file()); + assert_eq!(restored.get_string("window"), None); + assert_eq!(restored.get_string("settings").as_deref(), Some("(notify:true)")); + assert_eq!(restored.get_string("egui").as_deref(), Some("remember my layout")); + } + + #[test] + fn keeps_valid_window_geometry_unchanged() { + let dir = Scratch::new("valid-window"); + let window = "(inner_size_points:Some((x:1120.0,y:760.0)),outer_position_pixels:Some((x:150.0,y:80.0)))"; + let mut file = RonFile::default(); + file.set_string("window", window.to_owned()); + assert_eq!(file.write(&dir.file()), Ok(())); + assert_eq!(discard_invalid_window(&dir.file()), Ok(false)); + assert_eq!(RonFile::read(&dir.file()).get_string("window").as_deref(), Some(window)); + } + + #[test] + fn no_window_state_does_not_create_storage_file() { + let dir = Scratch::new("no-window"); + assert_eq!(discard_invalid_window(&dir.file()), Ok(false)); + assert!(!dir.0.exists()); + } + #[test] fn a_missing_or_broken_file_has_no_settings() { let dir = Scratch::new("settings-broken"); diff --git a/apps/artcraft-launcher/src/window_state.rs b/apps/artcraft-launcher/src/window_state.rs new file mode 100644 --- /dev/null +++ b/apps/artcraft-launcher/src/window_state.rs @@ -0,0 +1,136 @@ +//! Keep the last usable native window geometry instead of eframe's minimized snapshot. +//! +//! eframe 0.36.2 writes its own `window` entry before calling `App::save`. On Windows, +//! a minimized window may report a zero inner size or off-screen coordinates. Preserve +//! the last valid geometry and replace that entry when the launcher is saved. + +use eframe::Storage; +use egui_winit::WindowSettings; + +const WINDOW_KEY: &str = "window"; + +/// A missing or tiny inner size must not override our initial 1120 x 760 viewport. +pub fn is_valid(settings: &WindowSettings) -> bool { + let Some(size) = settings.inner_size_points() else { return false }; + let [min_width, min_height] = crate::MIN_WINDOW_SIZE; + size.x.is_finite() && size.y.is_finite() && size.x >= min_width && size.y >= min_height +} + +#[derive(Default)] +pub struct WindowState { + last_valid: Option, +} + +impl WindowState { + pub fn new(storage: Option<&dyn Storage>) -> Self { + let last_valid = storage.and_then(|storage| eframe::get_value(storage, WINDOW_KEY)).filter(is_valid); + Self { last_valid } + } + + /// Remember actual native geometry only while the window can be safely measured. + pub fn record(&mut self, frame: &eframe::Frame, zoom_factor: f32, visible: bool) { + let Some(window) = frame.winit_window() else { return }; + if !visible || window.is_minimized() == Some(true) || window.is_visible() == Some(false) { + return; + } + let geometry = WindowSettings::from_window(zoom_factor, window); + if is_valid(&geometry) { + self.last_valid = Some(geometry); + } + } + + /// eframe wrote its possibly minimized snapshot *before* `App::save`. + pub fn save(&self, storage: &mut dyn Storage) { + if let Some(geometry) = &self.last_valid { + eframe::set_value(storage, WINDOW_KEY, geometry); + } else { + // On an initial minimized start with no valid frame, let the next launch + // use the default 1120 x 760 size rather than a zero-sized snapshot. + storage.remove_string(WINDOW_KEY); + } + } +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use super::*; + + #[derive(Default)] + struct TestStorage(HashMap); + + impl Storage for TestStorage { + fn get_string(&self, key: &str) -> Option { + self.0.get(key).cloned() + } + + fn set_string(&mut self, key: &str, value: String) { + self.0.insert(key.to_owned(), value); + } + + fn remove_string(&mut self, key: &str) { + self.0.remove(key); + } + + fn flush(&mut self) {} + } + + #[test] + fn only_accepts_usable_size() { + for (width, height, expected) in [ + (1120.0, 760.0, true), + (720.0, 500.0, true), + (0.0, 0.0, false), + (64.0, 64.0, false), + (719.0, 500.0, false), + (720.0, 499.0, false), + (-1.0, 760.0, false), + ] { + let serialized = format!("(inner_size_points:Some((x:{width:?},y:{height:?})))"); + let candidate = ron::from_str::(&serialized); + assert_eq!(candidate.as_ref().ok().map(is_valid), Some(expected), "{serialized}"); + } + assert!(!is_valid(&WindowSettings::default())); + } + + #[test] + fn a_valid_size_without_saved_coordinates_is_accepted() { + let window = "(inner_size_points:Some((x:1120.0,y:760.0)))"; + let mut storage = TestStorage::default(); + storage.set_string(WINDOW_KEY, window.to_owned()); + let state = WindowState::new(Some(&storage)); + assert!(state.last_valid.is_some()); + } + + #[test] + fn ignores_invalid_geometry_already_in_storage() { + let mut storage = TestStorage::default(); + storage.set_string(WINDOW_KEY, "(inner_size_points:Some((x:0.0,y:0.0)))".to_owned()); + let state = WindowState::new(Some(&storage)); + state.save(&mut storage); + assert_eq!(storage.get_string(WINDOW_KEY), None); + } + + #[test] + fn saved_good_geometry_replaces_a_minimized_snapshot() { + let mut storage = TestStorage::default(); + let last_valid = "(inner_size_points:Some((x:1120.0,y:760.0)),outer_position_pixels:Some((x:150.0,y:80.0)))"; + storage.set_string(WINDOW_KEY, last_valid.to_owned()); + let state = WindowState::new(Some(&storage)); + // Simulate what eframe did just before calling App::save. + storage.set_string(WINDOW_KEY, "(inner_size_points:Some((x:0.0,y:0.0)))".to_owned()); + state.save(&mut storage); + let saved = storage.get_string(WINDOW_KEY).and_then(|s| ron::from_str::(&s).ok()); + assert_eq!(saved.and_then(|s| s.inner_size_points()), Some(egui::vec2(1120.0, 760.0))); + } + + #[test] + fn no_valid_geometry_removes_minimized_snapshot() { + let mut storage = TestStorage::default(); + let state = WindowState::new(None); + storage.set_string(WINDOW_KEY, "(inner_size_points:Some((x:0.0,y:0.0)))".to_owned()); + state.save(&mut storage); + assert_eq!(storage.get_string(WINDOW_KEY), None); + } +}