From 40f8d62e822132841163eaa8f863609e04b21f17 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Thu, 23 Aug 2018 16:52:09 +0200 Subject: [PATCH 1/3] Better encapsulate image loading The render queue doesn't need to know how to wait for images to load. --- core/display.js | 34 +++++++++++++++++----------------- tests/test.display.js | 28 ++-------------------------- 2 files changed, 19 insertions(+), 43 deletions(-) diff --git a/core/display.js b/core/display.js index 8eaa8001c..491cf99c1 100644 --- a/core/display.js +++ b/core/display.js @@ -360,17 +360,27 @@ export default class Display { return; } - const img = new Image(); - img.src = "data: " + mime + ";base64," + Base64.encode(arr); - - this._renderQPush({ + let a = { 'type': 'img', - 'img': img, 'x': x, 'y': y, 'width': width, 'height': height - }); + }; + + const img = new Image(); + img.src = "data: " + mime + ";base64," + Base64.encode(arr); + /* IE tends to set "complete" prematurely, so check dimensions */ + if (img.complete && (img.width !== 0) && (img.height !== 0)) { + a.img = img; + } else { + img.addEventListener('load', () => { + a.img = img; + this._scanRenderQ(); + }); + } + + this._renderQPush(a); } blitImage(x, y, width, height, arr, offset, fromQueue) { @@ -469,13 +479,6 @@ export default class Display { } } - _resumeRenderQ() { - // "this" is the object that is ready, not the - // display object - this.removeEventListener('load', this._noVNCDisplay._resumeRenderQ); - this._noVNCDisplay._scanRenderQ(); - } - _scanRenderQ() { let ready = true; while (ready && this._renderQ.length > 0) { @@ -494,8 +497,7 @@ export default class Display { this.blitImage(a.x, a.y, a.width, a.height, a.data, 0, true); break; case 'img': - /* IE tends to set "complete" prematurely, so check dimensions */ - if (a.img.complete && (a.img.width !== 0) && (a.img.height !== 0)) { + if (a.img) { if (a.img.width !== a.width || a.img.height !== a.height) { Log.Error("Decoded image has incorrect dimensions. Got " + a.img.width + "x" + a.img.height + ". Expected " + @@ -504,8 +506,6 @@ export default class Display { } this.drawImage(a.img, a.x, a.y); } else { - a.img._noVNCDisplay = this; - a.img.addEventListener('load', this._resumeRenderQ); // We need to wait for this image to 'load' // to keep things in-order ready = false; diff --git a/tests/test.display.js b/tests/test.display.js index bd5a55f9b..de2e1ea3b 100644 --- a/tests/test.display.js +++ b/tests/test.display.js @@ -343,8 +343,7 @@ describe('Display/Canvas Helper', function () { }); it('should wait until an image is loaded to attempt to draw it and the rest of the queue', function () { - const img = { complete: false, width: 4, height: 4, addEventListener: sinon.spy() }; - display._renderQ = [{ type: 'img', x: 3, y: 4, width: 4, height: 4, img: img }, + display._renderQ = [{ type: 'img', x: 3, y: 4, width: 4, height: 4 }, { type: 'fill', x: 1, y: 2, width: 3, height: 4, color: 5 }]; display.drawImage = sinon.spy(); display.fillRect = sinon.spy(); @@ -352,34 +351,11 @@ describe('Display/Canvas Helper', function () { display._scanRenderQ(); expect(display.drawImage).to.not.have.been.called; expect(display.fillRect).to.not.have.been.called; - expect(img.addEventListener).to.have.been.calledOnce; - display._renderQ[0].img.complete = true; + display._renderQ[0].img = { width: 4, height: 4 }; display._scanRenderQ(); expect(display.drawImage).to.have.been.calledOnce; expect(display.fillRect).to.have.been.calledOnce; - expect(img.addEventListener).to.have.been.calledOnce; - }); - - it('should wait if an image is incorrectly loaded', function () { - const img = { complete: true, width: 0, height: 0, addEventListener: sinon.spy() }; - display._renderQ = [{ type: 'img', x: 3, y: 4, width: 4, height: 4, img: img }, - { type: 'fill', x: 1, y: 2, width: 3, height: 4, color: 5 }]; - display.drawImage = sinon.spy(); - display.fillRect = sinon.spy(); - - display._scanRenderQ(); - expect(display.drawImage).to.not.have.been.called; - expect(display.fillRect).to.not.have.been.called; - expect(img.addEventListener).to.have.been.calledOnce; - - display._renderQ[0].img.complete = true; - display._renderQ[0].img.width = 4; - display._renderQ[0].img.height = 4; - display._scanRenderQ(); - expect(display.drawImage).to.have.been.calledOnce; - expect(display.fillRect).to.have.been.calledOnce; - expect(img.addEventListener).to.have.been.calledOnce; }); it('should call callback when queue is flushed', function () { From 7a6d80584f2f891948d2b770cf157fd4ca035827 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Fri, 24 Aug 2018 14:37:16 +0200 Subject: [PATCH 2/3] Use Uint8Array in Base64 decoding Otherwise each element isn't considered a single byte. This breaks conversion to other types, e.g. Blob. --- core/base64.js | 2 +- tests/test.base64.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/base64.js b/core/base64.js index db572c2db..a59945c22 100644 --- a/core/base64.js +++ b/core/base64.js @@ -62,7 +62,7 @@ export default { /* Every four characters is 3 resulting numbers */ const resultLength = (dataLength >> 2) * 3 + Math.floor((dataLength % 4) / 1.5); - const result = new Array(resultLength); + const result = new Uint8Array(resultLength); // Convert one by one. diff --git a/tests/test.base64.js b/tests/test.base64.js index 04bd207b7..365e74754 100644 --- a/tests/test.base64.js +++ b/tests/test.base64.js @@ -5,7 +5,7 @@ import Base64 from '../core/base64.js'; describe('Base64 Tools', function () { "use strict"; - const BIN_ARR = new Array(256); + const BIN_ARR = new Uint8Array(256); for (let i = 0; i < 256; i++) { BIN_ARR[i] = i; } From 8804b91480feaeaecde5bc04ad86f567db83e8be Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Thu, 23 Aug 2018 16:52:48 +0200 Subject: [PATCH 3/3] Use ImageBitmap when possible These are much faster than Image object in some browsers, so prefer them when possible. --- core/display.js | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/core/display.js b/core/display.js index 491cf99c1..e937f6cff 100644 --- a/core/display.js +++ b/core/display.js @@ -368,16 +368,25 @@ export default class Display { 'height': height }; - const img = new Image(); - img.src = "data: " + mime + ";base64," + Base64.encode(arr); - /* IE tends to set "complete" prematurely, so check dimensions */ - if (img.complete && (img.width !== 0) && (img.height !== 0)) { - a.img = img; + if (window.createImageBitmap) { + let blob = new Blob([arr], { type: mime }); + createImageBitmap(blob) + .then((img) => { + a.img = img; + this._scanRenderQ(); + }); } else { - img.addEventListener('load', () => { + const img = new Image(); + img.src = "data: " + mime + ";base64," + Base64.encode(arr); + /* IE tends to set "complete" prematurely, so check dimensions */ + if (img.complete && (img.width !== 0) && (img.height !== 0)) { a.img = img; - this._scanRenderQ(); - }); + } else { + img.addEventListener('load', () => { + a.img = img; + this._scanRenderQ(); + }); + } } this._renderQPush(a);