Defend renderer against NaN or non-number sprite size

[towerofnix]

Related to leopard-js/leopard-mentors#4. Resolves #105.

The main substantive change here is that Renderer.renderSprite will now always treat sprites with NaN or non-number size as being 100%. This isn't "right" per se - "set size to NaN" is equal to "set size to 0" - but sprites should never be size NaN anyway, and I figure "treating as 100%" is more clearly-broken behavior than "treating as literally invisible". (Matching Scratch exactly would mean setting the size to the minimum valid size for that sprite's current costume, which the renderer isn't suited to handle on its own.)

Secondary changes include warnings in the SpeechBubbleSkin.getTexture and VectorSkin.getTexture interfaces, which expect a non-NaN scale, and some light refactoring for VectorSkin._getMipmap and Renderer.renderSprite.

This doesn't fix the case project (leopard-js/leopard-mentors#4), but it does clarify the error from a generic WebGL: INVALID_VALUE: texImage2D: no canvas into Expected a number, sprite TiledChevron size is NaN. Treating as 100%..

[PullJosh]

I will be gone for a few days for the 4th of July so if @adroitwhiz is happy with this (or any other PR) feel free to merge. Otherwise I'll look into it when I return. :)

[PullJosh]

Still on vacation but I have some time at a coffee shop. I created a test project to try this out, and I'm still experiencing some issues.

When I set the sprite size to NaN (0 / 0), I get the following console warning, which is correct.

But the sprite becomes invisible, as if it's being rendered at 0% as opposed to the promised 100%.

Additionally, setting the sprite size to Infinity and -Infinity is broken. (Actually, setting the sprite size to be any super large finite number causes the project to crash in Chrome on my Mac.) I know dealing with Infinity wasn't originally the goal of this PR, but it feels like maybe a good thing to do now? But if that belongs in a separate PR that's also fine.

[towerofnix]

Thanks for checking that!!

Okay, so it turns out the issue with NaN is that we actually perform the sprite.size / 100 operation in two places. I haven't asserted this but I believe both are key to rendering and they really should stay in sync with each other.

So we moved the math for getting a sprite size into Drawable (and tidied up its logic a little), in a public getSpriteScale function, which is so-named based on the const spriteScale = spr.size / 100 lines (in both Renderer and Drawable) that it effectively replaces. That's now reused where we'd first inserted the logic (in Renderer).

This makes NaN actually render as 100%. Thanks for the demo project catching it!

[towerofnix]

I don't think we can super reasonably handle setting the sprite size to massive values in this PR, because that's part of the "cap to maximum and minimum sizes" logic that would belong as its own thing — but -Infinity flat-out causes the same sort of error as NaN, yet doesn't get caught. We'll address that for sure!

And, I think it's reasonable to, within the same function, banish any negative number. Negative size is completely meaningless in Scratch. In Leopard it doesn't appear to "break" - the sprite just goes invisible - but it's not rendering the speech bubble either, which is weird.

Here's our thought:

• Change NaN to act as 0% instead of 100%. Again, playing different from Scratch doesn't really make sense. Although Scratch's renderer probably never interacts with broken-size sprites, we may as well model off the closest thing, which is the "set size to NaN %" block - and this behaves the same as "set size to 0%". We can still print a warning. (When we implement minimum/maximum sizes, user code should literally never run into this warning.)
• Make all negative numbers act as 0% (also with a warning). This includes -Infinity.
• Don't hard-code a (constant) maximum size in the renderer. As you saw, 9999% is already enough to cause problems... yet that's a reasonable size for an utterly minuscule costume, so we'd best not just code a limit that doesn't exist in Scratch. The size limiting code will take care of determining the appropriate maximum size for e.g. the Scratch cat, and prevent the crashes you can run into now.

Edit: ^ These updated checks are implemented in the commits below!

[towerofnix]

We've followed up with an issue for handling minimum/maximum size: #202

[PullJosh]

Tested and behavior looks great 👍

It seems like @adroitwhiz has code review covered, so I'm happy to merge once everyone else is happy.