Bug 84006 - support canvas.resetTransform()
Summary: support canvas.resetTransform()
Status: RESOLVED CONFIGURATION CHANGED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Lu Guanqun
URL:
Keywords:
Depends on:
Blocks: 82797 110001
  Show dependency treegraph
 
Reported: 2012-04-15 20:22 PDT by Lu Guanqun
Modified: 2017-07-07 14:28 PDT (History)
10 users (show)

See Also:


Attachments
Patch (7.45 KB, patch)
2012-04-15 20:24 PDT, Lu Guanqun
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-03 (6.23 MB, application/zip)
2012-04-15 21:49 PDT, WebKit Review Bot
no flags Details
Patch (7.91 KB, patch)
2012-04-15 21:57 PDT, Lu Guanqun
adele: review-
simon.fraser: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Lu Guanqun 2012-04-15 20:22:53 PDT
Support canvas.resetTransform() interface.
Comment 1 Lu Guanqun 2012-04-15 20:24:32 PDT
Created attachment 137269 [details]
Patch
Comment 2 WebKit Review Bot 2012-04-15 21:49:25 PDT
Comment on attachment 137269 [details]
Patch

Attachment 137269 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12408859

New failing tests:
platform/chromium/virtual/gpu/fast/canvas/canvas-resetTransform.html
fast/canvas/canvas-resetTransform.html
Comment 3 WebKit Review Bot 2012-04-15 21:49:31 PDT
Created attachment 137276 [details]
Archive of layout-test-results from ec2-cr-linux-03

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-03  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 4 Lu Guanqun 2012-04-15 21:57:39 PDT
Created attachment 137278 [details]
Patch
Comment 5 Alexey Proskuryakov 2012-04-16 09:11:55 PDT
Comment on attachment 137278 [details]
Patch

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

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:670
> +    state().m_transform = AffineTransform();

Shouldn't this be makeIdentity()?
Comment 6 Dean Jackson 2012-04-16 12:11:10 PDT
Comment on attachment 137278 [details]
Patch

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

> LayoutTests/fast/canvas/script-tests/canvas-resetTransform.js:15
> +ctx.beginPath();
> +ctx.scale(0.5, 0.5);
> +ctx.resetTransform();
> +ctx.fillStyle = 'green';
> +ctx.fillRect(0, 0, 100, 100);
> +
> +var imageData = ctx.getImageData(1, 1, 98, 98);
> +var imgdata = imageData.data;
> +shouldBe("imgdata[4]", "0");
> +shouldBe("imgdata[5]", "128");
> +shouldBe("imgdata[6]", "0");

I would expect this test to draw two identical rects with different colours, one with a scale and one after the resetTransform, and test both corners of the imagedata. Am I missing something?

And maybe have some debug() statements to explain what's going on.

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:668
> +    if (!ctm.isInvertible())
> +        return;

Is this behaviour explained anywhere? I can understand why the other transformation functions have it, but in this case we're trying to reset the transform to identity, so I don't know why it is should fail if there is a currently un-invertable state.

>> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:670
>> +    state().m_transform = AffineTransform();
> 
> Shouldn't this be makeIdentity()?

Agreed.
Comment 7 Adele Peterson 2012-04-19 16:15:23 PDT
Comment on attachment 137278 [details]
Patch

r- until Dean and Alexey's comments are addressed.
Comment 8 Joseph Pecoraro 2017-07-07 14:28:30 PDT
It seems we already support resetTransform. Lets close this out.