Skip to content
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.

Sign up for GitHub

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

Jump to bottom

GUACAMOLE-1196: Fix VNC resizing behavior. #544

Merged
merged 2 commits into from
Sep 5, 2024
Merged

GUACAMOLE-1196: Fix VNC resizing behavior. #544

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5def98a
GUACAMOLE-1196: Fix VNC resizing behavior.
jmuehlner Sep 5, 2024
fe6befc
GUACAMOLE-1196: Remove uninitialized check from VNC resize routine to…
jmuehlner Sep 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 26 additions & 20 deletions src/protocols/vnc/display.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,22 +182,22 @@ static rfbBool guac_vnc_send_desktop_size(rfbClient* client, int width, int heig
/* Get the Guacamole client data */
guac_client* gc = rfbClientGetClientData(client, GUAC_VNC_CLIENT_KEY);

guac_client_log(gc, GUAC_LOG_TRACE,
"Current screen size is %ix%i; setting new size %ix%i\n",
rfbClientSwap16IfLE(client->screen.width),
rfbClientSwap16IfLE(client->screen.height),
width, height);

#ifdef LIBVNC_CLIENT_HAS_SCREEN
/* Don't send an update if the sreen appears to be uninitialized. */
Copy link
Contributor Author

@jmuehlner jmuehlner Sep 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this because the screen size seems to be never initialized by the time that the owner attempts to set the initial size, so any initial resize will fail.

The screen size will be updated when a message comes back from the server specifying the updated size, but this in practice does not happen before the initial resize attempt.

In any case, removing this check fixes the initial resize issue, and doesn't seem to cause any problems that I can see.

See https://github.com/LibVNC/libvncserver/blob/27617e141ed91551087960ff8e48db0db8258dad/src/libvncclient/rfbclient.c#L2126-L2128

if (client->screen.width == 0 || client->screen.height == 0) {
guac_client_log(gc, GUAC_LOG_ERROR, "Screen has not been initialized, cannot send resize.");
return FALSE;
}

/* Don't send an update if the requested dimensions are identical to current dimensions. */
if (client->screen.width == rfbClientSwap16IfLE(width) && client->screen.height == rfbClientSwap16IfLE(height)) {
guac_client_log(gc, GUAC_LOG_WARNING, "Screen size has not changed, not sending update.");
return FALSE;
}
#else
/* Don't send an update if the sreen appears to be uninitialized. */
/* Don't send an update if the screen appears to be uninitialized. */
if (client->width == 0 || client->height == 0) {
guac_client_log(gc, GUAC_LOG_ERROR, "Framebuffer has not been initialized, cannot send resize.");
guac_client_log(gc, GUAC_LOG_WARNING, "Framebuffer has not been initialized, cannot send resize.");
return FALSE;
}

Expand All @@ -219,8 +219,8 @@ static rfbBool guac_vnc_send_desktop_size(rfbClient* client, int width, int heig
*/

/* Set up the messages. */
rfbSetDesktopSizeMsg size_msg;
rfbExtDesktopScreen new_screen;
rfbSetDesktopSizeMsg size_msg = { 0 };
rfbExtDesktopScreen new_screen = { 0 };

/* Configure the desktop size update message. */
size_msg.type = rfbSetDesktopSize;
Expand All @@ -230,13 +230,13 @@ static rfbBool guac_vnc_send_desktop_size(rfbClient* client, int width, int heig

#ifdef LIBVNC_CLIENT_HAS_SCREEN
/* Configure the screen update message. */
new_screen.id = client->screen.id;
new_screen.id = GUAC_VNC_SCREEN_ID;
new_screen.x = client->screen.x;
new_screen.y = client->screen.y;
new_screen.flags = client->screen.flags;
#else
/* Assume screen starts at the origin. */
new_screen.id = 0;
new_screen.id = GUAC_VNC_SCREEN_ID;
new_screen.x = 0;
new_screen.y = 0;
new_screen.flags = 0;
Expand All @@ -245,16 +245,12 @@ static rfbBool guac_vnc_send_desktop_size(rfbClient* client, int width, int heig
new_screen.width = rfbClientSwap16IfLE(width);
new_screen.height = rfbClientSwap16IfLE(height);

#ifdef LIBVNC_CLIENT_HAS_REQUESTED_RESIZE
/* Stop updates while the resize is in progress. */
client->requestedResize = TRUE;
#endif // LIBVNC_HAS_REQUESTED_RESIZE

/* Send the resize messages to the remote server. */
if (!WriteToRFBServer(client, (char *)&size_msg, sz_rfbSetDesktopSizeMsg)
|| !WriteToRFBServer(client, (char *)&new_screen, sz_rfbExtDesktopScreen)) {

guac_client_log(gc, GUAC_LOG_ERROR, "Failed to send new desktop and screen size to the VNC server.");
guac_client_log(gc, GUAC_LOG_WARNING,
"Failed to send new desktop and screen size to the VNC server.");
return FALSE;

}
Expand All @@ -266,14 +262,17 @@ static rfbBool guac_vnc_send_desktop_size(rfbClient* client, int width, int heig
#endif // LIBVNC_CLIENT_HAS_SCREEN

#ifdef LIBVNC_CLIENT_HAS_REQUESTED_RESIZE
/* Request a full screen update. */
client->requestedResize = FALSE;
#endif // LIBVNC_HAS_REQUESTED_RESIZE

if (!SendFramebufferUpdateRequest(client, 0, 0, width, height, FALSE)) {
guac_client_log(gc, GUAC_LOG_WARNING, "Failed to request a full screen update.");
}

#ifdef LIBVNC_CLIENT_HAS_REQUESTED_RESIZE
client->requestedResize = TRUE;
#endif // LIBVNC_HAS_REQUESTED_RESIZE

/* Update should be successful. */
return TRUE;
}
Expand Down Expand Up @@ -310,8 +309,15 @@ void guac_vnc_display_set_size(rfbClient* client, int width, int height) {

/* Send the display size update. */
guac_client_log(gc, GUAC_LOG_TRACE, "Setting VNC display size.");
if (guac_vnc_send_desktop_size(client, width, height))
if (guac_vnc_send_desktop_size(client, width, height)) {
guac_client_log(gc, GUAC_LOG_TRACE, "Successfully sent desktop size message.");

/* Resize the surface now that the VNC size update has completed */
if (vnc_client->display != NULL)
guac_common_surface_resize(vnc_client->display->default_surface,
width, height);
}

else
guac_client_log(gc, GUAC_LOG_TRACE, "Failed to send desktop size message.");

Expand Down
6 changes: 6 additions & 0 deletions src/protocols/vnc/vnc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@

#include <pthread.h>

/**
* The ID of the RFB client screen. If multi-screen support is added, more than
* one ID will be needed as well.
*/
#define GUAC_VNC_SCREEN_ID 1

/**
* VNC-specific client data.
*/
Expand Down
Loading