GUACAMOLE-377: Allow the pending frame buffer for a guac_display_laye…

…r to be external.

src/libguac/display-flush.c

@@ -115,8 +115,39 @@ static int PFW_LFW_guac_display_frame_complete(guac_display* display) {
     guac_display_layer* current = display->pending_frame.layers;
     while (current != NULL) {
 
-        /* Copy over pending frame contents if actually changed */
-        if (!guac_rect_is_empty(&current->pending_frame.dirty)) {
+        /* Always resize the last_frame buffer to match the pending_frame prior
+         * to copying over any changes (this is particularly important given
+         * that the pending_frame buffer can be replaced with an external
+         * buffer). Since this involves copying over all data from the
+         * pending frame, we can skip the later pending frame copy based on
+         * whether the pending frame is dirty. */
+        if (current->last_frame.buffer_stride != current->pending_frame.buffer_stride
+                || current->last_frame.buffer_width != current->pending_frame.buffer_width
+                || current->last_frame.buffer_height != current->pending_frame.buffer_height) {
+
+            size_t buffer_size = guac_mem_ckd_mul_or_die(current->pending_frame.buffer_height,
+                    current->pending_frame.buffer_stride);
+
+            guac_mem_free(current->last_frame.buffer);
+            current->last_frame.buffer = guac_mem_zalloc(buffer_size);
+            memcpy(current->last_frame.buffer, current->pending_frame.buffer, buffer_size);
+
+            current->last_frame.buffer_stride = current->pending_frame.buffer_stride;
+            current->last_frame.buffer_width = current->pending_frame.buffer_width;
+            current->last_frame.buffer_height = current->pending_frame.buffer_height;
+
+            current->last_frame.dirty = current->pending_frame.dirty;
+            current->pending_frame.dirty = (guac_rect) { 0 };
+
+            retval = 1;
+
+        }
+
+        /* Copy over pending frame contents if actually changed (this is not
+         * necessary if the last_frame buffer was resized to match
+         * pending_frame, as a copy from pending_frame to last_frame is
+         * inherently part of that) */
+        else if (!guac_rect_is_empty(&current->pending_frame.dirty)) {
 
             unsigned char* pending_frame = current->pending_frame.buffer;
             unsigned char* last_frame = current->last_frame.buffer;

src/libguac/display-layer-list.c

@@ -118,91 +118,50 @@ static void guac_imgcpy(void* dst, size_t dst_stride, int dst_width, int dst_hei
  * @param height
  *     The new height, in pixels.
  */
-static void PFW_LFW_guac_display_layer_buffers_resize(guac_display_layer_state* last_frame,
-        guac_display_layer_state* pending_frame, int width, int height) {
+static void XFW_guac_display_layer_buffer_resize(guac_display_layer_state* frame_state,
+        int width, int height) {
 
-    GUAC_ASSERT(last_frame->buffer_width == pending_frame->buffer_width);
-    GUAC_ASSERT(last_frame->buffer_height == pending_frame->buffer_height);
+    /* We should never be trying to resize an externally-maintained buffer */
+    GUAC_ASSERT(!frame_state->buffer_is_external);
 
     /* Round up to nearest multiple of resize factor */
     width  = ((width  + GUAC_DISPLAY_RESIZE_FACTOR - 1) / GUAC_DISPLAY_RESIZE_FACTOR) * GUAC_DISPLAY_RESIZE_FACTOR;
     height = ((height + GUAC_DISPLAY_RESIZE_FACTOR - 1) / GUAC_DISPLAY_RESIZE_FACTOR) * GUAC_DISPLAY_RESIZE_FACTOR;
 
     /* Do nothing if size isn't actually changing */
-    if (width == last_frame->buffer_width
-            && height == last_frame->buffer_height)
+    if (width == frame_state->buffer_width
+            && height == frame_state->buffer_height)
         return;
 
-    /* The request to resize applies only to the pending frame, but space for
-     * the last frame must be maintained. If either requested dimension is
-     * smaller than the last frame dimensions, the relevant dimension of the
-     * last frame must be used instead. */
-
-    int new_buffer_width = last_frame->buffer_width;
-    if (width > new_buffer_width)
-        new_buffer_width = width;
-
-    int new_buffer_height = last_frame->buffer_height;
-    if (height > new_buffer_height)
-        new_buffer_height = height;
-
-    /* Determine details of shared buffer space sufficient for both the
-     * established last frame and the resized pending frame. Allocate new
-     * shared buffer space for last and pending frames, interleaving their
-     * rows.
-     *
-     * NOTE: We interleave the rows of the last and pending frames to promote
-     * locality of reference. The comparisons performed between last and
-     * pending frames to determine what has changed are faster when the rows
-     * are interleaved, as data relevant to those comparisons will tend to be
-     * present in the CPU cache. */
-
-    int new_last_frame_offset = cairo_format_stride_for_width(CAIRO_FORMAT_ARGB32, new_buffer_width);
-    int new_common_stride = cairo_format_stride_for_width(CAIRO_FORMAT_ARGB32, new_last_frame_offset * 2);
-    unsigned char* new_buffer_base = guac_mem_zalloc(new_buffer_height, new_common_stride);
-    unsigned char* new_pending_frame_buffer = new_buffer_base;
-    unsigned char* new_last_frame_buffer = new_buffer_base + new_last_frame_offset;
+    int stride = cairo_format_stride_for_width(CAIRO_FORMAT_ARGB32, width);
+    unsigned char* buffer = guac_mem_zalloc(height, stride);
 
     /* Copy over data from old shared buffer, if that data exists and is
      * relevant */
 
-    if (last_frame->buffer != NULL && pending_frame->buffer != NULL) {
+    if (frame_state->buffer != NULL) {
 
         guac_imgcpy(
 
-                /* Copy to newly-allocated pending frame buffer ... */
-                new_pending_frame_buffer, new_common_stride,
-                new_buffer_width, new_buffer_height,
+                /* Copy to newly-allocated frame buffer ... */
+                buffer, stride,
+                width, height,
 
-                /* ... from old pending frame buffer. */
-                pending_frame->buffer, pending_frame->buffer_stride,
-                pending_frame->buffer_width, pending_frame->buffer_height,
+                /* ... from old frame buffer. */
+                frame_state->buffer, frame_state->buffer_stride,
+                frame_state->buffer_width, frame_state->buffer_height,
 
                 /* All pixels are 32-bit */
                 GUAC_DISPLAY_LAYER_RAW_BPP);
 
-        guac_imgcpy(
-
-                /* Copy to newly-allocated last frame buffer ... */
-                new_last_frame_buffer, new_common_stride,
-                last_frame->buffer_width, last_frame->buffer_height,
-
-                /* ... from old last frame buffer. */
-                last_frame->buffer, last_frame->buffer_stride,
-                last_frame->buffer_width, last_frame->buffer_height,
-
-                /* All pixels are 32-bit */
-                GUAC_DISPLAY_LAYER_RAW_BPP);
+        guac_mem_free(frame_state->buffer);
 
     }
 
-    guac_mem_free(pending_frame->buffer);
-    last_frame->buffer = new_buffer_base + new_last_frame_offset;
-    pending_frame->buffer = new_buffer_base;
-
-    last_frame->buffer_width = pending_frame->buffer_width = new_buffer_width;
-    last_frame->buffer_height = pending_frame->buffer_height = new_buffer_height;
-    last_frame->buffer_stride = pending_frame->buffer_stride = new_common_stride;
+    frame_state->buffer = buffer;
+    frame_state->buffer_width = width;
+    frame_state->buffer_height = height;
+    frame_state->buffer_stride = stride;
 
 }
 
@@ -227,10 +186,10 @@ static void PFW_LFW_guac_display_layer_state_init(guac_display_layer_state* last
     last_frame->opacity = pending_frame->opacity = 0xFF;
     last_frame->parent = pending_frame->parent = GUAC_DEFAULT_LAYER;
 
-    /* Allocate shared buffer space for last and pending frames, interleaving
-     * their rows */
+    XFW_guac_display_layer_buffer_resize(last_frame,
+            last_frame->width, last_frame->height);
 
-    PFW_LFW_guac_display_layer_buffers_resize(last_frame, pending_frame,
+    XFW_guac_display_layer_buffer_resize(pending_frame,
             pending_frame->width, pending_frame->height);
 
 }
@@ -395,11 +354,14 @@ void guac_display_remove_layer(guac_display_layer* display_layer) {
 
     }
 
-    /* Free memory for underlying image surface and change tracking cells
-     * (NOTE: Freeing pending_frame.buffer inherently also frees
-     * last_frame.buffer because they are actually interleaved views of the
-     * same block) */
-    guac_mem_free(display_layer->pending_frame.buffer);
+    /* Free memory for underlying image surface and change tracking cells. Note
+     * that we do NOT free the associated memory for the pending frame if it
+     * was replaced with an external buffer. */
+
+    if (!display_layer->pending_frame.buffer_is_external)
+        guac_mem_free(display_layer->pending_frame.buffer);
+
+    guac_mem_free(display_layer->last_frame.buffer);
     guac_mem_free(display_layer->pending_frame_cells);
 
     guac_mem_free(display_layer);
@@ -421,7 +383,11 @@ void PFW_LFW_guac_display_layer_resize(guac_display_layer* layer, int width, int
 
     }
 
-    PFW_LFW_guac_display_layer_buffers_resize(&layer->last_frame, &layer->pending_frame, width, height);
+    /* Skip resizing underlying buffer if it's the caller that's responsible
+     * for resizing the buffer */
+    if (!layer->pending_frame.buffer_is_external)
+        XFW_guac_display_layer_buffer_resize(&layer->pending_frame, width, height);
+
     PFW_guac_display_layer_pending_frame_cells_resize(layer, width, height);
 
     layer->pending_frame.width = width;

src/libguac/display-layer.c

@@ -218,6 +218,38 @@ void guac_display_layer_close_raw(guac_display_layer* layer, guac_display_layer_
 
     guac_display* display = layer->display;
 
+    /* Replace buffer if requested with an external buffer. This intentionally
+     * falls through to the following buffer_is_external check to update the
+     * buffer details. */
+    if (context->buffer != layer->pending_frame.buffer
+            && !layer->pending_frame.buffer_is_external) {
+        guac_mem_free(layer->pending_frame.buffer);
+        layer->pending_frame.buffer_is_external = 1;
+    }
+
+    /* The details covering the structure of the buffer and the dimensions of
+     * the layer must be copied from the context if the buffer is external
+     * (there is no other way to resize a layer with an external buffer) */
+    if (layer->pending_frame.buffer_is_external) {
+
+        int width = guac_rect_width(&context->bounds);
+        if (width > GUAC_DISPLAY_MAX_WIDTH)
+            width = GUAC_DISPLAY_MAX_WIDTH;
+
+        int height = guac_rect_height(&context->bounds);
+        if (height > GUAC_DISPLAY_MAX_HEIGHT)
+            height = GUAC_DISPLAY_MAX_HEIGHT;
+
+        layer->pending_frame.buffer = context->buffer;
+        layer->pending_frame.buffer_width = width;
+        layer->pending_frame.buffer_height = height;
+        layer->pending_frame.buffer_stride = context->stride;
+
+        layer->pending_frame.width = layer->pending_frame.buffer_width;
+        layer->pending_frame.height = layer->pending_frame.buffer_height;
+
+    }
+
     guac_rect_extend(&layer->pending_frame.dirty, &context->dirty);
     PFW_guac_display_layer_touch(layer);
 

src/libguac/display-priv.h

@@ -70,6 +70,18 @@
  *     The function writes (and possibly reads) the state of the last frame.
  *     This prefix and "LFR_" are mutually-exclusive.
  *
+ *   "XFR_"
+ *     The function reads (but does not write) the state of a frame, and
+ *     whether that frame is the pending frame or the last frame depends on
+ *     which frame is provided via function parameters. This prefix and "XFW_"
+ *     are mutually-exclusive.
+ *
+ *   "XFW_"
+ *     The function writes (but does not read) the state of a frame, and
+ *     whether that frame is the pending frame or the last frame depends on
+ *     which frame is provided via function parameters. This prefix and "XFR_"
+ *     are mutually-exclusive.
+ *
  * Any functions lacking these prefixes either do not access last/pending
  * frames in any way or take care of acquiring/releasing locks entirely on
  * their own.
@@ -354,6 +366,13 @@ typedef struct guac_display_layer_state {
      */
     size_t buffer_stride;
 
+    /**
+     * Non-zero if the image data referenced by the buffer pointer was
+     * allocated externally and should not be automatically freed or managed by
+     * guac_display, zero otherwise.
+     */
+    int buffer_is_external;
+
     /**
      * The approximate rectangular region containing all pixels within this
      * layer that have been modified since the frame that occurred before this

src/libguac/guacamole/display.h

@@ -124,6 +124,12 @@ struct guac_display_layer_raw_context {
      * this image is 32-bit ARGB with 8 bits per color component, where the
      * lowest-order byte is the blue component and the highest-order byte is
      * alpha.
+     *
+     * This value may be replaced with a manually-allocated buffer if the
+     * associated layer should instead use that manualy-allocated buffer for
+     * future rendering operations. If the buffer is replaced, it must be
+     * maintained manually going forward, including when the buffer needs to be
+     * resized or after the corresponding layer/display have been freed.
      */
     unsigned char* buffer;
 
@@ -132,12 +138,28 @@ struct guac_display_layer_raw_context {
      * necessarily the same as the width of the image multiplied by the size of
      * each pixel. Additional space may be allocated to allow for memory
      * alignment or to make future resize operations more efficient.
+     *
+     * If the buffer for this layer is replaced with an external buffer, or if
+     * the external buffer changes structure, then this value must be manually
+     * kept up-to-date with the stride of the external buffer.
      */
     size_t stride;
 
     /**
      * A rectangle covering the current bounds of the graphical surface. The
      * buffer must not be addressed outside these bounds.
+     *
+     * If the buffer for this layer is replaced with an external buffer, or if
+     * the external buffer changes size, then the dimensions of this bounds
+     * rect must be manually kept up-to-date with the dimensions of the
+     * external buffer. These dimensions will also be passed through to become
+     * the dimensions of the layer, since layers with external buffers cannot
+     * be resized with guac_display_layer_resize().
+     *
+     * NOTE: If an external buffer is used and bounds dimensions are provided
+     * that are greater than GUAC_DISPLAY_MAX_WIDTH and
+     * GUAC_DISPLAY_MAX_HEIGHT, those values will instead be interpreted as
+     * equal to GUAC_DISPLAY_MAX_WIDTH and GUAC_DISPLAY_MAX_HEIGHT.
      */
     guac_rect bounds;
 
@@ -164,10 +186,16 @@ struct guac_display_layer_raw_context {
 /**
  * Allocates a new guac_display representing the remote display shared by all
  * connected users of the given guac_client. The dimensions of the display
- * should be set with guac_display_defaulta_layer() and
+ * should be set with guac_display_default_layer() and
  * guac_display_layer_resize() once the desired display size is known. The
  * guac_display must eventually be freed through a call to guac_display_free().
  *
+ * NOTE: If the buffer of a layer has been replaced with an externally
+ * maintained buffer, this function CANNOT be used to resize the layer. The
+ * layer must instead be resized through changing the bounds of a
+ * guac_display_layer_raw_context and, if necessary, replacing the underlying
+ * buffer again.
+ *
  * @param client
  *     The guac_client whose remote display should be represented by the new
  *     guac_display.
@@ -520,6 +548,12 @@ void guac_display_layer_set_multitouch(guac_display_layer* layer, int touches);
  * will be made as part of the current pending frame, and will not take effect
  * on remote displays until the pending frame is complete.
  *
+ * This function will not resize the underlying buffer containing image data if
+ * the layer has been manually reassociated with a different,
+ * externally-maintained buffer using a guac_display_layer_raw_context. If this
+ * is the case, that buffer must instead be manually maintained by the caller,
+ * and resizing will typically involve replacing the buffer again.
+ *
  * @param layer
  *     The layer to set the size of.
  *
@@ -535,6 +569,45 @@ void guac_display_layer_set_multitouch(guac_display_layer* layer, int touches);
  */
 void guac_display_layer_resize(guac_display_layer* layer, int width, int height);
 
+/**
+ * Replaces the image data buffer of the given layer with the provided buffer.
+ * If the buffer associated with the layer was provided via a previous call to
+ * this function, it is not freed. The caller is responsible for memory
+ * management around the provided buffer, including ensuring that the buffer
+ * continues to exist until the layer or display are freed or the buffer is
+ * replaced again.
+ *
+ * The caller MUST still ensure that changes to the provided buffer are made
+ * using proper contexts, as obtained through calling
+ * guac_display_layer_open_raw() or guac_display_layer_open_cairo(). This is
+ * required to ensure that exclusive access is acquired and to ensure that
+ * changes to the buffer are correctly tracked.
+ *
+ * @param layer
+ *     The layer to replace the buffer of.
+ *
+ * @param buffer
+ *     The buffer that should be used by the given layer to store/retrieve image
+ *     data associated with an in-progress frame. The buffer MUST be a raw,
+ *     32-bit buffer of ARGB image data. If transparency is not used by this
+ *     layer, the alpha channel of that ARGB data should be set to 0xFF.
+ *
+ * @param width
+ *     The new width to assign to the layer, in pixels. Any values provided
+ *     that are greater than GUAC_DISPLAY_MAX_WIDTH will instead be interpreted
+ *     as equal to GUAC_DISPLAY_MAX_WIDTH.
+ *
+ * @param height
+ *     The new height to assign to the layer, in pixels. Any values provided
+ *     that are greater than GUAC_DISPLAY_MAX_HEIGHT will instead be
+ *     interpreted as equal to GUAC_DISPLAY_MAX_HEIGHT.
+ *
+ * @param stride
+ *     The number of bytes in each row of image data within the provided buffer.
+ */
+void guac_display_layer_replace_buffer(guac_display_layer* layer, void* buffer,
+        int width, int height, size_t stride);
+
 /**
  * Begins a drawing operation for the given layer, returning a context that can
  * be used to draw directly to the raw image buffer containing the layer's