diff --git a/src/components/scene/screenshot.js b/src/components/scene/screenshot.js index b8edc2c84ca..984935ba5fa 100644 --- a/src/components/scene/screenshot.js +++ b/src/components/scene/screenshot.js @@ -236,16 +236,12 @@ export var Component = registerComponent('screenshot', { }, flipPixelsVertically: function (pixels, width, height) { - var flippedPixels = pixels.slice(0); - for (var x = 0; x < width; ++x) { - for (var y = 0; y < height; ++y) { - var from = x * 4 + (height - y - 1) * width * 4; - var to = x * 4 + y * width * 4; - flippedPixels[to] = pixels[from]; - flippedPixels[to + 1] = pixels[from + 1]; - flippedPixels[to + 2] = pixels[from + 2]; - flippedPixels[to + 3] = pixels[from + 3]; - } + var flippedPixels = new Uint8Array(pixels.length); + var rowSize = width * 4; + // Copy contiguous rows to avoid cache misses from traversing columns. + for (var y = 0; y < height; ++y) { + var from = (height - y - 1) * rowSize; + flippedPixels.set(pixels.subarray(from, from + rowSize), y * rowSize); } return flippedPixels; }, diff --git a/tests/components/scene/screenshot.test.js b/tests/components/scene/screenshot.test.js index 6cb0548a343..8765734aaea 100644 --- a/tests/components/scene/screenshot.test.js +++ b/tests/components/scene/screenshot.test.js @@ -1,4 +1,4 @@ -/* global THREE, assert, setup, suite, test */ +/* global AFRAME, THREE, assert, setup, suite, test */ suite('screenshot', function () { var component; var sceneEl; @@ -21,7 +21,7 @@ suite('screenshot', function () { // variation of colorManagement parameter, which can only be set on screen creation. }); - test('capture is called when key shortcut is pressed', function () { + test('capture is called when key shortcut is pressed', function (done) { sceneEl.addEventListener('loaded', () => { component = sceneEl.components.screenshot; var captureStub = this.sinon.stub(component, 'capture'); @@ -33,26 +33,56 @@ suite('screenshot', function () { ctrlKey: true }); assert.ok(captureStub.called); + done(); }); document.body.appendChild(sceneEl); }); - test('capture renders screenshot correctly (w/o Color Management)', function () { + test('capture renders screenshot correctly (w/o Color Management)', function (done) { sceneEl.setAttribute('renderer', 'colorManagement: false'); sceneEl.addEventListener('loaded', () => { component = sceneEl.components.screenshot; const renderTarget = component.getRenderTarget(); checkRenderTarget(renderTarget, THREE.LinearSRGBColorSpace); + renderTarget.dispose(); + done(); }); document.body.appendChild(sceneEl); }); - test('capture renders screenshot correctly (w/ Color Management)', function () { + test('capture renders screenshot correctly (w/ Color Management)', function (done) { sceneEl.addEventListener('loaded', () => { component = sceneEl.components.screenshot; const renderTarget = component.getRenderTarget(); checkRenderTarget(renderTarget, THREE.SRGBColorSpace); + renderTarget.dispose(); + done(); }); document.body.appendChild(sceneEl); }); + + suite('flipPixelsVertically', function () { + [ + {width: 3, height: 2}, + {width: 2, height: 3} + ].forEach(function (size) { + test('flips RGBA rows for ' + size.width + 'x' + size.height, function () { + var pixels = new Uint8Array(size.width * size.height * 4); + for (var i = 0; i < pixels.length; i++) { pixels[i] = i; } + var original = pixels.slice(); + var flip = AFRAME.components.screenshot.Component.prototype.flipPixelsVertically; + var flipped = flip(pixels, size.width, size.height); + for (var y = 0; y < size.height; y++) { + for (var x = 0; x < size.width; x++) { + for (var channel = 0; channel < 4; channel++) { + assert.equal(flipped[(y * size.width + x) * 4 + channel], + pixels[((size.height - y - 1) * size.width + x) * 4 + channel]); + } + } + } + assert.deepEqual(pixels, original, 'source pixels are unchanged'); + assert.notStrictEqual(flipped.buffer, pixels.buffer); + }); + }); + }); });