From 4e130d2afdda8ddeb49452ec603498adb688470f Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 29 Sep 2019 21:57:02 -0700 Subject: [PATCH 1/3] GUACAMOLE-884: Leverage createImageBitmap() for reading image data where supported. Some browsers suffer from a memory leak when reading image data repeatedly using the Image object. Reading from Blobs does not exhibit the same behavior. While reading from Blobs has previously been seen to perform poorly compared to data URIs, this was observed when reading using createObjectURL(). The createImageBitmap() function appears to perform identically to reading data URIs using Image. --- .../src/main/webapp/modules/Client.js | 9 +- .../src/main/webapp/modules/Display.js | 98 ++++++++++++++++--- 2 files changed, 86 insertions(+), 21 deletions(-) diff --git a/guacamole-common-js/src/main/webapp/modules/Client.js b/guacamole-common-js/src/main/webapp/modules/Client.js index 07baf7731..ed8085d29 100644 --- a/guacamole-common-js/src/main/webapp/modules/Client.js +++ b/guacamole-common-js/src/main/webapp/modules/Client.js @@ -1190,13 +1190,10 @@ Guacamole.Client = function(tunnel) { // Create stream var stream = streams[stream_index] = new Guacamole.InputStream(guac_client, stream_index); - var reader = new Guacamole.DataURIReader(stream, mimetype); - // Draw image when stream is complete - reader.onend = function drawImageBlob() { - display.setChannelMask(layer, channelMask); - display.draw(layer, x, y, reader.getURI()); - }; + // Draw received contents once decoded + display.setChannelMask(layer, channelMask); + display.drawStream(layer, x, y, stream, mimetype); }, diff --git a/guacamole-common-js/src/main/webapp/modules/Display.js b/guacamole-common-js/src/main/webapp/modules/Display.js index be5bf6c52..0bc569f0a 100644 --- a/guacamole-common-js/src/main/webapp/modules/Display.js +++ b/guacamole-common-js/src/main/webapp/modules/Display.js @@ -543,26 +543,94 @@ Guacamole.Display = function() { */ this.drawBlob = function(layer, x, y, blob) { - // Create URL for blob - var url = URL.createObjectURL(blob); + var task; - // Draw and free blob URL when ready - var task = scheduleTask(function __display_drawBlob() { + // Prefer createImageBitmap() over blob URLs if available + if (window.createImageBitmap) { - // Draw the image only if it loaded without errors - if (image.width && image.height) - layer.drawImage(x, y, image); + var bitmap; - // Blob URL no longer needed - URL.revokeObjectURL(url); + // Draw image once loaded + task = scheduleTask(function drawImageBitmap() { + layer.drawImage(x, y, bitmap); + }, true); - }, true); + // Load image from provided blob + window.createImageBitmap(blob).then(function bitmapLoaded(decoded) { + bitmap = decoded; + task.unblock(); + }); - // Load image from URL - var image = new Image(); - image.onload = task.unblock; - image.onerror = task.unblock; - image.src = url; + } + + // Use blob URLs and the Image object if createImageBitmap() is + // unavailable + else { + + // Create URL for blob + var url = URL.createObjectURL(blob); + + // Draw and free blob URL when ready + task = scheduleTask(function __display_drawBlob() { + + // Draw the image only if it loaded without errors + if (image.width && image.height) + layer.drawImage(x, y, image); + + // Blob URL no longer needed + URL.revokeObjectURL(url); + + }, true); + + // Load image from URL + var image = new Image(); + image.onload = task.unblock; + image.onerror = task.unblock; + image.src = url; + + } + + }; + + /** + * Draws the image within the given stream at the given coordinates. The + * image will be loaded automatically, and this and any future operations + * will wait for the image to finish loading. + * + * @param {Guacamole.Layer} layer + * The layer to draw upon. + * + * @param {Number} x + * The destination X coordinate. + * + * @param {Number} y + * The destination Y coordinate. + * + * @param {Guacamole.InputStream} stream + * The stream along which image data will be received. + * + * @param {String} mimetype + * The mimetype of the image within the stream. + */ + this.drawStream = function drawStream(layer, x, y, stream, mimetype) { + + // If createImageBitmap() is available, load the image as a blob so + // that function can be used + if (window.createImageBitmap) { + var reader = new Guacamole.BlobReader(stream, mimetype); + reader.onend = function drawImageBlob() { + guac_display.drawBlob(layer, x, y, reader.getBlob()); + }; + } + + // Lacking createImageBitmap(), fall back to data URIs and the Image + // object + else { + var reader = new Guacamole.DataURIReader(stream, mimetype); + reader.onend = function drawImageDataURI() { + guac_display.draw(layer, x, y, reader.getURI()); + }; + } }; From 6649c5d3340224bc4245673903b9f5cb341dc065 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 29 Sep 2019 22:13:19 -0700 Subject: [PATCH 2/3] GUACAMOLE-884: Document implications of using drawStream() vs. the other drawing functions. --- guacamole-common-js/src/main/webapp/modules/Display.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/guacamole-common-js/src/main/webapp/modules/Display.js b/guacamole-common-js/src/main/webapp/modules/Display.js index 0bc569f0a..853c69acc 100644 --- a/guacamole-common-js/src/main/webapp/modules/Display.js +++ b/guacamole-common-js/src/main/webapp/modules/Display.js @@ -595,7 +595,10 @@ Guacamole.Display = function() { /** * Draws the image within the given stream at the given coordinates. The * image will be loaded automatically, and this and any future operations - * will wait for the image to finish loading. + * will wait for the image to finish loading. This function will + * automatically choose an approriate method for reading and decoding the + * given image stream, and should be preferred for received streams except + * where manual decoding of the stream is unavoidable. * * @param {Guacamole.Layer} layer * The layer to draw upon. From 1e9ddc2183fe5aece390312387592a7c5c9cb8ef Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Sun, 29 Sep 2019 22:17:45 -0700 Subject: [PATCH 3/3] GUACAMOLE-884: Note that the drawImage() functions accept anything implementing the CanvasImageSource interface, not just Image. --- .../src/main/webapp/modules/Display.js | 16 +++++++++++----- .../src/main/webapp/modules/Layer.js | 12 ++++++++---- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/guacamole-common-js/src/main/webapp/modules/Display.js b/guacamole-common-js/src/main/webapp/modules/Display.js index 853c69acc..41f04c7b3 100644 --- a/guacamole-common-js/src/main/webapp/modules/Display.js +++ b/guacamole-common-js/src/main/webapp/modules/Display.js @@ -512,11 +512,17 @@ Guacamole.Display = function() { * Draws the specified image at the given coordinates. The image specified * must already be loaded. * - * @param {Guacamole.Layer} layer The layer to draw upon. - * @param {Number} x The destination X coordinate. - * @param {Number} y The destination Y coordinate. - * @param {Image} image The image to draw. Note that this is an Image - * object - not a URL. + * @param {Guacamole.Layer} layer + * The layer to draw upon. + * + * @param {Number} x + * The destination X coordinate. + * + * @param {Number} y + * The destination Y coordinate. + * + * @param {CanvasImageSource} image + * The image to draw. Note that this not a URL. */ this.drawImage = function(layer, x, y, image) { scheduleTask(function __display_drawImage() { diff --git a/guacamole-common-js/src/main/webapp/modules/Layer.js b/guacamole-common-js/src/main/webapp/modules/Layer.js index 5384d2c74..cd87d8d1d 100644 --- a/guacamole-common-js/src/main/webapp/modules/Layer.js +++ b/guacamole-common-js/src/main/webapp/modules/Layer.js @@ -319,10 +319,14 @@ Guacamole.Layer = function(width, height) { * Draws the specified image at the given coordinates. The image specified * must already be loaded. * - * @param {Number} x The destination X coordinate. - * @param {Number} y The destination Y coordinate. - * @param {Image} image The image to draw. Note that this is an Image - * object - not a URL. + * @param {Number} x + * The destination X coordinate. + * + * @param {Number} y + * The destination Y coordinate. + * + * @param {CanvasImageSource} image + * The image to draw. Note that this is not a URL. */ this.drawImage = function(x, y, image) { if (layer.autosize) fitRect(x, y, image.width, image.height);