GUACAMOLE-1196: Implement support for VNC display size updates #469
Conversation
|
Okay, did some initial testing, and the desktop size messages are being sent correctly through to the VNC server. Unfortunately I'm hitting an issue with the VNC server (tigervnc) where it is rejecting the size updates due to missing modelines for Xvnc. Obviously having to load all the possible modelines for all of the possible resolutions that Guacamole might request is a bit silly, so I'm thinking that I might need to find another VNC server that doesn't have this requirement. Good news, though, is that the changes on the Guacamole side seem sound. |
necouchman
force-pushed
the
jira/1196
branch
from
01672cd
to
171b560
Compare
|
Hi , any update regarding this wonderfull and waited PR ? =D |
src/protocols/vnc/display.c
Outdated
| void guac_vnc_display_set_size(guac_vnc_display* display, | ||
| rfbClient* rfb_client, int width, int height) { | ||
|
|
||
| /* Fit width within bounds, adjusting height to maintain aspect ratio */ | ||
| guac_common_display_fit(&width, &height); | ||
|
|
||
| /* Fit height within bounds, adjusting width to maintain aspect ratio */ | ||
| guac_common_display_fit(&height, &width); | ||
|
|
||
| /* Width must be even */ | ||
| if (width % 2 == 1) | ||
| width -= 1; | ||
|
|
||
| /* Store deferred size */ | ||
| display->requested_width = width; | ||
| display->requested_height = height; | ||
|
|
||
| /* Send display update notification if possible */ | ||
| guac_vnc_display_update_size(display, rfb_client); | ||
|
|
||
| } |
There was a problem hiding this comment.
The limitations enforced here were originally established specifically due to RDP:
- The "Display Update" channel for RDP defines an upper limit of 8192 pixels for display dimensions.
- The "Display Update" channel mandates that the width provided is even.
- Windows was known to become unresponsive (requiring a reboot to resolve) if multiple display update requests were sent in rapid succession.
Except for enforcing our own upper bound on screen size by choice, which is a good idea regardless of whether it's required by the underlying protocol, I'm not sure these same limitations apply to VNC.
If we instead send the display size request immediately upon receiving the size instruction from the user (adjusting received values to fit our own maximum), do things still work?
There was a problem hiding this comment.
I'm happy to give it a go, although my overall ability to test these changes is limited. I'll hunt around, again, for a VNC server that supports dynamic resize and see if I can find one that is free and try it out, but I had limited success last time around.
Looks like Mike has started reviewing it. Any chance you'd be able to test it out? I've been unable to get access to a VNC server that actually supports it... |
tbh, i only know tigerVnc, i can try the latest one , because it seems taht the options is presnet in latest tigerVNC + vncserver => |
If you're able to give it a try with the changes from this PR, that'd be great. I'm also using TigerVNC, and, according to the Xvnc man page, AcceptSetDesktopSize is enabled by default, so it should be working, but I was getting errors indicating that any resolution I wanted to set needed a modeline in the X configuration, which just isn't tenable. |
|
Okay, I was able to test it and am successfully sending the So, something about the resize is not working correctly - I'm not sure if there's something that needs to be updated in the |
|
|
@x-7 Thank you for testing! I'm glad to hear that it works for you - maybe the error that I'm currently seeing is actually a bug in the VNC Server, and not something with how I've implemented it. I'll try to find some additional VNC servers to test with - maybe even grab an eval copy of RealVNC or something like that and see if that works. |
yes,I used two clients for testing: next-terminal(https://github.com/dushixiang/next-terminal) and gucalmole-client.
|
This PR takes a crack at implementing the recently-added libvncclient support for
SendExtDesktopSize(), which should allow Guacamole as a VNC client to send its size to VNC servers that support such functionality and have the display dynamically resized.I'm creating as a draft for the moment since I have not actually tested the functionality. Among the things I'm not sure of:
resolutionsetting which specifies the DPI resolution of the screen. I don't see any reason to try to mirror this into the VNC protocol, as I don't think VNC actually does anything with it (I think the DPI is fixed), but if I'm wrong let me know and I can add that support and try to update the code accordingly.WriteToRFBServer()- I'm wondering if it's worth trying to implement the display update functionality with that in cases where libvncclient doesn't contain theSendExtDesktopSize()function?message_lockmutex for sending this message, as in the RDP protocol, but I don't know if there are other places where this lock should be used (authentication, for example), or if it's even required in this scenario?