plover icon indicating copy to clipboard operation
plover copied to clipboard

Add support for wlroots-based Wayland compositors like Sway

Open matteodelabre opened this issue 2 years ago • 39 comments

Summary of changes

Compatibility and testing

This PR adds keyboard emulation and capture support for some Wayland compositors using the virtual_keyboard_unstable_v1 and input_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

  • Related issue: #1050 (thanks @antoniusf for some very useful pointers in that thread!)
  • This implementation is not affected by #650

Pull Request Checklist

  • [ ] ~~Changes have tests~~ (is there a simple way to test these changes automatically?)
  • [x] News fragment added in news.d. See documentation for details

matteodelabre avatar Jan 02 '22 20:01 matteodelabre

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

ModuleNotFoundError: No module named 'plover.oslayer.wayland'

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:

    plover.oslayer.wayland
    plover.oslayer.wayland.wayland
    plover.oslayer.wayland.input_method_unstable_v2
    plover.oslayer.wayland.virtual_keyboard_unstable_v1

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.

antoniusf avatar Jan 03 '22 16:01 antoniusf

Thank you for testing! I had only tested running with tox -e launch, so thanks for bringing that point up. I’ve added some changes to try to fix the build. With the latest changes:

  • Using the build command that’s used to generate the AUR package works.
  • Activating the dev environment with source .tox/dev/bin/activate and running python setup.py bdist_wheel works.
  • Using tox -e setup -- bdist_wheel fails (it looks like it’s trying to run some commands before the Wayland module generation instructions are reached, and those commands expect those modules to already exist).

I assume the maintainers would have more insights than me regarding this.

matteodelabre avatar Jan 03 '22 20:01 matteodelabre

Thanks for the fix! The modified nix derivation I used for building now works as well.

antoniusf avatar Jan 04 '22 00:01 antoniusf

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:

sway-git r6855.dbaf2e4f-1
wlroots-eglstreams-git 0.16.0.r5319.f4578db2-1
xorg-xwayland 21.1.4-1

Has anyone else run into similar behaviour with XWayland applications?

mtoohey31 avatar Jan 12 '22 17:01 mtoohey31

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.

matteodelabre avatar Jan 12 '22 21:01 matteodelabre

Related issue in a project that uses the same virtual keyboard protocol as this implementation: https://github.com/atx/wtype/issues/1

matteodelabre avatar Jan 14 '22 04:01 matteodelabre

@mtoohey31 Should be fixed with this latest commit. I tested it on my end by running GDK_BACKEND=x11 gedit.

matteodelabre avatar Jan 14 '22 13:01 matteodelabre

Can confirm, I just double checked with Brave Browser on my setup and it's fixed. Thanks!

mtoohey31 avatar Jan 14 '22 15:01 mtoohey31

Thank you

ghost avatar Jan 27 '22 02:01 ghost

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.

bump-pywayland.txt cache-kb-mapping.txt

BryanJacobs avatar Feb 15 '22 21:02 BryanJacobs

thank you! :)

antoniusf avatar Feb 15 '22 23:02 antoniusf

Hi @BryanJacobs, thanks for the patches!

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.

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?

I made a few changes to cache a "default" keymap with the anglocentric usual characters.. (…) 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?

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.

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.

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.

matteodelabre avatar Feb 27 '22 16:02 matteodelabre

Versions:

System: Arch Linux

  • Sway 1.7-2
  • Wlroots 0.15.1-2
  • Slack-wayland AUR 4.23.0
  • Plover from your exact branch
  • Machine: Gemini PR via serial (emulated from a QMK keyboard)

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.

BryanJacobs avatar Feb 27 '22 21:02 BryanJacobs

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)

DarkKirb avatar Mar 05 '22 13:03 DarkKirb

I've been looking at this PR the last couple of days, and there are several issues:

  • the Wayland generation protocol stuff means it's not possible anymore to build a platform-agnostic wheel (and the sdist generation from a clean checkout is broken).
  • the keyboard capture logic is wrong: the numlock state should be ignored, and even if output is suppressed, relevant key events must still be forwarded to the machine (so for example {PLOVER:RESUME} work as expected).
  • similarly to others, I've seen issues with the keyboard emulation (wrong characters), I think it's the same issue as with X11: changing the mapping of keycode too fast can result in the wrong keysym being used by clients.
  • the way keyboard events end up being simulated 2 times (and the associated keymap changes churn) is not great: the keyboard emulation will simulate a key with its virtual keyboard, and the capture code will replay it again in its grab handler.

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 xkbcommon), and the fact that not all Wayland compositors are supported, I don't think Wayland support should be merged, if ever, to Plover's core. Instead, what I'm proposing to do, is to fast-track the output plugins support, so a plover_wayland_output plugin can be created.

Once it's robust enough, we can add it to the Plover distributions, as part of the standard plugins.

Thoughts?

benoit-pierre avatar Mar 11 '22 20:03 benoit-pierre

Forgot to mention known issues / limitations:

  • because in Wayland key repeat is handled manually by each client, the keyboard capture cannot correctly handle those repeat events (e.g. to suppress a heavy-handed stroke mapped to {PLOVER:RESUME}).
  • contrary to the X11 version, keyboard input is always grabbed (when the keyboard machine is used): so you better not kill -STOP Plover, or you'll completely loose keyboard input...
  • similarly, I've tried to ensure the grab code is robust, and replay unsuppressed key events even in the event of an exception.

benoit-pierre avatar Mar 11 '22 20:03 benoit-pierre

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.

