Bug 48277 - Fix canvas/philip/tests/2d.imageData.get.source.negative
Summary: Fix canvas/philip/tests/2d.imageData.get.source.negative
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
Depends on:
Blocks: 46506
  Show dependency treegraph
Reported: 2010-10-25 16:27 PDT by Mike Lawther
Modified: 2010-10-29 18:10 PDT (History)
4 users (show)

See Also:

Patch (4.59 KB, patch)
2010-10-25 17:08 PDT, Mike Lawther
no flags Details | Formatted Diff | Diff
Patch (4.89 KB, patch)
2010-10-29 15:18 PDT, Mike Lawther
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Lawther 2010-10-25 16:27:05 PDT
Update behaviour as per http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#pixel-manipulation: 
"The getImageData(sx, sy, sw, sh) method must return an ImageData object representing the underlying pixel data for the area of the canvas denoted by the rectangle whose corners are the four points (sx, sy), (sx+sw, sy), (sx+sw, sy+sh), (sx, sy+sh), in canvas coordinate space units."
Comment 1 Mike Lawther 2010-10-25 16:28:40 PDT
Added block on master bug
Comment 2 Mike Lawther 2010-10-25 17:08:52 PDT
Created attachment 71821 [details]
Comment 3 Mike Lawther 2010-10-25 17:25:32 PDT
Added jamesr to cc list.
Comment 4 Matthew Delaney 2010-10-25 17:48:54 PDT
FWIW, this patch looks good to me. Though, does it end up making 2d.imageData.get.source.negative pass entirely on Mac? I note on my machine that the test fails 3 pixels of the imgdata1 and one of the imgdata2's pixels. Off the top of my head, I would guess that that imgdata2's pixel still fails.
Comment 5 Mike Lawther 2010-10-25 18:06:08 PDT
I tested it on my Mac in Safari, and on Chromium Windows, and it passed entirely on both. run-webkit-tests also reported it passing correctly. 

Running the test in a current Chrome, I get:

Failed assertion imgdata1.data["0"] === 255 (got 0[number], expected 255[number])
Failed assertion imgdata1.data["1"] === 255 (got 0[number], expected 255[number])
Failed assertion imgdata1.data["2"] === 255 (got 0[number], expected 255[number])
Failed assertion imgdata2.data["3"] === 0 (got 255[number], expected 0[number])

But running it in a patched Safari, I get it saying 'Passed', as per the expectations.
Comment 6 Matthew Delaney 2010-10-25 19:33:42 PDT
Ok great. Make sure then to remove the test from the LayoutTests/platform/mac/Skipped list and that it still passes from run-webkit-tests.

I had a patch out to unskip all the tests and reset the expectations, but it had some troubles with the bots and I need to resubmit. That should hopefully clear up future confusions about whether these tests are passing/failing and fixed or not.
Comment 7 Mike Lawther 2010-10-25 19:42:09 PDT
Yep - the patch does remove the test from LayoutTests/platform/mac/Skipped (and also from chromium's test_expectations.txt, since I tested that platform too).
Comment 8 Mike Lawther 2010-10-26 22:01:58 PDT
Adding Oliver to cc: 

Oliver: could you take a look at this patch? Thanks!
Comment 9 James Robinson 2010-10-27 11:24:57 PDT
Comment on attachment 71821 [details]

R=me.  Thanks!
Comment 10 WebKit Commit Bot 2010-10-27 11:40:29 PDT
Comment on attachment 71821 [details]

Rejecting patch 71821 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'land-attachment', '--force-clean', '--ignore-builders', '--non-interactive', '--parent-command=commit-queue', 71821]" exit_code: 1
Logging in as commit-queue@webkit.org...
Fetching: https://bugs.webkit.org/attachment.cgi?id=71821&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=48277&ctype=xml
Processing 1 patch from 1 bug.
Cleaning working directory
Updating working directory
Processing patch 71821 from bug 48277.
ERROR: /Projects/CommitQueue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/4746045
Comment 11 Mike Lawther 2010-10-29 15:18:17 PDT
Created attachment 72402 [details]
Comment 12 Matthew Delaney 2010-10-29 15:32:45 PDT
Since I landed the patch to (finally) stop skipping all those (including this) canvas tests on mac, if this patch fixes the mac failing expectations then you'll need to update that file as well. It's located at LayoutTests/platform/mac-snowleopard/canvas/philip…

The mac-leopard skipped list currently contains this test so there's no need to update those expectations. Though, if this certainly fixes the problem on both snowleopard and leopard (likely the case) you can just get the expectation (of passing) file into LayoutTests/platform/mac/canvas/philip… in which case it'll apple to both leopard and snowleopard.
Comment 13 Mike Lawther 2010-10-29 15:36:16 PDT
The patch does remove the snowleopard expectations file, since I can confirm that the test passes on snowleopard. I can't see why it wouldn't work on leopard, but since I didn't test it, I haven't touched those.
Comment 14 Matthew Delaney 2010-10-29 15:57:19 PDT
Are you on IRC? I can't find you. I can explain the Skipped lists in more detail.

Basically, if you have a fix (presumably) for all the ports, you'll just want to reset the project-wide expectations file (to now be passing) and then get rid of each port's expectation file if they have one. That way none of the test bots for those ports will show red on that test if the patch did indeed fix the bug.
Comment 15 WebKit Commit Bot 2010-10-29 17:13:25 PDT
Comment on attachment 72402 [details]

Clearing flags on attachment: 72402

Committed r70957: <http://trac.webkit.org/changeset/70957>
Comment 16 WebKit Commit Bot 2010-10-29 17:13:31 PDT
All reviewed patches have been landed.  Closing bug.
Comment 17 Mike Lawther 2010-10-29 18:10:52 PDT
Followup patch to remove entries in other ports Skipped files: https://bugs.webkit.org/show_bug.cgi?id=48702