Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serialize window maximized state in WindowSettings #5554

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

landaire
Copy link

@landaire landaire commented Dec 31, 2024

A user of my Windows application reported a papercut where the application restores its size on next load, but does not restore its maximized state. This PR fixes that.

To test, I patched https://github.com/emilk/eframe_template to use my local code since I knew that template saves/restores window data. Testing methodology was to simply cargo run, maximize the application, then close the application. cargo run again and the application should start maximized.

Closes #1517.

  • I have followed the instructions in the PR template
    • This is mostly true, I had difficulties running ./scripts/check.sh for some reason. Possibly a bad Python version?

@landaire landaire marked this pull request as ready for review January 2, 2025 20:52
Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code LGTM

@emilk emilk added the eframe Relates to epi and eframe label Jan 2, 2025
Copy link

github-actions bot commented Jan 2, 2025

Preview available at https://egui-pr-preview.github.io/pr/5554-restorewindowmaximizedstate
Note that it might take a couple seconds for the update to show up after the preview_build workflow has completed.

@landaire
Copy link
Author

landaire commented Jan 3, 2025

Just wanted to note I think this fixes #1517 and the last remaining item from #3494. If you can confirm, I can update the PR description to include the magic "closes" text.

Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this leads to a deadlock on Mac when running cargo run -p custom_window_frame

See also #3612

@landaire
Copy link
Author

landaire commented Jan 3, 2025

Will take a look to see if I can resolve the deadlock.

@landaire landaire force-pushed the restore_window_maximized_state branch from cf9d63b to 2d5aeb2 Compare January 3, 2025 19:40
@landaire
Copy link
Author

landaire commented Jan 3, 2025

Left a comment in #3494 and also created rust-windowing/winit#4071.

For now I've removed the erroneous unconditional maximized logic in update_viewport_info. This retains the desired behavior of this diff, while preventing the logic from being invoked on macOS (tested with both cargo run -p custom_window_frame and my original testing steps)

@landaire landaire requested a review from emilk January 3, 2025 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
eframe Relates to epi and eframe
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Maximize Window even if initial_window_size and initial_window_position are set
2 participants