antoniusf avatar Mar 30 '22 14:03 antoniusf

so, um, I did actually manage to test things out today, and I think there's a problem with the packaging?

2022-03-30 23:18:33,646 [MainThread] ERROR: error loading gui plugin: none (from plover.gui_none.main)
Traceback (most recent call last):
...
  File "/nix/store/3ln8ivqvjvk9zd2lfdwkmdrlh60wclaj-python3.9-plover-modified-4.0.0.dev10/lib/python3.9/site-packages/plover/oslayer/wayland/keyboardcontrol.py", line 21, in <module>
    from .input_method_unstable_v2 import ZwpInputMethodManagerV2
ModuleNotFoundError: No module named 'plover.oslayer.wayland.input_method_unstable_v2'

I'll see if I can find and fix the problem, but it might take a while.

antoniusf avatar Mar 30 '22 21:03 antoniusf

@antoniusf: my bad, should be fixed now.

benoit-pierre avatar Mar 31 '22 16:03 benoit-pierre

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.

antoniusf avatar Apr 01 '22 13:04 antoniusf

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):

2022-04-03 00:14:21,809 [Dummy-2] ERROR: engine on_stroked failed
Traceback (most recent call last):
...
  File "/nix/store/43kdzpv6k3p9v48nqhi3984jn3n9l2cr-python3.9-plover-modified-4.0.0.dev10/lib/python3.9/site-packages/plover/oslayer/wayland/keyboardlayout.py", line 81, in parse_key_combo
    return parse_key_combo(combo_string, self._combo_from_keyname.__getitem__)
  File "/nix/store/43kdzpv6k3p9v48nqhi3984jn3n9l2cr-python3.9-plover-modified-4.0.0.dev10/lib/python3.9/site-packages/plover/key_combo.py", line 175, in parse_key_combo
    key_code = key_name_to_key_code(key_name)
KeyError: 'page_up'

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

antoniusf avatar Apr 02 '22 22:04 antoniusf

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):

2022-04-03 00:14:21,809 [Dummy-2] ERROR: engine on_stroked failed
Traceback (most recent call last):
...
  File "/nix/store/43kdzpv6k3p9v48nqhi3984jn3n9l2cr-python3.9-plover-modified-4.0.0.dev10/lib/python3.9/site-packages/plover/oslayer/wayland/keyboardlayout.py", line 81, in parse_key_combo
    return parse_key_combo(combo_string, self._combo_from_keyname.__getitem__)
  File "/nix/store/43kdzpv6k3p9v48nqhi3984jn3n9l2cr-python3.9-plover-modified-4.0.0.dev10/lib/python3.9/site-packages/plover/key_combo.py", line 175, in parse_key_combo
    key_code = key_name_to_key_code(key_name)
KeyError: 'page_up'

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 page_up is not working is because xkb.keysym_get_name is returning Prior, which is another name for Page_Up:

#define XK_Prior                         0xff55  /* Prior, previous */
#define XK_Page_Up                       0xff55

So will need to make sure we add aliases for those cases.

benoit-pierre avatar Apr 02 '22 23:04 benoit-pierre

List of aliases:

0xd0: ETH, Eth
0xffc8: F11, L1
0xffc9: F12, L2
0xffca: F13, L3
0xffcb: F14, L4
0xffcc: F15, L5
0xffcd: F16, L6
0xffce: F17, L7
0xffcf: F18, L8
0xffd0: F19, L9
0xffd1: F20, L10
0xffd2: F21, R1
0xffd3: F22, R2
0xffd4: F23, R3
0xffd5: F24, R4
0xffd6: F25, R5
0xffd7: F26, R6
0xffd8: F27, R7
0xffd9: F28, R8
0xffda: F29, R9
0xffdb: F30, R10
0xffdc: F31, R11
0xffdd: F32, R12
0xffde: F33, R13
0xffdf: F34, R14
0xffe0: F35, R15
0xff23: Henkan, Henkan_Mode
0xff9b: KP_Next, KP_Page_Down
0xff9a: KP_Page_Up, KP_Prior
0xff3e: Mae_Koho, PreviousCandidate
0xff7e: Mode_switch, script_switch
0xff3d: MultipleCandidate, Zen_Koho
0xff56: Next, Page_Down
0xff55: Page_Up, Prior
0xde: THORN, Thorn
0x27: apostrophe, quoteright
0x60: grave, quoteleft

benoit-pierre avatar Apr 02 '22 23:04 benoit-pierre

Should be fixed now.

benoit-pierre avatar Apr 02 '22 23:04 benoit-pierre

Thanks for the quick fix! Works for me now :tada:

antoniusf avatar Apr 02 '22 23:04 antoniusf

@antoniusf I noticed the mention of /nix/store in your logs, would you be able to share the overlay you're using for this branch? I've been trying to get it working on Nixos too but I've run into many issues, the latest of which is related to /usr/share/wayland/wayland.xml being missing...

mtoohey31 avatar Apr 08 '22 19:04 mtoohey31

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

  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 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!

antoniusf avatar Apr 09 '22 19:04 antoniusf

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"
      '';
    });
})

mtoohey31 avatar Apr 11 '22 03:04 mtoohey31

Oh nice, thank you!

antoniusf avatar Apr 12 '22 21:04 antoniusf

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 input_method_unstable_v2 not being supported when trying GeminiPR.

Does it still need XWayland for the GUI?

sagehane avatar Jul 16 '22 06:07 sagehane