From 7fc3fe1fdce245c1fcee2b5415a78c74212be81c Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 9 May 2014 15:39:00 -0700 Subject: [PATCH] GUAC-674: Queue and combine png updates. Lengthen RDP frame timeout to 10ms. --- src/common/guac_surface.c | 146 +++++++++++++++++++++++++++++++------ src/common/guac_surface.h | 52 +++++++++++++ src/protocols/rdp/client.h | 2 +- 3 files changed, 177 insertions(+), 23 deletions(-) diff --git a/src/common/guac_surface.c b/src/common/guac_surface.c index 8aaf7edd..09f8223f 100644 --- a/src/common/guac_surface.c +++ b/src/common/guac_surface.c @@ -29,6 +29,7 @@ #include #include +#include #include @@ -237,6 +238,42 @@ static void __guac_common_mark_dirty(guac_common_surface* surface, int x, int y, } +static void __guac_common_surface_flush_to_queue(guac_common_surface* surface) { + + guac_common_surface_png_rect* rect; + + /* Do not flush if not dirty */ + if (!surface->dirty) + return; + + /* Add new rect to queue */ + rect = &(surface->png_queue[surface->png_queue_length++]); + rect->x = surface->dirty_x; + rect->y = surface->dirty_y; + rect->width = surface->dirty_width; + rect->height = surface->dirty_height; + + /* Surface now flushed */ + surface->dirty = 0; + +} + +void guac_common_surface_flush_deferred(guac_common_surface* surface) { + + /* Do not flush if not dirty */ + if (!surface->dirty) + return; + + /* Flush if queue size has reached maximum (space is reserved for the final dirty rect, + * as guac_common_surface_flush() MAY add an additional rect to the queue */ + if (surface->png_queue_length == GUAC_COMMON_SURFACE_QUEUE_SIZE-1) + guac_common_surface_flush(surface); + + /* Append dirty rect to queue */ + __guac_common_surface_flush_to_queue(surface); + +} + static void __guac_common_surface_transfer_int(guac_transfer_function op, uint32_t* src, uint32_t* dst) { switch (op) { @@ -473,6 +510,7 @@ guac_common_surface* guac_common_surface_alloc(guac_socket* socket, const guac_l surface->width = w; surface->height = h; surface->dirty = 0; + surface->png_queue_length = 0; /* Create corresponding Cairo surface */ surface->stride = cairo_format_stride_for_width(CAIRO_FORMAT_RGB24, w); @@ -589,16 +627,16 @@ void guac_common_surface_draw(guac_common_surface* surface, int x, int y, cairo_ if (w <= 0 || h <= 0) return; + /* Update backing surface */ + __guac_common_surface_put(buffer, stride, sx, sy, w, h, surface, x, y, format != CAIRO_FORMAT_ARGB32); + /* Flush if not combining */ if (!__guac_common_should_combine(surface, x, y, w, h, 0)) - guac_common_surface_flush(surface); + guac_common_surface_flush_deferred(surface); /* Always defer draws */ __guac_common_mark_dirty(surface, x, y, w, h); - /* Update backing surface */ - __guac_common_surface_put(buffer, stride, sx, sy, w, h, surface, x, y, format != CAIRO_FORMAT_ARGB32); - } void guac_common_surface_paint(guac_common_surface* surface, int x, int y, cairo_surface_t* src, @@ -617,16 +655,16 @@ void guac_common_surface_paint(guac_common_surface* surface, int x, int y, cairo if (w <= 0 || h <= 0) return; + /* Update backing surface */ + __guac_common_surface_fill_mask(buffer, stride, sx, sy, w, h, surface, x, y, red, green, blue); + /* Flush if not combining */ if (!__guac_common_should_combine(surface, x, y, w, h, 0)) - guac_common_surface_flush(surface); + guac_common_surface_flush_deferred(surface); /* Always defer draws */ __guac_common_mark_dirty(surface, x, y, w, h); - /* Update backing surface */ - __guac_common_surface_fill_mask(buffer, stride, sx, sy, w, h, surface, x, y, red, green, blue); - } void guac_common_surface_copy(guac_common_surface* src, int sx, int sy, int w, int h, @@ -641,6 +679,9 @@ void guac_common_surface_copy(guac_common_surface* src, int sx, int sy, int w, i if (w <= 0 || h <= 0) return; + /* Update backing surface */ + __guac_common_surface_transfer(src, sx, sy, w, h, GUAC_TRANSFER_BINARY_SRC, dst, dx, dy); + /* Defer if combining */ if (__guac_common_should_combine(dst, dx, dy, w, h, 1)) __guac_common_mark_dirty(dst, dx, dy, w, h); @@ -653,9 +694,6 @@ void guac_common_surface_copy(guac_common_surface* src, int sx, int sy, int w, i dst->realized = 1; } - /* Update backing surface */ - __guac_common_surface_transfer(src, sx, sy, w, h, GUAC_TRANSFER_BINARY_SRC, dst, dx, dy); - } void guac_common_surface_transfer(guac_common_surface* src, int sx, int sy, int w, int h, @@ -670,6 +708,9 @@ void guac_common_surface_transfer(guac_common_surface* src, int sx, int sy, int if (w <= 0 || h <= 0) return; + /* Update backing surface */ + __guac_common_surface_transfer(src, sx, sy, w, h, op, dst, dx, dy); + /* Defer if combining */ if (__guac_common_should_combine(dst, dx, dy, w, h, 1)) __guac_common_mark_dirty(dst, dx, dy, w, h); @@ -682,9 +723,6 @@ void guac_common_surface_transfer(guac_common_surface* src, int sx, int sy, int dst->realized = 1; } - /* Update backing surface */ - __guac_common_surface_transfer(src, sx, sy, w, h, op, dst, dx, dy); - } void guac_common_surface_rect(guac_common_surface* surface, @@ -699,6 +737,9 @@ void guac_common_surface_rect(guac_common_surface* surface, if (w <= 0 || h <= 0) return; + /* Update backing surface */ + __guac_common_surface_rect(surface, x, y, w, h, red, green, blue); + /* Defer if combining */ if (__guac_common_should_combine(surface, x, y, w, h, 1)) __guac_common_mark_dirty(surface, x, y, w, h); @@ -711,9 +752,6 @@ void guac_common_surface_rect(guac_common_surface* surface, surface->realized = 1; } - /* Update backing surface */ - __guac_common_surface_rect(surface, x, y, w, h, red, green, blue); - } void guac_common_surface_clip(guac_common_surface* surface, int x, int y, int w, int h) { @@ -753,26 +791,90 @@ void guac_common_surface_reset_clip(guac_common_surface* surface) { surface->bounds_height = surface->height; } -void guac_common_surface_flush(guac_common_surface* surface) { +void __guac_common_surface_flush_to_png(guac_common_surface* surface) { if (surface->dirty) { guac_socket* socket = surface->socket; const guac_layer* layer = surface->layer; - unsigned char* buffer = surface->buffer + surface->dirty_y * surface->stride + surface->dirty_x * 4; + /* Get Cairo surface for specified rect */ + unsigned char* buffer = surface->buffer + surface->dirty_y * surface->stride + surface->dirty_x * 4; cairo_surface_t* rect = cairo_image_surface_create_for_data(buffer, CAIRO_FORMAT_RGB24, surface->dirty_width, surface->dirty_height, surface->stride); - /* Send PNG for dirty rect */ + /* Send PNG for rect */ guac_protocol_send_png(socket, GUAC_COMP_OVER, layer, surface->dirty_x, surface->dirty_y, rect); cairo_surface_destroy(rect); - - surface->dirty = 0; surface->realized = 1; + /* Surface is no longer dirty */ + surface->dirty = 0; + } } +static int __guac_common_surface_png_rect_compare(const void* a, const void* b) { + + guac_common_surface_png_rect* ra = (guac_common_surface_png_rect*) a; + guac_common_surface_png_rect* rb = (guac_common_surface_png_rect*) b; + + /* Order roughly top to bottom, left to right */ + if (ra->y >> 6 != rb->y >> 6) return ra->y - rb->y; + if (ra->x != rb->x) return ra->x - rb->x; + + /* Wider updates should come first (more likely to intersect later) */ + if (ra->width != rb->width) return rb->width - ra->width; + + /* Shorter updates should come first (less likely to increase cost) */ + return ra->height - rb->height; + +} + +void guac_common_surface_flush(guac_common_surface* surface) { + + guac_common_surface_png_rect* current = surface->png_queue; + + int i; + int flushed = 0; + + /* Flush final dirty rect to queue */ + __guac_common_surface_flush_to_queue(surface); + + /* Sort updates to make combination less costly */ + qsort(surface->png_queue, surface->png_queue_length, sizeof(guac_common_surface_png_rect), + __guac_common_surface_png_rect_compare); + + /* Flush all rects in queue */ + for (i=0; i < surface->png_queue_length; i++) { + + int x = current->x; + int y = current->y; + int w = current->width; + int h = current->height; + + /* Flush if not combining */ + if (!__guac_common_should_combine(surface, x, y, w, h, 0)) { + if (surface->dirty) flushed++; + __guac_common_surface_flush_to_png(surface); + } + + /* Defer, attempt to combine with next rect */ + __guac_common_mark_dirty(surface, x, y, w, h); + current++; + + } + + /* Flush the final png */ + if (surface->dirty) flushed++; + __guac_common_surface_flush_to_png(surface); + + /* Flush complete */ + if (flushed < surface->png_queue_length) + fprintf(stderr, "IMPROVEMENT: Flushed %i (originally %i) updates.\n", flushed, surface->png_queue_length); + surface->png_queue_length = 0; + +} + diff --git a/src/common/guac_surface.h b/src/common/guac_surface.h index bfde637d..28e7485e 100644 --- a/src/common/guac_surface.h +++ b/src/common/guac_surface.h @@ -32,6 +32,38 @@ #include +/** + * The maximum number of updates to allow within the PNG queue. + */ +#define GUAC_COMMON_SURFACE_QUEUE_SIZE 256 + +/** + * Simple representation of a rectangle, having a defined corner and dimensions. + */ +typedef struct guac_common_surface_png_rect { + + /** + * The X coordinate of the upper-left corner of this rectangle. + */ + int x; + + /** + * The Y coordinate of the upper-left corner of this rectangle. + */ + int y; + + /** + * The width of this rectangle. + */ + int width; + + /** + * The height of this rectangle. + */ + int height; + +} guac_common_surface_png_rect; + /** * Surface which backs a Guacamole buffer or layer, automatically * combining updates when possible. @@ -122,6 +154,16 @@ typedef struct guac_common_surface { */ int bounds_height; + /** + * The number of updates in the PNG queue. + */ + int png_queue_length; + + /** + * All queued PNG updates. + */ + guac_common_surface_png_rect png_queue[GUAC_COMMON_SURFACE_QUEUE_SIZE]; + } guac_common_surface; /** @@ -253,5 +295,15 @@ void guac_common_surface_reset_clip(guac_common_surface* surface); */ void guac_common_surface_flush(guac_common_surface* surface); +/** + * Schedules a deferred flush of the given surface. This will not immediately + * flush the surface to the client. Instead, the result of the flush is + * added to a queue which is reinspected and combined (if possible) with other + * deferred flushes during the call to guac_common_surface_flush(). + * + * @param surface The surface to flush. + */ +void guac_common_surface_flush_deferred(guac_common_surface* surface); + #endif diff --git a/src/protocols/rdp/client.h b/src/protocols/rdp/client.h index 726462cd..b25ebd4c 100644 --- a/src/protocols/rdp/client.h +++ b/src/protocols/rdp/client.h @@ -52,7 +52,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 10 /** * The native resolution of most RDP connections. As Windows and other systems