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

Can't make Guake run under Wayland #1934

Closed
VitalyAnkh opened this issue Oct 17, 2021 · 24 comments · Fixed by #1975
Closed

Can't make Guake run under Wayland #1934

VitalyAnkh opened this issue Oct 17, 2021 · 24 comments · Fixed by #1975

Comments

@VitalyAnkh
Copy link
Contributor

Describe the bug
After updating Guake to 3.8.0, I found it running under x11 rather than wayland. My desktop is Gnome 41 on Wayland, and before 3.8.0 Guake runs under Wayland as I expect. And I didn't set GDK_BACKEND environment variable to not break electron apps.

Expected behavior
Guake runs under Wayland

Actual behavior
Guake doesn't run under Wayland

To Reproduce
Use Gnome 41 on Wayland, install guake 3.8.0, and launch it.


Please run $ guake --support, and paste the results here. Don't put backticks (`) around it! The output already contains Markdown formatting. And make sure you run the command OUTSIDE the Guake.

$ guake --support

Guake Version: 3.8.0

Vte Version: 0.66.0

Vte Runtime Version: 0.66.0


GTK+ Version: 3.24.30

GDK Backend: <GdkX11.X11Display


Desktop Session: gnome


Display: :0

RGBA visual: True

Composited: True

  • Monitor: 0 - XWAYLAND0
    • Geometry: 1920 x 1080 % 2 at 0, 0
    • Size: 600 x 330 mm²
    • Primary: True
    • Refresh rate: 59.980000000000004 Hz
    • Subpixel layout: unknown
@Davidy22
Copy link
Collaborator

Looks like #1821 is the only thing that should have anything to do with x11 between 3.7 and 3.8, and it ensures that setting GDK_BACKEND happens so that Guake runs under X11 in all cases. It resolves another issue under wayland, #1820, are you finding guake to exhibit any negative behaviors when it's forced to run under X11?

@VitalyAnkh
Copy link
Contributor Author

Yes, I found many negative behaviours when it's forced to run under X11:

  1. Sometimes the characters are flickering when I'm typing,
  2. Sometimes the UI is not very responsive. I have to wait 1 or 2seconds for the characters appearing after typing a command;
  3. I set Previous/Next Tab shortcuts to Ctrl + P / Ctrl + N, but the shortcuts don't work after updating to 3.8.0. I think this is caused by not running under Wayland for my system is running under Wayland, and before updating Guake runs under Wayland and works well
  4. Maybe blurry under Xwayland with HiDPI screen. It's a Xwayland bug: https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/111 .

I think the best way to solve thia problem is to let Guake run under Wayland. I set GDK_BACKEND environment variable to wayland then launch Guake, but Guake still runs under X11. What should I do?

@VitalyAnkh
Copy link
Contributor Author

I think Wayland is the future of Linux desktops. We should make Guake run under Wayland by default.

@Davidy22
Copy link
Collaborator

Davidy22 commented Oct 18, 2021

Wayland being very not backwards compatible and not being the majority installation means Guake is not going to default wayland, but we can drop/relax the x11 force and find alternate fixes to wayland problems. Hopefully upstream wayland fixes, but that's an indeterminate wait with how long some things have been unfixed in wayland.

The keyboard shortcut thing may be a different issue, the keyboard shortcut backend got revamped to make keyboard shortcuts with the tab key work so that CTRL+TAB and CTRL+SHIFT+TAB can work. Hopefully wayland doesn't have an issue with programs using Gtk keyboard events, otherwise we'll have to restore the old keybinding code as a fallback for wayland.

For isolation of changes, you can delete the line in main.py on line 45 that says os.environ["GDK_BACKEND"] = "x11" and install from source. That'll stop it from forcing x11 so you can see what works and what doesn't. There is a multitude of attached issues to that line that are going to regress if the line is removed, although if you encountered these issues upgrading from 3.7 to 3.8, #1821 should have been the only relevant change and we possibly can just revert that.

@VitalyAnkh
Copy link
Contributor Author

@Davidy22 Thanks for your kind and detailed reply! I will try deleting the line 45 then installing from source before the code changes.

@VitalyAnkh
Copy link
Contributor Author

Following the guide for installing from source, I installed it but when I run Guake I got:

guake
Traceback (most recent call last):
  File "/usr/local/bin/guake", line 6, in <module>
    from guake.main import exec_main
ModuleNotFoundError: No module named 'guake'

It seems the installation is not correct. But I follow the guide, cloned the repo, installed the dependencies, run make and sudo make install.
Anyway, I think we shouldn't force Guake to run under X11. If anyone wants this, they could set GDK_BACKEND to x11 to make this happen. Hardcoding the environment variable may be not a good idea.

@Davidy22
Copy link
Collaborator

We're going to need confirmation from people who can build from source on wayland that all issues resolved by including the line don't return when removing it. We're not going to delete a line of code that resolved issues because of untested hypothesis that running under wayland is better when running under wayland broke things previously. The line was originally added in commit 51aa791 and appears to have resolved four different issues. We need someone with wayland who can build with the change to confirm it works before we do the change.

