From 9bd1bc591562d98166432b01d42e1d37902e20b6 Mon Sep 17 00:00:00 2001 From: "Michael \"Z\" Goddard" Date: Mon, 4 Feb 2019 12:43:31 -0500 Subject: [PATCH 1/3] load-costume: use one canvas pool --- src/import/load-costume.js | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/import/load-costume.js b/src/import/load-costume.js index e2ce43345..794ed7ed2 100644 --- a/src/import/load-costume.js +++ b/src/import/load-costume.js @@ -32,6 +32,26 @@ const loadVector_ = function (costume, runtime, rotationCenter, optVersion) { }); }; +const getCanvas = (function () { + const _canvases = []; + let clearSoon = null; + + return Object.assign(() => ( + _canvases.pop() || document.createElement('canvas') + ), { + release (canvas) { + if (!clearSoon) { + clearSoon = new Promise(resolve => setTimeout(resolve, 1000)) + .then(() => { + _canvases.length = 0; + clearSoon = null; + }); + } + _canvases.push(canvas); + } + }); +}()); + /** * 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) @@ -100,7 +120,7 @@ const fetchBitmapCanvas_ = function (costume, runtime, rotationCenter) { }).then(imageElements => { const [baseImageElement, textImageElement] = imageElements; - let canvas = document.createElement('canvas'); + let canvas = getCanvas(); const scale = costume.bitmapResolution === 1 ? 2 : 1; canvas.width = baseImageElement.width; canvas.height = baseImageElement.height; @@ -171,6 +191,7 @@ const loadBitmap_ = function (costume, runtime, rotationCenter) { .then(canvas => { // createBitmapSkin does the right thing if costume.bitmapResolution or rotationCenter are undefined... costume.skinId = runtime.renderer.createBitmapSkin(canvas, costume.bitmapResolution, rotationCenter); + getCanvas.release(canvas); const renderSize = runtime.renderer.getSkinSize(costume.skinId); costume.size = [renderSize[0] * 2, renderSize[1] * 2]; // Actual size, since all bitmaps are resolution 2 From c01175613bf2684f704b37ffd28aae75cf9dc53a Mon Sep 17 00:00:00 2001 From: "Michael \"Z\" Goddard" Date: Mon, 4 Feb 2019 12:45:07 -0500 Subject: [PATCH 2/3] load-costume: idiomatic promise and createImageBitmap --- src/import/load-costume.js | 186 ++++++++++++++++++------------------- 1 file changed, 88 insertions(+), 98 deletions(-) diff --git a/src/import/load-costume.js b/src/import/load-costume.js index 794ed7ed2..7662821ee 100644 --- a/src/import/load-costume.js +++ b/src/import/load-costume.js @@ -74,86 +74,70 @@ const fetchBitmapCanvas_ = function (costume, runtime, rotationCenter) { return Promise.reject('No V2 Bitmap adapter present.'); } - return new Promise((resolve, reject) => { - const baseImageElement = new Image(); - let textImageElement; + return Promise.all([costume.asset, costume.textLayerAsset].map(asset => { + if (!asset) { + return null; + } - // We need to wait for 2 images total to load. loadedOne will be true when one - // is done, and we are just waiting for one more. - let loadedOne = false; + if (typeof createImageBitmap !== 'undefined') { + return createImageBitmap( + new Blob([asset.data], {type: asset.assetType.contentType}) + ); + } - const onError = function () { - // eslint-disable-next-line no-use-before-define - removeEventListeners(); - reject('Costume load failed. Asset could not be read.'); - }; - const onLoad = function () { - if (loadedOne) { - // eslint-disable-next-line no-use-before-define - removeEventListeners(); - resolve([baseImageElement, textImageElement]); - } else { - loadedOne = true; - } - }; + return new Promise((resolve, reject) => { + const image = new Image(); + image.onload = function () { + resolve(image); + image.onload = null; + image.onerror = null; + }; + image.onerror = function () { + reject('Costume load failed. Asset could not be read.'); + image.onload = null; + image.onerror = null; + }; + image.src = asset.encodeDataURI(); + }); + })) + .then(([baseImageElement, textImageElement]) => { + const mergeCanvas = getCanvas(); - const removeEventListeners = function () { - baseImageElement.removeEventListener('error', onError); - baseImageElement.removeEventListener('load', onLoad); + const scale = costume.bitmapResolution === 1 ? 2 : 1; + mergeCanvas.width = baseImageElement.width; + mergeCanvas.height = baseImageElement.height; + + const ctx = mergeCanvas.getContext('2d'); + ctx.drawImage(baseImageElement, 0, 0); if (textImageElement) { - textImageElement.removeEventListener('error', onError); - textImageElement.removeEventListener('load', onLoad); + ctx.drawImage(textImageElement, 0, 0); + } + let canvas = mergeCanvas; + if (scale !== 1) { + canvas = runtime.v2BitmapAdapter.resize(mergeCanvas, canvas.width * scale, canvas.height * scale); } - }; - 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; + // By scaling, we've converted it to bitmap resolution 2 + if (rotationCenter) { + rotationCenter[0] = rotationCenter[0] * scale; + rotationCenter[1] = rotationCenter[1] * scale; + costume.rotationCenterX = rotationCenter[0]; + costume.rotationCenterY = rotationCenter[1]; + } + costume.bitmapResolution = 2; - let canvas = getCanvas(); - const scale = costume.bitmapResolution === 1 ? 2 : 1; - canvas.width = baseImageElement.width; - canvas.height = baseImageElement.height; + // Clean up the costume object + delete costume.textLayerMD5; + delete costume.textLayerAsset; - const ctx = canvas.getContext('2d'); - ctx.drawImage(baseImageElement, 0, 0); - if (textImageElement) { - ctx.drawImage(textImageElement, 0, 0); - } - if (scale !== 1) { - canvas = runtime.v2BitmapAdapter.resize(canvas, canvas.width * scale, canvas.height * scale); - } - - // By scaling, we've converted it to bitmap resolution 2 - if (rotationCenter) { - rotationCenter[0] = rotationCenter[0] * scale; - rotationCenter[1] = rotationCenter[1] * scale; - costume.rotationCenterX = rotationCenter[0]; - costume.rotationCenterY = rotationCenter[1]; - } - costume.bitmapResolution = 2; - - // Clean up the costume object - delete costume.textLayerMD5; - delete costume.textLayerAsset; - - return { - canvas: canvas, - rotationCenter: rotationCenter, - // True if the asset matches the base layer; false if it required adjustment - assetMatchesBase: scale === 1 && !textImageElement - }; - }) + return { + canvas, + mergeCanvas, + rotationCenter, + // True if the asset matches the base layer; false if it required adjustment + assetMatchesBase: scale === 1 && !textImageElement + }; + }) .catch(() => { // Clean up the text layer properties if it fails to load delete costume.textLayerMD5; @@ -161,37 +145,43 @@ const fetchBitmapCanvas_ = function (costume, runtime, rotationCenter) { }); }; -const loadBitmap_ = function (costume, runtime, rotationCenter) { - return fetchBitmapCanvas_(costume, runtime, rotationCenter).then(fetched => new Promise(resolve => { - rotationCenter = fetched.rotationCenter; +const loadBitmap_ = function (costume, runtime, _rotationCenter) { + return fetchBitmapCanvas_(costume, runtime, _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.'); + } - const updateCostumeAsset = function (dataURI) { - if (!runtime.v2BitmapAdapter) { - return Promise.reject('No V2 Bitmap adapter present.'); + const storage = runtime.storage; + costume.asset = storage.createAsset( + storage.AssetType.ImageBitmap, + storage.DataFormat.PNG, + runtime.v2BitmapAdapter.convertDataURIToBinary(dataURI), + null, + true // generate md5 + ); + costume.dataFormat = storage.DataFormat.PNG; + costume.assetId = costume.asset.assetId; + costume.md5 = `${costume.assetId}.${costume.dataFormat}`; + }; + + if (!fetched.assetMatchesBase) { + updateCostumeAsset(fetched.canvas.toDataURL()); } - const storage = runtime.storage; - costume.asset = storage.createAsset( - storage.AssetType.ImageBitmap, - storage.DataFormat.PNG, - runtime.v2BitmapAdapter.convertDataURIToBinary(dataURI), - null, - true // generate md5 - ); - costume.dataFormat = storage.DataFormat.PNG; - costume.assetId = costume.asset.assetId; - costume.md5 = `${costume.assetId}.${costume.dataFormat}`; - }; - - if (!fetched.assetMatchesBase) { - updateCostumeAsset(fetched.canvas.toDataURL()); - } - resolve(fetched.canvas); - })) - .then(canvas => { + return fetched; + }) + .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); - getCanvas.release(canvas); + getCanvas.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 From d05a414439dfa7e8422ed9c8fbd52df50efa2f76 Mon Sep 17 00:00:00 2001 From: "Michael \"Z\" Goddard" Date: Mon, 4 Feb 2019 13:38:46 -0500 Subject: [PATCH 3/3] turn getCanvas into a class CanvasPool --- src/import/load-costume.js | 63 +++++++++++++++++++++++++++++--------- 1 file changed, 48 insertions(+), 15 deletions(-) diff --git a/src/import/load-costume.js b/src/import/load-costume.js index 7662821ee..a8da72a48 100644 --- a/src/import/load-costume.js +++ b/src/import/load-costume.js @@ -32,24 +32,51 @@ const loadVector_ = function (costume, runtime, rotationCenter, optVersion) { }); }; -const getCanvas = (function () { - const _canvases = []; - let clearSoon = null; +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; + } - return Object.assign(() => ( - _canvases.pop() || document.createElement('canvas') - ), { - release (canvas) { - if (!clearSoon) { - clearSoon = new Promise(resolve => setTimeout(resolve, 1000)) + /** + * 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(() => { - _canvases.length = 0; - clearSoon = null; + this.pool.length = 0; + this.clearSoon = null; }); } - _canvases.push(canvas); } - }); + + /** + * 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(); }()); /** @@ -101,7 +128,7 @@ const fetchBitmapCanvas_ = function (costume, runtime, rotationCenter) { }); })) .then(([baseImageElement, textImageElement]) => { - const mergeCanvas = getCanvas(); + const mergeCanvas = canvasPool.create(); const scale = costume.bitmapResolution === 1 ? 2 : 1; mergeCanvas.width = baseImageElement.width; @@ -112,6 +139,12 @@ const fetchBitmapCanvas_ = function (costume, runtime, rotationCenter) { 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(mergeCanvas, canvas.width * scale, canvas.height * scale); @@ -181,7 +214,7 @@ const loadBitmap_ = function (costume, runtime, _rotationCenter) { .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); - getCanvas.release(mergeCanvas); + 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