From 7f176d0dd15f7c10cebc67d78737c640fb850c45 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 7 Oct 2016 21:14:17 -0700 Subject: [PATCH 1/2] GUACAMOLE-187: Reduce the number of full canvas copies that occur when continuously resizing a layer. --- .../src/main/webapp/modules/Layer.js | 109 +++++++++++------- 1 file changed, 70 insertions(+), 39 deletions(-) diff --git a/guacamole-common-js/src/main/webapp/modules/Layer.js b/guacamole-common-js/src/main/webapp/modules/Layer.js index 98da309c3..a3eb22779 100644 --- a/guacamole-common-js/src/main/webapp/modules/Layer.js +++ b/guacamole-common-js/src/main/webapp/modules/Layer.js @@ -42,6 +42,17 @@ Guacamole.Layer = function(width, height) { */ var layer = this; + /** + * The number of pixels the width or height of a layer must change before + * the underlying canvas is resized. The underlying canvas will be kept at + * dimensions which are integer multiples of this factor. + * + * @private + * @constant + * @type Number + */ + var CANVAS_SIZE_FACTOR = 64; + /** * The canvas element backing this Layer. * @private @@ -98,57 +109,78 @@ Guacamole.Layer = function(width, height) { }; /** - * Resizes the canvas element backing this Layer without testing the - * new size. This function should only be used internally. + * Resizes the canvas element backing this Layer. This function should only + * be used internally. * * @private - * @param {Number} newWidth The new width to assign to this Layer. - * @param {Number} newHeight The new height to assign to this Layer. + * @param {Number} [newWidth=0] + * The new width to assign to this Layer. + * + * @param {Number} [newHeight=0] + * The new height to assign to this Layer. */ - function resize(newWidth, newHeight) { + var resize = function resize(newWidth, newHeight) { - // Only preserve old data if width/height are both non-zero - var oldData = null; - if (layer.width !== 0 && layer.height !== 0) { + // Default size to zero + newWidth = newWidth || 0; + newHeight = newHeight || 0; - // Create canvas and context for holding old data - oldData = document.createElement("canvas"); - oldData.width = layer.width; - oldData.height = layer.height; + // Calculate new dimensions of internal canvas + var canvasWidth = Math.ceil(newWidth / CANVAS_SIZE_FACTOR) * CANVAS_SIZE_FACTOR; + var canvasHeight = Math.ceil(newHeight / CANVAS_SIZE_FACTOR) * CANVAS_SIZE_FACTOR; - var oldDataContext = oldData.getContext("2d"); + // Resize only if canvas dimensions are actually changing + if (canvas.width !== canvasWidth || canvas.height !== canvasHeight) { - // Copy image data from current - oldDataContext.drawImage(canvas, - 0, 0, layer.width, layer.height, - 0, 0, layer.width, layer.height); + // Copy old data only if relevant + var oldData = null; + if (canvas.width !== 0 && canvas.height !== 0) { + + // Create canvas and context for holding old data + oldData = document.createElement("canvas"); + oldData.width = Math.min(layer.width, newWidth); + oldData.height = Math.min(layer.height, newHeight); + + var oldDataContext = oldData.getContext("2d"); + + // Copy image data from current + oldDataContext.drawImage(canvas, + 0, 0, oldData.width, oldData.height, + 0, 0, oldData.width, oldData.height); + + } + + // Preserve composite operation + var oldCompositeOperation = context.globalCompositeOperation; + + // Resize canvas + canvas.width = canvasWidth; + canvas.height = canvasHeight; + + // Redraw old data, if any + if (oldData) + context.drawImage(oldData, + 0, 0, oldData.width, oldData.height, + 0, 0, oldData.width, oldData.height); + + // Restore composite operation + context.globalCompositeOperation = oldCompositeOperation; + + // Acknowledge reset of stack (happens on resize of canvas) + stackSize = 0; + context.save(); } - // Preserve composite operation - var oldCompositeOperation = context.globalCompositeOperation; - - // Resize canvas - canvas.width = newWidth; - canvas.height = newHeight; - - // Redraw old data, if any - if (oldData) - context.drawImage(oldData, - 0, 0, layer.width, layer.height, - 0, 0, layer.width, layer.height); - - // Restore composite operation - context.globalCompositeOperation = oldCompositeOperation; + // If the canvas size is not changing, manually force state reset + else + layer.reset(); + // Assign new layer dimensions layer.width = newWidth; layer.height = newHeight; - // Acknowledge reset of stack (happens on resize of canvas) - stackSize = 0; - context.save(); - - } + }; /** * Given the X and Y coordinates of the upper-left corner of a rectangle @@ -781,8 +813,7 @@ Guacamole.Layer = function(width, height) { }; // Initialize canvas dimensions - canvas.width = width; - canvas.height = height; + resize(width, height); // Explicitly render canvas below other elements in the layer (such as // child layers). Chrome and others may fail to render layers properly From 053f34a54d9e283042592a6492b7bd300d2c3a19 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 7 Oct 2016 23:08:51 -0700 Subject: [PATCH 2/2] GUACAMOLE-187: Only preserve canvas contents during resize if there are contents to preserve. --- .../src/main/webapp/modules/Layer.js | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/guacamole-common-js/src/main/webapp/modules/Layer.js b/guacamole-common-js/src/main/webapp/modules/Layer.js index a3eb22779..1ed9d4c54 100644 --- a/guacamole-common-js/src/main/webapp/modules/Layer.js +++ b/guacamole-common-js/src/main/webapp/modules/Layer.js @@ -66,6 +66,16 @@ Guacamole.Layer = function(width, height) { var context = canvas.getContext("2d"); context.save(); + /** + * Whether the layer has not yet been drawn to. Once any draw operation + * which affects the underlying canvas is invoked, this flag will be set to + * false. + * + * @private + * @type Boolean + */ + var empty = true; + /** * Whether a new path should be started with the next path drawing * operations. @@ -132,9 +142,9 @@ Guacamole.Layer = function(width, height) { // Resize only if canvas dimensions are actually changing if (canvas.width !== canvasWidth || canvas.height !== canvasHeight) { - // Copy old data only if relevant + // Copy old data only if relevant and non-empty var oldData = null; - if (canvas.width !== 0 && canvas.height !== 0) { + if (!empty && canvas.width !== 0 && canvas.height !== 0) { // Create canvas and context for holding old data oldData = document.createElement("canvas"); @@ -289,6 +299,7 @@ Guacamole.Layer = function(width, height) { this.drawImage = function(x, y, image) { if (layer.autosize) fitRect(x, y, image.width, image.height); context.drawImage(image, x, y); + empty = false; }; /** @@ -367,6 +378,7 @@ Guacamole.Layer = function(width, height) { // Draw image data context.putImageData(dst, x, y); + empty = false; }; @@ -410,6 +422,7 @@ Guacamole.Layer = function(width, height) { // Get image data from src and dst var src = srcLayer.getCanvas().getContext("2d").getImageData(srcx, srcy, srcw, srch); context.putImageData(src, x, y); + empty = false; }; @@ -453,6 +466,7 @@ Guacamole.Layer = function(width, height) { if (layer.autosize) fitRect(x, y, srcw, srch); context.drawImage(srcCanvas, srcx, srcy, srcw, srch, x, y, srcw, srch); + empty = false; }; @@ -615,6 +629,7 @@ Guacamole.Layer = function(width, height) { context.lineWidth = thickness; context.strokeStyle = "rgba(" + r + "," + g + "," + b + "," + a/255.0 + ")"; context.stroke(); + empty = false; // Path now implicitly closed pathClosed = true; @@ -637,6 +652,7 @@ Guacamole.Layer = function(width, height) { // Fill with color context.fillStyle = "rgba(" + r + "," + g + "," + b + "," + a/255.0 + ")"; context.fill(); + empty = false; // Path now implicitly closed pathClosed = true; @@ -669,6 +685,7 @@ Guacamole.Layer = function(width, height) { "repeat" ); context.stroke(); + empty = false; // Path now implicitly closed pathClosed = true; @@ -693,6 +710,7 @@ Guacamole.Layer = function(width, height) { "repeat" ); context.fill(); + empty = false; // Path now implicitly closed pathClosed = true;