Implement Wayland support using PipeWire and xdg-desktop-portal
ClosedPublic

Authored by jgrulich on Apr 9 2019, 7:13 AM.

Details

Summary

Adds a new framebuffer implementation, which uses xdg-desktop-portal to support remote
desktop on Wayland and uses PipeWire to deliver the screen content. So far only mouse
support is implemented, because keyboard support is missing on KWin side.

Diff Detail

Repository
R437 Desktop Sharing
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
jgrulich requested review of this revision.Apr 9 2019, 7:13 AM
jgrulich created this revision.
jgrulich updated this revision to Diff 55800.Apr 9 2019, 7:14 AM

Remove unwanted kdev4 file

Note that this change requires KWin and xdg-desktop-portal-kde from master (upcoming Plasma 5.16) for mouse control, otherwise only screen content will be seen, but you won't be able to control it with your mouse.

PW framebuffer looks okay, it's the event system that worries me.

  1. We kind of force krfb to link with DBus, even if our plugin is not present.
  2. Are we moving mouse via D-Bus?..

The thing is, we are stuck with old event system in krfb that was designed with X11 in mind.
What I tried to do in the past is break it down into plugins, this allowed me to get rid of clauses like "are we on X11?"
and "do this, but only if we have that one protocol" and so on. Unfortunately no one was there to review it.

Take a look at the last commits in this repo: https://gitlab.com/Kanedias/KRfb/commits/gbm-vnc,
especially those that are related to event system.

  • We already have fakeinput protocol (at least for mouse) baked into KWin so you can use it, it's faster than D-Bus for sure.
  • We can implement D-Bus based events plugin without raising potential bugs in whole event system.
  • You can move linking against D-Bus into the events plugin, so distro maintainers will have more flexibility over deps.
krfb/CMakeLists.txt
24

Redundant change fragment

krfb/events.cpp
42

What's supposed to be here?

krfb/rfbserver.cpp
149

Doesn't QClipboard work on Wayland?

Kanedias added a subscriber: romangg.

@romangg, as you are now maintaining KWin, take a look at this as well.

P.S.

@jgrulich I also have to say that I'm not against DBus implementation in general, merely worried a bit. I see you also have keyboard protocol, that's a breakthrough! Never managed to grasp keysyms conversion myself.

And about event system again, is it okay that it retrieves custom properties from framebuffer plugin? Maybe we can have it establish other connection just for events?

krfb/rfbservermanager.cpp
137

Not sure KRfb codebase was converted but we are using C++11 for 8 years now, we can have for (auto server: d->servers)

PW framebuffer looks okay, it's the event system that worries me.

  1. We kind of force krfb to link with DBus, even if our plugin is not present.
  2. Are we moving mouse via D-Bus?..

Yes, we are, the reason why I use portals for this is that it will work in Flatpak/Snap without any modifications and it will also work in Gnome if someone decides to use it there.

The thing is, we are stuck with old event system in krfb that was designed with X11 in mind.
What I tried to do in the past is break it down into plugins, this allowed me to get rid of clauses like "are we on X11?"
and "do this, but only if we have that one protocol" and so on. Unfortunately no one was there to review it.

Take a look at the last commits in this repo: https://gitlab.com/Kanedias/KRfb/commits/gbm-vnc,
especially those that are related to event system.

I see, I will look and try to improve this D-Bus dependency situation.

  • We already have fakeinput protocol (at least for mouse) baked into KWin so you can use it, it's faster than D-Bus for sure.

I know and it's what I use in xdg-desktop-portal-kde, the reason why I don't use fakeinput directly is mentioned above.

P.S.

@jgrulich I also have to say that I'm not against DBus implementation in general, merely worried a bit. I see you also have keyboard protocol, that's a breakthrough! Never managed to grasp keysyms conversion myself.

I don't have keyboard protocol, it's still missing. There are defined methods in xdg-desktop-portal for keyboard, but we don't have a KDE implementation, because there is no support for this in fakeinput protocol in KWayland.

And about event system again, is it okay that it retrieves custom properties from framebuffer plugin? Maybe we can have it establish other connection just for events?

