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.

6 files changed, 27 insertions, 120 deletions

diff --git a/core/decoders/tight.js b/core/decoders/tight.js
index 81335ab..e61f132 100644
--- a/core/decoders/tight.js
+++ b/core/decoders/tight.js
@@ -165,7 +165,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;
     }
@@ -237,9 +245,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;
                 }
             }
@@ -247,14 +255,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) {
@@ -263,13 +271,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/core/display.js b/core/display.js
index 378c6f1..f06aa37 100644
--- a/core/display.js
+++ b/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) {
         this._drawCtx.drawImage(img, x, y);
         this._damage(x, y, img.width, img.height);
@@ -487,19 +446,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;
@@ -513,19 +459,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) {
@@ -559,12 +492,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/core/util/browser.js b/core/util/browser.js
index 1554801..a18d5a4 100644
--- a/core/util/browser.js
+++ b/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/docs/API-internal.md b/docs/API-internal.md
index 1b5dfa9..4eec6e3 100644
--- a/docs/API-internal.md
+++ b/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, width, height, 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/tests/test.copyrect.js b/tests/test.copyrect.js
index 5a2b73b..fcf4219 100644
--- a/tests/test.copyrect.js
+++ b/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/tests/test.display.js b/tests/test.display.js
index 3492f1e..88c7607 100644
--- a/tests/test.display.js
+++ b/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 });