Improve screenshot pixel flipping with contiguous row copies

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;
   },

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);
+      });
+    });
+  });
 });