GUACAMOLE-377: Migrate RDP to default render thread.

src/protocols/rdp/client.h

@@ -23,29 +23,14 @@
 #include <guacamole/client.h>
 
 /**
- * The maximum duration of a frame in milliseconds. This ensures we at least
- * meet a reasonable minimum framerate in the case that the RDP server provides
- * no frame boundaries and streams data continuously enough that frame
- * boundaries are not discernable through timing.
+ * The amount of time to wait for new messages from the RDP server before
+ * moving on to internal matters, in milliseconds. This value must be kept
+ * reasonably small such that a slow RDP server will not prevent external
+ * events from being handled (such as the stop signal from guac_client_stop()),
+ * but large enough that the message handling loop does not eat up CPU
+ * spinning.
  */
-#define GUAC_RDP_MAX_FRAME_DURATION 33
-
-/**
- * The minimum duration of a frame in milliseconds. This ensures we don't start
- * flushing a ton of tiny frames if an RDP server provides no frame boundaries
- * and streams data inconsistently enough that timing would suggest frame
- * boundaries in the middle of a frame.
- */
-#define GUAC_RDP_MIN_FRAME_DURATION 10
-
-/**
- * The amount of time to wait for a new message from the RDP server when
- * beginning a new frame, in milliseconds. This value must be kept reasonably
- * small such that a slow RDP server will not prevent external events from
- * being handled (such as the stop signal from guac_client_stop()), but large
- * enough that the message handling loop does not eat up CPU spinning.
- */
-#define GUAC_RDP_FRAME_START_TIMEOUT 250
+#define GUAC_RDP_MESSAGE_CHECK_INTERVAL 1000
 
 /**
  * The native resolution of most RDP connections. As Windows and other systems

src/protocols/rdp/gdi.c

@@ -39,15 +39,9 @@ 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;
 
-    /* A new frame is beginning */
-    if (starting) {
-        rdp_client->in_frame = 1;
-        return;
-    }
-
     /* A new frame has been received from the RDP server and processed */
-    rdp_client->in_frame = 0;
-    rdp_client->frames_received++;
+    if (!starting)
+        guac_display_render_thread_notify_frame(rdp_client->render_thread);
 
 }
 
@@ -133,6 +127,8 @@ BOOL guac_rdp_gdi_end_paint(rdpContext* context) {
     guac_rect_constrain(&dst_rect, &current_context->bounds);
     guac_rect_extend(&current_context->dirty, &dst_rect);
 
+    guac_display_render_thread_notify_modified(rdp_client->render_thread);
+
 paint_complete:
 
     /* There will be no further drawing operations */

src/protocols/rdp/pointer.c

@@ -103,7 +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);
 
-    guac_display_end_mouse_frame(rdp_client->display);
+    guac_display_render_thread_notify_modified(rdp_client->render_thread);
     return TRUE;
 
 }
@@ -128,7 +128,7 @@ BOOL guac_rdp_pointer_set_null(rdpContext* context) {
     /* Set cursor to empty/blank graphic */
     guac_display_set_cursor(rdp_client->display, GUAC_DISPLAY_CURSOR_NONE);
 
-    guac_display_end_mouse_frame(rdp_client->display);
+    guac_display_render_thread_notify_modified(rdp_client->render_thread);
     return TRUE;
 
 }
@@ -141,6 +141,6 @@ BOOL guac_rdp_pointer_set_default(rdpContext* context) {
     /* Set cursor to embedded pointer */
     guac_display_set_cursor(rdp_client->display, GUAC_DISPLAY_CURSOR_POINTER);
 
-    guac_display_end_mouse_frame(rdp_client->display);
+    guac_display_render_thread_notify_modified(rdp_client->render_thread);
     return TRUE;
 }

src/protocols/rdp/rdp.c

