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

Reload DPI on PropertyChange #3222

Merged
merged 4 commits into from
Nov 24, 2023
Merged

Reload DPI on PropertyChange #3222

merged 4 commits into from
Nov 24, 2023

Conversation

notgull
Copy link
Member

@notgull notgull commented Nov 12, 2023

Supersedes #2874, fixes #1228

  • Tested on all platforms changed
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

Supersedes #2874, fixes #1228

Signed-off-by: John Nunley <dev@notgull.net>
@notgull notgull added DS - x11 C - waiting on maintainer A maintainer must review this code labels Nov 14, 2023
@frebib
Copy link

frebib commented Nov 20, 2023

This is amazing! Thank you 😍
I don't know if it's easy to do, but the cursor size should probably update at the same. Once this is merged, I'll test it and raise a feature request depending on how it goes

@notgull
Copy link
Member Author

notgull commented Nov 21, 2023

I don't know if it's easy to do, but the cursor size should probably update at the same.

How would this be done?

Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

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

You'd need a changelog entry for that.

src/platform_impl/linux/x11/event_processor.rs Outdated Show resolved Hide resolved
@kchibisov
Copy link
Member

I don't know if it's easy to do, but the cursor size should probably update at the same. Once this is merged, I'll test it and raise a feature request depending on how it goes

Cursors are server side on X11, so it's up to it to resize the cursor, not winit.

Signed-off-by: John Nunley <dev@notgull.net>
@notgull
Copy link
Member Author

notgull commented Nov 23, 2023

The error here appears to be spurious

Comment on lines 1433 to 1441
for (_, window) in wt.windows.borrow().iter() {
if let Some(window) = window.upgrade() {
window.refresh_dpi_for_monitor(
&new_monitor,
maybe_prev_scale_factor,
&mut *callback,
)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I think you can move upgrade part into iterator by doing filter_map? So it'll be even less nesting.

Comment on lines 1419 to 1423
if let Some(prev_list) = prev_list {
let new_list = wt
.xconn
.available_monitors()
.expect("Failed to get monitor list");
Copy link
Member

Choose a reason for hiding this comment

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

This really demands early return with match.

@kchibisov kchibisov removed the C - waiting on maintainer A maintainer must review this code label Nov 23, 2023
@kchibisov kchibisov added this to the Version 0.29.4 milestone Nov 23, 2023
Signed-off-by: John Nunley <dev@notgull.net>
Signed-off-by: John Nunley <dev@notgull.net>
@kchibisov kchibisov merged commit b3c87ca into master Nov 24, 2023
50 checks passed
@kchibisov kchibisov deleted the notgull/p1 branch November 24, 2023 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

Allow changing X11 DPI at runtime
3 participants