From 2a1d215e5040476387ffc7a4e09458ecb65fda46 Mon Sep 17 00:00:00 2001 From: Karishma Chadha Date: Thu, 12 Sep 2019 13:43:32 -0400 Subject: [PATCH 1/2] Revert "Revert "Revert "Revert "Skin alter push"""" --- src/BitmapSkin.js | 14 +++++----- src/Drawable.js | 9 ------- src/PenSkin.js | 23 +++++++++-------- src/RenderWebGL.js | 19 ++++++++++++++ src/SVGSkin.js | 48 ++++++++++++++++++++++++++--------- src/Skin.js | 15 +++++------ test/fixtures/MockSkinPool.js | 35 +++++++++++++++++++++++++ test/unit/DrawableTests.js | 7 +++++ 8 files changed, 124 insertions(+), 46 deletions(-) create mode 100644 test/fixtures/MockSkinPool.js diff --git a/src/BitmapSkin.js b/src/BitmapSkin.js index 94f2984..13d17bd 100644 --- a/src/BitmapSkin.js +++ b/src/BitmapSkin.js @@ -23,6 +23,12 @@ class BitmapSkin extends Skin { /** @type {Array} */ this._textureSize = [0, 0]; + + /** + * The "native" size, in texels, of this skin. + * @type {Array} + */ + this.size = [0, 0]; } /** @@ -43,13 +49,6 @@ class BitmapSkin extends Skin { return true; } - /** - * @return {Array} the "native" size, in texels, of this skin. - */ - get size () { - return [this._textureSize[0] / this._costumeResolution, this._textureSize[1] / this._costumeResolution]; - } - /** * @param {Array} scale - The scaling factors to be used. * @return {WebGLTexture} The GL texture representation of this skin when drawing at the given scale. @@ -110,6 +109,7 @@ class BitmapSkin extends Skin { // Do these last in case any of the above throws an exception this._costumeResolution = costumeResolution || 2; this._textureSize = BitmapSkin._getBitmapSize(bitmapData); + this.size = [this._textureSize[0] / this._costumeResolution, this._textureSize[1] / this._costumeResolution]; if (typeof rotationCenter === 'undefined') rotationCenter = this.calculateRotationCenter(); this.setRotationCenter.apply(this, rotationCenter); diff --git a/src/Drawable.js b/src/Drawable.js index cf00d3f..786dcac 100644 --- a/src/Drawable.js +++ b/src/Drawable.js @@ -3,7 +3,6 @@ const twgl = require('twgl.js'); const Rectangle = require('./Rectangle'); const RenderConstants = require('./RenderConstants'); const ShaderManager = require('./ShaderManager'); -const Skin = require('./Skin'); const EffectTransform = require('./EffectTransform'); /** @@ -101,8 +100,6 @@ class Drawable { /** @todo move convex hull functionality, maybe bounds functionality overall, to Skin classes */ this._convexHullPoints = null; this._convexHullDirty = true; - - this._skinWasAltered = this._skinWasAltered.bind(this); } /** @@ -141,13 +138,7 @@ class Drawable { */ set skin (newSkin) { if (this._skin !== newSkin) { - if (this._skin) { - this._skin.removeListener(Skin.Events.WasAltered, this._skinWasAltered); - } this._skin = newSkin; - if (this._skin) { - this._skin.addListener(Skin.Events.WasAltered, this._skinWasAltered); - } this._skinWasAltered(); } } diff --git a/src/PenSkin.js b/src/PenSkin.js index 1e500ba..273cac1 100644 --- a/src/PenSkin.js +++ b/src/PenSkin.js @@ -88,6 +88,9 @@ class PenSkin extends Skin { /** @type {HTMLCanvasElement} */ this._canvas = document.createElement('canvas'); + /** @type {Array} */ + this._canvasSize = twgl.v3.create(); + /** @type {WebGLTexture} */ this._texture = null; @@ -165,7 +168,7 @@ class PenSkin extends Skin { * @return {Array} the "native" size, in texels, of this skin. [width, height] */ get size () { - return [this._canvas.width, this._canvas.height]; + return this._canvasSize; } /** @@ -188,13 +191,13 @@ class PenSkin extends Skin { clear () { const gl = this._renderer.gl; twgl.bindFramebufferInfo(gl, this._framebuffer); - + /* Reset framebuffer to transparent black */ gl.clearColor(0, 0, 0, 0); gl.clear(gl.COLOR_BUFFER_BIT); const ctx = this._canvas.getContext('2d'); - ctx.clearRect(0, 0, this._canvas.width, this._canvas.height); + ctx.clearRect(0, 0, this._canvasSize[0], this._canvasSize[1]); this._silhouetteDirty = true; } @@ -451,7 +454,7 @@ class PenSkin extends Skin { * @param {number} x - centered at x * @param {number} y - centered at y */ - _drawRectangle (currentShader, texture, bounds, x = -this._canvas.width / 2, y = this._canvas.height / 2) { + _drawRectangle (currentShader, texture, bounds, x = -this._canvasSize[0] / 2, y = this._canvasSize[1] / 2) { const gl = this._renderer.gl; const projection = twgl.m4.ortho( @@ -514,7 +517,7 @@ class PenSkin extends Skin { * @param {number} x - texture centered at x * @param {number} y - texture centered at y */ - _drawToBuffer (texture = this._texture, x = -this._canvas.width / 2, y = this._canvas.height / 2) { + _drawToBuffer (texture = this._texture, x = -this._canvasSize[0] / 2, y = this._canvasSize[1] / 2) { if (texture !== this._texture && this._canvasDirty) { this._drawToBuffer(); } @@ -528,7 +531,7 @@ class PenSkin extends Skin { gl.texImage2D(gl.TEXTURE_2D, 0, gl.RGBA, gl.RGBA, gl.UNSIGNED_BYTE, this._canvas); const ctx = this._canvas.getContext('2d'); - ctx.clearRect(0, 0, this._canvas.width, this._canvas.height); + ctx.clearRect(0, 0, this._canvasSize[0], this._canvasSize[1]); this._canvasDirty = false; } @@ -564,8 +567,8 @@ class PenSkin extends Skin { this._bounds = new Rectangle(); this._bounds.initFromBounds(width / 2, width / -2, height / 2, height / -2); - this._canvas.width = width; - this._canvas.height = height; + this._canvas.width = this._canvasSize[0] = width; + this._canvas.height = this._canvasSize[1] = height; this._rotationCenter[0] = width / 2; this._rotationCenter[1] = height / 2; @@ -651,8 +654,8 @@ class PenSkin extends Skin { this._renderer.enterDrawRegion(this._toBufferDrawRegionId); // Sample the framebuffer's pixels into the silhouette instance - const skinPixels = new Uint8Array(Math.floor(this._canvas.width * this._canvas.height * 4)); - gl.readPixels(0, 0, this._canvas.width, this._canvas.height, gl.RGBA, gl.UNSIGNED_BYTE, skinPixels); + const skinPixels = new Uint8Array(Math.floor(this._canvasSize[0] * this._canvasSize[1] * 4)); + gl.readPixels(0, 0, this._canvasSize[0], this._canvasSize[1], gl.RGBA, gl.UNSIGNED_BYTE, skinPixels); const skinCanvas = this._canvas; skinCanvas.width = bounds.width; diff --git a/src/RenderWebGL.js b/src/RenderWebGL.js index 5277235..4186b11 100644 --- a/src/RenderWebGL.js +++ b/src/RenderWebGL.js @@ -3,6 +3,7 @@ const EventEmitter = require('events'); const hull = require('hull.js'); const twgl = require('twgl.js'); +const Skin = require('./Skin'); const BitmapSkin = require('./BitmapSkin'); const Drawable = require('./Drawable'); const Rectangle = require('./Rectangle'); @@ -291,6 +292,20 @@ class RenderWebGL extends EventEmitter { this.emit(RenderConstants.Events.NativeSizeChanged, {newSize: this._nativeSize}); } + /** + * Notify Drawables whose skin is the skin that changed. + * @param {Skin} skin - the skin that changed. + * @private + */ + _skinWasAltered (skin) { + for (let i = 0; i < this._allDrawables.length; i++) { + const drawable = this._allDrawables[i]; + if (drawable && drawable._skin === skin) { + drawable._skinWasAltered(); + } + } + } + /** * Create a new bitmap skin from a snapshot of the provided bitmap data. * @param {ImageData|HTMLImageElement|HTMLCanvasElement|HTMLVideoElement} bitmapData - new contents for this skin. @@ -303,6 +318,7 @@ class RenderWebGL extends EventEmitter { const skinId = this._nextSkinId++; const newSkin = new BitmapSkin(skinId, this); newSkin.setBitmap(bitmapData, costumeResolution, rotationCenter); + newSkin.addListener(Skin.Events.WasAltered, this._skinWasAltered.bind(this, newSkin)); this._allSkins[skinId] = newSkin; return skinId; } @@ -318,6 +334,7 @@ class RenderWebGL extends EventEmitter { const skinId = this._nextSkinId++; const newSkin = new SVGSkin(skinId, this); newSkin.setSVG(svgData, rotationCenter); + newSkin.addListener(Skin.Events.WasAltered, this._skinWasAltered.bind(this, newSkin)); this._allSkins[skinId] = newSkin; return skinId; } @@ -329,6 +346,7 @@ class RenderWebGL extends EventEmitter { createPenSkin () { const skinId = this._nextSkinId++; const newSkin = new PenSkin(skinId, this); + newSkin.addListener(Skin.Events.WasAltered, this._skinWasAltered.bind(this, newSkin)); this._allSkins[skinId] = newSkin; return skinId; } @@ -345,6 +363,7 @@ class RenderWebGL extends EventEmitter { const skinId = this._nextSkinId++; const newSkin = new TextBubbleSkin(skinId, this); newSkin.setTextBubble(type, text, pointsLeft); + newSkin.addListener(Skin.Events.WasAltered, this._skinWasAltered.bind(this, newSkin)); this._allSkins[skinId] = newSkin; return skinId; } diff --git a/src/SVGSkin.js b/src/SVGSkin.js index 90e3908..90852ca 100644 --- a/src/SVGSkin.js +++ b/src/SVGSkin.js @@ -30,6 +30,24 @@ class SVGSkin extends Skin { /** @type {Number} */ this._maxTextureScale = 0; + + /** + * The natural size, in Scratch units, of this skin. + * @type {Array} + */ + this.size = [0, 0]; + + /** + * The viewbox offset of the svg. + * @type {Array} + */ + this._viewOffset = [0, 0]; + + /** + * The rotation center before offset by _viewOffset. + * @type {Array} + */ + this._rawRotationCenter = [NaN, NaN]; } /** @@ -43,21 +61,17 @@ class SVGSkin extends Skin { super.dispose(); } - /** - * @return {Array} the natural size, in Scratch units, of this skin. - */ - get size () { - return this._svgRenderer.size; - } - /** * Set the origin, in object space, about which this Skin should rotate. * @param {number} x - The x coordinate of the new rotation center. * @param {number} y - The y coordinate of the new rotation center. */ setRotationCenter (x, y) { - const viewOffset = this._svgRenderer.viewOffset; - super.setRotationCenter(x - viewOffset[0], y - viewOffset[1]); + if (x !== this._rawRotationCenter[0] || y !== this._rawRotationCenter[1]) { + this._rawRotationCenter[0] = x; + this._rawRotationCenter[1] = y; + super.setRotationCenter(x - this._viewOffset[0], y - this._viewOffset[1]); + } } /** @@ -100,7 +114,19 @@ class SVGSkin extends Skin { * @fires Skin.event:WasAltered */ setSVG (svgData, rotationCenter) { - this._svgRenderer.fromString(svgData, 1, () => { + this._svgRenderer.loadString(svgData); + + // Size must be updated synchronously because the VM sets the costume's `size` immediately after calling this. + // TODO: add either a callback to this function so the costume size can be set after this is done, + // or something in the VM to handle setting the costume size when Skin.Events.WasAltered is emitted. + this.size = this._svgRenderer.size; + if (typeof rotationCenter === 'undefined') rotationCenter = this.calculateRotationCenter(); + this._viewOffset = this._svgRenderer.viewOffset; + // Reset rawRotationCenter when we update viewOffset. + this._rawRotationCenter = [NaN, NaN]; + this.setRotationCenter(rotationCenter[0], rotationCenter[1]); + + this._svgRenderer._draw(1, () => { const gl = this._renderer.gl; this._textureScale = this._maxTextureScale = 1; @@ -133,8 +159,6 @@ class SVGSkin extends Skin { this._maxTextureScale = testScale; } - if (typeof rotationCenter === 'undefined') rotationCenter = this.calculateRotationCenter(); - this.setRotationCenter.apply(this, rotationCenter); this.emit(Skin.Events.WasAltered); }); } diff --git a/src/Skin.js b/src/Skin.js index 473c70b..8bd89d6 100644 --- a/src/Skin.js +++ b/src/Skin.js @@ -33,6 +33,13 @@ class Skin extends EventEmitter { /** @type {Vec3} */ this._rotationCenter = twgl.v3.create(0, 0); + /** + * The "native" size, in texels, of this skin. + * @member size + * @abstract + * @type {Array} + */ + /** * The uniforms to be used by the vertex and pixel shaders. * Some of these are used by other parts of the renderer as well. @@ -97,14 +104,6 @@ class Skin extends EventEmitter { return this._rotationCenter; } - /** - * @abstract - * @return {Array} the "native" size, in texels, of this skin. - */ - get size () { - return [0, 0]; - } - /** * Set the origin, in object space, about which this Skin should rotate. * @param {number} x - The x coordinate of the new rotation center. diff --git a/test/fixtures/MockSkinPool.js b/test/fixtures/MockSkinPool.js new file mode 100644 index 0000000..208d933 --- /dev/null +++ b/test/fixtures/MockSkinPool.js @@ -0,0 +1,35 @@ +const Skin = require('../../src/Skin'); + +class MockSkinPool { + constructor () { + this._allDrawables = []; + } + + static forDrawableSkin (drawable) { + const pool = new MockSkinPool(); + pool.addDrawable(drawable); + pool.addSkin(drawable.skin); + return pool; + } + + _skinWasAltered (skin) { + for (let i = 0; i < this._allDrawables.length; i++) { + const drawable = this._allDrawables[i]; + if (drawable && drawable._skin === skin) { + drawable._skinWasAltered(); + } + } + } + + addDrawable (drawable) { + this._allDrawables.push(drawable); + return drawable; + } + + addSkin (skin) { + skin.addListener(Skin.Events.WasAltered, this._skinWasAltered.bind(this, skin)); + return skin; + } +} + +module.exports = MockSkinPool; diff --git a/test/unit/DrawableTests.js b/test/unit/DrawableTests.js index f0f163b..235e3d3 100644 --- a/test/unit/DrawableTests.js +++ b/test/unit/DrawableTests.js @@ -8,6 +8,7 @@ global.document = { const Drawable = require('../../src/Drawable'); const MockSkin = require('../fixtures/MockSkin'); +const MockSkinPool = require('../fixtures/MockSkinPool'); const Rectangle = require('../../src/Rectangle'); /** @@ -31,6 +32,7 @@ test('translate by position', t => { const drawable = new Drawable(); drawable.skin = new MockSkin(); drawable.skin.size = [200, 50]; + MockSkinPool.forDrawableSkin(drawable); expected.initFromBounds(0, 200, -50, 0); t.same(snapToNearest(drawable.getAABB()), expected); @@ -47,6 +49,7 @@ test('translate by costume center', t => { const drawable = new Drawable(); drawable.skin = new MockSkin(); drawable.skin.size = [200, 50]; + MockSkinPool.forDrawableSkin(drawable); drawable.skin.setRotationCenter(1, 0); expected.initFromBounds(-1, 199, -50, 0); @@ -64,6 +67,7 @@ test('translate and rotate', t => { const drawable = new Drawable(); drawable.skin = new MockSkin(); drawable.skin.size = [200, 50]; + MockSkinPool.forDrawableSkin(drawable); drawable.updateProperties({position: [1, 2], direction: 0}); expected.initFromBounds(1, 51, 2, 202); @@ -90,6 +94,7 @@ test('rotate by non-right-angles', t => { drawable.skin = new MockSkin(); drawable.skin.size = [10, 10]; drawable.skin.setRotationCenter(5, 5); + MockSkinPool.forDrawableSkin(drawable); expected.initFromBounds(-5, 5, -5, 5); t.same(snapToNearest(drawable.getAABB()), expected); @@ -106,6 +111,7 @@ test('scale', t => { const drawable = new Drawable(); drawable.skin = new MockSkin(); drawable.skin.size = [200, 50]; + MockSkinPool.forDrawableSkin(drawable); drawable.updateProperties({scale: [100, 50]}); expected.initFromBounds(0, 200, -25, 0); @@ -128,6 +134,7 @@ test('rotate and scale', t => { const drawable = new Drawable(); drawable.skin = new MockSkin(); drawable.skin.size = [100, 1000]; + MockSkinPool.forDrawableSkin(drawable); drawable.skin.setRotationCenter(50, 50); expected.initFromBounds(-50, 50, -950, 50); From 110371b029f456f5151dbb2ebed2db9d155d9cf3 Mon Sep 17 00:00:00 2001 From: "Michael \"Z\" Goddard" Date: Mon, 16 Sep 2019 17:22:27 -0400 Subject: [PATCH 2/2] synchronously store SVGRenderer props used in SVGSkin Some APIs need SVGSkin to update some of its values immediately when changing the SVG. Namely size needs to be synchronously set. However rotation center doesn't need to be set at that time so we can leave it to update where it currently is. --- src/SVGSkin.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/SVGSkin.js b/src/SVGSkin.js index 90852ca..7f49090 100644 --- a/src/SVGSkin.js +++ b/src/SVGSkin.js @@ -116,15 +116,13 @@ class SVGSkin extends Skin { setSVG (svgData, rotationCenter) { this._svgRenderer.loadString(svgData); - // Size must be updated synchronously because the VM sets the costume's `size` immediately after calling this. - // TODO: add either a callback to this function so the costume size can be set after this is done, - // or something in the VM to handle setting the costume size when Skin.Events.WasAltered is emitted. + // Size must be updated synchronously because the VM sets the costume's + // `size` immediately after calling this. this.size = this._svgRenderer.size; - if (typeof rotationCenter === 'undefined') rotationCenter = this.calculateRotationCenter(); this._viewOffset = this._svgRenderer.viewOffset; - // Reset rawRotationCenter when we update viewOffset. + // Reset rawRotationCenter when we update viewOffset. The rotation + // center used to render will be updated later. this._rawRotationCenter = [NaN, NaN]; - this.setRotationCenter(rotationCenter[0], rotationCenter[1]); this._svgRenderer._draw(1, () => { const gl = this._renderer.gl; @@ -159,6 +157,9 @@ class SVGSkin extends Skin { this._maxTextureScale = testScale; } + if (typeof rotationCenter === 'undefined') rotationCenter = this.calculateRotationCenter(); + this.setRotationCenter(rotationCenter[0], rotationCenter[1]); + this.emit(Skin.Events.WasAltered); }); }