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

Position persistence and sane clamping to still-available monitors for Windows #2583

Merged
merged 20 commits into from
Feb 4, 2023

Conversation

Shel-M
Copy link
Contributor

@Shel-M Shel-M commented Jan 13, 2023

Closes #1469.

Doesn't know where the task bar is, and that doesn't seem to be available in winit, so clamping vertically may result in some of the window being behind it.

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.

Nice with this fix, but the code needs cleanup.

Have you tested it with pixels_per_point != 1 ?

crates/egui-winit/src/window_settings.rs Outdated Show resolved Hide resolved
crates/egui-winit/src/window_settings.rs Outdated Show resolved Hide resolved
crates/egui-winit/src/window_settings.rs Outdated Show resolved Hide resolved
crates/egui-winit/src/window_settings.rs Outdated Show resolved Hide resolved
crates/egui-winit/src/window_settings.rs Outdated Show resolved Hide resolved
crates/egui-winit/src/window_settings.rs Outdated Show resolved Hide resolved
@Shel-M
Copy link
Contributor Author

Shel-M commented Jan 23, 2023

Nice with this fix, but the code needs cleanup.

Have you tested it with pixels_per_point != 1 ?

That's a good point, I have not. I'll give it a look soon.

@Shel-M
Copy link
Contributor Author

Shel-M commented Jan 23, 2023

Nice with this fix, but the code needs cleanup.
Have you tested it with pixels_per_point != 1 ?

That's a good point, I have not. I'll give it a look soon.

I have tested this on a display with resolution scaling applied. I had already done that, but was second guessing that I had done it properly.

Shel-M and others added 7 commits January 23, 2023 12:22
Discovered an issue where putting the monitor off a non-primary monitor to the left causes the position to be off the monitor x and y range, clamping to the primary instead of the non-primary.
…any part of the window had been visible on a remaining monitor.
Rather than just wrap in blocks, I kind of prefer the more explicit if cfg! call for line 114.

CHANGELOG.md - correct a missing paren I noticed
@Shel-M Shel-M requested a review from emilk January 24, 2023 18:55
emilk suggested changelog formatting

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
@Shel-M Shel-M requested a review from emilk January 25, 2023 20:23
Nykseli added a commit to Nykseli/egui that referenced this pull request Feb 1, 2023
* Patch is a modified version of emilk#2583
@emilk emilk merged commit 53f8e40 into emilk:master Feb 4, 2023
lictex pushed a commit to lictex/egui that referenced this pull request Feb 5, 2023
…r Windows (emilk#2583)

* Attempt to fix monitor clamping on Windows so window positions can be restored between sessions.

* Missed a change.

* Renamed variables, reorganized some lines of code, and added some more comments.

* Cargo fmt run

* Updated CHANGELOG.md to briefly describe my change

* Updated CHANGELOG.md to briefly describe my change

* Applied suggested fixes from emilk
Discovered an issue where putting the monitor off a non-primary monitor to the left causes the position to be off the monitor x and y range, clamping to the primary instead of the non-primary.

* Fix for matching negative restored window positions. Should clamp if any part of the window had been visible on a remaining monitor.

* Apparently compiler attributes on statements have been marked unstable.
Rather than just wrap in blocks, I kind of prefer the more explicit if cfg! call for line 114.

CHANGELOG.md - correct a missing paren I noticed

* I was being silly, I don't need to clone inner_size_points on line 112

* Cargo fmt run

* Update crates/egui-winit/CHANGELOG.md

emilk suggested changelog formatting

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>

* Update window_settings.rs

Satisfy CI Error

* clippy

---------

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Persistence of window position is not working correctly on Windows and Linux
2 participants