Bug 77550 - [CHROMIUM/SKIA] canvas/philip/tests/2d.gradient.interpolate.colouralpha.html fails in GPU
Summary: [CHROMIUM/SKIA] canvas/philip/tests/2d.gradient.interpolate.colouralpha.html ...
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Brian Salomon
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-01 06:45 PST by Brian Salomon
Modified: 2013-04-09 13:14 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.96 KB, patch)
2012-08-28 07:29 PDT, Brian Salomon
no flags Details | Formatted Diff | Diff
Patch (2.00 KB, patch)
2012-08-30 11:02 PDT, Brian Salomon
no flags Details | Formatted Diff | Diff
Patch (1.97 KB, patch)
2012-09-10 10:49 PDT, Brian Salomon
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brian Salomon 2012-02-01 06:45:38 PST
This test passes fails in the GPU Skia backend when we do the getImageData unpremultiply step on the GPU.

The test draws a gradient from c1=(255, 255, 0, 0) to c2=(0, 0, 255, 255) on a x = 0 ... 100 rectangle. It samples the rect at x = 25, 50, and 75. At each sample it asserts that the read value's color components are within three of the expected value.

The color expected is (191, 191, 63, 63) which roughly corresponds to .75*c1 + .25*c2 = (191.25, 191.25, 63.75, 63.75). Skia computes the same premultiplied value at the sample point in both the GPU and CPU backends (48, 48, 17, 65). Unpremultiplying gives approximately (188.31, 188.31, 66.69, 65).

In the code path where the unpremultiply step is done on the CPU in WebKit code the final value is (188, 188, 66, 65) which barely passes. When the GPU unpremultiply conversion is used Skia gets (188, 188, 67, 65) and so fails by the blue component.

Though the GPU conversion pushes us over the edge to failing the test, it isn't the fundamental reason why we fail. The gradient spans the range x = 0 to 100 but Skia samples it at pixel centers falling halfway between integer coords. So we are sampling the gradient at x = 0.5, 1.5,..., 99.5. When we read back the 25th pixel we're getting Skia's computation of .255 * c1 + .745 * c2. The exact answer would be (189.975, 189.75, 65.25, 65.25).

If the reference color was the exact color at 25.5 rounded it would be (190, 190, 65, 65) and Skia would pass. Is the current reference color incorrect? Is Skia computing the gradient incorrectly? I took a look at the canvas spec and didn't see anything that clarified this issue.
Comment 1 Stephen White 2012-07-22 09:34:10 PDT
This seems to have been fixed by the skia roll 4647-4683, most likely by Riley's linear gradient change in https://code.google.com/p/skia/source/detail?r=4680.
Comment 2 Brian Salomon 2012-08-28 07:29:43 PDT
Reopening to attach new patch.
Comment 3 Brian Salomon 2012-08-28 07:29:45 PDT
Created attachment 160972 [details]
Patch
Comment 4 Brian Salomon 2012-08-28 07:30:19 PDT
It turns out that this was not really fixed. In the range of the Skia roll where this started passing TomH made a changesto the shader generator which inadvertently broke the gpu-based unpremul<->premul conversions. We fell back to SW conversions and went back to barely passing the test because the conversions come out slightly different. The next Skia roll fixes the gpu-based conversions and so this test (along with canvas-fillPath-shadow.html which was also removed in r123297) will fail once again. The attached patch reverts the removal of these tests from TE.
Comment 5 Stephen White 2012-08-28 07:55:53 PDT
Comment on attachment 160972 [details]
Patch

Darn.  r=me
Comment 6 Brian Salomon 2012-08-30 07:02:58 PDT
Comment on attachment 160972 [details]
Patch

I held off on c? so that it wouldn't hit before the skia roll makes it to WK DEPS (and get reverted due to unexpected pass) but that has happened. So I think this is good to go.
Comment 7 WebKit Review Bot 2012-08-30 07:14:43 PDT
Comment on attachment 160972 [details]
Patch

Rejecting attachment 160972 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: /mnt/git/webkit-commit-queue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/13685753
Comment 8 Brian Salomon 2012-08-30 11:02:46 PDT
Created attachment 161517 [details]
Patch
Comment 9 Brian Salomon 2012-08-30 11:03:19 PDT
Comment on attachment 161517 [details]
Patch

I messed up the Reviewed By line in the original patch. :(
Comment 10 Brian Salomon 2012-09-10 10:49:06 PDT
Created attachment 163164 [details]
Patch
Comment 11 Stephen White 2012-09-10 11:24:38 PDT
Comment on attachment 163164 [details]
Patch

OK, bots willing.  r=me.
Comment 12 WebKit Review Bot 2012-09-10 12:44:07 PDT
Comment on attachment 163164 [details]
Patch

Rejecting attachment 163164 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
): Merge conflict in LayoutTests/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 Javascript in foreground tabs should not wait synchronously for plug-ins to load

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 164.

Full output: http://queues.webkit.org/results/13810328