-
Notifications
You must be signed in to change notification settings - Fork 278
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
Add support for wlroots-based Wayland compositors like Sway #1461
base: main
Are you sure you want to change the base?
Conversation
Thank you so much for implementing this! I haven't looked at things in detail yet, but the implementation looks nice on first glance. I've only tested this briefly but so far everything worked nicely with both the keyboard machine and an external machine (dnaq's HID protocol), and that's with a non-US keyboard layout. (Also on Sway 1.6.1.) It's unfortunate that this leaves out gnome, but it seems like we'll need a separate implementation to support them well anyways; for compositors that do support these two protocols, all things considered, this is probably the optimal solution. I'm very happy I can finally retire my temporary workaround and replace it with this. A few notes: This might be due to my build environment, but I get a
when trying to do a clean build. The workaround for me was to add these four lines to my setup.cfg packages list, and create the corresponding empty directories before building:
I don't know enough about python packaging to say what the actual proper solution is here, but I wanted to bring it up in case you do. |
Thank you for testing! I had only tested running with
I assume the maintainers would have more insights than me regarding this. |
Thanks for the fix! The modified nix derivation I used for building now works as well. |
First off, thanks so much for this PR, I've been wanting to get Plover working on Sway for forever! I was testing this out and I ran into some strange behaviour, it seems isolated to XWayland applications though. From what I can tell, keystrokes result in a lot of backspaces and an occasional number after those backspaces. My setup is a little non-standard though, so it may be a bug elsewhere, I'm running arch with:
Has anyone else run into similar behaviour with XWayland applications? |
Thank you for testing this! I’m also able to reproduce this issue on XWayland apps. I’ll take a look and see if I can fix this. |
Related issue in a project that uses the same virtual keyboard protocol as this implementation: atx/wtype#1 |
@mtoohey31 Should be fixed with this latest commit. I tested it on my end by running |
Can confirm, I just double checked with Brave Browser on my setup and it's fixed. Thanks! |
Thank you |
You can see the comment linked above, but I was asked to repost this here: I tried out this PR as-is, but ran into serious problems in Slack (an Electron app) on both Wayland and X11 - backspaces not working, extraneous 'e' characters, etc. After doing some debugging, I discovered the issues only arise when sending a new keymap with every emitted string (as the PR does). At first I thought it was a timing issue, but no amount of slowing down the output rate seemed to help... I made a few changes to cache a "default" keymap with the anglocentric usual characters (most of the output characters from the default Plover dictionary). I left fallback code in place to do what this PR was doing before. With this change the Wayland support works flawlessly for me including in Electron apps. I think it also would make Plover behave like a more normal Wayland app - how many applications use two-plus new keymaps per second in the wild? If the lack of non-English characters is an issue, this patch could be modified to build a keymap that includes all the characters seen in any emitted string so far, or even all the characters in the configured dictionaries. |
thank you! :) |
Hi @BryanJacobs, thanks for the patches!
I’m having trouble reproducing the exact behavior you’re describing. I’ve tested Slack and Discord and they work mostly fine; sometimes the first word I type doesn’t use the right keymap, but no extraneous 'e's and the '*' key works correctly. Do you have any particular settings that I should enable on my end, or dictionaries/strokes that I should test, to reproduce your issue?
Interesting! Unless we manage to reproduce this bug in non-Electron apps (for example, does this happen in Chromium too?), it does seem like a bug that would be worth reporting to Electron folks. I do agree that it’s a bit unusual to regenerate the keymap dynamically at such a high rate. It’s allowed in the Wayland protocol though, and seems to work fine with most apps.
It would sure be great to keep this PR language-agnostic, if possible. Your suggestion to build an incremental keymap is interesting, we need to be careful though as there is a limit on the number of characters in an XKB keymap, and sending very large keymaps could hang the compositor. |
Versions: System: Arch Linux
With these versions the problem can be immediately reproduced. I'm sure if you were to launch with the original strategy in here some others would have the same issues I did. By the way, I was able to produce the issues (extra 'e' characters) in GDK_BACKEND="x11" gedit as well, so it's not specific to Electron 100%. An easy case that shows problems every time is stroking TAOEUPL/-G/* ("timing", then an undo). For me, I get "time", "timening", then "timeningeeeee" ('e' characters being emitted instead of backspaces). I'm also unable to make R-R send carriage returns properly in this mode. They do nothing. Suffix strokes that don't involve backspaces work fine; suffix strokes that do involve backspaces don't function properly, likely because in the base PR here they try to send two keymaps in extremely rapid succession (one for backspacing and one for typing). Note that my patch uses a more recent version of the Pywayland binding than your original PR. On another note, I believe the server allows keymap sizes up to 2GB and two billion characters, so it's rather unlikely that pre-sending a keymap that contains every output character in the user dictionary would be a practical problem. You wouldn't need to build an incremental keymap, you'd just need to hook into the dictionary update event (and startup), scan the dicts, and send the union set of every output character present. For most users that would be a one to two hundred characters maximum, and for a select few it would be a small number of thousands. This would also improve overall application performance by avoiding the need for a keymap event on every stroke - keeping in mind that fast stenographers produce over six strokes per second. |
thanks for the patches! They seem to work in ail programs I write in, except for discord, where most strokes are changed to numbers (Plover becomes 21l3v54 for example) |
I've been looking at this PR the last couple of days, and there are several issues:
I've pushed a version here that should address all those issues (mainly by sharing the same Wayland connection between the capture and emulation code). Anyway, because of the aforementioned setup issue (which is exacerbated in my version due to an additional dependency on Once it's robust enough, we can add it to the Plover distributions, as part of the standard plugins. Thoughts? |
Forgot to mention known issues / limitations:
|
i'm sorry for the late response, i haven't been able to use a computer for the past few weeks due to health reasons. i'm still not fully recovered and can't take a closer look at your changes, but i did at least want to write a small response because no one has said anything yet and i'm very grateful you took the time to look at this in detail and make a significant amount of improvements. i can't speak on the issues you've mentioned, but they do seem sensible and i look forward to being able to take a closer look at the fixes you've implemented. as for the suggestion of making this an output plugin, i am very much in favor, precisely because of the multitude of approaches that is currently required to cover all wayland-based compositors, and especially considering all of the amazing work that @user202729 has already done the this area. i'd offer my help for this, but i doubt i could be of much use even if i was fully capable of using a computer right now. |
so, um, I did actually manage to test things out today, and I think there's a problem with the packaging?
I'll see if I can find and fix the problem, but it might take a while. |
@antoniusf: my bad, should be fixed now. |
Thank you, it works great now! This also fixes problems that I had with \n not working, and I was also able to reproduce the problems with electron apps which this version fixes. |
I have another question actually: I currently have problems with certain key combinations. In particular, trying to use page up and page down doesn't work and results in the following error (some lines cut from the traceback):
I've spent way too long looking at all this and trying to figure something out, but I did notice that the key combinations are sent via the replay virtual keyboard, which uses the system keymap, and not the custom output keyboard, which uses the flexible keymap. Is that correct? Is there reason for this? My current theory is that the replay keyboard just doesn't have these keys for some reason... |
We're using the replay keyboard because the goal of key combos is to simulate shortcuts, as you would type them on the keyboard. Anyway, the reason #define XK_Prior 0xff55 /* Prior, previous */
#define XK_Page_Up 0xff55 So will need to make sure we add aliases for those cases. |
List of aliases:
|
Should be fixed now. |
Thanks for the quick fix! Works for me now 🎉 |
@antoniusf I noticed the mention of |
@mtoohey31 sure! It's not really an overlay though, just a small mess of a few nix derivations and duct tape. I'll see if I can get this into more proper form, but until then, the lines you're missing are probably these?
I also had to add the python packages rtf_tokenize and plover_stroke, which aren't in nixpkgs yet, but are very straightforward to write derivations for. (But which is also why this is a bit messy.) So, I'll get back to you with something that's shareable, but if you manage to make something neat I'd love to see it! |
Thanks, that was it! This is what I ended up with, it works but it's a bit of a mess too. I'll edit this and clean it up if I get around to it: (self: super: rec {
python3Packages = {
plover-stroke = self.python3Packages.buildPythonPackage rec {
pname = "plover_stroke";
version = "1.0.1";
src = super.python3Packages.fetchPypi {
inherit pname version;
sha256 = "t+ZM0oDEwitFDC1L4due5IxCWEPzJbF3fi27HDyto8Q=";
};
};
rtf-tokenize = self.python3Packages.buildPythonPackage rec {
pname = "rtf_tokenize";
version = "1.0.0";
src = super.python3Packages.fetchPypi {
inherit pname version;
sha256 = "XD3zkNAEeb12N8gjv81v37Id3RuWroFUY95+HtOS1gg=";
};
};
pywayland_0_4_7 = super.python3Packages.pywayland.overridePythonAttrs
(oldAttrs: rec {
pname = "pywayland";
version = "0.4.7";
src = super.python3Packages.fetchPypi {
inherit pname version;
sha256 = "0IMNOPTmY22JCHccIVuZxDhVr41cDcKNkx8bp+5h2CU=";
};
});
} // super.python3Packages;
plover.dev = super.plover.dev.overridePythonAttrs
(oldAttrs: {
src = self.fetchFromGitHub {
owner = "openstenoproject";
repo = "plover";
rev = "fd5668a3ad9bd091289dd2e5e8e2c1dec063d51f";
sha256 = "2xvcNcJ07q4BIloGHgmxivqGq1BuXwZY2XWPLbFrdXg=";
};
propagatedBuildInputs = oldAttrs.propagatedBuildInputs
++ [
python3Packages.plover-stroke
python3Packages.rtf-tokenize
python3Packages.pywayland_0_4_7
];
nativeBuildInputs = (oldAttrs.nativeBuildInputs or [ ]) ++ [
self.pkg-config
];
doCheck = false; # TODO: get tests working
postPatch = ''
sed -i /PyQt5/d setup.cfg
substituteInPlace plover_build_utils/setup.py \
--replace "/usr/share/wayland/wayland.xml" "${self.wayland}/share/wayland/wayland.xml"
'';
});
}) |
Oh nice, thank you! |
I tried the Nix expression provided by @mtoohey31 and got the following runtime error: $ plover
<omitted>
File "/nix/store/sa1wvfw3hcwyc4bd7r23kyvrcshy4p0s-python3.10-xlib-0.31/lib/python3.10/site-packages/Xlib/support/unix_connect.py", line 76, in get_display
raise error.DisplayNameError(display)
Xlib.error.DisplayNameError: Bad display name ""
Exception ignored in: <function Application.__del__ at 0x7f9ea26303a0>
Traceback (most recent call last):
File "/nix/store/k3f35vmkywwbmj9m8zpywkbj0js1dvv8-python3.10-plover-4.0.0.dev10/lib/python3.10/site-packages/plover/gui_qt/main.py", line 71, in __del__
del self._win
AttributeError: _win I'm on RiverWM with XWayland disabled. I recall it working a bit better with XWayland, at least up to the point until it complained about Does it still need XWayland for the GUI? |
Hmm, so I just tried running plover with DISPLAY unset, and I got a similar error message. Reading the traceback, it seems like the error came from |
I totally forgot to set And yeah, $ QT_QPA_PLATFORM=wayland plover
Unexpected error: Traceback (most recent call last):
File "/nix/store/k3f35vmkywwbmj9m8zpywkbj0js1dvv8-python3.10-plover-4.0.0.dev10/lib/python3.10/site-packages/plover/scripts/main.py", line 131, in main
code = gui.main(config, controller)
File "/nix/store/k3f35vmkywwbmj9m8zpywkbj0js1dvv8-python3.10-plover-4.0.0.dev10/lib/python3.10/site-packages/plover/gui_qt/main.py", line 98, in main
app = Application(config, controller, use_qt_notifications)
File "/nix/store/k3f35vmkywwbmj9m8zpywkbj0js1dvv8-python3.10-plover-4.0.0.dev10/lib/python3.10/site-packages/plover/gui_qt/main.py", line 31, in __init__
from plover.gui_qt.main_window import MainWindow
File "/nix/store/k3f35vmkywwbmj9m8zpywkbj0js1dvv8-python3.10-plover-4.0.0.dev10/lib/python3.10/site-packages/plover/gui_qt/main_window.py", line 16, in <module>
from plover.oslayer import wmctrl
File "/nix/store/k3f35vmkywwbmj9m8zpywkbj0js1dvv8-python3.10-plover-4.0.0.dev10/lib/python3.10/site-packages/plover/oslayer/wmctrl.py", line 34, in <module>
wmctrl = WmCtrl()
File "/nix/store/k3f35vmkywwbmj9m8zpywkbj0js1dvv8-python3.10-plover-4.0.0.dev10/lib/python3.10/site-packages/plover/oslayer/xwmctrl.py", line 9, in __init__
self._display = display.Display()
File "/nix/store/sa1wvfw3hcwyc4bd7r23kyvrcshy4p0s-python3.10-xlib-0.31/lib/python3.10/site-packages/Xlib/display.py", line 89, in __init__
self.display = _BaseDisplay(display)
File "/nix/store/sa1wvfw3hcwyc4bd7r23kyvrcshy4p0s-python3.10-xlib-0.31/lib/python3.10/site-packages/Xlib/display.py", line 71, in __init__
protocol_display.Display.__init__(self, *args, **keys)
File "/nix/store/sa1wvfw3hcwyc4bd7r23kyvrcshy4p0s-python3.10-xlib-0.31/lib/python3.10/site-packages/Xlib/protocol/display.py", line 84, in __init__
name, protocol, host, displayno, screenno = connect.get_display(display)
File "/nix/store/sa1wvfw3hcwyc4bd7r23kyvrcshy4p0s-python3.10-xlib-0.31/lib/python3.10/site-packages/Xlib/support/connect.py", line 73, in get_display
return mod.get_display(display)
File "/nix/store/sa1wvfw3hcwyc4bd7r23kyvrcshy4p0s-python3.10-xlib-0.31/lib/python3.10/site-packages/Xlib/support/unix_connect.py", line 76, in get_display
raise error.DisplayNameError(display)
Xlib.error.DisplayNameError: Bad display name ""
Exception ignored in: <function Application.__del__ at 0x7fe2663583a0>
Traceback (most recent call last):
File "/nix/store/k3f35vmkywwbmj9m8zpywkbj0js1dvv8-python3.10-plover-4.0.0.dev10/lib/python3.10/site-packages/plover/gui_qt/main.py", line 71, in __del__
del self._win
AttributeError: _win |
Huh, maybe QT_QPA_PLATFORM isn't necessary if there isn't an X server to connect to. Either way, your error matches mine, so that module will probably have to be moved to wayland as well (or, at least, disabled so it doesn't cause errors). Most people probably have an xwayland running along though, and I don't think the module is causing any problems as is. I guess that does answer your question though? The gui doesn't need X, but some other bit of code still tries to connect. |
This repository currently contains the following packages: - Akkoma, and its pleroma and admin frontends, built from git, with patches for my personal instance - matrix-media-repo built from git - git versions of mautrix-signal and mautrix-telegram - mautrix-discord and mautrix-whatsapp, also built from git - a usually more updated version of papermc - Plover with openstenoproject/plover#1461 (comment) applied (wayland support) - a few plover plugins, latest pypi version - KreativeKorp’s fonts - Emoji for akkoma made by volpeon (https://volpeon.ink/)
@benoit-pierre Sorry I haven’t followed up, but thanks for your changes! I would be willing to spend some time porting this as a plugin over the holidays, if you still think this is the best direction. I see that #1276 has been closed recently though, so do you have something else in mind? |
Regarding the closure of the pull request, I'll quote Ted's announcement in the Discord server here...
|
Thanks for the context. |
I have tried running off this branch using the overlay posted above, but I assume something (hyprland?) crashes in the process, because I see a black screen for half a second and then get logged out. |
This repository currently contains the following packages: - Akkoma, and its pleroma and admin frontends, built from git, with patches for my personal instance - matrix-media-repo built from git - git versions of mautrix-signal and mautrix-telegram - mautrix-discord and mautrix-whatsapp, also built from git - a usually more updated version of papermc - Plover with openstenoproject/plover#1461 (comment) applied (wayland support) - a few plover plugins, latest pypi version - KreativeKorp’s fonts - Emoji for akkoma made by volpeon (https://volpeon.ink/)
I'm pretty sure this is a bug on Hyprland or whatever login manager you're using (idk what you mean by "logged out") as the compositor probably should never crash as a result of software interaction. Since https://github.com/riverwm/river/issues/351 has been resolved, I finally decided to give Plover a try again. I modified the Nix expression slightly as I forgot how to get it work. I'm happy to say that Plover seems to work on every app I tried but Element Desktop (Flatpak version is the one I tested) where it refuses to append spaces after words and Expression: # Build with `nix-build <name of file>`
with import <nixpkgs> {}; let
plover-stroke = python3Packages.buildPythonPackage rec {
pname = "plover_stroke";
version = "1.1.0";
src = python3Packages.fetchPypi {
inherit pname version;
sha256 = "sha256-3gOyP0ruZrZfaffU7MQjNoG0NUFQLYa/FP3inqpy0VM=";
};
};
rtf-tokenize = python3Packages.buildPythonPackage rec {
pname = "rtf_tokenize";
version = "1.0.0";
src = python3Packages.fetchPypi {
inherit pname version;
sha256 = "XD3zkNAEeb12N8gjv81v37Id3RuWroFUY95+HtOS1gg=";
};
};
pywayland_0_4_7 =
python3Packages.pywayland.overridePythonAttrs
(oldAttrs: rec {
pname = "pywayland";
version = "0.4.7";
src = python3Packages.fetchPypi {
inherit pname version;
sha256 = "0IMNOPTmY22JCHccIVuZxDhVr41cDcKNkx8bp+5h2CU=";
};
doCheck = false;
});
in
plover.dev.overridePythonAttrs
(oldAttrs: {
src = fetchFromGitHub {
owner = "openstenoproject";
repo = "plover";
rev = "fd5668a3ad9bd091289dd2e5e8e2c1dec063d51f";
sha256 = "2xvcNcJ07q4BIloGHgmxivqGq1BuXwZY2XWPLbFrdXg=";
};
propagatedBuildInputs =
oldAttrs.propagatedBuildInputs
++ [
plover-stroke
rtf-tokenize
pywayland_0_4_7
];
nativeBuildInputs = (oldAttrs.nativeBuildInputs or []) ++ [pkg-config];
doCheck = false; # TODO: get tests working
postPatch = ''
sed -i /PyQt5/d setup.cfg
substituteInPlace plover_build_utils/setup.py \
--replace "/usr/share/wayland/wayland.xml" "${wayland}/share/wayland/wayland.xml"
'';
}) |
I just noticed that with the expression I posted above and the latest commit of River, Gemini PR works fine but setting the machine mode to "Keyboard" causes pretty much all keys but Space and Tab to output nothing. Plover is clearly doing something in that it's successfully eating my inputs, but it seems far from functional. I would appreciate it if anyone else could try reproducing it. |
Summary of changes
Compatibility and testing
This PR adds keyboard emulation and capture support for some Wayland compositors using the
virtual_keyboard_unstable_v1
andinput_method_unstable_v2
protocols. As their names state, these protocols are not yet stable and may be changed in the future. Such changes would probably require changes in this implementation.The required protocols are currently only implemented in wlroots-based compositors, such as Sway (list of wlroots-based compositors). GNOME has stated they don’t plan on implementing them (but have alternatives in mind), so it’s not covered by this PR. When Plover is started on an unsupported Wayland compositor, it will display an error message stating this incompatibility.
I have tested this PR on Sway (sway version 1.6.1, wlroots version 0.14.1), with TX Bolt and Keyboard machines (and an otherwise default Plover configuration).
Implementation details
This adds a new dependency on the
pywayland
library, used to communicate with the compositor. Wayland protocols are defined as XML files and require a code generation step to be available from Python. I checked out the XML definitions, and added a build step to generate the required code (using a similar process as used to generate Qt code).References
(thanks @antoniusf for some very useful pointers in that thread!)
Pull Request Checklist
Changes have tests(is there a simple way to test these changes automatically?)