-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add "labwc over VNC" backend (Wayland) #3587
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
base: devel
Are you sure you want to change the base?
Conversation
Wow, this is quite a monumental work! The refactoring aspects look good as well. I really appreciate the work you put into this project and the way you interact with the user community of xrdp. I will give this a build and a test run as soon as I have time. |
Thanks for that @lcniel I've not gotten to the bottom of the resize yet. The resize state machine triggers, but then labwc resizes back to a smaller size. I've added some (untested) code to enable logging for labwc and wayvnc which might help to debug this. |
It's weird. I had some occasional wonkiness with interactive resizing with TurboVNC when connected to WayVNC (like the configuration resetting once connected, I had to change it away from server-side resizing every time, but that might just be some RFB implementation thing), but most of the time it worked without issue. |
0cd609c
to
6edd861
Compare
I'm fairly confident the resize issue is an xrdp problem. Resizing is a complicated dance involving the client and server. wayvnc is using an additional message in its ExtendedDesktopSize response which essentially informs xrdp the resize request has been forwarded. We don't cope well with that message. I suspect what's happening is the resize to the client screen size is happening, and then it is getting overridden when the server reports its original screen size late. I'm not completely sure about this, but I'm sure it's a fixable problem. I'm a little busy IRL for the next week or so, but I'll get to it when I can. Debug logging for both labwc and wayvnc can now be enabled in sesman.ini. |
I had a look at the neatvnc source code: https://github.com/any1/neatvnc/blob/4962e0af5d550b2c4fd34c79dbdca543f233d87c/src/server.c#L1632 and that looks like exactly what happens, yeah (and it makes perfect sense architecture-wise). It looks like there's no code in xrdp for handling this at all and xrdp only ever checks that the response code is 0 (if I am parsing its logic correctly) and otherwise calls failure. So I think another case needs to be added here to capture request forwarded? |
I think you're right, from the debugging I've done. I also need a timer event so can pick up if the resize doesn't happen. I'm adding that in at the moment, but it's going to be a bit slow for the rest of the week. |
5451d0f
to
971bb63
Compare
I've added support for the forwarded resize request to VNC. This seems to work pretty well, but occasionally I get a We're going to need the 'forwarded resize request' support in |
971bb63
to
06fd9fd
Compare
The required VNC functionality is now merged into devel, and I've rebased this PR. |
Hi, I tried it out, it works as advertised! Nice work! I can confirm though that wayvnc will segfault from time to time. I seem to be able to consistently trigger it by resizing the window below c. 860x720 (extrapolating from my monitor resolution). However, if I directly connect at this resolution, it will connect without crashing. In general this is not consistent with how my client (Remmina) normally behaves when connecting to xrdp. |
I think the next step will be for me to move to Trixie, build |
I had a bit of a look but I can't seem to get trace-level debug for wayvnc via the sesman.ini settings. I did get this, by resizing back and forth: From journald:
From wayvnc debug (snipped)
I also get some tearing from time to time. It looks to me like screen width is off by some (variable?) amount at some points. I think there might be both that and a race condition going on? I can't easily match up logs since the wayvnc ones don't have timestamps. EDIT: Another try. xrdp:
wayvnc:
Sizes seem to match here and I didn't see the same kind of tearing. But I'm not sure wayvnc's messages should look like that? Should it be sending multiple extended desktop sizes that are then ignored? My vacation starts next week so depending on how things are I might also invest some time into this. At least I can test stuff on RHEL-likes. I don't know if it would be feasible to run valgrind or something to try and catch the segafault. I'm going to guess there's an incorrectly sized buffer or similar at play here. |
I'll try with the latest |
The monolithic session module is split into: - A control modile (session.[hc]) - A session base class module (session_base.[hc]) - An X11 specific class, derived from the base class. The session_parameters object gets its own include file, so that the class files do not need to include session.h. This picks up layering violations. The hope is it will be simple to add another labwc-specific class with minimal changes to other modules.
xrdp makes a lot of assumptions that a display will have an integer associated with it. This commit removes that assumption, and allows a display (or wayland display) to be either something like ":10" (for X11 display ten), or "wayland-1" (for wayland display 1).
Make it clear that the display number passed to auth_start_session() is for X11 only.
Add a way to create debug logs for labwc and wayvnc
06fd9fd
to
5818fce
Compare
Force-push to incorporate #3596 and #3597 I've put this onto Trixie now. Running with the stock wayvnc was identical to Xubuntu 25-04 in terms of results. I've now rebuilt wayvnc from source:-
Using the devel version seems a lot more solid and I've not crashed it yet with resizing. I'll try to work out if a recent change to one of the three products above has fixed the resize issue. |
I've tried rolling back to :
The change is a few fixes to the https://github.com/any1/neatvnc/releases/tag/v0.9.2 That seems pretty solid too. I've rebased on wayvnc 0.9.1 / neatvnc 0.9.5, and I'll continue testing with that for now. |
I tested with wayvnc 0.9.1, neatvnc 0.9.5, and a built-from-source aml 0.3.0 (all very simple on Rocky 10, it's enough to just download source RPM:s and replace the version numbers before Very promising, all of this! |
See #2637 (pinned issue) for the background on this.
This is more a show-and-tell than a PR at the moment. I though I'd put it out there to get some comments though. If we get somewhere useful with this I'll refactor a different PR for a merge.
First a couple of screenshots. This is a remmina client talking to an Xunutu25-04 backend. I've tested nothing else yet:-
Session is XFCE 4.20 running under the labwc compositor.
Working:-
Not working:-
There are some code problems too:-
The biggest issue to my mind is with X11 display numbers which are used extensively in xrdp. For X11 these are globals, and (effectively) always integers. Wayland displays are always strings. So the socket dir for a running session looks like this:-
I've tried to keep compatibility with number for X11, but this is distorting the code in places. I think it might be better to use strings like
x11-10
(e.g. for X11 display 10) rather than a straight10
. This will need a change to the audio drivers, but we can stick with the plain number for xorgxrdp support.I'm quite busy IRL for a week or so. After that, I'll start work on fixing deficiencies and trying other platforms, in particular Trixie which is now out.
Comments/questions are very welcome!