When installing from source, make sure to remove existing installations, as they may interfere with the one from source.

@ccolorado
Copy link

@Davidy22 I am read the first line of your statement and can't figure out what this means.
don't return when removing it.
I can probably try building from source.

@Davidy22
Copy link
Collaborator

We need the confirmation so that we can check that we aren't regressing the issues fixed by adding that line. Thanks for the assistance, I can probably try to get wayland onto my machine so I can finally test these wayland issues for myself instead of doing this telephone game for debugging

@fancypantalons
Copy link

As an FYI, on my Gnome 41 system, issue #1817 resurfaces when using the Wayland backend.

@VitalyAnkh
Copy link
Contributor Author

@fancypantalons I also meet #1817 if guake runs under Wayland. But if you want Guake to run under x11, you could set GDK_BACKEND=x11 to guake’s launch command by yourself. Hardcoding this environment variable is bad.

@Davidy22
Copy link
Collaborator

Davidy22 commented Oct 31, 2021

Alright, there's a lot of talk happening so I'm just going to open a PR at #1963 removing the line so we can duke it out in there. I think everyone who's using wayland wants the line gone so the branch'll be a way for people to actually run it to see if it's preferred, and once the PR gets merged we'll have to figure out a different way to deal with the regressions induced by using wayland.

To note, the initial pull request that introduced this line didn't actually seem to do anything because it was probably in the wrong place, there was a future pull request merged between 3.7 and 3.8, which moved the line to somewhere where it actually had an effect, which generated the wave of wayland feedback from 3.8.

@VitalyAnkh
Copy link
Contributor Author

Thanks!

@mlouielu
Copy link
Collaborator

mlouielu commented Nov 14, 2021

  1. I agree with @VitalyAnkh hard-code environment variable is not a adequate move, but
  2. As I testing Remove GDK_BACKEND to force x11 #1963 by weston (GDK_BACKEND=wayland), at least 1) no global shortcut, 2) not placing at correct place, 3) @fancypantalons & @VitalyAnkh found Width resize rises Alignment issues #1817 shows again. Rhey are all correct behavior as my knowledge to wayland.
  3. So as least, Guake (still) ins't supporting wayland (reference weston) well, as that is how wayland designed...
  4. I propose not to remove force x11 backend, but to add an environment variable GUAKE_ENABLE_WAYLAND=1 like *1, which will setup Guake with GDK_BACKEND=wayland to solve this problem. Also we won't need to remove 856f5b9, as this happened in electron before *2.

Can @VitalyAnkh also re-install with 3.7.0 and upload guake --support? (or, remove force x11 in source code and upload guake --support)

BTW, I'm curious, so Gnome didn't suffer from bad placing like this?

Selection_53c60_2021_11_14_19_38_02

*1 how firefox deal with this: https://wiki.archlinux.org/title/firefox#Wayland
*2 electron/electron#28436

@VitalyAnkh
Copy link
Contributor Author

@mlouielu I installed @Davidy22's PR branch and it works great. The guake --support is here:


Guake Version:		3.7.1.dev203

Vte Version:		0.66.1

Vte Runtime Version:	0.66.1

--------------------------------------------------
GTK+ Version:		3.24.30

GDK Backend:		<__gi__.GdkWaylandDisplay

--------------------------------------------------
Desktop Session: gnome

--------------------------------------------------
Display: wayland-0

RGBA visual: True

Composited: True

* Monitor: 0 - ISP T270LG
    * Geometry:		3840 x 2160 % 2 at 0, 0
    * Size:		600 x 330 mm²
    * Primary:		False
    * Refresh rate:	60.0 Hz
    * Subpixel layout:	unknown

And the problems you met under weston can be solved with a more powerful desktop environment:

  1. We can set global shortcut via DE's shortcut manager, for example, settings -> keyboard -> shortcuts on Gnome;
  2. We could make Guake align at right place with a few adjustments to its preferences (adjust the window height);
  3. Solve this in 2's way.

Anyway, it's great to let users choose wayland or x11. Just don't hardcode the environment variable GDK_BACKEND.
In my humble opinion, introducing environment variable GUAKE_ENABLE_WAYLAND would be great, just like the way Firefox does. This could separate the global GDK_BACKEND with that Guake uses, and also leave users choices.

@VitalyAnkh
Copy link
Contributor Author

BTW, following https://guake.readthedocs.io/en/latest/user/installing.html#install-from-source I tried installing Guake from source but I got errors when running

$ guake
Traceback (most recent call last):
  File "/usr/local/bin/guake", line 6, in <module>
    from guake.main import exec_main
ModuleNotFoundError: No module named 'guake'

I figured out that I must install guake through system's package manager. Then I installed Guake with a modified PKGBUILD (I'm using Arch Linux). Is this a common problem that the way installing from source doesn't work?

