Bug 52141 - [Chromium] Issue 57432: Layout Test fast/canvas/drawImage-with-negative-source-destination.html failing with --accelerated-2d-canvas
Summary: [Chromium] Issue 57432: Layout Test fast/canvas/drawImage-with-negative-sourc...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-10 00:36 PST by Naoki Takano
Modified: 2011-01-10 19:30 PST (History)
6 users (show)

See Also:


Attachments
Fix shader program (1.22 KB, patch)
2011-01-10 00:36 PST, Naoki Takano
no flags Details | Formatted Diff | Diff
Patch (4.30 KB, patch)
2011-01-10 15:49 PST, Stephen White
jamesr: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Naoki Takano 2011-01-10 00:36:05 PST
Created attachment 78379 [details]
Fix shader program

Issue 57432:	Layout Test fast/canvas/drawImage-with-negative-source-destination.html failing with --accelerated-2d-canvas

Originally the test result was following,

run_webkit_tests --accelerated-2d-canvas fast/canvas/drawImage-with-negative-source-destination.html

Txt diffs:

--- D:\src\chromium1\src\webkit\Release\layout-test-results\fast/canvas/drawImage-with-negative-source-destination-expected.txt 
+++ D:\src\chromium1\src\webkit\Release\layout-test-results\fast/canvas/drawImage-with-negative-source-destination-actual.txt 
@@ -3,8 +3,8 @@
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS imgdata[4] is 0
-PASS imgdata[5] is 255
+FAIL imgdata[4] should be 0. Was 255.
+FAIL imgdata[5] should be 255. Was 0.
 PASS imgdata[6] is 0
 PASS successfullyParsed is true
Comment 1 Naoki Takano 2011-01-10 00:40:50 PST
Could you review my patch?

This is originally reported in Chromium project, but the same error can be shown in WebKit itself.

Thanks,
Comment 2 Adam Barth 2011-01-10 01:05:25 PST
Comment on attachment 78379 [details]
Fix shader program

Thanks for the patch, but all patches need to have a ChangeLog entry that describes why we're making the change.  In the case, you'll want to mention the test that you're fixing and why we should replace texCoord with gl_TexCoord[0].  This page has more detailed information: http://webkit.org/coding/contributing.html
Comment 3 Stephen White 2011-01-10 07:14:09 PST
Comment on attachment 78379 [details]
Fix shader program

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

> WebCore/platform/graphics/gpu/TexShader.cpp:58
> +        "    gl_TexCoord[0] = texMatrix * position;\n"

This fix doesn't make sense to me.  AFAIK, gl_TexCoord is not available in OpenGL ES 2.0 (and deprecated as of desktop GLSL 1.3) and we shouldn't be introducing it into new code.  It's possible that this worked due to some kind of artifact of the GLES2 -> desktop GL translation, but we shouldn't be relying on that.
Comment 4 Stephen White 2011-01-10 15:49:34 PST
Created attachment 78463 [details]
Patch
Comment 5 James Robinson 2011-01-10 15:53:22 PST
Comment on attachment 78463 [details]
Patch

R=me
Comment 6 Naoki Takano 2011-01-10 19:28:54 PST
I see,

Anyway, thank you for your patch!!

It is good study for me;-)
Comment 7 Stephen White 2011-01-10 19:30:06 PST
Committed r75469: <http://trac.webkit.org/changeset/75469>