rust-windowing / winit

Window handling library in pure Rust
https://docs.rs/winit/
Apache License 2.0
4.84k stars 902 forks source link

Allow changing X11 DPI at runtime #1228

Closed chrisduerr closed 9 months ago

chrisduerr commented 5 years ago

This has been reported to Alacritty in https://github.com/jwilm/alacritty/issues/2886.

It seems like currently winit does not allow to modify the DPI at runtime, which I can definitely see as desirable for some users. In Alacritty it would be possible to just change the font size, but I feel like it would still be worth it to just change one thing and the whole system properly rescales.

frebib commented 4 years ago

Looks related to https://github.com/rust-windowing/winit/issues/1377

From testing with alacritty, I can't seem to invoke any ScaleFactorChanged events, presumably due to the above issue (reproduce with alacritty --print-events 2>&1 | grep -ie scale and change monitor DPI or move window to another display with different DPI).

At least in X11, I'm leveraging the xsettings behaviour built into GTK to resize at runtime. https://www.freedesktop.org/wiki/Specifications/XSettingsRegistry/ It's definitely not the most scalable solution, but it is at least one source of dpi changed events

frebib commented 4 years ago

I spent some time digging into this. I managed to cobble together something that works, but is definitely sub-optimal. This is the first Rust I've ever "written" so even getting this to work was a challenge. It uses the existing code to reload monitors and trigger ScaleFactorChanged events which is completely overkill for the little work that is actually required. I think this needs restructuring such that monitors can update their own DPI and propagate the events down to the windows, or something like that. I didn't even want to attempt to work out the lifetime/ownership headaches around that. If anyone feels inclined, or motivated to, I'm happy for this to be adapted in a cleaner way so it can be merged. That being said, I'll be using this patch until something better comes along as it will probably work well enough for my changing-dpi needs.

https://github.com/frebib/winit/commit/74d5070c91ff8150c7630dd3b80d66a14cbe5fb6

rhelmot commented 2 years ago

I'm interested in working on this, but I have no familiarity with winit code. Could someone on the winit team give me a high-level explanation of what's unacceptable about the above patch so I can start hacking on it?

frebib commented 2 years ago

Bump? Anyone able to help out with this patch? I've been using https://github.com/frebib/winit/commit/74d5070c91ff8150c7630dd3b80d66a14cbe5fb6 for the last 2 years with zero issues, although it seems to stop working after https://github.com/rust-windowing/winit/commit/c916eb613734b389d7add7efec7f698ca739bae2

frebib commented 2 years ago

I rebased my patch onto v0.27.2 and reverted c916eb613734b389d7add7efec7f698ca739bae2 and it seems to still work. Here is the updated commit ec4af7d42b7e6e4808c3208e278703afb65d564d

asch commented 2 years ago

Any reason why frebib's patch is not merged?

madsmtm commented 2 years ago

It would probably help if they (or someone else) opened a pull request with the patch, it's a bit cumbersome to discuss an implementation otherwise

frebib commented 10 months ago

This is closed, but at least in Alacritty it doesn't work. Whilst this listens for the correct events, it doesn't actually read the new DPI value, so it still doesn't change scale

madsmtm commented 10 months ago

CC @notgull?

notgull commented 10 months ago

It looks like that it's still re-reading the Xft.dpi variables and the CRTC info, so I'm not sure why it wouldn't be updating.

frebib commented 10 months ago

In my hacky original patch, the code reread the X11 resources from X https://github.com/frebib/winit/commit/fb5630332c4e5ab42c0f9ee55b2327c4ee580e3e#diff-c2d9fcab6774ee66ce076daadd1d1d1a376066dfcc6ab6584e0925c9fe61ca47R55

I shamelessly stole this from dunst. I don't know anything about X11 but I doubt this is the only approach to fix this problem. I would guess the issue with the current implementation is that it doesn't read the updated xft.dpi value from xresources because the application has the xresources from when the window was opened.

notgull commented 10 months ago

Good catch! I'll file a patch to fix that

jfly commented 9 months ago

For the record, this is still not working for me on Alacritty 0.13.1, as already described by @frebib here: https://github.com/rust-windowing/winit/pull/3309#issuecomment-1886950866.

@notgull @kchibisov, can we (re)-reopen this issue?

notgull commented 9 months ago

This should be fixed as of #3423, unless Qt/KDE has a way of updating DPI that I don't know about.

notgull commented 9 months ago

Huh, seems like Qt5 uses XSETTINGS as well: https://github.com/qt/qtbase/blob/dev/src/plugins/platforms/xcb/qxcbxsettings.cpp

So I think our bases should be covered, at least for platforms that aren't super weird.