Set rotation center to [0, 0] in setEmptyImageData

[adroitwhiz]

Resolves

Resolves #562

Proposed Changes

This PR adds logic in Skin.setEmptyImageData to set the Skin's rotation center to [0, 0].

Reason for Changes

Both BitmapSkin and SVGSkin have logic to call setEmptyImageData and return early if they are passed an image whose width or height are 0.

However, because of this early return, their rotation centers are never properly set, and so continue to use their previously set values. The first time you load a project, these will be initialized to [0, 0], but when you, for instance, delete the contents of a previously non-empty costume, they will be set to the previous costume's rotation center. This causes the costume to be fenced in too far.

I'm directly setting the skin's _rotationCenter instead of calling setRotationCenter both to avoid emitting Skin.Events.WasAltered twice, and because I plan to remove setRotationCenter eventually.

[adroitwhiz]

/cc @BryceLTaylor This may fix the fencing issue you encountered.

[fsih]

We are in the process of updating our contributor guidelines to be consistent across repos and more useful. One of the rules we're planning to be stricter about again is that we want all PRs to have issues associated with them. This way, if it's an issue we aren't planning to fix, or one that requires discussion or design from the scratch team, we can close it or add clarification to it before a contributor puts a lot of work into a PR that might end up being turned away. I know the problem in the past has been that people have filed issues and they've been ignored 😓and we're working on having a process where we queue them all up to be reviewed.

With that in mind, could you file an issue for this PR, and I'll assign it back to you?

Thanks again for all the work you put into contributing to Scratch, and especially all the detail you put in your write-ups and reasoning. We really appreciate it!!

[adroitwhiz]

An issue has been created: #562

Thanks for taking the time to review these PRs!

It may be beneficial to clarify the "associated issue" thing in the pull request template, maybe something like:

### Resolves

_Include a link to the GitHub issue that this PR resolves. If an issue does not exist, please create one._

[adroitwhiz]

Also, where would performance improvement PRs fit into this? Oftentimes, the "issue" for those isn't "this operation is particularly slow", just "this operation could be faster", and explaining why it could be faster essentially requires you to implement the optimization in the first place, especially if you're testing it out and don't actually know beforehand whether it'll improve performance.

[fsih]

@adroitwhiz that's a great question! Performance is usually quite difficult to contribute to because it usually requires a deep understanding the whole system, so we weren't really thinking about as we were rewriting our contributor guidelines, which are mostly aimed at newbies. You're right that it would be pretty difficult to predict how much success you would have before you started trying stuff.

We have a performance label in github, so you can see examples of issues we've written for ourselves for performance investigation: https://github.com/LLK/scratch-render/issues?q=is%3Aissue+label%3Aperformance They're usually along the lines of, this looks like it could potentially benefit from investigation, should we do it? We would probably have to go with our and your best guess on which of those kinds of issues would be most worth pursuing.

[fsih]

LG, thanks!