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

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

Open
tones111 opened this issue Dec 20, 2018 · 7 comments

Comments

Projects
None yet
5 participants
@tones111
Copy link

commented Dec 20, 2018

I'm trying to find the root cause of alacritty #1817. I've determined that the issue was introduced in 1b74822. 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

This comment has been minimized.

Copy link
Collaborator

commented Dec 21, 2018

@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

This comment has been minimized.

Copy link

commented Dec 22, 2018

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

@francesca64

This comment has been minimized.

Copy link
Collaborator

commented Dec 22, 2018

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

This comment has been minimized.

Copy link
Contributor

commented Dec 22, 2018

@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

This comment has been minimized.

Copy link

commented Dec 23, 2018

@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

This comment has been minimized.

Copy link
Author

commented Dec 23, 2018

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

This comment has been minimized.

Copy link
Contributor

commented Dec 23, 2018

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.