Revert "Revert "Put Skin Alter Push Back In""

Revert "Put Skin Alter Push Back In"

src/BitmapSkin.js

@@ -23,6 +23,12 @@ class BitmapSkin extends Skin {
 
         /** @type {Array<int>} */
         this._textureSize = [0, 0];
+
+        /**
+         * The "native" size, in texels, of this skin.
+         * @type {Array<number>}
+         */
+        this.size = [0, 0];
     }
 
     /**
@@ -43,13 +49,6 @@ class BitmapSkin extends Skin {
         return true;
     }
 
-    /**
-     * @return {Array<number>} the "native" size, in texels, of this skin.
-     */
-    get size () {
-        return [this._textureSize[0] / this._costumeResolution, this._textureSize[1] / this._costumeResolution];
-    }
-
     /**
      * @param {Array<number>} 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);

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

src/PenSkin.js

@@ -88,6 +88,9 @@ class PenSkin extends Skin {
         /** @type {HTMLCanvasElement} */
         this._canvas = document.createElement('canvas');
 
+        /** @type {Array<number>} */
+        this._canvasSize = twgl.v3.create();
+
         /** @type {WebGLTexture} */
         this._texture = null;
 
@@ -165,7 +168,7 @@ class PenSkin extends Skin {
      * @return {Array<number>} 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;

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

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<number>}
+         */
+        this.size = [0, 0];
+
+        /**
+         * The viewbox offset of the svg.
+         * @type {Array<number>}
+         */
+        this._viewOffset = [0, 0];
+
+        /**
+         * The rotation center before offset by _viewOffset.
+         * @type {Array<number>}
+         */
+        this._rawRotationCenter = [NaN, NaN];
     }
 
     /**
@@ -43,21 +61,17 @@ class SVGSkin extends Skin {
         super.dispose();
     }
 
-    /**
-     * @return {Array<number>} 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,17 @@ 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.
+        this.size = this._svgRenderer.size;
+        this._viewOffset = this._svgRenderer.viewOffset;
+        // Reset rawRotationCenter when we update viewOffset. The rotation
+        // center used to render will be updated later.
+        this._rawRotationCenter = [NaN, NaN];
+
+        this._svgRenderer._draw(1, () => {
             const gl = this._renderer.gl;
             this._textureScale = this._maxTextureScale = 1;
 
@@ -134,7 +158,8 @@ class SVGSkin extends Skin {
             }
 
             if (typeof rotationCenter === 'undefined') rotationCenter = this.calculateRotationCenter();
-            this.setRotationCenter.apply(this, rotationCenter);
+            this.setRotationCenter(rotationCenter[0], rotationCenter[1]);
+
             this.emit(Skin.Events.WasAltered);
         });
     }

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<number>}
+         */
+
         /**
          * 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<number>} 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.

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;

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