rust-windowing / winit

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

linux (kwin+x11): window opening at full height #745

Open tones111 opened 5 years ago

tones111 commented 5 years ago

I'm trying to find the root cause of alacritty #1817. I've determined that the issue was introduced in 1b74822cfc0cb7fba4ed4940a3faae9499fcda95. The problem does not occur on wayland unless running over XWayland.

If I build alacritty against that commit I've found that commenting out the following block from src/platform/linux/x11/mod.rs fixes the incorrect window size.

/*                    if last_hidpi_factor != new_hidpi_factor {
                        events.dpi_changed = Some(WindowEvent::HiDpiFactorChanged(new_hidpi_factor));
                        let (new_width, new_height, flusher) = window.adjust_for_dpi(
                            last_hidpi_factor,
                            new_hidpi_factor,
                            width,
                            height,
                        );
                        flusher.queue();
                        shared_state_lock.dpi_adjusted = Some((new_width, new_height));
                    }
*/

I haven't had much luck minimizing the problem further. I have also built against v0.18.0 and the problem still exists. Removing the equivalent section of code doesn't help either. Any help in narrowing down this issue is appreciated.

mitchmindtree commented 5 years ago

@tones111 this could possibly be due to the DPI refactor that happened. On my linux machine, I use Gnome X11 with a hidpi display. Ever since the DPI refactor (which seems for the most part to have been for the best!) I've had to manually specify the DPI factor for winit as winit seems to retrieve and use my hardware DPI factor even though the X11 desktop is already doing all the scaling itself (which is annoying but a whole other unrelated rabbit hole).

I recommend reading these docs thoroughly. In my case, I have to specify the env var WINIT_HIDPI_FACTOR=1 before running winit programs, e.g.

WINIT_HIDPI_FACTOR=1 cargo run --release

I suspect you may have a similar case to me?

Quintasan commented 5 years ago

@mitchmindtree setting WINIT_HIDPI_FACTOR=1 made Alacritty respect the window.dimensions.lines setting. Thanks

francesca64 commented 5 years ago

I think I'm convinced that merging #606 (and making sure the old behavior is still a fallback / can be opted into) is the right step forward. While X11's lack of proper mixed DPI support sucks, I think it's generally more important to follow the conventions of the platform.

(Plus, I don't use Linux as my main OS anymore, so I don't really have a horse in this race. People who stick with X11 presumably like the way it does things.)

@chrisduerr does this seem reasonable to you? I imagine you have the most awareness of the user impact for something like this.

chrisduerr commented 5 years ago

@francesca64 I think most people do not expect the current behavior of DPI in winit on X11 and it comes as a surprise to them. That being said, I don't think it's necessarily a bad idea, since WINIT_HIDPI_FACTOR exists, getting the 'normal' behavior is trivial.

If you remember the original issue/PR for the DPI rewrite, the Xft.dpi was actually mentioned and I think the way it was mentioned there is much more reasonable. Relying just on the Xft.dpi makes it impossible to get per-monitor DPI right. However, we can still use the Xft.dpi on top of the proper scaled DPI for each monitor. So we can calculate the DPI correctly, then just scale each display by the Xft.dpi factor (calculated against 'default').

If people would want the 'expected' linux behavior they could still use WINIT_HIDPI_FACTOR=1 and then have the base for each be 1.0 while the Xft.dpi scaling could still apply even if the hidpi factor is set to 1.

An alternative would be to make the current behavior opt-in like QT. I'd still go with everything exactly the way I've just described it, however the default behavior would be to ignore per monitor DPI scaling and using a separate command line flag enables it.

If this is all about behaving exactly the way an X11 user would expect things to work, then chances are per-monitor DPI should be ignored by default. But I feel like the way it is right now with Xft.dpi applied on top of it would actually be the better solution, but many people would likely have to get used to it.

Quintasan commented 5 years ago

@chrisduerr I believe keeping the normal X11 behaviour would stay in line with the principle of least astonishment. It took me at least 30 minutes to figure out WHY Alacritty stopped respecting my alacritty.yml lines setting. Opt-in sounds like the best option for now and it could be mentioned in the README.

tones111 commented 5 years ago

Playing with the WINIT_HIDPI_FACTOR env variable showed some interesting behavior. Setting it to 1 did allow the alacritty window to be prorpotioned as I would expect. Increasing the value to 1.1, 1.2, 1.3, and I can see the window scaling in size. However, setting a value > 1.315 results in the unexpected vertical dimension initially described. This explains why my calculated dpi of 1.5 is problematic.

There appears to be quite a back story here. Does anyone know why the dpi value only scales up to a certain point? Thanks

chrisduerr commented 5 years ago

@Quintasan Line settings not being respected is a bug and unrelated to the way winit handles DPI. There's absolutely no reason why it wouldn't be possible to have that work while keeping the current way DPI is handled.