Bug 52141

Summary: [Chromium] Issue 57432: Layout Test fast/canvas/drawImage-with-negative-source-destination.html failing with --accelerated-2d-canvas
Product: WebKit Reporter: Naoki Takano <honten>
Component: CanvasAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, honten, jamesr, kbr, mdelaney7, senorblanco
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Fix shader program
none
Patch jamesr: review+

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>