Skip to content

Commit

Permalink
GUACAMOLE-377: Add ability to "hint" that a drawing operation copied …
Browse files Browse the repository at this point in the history
…data from another layer.
  • Loading branch information
mike-jumper committed Jun 19, 2024
1 parent 0e8c111 commit 483700a
Show file tree
Hide file tree
Showing 8 changed files with 107 additions and 32 deletions.
8 changes: 8 additions & 0 deletions src/libguac/display-flush.c
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,14 @@ static void PFW_LFW_guac_display_frame_complete(guac_display* display) {
current->last_frame.touches = current->pending_frame.touches;
}

/* Commit any hinting regarding scroll/copy optimization (NOTE: While
* this value is copied for consistency, it will already have taken
* effect in the context of the pending frame due to the scroll/copy
* optimization pass having occurred prior to the call to this
* function) */
current->last_frame.search_for_copies = current->pending_frame.search_for_copies;
current->pending_frame.search_for_copies = 0;

/* Commit any change in lossless setting (no need to synchronize this
* to the client - it affects only how last_frame is interpreted) */
current->last_frame.lossless = current->pending_frame.lossless;
Expand Down
10 changes: 10 additions & 0 deletions src/libguac/display-layer.c
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ guac_display_layer_raw_context* guac_display_layer_open_raw(guac_display_layer*
.buffer = layer->pending_frame.buffer,
.stride = layer->pending_frame.buffer_stride,
.dirty = { 0 },
.hint_from = layer,
.bounds = {
.left = 0,
.top = 0,
Expand All @@ -224,6 +225,10 @@ void guac_display_layer_close_raw(guac_display_layer* layer, guac_display_layer_
guac_rect_extend(&layer->pending_frame.dirty, &context->dirty);
PFW_guac_display_layer_touch(layer);

/* Apply any hinting regarding scroll/copy optimization */
if (context->hint_from != NULL)
context->hint_from->pending_frame.search_for_copies = 1;

guac_rwlock_release_lock(&display->pending_frame.lock);

}
Expand All @@ -236,6 +241,7 @@ guac_display_layer_cairo_context* guac_display_layer_open_cairo(guac_display_lay
guac_display_layer_cairo_context* context = &(layer->pending_frame_cairo_context);

context->dirty = (guac_rect) { 0 };
context->hint_from = layer;
context->bounds = (guac_rect) {
.left = 0,
.top = 0,
Expand Down Expand Up @@ -267,6 +273,10 @@ void guac_display_layer_close_cairo(guac_display_layer* layer, guac_display_laye
guac_rect_extend(&layer->pending_frame.dirty, &context->dirty);
PFW_guac_display_layer_touch(layer);

/* Apply any hinting regarding scroll/copy optimization */
if (context->hint_from != NULL)
context->hint_from->pending_frame.search_for_copies = 1;

guac_rwlock_release_lock(&display->pending_frame.lock);

}
16 changes: 9 additions & 7 deletions src/libguac/display-plan-combine.c
Original file line number Diff line number Diff line change
Expand Up @@ -125,14 +125,16 @@ static int guac_display_plan_should_combine(const guac_display_plan_operation* o
switch (op_a->type) {

/* Copy operations can be combined if they are perfectly adjacent
* (exactly share an edge) and copy in the same direction */
* (exactly share an edge) and copy from the same source layer in
* the same direction */
case GUAC_DISPLAY_PLAN_OPERATION_COPY:
if (guac_display_plan_has_common_edge(op_a, op_b)) {
if (op_a->src.layer_rect.layer == op_b->src.layer_rect.layer
&& guac_display_plan_has_common_edge(op_a, op_b)) {

int delta_xa = op_a->dest.left - op_a->src.rect.left;
int delta_ya = op_a->dest.top - op_a->src.rect.top;
int delta_xb = op_b->dest.left - op_b->src.rect.left;
int delta_yb = op_b->dest.top - op_b->src.rect.top;
int delta_xa = op_a->dest.left - op_a->src.layer_rect.rect.left;
int delta_ya = op_a->dest.top - op_a->src.layer_rect.rect.top;
int delta_xb = op_b->dest.left - op_b->src.layer_rect.rect.left;
int delta_yb = op_b->dest.top - op_b->src.layer_rect.rect.top;

return delta_xa == delta_xb
&& delta_ya == delta_yb;
Expand Down Expand Up @@ -221,7 +223,7 @@ static int guac_display_plan_combine_if_improved(guac_display_plan_operation* op
/* When combining two copy operations, additionally combine their
* source rects (NOT just the destination rects) */
else if (op_a->type == GUAC_DISPLAY_PLAN_OPERATION_COPY)
guac_rect_extend(&op_a->src.rect, &op_b->src.rect);
guac_rect_extend(&op_a->src.layer_rect.rect, &op_b->src.layer_rect.rect);

op_a->dirty_size += op_b->dirty_size;

Expand Down
40 changes: 24 additions & 16 deletions src/libguac/display-plan-search.c
Original file line number Diff line number Diff line change
Expand Up @@ -373,28 +373,31 @@ static int guac_image_cmp(const unsigned char* restrict data_a, int width_a, int
static void PFR_LFR_guac_display_plan_find_copies(guac_display_plan* plan,
int x, int y, uint64_t hash, void* closure) {

guac_display_layer* layer = (guac_display_layer*) closure;
guac_display_layer* copy_from_layer = (guac_display_layer*) closure;

/* Transform the matching operation into a copy of the current region if
* any operations match, banning the underlying hash from further checks if
* a collision occurs */
guac_display_plan_operation* op = guac_display_plan_remove_indexed_op(plan, hash);
if (op != NULL && op->layer == layer) {
if (op != NULL) {

guac_display_layer* copy_to_layer = op->layer;

guac_rect src_rect;
guac_rect_init(&src_rect, x, y, GUAC_DISPLAY_CELL_SIZE, GUAC_DISPLAY_CELL_SIZE);

guac_rect dst_rect;
guac_display_cell_init_rect(&dst_rect, op->dest.left, op->dest.top);

const unsigned char* copy_from = GUAC_DISPLAY_LAYER_STATE_CONST_BUFFER(layer->last_frame, src_rect);
const unsigned char* copy_to = GUAC_DISPLAY_LAYER_STATE_CONST_BUFFER(layer->pending_frame, dst_rect);
const unsigned char* copy_from = GUAC_DISPLAY_LAYER_STATE_CONST_BUFFER(copy_from_layer->last_frame, src_rect);
const unsigned char* copy_to = GUAC_DISPLAY_LAYER_STATE_CONST_BUFFER(copy_to_layer->pending_frame, dst_rect);

/* Only transform into a copy if the image data is truly identical (not a collision) */
if (!guac_image_cmp(copy_from, GUAC_DISPLAY_CELL_SIZE, GUAC_DISPLAY_CELL_SIZE, layer->last_frame.buffer_stride,
copy_to, GUAC_DISPLAY_CELL_SIZE, GUAC_DISPLAY_CELL_SIZE, layer->pending_frame.buffer_stride)) {
if (!guac_image_cmp(copy_from, GUAC_DISPLAY_CELL_SIZE, GUAC_DISPLAY_CELL_SIZE, copy_from_layer->last_frame.buffer_stride,
copy_to, GUAC_DISPLAY_CELL_SIZE, GUAC_DISPLAY_CELL_SIZE, copy_to_layer->pending_frame.buffer_stride)) {
op->type = GUAC_DISPLAY_PLAN_OPERATION_COPY;
op->src.rect = src_rect;
op->src.layer_rect.layer = copy_from_layer->last_frame_buffer;
op->src.layer_rect.rect = src_rect;
op->dest = dst_rect;
}

Expand All @@ -408,17 +411,22 @@ void PFR_LFR_guac_display_plan_rewrite_as_copies(guac_display_plan* plan) {
guac_display_layer* current = display->last_frame.layers;
while (current != NULL) {

guac_rect search_region;
guac_rect_init(&search_region, 0, 0, current->last_frame.width, current->last_frame.height);
/* Search only the layers that are specifically noted as possible
* sources for copies */
if (current->pending_frame.search_for_copies) {

guac_rect search_region;
guac_rect_init(&search_region, 0, 0, current->last_frame.width, current->last_frame.height);

/* Avoid excessive computation by restricting the search region to only
* the area that was changed in the upcoming frame (in the case of
* scrolling, absolutely all data relevant to the scroll will have been
* modified) */
guac_rect_constrain(&search_region, &current->pending_frame.dirty);
/* Avoid excessive computation by restricting the search region to only
* the area that was changed in the upcoming frame (in the case of
* scrolling, absolutely all data relevant to the scroll will have been
* modified) */
guac_rect_constrain(&search_region, &current->pending_frame.dirty);

guac_hash_foreach_image_rect(plan, &current->last_frame, &search_region,
PFR_LFR_guac_display_plan_find_copies, current);
guac_hash_foreach_image_rect(plan, &current->last_frame, &search_region,
PFR_LFR_guac_display_plan_find_copies, current);
}

current = current->last_frame.next;

Expand Down
22 changes: 21 additions & 1 deletion src/libguac/display-plan.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ typedef enum guac_display_plan_operation_type {

/**
* Copy image data from the associated source rect to the destination rect.
* The source and destination layers are not necessarily the same.
*/
GUAC_DISPLAY_PLAN_OPERATION_COPY,

Expand All @@ -155,6 +156,25 @@ typedef enum guac_display_plan_operation_type {

} guac_display_plan_operation_type;

/**
* A reference to a rectangular region of image data within a layer of the
* remote Guacamole display.
*/
typedef struct guac_display_plan_layer_rect {

/**
* The rectangular region that should serve as source data for an
* operation.
*/
guac_rect rect;

/**
* The layer that the source data is coming from.
*/
const guac_layer* layer;

} guac_display_plan_layer_rect;

/**
* Any one of several operations that may be contained in a guac_display_plan.
*/
Expand Down Expand Up @@ -208,7 +228,7 @@ typedef struct guac_display_plan_operation {
* The rectangle that should be copied to the destination rect. This
* value applies only to GUAC_DISPLAY_PLAN_OPERATION_COPY operations.
*/
guac_rect rect;
guac_display_plan_layer_rect layer_rect;

} src;

Expand Down
6 changes: 6 additions & 0 deletions src/libguac/display-priv.h
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,12 @@ typedef struct guac_display_layer_state {
*/
guac_rect dirty;

/**
* Whether this layer should be searched for possible scroll/copy
* optimizations.
*/
int search_for_copies;

/* ---------------- LAYER LIST POINTERS ---------------- */

/**
Expand Down
5 changes: 3 additions & 2 deletions src/libguac/display-worker.c
Original file line number Diff line number Diff line change
Expand Up @@ -370,8 +370,9 @@ void* guac_display_worker_thread(void* data) {
break;

case GUAC_DISPLAY_PLAN_OPERATION_COPY:
guac_protocol_send_copy(client->socket, display_layer->last_frame_buffer,
op.src.rect.left, op.src.rect.top, guac_rect_width(&op.src.rect), guac_rect_height(&op.src.rect),
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;

Expand Down
32 changes: 26 additions & 6 deletions src/libguac/guacamole/display.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,22 @@ struct guac_display_layer_cairo_context {

/**
* A rectangle covering the region of the guac_display_layer that has
* changed since the last frame. This rectangle must be manually updated to
* cover any additional changed regions before closing the
* guac_display_layer_cairo_context.
* changed since the last frame. This rectangle is initially empty and must
* be manually updated to cover any additional changed regions before
* closing the guac_display_layer_cairo_context.
*/
guac_rect dirty;

/**
* The layer that should be searched for possible scroll/copy operations
* related to the changes being made via this guac_display_layer_cairo_context.
* This value is initially the layer being drawn to and must be updated
* before closing the context if a different source layer should be
* considered for scroll/copy optimizations. This value may be set to NULL
* to hint that no scroll/copy optimization should be performed.
*/
guac_display_layer* hint_from;

};

struct guac_display_layer_raw_context {
Expand Down Expand Up @@ -111,12 +121,22 @@ struct guac_display_layer_raw_context {

/**
* A rectangle covering the region of the guac_display_layer that has
* changed since the last frame. This rectangle must be manually updated to
* cover any additional changed regions before closing the
* guac_display_layer_raw_context.
* changed since the last frame. This rectangle is initially empty and must
* be manually updated to cover any additional changed regions before
* closing the guac_display_layer_raw_context.
*/
guac_rect dirty;

/**
* The layer that should be searched for possible scroll/copy operations
* related to the changes being made via this guac_display_layer_raw_context.
* This value is initially the layer being drawn to and must be updated
* before closing the context if a different source layer should be
* considered for scroll/copy optimizations. This value may be set to NULL
* to hint that no scroll/copy optimization should be performed.
*/
guac_display_layer* hint_from;

};

/**
Expand Down

0 comments on commit 483700a

Please sign in to comment.