@@ -597,6 +597,8 @@ static int guac_rdp_handle_connection(guac_client* client) {
 
     guac_rwlock_release_lock(&(rdp_client->lock));
 
+    rdp_client->render_thread = guac_display_render_thread_create(rdp_client->display);
+
     /* Handle messages from RDP server while client is running */
     while (client->state == GUAC_CLIENT_RUNNING
             && !guac_rdp_disp_reconnect_needed(rdp_client->disp)) {
@@ -605,48 +607,16 @@ static int guac_rdp_handle_connection(guac_client* client) {
         guac_rdp_disp_update_size(rdp_client->disp, settings, rdp_inst);
 
         /* Wait for data and construct a reasonable frame */
-        int wait_result = rdp_guac_client_wait_for_messages(client, GUAC_RDP_FRAME_START_TIMEOUT);
-        if (wait_result > 0) {
-
-            /* Read server messages until frame is built */
-            guac_timestamp frame_start = guac_timestamp_current();
-            int frames_at_start = rdp_client->frames_received;
-            do {
-
-                /* Handle any queued FreeRDP events (this may result in RDP
-                 * messages being sent), aborting if FreeRDP event handling
-                 * fails */
-                if (!guac_rdp_handle_events(rdp_client)) {
-                    wait_result = -1;
-                    break;
-                }
-
-                int frame_duration = guac_timestamp_current() - frame_start;
-
-                if (!rdp_client->in_frame) {
-
-                    /* Flush frame if at least one frame has been produced */
-                    if (rdp_client->frames_received > frames_at_start)
-                        break;
-
-                    /* Continue processing messages for up to a reasonable
-                     * minimum framerate without an explicit frame boundary
-                     * indicating that the frame is not yet complete */
-                    if (frame_duration > GUAC_RDP_MAX_FRAME_DURATION)
-                        break;
-
-                }
 
-                /* Do not exceed a reasonable maximum framerate without an
-                 * explicit frame boundary terminating the frame early */
-                int allowed_wait = GUAC_RDP_MIN_FRAME_DURATION - frame_duration;
-                if (allowed_wait < 0)
-                    allowed_wait = 0;
-
-                wait_result = rdp_guac_client_wait_for_messages(client, allowed_wait);
-
-            } while (wait_result > 0);
+        int wait_result = rdp_guac_client_wait_for_messages(client, GUAC_RDP_MESSAGE_CHECK_INTERVAL);
+        if (wait_result < 0)
+            break;
 
+        /* Handle any queued FreeRDP events (this may result in RDP messages
+         * being sent), aborting if FreeRDP event handling fails */
+        if (!guac_rdp_handle_events(rdp_client)) {
+            wait_result = -1;
+            break;
         }
 
         /* Test whether the RDP server is closing the connection */
@@ -666,13 +636,6 @@ static int guac_rdp_handle_connection(guac_client* client) {
             guac_client_abort(client, GUAC_PROTOCOL_STATUS_UPSTREAM_UNAVAILABLE,
                     "Connection closed.");
 
-        /* Flush frame only if successful and an RDP frame is not known to be
-         * in progress */
-        else if (!rdp_client->in_frame) {
-            guac_display_end_multiple_frames(rdp_client->display, rdp_client->frames_received);
-            rdp_client->frames_received = 0;
-        }
-
     }
 
     guac_rwlock_acquire_write_lock(&(rdp_client->lock));
@@ -688,6 +651,10 @@ static int guac_rdp_handle_connection(guac_client* client) {
     freerdp_disconnect(rdp_inst);
     pthread_mutex_unlock(&(rdp_client->message_lock));
 
+    /* Stop render loop */
+    guac_display_render_thread_destroy(rdp_client->render_thread);
+    rdp_client->render_thread = NULL;
+
     /* Remove reference to FreeRDP's GDI buffer so that it can be safely freed
      * prior to freeing the guac_display */
     guac_display_layer_raw_context* context = guac_display_layer_open_raw(default_layer);

src/protocols/rdp/rdp.h

@@ -115,17 +115,10 @@ typedef struct guac_rdp_client {
     guac_display_layer_raw_context* current_context;
 
     /**
-     * Whether the RDP server has reported that a new frame is in progress, and
-     * we are now receiving updates relevant to that frame.
+     * The current instance of the guac_display render thread. If the thread
+     * has not yet been started, this will be NULL.
      */
-    int in_frame;
-
-    /**
-     * The number of distinct frames received from the RDP server since last
-     * flush, if the RDP server supports reporting frame boundaries. If the RDP
-     * server does not support tracking frames, this will be zero.
-     */
-    int frames_received;
+    guac_display_render_thread* render_thread;
 
     /**
      * The current state of the keyboard with respect to the RDP session.

src/protocols/rdp/settings.c

@@ -1774,6 +1774,10 @@ void guac_rdp_push_settings(guac_client* client,
     rdp_settings->FrameMarkerCommandEnabled = TRUE;
     rdp_settings->SurfaceFrameMarkerEnabled = TRUE;
 
+    /* Always handle input events asynchronously (rather than synchronously
+     * with the rest of FreeRDP's event loop, including graphics) */
+    rdp_settings->AsyncInput = TRUE;
+
     /* Enable RemoteFX / Graphics Pipeline */
     if (guac_settings->enable_gfx) {