I did this only to be able to initialize framebuffer after we establish a PW connection, this cannot be done in advance and I need to know the resolution. Once I receive information about the resolution from PW/Portals, I can initialize framebuffer.

krfb/events.cpp
42

Keyboard support, once it's implemented . I should maybe remove this completely until there is keyboard support we can use.

krfb/rfbserver.cpp
149

Nope, or at least this way it was breaking VNC protocol and I wasn't able to connect.

Kanedias added inline comments.Apr 11 2019, 5:52 PM
krfb/rfbserver.cpp
149

Hmm I think I know the reason. Try to write the password without copying it from anywhere or selecting anything.
We need to disable sending paste events to the client before password ask request is answered.

jgrulich updated this revision to Diff 57246.Apr 30 2019, 10:40 AM

Make Krfb event handlers as plugins

jgrulich added inline comments.Apr 30 2019, 10:41 AM
krfb/rfbserver.cpp
149

I always type the password, I don't paste it and I don't have any text selected.

Oh, only now noticed this patch. Should I take directly a look at it or is there some more work to do at the moment?

Oh, only now noticed this patch. Should I take directly a look at it or is there some more work to do at the moment?

No more work is needed or in plan at this moment. There is missing support for keyboard, but this is missing also on xdg-desktop-portal-kde and KWayland side.

pino added a subscriber: pino.May 18 2019, 10:18 AM

Btw, please remove all the translations (ru, and x-test) from .desktop, and .json files. There is a system to handle them, so these manually injected ones will be removed the day after this work is merged.

jgrulich updated this revision to Diff 58328.May 20 2019, 6:10 AM

Remove [ru] and [x-test] translations from desktop and json files

pino added a comment.May 20 2019, 6:19 AM

Remove [ru] and [x-test] translations from desktop and json files

Almost, there are still krfb/krfb-events.desktop, krfb/krfb-events.json.

Also, please remove the whitespace-only changes in main.cpp, and rfbservermanager.cpp.

Furthermore: since D21267: Do not crash on wayland, gracefully exit with error instead was merged, you must add a new check to allow wayland as display server, otherwise krfb will not even start... Even a simple

else if (is_wayland /* whatever is the proper check */) {
  /* nothing to check */
}

will do it.

Kanedias accepted this revision.EditedMay 22 2019, 9:17 PM

The only issue left is clipboard sharing. I'm certain it's triggered in some weird way and sends stray packets when VNC Server expects password. I've definitely seen network activity when the password field is selected/filled on Wayland.
Something I didn't see on X. Other than that, I'm mostly satisfied with the result.

When this gets merged we should notify maintainers that event system is plugin-based now too. They won't like it :)

P.S. Thanks for your hard work @jgrulich, I finally feel my work was not in vain.
Who knows, maybe I'll find time to work on KDE again.

This revision is now accepted and ready to land.May 22 2019, 9:17 PM
This revision was automatically updated to reflect the committed changes.

The only issue left is clipboard sharing. I'm certain it's triggered in some weird way and sends stray packets when VNC Server expects password. I've definitely seen network activity when the password field is selected/filled on Wayland.
Something I didn't see on X. Other than that, I'm mostly satisfied with the result.

I merged it for now as I don't have time to look into this further, but I will try to return back to it and figure out what's wrong.

When this gets merged we should notify maintainers that event system is plugin-based now too. They won't like it :)

Also that new dependencies were introduced, some packagers do not check for missing optional dependencies and may end up without Wayland support.

\o/

For promo purposes, how can I use this or see it in action? What apps are necessary?

\o/

For promo purposes, how can I use this or see it in action? What apps are necessary?

Things you need:

  1. PipeWire (it's also needed as build dependency for xdg-desktop-portal-kde and krfb)
  2. xdg-desktop-portal-kde with remote desktop (Plasma 5.16 beta)
  3. krfb

Everything should be started automatically so no need to do anything special, just in krfb you need choose "pw" plugin to be used in configuration. Next time you start krfb you will get a dialog with screen to choose for sharing. Once you start sharing a screen, you can connect over VNC.