From 7a55ca08b4eed798ecfec66556dda36b68e4c50e Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 8 Sep 2024 18:10:05 -0700 Subject: [PATCH] GUACAMOLE-377: Restore heuristic detection of RDP frame boundaries. --- src/protocols/rdp/client.h | 2 +- src/protocols/rdp/gdi.c | 34 +--------------------------------- src/protocols/rdp/gdi.h | 18 ++---------------- src/protocols/rdp/pointer.c | 36 +++--------------------------------- src/protocols/rdp/rdp.c | 37 ++----------------------------------- src/protocols/rdp/rdp.h | 5 ----- 6 files changed, 9 insertions(+), 123 deletions(-) diff --git a/src/protocols/rdp/client.h b/src/protocols/rdp/client.h index 943d39abb..95b10c8b1 100644 --- a/src/protocols/rdp/client.h +++ b/src/protocols/rdp/client.h @@ -32,7 +32,7 @@ * milliseconds. If the server is silent for at least this amount of time, the * frame will be considered finished. */ -#define GUAC_RDP_FRAME_TIMEOUT 0 +#define GUAC_RDP_FRAME_TIMEOUT 16 /** * The amount of time to wait for a new message from the RDP server when diff --git a/src/protocols/rdp/gdi.c b/src/protocols/rdp/gdi.c index 3b1a973eb..d96f76902 100644 --- a/src/protocols/rdp/gdi.c +++ b/src/protocols/rdp/gdi.c @@ -38,30 +38,16 @@ void guac_rdp_gdi_mark_frame(rdpContext* context, int starting) { guac_client* client = ((rdp_freerdp_context*) context)->client; guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; - /* The server supports defining explicit frames */ - rdp_client->frames_supported = 1; - /* A new frame is beginning */ if (starting) { rdp_client->in_frame = 1; return; } - /* The current frame has ended */ - guac_timestamp frame_end = guac_timestamp_current(); - int time_elapsed = frame_end - client->last_sent_timestamp; - rdp_client->in_frame = 0; - /* A new frame has been received from the RDP server and processed */ + rdp_client->in_frame = 0; rdp_client->frames_received++; - /* Flush a new frame if the client is ready for it */ - if (time_elapsed >= guac_client_get_processing_lag(client)) { - guac_display_end_multiple_frames(rdp_client->display, rdp_client->frames_received); - guac_socket_flush(client->socket); - rdp_client->frames_received = 0; - } - } BOOL guac_rdp_gdi_frame_marker(rdpContext* context, const FRAME_MARKER_ORDER* frame_marker) { @@ -88,19 +74,6 @@ 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; - - /* Leverage BeginPaint handler to detect start of frame for RDPGFX channel */ - if (rdp_client->settings->enable_gfx && rdp_client->frames_supported) - guac_rdp_gdi_mark_frame(context, 1); - - return TRUE; - -} - BOOL guac_rdp_gdi_end_paint(rdpContext* context) { guac_client* client = ((rdp_freerdp_context*) context)->client; @@ -135,11 +108,6 @@ BOOL guac_rdp_gdi_end_paint(rdpContext* context) { guac_display_layer_close_raw(default_layer, dst_context); - /* Next frame */ - if (gdi->inGfxFrame) { - guac_rdp_gdi_mark_frame(context, 0); - } - return TRUE; } diff --git a/src/protocols/rdp/gdi.h b/src/protocols/rdp/gdi.h index 1f63a8fa2..24593db09 100644 --- a/src/protocols/rdp/gdi.h +++ b/src/protocols/rdp/gdi.h @@ -72,22 +72,8 @@ 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 a paint operation is complete. This function is - * expected to be called by the FreeRDP GDI implementation of RemoteFX when a - * new frame has been completed. + * Handler called when FreeRDP has finished performing updates to the backing + * surface of its GDI (graphics) implementation. * * @param context * The rdpContext associated with the current RDP session. diff --git a/src/protocols/rdp/pointer.c b/src/protocols/rdp/pointer.c index 117ff56b3..5d77af786 100644 --- a/src/protocols/rdp/pointer.c +++ b/src/protocols/rdp/pointer.c @@ -75,14 +75,6 @@ BOOL guac_rdp_pointer_set(rdpContext* context, POINTER_SET_CONST rdpPointer* poi guac_client* client = ((rdp_freerdp_context*) context)->client; guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; - /* Add explicit frame boundaries around cursor set operation if not already - * in a frame (the RDP protocol does not send nor expect frame boundaries - * for cursor changes, but Guacamole does expect this) */ - int in_frame = rdp_client->in_frame; - - if (rdp_client->frames_supported && !in_frame) - guac_rdp_gdi_mark_frame(context, 1); - guac_display_layer* src_layer = ((guac_rdp_pointer*) pointer)->layer; guac_display_layer_raw_context* src_context = guac_display_layer_open_raw(src_layer); @@ -111,9 +103,7 @@ BOOL guac_rdp_pointer_set(rdpContext* context, POINTER_SET_CONST rdpPointer* poi guac_display_layer_close_raw(cursor_layer, dst_context); guac_display_layer_close_raw(src_layer, src_context); - if (rdp_client->frames_supported && !in_frame) - guac_rdp_gdi_mark_frame(context, 0); - + guac_display_end_mouse_frame(rdp_client->display); return TRUE; } @@ -135,20 +125,10 @@ BOOL guac_rdp_pointer_set_null(rdpContext* context) { guac_client* client = ((rdp_freerdp_context*) context)->client; guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; - /* Add explicit frame boundaries around cursor set operation if not already - * in a frame (the RDP protocol does not send nor expect frame boundaries - * for cursor changes, but Guacamole does expect this) */ - int in_frame = rdp_client->in_frame; - - if (rdp_client->frames_supported && !in_frame) - guac_rdp_gdi_mark_frame(context, 1); - /* Set cursor to empty/blank graphic */ guac_display_set_cursor(rdp_client->display, GUAC_DISPLAY_CURSOR_NONE); - if (rdp_client->frames_supported && !in_frame) - guac_rdp_gdi_mark_frame(context, 0); - + guac_display_end_mouse_frame(rdp_client->display); return TRUE; } @@ -158,19 +138,9 @@ BOOL guac_rdp_pointer_set_default(rdpContext* context) { guac_client* client = ((rdp_freerdp_context*) context)->client; guac_rdp_client* rdp_client = (guac_rdp_client*) client->data; - /* Add explicit frame boundaries around cursor set operation if not already - * in a frame (the RDP protocol does not send nor expect frame boundaries - * for cursor changes, but Guacamole does expect this) */ - int in_frame = rdp_client->in_frame; - - if (rdp_client->frames_supported && !in_frame) - guac_rdp_gdi_mark_frame(context, 1); - /* Set cursor to embedded pointer */ guac_display_set_cursor(rdp_client->display, GUAC_DISPLAY_CURSOR_POINTER); - if (rdp_client->frames_supported && !in_frame) - guac_rdp_gdi_mark_frame(context, 0); - + guac_display_end_mouse_frame(rdp_client->display); return TRUE; } diff --git a/src/protocols/rdp/rdp.c b/src/protocols/rdp/rdp.c index 385c32442..044b78910 100644 --- a/src/protocols/rdp/rdp.c +++ b/src/protocols/rdp/rdp.c @@ -159,7 +159,6 @@ 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; @@ -528,14 +527,9 @@ static int guac_rdp_handle_connection(guac_client* client) { GUAC_RDP_FRAME_START_TIMEOUT); if (wait_result > 0) { - int processing_lag = guac_client_get_processing_lag(client); - /* Read server messages until frame is built */ do { - guac_timestamp frame_end; - int frame_remaining; - /* Handle any queued FreeRDP events (this may result in RDP * messages being sent) */ pthread_mutex_lock(&(rdp_client->message_lock)); @@ -548,34 +542,7 @@ static int guac_rdp_handle_connection(guac_client* client) { break; } - /* Continue handling inbound data if we are in the middle of an RDP frame */ - if (rdp_client->in_frame) { - wait_result = rdp_guac_client_wait_for_messages(client, GUAC_RDP_FRAME_START_TIMEOUT); - if (wait_result >= 0) - continue; - } - - /* Calculate time remaining in frame */ - guac_timestamp frame_start = client->last_sent_timestamp; - frame_end = guac_timestamp_current(); - frame_remaining = frame_start + GUAC_RDP_FRAME_DURATION - - frame_end; - - /* Calculate time that client needs to catch up */ - int time_elapsed = frame_end - frame_start; - int required_wait = processing_lag - time_elapsed; - - /* Increase the duration of this frame if client is lagging */ - if (required_wait > GUAC_RDP_FRAME_TIMEOUT) - wait_result = rdp_guac_client_wait_for_messages(client, - required_wait); - - /* Wait again if frame remaining */ - else if (frame_remaining > 0) - wait_result = rdp_guac_client_wait_for_messages(client, - GUAC_RDP_FRAME_TIMEOUT); - else - break; + wait_result = rdp_guac_client_wait_for_messages(client, GUAC_RDP_FRAME_TIMEOUT); } while (wait_result > 0); @@ -600,7 +567,7 @@ static int guac_rdp_handle_connection(guac_client* client) { /* Flush frame only if successful and an RDP frame is not known to be * in progress */ - else if (!rdp_client->frames_supported || rdp_client->frames_received) { + else if (!rdp_client->in_frame) { guac_display_end_multiple_frames(rdp_client->display, rdp_client->frames_received); guac_socket_flush(client->socket); rdp_client->frames_received = 0; diff --git a/src/protocols/rdp/rdp.h b/src/protocols/rdp/rdp.h index 517e9e471..d6f88bea1 100644 --- a/src/protocols/rdp/rdp.h +++ b/src/protocols/rdp/rdp.h @@ -106,11 +106,6 @@ typedef struct guac_rdp_client { */ guac_display_layer* current_surface; - /** - * Whether the RDP server supports defining explicit frame boundaries. - */ - int frames_supported; - /** * Whether the RDP server has reported that a new frame is in progress, and * we are now receiving updates relevant to that frame.