The call to svg-renderer._draw in SVGSkin.setSVG isn't necessary

[fsih]

During SVGSkin.setSVG, we call
this._svgRenderer.fromString(svgData, 1, () => {...})
which triggers a call to svg-renderer._draw at a texture scale of 1 (hard-coded).

Milliseconds later, as a part of vm's runtime._step method, we call draw on all drawables, which also calls svg-renderer._draw, this time at the correct texture scale (one that takes into account the device pixel density.) That call to _draw immediately replaces the previous texture.

Here are the 2 stack traces

This should mean that the first call to _draw with scale 1 is unnecessary. However, setSVG is only happening whenever a user action happens in the paint editor right now, which is not very often (and so probably not a high priority).

[adroitwhiz]

This call appears to be necessary because of the issue described in #509. Essentially, since Skin.Events.WasAltered is emitted when setSVG is called, the drawable's matrix is recalculated. If there's no properly-sized SVG, the old costume appears stretched to the new costume's size for however many frames it takes the new costume to render.

[fsih]

I talked to @cwillisf, and _drawThese is the function that is actually drawing to the stage, which should mean that even if setSVG changes the texture and matrix, the changes aren't applied until the call to _drawThese (which will create a texture at the right scale, and never use the previous texture made in setSVG)

[adroitwhiz]

_drawThese calls getTexture, but the function inside SVGSkins' getTexture to draw the new SVG is asynchronous--it sets the texture after the callback fires. The matrix update is synchronous, however. If the _draw call is removed, here's what happens (I've tested it):

1. setSVG is called, loading the new SVG data into the skin's _svgRenderer and updating the skin's size.
2. WasAltered is emitted, marking the skin's Drawables' matrices as "dirty".
3. Sometime later, drawThese is called, which calls getTexture and getUniforms.
4. getTexture executes, which calls _svgRenderer._draw. However, since that method is asynchronous, the skin's texture is not actually updated.
5. getUniforms is called, which synchronously updates the Drawables' matrices. This is the point where the old texture is stretched to the size of the new texture, as the new matrices are calculated based on the new skin size.
6. A couple frames later, the _svgRenderer._draw callback fires, and everything looks correct again.

I have a branch which fixes this by moving the emission of the WasAltered event inside the _svgRenderer._draw callback, but this creates issues when stamping sprites which have been hidden when the project is loaded.

In such a case, setSVG is called when the project is loaded, but the SVG is never rendered because the _svgRenderer.fromString call has been removed, and the sprite is hidden and thus getTexture is never called.

The first time the SVG renderer is told to render an image, it will create an <img> element, and set its src to the SVG string. This is an asynchronous process, and will result in callbacks being delayed by a couple frames.

For every render after that, however, it skips this asynchronous step as the <img> has already been created, and calls the callback immediately.

This is what allows hidden sprites which have never been rendered to currently stamp properly--getTexture sets the texture synchronously if the SVG renderer has already rendered the SVG once before.

By removing the fromString call from setSVG, it never creates this <img> element, so the first time the renderer attempts to draw the skin, the SVG-drawing callback will fire asynchronously.

Fixing this involves moving the SVG renderer's creation of an <img> into the loadString method.

I made two new PRs (scratchfoundation/scratch-svg-renderer#107 and #522) that fix this.