Merge pull request #946 from adroitwhiz/fix-0x0

Fix SVG costume dimensions appearing as 0x0 in GUI

Author: cwillisf

src/SVGSkin.js

@@ -196,12 +196,17 @@ class SVGSkin extends Skin {
         const svgText = serializeSvgToString(svgTag, true /* shouldInjectFonts */);
         this._svgImageLoaded = false;
 
+        const {x, y, width, height} = svgTag.viewBox.baseVal;
+        // While we're setting the size before the image is loaded, this doesn't cause the skin to appear with the wrong
+        // size for a few frames while the new image is loading, because we don't emit the `WasAltered` event, telling
+        // drawables using this skin to update, until the image is loaded.
+        // We need to do this because the VM reads the skin's `size` directly after calling `setSVG`.
+        // TODO: return a Promise so that the VM can read the skin's `size` after the image is loaded.
+        this._size[0] = width;
+        this._size[1] = height;
+
         // 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;
-            this._size[1] = height;
-
             if (width === 0 || height === 0) {
                 super.setEmptyImageData();
                 return;

test/integration/skin-size-tests.js

@@ -0,0 +1,61 @@
+/* global render, ImageData */
+const {chromium} = require('playwright-chromium');
+const test = require('tap').test;
+const path = require('path');
+
+const indexHTML = path.resolve(__dirname, 'index.html');
+
+// immediately invoked async function to let us wait for each test to finish before starting the next.
+(async () => {
+    const browser = await chromium.launch();
+    const page = await browser.newPage();
+
+    await page.goto(`file://${indexHTML}`);
+
+    await test('SVG skin size set properly', async t => {
+        t.plan(1);
+        const skinSize = await page.evaluate(() => {
+            const skinID = render.createSVGSkin(`<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 50 100"></svg>`);
+            return render.getSkinSize(skinID);
+        });
+        t.same(skinSize, [50, 100]);
+    });
+
+    await test('Bitmap skin size set correctly', async t => {
+        t.plan(1);
+        const skinSize = await page.evaluate(() => {
+            // Bitmap costumes are double resolution, so double the ImageData size
+            const skinID = render.createBitmapSkin(new ImageData(100, 200), 2);
+            return render.getSkinSize(skinID);
+        });
+        t.same(skinSize, [50, 100]);
+    });
+
+    await test('Pen skin size set correctly', async t => {
+        t.plan(1);
+        const skinSize = await page.evaluate(() => {
+            const skinID = render.createPenSkin();
+            return render.getSkinSize(skinID);
+        });
+        const nativeSize = await page.evaluate(() => render.getNativeSize());
+        t.same(skinSize, nativeSize);
+    });
+
+    await test('Text bubble skin size set correctly', async t => {
+        t.plan(1);
+        const skinSize = await page.evaluate(() => {
+            const skinID = render.createTextSkin('say', 'Hello', false);
+            return render.getSkinSize(skinID);
+        });
+        // The subtleties in font rendering may cause the size of the text bubble to vary, so just make sure it's not 0
+        t.notSame(skinSize, [0, 0]);
+    });
+
+    // close the browser window we used
+    await browser.close();
+})().catch(err => {
+    // Handle promise rejections by exiting with a nonzero code to ensure that tests don't erroneously pass
+    // eslint-disable-next-line no-console
+    console.error(err.message);
+    process.exit(1);
+});