On Thu, Mar 11, 2021 at 10:37 PM Daniel P. Berrangé <berra...@redhat.com>
wrote:
> We plan framebuffer update rects based on the VNC server surface. If the
> client doesn't support desktop resize, then the client bounds may differ
> from the server surface bounds. VNC clients may become upset if we then
> send an update message outside the bounds of the client desktop.
>
> This takes the approach of clamping the rectangles from the worker
> thread immediately before sending them. This may sometimes results in
> sending a framebuffer update message with zero rectangles.
>
> Signed-off-by: Daniel P. Berrangé <berra...@redhat.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
---
> ui/trace-events | 5 +++++
> ui/vnc-jobs.c | 44 ++++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 45 insertions(+), 4 deletions(-)
>
> diff --git a/ui/trace-events b/ui/trace-events
> index bd8f8a9d18..3838ae2d84 100644
> --- a/ui/trace-events
> +++ b/ui/trace-events
> @@ -59,6 +59,11 @@ vnc_client_throttle_audio(void *state, void *ioc,
> size_t offset) "VNC client thr
> vnc_client_unthrottle_forced(void *state, void *ioc) "VNC client
> unthrottle forced offset state=%p ioc=%p"
> vnc_client_unthrottle_incremental(void *state, void *ioc, size_t offset)
> "VNC client unthrottle incremental state=%p ioc=%p offset=%zu"
> vnc_client_output_limit(void *state, void *ioc, size_t offset, size_t
> threshold) "VNC client output limit state=%p ioc=%p offset=%zu
> threshold=%zu"
> +vnc_job_add_rect(void *state, void *job, int x, int y, int w, int h) "VNC
> add rect state=%p job=%p offset=%d,%d size=%dx%d"
> +vnc_job_discard_rect(void *state, void *job, int x, int y, int w, int h)
> "VNC job discard rect state=%p job=%p offset=%d,%d size=%dx%d"
> +vnc_job_clamp_rect(void *state, void *job, int x, int y, int w, int h)
> "VNC job clamp rect state=%p job=%p offset=%d,%d size=%dx%d"
> +vnc_job_clamped_rect(void *state, void *job, int x, int y, int w, int h)
> "VNC job clamp rect state=%p job=%p offset=%d,%d size=%dx%d"
> +vnc_job_nrects(void *state, void *job, int nrects) "VNC job state=%p
> job=%p nrects=%d"
> vnc_auth_init(void *display, int websock, int auth, int subauth) "VNC
> auth init state=%p websock=%d auth=%d subauth=%d"
> vnc_auth_start(void *state, int method) "VNC client auth start state=%p
> method=%d"
> vnc_auth_pass(void *state, int method) "VNC client auth passed state=%p
> method=%d"
> diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
> index dbbfbefe56..4562bf8928 100644
> --- a/ui/vnc-jobs.c
> +++ b/ui/vnc-jobs.c
> @@ -32,6 +32,7 @@
> #include "qemu/sockets.h"
> #include "qemu/main-loop.h"
> #include "block/aio.h"
> +#include "trace.h"
>
> /*
> * Locking:
> @@ -94,6 +95,8 @@ int vnc_job_add_rect(VncJob *job, int x, int y, int w,
> int h)
> {
> VncRectEntry *entry = g_new0(VncRectEntry, 1);
>
> + trace_vnc_job_add_rect(job->vs, job, x, y, w, h);
> +
> entry->rect.x = x;
> entry->rect.y = y;
> entry->rect.w = w;
> @@ -190,6 +193,8 @@ static void vnc_async_encoding_start(VncState *orig,
> VncState *local)
> local->zlib = orig->zlib;
> local->hextile = orig->hextile;
> local->zrle = orig->zrle;
> + local->client_width = orig->client_width;
> + local->client_height = orig->client_height;
> }
>
> static void vnc_async_encoding_end(VncState *orig, VncState *local)
> @@ -202,6 +207,34 @@ static void vnc_async_encoding_end(VncState *orig,
> VncState *local)
> orig->lossy_rect = local->lossy_rect;
> }
>
> +static bool vnc_worker_clamp_rect(VncState *vs, VncJob *job, VncRect
> *rect)
> +{
> + trace_vnc_job_clamp_rect(vs, job, rect->x, rect->y, rect->w, rect->h);
> +
> + if (rect->x >= vs->client_width) {
> + goto discard;
> + }
> + rect->w = MIN(vs->client_width - rect->x, rect->w);
> + if (rect->w == 0) {
> + goto discard;
> + }
> +
> + if (rect->y >= vs->client_height) {
> + goto discard;
> + }
> + rect->h = MIN(vs->client_height - rect->y, rect->h);
> + if (rect->h == 0) {
> + goto discard;
> + }
> +
> + trace_vnc_job_clamped_rect(vs, job, rect->x, rect->y, rect->w,
> rect->h);
> + return true;
> +
> + discard:
> + trace_vnc_job_discard_rect(vs, job, rect->x, rect->y, rect->w,
> rect->h);
> + return false;
> +}
> +
> static int vnc_worker_thread_loop(VncJobQueue *queue)
> {
> VncJob *job;
> @@ -260,14 +293,17 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
> goto disconnected;
> }
>
> - n = vnc_send_framebuffer_update(&vs, entry->rect.x, entry->rect.y,
> - entry->rect.w, entry->rect.h);
> + if (vnc_worker_clamp_rect(&vs, job, &entry->rect)) {
> + n = vnc_send_framebuffer_update(&vs, entry->rect.x,
> entry->rect.y,
> + entry->rect.w, entry->rect.h);
>
> - if (n >= 0) {
> - n_rectangles += n;
> + if (n >= 0) {
> + n_rectangles += n;
> + }
> }
> g_free(entry);
> }
> + trace_vnc_job_nrects(&vs, job, n_rectangles);
> vnc_unlock_display(job->vs->vd);
>
> /* Put n_rectangles at the beginning of the message */
> --
> 2.29.2
>
>
>
--
Marc-André Lureau
