fix a crash in getImageDate if the rectangle is outside the canvas

[phipla]

Thanks for contributing!

• Have you updated CHANGELOG.md?

Fixes #2024

[phipla]

The tests failing seem related to the issue addressed by #2133 . Once this PR is rebased on #2133 the tests pass.

[phipla]

Rebased

[chearon]

Looks good, thanks. Can you fix the conflicts? "Allow edits by maintainers" is off.

[chearon]

(while you're at it, typo in the commit summary: s/getImageDate/getImageData)

[LinusU]

Sorry for being a bit late to this.

I think that this change is adding behaviour that is not in line with how canvas should work?

While it is of course great that we are fixing a crash, I think it would be better to throw an "not implemented" error rather than returning an incorrectly sized ImageData? Or am I missing something? 🤔

[zbjornson]

@LinusU is correct, the spec is sort of surprising here:

Pixels outside the canvas must be returned as transparent black.

See #1849 for more info and link to the spec.

@phipla are you up for trying to implement the standard behavior?

[phipla]

You are right (it was the existing behavior of the lib when the rectangle was partly outside, and I wanted to just fix the crash, not alter the existing behavior).

However I can alter it to comply with the standard. I will update the PR in a few days.

[phipla]

@zbjornson @LinusU Any input on the requested changes? This PR should now fix issues #2024 (crash) and #1849 (non-compliant behavior when retrieving data partly or completely outside the canvas)

[chearon]

@phipla wonderful job on the tests here. Sorry it took so long to get this merged, but I have done so: pushed a0c8031 to master since I can't write to this branch. I didn't make any changes to the logic, just:

• I removed all whitespace / indent changes so it was easier to review (related drive-by: added a goto instead of the if indent around the switch). I couldn't find any logic errors, and again, the tests look awesome
• Squashed everything
• Fixed conflicts