From ad526434a471dc1180de514d52c53b855538ac8b Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Wed, 11 Sep 2024 11:54:41 -0700 Subject: [PATCH] GUACAMOLE-377: Migrate RDP to direct access of FreeRDP's GDI buffer. --- src/protocols/rdp/gdi.c | 52 ++++++++++++++++++++++++++++++----------- src/protocols/rdp/gdi.h | 13 +++++++++++ src/protocols/rdp/rdp.c | 17 ++++++++++---- src/protocols/rdp/rdp.h | 7 ++++++ 4 files changed, 71 insertions(+), 18 deletions(-) diff --git a/src/protocols/rdp/gdi.c b/src/protocols/rdp/gdi.c index d96f76902..95319af1b 100644 --- a/src/protocols/rdp/gdi.c +++ b/src/protocols/rdp/gdi.c @@ -18,7 +18,6 @@ */ #include "color.h" -#include "guacamole/display.h" #include "rdp.h" #include "settings.h" @@ -27,7 +26,9 @@ #include #include #include +#include #include +#include #include #include @@ -74,40 +75,63 @@ BOOL guac_rdp_gdi_surface_frame_marker(rdpContext* context, const SURFACE_FRAME_ } +BOOL guac_rdp_gdi_begin_paint(rdpContext* context) { + + guac_client* client = ((rdp_freerdp_context*) context)->client; + 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; + + return TRUE; + +} + BOOL guac_rdp_gdi_end_paint(rdpContext* context) { guac_client* client = ((rdp_freerdp_context*) context)->client; 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); + /* Ignore paint if GDI output is suppressed */ if (gdi->suppressOutput) - return TRUE; + goto paint_complete; /* Ignore paint if nothing has been done (empty rect) */ if (gdi->primary->hdc->hwnd->invalid->null) - return TRUE; + goto paint_complete; INT32 x = gdi->primary->hdc->hwnd->invalid->x; INT32 y = gdi->primary->hdc->hwnd->invalid->y; UINT32 w = gdi->primary->hdc->hwnd->invalid->w; UINT32 h = gdi->primary->hdc->hwnd->invalid->h; - guac_display_layer* default_layer = guac_display_default_layer(rdp_client->display); - guac_display_layer_raw_context* dst_context = guac_display_layer_open_raw(default_layer); - guac_rect dst_rect; guac_rect_init(&dst_rect, x, y, w, h); - guac_rect_constrain(&dst_rect, &dst_context->bounds); - - guac_display_layer_raw_context_put(dst_context, &dst_rect, - GUAC_RECT_CONST_BUFFER(dst_rect, gdi->primary_buffer, gdi->stride, 4), - gdi->stride); - - guac_rect_extend(&dst_context->dirty, &dst_rect); + guac_rect_constrain(&dst_rect, ¤t_paint->bounds); - guac_display_layer_close_raw(default_layer, dst_context); + guac_rect_extend(¤t_paint->dirty, &dst_rect); +paint_complete: + guac_display_layer_close_raw(default_layer, current_paint); return TRUE; } diff --git a/src/protocols/rdp/gdi.h b/src/protocols/rdp/gdi.h index 24593db09..c6cfeb62a 100644 --- a/src/protocols/rdp/gdi.h +++ b/src/protocols/rdp/gdi.h @@ -71,6 +71,19 @@ BOOL guac_rdp_gdi_frame_marker(rdpContext* context, const FRAME_MARKER_ORDER* fr */ BOOL guac_rdp_gdi_surface_frame_marker(rdpContext* context, const SURFACE_FRAME_MARKER* surface_frame_marker); +/** + * Handler called when a paint operation is beginning. This function is + * expected to be called by the FreeRDP GDI implementation of RemoteFX when a + * new frame has started. + * + * @param context + * The rdpContext associated with the current RDP session. + * + * @return + * TRUE if successful, FALSE otherwise. + */ +BOOL guac_rdp_gdi_begin_paint(rdpContext* context); + /** * Handler called when FreeRDP has finished performing updates to the backing * surface of its GDI (graphics) implementation. diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c index 7bb2359b8..535ce22c3 100644 --- a/src/protocols/rdp/rdp.c +++ b/src/protocols/rdp/rdp.c @@ -214,6 +214,7 @@ static BOOL rdp_freerdp_pre_connect(freerdp* instance) { /* Set up GDI */ GUAC_RDP_CONTEXT(instance)->update->DesktopResize = guac_rdp_gdi_desktop_resize; + GUAC_RDP_CONTEXT(instance)->update->BeginPaint = guac_rdp_gdi_begin_paint; GUAC_RDP_CONTEXT(instance)->update->EndPaint = guac_rdp_gdi_end_paint; GUAC_RDP_CONTEXT(instance)->update->SurfaceFrameMarker = guac_rdp_gdi_surface_frame_marker; @@ -645,6 +646,18 @@ 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; + /* Clean up FreeRDP internal GDI implementation */ gdi_free(rdp_inst); @@ -663,10 +676,6 @@ static int guac_rdp_handle_connection(guac_client* client) { guac_rdp_keyboard_free(rdp_client->keyboard); rdp_client->keyboard = NULL; - /* Free display */ - guac_display_free(rdp_client->display); - rdp_client->display = NULL; - guac_rwlock_release_lock(&(rdp_client->lock)); /* Client is now disconnected */ diff --git a/src/protocols/rdp/rdp.h b/src/protocols/rdp/rdp.h index d6f88bea1..96d66284c 100644 --- a/src/protocols/rdp/rdp.h +++ b/src/protocols/rdp/rdp.h @@ -106,6 +106,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. + */ + guac_display_layer_raw_context* current_paint; + /** * Whether the RDP server has reported that a new frame is in progress, and * we are now receiving updates relevant to that frame.