diff --git a/src/protocols/rdp/gdi.c b/src/protocols/rdp/gdi.c index ff22f2824..f2fdf88e2 100644 --- a/src/protocols/rdp/gdi.c +++ b/src/protocols/rdp/gdi.c @@ -81,18 +81,10 @@ BOOL guac_rdp_gdi_begin_paint(rdpContext* context) { guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; rdpGdi* gdi = context->gdi; - GUAC_ASSERT(rdp_client->current_paint == NULL); - - guac_display_layer* default_layer = - guac_display_default_layer(rdp_client->display); - - guac_display_layer_raw_context* current_paint = - guac_display_layer_open_raw(default_layer); - current_paint->buffer = gdi->primary_buffer; - current_paint->stride = gdi->stride; - guac_rect_init(¤t_paint->bounds, 0, 0, gdi->width, gdi->height); - - rdp_client->current_paint = current_paint; + guac_display_layer_raw_context* current_context = rdp_client->current_context; + current_context->buffer = gdi->primary_buffer; + current_context->stride = gdi->stride; + guac_rect_init(¤t_context->bounds, 0, 0, gdi->width, gdi->height); return TRUE; @@ -104,12 +96,7 @@ BOOL guac_rdp_gdi_end_paint(rdpContext* context) { guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; rdpGdi* gdi = context->gdi; - GUAC_ASSERT(rdp_client->current_paint != NULL); - guac_display_layer_raw_context* current_paint = rdp_client->current_paint; - rdp_client->current_paint = NULL; - - guac_display_layer* default_layer = - guac_display_default_layer(rdp_client->display); + guac_display_layer_raw_context* current_context = rdp_client->current_context; /* Ignore paint if GDI output is suppressed */ if (gdi->suppressOutput) @@ -126,12 +113,11 @@ BOOL guac_rdp_gdi_end_paint(rdpContext* context) { guac_rect dst_rect; guac_rect_init(&dst_rect, x, y, w, h); - guac_rect_constrain(&dst_rect, ¤t_paint->bounds); + guac_rect_constrain(&dst_rect, ¤t_context->bounds); - guac_rect_extend(¤t_paint->dirty, &dst_rect); + guac_rect_extend(¤t_context->dirty, &dst_rect); paint_complete: - guac_display_layer_close_raw(default_layer, current_paint); return TRUE; } @@ -146,27 +132,19 @@ BOOL guac_rdp_gdi_desktop_resize(rdpContext* context) { int width = guac_rdp_get_width(context->instance); int height = guac_rdp_get_height(context->instance); - /* It's not expected that a resize will be requested by FreeRDP in the - * middle of a paint operation, but if this does happen we incorporate the - * resize into that operation */ - guac_display_layer_raw_context* current_paint = rdp_client->current_paint; - if (current_paint == NULL) - current_paint = guac_display_layer_open_raw(default_layer); + guac_display_layer_raw_context* current_context = rdp_client->current_context; + /* Resize FreeRDP's GDI buffer */ BOOL retval = gdi_resize(context->gdi, width, height); GUAC_ASSERT(gdi->primary_buffer != NULL); /* Update our reference to the GDI buffer, as well as any structural * details, which may now all be different */ - current_paint->buffer = gdi->primary_buffer; - current_paint->stride = gdi->stride; - guac_rect_init(¤t_paint->bounds, 0, 0, gdi->width, gdi->height); - - /* There's no need to close the context if we are just adding to an - * in-progress paint operation */ - if (current_paint != rdp_client->current_paint) - guac_display_layer_close_raw(default_layer, current_paint); + current_context->buffer = gdi->primary_buffer; + current_context->stride = gdi->stride; + guac_rect_init(¤t_context->bounds, 0, 0, gdi->width, gdi->height); + /* Resize layer to match new display dimensions and underlying buffer */ guac_display_layer_resize(default_layer, gdi->width, gdi->height); guac_client_log(client, GUAC_LOG_DEBUG, "Server resized display to %ix%i", gdi->width, gdi->height); diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c index e0d53d843..5022b616a 100644 --- a/src/protocols/rdp/rdp.c +++ b/src/protocols/rdp/rdp.c @@ -465,6 +465,41 @@ static int rdp_guac_client_wait_for_messages(guac_client* client, } +/** + * Handles any queued RDP-related events, including inbound RDP messages that + * have been received, updating the Guacamole display accordingly. + * + * @param rdp_client + * The guac_rdp_client of the RDP connection whose current messages should + * be handled. + * + * @return + * True (non-zero) if messages were handled successfully, false (zero) + * otherwise. + */ +static int guac_rdp_handle_events(guac_rdp_client* rdp_client) { + + freerdp* rdp_inst = rdp_client->rdp_inst; + guac_display_layer* default_layer = guac_display_default_layer(rdp_client->display); + + /* All potential drawing operations must occur while holding an open context */ + guac_display_layer_raw_context* context = guac_display_layer_open_raw(default_layer); + rdp_client->current_context = context; + + /* Actually handle messages (this may result in drawing to the + * guac_display, resizing the display buffer, etc.) */ + pthread_mutex_lock(&(rdp_client->message_lock)); + int retval = freerdp_check_event_handles(GUAC_RDP_CONTEXT(rdp_inst)); + pthread_mutex_unlock(&(rdp_client->message_lock)); + + /* There will be no further drawing operations */ + guac_display_layer_close_raw(default_layer, context); + rdp_client->current_context = NULL; + + return retval; + +} + /** * Connects to an RDP server as described by the guac_rdp_settings structure * associated with the given client, allocating and freeing all objects @@ -590,13 +625,9 @@ static int guac_rdp_handle_connection(guac_client* client) { do { /* Handle any queued FreeRDP events (this may result in RDP - * messages being sent) */ - pthread_mutex_lock(&(rdp_client->message_lock)); - int event_result = freerdp_check_event_handles(GUAC_RDP_CONTEXT(rdp_inst)); - pthread_mutex_unlock(&(rdp_client->message_lock)); - - /* Abort if FreeRDP event handling fails */ - if (!event_result) { + * messages being sent), aborting if FreeRDP event handling + * fails */ + if (!guac_rdp_handle_events(rdp_client)) { wait_result = -1; break; } @@ -668,14 +699,6 @@ static int guac_rdp_handle_connection(guac_client* client) { freerdp_disconnect(rdp_inst); pthread_mutex_unlock(&(rdp_client->message_lock)); - /* Any outstanding paint operation must have completed by now (we must do - * this before freeing FreeRDP's GDI as the guac_display will have a - * reference to the GDI's primary_buffer) */ - if (rdp_client->current_paint) { - guac_display_layer_close_raw(guac_display_default_layer(rdp_client->display), rdp_client->current_paint); - rdp_client->current_paint = NULL; - } - /* Free display */ guac_display_free(rdp_client->display); rdp_client->display = NULL; diff --git a/src/protocols/rdp/rdp.h b/src/protocols/rdp/rdp.h index 96d66284c..6f63e4d48 100644 --- a/src/protocols/rdp/rdp.h +++ b/src/protocols/rdp/rdp.h @@ -107,11 +107,13 @@ typedef struct guac_rdp_client { guac_display_layer* current_surface; /** - * The current, in-progress paint operation, as signalled by FreeRDP's - * "BeginPaint" callback. Once the paint operation is complete (signalled - * by "EndPaint"), this will be NULL. + * The current raw context that can be used to draw to Guacamole's default + * layer. This context is obtained prior to handling any events/messages + * within FreeRDP and closed when FreeRDP is done handling those + * events/messages. If event handling is not currently underway, this will + * be NULL. */ - guac_display_layer_raw_context* current_paint; + guac_display_layer_raw_context* current_context; /** * Whether the RDP server has reported that a new frame is in progress, and diff --git a/src/protocols/vnc/display.c b/src/protocols/vnc/display.c index 6abbf36d0..70b03be8a 100644 --- a/src/protocols/vnc/display.c +++ b/src/protocols/vnc/display.c @@ -51,48 +51,25 @@ void guac_vnc_update(rfbClient* client, int x, int y, int w, int h) { guac_vnc_client* vnc_client = (guac_vnc_client*) gc->data; guac_display_layer* default_layer = guac_display_default_layer(vnc_client->display); - int rfb_height = client->height; - int rfb_width = client->width; - - guac_display_layer_raw_context* context; + guac_display_layer_raw_context* context = vnc_client->current_context; unsigned int vnc_bpp = client->format.bitsPerPixel / 8; - size_t vnc_stride = guac_mem_ckd_mul_or_die(vnc_bpp, rfb_width); + size_t vnc_stride = guac_mem_ckd_mul_or_die(vnc_bpp, client->width); /* Convert operation coordinates to guac_rect for easier manipulation */ guac_rect op_bounds; guac_rect_init(&op_bounds, x, y, w, h); - /* Point directly at framebuffer if the pixel format used is identical to - * that expected by guac_display. Resize of the layer is implicit in this - * case. */ - if (vnc_bpp == GUAC_DISPLAY_LAYER_RAW_BPP && !vnc_client->settings->swap_red_blue) { - - context = guac_display_layer_open_raw(default_layer); - context->buffer = client->frameBuffer; - context->stride = vnc_stride; - - /* Update bounds of pending frame to match those of RFB framebuffer */ - guac_rect_init(&context->bounds, 0, 0, rfb_width, rfb_height); - - /* Ensure operation bounds are within possibly updated bounds of the - * pending frame (now the RFB client framebuffer) */ - guac_rect_constrain(&op_bounds, &context->bounds); - - } - - /* All other framebuffer formats must be manually converted */ - else { + /* Ensure operation bounds are within possibly updated bounds of the + * pending frame (now the RFB client framebuffer) */ + guac_rect_constrain(&op_bounds, &context->bounds); - /* Resize the surface if VNC screen size has changed (this call - * automatically deals with invalid dimensions and is a no-op if the size - * has not changed) */ - guac_display_layer_resize(default_layer, rfb_width, rfb_height); + /* NOTE: The guac_display will be pointed directly at the libvncclient + * framebuffer if the pixel format used is identical to that expected by + * guac_display. No need to manually copy anything around in that case. */ - /* Begin drawing operation directly to default layer. NOTE: This is - * intentionally after the call to guac_display_layer_resize() to - * ensure the bounds of the resulting context are representative of the - * resize operation. */ - context = guac_display_layer_open_raw(default_layer); + /* All framebuffer formats must be manually converted if not identical to + * the format used by guac_display */ + if (vnc_bpp != GUAC_DISPLAY_LAYER_RAW_BPP || vnc_client->settings->swap_red_blue) { /* Ensure draw is within current bounds of the pending frame */ guac_rect_constrain(&op_bounds, &context->bounds); @@ -152,9 +129,6 @@ void guac_vnc_update(rfbClient* client, int x, int y, int w, int h) { vnc_client->copy_rect_used = 0; } - /* Draw operation is now complete */ - guac_display_layer_close_raw(default_layer, context); - } void guac_vnc_copyrect(rfbClient* client, int src_x, int src_y, int w, int h, int dest_x, int dest_y) { @@ -393,26 +367,8 @@ rfbBool guac_vnc_malloc_framebuffer(rfbClient* rfb_client) { guac_client* gc = rfbClientGetClientData(rfb_client, GUAC_VNC_CLIENT_KEY); guac_vnc_client* vnc_client = (guac_vnc_client*) gc->data; - /* Resize of underlying buffer must be performed while holding an open raw - * context if the guac_display is active (replacing the underlying - * framebuffer while guac_display may still attempt to flush a pending - * frame is bad news, as that flush may still reference the freed buffer) */ - if (vnc_client->display != NULL) { - - guac_display_layer* default_layer = guac_display_default_layer(vnc_client->display); - guac_display_layer_resize(default_layer, rfb_client->width, rfb_client->height); - - /* Use original, wrapped proc to resize the buffer maintained by libvncclient */ - guac_display_layer_raw_context* context = guac_display_layer_open_raw(default_layer); - rfbBool result = vnc_client->rfb_MallocFrameBuffer(rfb_client); - guac_display_layer_close_raw(default_layer, context); - - return result; - - } - - /* No need to bracket the buffer allocation in a raw context if there's no - * guac_display yet */ + /* Use original, wrapped proc to resize the buffer maintained by + * libvncclient */ return vnc_client->rfb_MallocFrameBuffer(rfb_client); } diff --git a/src/protocols/vnc/vnc.c b/src/protocols/vnc/vnc.c index d47e1510d..cdd21bd63 100644 --- a/src/protocols/vnc/vnc.c +++ b/src/protocols/vnc/vnc.c @@ -269,6 +269,57 @@ static int guac_vnc_wait_for_messages(rfbClient* rfb_client, int msec_timeout) { } +/** + * Handles any inbound VNC messages that have been received, updating the + * Guacamole display accordingly. + * + * @param vnc_client + * The guac_vnc_client of the VNC connection whose current messages should + * be handled. + * + * @return + * True (non-zero) if messages were handled successfully, false (zero) + * otherwise. + */ +static rfbBool guac_vnc_handle_messages(guac_vnc_client* vnc_client) { + + rfbClient* rfb_client = vnc_client->rfb_client; + guac_display_layer* default_layer = guac_display_default_layer(vnc_client->display); + + /* All potential drawing operations must occur while holding an open context */ + guac_display_layer_raw_context* context = guac_display_layer_open_raw(default_layer); + vnc_client->current_context = context; + + /* Actually handle messages (this may result in drawing to the + * guac_display, resizing the display buffer, etc.) */ + rfbBool retval = HandleRFBServerMessage(rfb_client); + + /* Use the buffer of libvncclient directly if it matches the guac_display + * format */ + unsigned int vnc_bpp = rfb_client->format.bitsPerPixel / 8; + if (vnc_bpp == GUAC_DISPLAY_LAYER_RAW_BPP && !vnc_client->settings->swap_red_blue) { + + context->buffer = rfb_client->frameBuffer; + context->stride = guac_mem_ckd_mul_or_die(vnc_bpp, rfb_client->width); + + /* Update bounds of pending frame to match those of RFB framebuffer */ + guac_rect_init(&context->bounds, 0, 0, rfb_client->width, rfb_client->height); + + } + + /* There will be no further drawing operations */ + guac_display_layer_close_raw(default_layer, context); + vnc_client->current_context = NULL; + + /* Resize the surface if VNC screen size has changed (this call + * automatically deals with invalid dimensions and is a no-op + * if the size has not changed) */ + guac_display_layer_resize(default_layer, rfb_client->width, rfb_client->height); + + return retval; + +} + void* guac_vnc_client_thread(void* data) { guac_client* client = (guac_client*) data; @@ -557,7 +608,7 @@ void* guac_vnc_client_thread(void* data) { do { /* Handle any message received */ - if (!HandleRFBServerMessage(rfb_client)) { + if (!guac_vnc_handle_messages(vnc_client)) { guac_client_abort(client, GUAC_PROTOCOL_STATUS_UPSTREAM_ERROR, "Error handling message from VNC server."); diff --git a/src/protocols/vnc/vnc.h b/src/protocols/vnc/vnc.h index fda6bb427..7e5676af8 100644 --- a/src/protocols/vnc/vnc.h +++ b/src/protocols/vnc/vnc.h @@ -107,6 +107,12 @@ typedef struct guac_vnc_client { */ guac_display* display; + /** + * The context of the current drawing (update) operation, if any. If no + * operation is in progress, this will be NULL. + */ + guac_display_layer_raw_context* current_context; + /** * Internal clipboard. */