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.

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

GUACAMOLE-1196: Fix VNC resizing behavior. #544

Merged
merged 2 commits into from
Sep 5, 2024

Conversation

jmuehlner
Copy link
Contributor

VNC resizing was not working for me - I'm getting this message every time I attempt to resize the window, as well as on initial load:

ERROR: Screen has not been initialized, cannot send resize.

I dug into it, and I found two problems:

  1. The RFB client screen ID was never set, which caused any message from the server specifying a screen size to be ignored by the client.
  2. The guacamole display was never resized after sending the resize request to the VNC server.

I've fixed these two issues, though I'm not sure if this is the cleanest way of doing it.
@necouchman mind having a look? I know you're much more familliar with this code than me.

#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

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

@jmuehlner jmuehlner changed the base branch from main to staging/1.6.0 September 5, 2024 22:37
Copy link
Contributor

@necouchman necouchman left a comment

Choose a reason for hiding this comment

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

Thanks @jmuehlner !

@necouchman necouchman merged commit 725aa12 into apache:staging/1.6.0 Sep 5, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants