widgets: fall back to Xwayland #234
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #234 +/- ##
==========================================
- Coverage 93.43% 93.18% -0.26%
==========================================
Files 57 58 +1
Lines 10999 11046 +47
==========================================
+ Hits 10277 10293 +16
- Misses 722 753 +31 ☔ View full report in Codecov by Sentry. |
|
This doesn't seem to work. For some reason it still connects to Wayland anyway. It does it when I set |
|
Defaulting to |
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2024122307-4.3&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2024111705-4.3&flavor=update
Failed tests87 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/119126#dependencies 4 fixed
Unstable tests
|
|
Is it necessary for clicking outside the menu to close the menu, or is it good enough to require clicking again on the widget? The latter might be fixable in the menu itself, by closing the menu when the widget is clicked. |
DemiMarie
force-pushed
the
widgets-xwayland
branch
from
9dee5a2 to
18901a0
Compare
It is necessary, it is a dom0 widget, it should not have the bad behavior of the vm widgets (which also should work better wrt to closing, but that's out of scope here). |
Can you try my most recent commit? That dismisses the menu when it loses focus. |
|
I don't think this addresses the root cause, which is some wayland failure - this works correctly under X and so, from what you said previously, should work correctly under XWayland. I don't think we want to patch the whole world, if it requires additional workarounds like this (which BTW duplicate what the menu generally does on its own) I don't think this is a good solution. |
DemiMarie
force-pushed
the
widgets-xwayland
branch
from
5d08789 to
d48cfae
Compare
DemiMarie
force-pushed
the
widgets-xwayland
branch
from
d48cfae to
b8abcd6
Compare
|
pylint complains a lot, and also rpm fails to build (%files not updated) |
DemiMarie
force-pushed
the
widgets-xwayland
branch
2 times, most recently
from
67cf62f to
aa0872a
Compare
DemiMarie
force-pushed
the
widgets-xwayland
branch
from
aa0872a to
f7ee54a
Compare
|
This has been tested on a real KDE Wayland session and works fine. |
|
Can you try to make it with GTK window? It should be significantly less verbose (if works). See for example https://stackoverflow.com/a/33294727 But also, make it conditional on Wayland session, we don't need this on real X11. |
DemiMarie
force-pushed
the
widgets-xwayland
branch
from
f7ee54a to
0160df6
Compare
Done, and it is indeed less verbose.
Also done, along with removing the change to |
DemiMarie
force-pushed
the
widgets-xwayland
branch
from
0160df6 to
6ad383b
Compare
|
Tested and working on real hardware, in a real Xwayland session, using an RPM built in the Qubes executor from the same commit I just pushed. |
DemiMarie
force-pushed
the
widgets-xwayland
branch
from
6ad383b to
30fbd1e
Compare
| No-op implementation of the hack, for use on stock X11. | ||
| """ | ||
|
|
||
| __slots__ = () |
There was a problem hiding this comment.
Slots are generally discouraged unless really necessary (mostly in very memory constrained environments, or when operating on enormous data sets).
There was a problem hiding this comment.
I’ll delete this, but why are slots discouraged? I use them for the run-time checking they provide.
There was a problem hiding this comment.
They severely limit extensibility.
| __slots__ = () | ||
|
|
||
| @classmethod | ||
| def get(cls, /) -> "DisgustingX11FullscreenWindowHack": |
There was a problem hiding this comment.
It's weird to have this as a class method here. Shouldn't it be a function outside of those classes (returning one or the other)?
DemiMarie
force-pushed
the
widgets-xwayland
branch
from
30fbd1e to
4a777ae
Compare
| No-op implementation of the hack, for use on stock X11. | ||
| """ | ||
|
|
||
| __slots__ = () |
There was a problem hiding this comment.
I’ll delete this, but why are slots discouraged? I use them for the run-time checking they provide.
This has two parts: 1. Set GDK_BACKEND=x11, so that GDK3 chooses to use X11 as its backend. This must be done while only one thread is running, as otherwise undefined behavior results. It also must be done before gi.overrides.Gdk is imported, as otherwise it will be too late to set the GDK backend. Therefore, this code is run very early, and RuntimeError is raised if the preconditions are violated. 2. Create a fullscreen invisible window for mouse input. This works around Xwayland not passing all pointer input to X11. The menu is dismissed if the user clicks on the fullscreen window. This hack is only used if WAYLAND_DISPLAY is set.
DemiMarie
force-pushed
the
widgets-xwayland
branch
from
4a777ae to
fb35583
Compare
The widgets do not work under Wayland, so just use Xwayland for now.
If GTK somehow still connects to Wayland despite WAYLAND_DISPLAY being unset I will not be happy.
This is broken under KDE because of a bug in KDE (https://bugs.kde.org/show_bug.cgi?id=468085), but that can be fixed.