[webkit-reviews] review granted: [Bug 174278] Add CanvasRenderingContext2D::getTransform : [Attachment 315035] Patch

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has granted Devin Rousso <drousso at apple.com>'s
request for review:
Bug 174278: Add CanvasRenderingContext2D::getTransform
https://bugs.webkit.org/show_bug.cgi?id=174278

Attachment 315035: Patch

https://bugs.webkit.org/attachment.cgi?id=315035&action=review




--- Comment #30 from Dean Jackson <dino at apple.com> ---
Comment on attachment 315035
  --> https://bugs.webkit.org/attachment.cgi?id=315035
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=315035&action=review

> LayoutTests/fast/canvas/2d.getTransform.modification.html:22
> +ctx.setTransform(1, 2, 3, 4, 5, 6);
> +shouldBe("copiedTransform.a", "1");
> +shouldBe("copiedTransform.b", "0");
> +shouldBe("copiedTransform.c", "0");
> +shouldBe("copiedTransform.d", "1");
> +shouldBe("copiedTransform.e", "0");
> +shouldBe("copiedTransform.f", "0");
> +
> +copiedTransform.translate(-1, -2);
> +var newTransform = ctx.getTransform();

How about another test that transforms the context directly, then checks to
make sure the original getTransform hasn't changed?

> LayoutTests/fast/canvas/2d.getTransform.modification.html:29
> +

Also a test that gets a transform, manipulates it, then does setTransform, then
get to make sure it worked.

Bonus, you could do it with two contexts. One you change via
getTransform/setTransform, the other directly, then compare getTransform on
both at the end.

> LayoutTests/fast/canvas/2d.setTransform.matrix.html:18
> +shouldEvaluateTo("originalTransform.a", 1);
> +shouldEvaluateTo("originalTransform.b", 0);
> +shouldEvaluateTo("originalTransform.c", 0);
> +shouldEvaluateTo("originalTransform.d", 1);
> +shouldEvaluateTo("originalTransform.e", 0);
> +shouldEvaluateTo("originalTransform.f", 0);
> +

Make a helper function?

shouldHaveMatrixEquality(originalTransform, [1, 0, 0, 1, 0, 0])

> LayoutTests/fast/canvas/2d.setTransform.matrix.html:45
> +ctx.setTransform(new DOMMatrix([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13,
14, 15, 16]));
> +var newTransformFrom3dDOMMatrix = ctx.getTransform();
> +shouldEvaluateTo("newTransformFrom3dDOMMatrix.a", 1);
> +shouldEvaluateTo("newTransformFrom3dDOMMatrix.b", 2);
> +shouldEvaluateTo("newTransformFrom3dDOMMatrix.c", 5);
> +shouldEvaluateTo("newTransformFrom3dDOMMatrix.d", 6);
> +shouldEvaluateTo("newTransformFrom3dDOMMatrix.e", 13);
> +shouldEvaluateTo("newTransformFrom3dDOMMatrix.f", 14);
> +

Is this enough to check that the 3d bits were dropped?

> LayoutTests/fast/canvas/2d.setTransform.matrix.html:66
> +shouldThrowErrorName(function() {
> +    ctx.setTransform({a: 1, m11: 11, b: 2, m12: 12, c: 3, m21: 21, d: 4,
m22: 22, e: 5, m41: 41, f: 6, m42: 42});
> +}, "TypeError");

Add

ctx.setTransform(1)
ctx.setTransform({a: {}})
ctx.setTransform({a: "hi"})
etc