Merge pull request #1970 from mzgoddard/image-bitmap

Use Promise.all, createImageBitmap, and one canvas in `load-costume.js`

src/import/load-costume.js

@@ -32,6 +32,53 @@ const loadVector_ = function (costume, runtime, rotationCenter, optVersion) {
     });
 };
 
+const canvasPool = (function () {
+    /**
+     * A pool of canvas objects that can be reused to reduce memory
+     * allocations. And time spent in those allocations and the later garbage
+     * collection.
+     */
+    class CanvasPool {
+        constructor () {
+            this.pool = [];
+            this.clearSoon = null;
+        }
+
+        /**
+         * After a short wait period clear the pool to let the VM collect
+         * garbage.
+         */
+        clear () {
+            if (!this.clearSoon) {
+                this.clearSoon = new Promise(resolve => setTimeout(resolve, 1000))
+                    .then(() => {
+                        this.pool.length = 0;
+                        this.clearSoon = null;
+                    });
+            }
+        }
+
+        /**
+         * Return a canvas. Create the canvas if the pool is empty.
+         * @returns {HTMLCanvasElement} A canvas element.
+         */
+        create () {
+            return this.pool.pop() || document.createElement('canvas');
+        }
+
+        /**
+         * Release the canvas to be reused.
+         * @param {HTMLCanvasElement} canvas A canvas element.
+         */
+        release (canvas) {
+            this.clear();
+            this.pool.push(canvas);
+        }
+    }
+
+    return new CanvasPool();
+}());
+
 /**
  * Return a promise to fetch a bitmap from storage and return it as a canvas
  * If the costume has bitmapResolution 1, it will be converted to bitmapResolution 2 here (the standard for Scratch 3)
@@ -54,64 +101,53 @@ const fetchBitmapCanvas_ = function (costume, runtime, rotationCenter) {
         return Promise.reject('No V2 Bitmap adapter present.');
     }
 
+    return Promise.all([costume.asset, costume.textLayerAsset].map(asset => {
+        if (!asset) {
+            return null;
+        }
+
+        if (typeof createImageBitmap !== 'undefined') {
+            return createImageBitmap(
+                new Blob([asset.data], {type: asset.assetType.contentType})
+            );
+        }
+
         return new Promise((resolve, reject) => {
-        const baseImageElement = new Image();
+            const image = new Image();
-        let textImageElement;
+            image.onload = function () {
-
+                resolve(image);
-        // We need to wait for 2 images total to load. loadedOne will be true when one
+                image.onload = null;
-        // is done, and we are just waiting for one more.
+                image.onerror = null;
-        let loadedOne = false;
+            };
-
+            image.onerror = function () {
-        const onError = function () {
-            // eslint-disable-next-line no-use-before-define
-            removeEventListeners();
                 reject('Costume load failed. Asset could not be read.');
+                image.onload = null;
+                image.onerror = null;
             };
-        const onLoad = function () {
+            image.src = asset.encodeDataURI();
-            if (loadedOne) {
+        });
-                // eslint-disable-next-line no-use-before-define
+    }))
-                removeEventListeners();
+        .then(([baseImageElement, textImageElement]) => {
-                resolve([baseImageElement, textImageElement]);
+            const mergeCanvas = canvasPool.create();
-            } else {
-                loadedOne = true;
-            }
-        };
 
-        const removeEventListeners = function () {
-            baseImageElement.removeEventListener('error', onError);
-            baseImageElement.removeEventListener('load', onLoad);
-            if (textImageElement) {
-                textImageElement.removeEventListener('error', onError);
-                textImageElement.removeEventListener('load', onLoad);
-            }
-        };
-
-        baseImageElement.addEventListener('load', onLoad);
-        baseImageElement.addEventListener('error', onError);
-        if (costume.textLayerAsset) {
-            textImageElement = new Image();
-            textImageElement.addEventListener('load', onLoad);
-            textImageElement.addEventListener('error', onError);
-            textImageElement.src = costume.textLayerAsset.encodeDataURI();
-        } else {
-            loadedOne = true;
-        }
-        baseImageElement.src = costume.asset.encodeDataURI();
-    }).then(imageElements => {
-        const [baseImageElement, textImageElement] = imageElements;
-
-        let canvas = document.createElement('canvas');
             const scale = costume.bitmapResolution === 1 ? 2 : 1;
-        canvas.width = baseImageElement.width;
+            mergeCanvas.width = baseImageElement.width;
-        canvas.height = baseImageElement.height;
+            mergeCanvas.height = baseImageElement.height;
 
-        const ctx = canvas.getContext('2d');
+            const ctx = mergeCanvas.getContext('2d');
             ctx.drawImage(baseImageElement, 0, 0);
             if (textImageElement) {
                 ctx.drawImage(textImageElement, 0, 0);
             }
+            // Track the canvas we merged the bitmaps onto separately from the
+            // canvas that we receive from resize if scale is not 1. We know
+            // resize treats mergeCanvas as read only data. We don't know when
+            // resize may use or modify the canvas. So we'll only release the
+            // mergeCanvas back into the canvas pool. Reusing the canvas from
+            // resize may cause errors.
+            let canvas = mergeCanvas;
             if (scale !== 1) {
-            canvas = runtime.v2BitmapAdapter.resize(canvas, canvas.width * scale, canvas.height * scale);
+                canvas = runtime.v2BitmapAdapter.resize(mergeCanvas, canvas.width * scale, canvas.height * scale);
             }
 
             // By scaling, we've converted it to bitmap resolution 2
@@ -128,8 +164,9 @@ const fetchBitmapCanvas_ = function (costume, runtime, rotationCenter) {
             delete costume.textLayerAsset;
 
             return {
-            canvas: canvas,
+                canvas,
-            rotationCenter: rotationCenter,
+                mergeCanvas,
+                rotationCenter,
                 // True if the asset matches the base layer; false if it required adjustment
                 assetMatchesBase: scale === 1 && !textImageElement
             };
@@ -141,12 +178,17 @@ const fetchBitmapCanvas_ = function (costume, runtime, rotationCenter) {
         });
 };
 
-const loadBitmap_ = function (costume, runtime, rotationCenter) {
+const loadBitmap_ = function (costume, runtime, _rotationCenter) {
-    return fetchBitmapCanvas_(costume, runtime, rotationCenter).then(fetched => new Promise(resolve => {
+    return fetchBitmapCanvas_(costume, runtime, _rotationCenter)
-        rotationCenter = fetched.rotationCenter;
+        .then(fetched => {
-
             const updateCostumeAsset = function (dataURI) {
                 if (!runtime.v2BitmapAdapter) {
+                    // TODO: This might be a bad practice since the returned
+                    // promise isn't acted on. If this is something we should be
+                    // creating a rejected promise for we should also catch it
+                    // somewhere and act on that error (like logging).
+                    //
+                    // Return a rejection to stop executing updateCostumeAsset.
                     return Promise.reject('No V2 Bitmap adapter present.');
                 }
 
@@ -166,11 +208,13 @@ const loadBitmap_ = function (costume, runtime, rotationCenter) {
             if (!fetched.assetMatchesBase) {
                 updateCostumeAsset(fetched.canvas.toDataURL());
             }
-        resolve(fetched.canvas);
+
-    }))
+            return fetched;
-        .then(canvas => {
+        })
+        .then(({canvas, mergeCanvas, rotationCenter}) => {
             // createBitmapSkin does the right thing if costume.bitmapResolution or rotationCenter are undefined...
             costume.skinId = runtime.renderer.createBitmapSkin(canvas, costume.bitmapResolution, rotationCenter);
+            canvasPool.release(mergeCanvas);
             const renderSize = runtime.renderer.getSkinSize(costume.skinId);
             costume.size = [renderSize[0] * 2, renderSize[1] * 2]; // Actual size, since all bitmaps are resolution 2