From ae1bc396bfcfb5a0f6e3153ef6f3421389d535b0 Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Sun, 7 Jun 2020 13:44:20 +0200 Subject: [PATCH] Standardise on a single blit function Keep everything simpler by always blitting in the same pixel format. It's up to the decoders to convert if they need to. --- kasmweb/core/decoders/tight.js | 26 +++++++----- kasmweb/core/display.js | 73 ---------------------------------- kasmweb/core/util/browser.js | 9 ----- kasmweb/docs/API-internal.md | 2 - kasmweb/tests/test.copyrect.js | 18 +++++---- kasmweb/tests/test.display.js | 19 --------- 6 files changed, 27 insertions(+), 120 deletions(-) diff --git a/kasmweb/core/decoders/tight.js b/kasmweb/core/decoders/tight.js index 7b4b1db..be9f1cc 100644 --- a/kasmweb/core/decoders/tight.js +++ b/kasmweb/core/decoders/tight.js @@ -179,7 +179,15 @@ export default class TightDecoder { this._zlibs[streamId].setInput(null); } - display.blitRgbImage(x, y, width, height, data, 0, false); + let bgrx = new Uint8Array(width * height * 4); + for (let i = 0, j = 0; i < width * height * 4; i += 4, j += 3) { + bgrx[i] = data[j + 2]; + bgrx[i + 1] = data[j + 1]; + bgrx[i + 2] = data[j]; + bgrx[i + 3] = 255; // Alpha + } + + display.blitImage(x, y, width, height, bgrx, 0, false); return true; } @@ -251,9 +259,9 @@ export default class TightDecoder { for (let b = 7; b >= 0; b--) { dp = (y * width + x * 8 + 7 - b) * 4; sp = (data[y * w + x] >> b & 1) * 3; - dest[dp] = palette[sp]; + dest[dp] = palette[sp + 2]; dest[dp + 1] = palette[sp + 1]; - dest[dp + 2] = palette[sp + 2]; + dest[dp + 2] = palette[sp]; dest[dp + 3] = 255; } } @@ -261,14 +269,14 @@ export default class TightDecoder { for (let b = 7; b >= 8 - width % 8; b--) { dp = (y * width + x * 8 + 7 - b) * 4; sp = (data[y * w + x] >> b & 1) * 3; - dest[dp] = palette[sp]; + dest[dp] = palette[sp + 2]; dest[dp + 1] = palette[sp + 1]; - dest[dp + 2] = palette[sp + 2]; + dest[dp + 2] = palette[sp]; dest[dp + 3] = 255; } } - display.blitRgbxImage(x, y, width, height, dest, 0, false); + display.blitImage(x, y, width, height, dest, 0, false); } _paletteRect(x, y, width, height, data, palette, display) { @@ -277,13 +285,13 @@ export default class TightDecoder { const total = width * height * 4; for (let i = 0, j = 0; i < total; i += 4, j++) { const sp = data[j] * 3; - dest[i] = palette[sp]; + dest[i] = palette[sp + 2]; dest[i + 1] = palette[sp + 1]; - dest[i + 2] = palette[sp + 2]; + dest[i + 2] = palette[sp]; dest[i + 3] = 255; } - display.blitRgbxImage(x, y, width, height, dest, 0, false); + display.blitImage(x, y, width, height, dest, 0, false); } _gradientFilter(streamId, x, y, width, height, sock, display, depth) { diff --git a/kasmweb/core/display.js b/kasmweb/core/display.js index f153cd8..332a880 100644 --- a/kasmweb/core/display.js +++ b/kasmweb/core/display.js @@ -8,7 +8,6 @@ import * as Log from './util/logging.js'; import Base64 from "./base64.js"; -import { supportsImageMetadata } from './util/browser.js'; export default class Display { constructor(target) { @@ -392,46 +391,6 @@ export default class Display { } } - blitRgbImage(x, y, width, height, arr, offset, fromQueue) { - if (this._renderQ.length !== 0 && !fromQueue) { - // NB(directxman12): it's technically more performant here to use preallocated arrays, - // but it's a lot of extra work for not a lot of payoff -- if we're using the render queue, - // this probably isn't getting called *nearly* as much - const newArr = new Uint8Array(width * height * 3); - newArr.set(new Uint8Array(arr.buffer, 0, newArr.length)); - this._renderQPush({ - 'type': 'blitRgb', - 'data': newArr, - 'x': x, - 'y': y, - 'width': width, - 'height': height, - }); - } else { - this._rgbImageData(x, y, width, height, arr, offset); - } - } - - blitRgbxImage(x, y, width, height, arr, offset, fromQueue) { - if (this._renderQ.length !== 0 && !fromQueue) { - // NB(directxman12): it's technically more performant here to use preallocated arrays, - // but it's a lot of extra work for not a lot of payoff -- if we're using the render queue, - // this probably isn't getting called *nearly* as much - const newArr = new Uint8Array(width * height * 4); - newArr.set(new Uint8Array(arr.buffer, 0, newArr.length)); - this._renderQPush({ - 'type': 'blitRgbx', - 'data': newArr, - 'x': x, - 'y': y, - 'width': width, - 'height': height, - }); - } else { - this._rgbxImageData(x, y, width, height, arr, offset); - } - } - drawImage(img, x, y, w, h) { if (img.width != w || img.height != h) { this._drawCtx.drawImage(img, x, y, w, h); @@ -491,19 +450,6 @@ export default class Display { } } - _rgbImageData(x, y, width, height, arr, offset) { - const img = this._drawCtx.createImageData(width, height); - const data = img.data; - for (let i = 0, j = offset; i < width * height * 4; i += 4, j += 3) { - data[i] = arr[j]; - data[i + 1] = arr[j + 1]; - data[i + 2] = arr[j + 2]; - data[i + 3] = 255; // Alpha - } - this._drawCtx.putImageData(img, x, y); - this._damage(x, y, img.width, img.height); - } - _bgrxImageData(x, y, width, height, arr, offset) { const img = this._drawCtx.createImageData(width, height); const data = img.data; @@ -517,19 +463,6 @@ export default class Display { this._damage(x, y, img.width, img.height); } - _rgbxImageData(x, y, width, height, arr, offset) { - // NB(directxman12): arr must be an Type Array view - let img; - if (supportsImageMetadata) { - img = new ImageData(new Uint8ClampedArray(arr.buffer, arr.byteOffset, width * height * 4), width, height); - } else { - img = this._drawCtx.createImageData(width, height); - img.data.set(new Uint8ClampedArray(arr.buffer, arr.byteOffset, width * height * 4)); - } - this._drawCtx.putImageData(img, x, y); - this._damage(x, y, img.width, img.height); - } - _renderQPush(action) { this._renderQ.push(action); if (this._renderQ.length === 1) { @@ -563,12 +496,6 @@ export default class Display { case 'blit': this.blitImage(a.x, a.y, a.width, a.height, a.data, 0, true); break; - case 'blitRgb': - this.blitRgbImage(a.x, a.y, a.width, a.height, a.data, 0, true); - break; - case 'blitRgbx': - this.blitRgbxImage(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)) { diff --git a/kasmweb/core/util/browser.js b/kasmweb/core/util/browser.js index 1554801..a18d5a4 100644 --- a/kasmweb/core/util/browser.js +++ b/kasmweb/core/util/browser.js @@ -45,15 +45,6 @@ try { export const supportsCursorURIs = _supportsCursorURIs; -let _supportsImageMetadata = false; -try { - new ImageData(new Uint8ClampedArray(4), 1, 1); - _supportsImageMetadata = true; -} catch (ex) { - // ignore failure -} -export const supportsImageMetadata = _supportsImageMetadata; - let _hasScrollbarGutter = true; try { // Create invisible container diff --git a/kasmweb/docs/API-internal.md b/kasmweb/docs/API-internal.md index 0e1b539..07fe20d 100644 --- a/kasmweb/docs/API-internal.md +++ b/kasmweb/docs/API-internal.md @@ -105,8 +105,6 @@ None | copyImage | (old_x, old_y, new_x, new_y, width, height, from_queue) | Copy a rectangular area | imageRect | (x, y, mime, arr) | Draw a rectangle with an image | blitImage | (x, y, width, height, arr, offset, from_queue) | Blit pixels (of R,G,B,A) to the display -| blitRgbImage | (x, y, width, height, arr, offset, from_queue) | Blit RGB encoded image to display -| blitRgbxImage | (x, y, width, height, arr, offset, from_queue) | Blit RGBX encoded image to display | drawImage | (img, x, y) | Draw image and track damage | autoscale | (containerWidth, containerHeight) | Scale the display diff --git a/kasmweb/tests/test.copyrect.js b/kasmweb/tests/test.copyrect.js index 5a2b73b..fcf4219 100644 --- a/kasmweb/tests/test.copyrect.js +++ b/kasmweb/tests/test.copyrect.js @@ -36,15 +36,10 @@ describe('CopyRect Decoder', function () { }); it('should handle the CopyRect encoding', function () { - let targetData = new Uint8Array([ - 0x00, 0x00, 0xff, 255, 0x00, 0x00, 0xff, 255, 0x00, 0xff, 0x00, 255, 0x00, 0xff, 0x00, 255, - 0x00, 0x00, 0xff, 255, 0x00, 0x00, 0xff, 255, 0x00, 0xff, 0x00, 255, 0x00, 0xff, 0x00, 255, - 0x00, 0xff, 0x00, 255, 0x00, 0xff, 0x00, 255, 0x00, 0x00, 0xff, 255, 0x00, 0x00, 0xff, 255, - 0x00, 0xff, 0x00, 255, 0x00, 0xff, 0x00, 255, 0x00, 0x00, 0xff, 255, 0x00, 0x00, 0xff, 255 - ]); - // seed some initial data to copy - display.blitRgbxImage(0, 0, 4, 2, new Uint8Array(targetData.slice(0, 32)), 0); + display.fillRect(0, 0, 4, 4, [ 0x11, 0x22, 0x33 ]); + display.fillRect(0, 0, 2, 2, [ 0xff, 0x00, 0x00 ]); + display.fillRect(2, 0, 2, 2, [ 0x00, 0xff, 0x00 ]); testDecodeRect(decoder, 0, 2, 2, 2, [0x00, 0x02, 0x00, 0x00], @@ -53,6 +48,13 @@ describe('CopyRect Decoder', function () { [0x00, 0x00, 0x00, 0x00], display, 24); + let targetData = new Uint8Array([ + 0x00, 0x00, 0xff, 255, 0x00, 0x00, 0xff, 255, 0x00, 0xff, 0x00, 255, 0x00, 0xff, 0x00, 255, + 0x00, 0x00, 0xff, 255, 0x00, 0x00, 0xff, 255, 0x00, 0xff, 0x00, 255, 0x00, 0xff, 0x00, 255, + 0x00, 0xff, 0x00, 255, 0x00, 0xff, 0x00, 255, 0x00, 0x00, 0xff, 255, 0x00, 0x00, 0xff, 255, + 0x00, 0xff, 0x00, 255, 0x00, 0xff, 0x00, 255, 0x00, 0x00, 0xff, 255, 0x00, 0x00, 0xff, 255 + ]); + expect(display).to.have.displayed(targetData); }); }); diff --git a/kasmweb/tests/test.display.js b/kasmweb/tests/test.display.js index 3492f1e..88c7607 100644 --- a/kasmweb/tests/test.display.js +++ b/kasmweb/tests/test.display.js @@ -322,18 +322,6 @@ describe('Display/Canvas Helper', function () { expect(display).to.have.displayed(checkedData); }); - it('should support drawing RGB blit images with true color via #blitRgbImage', function () { - const data = []; - for (let i = 0; i < 16; i++) { - data[i * 3] = checkedData[i * 4]; - data[i * 3 + 1] = checkedData[i * 4 + 1]; - data[i * 3 + 2] = checkedData[i * 4 + 2]; - } - display.blitRgbImage(0, 0, 4, 4, data, 0); - display.flip(); - expect(display).to.have.displayed(checkedData); - }); - it('should support drawing an image object via #drawImage', function () { const img = makeImageCanvas(checkedData); display.drawImage(img, 0, 0); @@ -416,13 +404,6 @@ describe('Display/Canvas Helper', function () { expect(display.blitImage).to.have.been.calledWith(3, 4, 5, 6, [7, 8, 9], 0); }); - it('should draw a blit RGB image on type "blitRgb"', function () { - display.blitRgbImage = sinon.spy(); - display._renderQPush({ type: 'blitRgb', x: 3, y: 4, width: 5, height: 6, data: [7, 8, 9] }); - expect(display.blitRgbImage).to.have.been.calledOnce; - expect(display.blitRgbImage).to.have.been.calledWith(3, 4, 5, 6, [7, 8, 9], 0); - }); - it('should copy a region on type "copy"', function () { display.copyImage = sinon.spy(); display._renderQPush({ type: 'copy', x: 3, y: 4, width: 5, height: 6, oldX: 7, oldY: 8 });