From 801b0dab1fbd55e19e6b509a8574001cd70bb794 Mon Sep 17 00:00:00 2001 From: adroitwhiz Date: Thu, 7 Jan 2021 20:48:13 -0500 Subject: [PATCH 1/3] Move SVG renderer logic back into SVGSkin This will allow for fancier stuff to be done with the SVG viewbox in the future to avoid subpixel jitter. --- src/SVGSkin.js | 68 ++++++++++++++++++++++++++++------------ test/helper/page-util.js | 4 +-- 2 files changed, 50 insertions(+), 22 deletions(-) diff --git a/src/SVGSkin.js b/src/SVGSkin.js index bf1aeca88..40fe208dd 100644 --- a/src/SVGSkin.js +++ b/src/SVGSkin.js @@ -1,7 +1,7 @@ const twgl = require('twgl.js'); const Skin = require('./Skin'); -const SvgRenderer = require('scratch-svg-renderer').SVGRenderer; +const {loadSvgString, serializeSvgToString} = require('scratch-svg-renderer'); const ShaderManager = require('./ShaderManager'); const MAX_TEXTURE_DIMENSION = 2048; @@ -28,8 +28,17 @@ class SVGSkin extends Skin { /** @type {RenderWebGL} */ this._renderer = renderer; - /** @type {SvgRenderer} */ - this._svgRenderer = new SvgRenderer(); + /** @type {HTMLImageElement} */ + this._svgImage = null; + + /** @type {Array} */ + this._size = [0, 0]; + + /** @type {HTMLCanvasElement} */ + this._canvas = document.createElement('canvas'); + + /** @type {CanvasRenderingContext2D} */ + this._context = this._canvas.getContext('2d'); /** @type {Array} */ this._scaledMIPs = []; @@ -56,7 +65,7 @@ class SVGSkin extends Skin { * @return {Array} the natural size, in Scratch units, of this skin. */ get size () { - return this._svgRenderer.size; + return [this._size[0], this._size[1]]; } useNearest (scale, drawable) { @@ -94,18 +103,27 @@ class SVGSkin extends Skin { * @return {SVGMIP} An object that handles creating and updating SVG textures. */ createMIP (scale) { - this._svgRenderer.draw(scale); + const [width, height] = this._size; + this._canvas.width = width * scale; + this._canvas.height = height * scale; + if ( + this._canvas.width <= 0 || + this._canvas.height <= 0 || + // Even if the canvas at the current scale has a nonzero size, the image's dimensions are floored + // pre-scaling; e.g. if an image has a width of 0.4 and is being rendered at 3x scale, the canvas will have + // a width of 1, but the image's width will be rounded down to 0 on some browsers (Firefox) prior to being + // drawn at that scale, resulting in an IndexSizeError if we attempt to draw it. + this._svgImage.naturalWidth <= 0 || + this._svgImage.naturalHeight <= 0 + ) return super.getTexture(); + this._context.clearRect(0, 0, this._canvas.width, this._canvas.height); + this._context.setTransform(scale, 0, 0, scale, 0, 0); + this._context.drawImage(this._svgImage, 0, 0); // Pull out the ImageData from the canvas. ImageData speeds up // updating Silhouette and is better handled by more browsers in // regards to memory. - const canvas = this._svgRenderer.canvas; - // If one of the canvas dimensions is 0, set this MIP to an empty image texture. - // This avoids an IndexSizeError from attempting to getImageData when one of the dimensions is 0. - if (canvas.width === 0 || canvas.height === 0) return super.getTexture(); - - const context = canvas.getContext('2d'); - const textureData = context.getImageData(0, 0, canvas.width, canvas.height); + const textureData = this._context.getImageData(0, 0, this._canvas.width, this._canvas.height); const textureOptions = { auto: false, @@ -147,7 +165,7 @@ class SVGSkin extends Skin { // Can't use bitwise stuff here because we need to handle negative exponents const mipScale = Math.pow(2, mipLevel - INDEX_OFFSET); - if (this._svgRenderer.loaded && !this._scaledMIPs[mipLevel]) { + if (this._svgImage.complete && !this._scaledMIPs[mipLevel]) { this._scaledMIPs[mipLevel] = this.createMIP(mipScale); } @@ -172,14 +190,21 @@ class SVGSkin extends Skin { * @param {Array} [rotationCenter] - Optional rotation center for the SVG. */ setSVG (svgData, rotationCenter) { - this._svgRenderer.loadSVG(svgData, false, () => { - const svgSize = this._svgRenderer.size; - if (svgSize[0] === 0 || svgSize[1] === 0) { + const svgTag = loadSvgString(svgData); + this._svgImage = document.createElement('img'); + const svgText = serializeSvgToString(svgTag, true /* shouldInjectFonts */); + + this._svgImage.addEventListener('load', () => { + const {x, y, width, height} = svgTag.viewBox.baseVal; + this._size[0] = width; + this._size[1] = height; + + if (width === 0 || height === 0) { super.setEmptyImageData(); return; } - const maxDimension = Math.ceil(Math.max(this.size[0], this.size[1])); + const maxDimension = Math.ceil(Math.max(width, height)); let testScale = 2; for (testScale; maxDimension * testScale <= MAX_TEXTURE_DIMENSION; testScale *= 2) { this._maxTextureScale = testScale; @@ -188,12 +213,15 @@ class SVGSkin extends Skin { this.resetMIPs(); if (typeof rotationCenter === 'undefined') rotationCenter = this.calculateRotationCenter(); - const viewOffset = this._svgRenderer.viewOffset; - this._rotationCenter[0] = rotationCenter[0] - viewOffset[0]; - this._rotationCenter[1] = rotationCenter[1] - viewOffset[1]; + // Compensate for viewbox offset. + // See https://github.com/LLK/scratch-render/pull/90. + this._rotationCenter[0] = rotationCenter[0] - x; + this._rotationCenter[1] = rotationCenter[1] - y; this.emit(Skin.Events.WasAltered); }); + + this._svgImage.src = `data:image/svg+xml;utf8,${encodeURIComponent(svgText)}`; } } diff --git a/test/helper/page-util.js b/test/helper/page-util.js index 36277a7e5..e35a0f602 100644 --- a/test/helper/page-util.js +++ b/test/helper/page-util.js @@ -14,7 +14,7 @@ window.waitForSVGSkinLoad = renderer => new Promise(resolve => { for (const skin of renderer._allSkins) { if (skin.constructor.name !== 'SVGSkin') continue; numSVGSkins++; - if (skin._svgRenderer.loaded) numLoadedSVGSkins++; + if (skin._svgImage.complete) numLoadedSVGSkins++; } if (numSVGSkins === numLoadedSVGSkins) { @@ -47,7 +47,7 @@ window.initVM = render => { vm.attachStorage(storage); vm.attachRenderer(render); - vm.attachV2SVGAdapter(new ScratchSVGRenderer.SVGRenderer()); + vm.attachV2SVGAdapter(ScratchSVGRenderer.V2SVGAdapter); vm.attachV2BitmapAdapter(new ScratchSVGRenderer.BitmapAdapter()); return vm; From 7af5b3e2074d054b7e6903fb51fefff4913d8fbb Mon Sep 17 00:00:00 2001 From: adroitwhiz Date: Tue, 1 Jun 2021 18:01:24 -0400 Subject: [PATCH 2/3] Use one SVG image and track loaded state w/ flag Instead of creating a new image every time `setSVG` is called and checking whether that image is complete with `loaded`, reuse the same image, override its `src`, & use `onload` instead of `addEventListener` to replace the previous event listener. This is necessary to avoid race conditions with the `_svgImageLoaded` flag. --- src/SVGSkin.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/SVGSkin.js b/src/SVGSkin.js index 40fe208dd..256a784f9 100644 --- a/src/SVGSkin.js +++ b/src/SVGSkin.js @@ -29,7 +29,10 @@ class SVGSkin extends Skin { this._renderer = renderer; /** @type {HTMLImageElement} */ - this._svgImage = null; + this._svgImage = document.createElement('img'); + + /** @type {boolean} */ + this._svgImageLoaded = false; /** @type {Array} */ this._size = [0, 0]; @@ -165,7 +168,7 @@ class SVGSkin extends Skin { // Can't use bitwise stuff here because we need to handle negative exponents const mipScale = Math.pow(2, mipLevel - INDEX_OFFSET); - if (this._svgImage.complete && !this._scaledMIPs[mipLevel]) { + if (this._svgImageLoaded && !this._scaledMIPs[mipLevel]) { this._scaledMIPs[mipLevel] = this.createMIP(mipScale); } @@ -191,10 +194,10 @@ class SVGSkin extends Skin { */ setSVG (svgData, rotationCenter) { const svgTag = loadSvgString(svgData); - this._svgImage = document.createElement('img'); const svgText = serializeSvgToString(svgTag, true /* shouldInjectFonts */); + this._svgImageLoaded = false; - this._svgImage.addEventListener('load', () => { + this._svgImage.onload = () => { const {x, y, width, height} = svgTag.viewBox.baseVal; this._size[0] = width; this._size[1] = height; @@ -218,8 +221,10 @@ class SVGSkin extends Skin { this._rotationCenter[0] = rotationCenter[0] - x; this._rotationCenter[1] = rotationCenter[1] - y; + this._svgImageLoaded = true; + this.emit(Skin.Events.WasAltered); - }); + }; this._svgImage.src = `data:image/svg+xml;utf8,${encodeURIComponent(svgText)}`; } From 3b21ee98b658ad819c3440ad132c552780bc78ae Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford <7019101+cwillisf@users.noreply.github.com> Date: Tue, 29 Jun 2021 13:25:20 -0700 Subject: [PATCH 3/3] add comment explaining `onload` choice --- src/SVGSkin.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/SVGSkin.js b/src/SVGSkin.js index 256a784f9..a5e3e50c9 100644 --- a/src/SVGSkin.js +++ b/src/SVGSkin.js @@ -197,6 +197,7 @@ class SVGSkin extends Skin { const svgText = serializeSvgToString(svgTag, true /* shouldInjectFonts */); this._svgImageLoaded = false; + // If there is another load already in progress, replace the old onload to effectively cancel the old load this._svgImage.onload = () => { const {x, y, width, height} = svgTag.viewBox.baseVal; this._size[0] = width;