diff --git a/src/libguac/display-plan.c b/src/libguac/display-plan.c index e8091250d..07f0d9c17 100644 --- a/src/libguac/display-plan.c +++ b/src/libguac/display-plan.c @@ -20,9 +20,12 @@ #include "display-plan.h" #include "display-priv.h" #include "guacamole/assert.h" +#include "guacamole/client.h" #include "guacamole/display.h" #include "guacamole/fifo.h" #include "guacamole/mem.h" +#include "guacamole/protocol.h" +#include "guacamole/socket.h" #include "guacamole/timestamp.h" #include @@ -365,10 +368,59 @@ void guac_display_plan_free(guac_display_plan* plan) { void guac_display_plan_apply(guac_display_plan* plan) { guac_display* display = plan->display; + guac_client* client = display->client; guac_display_plan_operation* op = plan->ops; + /* Immediately send instructions for all updates that do not involve + * significant processing (do not involve encoding anything). This allows + * us to use the worker threads solely for encoding, reducing contention + * between the threads. */ for (int i = 0; i < plan->length; i++) { - guac_fifo_enqueue(&display->ops, op++); + + guac_display_layer* display_layer = op->layer; + switch (op->type) { + + case GUAC_DISPLAY_PLAN_OPERATION_COPY: + guac_protocol_send_copy(client->socket, op->src.layer_rect.layer, + op->src.layer_rect.rect.left, op->src.layer_rect.rect.top, + guac_rect_width(&op->src.layer_rect.rect), guac_rect_height(&op->src.layer_rect.rect), + GUAC_COMP_OVER, display_layer->layer, op->dest.left, op->dest.top); + break; + + case GUAC_DISPLAY_PLAN_OPERATION_RECT: + + guac_protocol_send_rect(client->socket, display_layer->layer, + op->dest.left, op->dest.top, guac_rect_width(&op->dest), guac_rect_height(&op->dest)); + + int alpha = (op->src.color & 0xFF000000) >> 24; + int red = (op->src.color & 0x00FF0000) >> 16; + int green = (op->src.color & 0x0000FF00) >> 8; + int blue = (op->src.color & 0x000000FF); + + /* Clear before drawing if layer is not opaque (transparency + * will not be copied correctly otherwise) */ + if (!display_layer->opaque) + guac_protocol_send_cfill(client->socket, GUAC_COMP_ROUT, display_layer->layer, + 0x00, 0x00, 0x00, 0xFF); + + guac_protocol_send_cfill(client->socket, GUAC_COMP_OVER, display_layer->layer, + red, green, blue, alpha); + + break; + + /* Simply ignore and drop NOP */ + case GUAC_DISPLAY_PLAN_OPERATION_NOP: + break; + + /* All other operations should be handled by the workers */ + default: + guac_fifo_enqueue(&display->ops, op); + break; + + } + + op++; + } } diff --git a/src/libguac/display-priv.h b/src/libguac/display-priv.h index ca9298f31..673bc456a 100644 --- a/src/libguac/display-priv.h +++ b/src/libguac/display-priv.h @@ -92,7 +92,6 @@ * 2) last_frame.lock * 3) ops * 4) render_state - * 5) op_path_lock * * Acquiring these locks in any other order risks deadlock. Don't do it. */ @@ -667,15 +666,6 @@ struct guac_display { */ guac_fifo ops; - /** - * Lock which ensures instructions that make use of a layer's current path - * (such as "rect" and "cfill") do not get inadvertently interleaved. - * Interleaving of path instructions can result in those paths not matching - * the expectatiosn of subsequent instructions, causing graphical - * artifacts. - */ - pthread_mutex_t op_path_lock; - /** * Storage for any items within the ops fifo. */ diff --git a/src/libguac/display-worker.c b/src/libguac/display-worker.c index e39513bbc..39649f3f7 100644 --- a/src/libguac/display-worker.c +++ b/src/libguac/display-worker.c @@ -104,16 +104,12 @@ static void guac_display_layer_clear_non_opaque(guac_display_layer* display_laye * being non-opaque */ if (!display_layer->opaque) { - pthread_mutex_lock(&display->op_path_lock); - guac_protocol_send_rect(socket, layer, dirty->left, dirty->top, guac_rect_width(dirty), guac_rect_height(dirty)); guac_protocol_send_cfill(socket, GUAC_COMP_ROUT, layer, 0x00, 0x00, 0x00, 0xFF); - pthread_mutex_unlock(&display->op_path_lock); - } } @@ -371,36 +367,14 @@ void* guac_display_worker_thread(void* data) { break; case GUAC_DISPLAY_PLAN_OPERATION_COPY: - guac_protocol_send_copy(client->socket, op.src.layer_rect.layer, - op.src.layer_rect.rect.left, op.src.layer_rect.rect.top, - guac_rect_width(&op.src.layer_rect.rect), guac_rect_height(&op.src.layer_rect.rect), - GUAC_COMP_OVER, display_layer->layer, op.dest.left, op.dest.top); - break; - case GUAC_DISPLAY_PLAN_OPERATION_RECT: - - pthread_mutex_lock(&display->op_path_lock); - guac_protocol_send_rect(client->socket, display_layer->layer, - op.dest.left, op.dest.top, guac_rect_width(&op.dest), guac_rect_height(&op.dest)); - - int alpha = (op.src.color & 0xFF000000) >> 24; - int red = (op.src.color & 0x00FF0000) >> 16; - int green = (op.src.color & 0x0000FF00) >> 8; - int blue = (op.src.color & 0x000000FF); - - /* Clear before drawing if layer is not opaque (transparency - * will not be copied correctly otherwise) */ - if (!display_layer->opaque) - guac_protocol_send_cfill(client->socket, GUAC_COMP_ROUT, display_layer->layer, - 0x00, 0x00, 0x00, 0xFF); - - guac_protocol_send_cfill(client->socket, GUAC_COMP_OVER, display_layer->layer, - red, green, blue, alpha); - - pthread_mutex_unlock(&display->op_path_lock); - break; - case GUAC_DISPLAY_PLAN_OPERATION_NOP: + guac_client_log(client, GUAC_LOG_DEBUG, "Operation type %i " + "should NOT be present in the set of operations given " + "to guac_display worker thread. All operations except " + "IMG and END_FRAME are handled during the initial, " + "single-threaded flush step. This is likely a bug.", + op.type); break; case GUAC_DISPLAY_PLAN_END_FRAME: diff --git a/src/libguac/display.c b/src/libguac/display.c index 731a44632..5cd739d65 100644 --- a/src/libguac/display.c +++ b/src/libguac/display.c @@ -137,9 +137,6 @@ guac_display* guac_display_alloc(guac_client* client) { guac_flag_init(&display->render_state); guac_flag_set(&display->render_state, GUAC_DISPLAY_RENDER_STATE_FRAME_NOT_IN_PROGRESS); - /* Init lock specific to the GUAC_DISPLAY_PLAN_OPERATION_RECT operation */ - pthread_mutex_init(&display->op_path_lock, NULL); - int cpu_count = guac_display_nproc(); if (cpu_count <= 0) { guac_client_log(client, GUAC_LOG_WARNING, "Number of available " @@ -176,7 +173,6 @@ void guac_display_free(guac_display* display) { pthread_join(display->worker_threads[i], NULL); /* All locks, FIFOs, etc. are now unused and can be safely destroyed */ - pthread_mutex_destroy(&display->op_path_lock); guac_flag_destroy(&display->render_state); guac_fifo_destroy(&display->ops); guac_rwlock_destroy(&display->last_frame.lock);