@mlouielu
Copy link
Collaborator

BTW, following https://guake.readthedocs.io/en/latest/user/installing.html#install-from-source I tried installing Guake from source but I got errors when running

$ guake
Traceback (most recent call last):
  File "/usr/local/bin/guake", line 6, in <module>
    from guake.main import exec_main
ModuleNotFoundError: No module named 'guake'

I figured out that I must install guake through system's package manager. Then I installed Guake with a modified PKGBUILD (I'm using Arch Linux). Is this a common problem that the way installing from source doesn't work?

This should checked by which interpreter you install, and which interpreter used in guake (that will be /usr/bin/python3 in the shebang), if they didn't match, then it is possible to show ModuleNotFoundError.

@Davidy22
Copy link
Collaborator

When installing from source, you need to remove other existing installations, and sometimes python does a weird thing where it prefers files in the directory you're in and that sometimes messes up running so you have to run the guake command from another directory.

I kind of want to just remove the force entirely, just have Guake use wayland when the system is wayland and work towards just fixing the wayland issues. Pre 3.8, the force was broken and wayland users were apparently doing fine, and then the floodgates of issues opened when we actually "fixed" it and started properly forcing x11 on wayland. Even in the issue where you noted that removing the force causes a regression, the issue under wayland is poor resizing behavior, which is relatively minor compared to issues under normal use reported now, and the reporter in the issue you linked even states that they themself would prefer to have it just use wayland despite the regression.

@mlouielu
Copy link
Collaborator

First of all, please let me re-iterate one thing: Wayland did not let user touch absolute coordinate. this didn't change in 2021 as far as I know *1.

Secondly, as Guake can work as a "drop-down" terminal in Wayland backend, is a coincident. If you shrink the width of Guake (perf -> main window -> Geometry -> Width), then it will not work as drop down terminal (Gnome 41, mutter):

guake-371

And, as expected, Horizontal Alignment (left, center, right) won't work, too. Also mutter center-window will make Guake pop out from the center of the workspace.

This Guake is from archlinux archive built at 12-Nov-2020 17:17 *2, as this didn't include ed8e561, guake --support shows that its GDK Backend is Xwayland, but actually it use Wayland.

On the other hand, only user suffer from unexpected result will fill the bug report, I can imagine if we remove ed8e561, it will cause more user come to report issues.

To summarize, I would describe Guake as a x11-based application, it should be default running under x11 GDK backend, user may want to run under wayland backend, that's fine, we should let user have the ability to set this up.

That is why I proposed to add GUAKE_ENABLE_WAYLAND=1 for this.


*1 From mutter maintainer: https://gitlab.gnome.org/GNOME/mutter/-/issues/2003#note_1307150
(Please forgive me that I didn't try to find out original blog post which describe this)

*2 https://archive.archlinux.org/packages/g/guake/guake-3.7.0-2-any.pkg.tar.zst

@Davidy22
Copy link
Collaborator

Hum, I see buried in one of the threads some notes on start menu positioning. The wayland devs are pretty rigid here it seems, but it must be possible to do some kind of positioning or it just wouldn't be possible to spawn the start menu next to the start button in Wayland.

Also noticed in a thread that a mutter dev thinks we should use Gdk.set_supported_backends("x11") instead of setting an environment variable. I'll try it out once my eagerly awaited new machine arrives and I get something that supports wayland onto it.

I keep bouncing around because I can't give firsthand testimonial on wayland, but I'll follow the retain x11 force, make wayland optional route. About the flag, why format it as GUAKE_ENABLE_WAYLAND=1 instead of using the flag format that we already parse in main? It's not like anyone is ever going to type GUAKE_ENABLE_WAYLAND and then decide to follow it up with a 0, we can surely just add it to the optionparser as -W or something.

Also, we could probably do something about hiding the items in preferences that are disabled by wayland, or maybe there is a way to do them still and we just need to do some work.

@VitalyAnkh
Copy link
Contributor Author

@Davidy22

It's not like anyone is ever going to type GUAKE_ENABLE_WAYLAND and then decide to follow it up with a 0, we can surely just add it to the optionparser as -W or something.

In my opinion, using the environment variable is a cleaner way. If it is an option which should be passed when launching Guake, users who want Wayland need to edit /usr/share/applications/guake.desktop file when making Guake autostart. It is less convenient and harder to manage dotfiles than just setting an environment variable. I have an export MOZ_ENABLE_WAYLAND=1 in my ~/.profile which is convenient to make firefox run under Wayland.

@VitalyAnkh
Copy link
Contributor Author

@Davidy22 @mlouielu Could I make a PR to introduce GUAKE_ENABLE_WAYLAND and fix this issue?

@Davidy22
Copy link
Collaborator

Oh, that'd certainly save us a bit of time, would be very welcome

@mlouielu
Copy link
Collaborator

mlouielu commented Nov 16, 2021

Thanks @VitalyAnkh for helping this out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants