Repository navigation
Conversation
FredApps
left a comment
There was a problem hiding this comment.
Approving at head 39e1126, with one open CI question below.
Read the diff: WindowState::keep copies strings between storage keys and repair reads viewport info with let Some(rect) = rect else { return }. There is no indexing, unwrap or unsafe. The module is desktop-only (cfg(not(target_arch = "wasm32"))) and repair runs once. A minimized close restores the last shown settings, or removes the entry when none exist, and a window at egui's 64-point floor in both directions gets a usable size.
Validated on a Linux validation host (detached worktree at this SHA, 2 cargo jobs):
cargo fmt --all -- --check: passcargo test --locked -p pdfcraft-ui-egui --lib window_state: 3 passed, 0 failedcargo test --locked -p pdfcraft-ui-egui --test editing: 129 passed, 0 failed, 4 ignoredcargo clippy --locked -p pdfcraft-ui-egui -p pdfcraft --all-targets -- -D warnings: passcargo check --locked -p pdfcraft-ui-egui --target wasm32-unknown-unknown: passcargo xtask layers: pass- Red check: with
keepno longer putting the shown settings back,a_minimized_window_keeps_the_last_shown_settingsfails (window_state.rs:97).
CI note: the hosted test (windows-latest) check failed on image_pixels_follow_the_frame_while_a_corner_is_dragged (tests/editing.rs:4243, "the picture is drawn in the frame ... not only at []"). That test and the code it covers are not touched by this PR, it passes in my Linux run above, and the failure looks like a picture-not-yet-loaded timing race. I did not reproduce it on Windows or confirm it on main, so a maintainer should re-run that job.
Not verified: the real Windows minimize-and-close behaviour (I rely on the author's manual run), macOS and other Linux window managers, and the hosted CI checks other than as noted.
eframe stores the window's size and place just before App::save. A minimized window has none: Windows reports 0 x 0 at (-32000, -32000), and the next start opened at egui's 64-point floor in the screen's corner (seen on a 6K display at 225%: a 144 x 206 px window). While the window is minimized the app now puts back the last settings it was shown with (or none), and a window that opens at that floor from settings saved before this fix is given a usable size. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RUNQSPVXx3dDBabRZHAnjz
39e1126 to
30c174b
Compare
FredApps
left a comment
There was a problem hiding this comment.
Re-reviewed rebase30c174be9e723ff5f9039c47eb3ee5586283c278 against my prior approved39e11261575e55bf96d49e0ce4784781c304ca92. All three branch patches are unchanged apart from hunk positions; window_state.rs retains the exact blob. I checked the two upstream lib.rs changes (trusted certificate application and window title lookup) and found no new scoped interaction blocker. Hosted CI38893960807 succeeded. All three window_state tests and image_pixels_follow_the_frame_while_a_corner_is_dragged explicitly passed on Ubuntu and Windows. No new local test or real desktop minimize/close qualification was performed.
…er saves a 0 x 0 window
What this changes
Closing PdfCraft while it is minimized (from the taskbar's "Close window", Alt+F4 on the taskbar button, or a sign-out) saved a window the next start could hardly use. eframe stores the window's settings just before
App::save, from the window as it is then; Windows reports a minimized window as 0 × 0 at (-32000, -32000). egui-winit then opens the next start at its 64-point floor (clamp_size_to_sane_values) and clamps the position into the screen's corner. Found while testing on a 6K display at 225%: a 172 × 223 px window at (0, 0), and the 820 × 520 minimum size did not apply to the restored size. Reproduced on current main by minimizing and closing.window_state::WindowState(desktop only) remembers the settings eframe stored while the window was shown, starting from those it started with. InApp::save, while the window is minimized, it puts them back; if there are none, it removes the entry so the next start opens at the default size.apps/pdfcraftseeds the state fromcc.storage. Nothing changes for a window closed while shown.How you verified it
cargo fmt --check;cargo clippy -p pdfcraft-ui-egui -p pdfcraft --all-targets -- -D warnings;cargo check -p pdfcraft-ui-egui --target wasm32-unknown-unknown(no new warnings)cargo test -p pdfcraft-ui-egui --lib window_state(3 tests)ShowWindow, close withWM_CLOSE) and readingapp.ron:inner_size_points: (0, 0)at -32000, and the next start is 172 × 223 px;Tests.
a_minimized_window_keeps_the_last_shown_settingsandwith_nothing_shown_yet_a_minimized_save_stores_no_window(with a map-backedeframe::Storage), anda_window_at_eguis_floor_gets_a_usable_size(a 64 × 64 window gets the size, an 820 × 520 one chosen by the user and a minimized one don't). The first fails ifkeepstops putting the settings back.Never-crash
unwrap/expect/panic!outside tests; nounsafeProvenance
🤖 Generated with Claude Code
https://claude.ai/code/session_01RUNQSPVXx3dDBabRZHAnjz