Implementation of the missing CanvasRenderingContext2D functions

[luckcolors]

So here it is as mentioned from #43 . If possible i would like help with reviewing and testing all of the changes ( i'm unfortunately not enough skilled with javascript or gopher js to do that).

I also added the other missing functions and reordered them in categories following Mozila documentation that can be seen here.

Any feedback will also be greatly appreciated. :)

[luckcolors]

This should be complete right now.

[luckcolors]

@dominikh Ping :)

[dominikh]

@shurcooL ping.

[dmitshur]

@luckcolors You haven't applied the changes from my last review on August 22 yet. You said in comments you agreed, but there haven't been any new commits since.

[luckcolors]

AH, sorry this is my first time doing a code review, i thought the "reviews" where automatically applied to the code. I will push the changes as soon as possible. :)

[luckcolors]

While i'm at this again if you want i can figure out the functions i left in the todo.

[dmitshur]

While i'm at this again if you want i can figure out the functions i left in the todo.

Feel free to decide for yourself how much work you're willing to take on. I would suggest leaving those out for a separate PR, if you want.

[dmitshur]

So i think this is now done, unless you have some other doubts or issues you would like to address.

I think this is almost ready to be merged. There are just 2 known issues we need to resolve:

1. GetLineDash() has a compile error. See inline comment above. We need to fix that.
2. I'm still thinking whether it's better to use ints or floats for GetImageData... If I don't have a conclusive answer on this, we can go with what you've chosen as is, and if there are good reasons, we can still change this later. People will be okay with breaking API changes here since it's a new API, and we can't always know the best long term solution before trying it.

Aside from that, I don't see any other issues. Thanks a lot for working on this!

[dmitshur]

I've pushed a fix for GetLineDash build error in df6ab95b9c4396bfdd1c300bca4992016d1ce894, please review @luckcolors.

[dmitshur]

I've done some reading of https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/getImageData and testing GetImageData:

println(ctx.GetImageData(50, 50, 1, 1).Object)
// Output: ImageData {data: Uint8ClampedArray(4), width: 1, height: 1}

println(ctx.GetImageData(50, 50, 1.999, 1).Object)
// Output: ImageData {data: Uint8ClampedArray(4), width: 1, height: 1}

println(ctx.GetImageData(50, 50, 0, 1).Object)
// Output: Failed to execute 'getImageData' on 'CanvasRenderingContext2D': The source width is 0.

println(ctx.GetImageData(50, 50, 0.999, 1).Object)
// Output: Failed to execute 'getImageData' on 'CanvasRenderingContext2D': The source width is 0.

It becomes quite clear that the float parameters are truncated to integers.

These are pixel-manipulation methods. They operate on pixels. A single pixel in ImageData is a 4 uint8 values (R, G, B, A components). It doesn't makes sense to have floats when it comes to precise pixel manipulation, so I'm very confident we must use int here.

Also, ImageData width/height properties are int:

type ImageData struct {
    *js.Object
    Width  int        `js:"width"`
    Height int        `js:"height"`

Not float64.

However, drawImage is different. It doesn't operate on exact pixels, and can accept fractional values. I tested by using a browser zoom of 500%. At that point, each pixel was actually 5 real pixels wide, and fractional values to drawImage had a visible effect.

In conclusion, I'll change all pixel manipulation methods to use int parameters, since they operate on exact pixel data and floating points don't make sense. But the rest are good with float64.

[dmitshur]

I testes the X/Y positions, and can confirm they're truncated as well as width/height:

var canvas = document.GetElementByID("canvas").(*dom.HTMLCanvasElement)
var ctx = canvas.GetContext2d()

ctx.FillStyle = "rgb(12, 34, 45)"
ctx.Rect(0, 0, 100, 100)
ctx.Fill()

println(ctx.GetImageData(50, 99, 1, 1).Data)
println(ctx.GetImageData(50, 99.999, 1, 1).Data)
println(ctx.GetImageData(50, 100, 1, 1).Data)

// Output:
// Uint8ClampedArray(4) [12, 34, 45, 255]
// Uint8ClampedArray(4) [12, 34, 45, 255]
// Uint8ClampedArray(4) [0, 0, 0, 0]

[dmitshur]

putImageData is similar to drawImage, in that it accepts fractional values for the position X/Y of where in the canvas to render image data. However, the dirty parameters can only be integers, since they refer to the pixel-based nature of the ImageData.

[luckcolors]

~~I did some more testing with https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/putImageData and i checked, the dirtyX and dirtyY do change based on the floating part (they don't get truncated), dirtyWidth and dirtHeight get truncated if a floating part exist (as you also verified with getImageData).~~

Ok i rechecked because it would error if any of the values had 0.5 as the floating part, and https://www.w3schools.com/tags/canvas_putimagedata.asp confirms what you found out.

[luckcolors]

I've reviewed the rest of this PR and addressed minor issues I found. This has my LGTM now.

No problem! Thanks to you for your time and patience. This has also my LGTM now, (thanks for spotting those tiny errors).

[dmitshur]

Sure thing, happy to collaborate! Merged. Thank you again @luckcolors!