Set PenSkin silhouette data directly

[adroitwhiz]

Proposed Changes

• Pre-allocate ImageData and an array for gl.readPixels for PenSkins.
• Set PenSkin's Silhouette directly from the skin's ImageData.

Reason for Changes

The previous texture > silhouette pipeline for PenSkin was:

1. gl.readPixels of the PenSkin's framebuffer into a newly created Uint8Array.
2. Create a new ImageData object from the array.
3. Draw the ImageData to the PenSkin's canvas.
4. Pass the canvas to the Silhouette, which:
5. Draws the passed canvas to its own canvas, then
6. Reads its canvas' ImageData and stores it.

The new pipeline is:

1. gl.readPixels of the PenSkin's framebuffer into a previously allocated Uint8Array.
2. Read the array into a previously created ImageData object.
3. Pass the ImageData to the Silhouette, which uses it directly.

By only creating a new array and ImageData object when the PenSkin's canvas size is changed, and passing the Silhouette the ImageData directly, we can greatly improve the speed of silhouette updates as shown in this benchmark:

Latest develop	This PR

[thisandagain]

/cc @mzgoddard @cwillisf @kchadha

[mzgoddard]

This is awesome.

During loading performance optimizations we found that passing an ImageData to a webgl texture instead of a canvas was both much faster and used much less memory. It made sense to add ImageData as an accepted type to update since we were always producing the ImageData outside Silhouette. But we overlooked updating PenSkin when those changes were made.

[mzgoddard]

Yes. We should definitely reuse these.

[mzgoddard]

Its great to see this updated to use the ImageData api change.

[cwillisf]

😎

[cwillisf]

Nit: the math.floor isn't necessary here

[adroitwhiz]

It seems to be used in every other place in the codebase that constructs a Uint8Array, specifically to prevent issues with Edge.

If I removed it here, I don't think it would cause any trouble (width and height should be integers), and newer versions of Edge (I tested it on Microsoft Edge 44.18362.449.0, Microsoft EdgeHTML 18.18363) have no trouble with floating-point values in typed array constructors, but it'd break the convention that the rest of the codebase is using. Is that okay?

(P.S. it may be worth revisiting this in the future by either testing whether Edge 15, the minimum version Scratch officially supports, supports floating-point values in TypedArray constructors, or by bumping the minimum supported Edge version in the FAQ to something newer.)

[cwillisf]

Oops - thanks for the reminder! I had completely forgotten about that.